[dpdk-dev] [PATCH 1/2] net/mlx5: fix port action assert timing

[dpdk-dev] [PATCH 1/2] net/mlx5: fix port action assert timing

[Bing Zhao]

After memory optimization, some action object handles are changed to
index to save the overhead. Assertion in debug mode will be helpful
for trouble shooting.
In the current implementation, only one port action is supported in
switchdev mode for one device flow. In debug mode, an assertion will
be used to check the if the port action is none, and it should
locate before the port action resource registration but not after
it. The action index in the handle should be 0 before registration.
Or else it will always cause a failure because the port action is
registered and the index is not 0.

Fixes: f3faf9e ("net/mlx5: convert port id action to indexed")
Cc: suanmingm@mellanox.com

Signed-off-by: Bing Zhao <bingz@mellanox.com>
Reviewed-by: Matan Azrad <matan@mellanox.com>
Reviewed-by: Suanming Mou <suanmingm@mellanox.com>
Acked-by: Viacheslav Ovsiienko <viacheslavo@mellanox.com>
---
 drivers/net/mlx5/mlx5_flow_dv.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/mlx5/mlx5_flow_dv.c b/drivers/net/mlx5/mlx5_flow_dv.c
index 71da5fb..45fa60c 100644
--- a/drivers/net/mlx5/mlx5_flow_dv.c
+++ b/drivers/net/mlx5/mlx5_flow_dv.c
@@ -7834,10 +7834,10 @@ struct field_modify_info modify_tcp[] = {
 				return -rte_errno;
 			memset(&port_id_resource, 0, sizeof(port_id_resource));
 			port_id_resource.port_id = port_id;
+			MLX5_ASSERT(!handle->rix_port_id_action);
 			if (flow_dv_port_id_action_resource_register
 			    (dev, &port_id_resource, dev_flow, error))
 				return -rte_errno;
-			MLX5_ASSERT(!handle->rix_port_id_action);
 			dev_flow->dv.actions[actions_n++] =
 					dev_flow->dv.port_id_action->action;
 			action_flags |= MLX5_FLOW_ACTION_PORT_ID;
-- 
1.8.3.1

[dpdk-dev] [PATCH 2/2] net/mlx5: fix port action resource initialization

[Bing Zhao]

After memory optimization, the organization of some resources are
changed from pointer based LIST to the index based ILIST. A lot of
code parts are touched due to such change.
Some static code checking and analysis tool will complain and raise
a false warning on the uninitialized value using. E.g. in the port
action registering function, the stack variable will be used as the
right value with some uninitialized field to initialize variable
allocated from heap. But indeed, it is not an error because all the
fileds set with the uninitialized value will be overwritten in the
following code part and the macros. All the fields will be used as
the left value explicitly.
It makes no sense to clear the stack variable to 0 in this case,
and the extra memset will introduce some cycles overhead. It just
needs to ignore the false warning from the tool, if any.

Fixes: f3faf9e ("net/mlx5: convert port id action to indexed")
Cc: suanmingm@mellanox.com

Signed-off-by: Bing Zhao <bingz@mellanox.com>
Reviewed-by: Matan Azrad <matan@mellanox.com>
Reviewed-by: Suanming Mou <suanmingm@mellanox.com>
Acked-by: Viacheslav Ovsiienko <viacheslavo@mellanox.com>
---
 drivers/net/mlx5/mlx5_flow_dv.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/drivers/net/mlx5/mlx5_flow_dv.c b/drivers/net/mlx5/mlx5_flow_dv.c
index 45fa60c..7fe4438 100644
--- a/drivers/net/mlx5/mlx5_flow_dv.c
+++ b/drivers/net/mlx5/mlx5_flow_dv.c
@@ -7832,7 +7832,6 @@ struct field_modify_info modify_tcp[] = {
 			if (flow_dv_translate_action_port_id(dev, action,
 							     &port_id, error))
 				return -rte_errno;
-			memset(&port_id_resource, 0, sizeof(port_id_resource));
 			port_id_resource.port_id = port_id;
 			MLX5_ASSERT(!handle->rix_port_id_action);
 			if (flow_dv_port_id_action_resource_register
-- 
1.8.3.1

Re: [dpdk-dev] [PATCH 1/2] net/mlx5: fix port action assert timing

[Raslan Darawsheh]

Hi,

> -----Original Message-----
> From: Bing Zhao <bingz@mellanox.com>
> Sent: Friday, May 15, 2020 2:45 PM
> To: Slava Ovsiienko <viacheslavo@mellanox.com>; Raslan Darawsheh
> <rasland@mellanox.com>
> Cc: Ori Kam <orika@mellanox.com>; Matan Azrad <matan@mellanox.com>;
> dev@dpdk.org; Suanming Mou <suanmingm@mellanox.com>
> Subject: [PATCH 1/2] net/mlx5: fix port action assert timing
> 
> After memory optimization, some action object handles are changed to
> index to save the overhead. Assertion in debug mode will be helpful
> for trouble shooting.
> In the current implementation, only one port action is supported in
> switchdev mode for one device flow. In debug mode, an assertion will
> be used to check the if the port action is none, and it should
> locate before the port action resource registration but not after
> it. The action index in the handle should be 0 before registration.
> Or else it will always cause a failure because the port action is
> registered and the index is not 0.
> 
> Fixes: f3faf9e ("net/mlx5: convert port id action to indexed")
> Cc: suanmingm@mellanox.com
> 
> Signed-off-by: Bing Zhao <bingz@mellanox.com>
> Reviewed-by: Matan Azrad <matan@mellanox.com>
> Reviewed-by: Suanming Mou <suanmingm@mellanox.com>
> Acked-by: Viacheslav Ovsiienko <viacheslavo@mellanox.com>
> ---
>  drivers/net/mlx5/mlx5_flow_dv.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/net/mlx5/mlx5_flow_dv.c
> b/drivers/net/mlx5/mlx5_flow_dv.c
> index 71da5fb..45fa60c 100644
> --- a/drivers/net/mlx5/mlx5_flow_dv.c
> +++ b/drivers/net/mlx5/mlx5_flow_dv.c
> @@ -7834,10 +7834,10 @@ struct field_modify_info modify_tcp[] = {
>  				return -rte_errno;
>  			memset(&port_id_resource, 0,
> sizeof(port_id_resource));
>  			port_id_resource.port_id = port_id;
> +			MLX5_ASSERT(!handle->rix_port_id_action);
>  			if (flow_dv_port_id_action_resource_register
>  			    (dev, &port_id_resource, dev_flow, error))
>  				return -rte_errno;
> -			MLX5_ASSERT(!handle->rix_port_id_action);
>  			dev_flow->dv.actions[actions_n++] =
>  					dev_flow->dv.port_id_action-
> >action;
>  			action_flags |= MLX5_FLOW_ACTION_PORT_ID;
> --
> 1.8.3.1


Series applied to next-net-mlx,

Kindest regards,
Raslan Darawsheh