[dpdk-dev] [PATCH 6/6] net/mlx5: make shared counters thread safe

[Suanming Mou <suanmingm@nvidia.com>]

The shared counters save the counter index to three level table. As
three level table supports multiple-thread opertations now, the shared
counters can take advantage of the table to support multiple-thread.

Once multiple threads saves the same ID counter to the same table entry
at the same time, only one will be success, others will get the EEXIST
errno with entry reference counter increase. In this case, the other
duplicate created counter will be released.

Signed-off-by: Suanming Mou <suanmingm@nvidia.com>
---
 drivers/net/mlx5/mlx5.h            |  1 -
 drivers/net/mlx5/mlx5_flow_dv.c    | 62 +++++++++++---------------------------
 drivers/net/mlx5/mlx5_flow_verbs.c | 19 +++---------
 3 files changed, 21 insertions(+), 61 deletions(-)

diff --git a/drivers/net/mlx5/mlx5.h b/drivers/net/mlx5/mlx5.h
index 8c951e2..6e0b2e2 100644
--- a/drivers/net/mlx5/mlx5.h
+++ b/drivers/net/mlx5/mlx5.h
@@ -348,7 +348,6 @@ struct flow_counter_stats {
 
 /* Shared counters information for counters. */
 struct mlx5_flow_counter_shared {
-	uint32_t ref_cnt; /**< Reference counter. */
 	uint32_t id; /**< User counter ID. */
 };
 
diff --git a/drivers/net/mlx5/mlx5_flow_dv.c b/drivers/net/mlx5/mlx5_flow_dv.c
index 31d7fe4..3adb905 100644
--- a/drivers/net/mlx5/mlx5_flow_dv.c
+++ b/drivers/net/mlx5/mlx5_flow_dv.c
@@ -4461,28 +4461,6 @@ struct field_modify_info modify_tcp[] = {
 }
 
 /**
- * Search for existed shared counter.
- *
- * @param[in] dev
- *   Pointer to the Ethernet device structure.
- * @param[in] id
- *   The shared counter ID to search.
- *
- * @return
- *   0 if not existed, otherwise shared counter index.
- */
-static uint32_t
-flow_dv_counter_shared_search(struct rte_eth_dev *dev, uint32_t id)
-{
-	struct mlx5_priv *priv = dev->data->dev_private;
-	union mlx5_l3t_data data;
-
-	if (mlx5_l3t_get_entry(priv->sh->cnt_id_tbl, id, &data))
-		return 0;
-	return data.dword;
-}
-
-/**
  * Allocate a flow counter.
  *
  * @param[in] dev
@@ -4510,24 +4488,15 @@ struct field_modify_info modify_tcp[] = {
 	enum mlx5_counter_type cnt_type =
 			age ? MLX5_COUNTER_TYPE_AGE : MLX5_COUNTER_TYPE_ORIGIN;
 	uint32_t cnt_idx;
+	union mlx5_l3t_data data;
 
 	if (!priv->config.devx) {
 		rte_errno = ENOTSUP;
 		return 0;
 	}
-	if (shared) {
-		cnt_idx = flow_dv_counter_shared_search(dev, id);
-		if (cnt_idx) {
-			cnt_free = flow_dv_counter_get_by_idx(dev, cnt_idx,
-							      NULL);
-			if (cnt_free->shared_info.ref_cnt + 1 == 0) {
-				rte_errno = E2BIG;
-				return 0;
-			}
-			cnt_free->shared_info.ref_cnt++;
-			return cnt_idx;
-		}
-	}
+	if (shared && !mlx5_l3t_get_entry(priv->sh->cnt_id_tbl, id, &data) &&
+	    data.dword)
+		return data.dword;
 	/* Get free counters from container. */
 	rte_spinlock_lock(&cmng->csl);
 	cnt_free = TAILQ_FIRST(&cmng->counters[cnt_type]);
@@ -4566,12 +4535,18 @@ struct field_modify_info modify_tcp[] = {
 				 &cnt_free->bytes))
 		goto err;
 	if (shared) {
-		union mlx5_l3t_data data;
-
 		data.dword = cnt_idx;
-		if (mlx5_l3t_set_entry(priv->sh->cnt_id_tbl, id, &data))
+		if (mlx5_l3t_set_entry(priv->sh->cnt_id_tbl, id, &data)) {
+			if (rte_errno == EEXIST) {
+				cnt_free->pool = pool;
+				rte_spinlock_lock(&cmng->csl);
+				TAILQ_INSERT_TAIL(&cmng->counters[cnt_type],
+						  cnt_free, next);
+				rte_spinlock_unlock(&cmng->csl);
+				return data.dword;
+			}
 			goto err;
-		cnt_free->shared_info.ref_cnt = 1;
+		}
 		cnt_free->shared_info.id = id;
 		cnt_idx |= MLX5_CNT_SHARED_OFFSET;
 	}
@@ -4667,12 +4642,9 @@ struct field_modify_info modify_tcp[] = {
 		return;
 	cnt = flow_dv_counter_get_by_idx(dev, counter, &pool);
 	MLX5_ASSERT(pool);
-	if (IS_SHARED_CNT(counter)) {
-		if (--cnt->shared_info.ref_cnt)
-			return;
-		mlx5_l3t_clear_entry(priv->sh->cnt_id_tbl,
-				     cnt->shared_info.id);
-	}
+	if (IS_SHARED_CNT(counter) &&
+	    mlx5_l3t_clear_entry(priv->sh->cnt_id_tbl, cnt->shared_info.id))
+		return;
 	if (IS_AGE_POOL(pool))
 		flow_dv_counter_remove_from_age(dev, counter, cnt);
 	cnt->pool = pool;
diff --git a/drivers/net/mlx5/mlx5_flow_verbs.c b/drivers/net/mlx5/mlx5_flow_verbs.c
index 69ecc27..f351a68 100644
--- a/drivers/net/mlx5/mlx5_flow_verbs.c
+++ b/drivers/net/mlx5/mlx5_flow_verbs.c
@@ -265,15 +265,8 @@
 	int ret;
 
 	if (shared && !mlx5_l3t_get_entry(priv->sh->cnt_id_tbl, id, &data) &&
-	    data.dword) {
-		cnt = flow_verbs_counter_get_by_idx(dev, data.dword, NULL);
-		if (cnt->shared_info.ref_cnt + 1 == 0) {
-			rte_errno = E2BIG;
-			return 0;
-		}
-		cnt->shared_info.ref_cnt++;
+	    data.dword)
 		return data.dword;
-	}
 	for (pool_idx = 0; pool_idx < n_valid; ++pool_idx) {
 		pool = cmng->pools[pool_idx];
 		if (!pool)
@@ -325,7 +318,6 @@
 		data.dword = cnt_idx;
 		if (mlx5_l3t_set_entry(priv->sh->cnt_id_tbl, id, &data))
 			return 0;
-		cnt->shared_info.ref_cnt = 1;
 		cnt->shared_info.id = id;
 		cnt_idx |= MLX5_CNT_SHARED_OFFSET;
 	}
@@ -360,12 +352,9 @@
 	struct mlx5_flow_counter_ext *cnt_ext;
 
 	cnt = flow_verbs_counter_get_by_idx(dev, counter, &pool);
-	if (IS_SHARED_CNT(counter)) {
-		if (--cnt->shared_info.ref_cnt)
-			return;
-		mlx5_l3t_clear_entry(priv->sh->cnt_id_tbl,
-				     cnt->shared_info.id);
-	}
+	if (IS_SHARED_CNT(counter) &&
+	    mlx5_l3t_clear_entry(priv->sh->cnt_id_tbl, cnt->shared_info.id))
+		return;
 	cnt_ext = MLX5_CNT_TO_CNT_EXT(pool, cnt);
 #if defined(HAVE_IBV_DEVICE_COUNTERS_SET_V42)
 	claim_zero(mlx5_glue->destroy_counter_set(cnt_ext->cs));
-- 
1.8.3.1