net/mlx5: save meter index instead of meter id

Message ID 1587869486-60185-1-git-send-email-suanmingm@mellanox.com (mailing list archive)
State Accepted, archived
Delegated to: Raslan Darawsheh
Headers
Series net/mlx5: save meter index instead of meter id |

Checks

Context Check Description
ci/checkpatch success coding style OK
ci/iol-nxp-Performance success Performance Testing PASS
ci/iol-intel-Performance success Performance Testing PASS
ci/Intel-compilation fail Compilation issues
ci/iol-testing fail Testing issues
ci/iol-mellanox-Performance success Performance Testing PASS

Commit Message

Suanming Mou April 26, 2020, 2:51 a.m. UTC
  Currently, while creating the flow with meter, meter id is saved to the
rte flow. While destroying the flow, the meter object will be found by
the meter id, so the meter object will be released accordingly. But as
the meter id is configured by user, while the meter id is set to 0, it
doesn't make any sense to flow destroy since 0 means flow doesn't have
meter. The meter object with id 0 will be leaked.

As meter object is allocated from indexed memory, and the index starts
from 1, save the internal generated index instead of user defined meter
id will never meet the issue as above.

This patch saves meter index instead of meter id in rte flow.

Signed-off-by: Suanming Mou <suanmingm@mellanox.com>
---
 drivers/net/mlx5/mlx5_flow_dv.c | 8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)
  

Comments

Slava Ovsiienko April 28, 2020, 8:02 a.m. UTC | #1
> -----Original Message-----
> From: Suanming Mou <suanmingm@mellanox.com>
> Sent: Sunday, April 26, 2020 5:51
> To: Matan Azrad <matan@mellanox.com>; Shahaf Shuler
> <shahafs@mellanox.com>; Slava Ovsiienko <viacheslavo@mellanox.com>
> Cc: dev@dpdk.org; Raslan Darawsheh <rasland@mellanox.com>
> Subject: [PATCH] net/mlx5: save meter index instead of meter id
> 
> Currently, while creating the flow with meter, meter id is saved to the rte
> flow. While destroying the flow, the meter object will be found by the meter
> id, so the meter object will be released accordingly. But as the meter id is
> configured by user, while the meter id is set to 0, it doesn't make any sense
> to flow destroy since 0 means flow doesn't have meter. The meter object
> with id 0 will be leaked.
> 
> As meter object is allocated from indexed memory, and the index starts from
> 1, save the internal generated index instead of user defined meter id will
> never meet the issue as above.
> 
> This patch saves meter index instead of meter id in rte flow.
> 
> Signed-off-by: Suanming Mou <suanmingm@mellanox.com>
Acked-by: Viacheslav Ovsiienko <viacheslavo@mellanox.com>

> ---
>  drivers/net/mlx5/mlx5_flow_dv.c | 8 +++++---
>  1 file changed, 5 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/net/mlx5/mlx5_flow_dv.c
> b/drivers/net/mlx5/mlx5_flow_dv.c index 6263ecc..2fdd403 100644
> --- a/drivers/net/mlx5/mlx5_flow_dv.c
> +++ b/drivers/net/mlx5/mlx5_flow_dv.c
> @@ -7867,11 +7867,12 @@ struct field_modify_info modify_tcp[] = {
>  						NULL,
>  						"meter not found "
>  						"or invalid parameters");
> -				flow->meter = fm->meter_id;
> +				flow->meter = fm->idx;
>  			}
>  			/* Set the meter action. */
>  			if (!fm) {
> -				fm = mlx5_flow_meter_find(priv, flow-
> >meter);
> +				fm = mlx5_ipool_get(priv->sh->ipool
> +						[MLX5_IPOOL_MTR], flow-
> >meter);
>  				if (!fm)
>  					return rte_flow_error_set(error,
>  						rte_errno,
> @@ -8591,7 +8592,8 @@ struct field_modify_info modify_tcp[] = {
>  	if (flow->meter) {
>  		struct mlx5_flow_meter *fm;
> 
> -		fm  = mlx5_flow_meter_find(priv, flow->meter);
> +		fm = mlx5_ipool_get(priv->sh->ipool[MLX5_IPOOL_MTR],
> +				    flow->meter);
>  		if (fm)
>  			mlx5_flow_meter_detach(fm);
>  		flow->meter = 0;
> --
> 1.8.3.1
  
Raslan Darawsheh April 29, 2020, 12:48 p.m. UTC | #2
Hi,

> -----Original Message-----
> From: Suanming Mou <suanmingm@mellanox.com>
> Sent: Sunday, April 26, 2020 5:51 AM
> To: Matan Azrad <matan@mellanox.com>; Shahaf Shuler
> <shahafs@mellanox.com>; Slava Ovsiienko <viacheslavo@mellanox.com>
> Cc: dev@dpdk.org; Raslan Darawsheh <rasland@mellanox.com>
> Subject: [PATCH] net/mlx5: save meter index instead of meter id
> 
> Currently, while creating the flow with meter, meter id is saved to the
> rte flow. While destroying the flow, the meter object will be found by
> the meter id, so the meter object will be released accordingly. But as
> the meter id is configured by user, while the meter id is set to 0, it
> doesn't make any sense to flow destroy since 0 means flow doesn't have
> meter. The meter object with id 0 will be leaked.
> 
> As meter object is allocated from indexed memory, and the index starts
> from 1, save the internal generated index instead of user defined meter
> id will never meet the issue as above.
> 
> This patch saves meter index instead of meter id in rte flow.
> 
> Signed-off-by: Suanming Mou <suanmingm@mellanox.com>
> ---
>  drivers/net/mlx5/mlx5_flow_dv.c | 8 +++++---
>  1 file changed, 5 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/net/mlx5/mlx5_flow_dv.c
> b/drivers/net/mlx5/mlx5_flow_dv.c
> index 6263ecc..2fdd403 100644
> --- a/drivers/net/mlx5/mlx5_flow_dv.c
> +++ b/drivers/net/mlx5/mlx5_flow_dv.c
> @@ -7867,11 +7867,12 @@ struct field_modify_info modify_tcp[] = {
>  						NULL,
>  						"meter not found "
>  						"or invalid parameters");
> -				flow->meter = fm->meter_id;
> +				flow->meter = fm->idx;
>  			}
>  			/* Set the meter action. */
>  			if (!fm) {
> -				fm = mlx5_flow_meter_find(priv, flow-
> >meter);
> +				fm = mlx5_ipool_get(priv->sh->ipool
> +						[MLX5_IPOOL_MTR], flow-
> >meter);
>  				if (!fm)
>  					return rte_flow_error_set(error,
>  						rte_errno,
> @@ -8591,7 +8592,8 @@ struct field_modify_info modify_tcp[] = {
>  	if (flow->meter) {
>  		struct mlx5_flow_meter *fm;
> 
> -		fm  = mlx5_flow_meter_find(priv, flow->meter);
> +		fm = mlx5_ipool_get(priv->sh->ipool[MLX5_IPOOL_MTR],
> +				    flow->meter);
>  		if (fm)
>  			mlx5_flow_meter_detach(fm);
>  		flow->meter = 0;
> --
> 1.8.3.1

Patch applied to next-net-mlx,

Kindest regards,
Raslan Darawsheh
  

Patch

diff --git a/drivers/net/mlx5/mlx5_flow_dv.c b/drivers/net/mlx5/mlx5_flow_dv.c
index 6263ecc..2fdd403 100644
--- a/drivers/net/mlx5/mlx5_flow_dv.c
+++ b/drivers/net/mlx5/mlx5_flow_dv.c
@@ -7867,11 +7867,12 @@  struct field_modify_info modify_tcp[] = {
 						NULL,
 						"meter not found "
 						"or invalid parameters");
-				flow->meter = fm->meter_id;
+				flow->meter = fm->idx;
 			}
 			/* Set the meter action. */
 			if (!fm) {
-				fm = mlx5_flow_meter_find(priv, flow->meter);
+				fm = mlx5_ipool_get(priv->sh->ipool
+						[MLX5_IPOOL_MTR], flow->meter);
 				if (!fm)
 					return rte_flow_error_set(error,
 						rte_errno,
@@ -8591,7 +8592,8 @@  struct field_modify_info modify_tcp[] = {
 	if (flow->meter) {
 		struct mlx5_flow_meter *fm;
 
-		fm  = mlx5_flow_meter_find(priv, flow->meter);
+		fm = mlx5_ipool_get(priv->sh->ipool[MLX5_IPOOL_MTR],
+				    flow->meter);
 		if (fm)
 			mlx5_flow_meter_detach(fm);
 		flow->meter = 0;