patch 'net/bnxt/tf_ulp: fix parent child DB counters' has been queued to stable release 22.11.7

[luca.boccassi@gmail.com]

Hi,

FYI, your patch has been queued to stable release 22.11.7

Note it hasn't been pushed to http://dpdk.org/browse/dpdk-stable yet.
It will be pushed if I get no objections before 11/22/24. So please
shout if anyone has objections.

Also note that after the patch there's a diff of the upstream commit vs the
patch applied to the branch. This will indicate if there was any rebasing
needed to apply to the stable branch. If there were code changes for rebasing
(ie: not only metadata diffs), please double check that the rebase was
correctly done.

Queued patches are on a temporary branch at:
https://github.com/bluca/dpdk-stable

This queued commit can be viewed at:
https://github.com/bluca/dpdk-stable/commit/768828f6a2b947f0641562ca8a4d0b88170244f6

Thanks.

Luca Boccassi

---
From 768828f6a2b947f0641562ca8a4d0b88170244f6 Mon Sep 17 00:00:00 2001
From: Kishore Padmanabha <kishore.padmanabha@broadcom.com>
Date: Thu, 7 Nov 2024 19:22:34 +0530
Subject: [PATCH] net/bnxt/tf_ulp: fix parent child DB counters

[ upstream commit 8782e4de3ef2e55bd4aed98dc18e26d2bfc83868 ]

The locking for the parent child counters need to be done till the
stats are retrieved. Also the OVS is creating multiple F1 flows for
same tunnel hence reference count needs to be maintined for the F1
flows.

Fix name conflicts for class and action tables. Matcher allocates
hash tables for class and action entries. These tables should have
different names for each port.

Fixes: bdf4a3c6316b ("net/bnxt: support tunnel offload")

Signed-off-by: Kishore Padmanabha <kishore.padmanabha@broadcom.com>
Signed-off-by: Shuanglin Wang <shuanglin.wang@broadcom.com>
Signed-off-by: Sriharsha Basavapatna <sriharsha.basavapatna@broadcom.com>
Reviewed-by: Mike Baucom <michael.baucom@broadcom.com>
Reviewed-by: Ajit Khaparde <ajit.khaparde@broadcom.com>
---
 drivers/net/bnxt/tf_ulp/ulp_fc_mgr.c  | 26 +++++++++++++++++---------
 drivers/net/bnxt/tf_ulp/ulp_flow_db.c | 21 ++++++++++++++-------
 drivers/net/bnxt/tf_ulp/ulp_flow_db.h |  2 ++
 3 files changed, 33 insertions(+), 16 deletions(-)

diff --git a/drivers/net/bnxt/tf_ulp/ulp_fc_mgr.c b/drivers/net/bnxt/tf_ulp/ulp_fc_mgr.c
index 85c9cbb7f2..69a244bd65 100644
--- a/drivers/net/bnxt/tf_ulp/ulp_fc_mgr.c
+++ b/drivers/net/bnxt/tf_ulp/ulp_fc_mgr.c
@@ -720,18 +720,21 @@ int ulp_fc_mgr_query_count_get(struct bnxt_ulp_context *ctxt,
 
 	} while (!rc && nxt_resource_index);
 
-	bnxt_ulp_cntxt_release_fdb_lock(ctxt);
-
-	if (rc || !found_cntr_resource)
+	if (rc || !found_cntr_resource) {
+		bnxt_ulp_cntxt_release_fdb_lock(ctxt);
 		return rc;
+	}
 
 	dir = params.direction;
 	hw_cntr_id = params.resource_hndl;
 	if (!found_parent_flow &&
 	    params.resource_sub_type ==
 			BNXT_ULP_RESOURCE_SUB_TYPE_INDEX_TABLE_INT_COUNT) {
-		if (!ulp_fc_info->num_counters)
-			return ulp_fc_tf_flow_stat_get(ctxt, &params, count);
+		if (!ulp_fc_info->num_counters) {
+			rc = ulp_fc_tf_flow_stat_get(ctxt, &params, count);
+			bnxt_ulp_cntxt_release_fdb_lock(ctxt);
+			return rc;
+		}
 
 		/* TODO:
 		 * Think about optimizing with try_lock later
@@ -755,9 +758,14 @@ int ulp_fc_mgr_query_count_get(struct bnxt_ulp_context *ctxt,
 		   params.resource_sub_type ==
 			BNXT_ULP_RESOURCE_SUB_TYPE_INDEX_TABLE_INT_COUNT) {
 		/* Get stats from the parent child table */
-		ulp_flow_db_parent_flow_count_get(ctxt, pc_idx,
-						  &count->hits, &count->bytes,
-						  count->reset);
+		if (ulp_flow_db_parent_flow_count_get(ctxt, flow_id,
+						      pc_idx,
+						      &count->hits,
+						      &count->bytes,
+						      count->reset)) {
+			bnxt_ulp_cntxt_release_fdb_lock(ctxt);
+			return -EIO;
+		}
 		if (count->hits)
 			count->hits_set = 1;
 		if (count->bytes)
@@ -766,7 +774,7 @@ int ulp_fc_mgr_query_count_get(struct bnxt_ulp_context *ctxt,
 		/* TBD: Handle External counters */
 		rc = -EINVAL;
 	}
-
+	bnxt_ulp_cntxt_release_fdb_lock(ctxt);
 	return rc;
 }
 
diff --git a/drivers/net/bnxt/tf_ulp/ulp_flow_db.c b/drivers/net/bnxt/tf_ulp/ulp_flow_db.c
index 9968311c44..51ea3203bc 100644
--- a/drivers/net/bnxt/tf_ulp/ulp_flow_db.c
+++ b/drivers/net/bnxt/tf_ulp/ulp_flow_db.c
@@ -1361,13 +1361,12 @@ ulp_flow_db_pc_db_parent_flow_set(struct bnxt_ulp_context *ulp_ctxt,
 
 	if (set_flag) {
 		pc_entry->parent_fid = parent_fid;
+		pc_entry->parent_ref_cnt++;
 	} else {
-		if (pc_entry->parent_fid != parent_fid)
-			BNXT_TF_DBG(ERR, "Panic: invalid parent id\n");
-		pc_entry->parent_fid = 0;
-
+		if (pc_entry->parent_ref_cnt > 0)
+			pc_entry->parent_ref_cnt--;
 		/* Free the parent child db entry if no user present */
-		if (!pc_entry->f2_cnt)
+		if (!pc_entry->parent_ref_cnt && !pc_entry->f2_cnt)
 			ulp_flow_db_pc_db_entry_free(ulp_ctxt, pc_entry);
 	}
 	return 0;
@@ -1422,7 +1421,7 @@ ulp_flow_db_pc_db_child_flow_set(struct bnxt_ulp_context *ulp_ctxt,
 		ULP_INDEX_BITMAP_RESET(t[a_idx], child_fid);
 		if (pc_entry->f2_cnt)
 			pc_entry->f2_cnt--;
-		if (!pc_entry->f2_cnt && !pc_entry->parent_fid)
+		if (!pc_entry->f2_cnt && !pc_entry->parent_ref_cnt)
 			ulp_flow_db_pc_db_entry_free(ulp_ctxt, pc_entry);
 	}
 	return 0;
@@ -1514,7 +1513,7 @@ ulp_flow_db_parent_flow_count_accum_set(struct bnxt_ulp_context *ulp_ctxt,
 	/* check for parent idx validity */
 	p_pdb = &flow_db->parent_child_db;
 	if (pc_idx >= p_pdb->entries_count ||
-	    !p_pdb->parent_flow_tbl[pc_idx].parent_fid) {
+	    !p_pdb->parent_flow_tbl[pc_idx].parent_ref_cnt) {
 		BNXT_TF_DBG(ERR, "Invalid parent child index %x\n", pc_idx);
 		return -EINVAL;
 	}
@@ -1761,6 +1760,7 @@ ulp_flow_db_parent_flow_count_update(struct bnxt_ulp_context *ulp_ctxt,
  */
 int32_t
 ulp_flow_db_parent_flow_count_get(struct bnxt_ulp_context *ulp_ctxt,
+				  uint32_t flow_id,
 				  uint32_t pc_idx, uint64_t *packet_count,
 				  uint64_t *byte_count, uint8_t count_reset)
 {
@@ -1773,6 +1773,13 @@ ulp_flow_db_parent_flow_count_get(struct bnxt_ulp_context *ulp_ctxt,
 		return -EINVAL;
 	}
 
+	/* stale parent fid */
+	if (flow_id != pc_entry->parent_fid) {
+		*packet_count = 0;
+		*byte_count = 0;
+		return 0;
+	}
+
 	if (pc_entry->counter_acc) {
 		*packet_count = pc_entry->pkt_count;
 		*byte_count = pc_entry->byte_count;
diff --git a/drivers/net/bnxt/tf_ulp/ulp_flow_db.h b/drivers/net/bnxt/tf_ulp/ulp_flow_db.h
index 2b02836a40..31afd55094 100644
--- a/drivers/net/bnxt/tf_ulp/ulp_flow_db.h
+++ b/drivers/net/bnxt/tf_ulp/ulp_flow_db.h
@@ -60,6 +60,7 @@ struct bnxt_ulp_flow_tbl {
 struct ulp_fdb_parent_info {
 	uint32_t	valid;
 	uint32_t	parent_fid;
+	uint32_t	parent_ref_cnt;
 	uint32_t	counter_acc;
 	uint64_t	pkt_count;
 	uint64_t	byte_count;
@@ -385,6 +386,7 @@ ulp_flow_db_parent_flow_count_update(struct bnxt_ulp_context *ulp_ctxt,
  */
 int32_t
 ulp_flow_db_parent_flow_count_get(struct bnxt_ulp_context *ulp_ctxt,
+				  uint32_t flow_id,
 				  uint32_t pc_idx,
 				  uint64_t *packet_count,
 				  uint64_t *byte_count,
-- 
2.45.2

---
  Diff of the applied patch vs upstream commit (please double-check if non-empty:
---
--- -	2024-11-20 23:41:22.833319971 +0000
+++ 0002-net-bnxt-tf_ulp-fix-parent-child-DB-counters.patch	2024-11-20 23:41:22.680195466 +0000
@@ -1 +1 @@
-From 8782e4de3ef2e55bd4aed98dc18e26d2bfc83868 Mon Sep 17 00:00:00 2001
+From 768828f6a2b947f0641562ca8a4d0b88170244f6 Mon Sep 17 00:00:00 2001
@@ -5,0 +6,2 @@
+[ upstream commit 8782e4de3ef2e55bd4aed98dc18e26d2bfc83868 ]
+
@@ -16 +17,0 @@
-Cc: stable@dpdk.org
@@ -24,2 +25,2 @@
- drivers/net/bnxt/tf_ulp/ulp_fc_mgr.c  | 37 +++++++++++++++++----------
- drivers/net/bnxt/tf_ulp/ulp_flow_db.c | 22 ++++++++++------
+ drivers/net/bnxt/tf_ulp/ulp_fc_mgr.c  | 26 +++++++++++++++++---------
+ drivers/net/bnxt/tf_ulp/ulp_flow_db.c | 21 ++++++++++++++-------
@@ -27,2 +28 @@
- drivers/net/bnxt/tf_ulp/ulp_matcher.c | 12 +++++++--
- 4 files changed, 50 insertions(+), 23 deletions(-)
+ 3 files changed, 33 insertions(+), 16 deletions(-)
@@ -31 +31 @@
-index b880b545da..0c46c7d4c9 100644
+index 85c9cbb7f2..69a244bd65 100644
@@ -34 +34 @@
-@@ -563,23 +563,29 @@ int ulp_fc_mgr_query_count_get(struct bnxt_ulp_context *ctxt,
+@@ -720,18 +720,21 @@ int ulp_fc_mgr_query_count_get(struct bnxt_ulp_context *ctxt,
@@ -47,10 +47 @@
--	if (!(ulp_fc_info->flags & ULP_FLAG_FC_SW_AGG_EN))
--		return fc_ops->ulp_flow_stat_get(ctxt, dir,
--						 params.resource_hndl, count);
-+	if (!(ulp_fc_info->flags & ULP_FLAG_FC_SW_AGG_EN)) {
-+		rc = fc_ops->ulp_flow_stat_get(ctxt, dir,
-+					       params.resource_hndl, count);
-+		bnxt_ulp_cntxt_release_fdb_lock(ctxt);
-+		return rc;
-+	}
- 
+ 	hw_cntr_id = params.resource_hndl;
@@ -60 +50,0 @@
- 		hw_cntr_id = params.resource_hndl;
@@ -62,2 +52 @@
--			return fc_ops->ulp_flow_stat_get(ctxt, dir,
--							 hw_cntr_id, count);
+-			return ulp_fc_tf_flow_stat_get(ctxt, &params, count);
@@ -65,2 +54 @@
-+			rc = fc_ops->ulp_flow_stat_get(ctxt, dir,
-+						       hw_cntr_id, count);
++			rc = ulp_fc_tf_flow_stat_get(ctxt, &params, count);
@@ -73 +61 @@
-@@ -603,9 +609,14 @@ int ulp_fc_mgr_query_count_get(struct bnxt_ulp_context *ctxt,
+@@ -755,9 +758,14 @@ int ulp_fc_mgr_query_count_get(struct bnxt_ulp_context *ctxt,
@@ -91,2 +79,2 @@
-@@ -613,7 +624,7 @@ int ulp_fc_mgr_query_count_get(struct bnxt_ulp_context *ctxt,
- 	} else {
+@@ -766,7 +774,7 @@ int ulp_fc_mgr_query_count_get(struct bnxt_ulp_context *ctxt,
+ 		/* TBD: Handle External counters */
@@ -101 +89 @@
-index 2ff5227774..27f1115990 100644
+index 9968311c44..51ea3203bc 100644
@@ -104,9 +92 @@
-@@ -838,7 +838,6 @@ ulp_flow_db_fid_free(struct bnxt_ulp_context *ulp_ctxt,
- #ifdef RTE_LIBRTE_BNXT_TRUFLOW_DEBUG
- 	BNXT_DRV_DBG(DEBUG, "flow_id = %u:%u freed\n", flow_type, fid);
- #endif
--
- 	/* all good, return success */
- 	return 0;
- }
-@@ -1385,13 +1384,12 @@ ulp_flow_db_pc_db_parent_flow_set(struct bnxt_ulp_context *ulp_ctxt,
+@@ -1361,13 +1361,12 @@ ulp_flow_db_pc_db_parent_flow_set(struct bnxt_ulp_context *ulp_ctxt,
@@ -119 +99 @@
--			BNXT_DRV_DBG(ERR, "Panic: invalid parent id\n");
+-			BNXT_TF_DBG(ERR, "Panic: invalid parent id\n");
@@ -130 +110 @@
-@@ -1446,7 +1444,7 @@ ulp_flow_db_pc_db_child_flow_set(struct bnxt_ulp_context *ulp_ctxt,
+@@ -1422,7 +1421,7 @@ ulp_flow_db_pc_db_child_flow_set(struct bnxt_ulp_context *ulp_ctxt,
@@ -139 +119 @@
-@@ -1538,7 +1536,7 @@ ulp_flow_db_parent_flow_count_accum_set(struct bnxt_ulp_context *ulp_ctxt,
+@@ -1514,7 +1513,7 @@ ulp_flow_db_parent_flow_count_accum_set(struct bnxt_ulp_context *ulp_ctxt,
@@ -145 +125 @@
- 		BNXT_DRV_DBG(ERR, "Invalid parent child index %x\n", pc_idx);
+ 		BNXT_TF_DBG(ERR, "Invalid parent child index %x\n", pc_idx);
@@ -148 +128 @@
-@@ -1786,6 +1784,7 @@ ulp_flow_db_parent_flow_count_update(struct bnxt_ulp_context *ulp_ctxt,
+@@ -1761,6 +1760,7 @@ ulp_flow_db_parent_flow_count_update(struct bnxt_ulp_context *ulp_ctxt,
@@ -156 +136 @@
-@@ -1798,6 +1797,13 @@ ulp_flow_db_parent_flow_count_get(struct bnxt_ulp_context *ulp_ctxt,
+@@ -1773,6 +1773,13 @@ ulp_flow_db_parent_flow_count_get(struct bnxt_ulp_context *ulp_ctxt,
@@ -171 +151 @@
-index 39810c81c4..5a4b5a1ebf 100644
+index 2b02836a40..31afd55094 100644
@@ -174 +154 @@
-@@ -63,6 +63,7 @@ struct bnxt_ulp_flow_tbl {
+@@ -60,6 +60,7 @@ struct bnxt_ulp_flow_tbl {
@@ -182 +162 @@
-@@ -389,6 +390,7 @@ ulp_flow_db_parent_flow_count_update(struct bnxt_ulp_context *ulp_ctxt,
+@@ -385,6 +386,7 @@ ulp_flow_db_parent_flow_count_update(struct bnxt_ulp_context *ulp_ctxt,
@@ -190,24 +169,0 @@
-diff --git a/drivers/net/bnxt/tf_ulp/ulp_matcher.c b/drivers/net/bnxt/tf_ulp/ulp_matcher.c
-index b508bd2df6..9615fc7919 100644
---- a/drivers/net/bnxt/tf_ulp/ulp_matcher.c
-+++ b/drivers/net/bnxt/tf_ulp/ulp_matcher.c
-@@ -327,9 +327,17 @@ error:
- int32_t ulp_matcher_init(struct bnxt_ulp_context *ulp_ctx)
- {
- 	struct rte_hash_parameters hash_tbl_params = {0};
--	char hash_class_tbl_name[64] = "bnxt_ulp_class_matcher";
--	char hash_act_tbl_name[64] = "bnxt_ulp_act_matcher";
-+	char hash_class_tbl_name[64] = {0};
-+	char hash_act_tbl_name[64] = {0};
- 	struct bnxt_ulp_matcher_data *data;
-+	uint16_t port_id;
-+
-+	/* append port_id to the buffer name */
-+	port_id = ulp_ctx->bp->eth_dev->data->port_id;
-+	snprintf(hash_class_tbl_name, sizeof(hash_class_tbl_name),
-+		 "bnxt_ulp_class_matcher_%d", port_id);
-+	snprintf(hash_act_tbl_name, sizeof(hash_act_tbl_name),
-+		 "bnxt_ulp_act_matcher_%d", port_id);
- 
- 	data = rte_zmalloc("bnxt_ulp_matcher_data",
- 			   sizeof(struct bnxt_ulp_matcher_data), 0);