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

Suanming Mou suanmingm at nvidia.com
Tue Oct 6 13:38:49 CEST 2020


Currently, when a counter is allocated, the counter list entry memory
will not be used until the counter is released and added back to the
counter container free list. In this case, if a counter is allocated
as shared counter, the shared information can be save to the counter
list entry memory. This adjustment engages to save more memory for
the shared counter.

One more thing is that now the shared counter is only available for
single counter as shared counter may be applied to both root and
none root table, but batch counter with offset is not supported in
root table in some old OFED version. The batch counter with offset
is now fully supported in the current OFED. This commit is also
the initialize change for the batch counter with offeset can be
shared in the later change.

Signed-off-by: Suanming Mou <suanmingm at nvidia.com>
---
 drivers/net/mlx5/mlx5.h            | 33 +++++++++-------
 drivers/net/mlx5/mlx5_flow_dv.c    | 78 +++++++++++++++-----------------------
 drivers/net/mlx5/mlx5_flow_verbs.c | 60 +++++++++++++++++------------
 3 files changed, 86 insertions(+), 85 deletions(-)

diff --git a/drivers/net/mlx5/mlx5.h b/drivers/net/mlx5/mlx5.h
index 27c8f45..fe6bd88 100644
--- a/drivers/net/mlx5/mlx5.h
+++ b/drivers/net/mlx5/mlx5.h
@@ -270,6 +270,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,11 +352,21 @@ 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 {
+		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. */
@@ -361,22 +375,15 @@ 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. */
 #if defined(HAVE_IBV_DEVICE_COUNTERS_SET_V42)
-		struct ibv_counter_set *cs;
+	struct ibv_counter_set *cs;
 #elif defined(HAVE_IBV_DEVICE_COUNTERS_SET_V45)
-		struct ibv_counters *cs;
+	struct ibv_counters *cs;
 #endif
-		struct mlx5_devx_obj *dcs; /**< Counter Devx object. */
-	};
+	struct mlx5_devx_obj *dcs; /**< Counter Devx object. */
 };
 
 TAILQ_HEAD(mlx5_counters, mlx5_flow_counter);
diff --git a/drivers/net/mlx5/mlx5_flow_dv.c b/drivers/net/mlx5/mlx5_flow_dv.c
index 1bd3899..10be990 100644
--- a/drivers/net/mlx5/mlx5_flow_dv.c
+++ b/drivers/net/mlx5/mlx5_flow_dv.c
@@ -4172,8 +4172,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;
 	}
@@ -4408,7 +4409,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,
@@ -4696,29 +4697,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;
 }
 
 /**
@@ -4765,16 +4756,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;
 		}
 	}
@@ -4817,17 +4807,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. */
@@ -4915,22 +4903,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 2f3035a..0463bea 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



More information about the dev mailing list