fixes for mlx5

PATCH Superseded Yunjian Wang 2 participants 12 messages Aug 3, 2022 – Sep 29, 2022 Network drivers

lore

Share & tools

lore

Yunjian Wang

author

2022-08-03 13:16 UTC #0 lore raw

cover fixes for mlx5

This series include two fixes patches for mlx5.

Yunjian Wang (2):
net/mlx5: fix use after free when releasing tx queues net/mlx5: fix resource leak when releasing a drop action

 drivers/net/mlx5/mlx5_devx.c | 6 ++++++
 drivers/net/mlx5/mlx5_rxq.c  | 6 ------
 drivers/net/mlx5/mlx5_txq.c  | 3 ++-
 3 files changed, 8 insertions(+), 7 deletions(-)

Signature · 1 line

2.27.0

Yunjian Wang

author

2022-08-03 13:16 UTC #1 lore raw

PATCH 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 [email protected]

Signed-off-by Yunjian Wang <[email protected]>

---
 drivers/net/mlx5/mlx5_txq.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

drivers/net/mlx5/mlx5_txq.c +2 -1

index 0140f8b3b2..cb2c33a060 100644
@@ -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)

Signature · 1 line

2.27.0

Yunjian Wang

author

2022-08-03 13:16 UTC #2 lore raw

PATCH 2/2 net/mlx5: fix resource leak when releasing a drop action

In reply to Yunjian Wang

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 [email protected]

Signed-off-by Yunjian Wang <[email protected]>

---
 drivers/net/mlx5/mlx5_devx.c | 6 ++++++
 drivers/net/mlx5/mlx5_rxq.c  | 6 ------
 2 files changed, 6 insertions(+), 6 deletions(-)

drivers/net/mlx5/mlx5_devx.c +6 -0

index 6886ae1f22..38a4a7c802 100644
@@ -907,6 +907,12 @@ 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_glue->destroy_flow_action(hrxq->action);
+#endif
 	claim_zero(mlx5_devx_cmd_destroy(hrxq->tir));
 }

drivers/net/mlx5/mlx5_rxq.c +0 -6

index eaf23d0df4..e518fe9bfd 100644
@@ -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,

Signature · 1 line

2.27.0

Yunjian Wang

author

2022-08-23 06:45 UTC #3 lore raw

v2 cover fixes for mlx5

In reply to Yunjian Wang

This series include two fixes patches for mlx5.

---
v2:
* update patch 2/2 code styles

Yunjian Wang (2):
net/mlx5: fix use after free when releasing tx queues net/mlx5: fix resource leak when releasing a drop action

 drivers/net/mlx5/mlx5_devx.c | 7 +++++++
 drivers/net/mlx5/mlx5_rxq.c  | 6 ------
 drivers/net/mlx5/mlx5_txq.c  | 3 ++-
 3 files changed, 9 insertions(+), 7 deletions(-)

Signature · 1 line

2.27.0

Yunjian Wang

author

2022-08-23 06:45 UTC #4 lore raw

PATCH v2 1/2 net/mlx5: fix use after free when releasing tx queues

Fixes 94e257ec8ca ("net/mlx5: fix Rx/Tx queue checks")

Cc [email protected]

Signed-off-by Yunjian Wang <[email protected]>

---
 drivers/net/mlx5/mlx5_txq.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

drivers/net/mlx5/mlx5_txq.c +2 -1

index 0140f8b3b2..cb2c33a060 100644
@@ -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)

Signature · 1 line

2.27.0

wangyunjian

author

2022-09-23 09:31 UTC #5 lore raw

RE: [dpdk-dev] [PATCH v2 1/2] net/mlx5: fix use after free when releasing tx queues

Friendly ping.

Quoted 40 lines

-----Original Message-----
From: wangyunjian
Sent: Tuesday, August 23, 2022 2:46 PM
To: [email protected]
Cc: [email protected]; [email protected]; [email protected]; [email protected]; Huangshaozhang <[email protected]>; wangyunjian <[email protected]>; [email protected]
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 [email protected]
Signed-off-by Yunjian Wang <[email protected]>
---
 drivers/net/mlx5/mlx5_txq.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)
drivers/net/mlx5/mlx5_txq.c index renamed from drivers/net/mlx5/mlx5_txq.c +0 -0
0140f8b3b2..cb2c33a060 100644
drivers/net/mlx5/mlx5_txq.c +2 -1
@@ -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

Slava Ovsiienko

2022-09-26 12:30 UTC #6 lore raw

RE: [dpdk-dev] [PATCH v2 1/2] net/mlx5: fix use after free when releasing tx queues

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:

Quoted 1 line

drivers/net/mlx5/mlx5_txq.c mlx5_txq_release(struct rte_eth_dev *dev, uint16_t idx) · line 1201
Quoted 1 line
drivers/net/mlx5/mlx5_txq.c +0 -1
-	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

Quoted 58 lines

-----Original Message-----
From: wangyunjian <[email protected]>
Sent: Friday, September 23, 2022 12:32
To: [email protected]
Cc: Matan Azrad <[email protected]>; Raslan Darawsheh <[email protected]>; Slava Ovsiienko <[email protected]>; Dmitry Kozlyuk <[email protected]>; Huangshaozhang <[email protected]>; [email protected]
Subject: RE: [dpdk-dev] [PATCH v2 1/2] net/mlx5: fix use after free when releasing tx queues
Friendly ping.
Quoted 33 lines
-----Original Message-----
From: wangyunjian
Sent: Tuesday, August 23, 2022 2:46 PM
To: [email protected]
Cc: [email protected]; [email protected]; [email protected]; [email protected]; Huangshaozhang <[email protected]>; wangyunjian <[email protected]>; [email protected]
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 [email protected]
Signed-off-by Yunjian Wang <[email protected]>
---
 drivers/net/mlx5/mlx5_txq.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)
drivers/net/mlx5/mlx5_txq.c +0 -0
index
0140f8b3b2..cb2c33a060 100644
drivers/net/mlx5/mlx5_txq.c +0 -0
@@ -1198,7 +1198,8 @@ mlx5_txq_release(struct rte_eth_dev *dev,
uint16_t
idx)
drivers/net/mlx5/mlx5_txq.c mlx5_txq_release(struct rte_eth_dev *dev, uint16_t idx)
Quoted 8 lines
drivers/net/mlx5/mlx5_txq.c +2 -1
 	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);
Quoted 3 lines
if (__atomic_sub_fetch(&txq_ctrl->refcnt, 1, __ATOMIC_RELAXED) > 1)
--
2.27.0

Yunjian Wang

author

2022-08-23 06:46 UTC #7 lore raw

PATCH v2 2/2 net/mlx5: fix resource leak when releasing a drop action

In reply to Yunjian Wang

Fixes bc5bee028ebc ("net/mlx5: create drop queue using DevX")

Cc [email protected]

Signed-off-by Yunjian Wang <[email protected]>

---
 drivers/net/mlx5/mlx5_devx.c | 7 +++++++
 drivers/net/mlx5/mlx5_rxq.c  | 6 ------
 2 files changed, 7 insertions(+), 6 deletions(-)

drivers/net/mlx5/mlx5_devx.c +7 -0

index 6886ae1f22..09c8856f05 100644
@@ -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));
 }

drivers/net/mlx5/mlx5_rxq.c +0 -6

index eaf23d0df4..e518fe9bfd 100644
@@ -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,

Signature · 1 line

2.27.0

wangyunjian

author

2022-09-23 09:31 UTC #8 lore raw

RE: [dpdk-dev] [PATCH v2 2/2] net/mlx5: fix resource leak when releasing a drop action

Friendly ping.

Quoted 61 lines

-----Original Message-----
From: wangyunjian
Sent: Tuesday, August 23, 2022 2:46 PM
To: [email protected]
Cc: [email protected]; [email protected]; [email protected]; [email protected]; Huangshaozhang <[email protected]>; wangyunjian <[email protected]>; [email protected]
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 [email protected]
Signed-off-by Yunjian Wang <[email protected]>
---
drivers/net/mlx5/mlx5_devx.c | 7 +++++++ drivers/net/mlx5/mlx5_rxq.c |
6 ------
 2 files changed, 7 insertions(+), 6 deletions(-)
drivers/net/mlx5/mlx5_devx.c index renamed from drivers/net/mlx5/mlx5_devx.c +0 -0
6886ae1f22..09c8856f05 100644
drivers/net/mlx5/mlx5_devx.c +0 -0
@@ -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)
drivers/net/mlx5/mlx5_devx.c +6 -0
+	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));
 }
drivers/net/mlx5/mlx5_rxq.c index renamed from drivers/net/mlx5/mlx5_rxq.c +0 -0
eaf23d0df4..e518fe9bfd 100644
drivers/net/mlx5/mlx5_rxq.c +0 -7
@@ -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

Slava Ovsiienko

2022-09-26 19:35 UTC #9 lore raw

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

Quoted 76 lines

-----Original Message-----
From: wangyunjian <[email protected]>
Sent: Friday, September 23, 2022 12:32
To: [email protected]
Cc: Matan Azrad <[email protected]>; Raslan Darawsheh <[email protected]>; Slava Ovsiienko <[email protected]>; Dmitry Kozlyuk <[email protected]>; Huangshaozhang <[email protected]>; [email protected]
Subject: RE: [dpdk-dev] [PATCH v2 2/2] net/mlx5: fix resource leak when releasing a drop action
Friendly ping.
Quoted 63 lines
-----Original Message-----
From: wangyunjian
Sent: Tuesday, August 23, 2022 2:46 PM
To: [email protected]
Cc: [email protected]; [email protected]; [email protected]; [email protected]; Huangshaozhang <[email protected]>; wangyunjian <[email protected]>; [email protected]
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 [email protected]
Signed-off-by Yunjian Wang <[email protected]>
---
drivers/net/mlx5/mlx5_devx.c | 7 +++++++ drivers/net/mlx5/mlx5_rxq.c
|
6 ------
 2 files changed, 7 insertions(+), 6 deletions(-)
(quoted diff) +0 -0
b/drivers/net/mlx5/mlx5_devx.c index
6886ae1f22..09c8856f05 100644
drivers/net/mlx5/mlx5_devx.c +0 -0
@@ -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)
drivers/net/mlx5/mlx5_devx.c +6 -0
+	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));
 }
drivers/net/mlx5/mlx5_rxq.c +0 -7
index eaf23d0df4..e518fe9bfd 100644
@@ -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

wangyunjian

author

2022-09-29 01:51 UTC #10 lore raw

RE: [dpdk-dev] [PATCH v2 2/2] net/mlx5: fix resource leak when releasing a drop action

I agree with you. Can you fix it?

Thanks
Yunjian

Quoted 143 lines

-----Original Message-----
From: Slava Ovsiienko [mailto:[email protected]]
Sent: Tuesday, September 27, 2022 3:36 AM
To: wangyunjian <[email protected]>; [email protected]
Cc: Matan Azrad <[email protected]>; Raslan Darawsheh <[email protected]>; Dmitry Kozlyuk <[email protected]>; Huangshaozhang <[email protected]>; [email protected]
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
Quoted 5 lines
-----Original Message-----
From: wangyunjian <[email protected]>
Sent: Friday, September 23, 2022 12:32
To: [email protected]
Cc: Matan Azrad <[email protected]>; Raslan Darawsheh
<[email protected]>;
Quoted 2 lines
Slava Ovsiienko <[email protected]>; Dmitry Kozlyuk <[email protected]>; Huangshaozhang
<[email protected]>;
Quoted 69 lines
[email protected]
Subject: RE: [dpdk-dev] [PATCH v2 2/2] net/mlx5: fix resource leak when releasing a drop action
Friendly ping.
Quoted 63 lines
-----Original Message-----
From: wangyunjian
Sent: Tuesday, August 23, 2022 2:46 PM
To: [email protected]
Cc: [email protected]; [email protected]; [email protected]; [email protected]; Huangshaozhang <[email protected]>; wangyunjian <[email protected]>; [email protected]
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 [email protected]
Signed-off-by Yunjian Wang <[email protected]>
---
drivers/net/mlx5/mlx5_devx.c | 7 +++++++ drivers/net/mlx5/mlx5_rxq.c
|
6 ------
 2 files changed, 7 insertions(+), 6 deletions(-)
(quoted diff) +0 -0
b/drivers/net/mlx5/mlx5_devx.c index
6886ae1f22..09c8856f05 100644
drivers/net/mlx5/mlx5_devx.c +0 -0
@@ -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)
drivers/net/mlx5/mlx5_devx.c +6 -0
+	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));
 }
drivers/net/mlx5/mlx5_rxq.c +0 -7
index eaf23d0df4..e518fe9bfd 100644
@@ -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

wangyunjian

author

2022-09-01 09:20 UTC #11 lore raw

In reply to Yunjian Wang

Friendly ping.

Quoted 25 lines

-----Original Message-----
From: wangyunjian
Sent: Tuesday, August 23, 2022 2:45 PM
To: [email protected]
Cc: [email protected]; [email protected]; [email protected]; [email protected]; Huangshaozhang <[email protected]>; wangyunjian <[email protected]>
Subject: [dpdk-dev] [PATCH v2 0/2] fixes for mlx5
This series include two fixes patches for mlx5.
---
v2:
* update patch 2/2 code styles
Yunjian Wang (2):
net/mlx5: fix use after free when releasing tx queues net/mlx5: fix resource leak when releasing a drop action
drivers/net/mlx5/mlx5_devx.c | 7 +++++++ drivers/net/mlx5/mlx5_rxq.c |
6 ------ drivers/net/mlx5/mlx5_txq.c | 3 ++-
 3 files changed, 9 insertions(+), 7 deletions(-)
--
2.27.0