net/mlx5: fix debug variable initialization
PATCH 1/1 net/mlx5: fix debug variable initialization
Use __rte_unused macro to mask a debug variable instead of #ifdef. This way, if the variable is used in a macro, it does not depend on how the macro is expanded.
--- drivers/net/mlx5/mlx5_flow_flex.c | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-)
drivers/net/mlx5/mlx5_flow_flex.c +1 -3
@@ -222,9 +222,7 @@ mlx5_flex_flow_translate_item(struct rte_eth_dev *dev,const struct rte_flow_item *item, bool is_inner) { -#ifdef RTE_LIBRTE_MLX5_DEBUG -struct mlx5_priv *priv = dev->data->dev_private; -#endif + __rte_unused struct mlx5_priv *priv = dev->data->dev_private;const struct rte_flow_item_flex *spec, *mask;void *misc4_m =MLX5_ADDR_OF (fte_match_param, matcher, misc_parameters_4);
Signature · 1 line
2.33.1
Quoted 15 lines · Gregory Etelson
Use
__rte_unusedmacro to mask a debug variable instead of#ifdef. This way, if the variable is used in a macro, it does not depend on how the macro is expanded.Fixes 91f0e029ce5f ("net/mlx5: translate flex item pattern into matcher")Signed-off-by Gregory Etelson <[email protected]>--- drivers/net/mlx5/mlx5_flow_flex.c | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-)drivers/net/mlx5/mlx5_flow_flex.c +0
-0
drivers/net/mlx5/mlx5_flow_flex.c mlx5_flex_flow_translate_item(struct rte_eth_dev *dev, · line 225
Quoted 7 lines
drivers/net/mlx5/mlx5_flow_flex.c +1
-3@@ -222,9 +222,7 @@ mlx5_flex_flow_translate_item(struct rte_eth_dev *dev,const struct rte_flow_item *item, bool is_inner) { -#ifdef RTE_LIBRTE_MLX5_DEBUG -struct mlx5_priv *priv = dev->data->dev_private; -#endif + __rte_unused struct mlx5_priv *priv = dev->data->dev_private;
Marking the variable as unused to silence compiler for some cases, but it is indeed used, what about something like this (inspired by sfc code):
#if defined(RTE_LIBRTE_MLX5_DEBUG) || defined(RTE_ENABLE_ASSERT)
struct mlx5_priv *priv = dev->data->dev_private;
#endif
Quoted 4 lines
drivers/net/mlx5/mlx5_flow_flex.c +0
-0const struct rte_flow_item_flex *spec, *mask;void *misc4_m =MLX5_ADDR_OF (fte_match_param, matcher, misc_parameters_4);
Hello Ferruh,
I posted updated patch in https://patchwork.dpdk.org/project/dpdk/patch/[email protected]/
Regards,
Gregory
Quoted 53 lines · Gregory Etelson
Quoted 1 line
Use
__rte_unusedmacro to mask a debugvariable instead of
#ifdef.Quoted 1 line
This way, if the variable is used in a macro, it
does not depend on
Quoted 3 lines
how the macro is expanded.
Fixes 91f0e029ce5f ("net/mlx5: translate flexitem pattern into matcher")
Quoted 1 line
Signed-off-by Gregory EtelsonReview
Quoted 5 lines
--- drivers/net/mlx5/mlx5_flow_flex.c | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-)(quoted diff) +0
-0
b/drivers/net/mlx5/mlx5_flow_flex.cdrivers/net/mlx5/mlx5_flow_flex.c
Quoted 4 lines
index
bdfa383c45..be0f9821ce100644drivers/net/mlx5/mlx5_flow_flex.c +0
-0@@ -222,9 +222,7 @@mlx5_flex_flow_translate_item(struct
rte_eth_dev*dev,drivers/net/mlx5/mlx5_flow_flex.c line 222
Quoted 1 line
drivers/net/mlx5/mlx5_flow_flex.c +0
-0const struct rte_flow_item*item,
drivers/net/mlx5/mlx5_flow_flex.c mlx5_flex_flow_translate_item(struct rte_eth_dev *dev,
Quoted 7 lines
drivers/net/mlx5/mlx5_flow_flex.c +1
-3bool is_inner) { -#ifdef RTE_LIBRTE_MLX5_DEBUG -struct mlx5_priv *priv = dev->data-dev_private ; -#endif + __rte_unused struct mlx5_priv *priv =
dev->data->dev_private;Marking the variable as unused to silence
compiler for some cases,
but it is indeed used, what about something like
this (inspired by
sfc code):#if
defined(RTE_LIBRTE_MLX5_DEBUG)||defined(RTE_ENABLE_ASSERT)struct mlx5_priv *priv = dev->data-drivers/net/mlx5/mlx5_flow_flex.c line 227
Quoted 1 line
drivers/net/mlx5/mlx5_flow_flex.c +0
-0dev_private;#endif
drivers/net/mlx5/mlx5_flow_flex.c line 228
Quoted 1 line
drivers/net/mlx5/mlx5_flow_flex.c +0
-0const struct rte_flow_item_flex *spec,*mask;
Quoted 1 line
void *misc4_m =MLX5_ADDR_OF(fte_match_param, matcher,
Quoted 2 lines
misc_parameters_4);
PATCH v2 1/1 net/mlx5: fix debug variable initialization
In reply to Gregory EtelsonMLX5_ASSERT macro expansion depends on RTE_LIBRTE_MLX5_DEBUG and RTE_ENABLE_ASSERT.
Existing implementation ignored the RTE_ENABLE_ASSERT dependency in the mlx5_flex_flow_translate_item() scope.
As the result, the priv variable was not defined.
--- drivers/net/mlx5/mlx5_flow_flex.c | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-)
drivers/net/mlx5/mlx5_flow_flex.c +1 -4
@@ -222,9 +222,6 @@ mlx5_flex_flow_translate_item(struct rte_eth_dev *dev, const struct rte_flow_item *item, bool is_inner) { -#ifdef RTE_LIBRTE_MLX5_DEBUG -struct mlx5_priv *priv = dev->data->dev_private; -#endif const struct rte_flow_item_flex *spec, *mask;void *misc4_m =MLX5_ADDR_OF (fte_match_param, matcher, misc_parameters_4);
@@ -237,7 +234,7 @@ mlx5_flex_flow_translate_item(struct rte_eth_dev *dev, spec = item->spec; mask = item->mask; tp = (struct mlx5_flex_item *)spec->handle; -MLX5_ASSERT (mlx5_flex_index (priv, tp) >=0 ); +MLX5_ASSERT (mlx5_flex_index (dev->data->dev_private, tp) >=0 );for (i =0 ; i < tp->mapnum; i++) {struct mlx5_flex_pattern_field *map = tp->map + i;uint32_t id = map->reg_id;
Signature · 1 line
2.33.1
Hi,
Quoted 9 lines
-----Original Message-----
From: Gregory Etelson <[email protected]>
Sent: Thursday, November 4, 2021 3:29 PM
To: [email protected]; Gregory Etelson <[email protected]>; Slava Ovsiienko <[email protected]>
Cc: [email protected]; Raslan Darawsheh <[email protected]>; Matan Azrad <[email protected]>
Subject: [PATCH v2] net/mlx5: fix debug variable initialization
Quoted 8 lines
MLX5_ASSERTmacro expansion depends onRTE_LIBRTE_MLX5_DEBUGandRTE_ENABLE_ASSERT.
Existing implementation ignored theRTE_ENABLE_ASSERTdependency in themlx5_flex_flow_translate_item()scope.
As the result, theprivvariable was not defined.Fixes 91f0e029ce5f ("net/mlx5: translate flex item pattern into matcher")Signed-off-by Gregory Etelson <[email protected]>
Squashed into relevant commit in next-net-mlx,
Kindest regards,
Raslan Darawsheh