[dpdk-dev] [PATCH 0/2] net/mlx5: fix table leak issue

[dpdk-dev] [PATCH 0/2] net/mlx5: fix table leak issue

[Suanming Mou]

Currently, the tables got by jump action and flow meter suffix flow are
not put correctly.

Fix the bugs with the refer_cnt issue.

Suanming Mou (2):
  net/mlx5: fix meter suffix table leak issue
  net/mlx5: fix jump table leak issue

 drivers/net/mlx5/mlx5.h         |  6 -----
 drivers/net/mlx5/mlx5_flow.h    |  2 ++
 drivers/net/mlx5/mlx5_flow_dv.c | 54 ++++++++++++++++++++---------------------
 3 files changed, 29 insertions(+), 33 deletions(-)

-- 
1.8.3.1

[dpdk-dev] [PATCH 1/2] net/mlx5: fix meter suffix table leak issue

[Suanming Mou]

Currently, the meter suffix table is created and saved in the mlx5
shared struct. It causes the suffix table will never be released
even without any meter rules.

Move the suffix table to meter domain struct to help the suffix table
be released when all the meter rules are destroyed.

Fixes: 46a5e6bc6a85 ("net/mlx5: prepare meter flow tables")
Cc: stable@dpdk.org

Signed-off-by: Suanming Mou <suanmingm@mellanox.com>
Acked-by: Viacheslav Ovsiienko <viacheslavo@mellanox.com>
---
 drivers/net/mlx5/mlx5.h         |  6 ------
 drivers/net/mlx5/mlx5_flow.h    |  2 ++
 drivers/net/mlx5/mlx5_flow_dv.c | 48 ++++++++++++++++++-----------------------
 3 files changed, 23 insertions(+), 33 deletions(-)

diff --git a/drivers/net/mlx5/mlx5.h b/drivers/net/mlx5/mlx5.h
index c58afb5..3e9558f 100644
--- a/drivers/net/mlx5/mlx5.h
+++ b/drivers/net/mlx5/mlx5.h
@@ -454,14 +454,8 @@ struct mlx5_ibv_shared {
 	uint32_t dv_regc0_mask; /* available bits of metatada reg_c[0]. */
 	uint32_t dv_refcnt; /* DV/DR data reference counter. */
 	void *fdb_domain; /* FDB Direct Rules name space handle. */
-	struct mlx5_flow_tbl_resource *fdb_mtr_sfx_tbl;
-	/* FDB meter suffix rules table. */
 	void *rx_domain; /* RX Direct Rules name space handle. */
-	struct mlx5_flow_tbl_resource *rx_mtr_sfx_tbl;
-	/* RX meter suffix rules table. */
 	void *tx_domain; /* TX Direct Rules name space handle. */
-	struct mlx5_flow_tbl_resource *tx_mtr_sfx_tbl;
-	/* TX meter suffix rules table. */
 	struct mlx5_hlist *flow_tbls;
 	/* Direct Rules tables for FDB, NIC TX+RX */
 	void *esw_drop_action; /* Pointer to DR E-Switch drop action. */
diff --git a/drivers/net/mlx5/mlx5_flow.h b/drivers/net/mlx5/mlx5_flow.h
index daa1f84..df09d0b 100644
--- a/drivers/net/mlx5/mlx5_flow.h
+++ b/drivers/net/mlx5/mlx5_flow.h
@@ -642,6 +642,8 @@ struct mlx5_flow_policer_stats {
 struct mlx5_meter_domain_info {
 	struct mlx5_flow_tbl_resource *tbl;
 	/**< Meter table. */
+	struct mlx5_flow_tbl_resource *sfx_tbl;
+	/**< Meter suffix table. */
 	void *any_matcher;
 	/**< Meter color not match default criteria. */
 	void *color_matcher;
diff --git a/drivers/net/mlx5/mlx5_flow_dv.c b/drivers/net/mlx5/mlx5_flow_dv.c
index 18ea577..0676b6f 100644
--- a/drivers/net/mlx5/mlx5_flow_dv.c
+++ b/drivers/net/mlx5/mlx5_flow_dv.c
@@ -8516,6 +8516,9 @@ struct field_modify_info modify_tcp[] = {
 	if (mtd->egress.tbl)
 		claim_zero(flow_dv_tbl_resource_release(dev,
 							mtd->egress.tbl));
+	if (mtd->egress.sfx_tbl)
+		claim_zero(flow_dv_tbl_resource_release(dev,
+							mtd->egress.sfx_tbl));
 	if (mtd->ingress.color_matcher)
 		claim_zero(mlx5_glue->dv_destroy_flow_matcher
 			  (mtd->ingress.color_matcher));
@@ -8525,6 +8528,9 @@ struct field_modify_info modify_tcp[] = {
 	if (mtd->ingress.tbl)
 		claim_zero(flow_dv_tbl_resource_release(dev,
 							mtd->ingress.tbl));
+	if (mtd->ingress.sfx_tbl)
+		claim_zero(flow_dv_tbl_resource_release(dev,
+							mtd->ingress.sfx_tbl));
 	if (mtd->transfer.color_matcher)
 		claim_zero(mlx5_glue->dv_destroy_flow_matcher
 			  (mtd->transfer.color_matcher));
@@ -8534,6 +8540,9 @@ struct field_modify_info modify_tcp[] = {
 	if (mtd->transfer.tbl)
 		claim_zero(flow_dv_tbl_resource_release(dev,
 							mtd->transfer.tbl));
+	if (mtd->transfer.sfx_tbl)
+		claim_zero(flow_dv_tbl_resource_release(dev,
+							mtd->transfer.sfx_tbl));
 	if (mtd->drop_actn)
 		claim_zero(mlx5_glue->destroy_flow_action(mtd->drop_actn));
 	rte_free(mtd);
@@ -8581,31 +8590,16 @@ struct field_modify_info modify_tcp[] = {
 		.match_mask = (void *)&mask,
 	};
 	void *actions[METER_ACTIONS];
-	struct mlx5_flow_tbl_resource **sfx_tbl;
 	struct mlx5_meter_domain_info *dtb;
 	struct rte_flow_error error;
 	int i = 0;
 
-	if (transfer) {
-		sfx_tbl = &sh->fdb_mtr_sfx_tbl;
+	if (transfer)
 		dtb = &mtb->transfer;
-	} else if (egress) {
-		sfx_tbl = &sh->tx_mtr_sfx_tbl;
+	else if (egress)
 		dtb = &mtb->egress;
-	} else {
-		sfx_tbl = &sh->rx_mtr_sfx_tbl;
+	else
 		dtb = &mtb->ingress;
-	}
-	/* If the suffix table in missing, create it. */
-	if (!(*sfx_tbl)) {
-		*sfx_tbl = flow_dv_tbl_resource_get(dev,
-						MLX5_FLOW_TABLE_LEVEL_SUFFIX,
-						egress, transfer, &error);
-		if (!(*sfx_tbl)) {
-			DRV_LOG(ERR, "Failed to create meter suffix table.");
-			return -1;
-		}
-	}
 	/* Create the meter table with METER level. */
 	dtb->tbl = flow_dv_tbl_resource_get(dev, MLX5_FLOW_TABLE_LEVEL_METER,
 					    egress, transfer, &error);
@@ -8613,6 +8607,14 @@ struct field_modify_info modify_tcp[] = {
 		DRV_LOG(ERR, "Failed to create meter policer table.");
 		return -1;
 	}
+	/* Create the meter suffix table with SUFFIX level. */
+	dtb->sfx_tbl = flow_dv_tbl_resource_get(dev,
+					    MLX5_FLOW_TABLE_LEVEL_SUFFIX,
+					    egress, transfer, &error);
+	if (!dtb->sfx_tbl) {
+		DRV_LOG(ERR, "Failed to create meter suffix table.");
+		return -1;
+	}
 	/* Create matchers, Any and Color. */
 	dv_attr.priority = 3;
 	dv_attr.match_criteria_enable = 0;
@@ -8786,8 +8788,6 @@ struct field_modify_info modify_tcp[] = {
  *   Pointer to flow meter structure.
  * @param[in] mtb
  *   Pointer to DV meter table set.
- * @param[in] sfx_tb
- *   Pointer to suffix table.
  * @param[in] mtr_reg_c
  *   Color match REG_C.
  *
@@ -8797,7 +8797,6 @@ struct field_modify_info modify_tcp[] = {
 static int
 flow_dv_create_policer_forward_rule(struct mlx5_flow_meter *fm,
 				    struct mlx5_meter_domain_info *dtb,
-				    struct mlx5_flow_tbl_resource *sfx_tb,
 				    uint8_t mtr_reg_c)
 {
 	struct mlx5_flow_dv_match_params matcher = {
@@ -8811,12 +8810,10 @@ struct field_modify_info modify_tcp[] = {
 	int i;
 
 	/* Create jump action. */
-	if (!sfx_tb)
-		return -1;
 	if (!dtb->jump_actn)
 		dtb->jump_actn =
 			mlx5_glue->dr_create_flow_action_dest_flow_tbl
-							(sfx_tb->obj);
+							(dtb->sfx_tbl->obj);
 	if (!dtb->jump_actn) {
 		DRV_LOG(ERR, "Failed to create policer jump action.");
 		goto error;
@@ -8871,7 +8868,6 @@ struct field_modify_info modify_tcp[] = {
 
 	if (attr->egress) {
 		ret = flow_dv_create_policer_forward_rule(fm, &mtb->egress,
-						priv->sh->tx_mtr_sfx_tbl,
 						priv->mtr_color_reg);
 		if (ret) {
 			DRV_LOG(ERR, "Failed to create egress policer.");
@@ -8880,7 +8876,6 @@ struct field_modify_info modify_tcp[] = {
 	}
 	if (attr->ingress) {
 		ret = flow_dv_create_policer_forward_rule(fm, &mtb->ingress,
-						priv->sh->rx_mtr_sfx_tbl,
 						priv->mtr_color_reg);
 		if (ret) {
 			DRV_LOG(ERR, "Failed to create ingress policer.");
@@ -8889,7 +8884,6 @@ struct field_modify_info modify_tcp[] = {
 	}
 	if (attr->transfer) {
 		ret = flow_dv_create_policer_forward_rule(fm, &mtb->transfer,
-						priv->sh->fdb_mtr_sfx_tbl,
 						priv->mtr_color_reg);
 		if (ret) {
 			DRV_LOG(ERR, "Failed to create transfer policer.");
-- 
1.8.3.1

[dpdk-dev] [PATCH 2/2] net/mlx5: fix jump table leak issue

[Suanming Mou]

Currently, when translate jump action, the table reference will be
increased all the time. But when release the jump action, the table
resource reference will only be decreased when jump action is released.
It means for jump action which was referenced more than one time, the
increased table reference only decrease one time when jump action is
released.

Add table release when the jump action was not new created.

Fixes: 684b9a1b1f5c ("net/mlx5: support jump action")
Cc: stable@dpdk.org

Signed-off-by: Suanming Mou <suanmingm@mellanox.com>
Acked-by: Viacheslav Ovsiienko <viacheslavo@mellanox.com>
---
 drivers/net/mlx5/mlx5_flow_dv.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/drivers/net/mlx5/mlx5_flow_dv.c b/drivers/net/mlx5/mlx5_flow_dv.c
index 0676b6f..e87e30f 100644
--- a/drivers/net/mlx5/mlx5_flow_dv.c
+++ b/drivers/net/mlx5/mlx5_flow_dv.c
@@ -73,6 +73,10 @@
 	uint32_t attr;
 };
 
+static int
+flow_dv_tbl_resource_release(struct rte_eth_dev *dev,
+			     struct mlx5_flow_tbl_resource *tbl);
+
 /**
  * Initialize flow attributes structure according to flow items' types.
  *
@@ -2530,6 +2534,8 @@ struct field_modify_info modify_tcp[] = {
 		DRV_LOG(DEBUG, "new jump table resource %p: refcnt %d++",
 			(void *)&tbl_data->jump, cnt);
 	} else {
+		/* old jump should not make the table ref++. */
+		flow_dv_tbl_resource_release(dev, &tbl_data->tbl);
 		MLX5_ASSERT(tbl_data->jump.action);
 		DRV_LOG(DEBUG, "existed jump table resource %p: refcnt %d++",
 			(void *)&tbl_data->jump, cnt);
-- 
1.8.3.1

Re: [dpdk-dev] [PATCH 0/2] net/mlx5: fix table leak issue

[Raslan Darawsheh]

Hi,

> -----Original Message-----
> From: Suanming Mou <suanmingm@mellanox.com>
> Sent: Monday, April 13, 2020 4:29 PM
> Cc: Raslan Darawsheh <rasland@mellanox.com>; dev@dpdk.org
> Subject: [PATCH 0/2] net/mlx5: fix table leak issue
> 
> Currently, the tables got by jump action and flow meter suffix flow are
> not put correctly.
> 
> Fix the bugs with the refer_cnt issue.
> 
> Suanming Mou (2):
>   net/mlx5: fix meter suffix table leak issue
>   net/mlx5: fix jump table leak issue
> 
>  drivers/net/mlx5/mlx5.h         |  6 -----
>  drivers/net/mlx5/mlx5_flow.h    |  2 ++
>  drivers/net/mlx5/mlx5_flow_dv.c | 54 ++++++++++++++++++++--------------
> -------
>  3 files changed, 29 insertions(+), 33 deletions(-)
> 
> --
> 1.8.3.1


Series applied to next-net-mlx,

Kindest regards,
Raslan Darawsheh