patches for DPDK stable branches
 help / color / mirror / Atom feed
* [dpdk-dev] [PATCH v2 1/2] net/mlx5: fix use after free when releasing tx queues
       [not found] <cover.1661223500.git.wangyunjian@huawei.com>
@ 2022-08-23  6:45 ` Yunjian Wang
  2022-09-23  9:31   ` wangyunjian
  2022-08-23  6:46 ` [dpdk-dev] [PATCH v2 2/2] net/mlx5: fix resource leak when releasing a drop action Yunjian Wang
  1 sibling, 1 reply; 7+ messages in thread
From: Yunjian Wang @ 2022-08-23  6:45 UTC (permalink / raw)
  To: dev
  Cc: matan, rasland, viacheslavo, dkozlyuk, huangshaozhang,
	Yunjian Wang, stable

The bonding slave remove function was calling the eth_dev_tx_queue_config
function, which frees dev->data->tx_queues, and then tries to free
priv->txqs[idx] in mlx5_txq_release function, which causes the heap use
after free issue. Add checks whether dev->data->tx_queues is not NULL.

Fixes: 94e257ec8ca ("net/mlx5: fix Rx/Tx queue checks")
Cc: stable@dpdk.org

Signed-off-by: Yunjian Wang <wangyunjian@huawei.com>
---
 drivers/net/mlx5/mlx5_txq.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/net/mlx5/mlx5_txq.c b/drivers/net/mlx5/mlx5_txq.c
index 0140f8b3b2..cb2c33a060 100644
--- a/drivers/net/mlx5/mlx5_txq.c
+++ b/drivers/net/mlx5/mlx5_txq.c
@@ -1198,7 +1198,8 @@ mlx5_txq_release(struct rte_eth_dev *dev, uint16_t idx)
 	struct mlx5_priv *priv = dev->data->dev_private;
 	struct mlx5_txq_ctrl *txq_ctrl;
 
-	if (priv->txqs == NULL || (*priv->txqs)[idx] == NULL)
+	if (dev->data->tx_queues == NULL || priv->txqs == NULL ||
+		(*priv->txqs)[idx] == NULL)
 		return 0;
 	txq_ctrl = container_of((*priv->txqs)[idx], struct mlx5_txq_ctrl, txq);
 	if (__atomic_sub_fetch(&txq_ctrl->refcnt, 1, __ATOMIC_RELAXED) > 1)
-- 
2.27.0


^ permalink raw reply	[flat|nested] 7+ messages in thread

* [dpdk-dev] [PATCH v2 2/2] net/mlx5: fix resource leak when releasing a drop action
       [not found] <cover.1661223500.git.wangyunjian@huawei.com>
  2022-08-23  6:45 ` [dpdk-dev] [PATCH v2 1/2] net/mlx5: fix use after free when releasing tx queues Yunjian Wang
@ 2022-08-23  6:46 ` Yunjian Wang
  2022-09-23  9:31   ` wangyunjian
  1 sibling, 1 reply; 7+ messages in thread
From: Yunjian Wang @ 2022-08-23  6:46 UTC (permalink / raw)
  To: dev
  Cc: matan, rasland, viacheslavo, dkozlyuk, huangshaozhang,
	Yunjian Wang, stable

Currently, the resources for hrxq->action are allocated in
mlx5_devx_hrxq_new(). But it was not being freed when the
drop action was released in mlx5_devx_drop_action_destroy().
So, fix is to free the resources in mlx5_devx_tir_destroy().

Fixes: bc5bee028ebc ("net/mlx5: create drop queue using DevX")
Cc: stable@dpdk.org

Signed-off-by: Yunjian Wang <wangyunjian@huawei.com>
---
 drivers/net/mlx5/mlx5_devx.c | 7 +++++++
 drivers/net/mlx5/mlx5_rxq.c  | 6 ------
 2 files changed, 7 insertions(+), 6 deletions(-)

diff --git a/drivers/net/mlx5/mlx5_devx.c b/drivers/net/mlx5/mlx5_devx.c
index 6886ae1f22..09c8856f05 100644
--- a/drivers/net/mlx5/mlx5_devx.c
+++ b/drivers/net/mlx5/mlx5_devx.c
@@ -907,6 +907,13 @@ mlx5_devx_hrxq_new(struct rte_eth_dev *dev, struct mlx5_hrxq *hrxq,
 static void
 mlx5_devx_tir_destroy(struct mlx5_hrxq *hrxq)
 {
+#if defined(HAVE_IBV_FLOW_DV_SUPPORT) || !defined(HAVE_INFINIBAND_VERBS_H)
+	if (hrxq->hws_flags)
+		mlx5dr_action_destroy(hrxq->action);
+	else
+		mlx5_flow_os_destroy_flow_action(hrxq->action);
+	hrxq->action = NULL;
+#endif
 	claim_zero(mlx5_devx_cmd_destroy(hrxq->tir));
 }
 
diff --git a/drivers/net/mlx5/mlx5_rxq.c b/drivers/net/mlx5/mlx5_rxq.c
index eaf23d0df4..e518fe9bfd 100644
--- a/drivers/net/mlx5/mlx5_rxq.c
+++ b/drivers/net/mlx5/mlx5_rxq.c
@@ -2861,12 +2861,6 @@ __mlx5_hrxq_remove(struct rte_eth_dev *dev, struct mlx5_hrxq *hrxq)
 {
 	struct mlx5_priv *priv = dev->data->dev_private;
 
-#ifdef HAVE_IBV_FLOW_DV_SUPPORT
-	if (hrxq->hws_flags)
-		mlx5dr_action_destroy(hrxq->action);
-	else
-		mlx5_glue->destroy_flow_action(hrxq->action);
-#endif
 	priv->obj_ops.hrxq_destroy(hrxq);
 	if (!hrxq->standalone) {
 		mlx5_ind_table_obj_release(dev, hrxq->ind_table,
-- 
2.27.0


^ permalink raw reply	[flat|nested] 7+ messages in thread

* RE: [dpdk-dev] [PATCH v2 1/2] net/mlx5: fix use after free when releasing tx queues
  2022-08-23  6:45 ` [dpdk-dev] [PATCH v2 1/2] net/mlx5: fix use after free when releasing tx queues Yunjian Wang
@ 2022-09-23  9:31   ` wangyunjian
  2022-09-26 12:30     ` Slava Ovsiienko
  0 siblings, 1 reply; 7+ messages in thread
From: wangyunjian @ 2022-09-23  9:31 UTC (permalink / raw)
  To: dev; +Cc: matan, rasland, viacheslavo, dkozlyuk, Huangshaozhang, stable

Friendly ping.

> -----Original Message-----
> From: wangyunjian
> Sent: Tuesday, August 23, 2022 2:46 PM
> To: dev@dpdk.org
> Cc: matan@nvidia.com; rasland@nvidia.com; viacheslavo@nvidia.com;
> dkozlyuk@nvidia.com; Huangshaozhang <huangshaozhang@huawei.com>;
> wangyunjian <wangyunjian@huawei.com>; stable@dpdk.org
> Subject: [dpdk-dev] [PATCH v2 1/2] net/mlx5: fix use after free when releasing
> tx queues
> 
> The bonding slave remove function was calling the eth_dev_tx_queue_config
> function, which frees dev->data->tx_queues, and then tries to free
> priv->txqs[idx] in mlx5_txq_release function, which causes the heap use
> after free issue. Add checks whether dev->data->tx_queues is not NULL.
> 
> Fixes: 94e257ec8ca ("net/mlx5: fix Rx/Tx queue checks")
> Cc: stable@dpdk.org
> 
> Signed-off-by: Yunjian Wang <wangyunjian@huawei.com>
> ---
>  drivers/net/mlx5/mlx5_txq.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/net/mlx5/mlx5_txq.c b/drivers/net/mlx5/mlx5_txq.c index
> 0140f8b3b2..cb2c33a060 100644
> --- a/drivers/net/mlx5/mlx5_txq.c
> +++ b/drivers/net/mlx5/mlx5_txq.c
> @@ -1198,7 +1198,8 @@ mlx5_txq_release(struct rte_eth_dev *dev, uint16_t
> idx)
>  	struct mlx5_priv *priv = dev->data->dev_private;
>  	struct mlx5_txq_ctrl *txq_ctrl;
> 
> -	if (priv->txqs == NULL || (*priv->txqs)[idx] == NULL)
> +	if (dev->data->tx_queues == NULL || priv->txqs == NULL ||
> +		(*priv->txqs)[idx] == NULL)
>  		return 0;
>  	txq_ctrl = container_of((*priv->txqs)[idx], struct mlx5_txq_ctrl, txq);
>  	if (__atomic_sub_fetch(&txq_ctrl->refcnt, 1, __ATOMIC_RELAXED) > 1)
> --
> 2.27.0


^ permalink raw reply	[flat|nested] 7+ messages in thread

* RE: [dpdk-dev] [PATCH v2 2/2] net/mlx5: fix resource leak when releasing a drop action
  2022-08-23  6:46 ` [dpdk-dev] [PATCH v2 2/2] net/mlx5: fix resource leak when releasing a drop action Yunjian Wang
@ 2022-09-23  9:31   ` wangyunjian
  2022-09-26 19:35     ` Slava Ovsiienko
  0 siblings, 1 reply; 7+ messages in thread
From: wangyunjian @ 2022-09-23  9:31 UTC (permalink / raw)
  To: dev; +Cc: matan, rasland, viacheslavo, dkozlyuk, Huangshaozhang, stable

Friendly ping.

> -----Original Message-----
> From: wangyunjian
> Sent: Tuesday, August 23, 2022 2:46 PM
> To: dev@dpdk.org
> Cc: matan@nvidia.com; rasland@nvidia.com; viacheslavo@nvidia.com;
> dkozlyuk@nvidia.com; Huangshaozhang <huangshaozhang@huawei.com>;
> wangyunjian <wangyunjian@huawei.com>; stable@dpdk.org
> Subject: [dpdk-dev] [PATCH v2 2/2] net/mlx5: fix resource leak when releasing a
> drop action
> 
> Currently, the resources for hrxq->action are allocated in
> mlx5_devx_hrxq_new(). But it was not being freed when the drop action was
> released in mlx5_devx_drop_action_destroy().
> So, fix is to free the resources in mlx5_devx_tir_destroy().
> 
> Fixes: bc5bee028ebc ("net/mlx5: create drop queue using DevX")
> Cc: stable@dpdk.org
> 
> Signed-off-by: Yunjian Wang <wangyunjian@huawei.com>
> ---
>  drivers/net/mlx5/mlx5_devx.c | 7 +++++++  drivers/net/mlx5/mlx5_rxq.c  |
> 6 ------
>  2 files changed, 7 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/net/mlx5/mlx5_devx.c b/drivers/net/mlx5/mlx5_devx.c index
> 6886ae1f22..09c8856f05 100644
> --- a/drivers/net/mlx5/mlx5_devx.c
> +++ b/drivers/net/mlx5/mlx5_devx.c
> @@ -907,6 +907,13 @@ mlx5_devx_hrxq_new(struct rte_eth_dev *dev,
> struct mlx5_hrxq *hrxq,  static void  mlx5_devx_tir_destroy(struct
> mlx5_hrxq *hrxq)  {
> +#if defined(HAVE_IBV_FLOW_DV_SUPPORT)
> || !defined(HAVE_INFINIBAND_VERBS_H)
> +	if (hrxq->hws_flags)
> +		mlx5dr_action_destroy(hrxq->action);
> +	else
> +		mlx5_flow_os_destroy_flow_action(hrxq->action);
> +	hrxq->action = NULL;
> +#endif
>  	claim_zero(mlx5_devx_cmd_destroy(hrxq->tir));
>  }
> 
> diff --git a/drivers/net/mlx5/mlx5_rxq.c b/drivers/net/mlx5/mlx5_rxq.c index
> eaf23d0df4..e518fe9bfd 100644
> --- a/drivers/net/mlx5/mlx5_rxq.c
> +++ b/drivers/net/mlx5/mlx5_rxq.c
> @@ -2861,12 +2861,6 @@ __mlx5_hrxq_remove(struct rte_eth_dev *dev,
> struct mlx5_hrxq *hrxq)  {
>  	struct mlx5_priv *priv = dev->data->dev_private;
> 
> -#ifdef HAVE_IBV_FLOW_DV_SUPPORT
> -	if (hrxq->hws_flags)
> -		mlx5dr_action_destroy(hrxq->action);
> -	else
> -		mlx5_glue->destroy_flow_action(hrxq->action);
> -#endif
>  	priv->obj_ops.hrxq_destroy(hrxq);
>  	if (!hrxq->standalone) {
>  		mlx5_ind_table_obj_release(dev, hrxq->ind_table,
> --
> 2.27.0


^ permalink raw reply	[flat|nested] 7+ messages in thread

* RE: [dpdk-dev] [PATCH v2 1/2] net/mlx5: fix use after free when releasing tx queues
  2022-09-23  9:31   ` wangyunjian
@ 2022-09-26 12:30     ` Slava Ovsiienko
  0 siblings, 0 replies; 7+ messages in thread
From: Slava Ovsiienko @ 2022-09-26 12:30 UTC (permalink / raw)
  To: wangyunjian, dev
  Cc: Matan Azrad, Raslan Darawsheh, Dmitry Kozlyuk, Huangshaozhang, stable

Hi, Yunjian

Could you, please, tell more details about problematic scenario?
In bonding slave? It is not fully clean for me how mlx5_txq_release
frees priv->txqs[idx] (BTW NULL is OK to free, it is safe).
We have check for NULL here:
> > -	if (priv->txqs == NULL || (*priv->txqs)[idx] == NULL)

priv->txq is internal objects managed by PMD, dev->data->tx_queues
are DPDK-wide ones. Theoretically it might happen when DPDK objects
are created and internals are not, and vice versa. So, checking 
for existence of external objects in the routine that manages internals
does not look so reasonable. Internal queue object management is based
on the atomic reference counter and, generally speaking, should not depend
on externals.

With best regards,
Slava 

> -----Original Message-----
> From: wangyunjian <wangyunjian@huawei.com>
> Sent: Friday, September 23, 2022 12:32
> To: dev@dpdk.org
> Cc: Matan Azrad <matan@nvidia.com>; Raslan Darawsheh <rasland@nvidia.com>;
> Slava Ovsiienko <viacheslavo@nvidia.com>; Dmitry Kozlyuk
> <dkozlyuk@nvidia.com>; Huangshaozhang <huangshaozhang@huawei.com>;
> stable@dpdk.org
> Subject: RE: [dpdk-dev] [PATCH v2 1/2] net/mlx5: fix use after free when
> releasing tx queues
> 
> Friendly ping.
> 
> > -----Original Message-----
> > From: wangyunjian
> > Sent: Tuesday, August 23, 2022 2:46 PM
> > To: dev@dpdk.org
> > Cc: matan@nvidia.com; rasland@nvidia.com; viacheslavo@nvidia.com;
> > dkozlyuk@nvidia.com; Huangshaozhang <huangshaozhang@huawei.com>;
> > wangyunjian <wangyunjian@huawei.com>; stable@dpdk.org
> > Subject: [dpdk-dev] [PATCH v2 1/2] net/mlx5: fix use after free when
> > releasing tx queues
> >
> > The bonding slave remove function was calling the
> > eth_dev_tx_queue_config function, which frees dev->data->tx_queues,
> > and then tries to free
> > priv->txqs[idx] in mlx5_txq_release function, which causes the heap
> > priv->use
> > after free issue. Add checks whether dev->data->tx_queues is not NULL.
> >
> > Fixes: 94e257ec8ca ("net/mlx5: fix Rx/Tx queue checks")
> > Cc: stable@dpdk.org
> >
> > Signed-off-by: Yunjian Wang <wangyunjian@huawei.com>
> > ---
> >  drivers/net/mlx5/mlx5_txq.c | 3 ++-
> >  1 file changed, 2 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/net/mlx5/mlx5_txq.c b/drivers/net/mlx5/mlx5_txq.c
> > index
> > 0140f8b3b2..cb2c33a060 100644
> > --- a/drivers/net/mlx5/mlx5_txq.c
> > +++ b/drivers/net/mlx5/mlx5_txq.c
> > @@ -1198,7 +1198,8 @@ mlx5_txq_release(struct rte_eth_dev *dev,
> > uint16_t
> > idx)
> >  	struct mlx5_priv *priv = dev->data->dev_private;
> >  	struct mlx5_txq_ctrl *txq_ctrl;
> >
> > -	if (priv->txqs == NULL || (*priv->txqs)[idx] == NULL)
> > +	if (dev->data->tx_queues == NULL || priv->txqs == NULL ||
> > +		(*priv->txqs)[idx] == NULL)
> >  		return 0;
> >  	txq_ctrl = container_of((*priv->txqs)[idx], struct mlx5_txq_ctrl,
> txq);
> >  	if (__atomic_sub_fetch(&txq_ctrl->refcnt, 1, __ATOMIC_RELAXED) > 1)
> > --
> > 2.27.0


^ permalink raw reply	[flat|nested] 7+ messages in thread

* RE: [dpdk-dev] [PATCH v2 2/2] net/mlx5: fix resource leak when releasing a drop action
  2022-09-23  9:31   ` wangyunjian
@ 2022-09-26 19:35     ` Slava Ovsiienko
  2022-09-29  1:51       ` wangyunjian
  0 siblings, 1 reply; 7+ messages in thread
From: Slava Ovsiienko @ 2022-09-26 19:35 UTC (permalink / raw)
  To: wangyunjian, dev
  Cc: Matan Azrad, Raslan Darawsheh, Dmitry Kozlyuk, Huangshaozhang, stable

Hi, Yunjian

We have drop action in mlx5.
To create/destroy drop action we have 
mlx5_drop_action_create/ mlx5_drop_action_destroy common routines.

As PMD supports operation either over FW (Verbs in rdma_core) or SW/HW steering
with DevX objects the "virtual" methods are used for objects:

priv->obj_ops.drop_action_create(dev) - create drop action
   a) mlx5_ibv_drop_action_create()- use Verbs (dv-flow_en == 0)
   b) mlx5_devx_drop_action_create() - use DevX (dv-flow_en != 0)

priv->obj_ops.drop_action_destroy(dev) - destroy drop action
   a) mlx5_ibv_drop_action_destroy()- use Verbs (dv-flow_en == 0)
   b) mlx5_devx_drop_action_destroy() - use DevX (dv-flow_en != 0)


Let's consider DevX case.
mlx5_devx_drop_action_create()
    mlx5_rxq_devx_obj_drop_create()
    mlx5_devx_ind_table_new()
    mlx5_devx_hrxq_new()
         xxx_create_dest_tir()
         xxx_action_create_dest_tir()


mlx5_devx_drop_action_destroy()
    /* It seems action destroy is missing here */
    mlx5_devx_tir_destroy()
    mlx5_devx_ind_table_destroy()
    mlx5_rxq_devx_obj_drop_release()

So, yes, I agree. There is missing action destroy, it seems we have leakage.

Let's look at 
__mlx5_hrxq_remove()
   destroy_action()
   priv->obj_ops.hrxq_destroy();


And there we have a problem.
obj_ops.hrxq_destroy() - not always mlx5_devx_tir_destroy() is called.
It might be mlx5_ibv_qp_destroy() (for Verbas) and patch would not work.

So, instead of fixing __mlx5_hrxq_remove() and mlx5_devx_tir_destroy()
I would consider patching:
- mlx5_devx_drop_action_destroy()
- mlx5_ibv_drop_action_destroy
by adding action desatroying.

With best regards,
Slava


> -----Original Message-----
> From: wangyunjian <wangyunjian@huawei.com>
> Sent: Friday, September 23, 2022 12:32
> To: dev@dpdk.org
> Cc: Matan Azrad <matan@nvidia.com>; Raslan Darawsheh <rasland@nvidia.com>;
> Slava Ovsiienko <viacheslavo@nvidia.com>; Dmitry Kozlyuk
> <dkozlyuk@nvidia.com>; Huangshaozhang <huangshaozhang@huawei.com>;
> stable@dpdk.org
> Subject: RE: [dpdk-dev] [PATCH v2 2/2] net/mlx5: fix resource leak when
> releasing a drop action
> 
> Friendly ping.
> 
> > -----Original Message-----
> > From: wangyunjian
> > Sent: Tuesday, August 23, 2022 2:46 PM
> > To: dev@dpdk.org
> > Cc: matan@nvidia.com; rasland@nvidia.com; viacheslavo@nvidia.com;
> > dkozlyuk@nvidia.com; Huangshaozhang <huangshaozhang@huawei.com>;
> > wangyunjian <wangyunjian@huawei.com>; stable@dpdk.org
> > Subject: [dpdk-dev] [PATCH v2 2/2] net/mlx5: fix resource leak when
> > releasing a drop action
> >
> > Currently, the resources for hrxq->action are allocated in
> > mlx5_devx_hrxq_new(). But it was not being freed when the drop action
> > was released in mlx5_devx_drop_action_destroy().
> > So, fix is to free the resources in mlx5_devx_tir_destroy().
> >
> > Fixes: bc5bee028ebc ("net/mlx5: create drop queue using DevX")
> > Cc: stable@dpdk.org
> >
> > Signed-off-by: Yunjian Wang <wangyunjian@huawei.com>
> > ---
> >  drivers/net/mlx5/mlx5_devx.c | 7 +++++++  drivers/net/mlx5/mlx5_rxq.c
> > |
> > 6 ------
> >  2 files changed, 7 insertions(+), 6 deletions(-)
> >
> > diff --git a/drivers/net/mlx5/mlx5_devx.c
> > b/drivers/net/mlx5/mlx5_devx.c index
> > 6886ae1f22..09c8856f05 100644
> > --- a/drivers/net/mlx5/mlx5_devx.c
> > +++ b/drivers/net/mlx5/mlx5_devx.c
> > @@ -907,6 +907,13 @@ mlx5_devx_hrxq_new(struct rte_eth_dev *dev,
> > struct mlx5_hrxq *hrxq,  static void  mlx5_devx_tir_destroy(struct
> > mlx5_hrxq *hrxq)  {
> > +#if defined(HAVE_IBV_FLOW_DV_SUPPORT)
> > || !defined(HAVE_INFINIBAND_VERBS_H)
> > +	if (hrxq->hws_flags)
> > +		mlx5dr_action_destroy(hrxq->action);
> > +	else
> > +		mlx5_flow_os_destroy_flow_action(hrxq->action);
> > +	hrxq->action = NULL;
> > +#endif
> >  	claim_zero(mlx5_devx_cmd_destroy(hrxq->tir));
> >  }
> >
> > diff --git a/drivers/net/mlx5/mlx5_rxq.c b/drivers/net/mlx5/mlx5_rxq.c
> > index eaf23d0df4..e518fe9bfd 100644
> > --- a/drivers/net/mlx5/mlx5_rxq.c
> > +++ b/drivers/net/mlx5/mlx5_rxq.c
> > @@ -2861,12 +2861,6 @@ __mlx5_hrxq_remove(struct rte_eth_dev *dev,
> > struct mlx5_hrxq *hrxq)  {
> >  	struct mlx5_priv *priv = dev->data->dev_private;
> >
> > -#ifdef HAVE_IBV_FLOW_DV_SUPPORT
> > -	if (hrxq->hws_flags)
> > -		mlx5dr_action_destroy(hrxq->action);
> > -	else
> > -		mlx5_glue->destroy_flow_action(hrxq->action);
> > -#endif
> >  	priv->obj_ops.hrxq_destroy(hrxq);
> >  	if (!hrxq->standalone) {
> >  		mlx5_ind_table_obj_release(dev, hrxq->ind_table,
> > --
> > 2.27.0


^ permalink raw reply	[flat|nested] 7+ messages in thread

* RE: [dpdk-dev] [PATCH v2 2/2] net/mlx5: fix resource leak when releasing a drop action
  2022-09-26 19:35     ` Slava Ovsiienko
@ 2022-09-29  1:51       ` wangyunjian
  0 siblings, 0 replies; 7+ messages in thread
From: wangyunjian @ 2022-09-29  1:51 UTC (permalink / raw)
  To: Slava Ovsiienko, dev
  Cc: Matan Azrad, Raslan Darawsheh, Dmitry Kozlyuk, Huangshaozhang, stable

I agree with you. Can you fix it?

Thanks
        Yunjian

> -----Original Message-----
> From: Slava Ovsiienko [mailto:viacheslavo@nvidia.com]
> Sent: Tuesday, September 27, 2022 3:36 AM
> To: wangyunjian <wangyunjian@huawei.com>; dev@dpdk.org
> Cc: Matan Azrad <matan@nvidia.com>; Raslan Darawsheh
> <rasland@nvidia.com>; Dmitry Kozlyuk <dkozlyuk@nvidia.com>;
> Huangshaozhang <huangshaozhang@huawei.com>; stable@dpdk.org
> Subject: RE: [dpdk-dev] [PATCH v2 2/2] net/mlx5: fix resource leak when
> releasing a drop action
> 
> Hi, Yunjian
> 
> We have drop action in mlx5.
> To create/destroy drop action we have
> mlx5_drop_action_create/ mlx5_drop_action_destroy common routines.
> 
> As PMD supports operation either over FW (Verbs in rdma_core) or SW/HW
> steering with DevX objects the "virtual" methods are used for objects:
> 
> priv->obj_ops.drop_action_create(dev) - create drop action
>    a) mlx5_ibv_drop_action_create()- use Verbs (dv-flow_en == 0)
>    b) mlx5_devx_drop_action_create() - use DevX (dv-flow_en != 0)
> 
> priv->obj_ops.drop_action_destroy(dev) - destroy drop action
>    a) mlx5_ibv_drop_action_destroy()- use Verbs (dv-flow_en == 0)
>    b) mlx5_devx_drop_action_destroy() - use DevX (dv-flow_en != 0)
> 
> 
> Let's consider DevX case.
> mlx5_devx_drop_action_create()
>     mlx5_rxq_devx_obj_drop_create()
>     mlx5_devx_ind_table_new()
>     mlx5_devx_hrxq_new()
>          xxx_create_dest_tir()
>          xxx_action_create_dest_tir()
> 
> 
> mlx5_devx_drop_action_destroy()
>     /* It seems action destroy is missing here */
>     mlx5_devx_tir_destroy()
>     mlx5_devx_ind_table_destroy()
>     mlx5_rxq_devx_obj_drop_release()
> 
> So, yes, I agree. There is missing action destroy, it seems we have leakage.
> 
> Let's look at
> __mlx5_hrxq_remove()
>    destroy_action()
>    priv->obj_ops.hrxq_destroy();
> 
> 
> And there we have a problem.
> obj_ops.hrxq_destroy() - not always mlx5_devx_tir_destroy() is called.
> It might be mlx5_ibv_qp_destroy() (for Verbas) and patch would not work.
> 
> So, instead of fixing __mlx5_hrxq_remove() and mlx5_devx_tir_destroy()
> I would consider patching:
> - mlx5_devx_drop_action_destroy()
> - mlx5_ibv_drop_action_destroy
> by adding action desatroying.
> 
> With best regards,
> Slava
> 
> 
> > -----Original Message-----
> > From: wangyunjian <wangyunjian@huawei.com>
> > Sent: Friday, September 23, 2022 12:32
> > To: dev@dpdk.org
> > Cc: Matan Azrad <matan@nvidia.com>; Raslan Darawsheh
> <rasland@nvidia.com>;
> > Slava Ovsiienko <viacheslavo@nvidia.com>; Dmitry Kozlyuk
> > <dkozlyuk@nvidia.com>; Huangshaozhang
> <huangshaozhang@huawei.com>;
> > stable@dpdk.org
> > Subject: RE: [dpdk-dev] [PATCH v2 2/2] net/mlx5: fix resource leak when
> > releasing a drop action
> >
> > Friendly ping.
> >
> > > -----Original Message-----
> > > From: wangyunjian
> > > Sent: Tuesday, August 23, 2022 2:46 PM
> > > To: dev@dpdk.org
> > > Cc: matan@nvidia.com; rasland@nvidia.com; viacheslavo@nvidia.com;
> > > dkozlyuk@nvidia.com; Huangshaozhang <huangshaozhang@huawei.com>;
> > > wangyunjian <wangyunjian@huawei.com>; stable@dpdk.org
> > > Subject: [dpdk-dev] [PATCH v2 2/2] net/mlx5: fix resource leak when
> > > releasing a drop action
> > >
> > > Currently, the resources for hrxq->action are allocated in
> > > mlx5_devx_hrxq_new(). But it was not being freed when the drop action
> > > was released in mlx5_devx_drop_action_destroy().
> > > So, fix is to free the resources in mlx5_devx_tir_destroy().
> > >
> > > Fixes: bc5bee028ebc ("net/mlx5: create drop queue using DevX")
> > > Cc: stable@dpdk.org
> > >
> > > Signed-off-by: Yunjian Wang <wangyunjian@huawei.com>
> > > ---
> > >  drivers/net/mlx5/mlx5_devx.c | 7 +++++++  drivers/net/mlx5/mlx5_rxq.c
> > > |
> > > 6 ------
> > >  2 files changed, 7 insertions(+), 6 deletions(-)
> > >
> > > diff --git a/drivers/net/mlx5/mlx5_devx.c
> > > b/drivers/net/mlx5/mlx5_devx.c index
> > > 6886ae1f22..09c8856f05 100644
> > > --- a/drivers/net/mlx5/mlx5_devx.c
> > > +++ b/drivers/net/mlx5/mlx5_devx.c
> > > @@ -907,6 +907,13 @@ mlx5_devx_hrxq_new(struct rte_eth_dev *dev,
> > > struct mlx5_hrxq *hrxq,  static void  mlx5_devx_tir_destroy(struct
> > > mlx5_hrxq *hrxq)  {
> > > +#if defined(HAVE_IBV_FLOW_DV_SUPPORT)
> > > || !defined(HAVE_INFINIBAND_VERBS_H)
> > > +	if (hrxq->hws_flags)
> > > +		mlx5dr_action_destroy(hrxq->action);
> > > +	else
> > > +		mlx5_flow_os_destroy_flow_action(hrxq->action);
> > > +	hrxq->action = NULL;
> > > +#endif
> > >  	claim_zero(mlx5_devx_cmd_destroy(hrxq->tir));
> > >  }
> > >
> > > diff --git a/drivers/net/mlx5/mlx5_rxq.c b/drivers/net/mlx5/mlx5_rxq.c
> > > index eaf23d0df4..e518fe9bfd 100644
> > > --- a/drivers/net/mlx5/mlx5_rxq.c
> > > +++ b/drivers/net/mlx5/mlx5_rxq.c
> > > @@ -2861,12 +2861,6 @@ __mlx5_hrxq_remove(struct rte_eth_dev *dev,
> > > struct mlx5_hrxq *hrxq)  {
> > >  	struct mlx5_priv *priv = dev->data->dev_private;
> > >
> > > -#ifdef HAVE_IBV_FLOW_DV_SUPPORT
> > > -	if (hrxq->hws_flags)
> > > -		mlx5dr_action_destroy(hrxq->action);
> > > -	else
> > > -		mlx5_glue->destroy_flow_action(hrxq->action);
> > > -#endif
> > >  	priv->obj_ops.hrxq_destroy(hrxq);
> > >  	if (!hrxq->standalone) {
> > >  		mlx5_ind_table_obj_release(dev, hrxq->ind_table,
> > > --
> > > 2.27.0


^ permalink raw reply	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2022-09-29  1:51 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
     [not found] <cover.1661223500.git.wangyunjian@huawei.com>
2022-08-23  6:45 ` [dpdk-dev] [PATCH v2 1/2] net/mlx5: fix use after free when releasing tx queues Yunjian Wang
2022-09-23  9:31   ` wangyunjian
2022-09-26 12:30     ` Slava Ovsiienko
2022-08-23  6:46 ` [dpdk-dev] [PATCH v2 2/2] net/mlx5: fix resource leak when releasing a drop action Yunjian Wang
2022-09-23  9:31   ` wangyunjian
2022-09-26 19:35     ` Slava Ovsiienko
2022-09-29  1:51       ` wangyunjian

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).