[dpdk-dev] [PATCH v2 2/8] net/mlx5: optimize shared counter memory

[Suanming Mou <suanmingm@nvidia.com>]

Instead of using special memory to indicate shared counter, this patch does
the optimization to use the counter handler reserved memory to indicate it.
The counter index with MLX5_CNT_SHARED_OFFSET means the shared counter.

This patch is also an arrangement for a new adjustment to use batch counter
as shared counter.

Signed-off-by: Suanming Mou <suanmingm@nvidia.com>
Acked-by: Matan Azrad <matan@nvidia.com>
---
 drivers/net/mlx5/mlx5.h            | 36 +++++++++++++-----
 drivers/net/mlx5/mlx5_flow_dv.c    | 78 +++++++++++++++-----------------------
 drivers/net/mlx5/mlx5_flow_verbs.c | 60 +++++++++++++++++------------
 3 files changed, 93 insertions(+), 81 deletions(-)

diff --git a/drivers/net/mlx5/mlx5.h b/drivers/net/mlx5/mlx5.h
index 26c603b..e3ac07f 100644
--- a/drivers/net/mlx5/mlx5.h
+++ b/drivers/net/mlx5/mlx5.h
@@ -272,6 +272,10 @@ struct mlx5_drop {
 #define MLX5_COUNTERS_PER_POOL 512
 #define MLX5_MAX_PENDING_QUERIES 4
 #define MLX5_CNT_CONTAINER_RESIZE 64
+#define MLX5_CNT_SHARED_OFFSET 0x80000000
+#define IS_SHARED_CNT(cnt) (!!((cnt) & MLX5_CNT_SHARED_OFFSET))
+#define IS_BATCH_CNT(cnt) (((cnt) & (MLX5_CNT_SHARED_OFFSET - 1)) >= \
+			   MLX5_CNT_BATCH_OFFSET)
 #define CNT_SIZE (sizeof(struct mlx5_flow_counter))
 #define CNTEXT_SIZE (sizeof(struct mlx5_flow_counter_ext))
 #define AGE_SIZE (sizeof(struct mlx5_age_param))
@@ -348,10 +352,29 @@ struct flow_counter_stats {
 	uint64_t bytes;
 };
 
+/* Shared counters information for counters. */
+struct mlx5_flow_counter_shared {
+	uint32_t ref_cnt; /**< Reference counter. */
+	uint32_t id; /**< User counter ID. */
+};
+
+struct mlx5_flow_counter_pool;
 /* Generic counters information. */
 struct mlx5_flow_counter {
-	TAILQ_ENTRY(mlx5_flow_counter) next;
-	/**< Pointer to the next flow counter structure. */
+	union {
+		/*
+		 * User-defined counter shared info is only used during
+		 * counter active time. And aging counter sharing is not
+		 * supported, so active shared counter will not be chained
+		 * to the aging list. For shared counter, only when it is
+		 * released, the TAILQ entry memory will be used, at that
+		 * time, shared memory is not used anymore.
+		 */
+		TAILQ_ENTRY(mlx5_flow_counter) next;
+		/**< Pointer to the next flow counter structure. */
+		struct mlx5_flow_counter_shared shared_info;
+		/**< Shared counter information. */
+	};
 	union {
 		uint64_t hits; /**< Reset value of hits packets. */
 		struct mlx5_flow_counter_pool *pool; /**< Counter pool. */
@@ -360,15 +383,10 @@ struct mlx5_flow_counter {
 	void *action; /**< Pointer to the dv action. */
 };
 
-/* Extend counters information for none batch counters. */
+/* Extend counters information for none batch fallback counters. */
 struct mlx5_flow_counter_ext {
-	uint32_t shared:1; /**< Share counter ID with other flow rules. */
-	uint32_t batch: 1;
 	uint32_t skipped:1; /* This counter is skipped or not. */
-	/**< Whether the counter was allocated by batch command. */
-	uint32_t ref_cnt:29; /**< Reference counter. */
-	uint32_t id; /**< User counter ID. */
-	union {  /**< Holds the counters for the rule. */
+	union {
 #if defined(HAVE_IBV_DEVICE_COUNTERS_SET_V42)
 		struct ibv_counter_set *cs;
 #elif defined(HAVE_IBV_DEVICE_COUNTERS_SET_V45)
diff --git a/drivers/net/mlx5/mlx5_flow_dv.c b/drivers/net/mlx5/mlx5_flow_dv.c
index 90b98cc..b16db1d 100644
--- a/drivers/net/mlx5/mlx5_flow_dv.c
+++ b/drivers/net/mlx5/mlx5_flow_dv.c
@@ -4607,8 +4607,9 @@ struct field_modify_info modify_tcp[] = {
 	struct mlx5_flow_counter_pool *pool;
 	uint32_t batch = 0;
 
-	idx--;
-	if (idx >= MLX5_CNT_BATCH_OFFSET) {
+	/* Decrease to original index and clear shared bit. */
+	idx = (idx - 1) & (MLX5_CNT_SHARED_OFFSET - 1);
+	if (IS_BATCH_CNT(idx)) {
 		idx -= MLX5_CNT_BATCH_OFFSET;
 		batch = 1;
 	}
@@ -4843,7 +4844,7 @@ struct field_modify_info modify_tcp[] = {
 
 	cnt = flow_dv_counter_get_by_idx(dev, counter, &pool);
 	MLX5_ASSERT(pool);
-	if (counter < MLX5_CNT_BATCH_OFFSET) {
+	if (!IS_BATCH_CNT(counter)) {
 		cnt_ext = MLX5_CNT_TO_CNT_EXT(pool, cnt);
 		if (priv->counter_fallback)
 			return mlx5_devx_cmd_flow_counter_query(cnt_ext->dcs, 0,
@@ -5132,29 +5133,19 @@ struct field_modify_info modify_tcp[] = {
  *   Pointer to the Ethernet device structure.
  * @param[in] id
  *   The shared counter ID to search.
- * @param[out] ppool
- *   mlx5 flow counter pool in the container,
  *
  * @return
- *   NULL if not existed, otherwise pointer to the shared extend counter.
+ *   0 if not existed, otherwise shared counter index.
  */
-static struct mlx5_flow_counter_ext *
-flow_dv_counter_shared_search(struct rte_eth_dev *dev, uint32_t id,
-			      struct mlx5_flow_counter_pool **ppool)
+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;
-	uint32_t cnt_idx;
 
-	if (mlx5_l3t_get_entry(priv->sh->cnt_id_tbl, id, &data) || !data.dword)
-		return NULL;
-	cnt_idx = data.dword;
-	/*
-	 * Shared counters don't have age info. The counter extend is after
-	 * the counter datat structure.
-	 */
-	return (struct mlx5_flow_counter_ext *)
-	       ((flow_dv_counter_get_by_idx(dev, cnt_idx, ppool)) + 1);
+	if (mlx5_l3t_get_entry(priv->sh->cnt_id_tbl, id, &data))
+		return 0;
+	return data.dword;
 }
 
 /**
@@ -5201,16 +5192,15 @@ struct field_modify_info modify_tcp[] = {
 		return 0;
 	}
 	if (shared) {
-		cnt_ext = flow_dv_counter_shared_search(dev, id, &pool);
-		if (cnt_ext) {
-			if (cnt_ext->ref_cnt + 1 == 0) {
+		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_ext->ref_cnt++;
-			cnt_idx = pool->index * MLX5_COUNTERS_PER_POOL +
-				  (cnt_ext->dcs->id % MLX5_COUNTERS_PER_POOL)
-				  + 1;
+			cnt_free->shared_info.ref_cnt++;
 			return cnt_idx;
 		}
 	}
@@ -5253,17 +5243,15 @@ struct field_modify_info modify_tcp[] = {
 	if (_flow_dv_query_count(dev, cnt_idx, &cnt_free->hits,
 				 &cnt_free->bytes))
 		goto err;
-	if (cnt_ext) {
-		cnt_ext->shared = shared;
-		cnt_ext->ref_cnt = 1;
-		cnt_ext->id = id;
-		if (shared) {
-			union mlx5_l3t_data data;
-
-			data.dword = cnt_idx;
-			if (mlx5_l3t_set_entry(priv->sh->cnt_id_tbl, id, &data))
-				return 0;
-		}
+	if (shared) {
+		union mlx5_l3t_data data;
+
+		data.dword = cnt_idx;
+		if (mlx5_l3t_set_entry(priv->sh->cnt_id_tbl, id, &data))
+			goto err;
+		cnt_free->shared_info.ref_cnt = 1;
+		cnt_free->shared_info.id = id;
+		cnt_idx |= MLX5_CNT_SHARED_OFFSET;
 	}
 	if (!priv->counter_fallback && !priv->sh->cmng.query_thread_on)
 		/* Start the asynchronous batch query by the host thread. */
@@ -5352,22 +5340,18 @@ struct field_modify_info modify_tcp[] = {
 	struct mlx5_priv *priv = dev->data->dev_private;
 	struct mlx5_flow_counter_pool *pool = NULL;
 	struct mlx5_flow_counter *cnt;
-	struct mlx5_flow_counter_ext *cnt_ext = NULL;
 	enum mlx5_counter_type cnt_type;
 
 	if (!counter)
 		return;
 	cnt = flow_dv_counter_get_by_idx(dev, counter, &pool);
 	MLX5_ASSERT(pool);
-	if (counter < MLX5_CNT_BATCH_OFFSET) {
-		cnt_ext = MLX5_CNT_TO_CNT_EXT(pool, cnt);
-		if (cnt_ext) {
-			if (--cnt_ext->ref_cnt)
-				return;
-			if (cnt_ext->shared)
-				mlx5_l3t_clear_entry(priv->sh->cnt_id_tbl,
-						     cnt_ext->id);
-		}
+
+	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_AGE_POOL(pool))
 		flow_dv_counter_remove_from_age(dev, counter, cnt);
diff --git a/drivers/net/mlx5/mlx5_flow_verbs.c b/drivers/net/mlx5/mlx5_flow_verbs.c
index 698fb2b..bda55c2 100644
--- a/drivers/net/mlx5/mlx5_flow_verbs.c
+++ b/drivers/net/mlx5/mlx5_flow_verbs.c
@@ -162,7 +162,7 @@
 	struct mlx5_pools_container *cont = MLX5_CNT_CONTAINER(priv->sh, 0);
 	struct mlx5_flow_counter_pool *pool;
 
-	idx--;
+	idx = (idx - 1) & (MLX5_CNT_SHARED_OFFSET - 1);
 	pool = cont->pools[idx / MLX5_COUNTERS_PER_POOL];
 	MLX5_ASSERT(pool);
 	if (ppool)
@@ -258,22 +258,21 @@
 	struct mlx5_flow_counter_pool *pool = NULL;
 	struct mlx5_flow_counter_ext *cnt_ext = NULL;
 	struct mlx5_flow_counter *cnt = NULL;
+	union mlx5_l3t_data data;
 	uint32_t n_valid = rte_atomic16_read(&cont->n_valid);
-	uint32_t pool_idx;
+	uint32_t pool_idx, cnt_idx;
 	uint32_t i;
 	int ret;
 
-	if (shared) {
-		for (pool_idx = 0; pool_idx < n_valid; ++pool_idx) {
-			pool = cont->pools[pool_idx];
-			for (i = 0; i < MLX5_COUNTERS_PER_POOL; ++i) {
-				cnt_ext = MLX5_GET_POOL_CNT_EXT(pool, i);
-				if (cnt_ext->shared && cnt_ext->id == id) {
-					cnt_ext->ref_cnt++;
-					return MLX5_MAKE_CNT_IDX(pool_idx, i);
-				}
-			}
+	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++;
+		return data.dword;
 	}
 	for (pool_idx = 0; pool_idx < n_valid; ++pool_idx) {
 		pool = cont->pools[pool_idx];
@@ -322,17 +321,23 @@
 		TAILQ_INSERT_HEAD(&cont->pool_list, pool, next);
 	}
 	i = MLX5_CNT_ARRAY_IDX(pool, cnt);
+	cnt_idx = MLX5_MAKE_CNT_IDX(pool_idx, i);
+	if (shared) {
+		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;
+	}
 	cnt_ext = MLX5_GET_POOL_CNT_EXT(pool, i);
-	cnt_ext->id = id;
-	cnt_ext->shared = shared;
-	cnt_ext->ref_cnt = 1;
 	cnt->hits = 0;
 	cnt->bytes = 0;
 	/* Create counter with Verbs. */
 	ret = flow_verbs_counter_create(dev, cnt_ext);
 	if (!ret) {
 		TAILQ_REMOVE(&pool->counters[0], cnt, next);
-		return MLX5_MAKE_CNT_IDX(pool_idx, i);
+		return cnt_idx;
 	}
 	/* Some error occurred in Verbs library. */
 	rte_errno = -ret;
@@ -350,23 +355,28 @@
 static void
 flow_verbs_counter_release(struct rte_eth_dev *dev, uint32_t counter)
 {
+	struct mlx5_priv *priv = dev->data->dev_private;
 	struct mlx5_flow_counter_pool *pool;
 	struct mlx5_flow_counter *cnt;
 	struct mlx5_flow_counter_ext *cnt_ext;
 
-	cnt = flow_verbs_counter_get_by_idx(dev, counter,
-					    &pool);
+	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);
+	}
 	cnt_ext = MLX5_CNT_TO_CNT_EXT(pool, cnt);
-	if (--cnt_ext->ref_cnt == 0) {
 #if defined(HAVE_IBV_DEVICE_COUNTERS_SET_V42)
-		claim_zero(mlx5_glue->destroy_counter_set(cnt_ext->cs));
-		cnt_ext->cs = NULL;
+	claim_zero(mlx5_glue->destroy_counter_set(cnt_ext->cs));
+	cnt_ext->cs = NULL;
 #elif defined(HAVE_IBV_DEVICE_COUNTERS_SET_V45)
-		claim_zero(mlx5_glue->destroy_counters(cnt_ext->cs));
-		cnt_ext->cs = NULL;
+	claim_zero(mlx5_glue->destroy_counters(cnt_ext->cs));
+	cnt_ext->cs = NULL;
 #endif
-		TAILQ_INSERT_HEAD(&pool->counters[0], cnt, next);
-	}
+	(void)cnt_ext;
+	TAILQ_INSERT_HEAD(&pool->counters[0], cnt, next);
 }
 
 /**
-- 
1.8.3.1