DPDK patches and discussions
 help / color / mirror / Atom feed
From: Suanming Mou <suanmingm@nvidia.com>
To: viacheslavo@nvidia.com, matan@nvidia.com
Cc: rasland@nvidia.com, dev@dpdk.org
Subject: [dpdk-dev] [PATCH 2/6] net/mlx5: optimize shared counter memory
Date: Tue,  6 Oct 2020 19:38:49 +0800	[thread overview]
Message-ID: <1601984333-304464-3-git-send-email-suanmingm@nvidia.com> (raw)
In-Reply-To: <1601984333-304464-1-git-send-email-suanmingm@nvidia.com>

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@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


  parent reply	other threads:[~2020-10-06 11:39 UTC|newest]

Thread overview: 17+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-10-06 11:38 [dpdk-dev] [PATCH 0/6] net/mlx5: make counter thread safe Suanming Mou
2020-10-06 11:38 ` [dpdk-dev] [PATCH 1/6] net/mlx5: locate aging pools in the general container Suanming Mou
2020-10-06 11:38 ` Suanming Mou [this message]
2020-10-06 11:38 ` [dpdk-dev] [PATCH 3/6] net/mlx5: remove single counter container Suanming Mou
2020-10-06 11:38 ` [dpdk-dev] [PATCH 4/6] net/mlx5: synchronize flow counter pool creation Suanming Mou
2020-10-06 11:38 ` [dpdk-dev] [PATCH 5/6] net/mlx5: make three level table thread safe Suanming Mou
2020-10-06 11:38 ` [dpdk-dev] [PATCH 6/6] net/mlx5: make shared counters " Suanming Mou
2020-10-20  3:02 ` [dpdk-dev] [PATCH v2 0/8] net/mlx5: make counter " Suanming Mou
2020-10-20  3:02   ` [dpdk-dev] [PATCH v2 1/8] net/mlx5: locate aging pools in the general container Suanming Mou
2020-10-20  3:02   ` [dpdk-dev] [PATCH v2 2/8] net/mlx5: optimize shared counter memory Suanming Mou
2020-10-20  3:02   ` [dpdk-dev] [PATCH v2 3/8] net/mlx5: remove single counter container Suanming Mou
2020-10-20  3:02   ` [dpdk-dev] [PATCH v2 4/8] net/mlx5: synchronize flow counter pool creation Suanming Mou
2020-10-20  3:02   ` [dpdk-dev] [PATCH v2 5/8] net/mlx5: make three level table thread safe Suanming Mou
2020-10-20  3:02   ` [dpdk-dev] [PATCH v2 6/8] net/mlx5: make shared counters " Suanming Mou
2020-10-20  3:02   ` [dpdk-dev] [PATCH v2 7/8] net/mlx5: rename flow counter macro Suanming Mou
2020-10-20  3:02   ` [dpdk-dev] [PATCH v2 8/8] net/mlx5: optimize counter extend memory Suanming Mou
2020-10-20 22:59   ` [dpdk-dev] [PATCH v2 0/8] net/mlx5: make counter thread safe Raslan Darawsheh

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1601984333-304464-3-git-send-email-suanmingm@nvidia.com \
    --to=suanmingm@nvidia.com \
    --cc=dev@dpdk.org \
    --cc=matan@nvidia.com \
    --cc=rasland@nvidia.com \
    --cc=viacheslavo@nvidia.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).