[v4,27/47] net/bnxt: tf_ulp: fixed parent child db counters

Message ID 20241004175338.3156160-28-sriharsha.basavapatna@broadcom.com (mailing list archive)
State Changes Requested
Delegated to: Ajit Khaparde
Headers
Series TruFlow update for Thor2 |

Checks

Context Check Description
ci/checkpatch success coding style OK

Commit Message

Sriharsha Basavapatna Oct. 4, 2024, 5:53 p.m. UTC
From: Kishore Padmanabha <kishore.padmanabha@broadcom.com>

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.

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: Michael Baucom <michael.baucom@broadcom.com>
Reviewed-by: Ajit Khaparde <ajit.khaparde@broadcom.com>
---
 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_flow_db.h |  2 ++
 drivers/net/bnxt/tf_ulp/ulp_matcher.c | 12 +++++++--
 4 files changed, 50 insertions(+), 23 deletions(-)
  

Patch

diff --git a/drivers/net/bnxt/tf_ulp/ulp_fc_mgr.c b/drivers/net/bnxt/tf_ulp/ulp_fc_mgr.c
index b880b545da..0c46c7d4c9 100644
--- a/drivers/net/bnxt/tf_ulp/ulp_fc_mgr.c
+++ b/drivers/net/bnxt/tf_ulp/ulp_fc_mgr.c
@@ -563,23 +563,29 @@  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;
-	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;
+	}
 
 	if (!found_parent_flow &&
 	    params.resource_sub_type ==
 			BNXT_ULP_RESOURCE_SUB_TYPE_INDEX_TABLE_INT_COUNT) {
 		hw_cntr_id = params.resource_hndl;
-		if (!ulp_fc_info->num_counters)
-			return fc_ops->ulp_flow_stat_get(ctxt, dir,
-							 hw_cntr_id, count);
+		if (!ulp_fc_info->num_counters) {
+			rc = fc_ops->ulp_flow_stat_get(ctxt, dir,
+						       hw_cntr_id, count);
+			bnxt_ulp_cntxt_release_fdb_lock(ctxt);
+			return rc;
+		}
 
 		/* TODO:
 		 * Think about optimizing with try_lock later
@@ -603,9 +609,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)
@@ -613,7 +624,7 @@  int ulp_fc_mgr_query_count_get(struct bnxt_ulp_context *ctxt,
 	} else {
 		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 099ae7adc8..679dab0f17 100644
--- a/drivers/net/bnxt/tf_ulp/ulp_flow_db.c
+++ b/drivers/net/bnxt/tf_ulp/ulp_flow_db.c
@@ -836,7 +836,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;
 }
@@ -1383,13 +1382,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_DRV_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;
@@ -1444,7 +1442,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;
@@ -1536,7 +1534,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_DRV_DBG(ERR, "Invalid parent child index %x\n", pc_idx);
 		return -EINVAL;
 	}
@@ -1784,6 +1782,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)
 {
@@ -1796,6 +1795,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 39810c81c4..5a4b5a1ebf 100644
--- a/drivers/net/bnxt/tf_ulp/ulp_flow_db.h
+++ b/drivers/net/bnxt/tf_ulp/ulp_flow_db.h
@@ -63,6 +63,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;
@@ -389,6 +390,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,
diff --git a/drivers/net/bnxt/tf_ulp/ulp_matcher.c b/drivers/net/bnxt/tf_ulp/ulp_matcher.c
index 2284fb6423..9e0a9458e7 100644
--- a/drivers/net/bnxt/tf_ulp/ulp_matcher.c
+++ b/drivers/net/bnxt/tf_ulp/ulp_matcher.c
@@ -327,9 +327,17 @@  ulp_matcher_action_match(struct ulp_rte_parser_params *params,
 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);