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
> -----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
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