DPDK patches and discussions
 help / color / mirror / Atom feed
From: Suanming Mou <suanmingm@mellanox.com>
To: Matan Azrad <matan@mellanox.com>,
	Shahaf Shuler <shahafs@mellanox.com>,
	Viacheslav Ovsiienko <viacheslavo@mellanox.com>
Cc: dev@dpdk.org, rasland@mellanox.com
Subject: [dpdk-dev] [PATCH 6/8] net/mlx5: optimize flow counter handle type
Date: Tue,  7 Apr 2020 11:59:45 +0800	[thread overview]
Message-ID: <1586231987-338112-7-git-send-email-suanmingm@mellanox.com> (raw)
In-Reply-To: <1586231987-338112-1-git-send-email-suanmingm@mellanox.com>

Currently, DV and verbs counters are both changed to indexed. It means
while creating the flow with counter, flow can save the indexed value to
address the counter.

Save the 4 bytes indexed value in the rte_flow instead of 8 bytes pointer
helps to save memory with millions of flows.

Signed-off-by: Suanming Mou <suanmingm@mellanox.com>
Acked-by: Matan Azrad <matan@mellanox.com>
---
 drivers/net/mlx5/mlx5.h            |  6 ++---
 drivers/net/mlx5/mlx5_flow.c       | 14 +++++-----
 drivers/net/mlx5/mlx5_flow.h       | 10 +++----
 drivers/net/mlx5/mlx5_flow_dv.c    | 54 ++++++++++++++++++--------------------
 drivers/net/mlx5/mlx5_flow_verbs.c | 36 +++++++++++--------------
 5 files changed, 56 insertions(+), 64 deletions(-)

diff --git a/drivers/net/mlx5/mlx5.h b/drivers/net/mlx5/mlx5.h
index 6f01eea..1501e61 100644
--- a/drivers/net/mlx5/mlx5.h
+++ b/drivers/net/mlx5/mlx5.h
@@ -764,9 +764,9 @@ void mlx5_flow_async_pool_query_handle(struct mlx5_ibv_shared *sh,
 				       uint64_t async_id, int status);
 void mlx5_set_query_alarm(struct mlx5_ibv_shared *sh);
 void mlx5_flow_query_alarm(void *arg);
-struct mlx5_flow_counter *mlx5_counter_alloc(struct rte_eth_dev *dev);
-void mlx5_counter_free(struct rte_eth_dev *dev, struct mlx5_flow_counter *cnt);
-int mlx5_counter_query(struct rte_eth_dev *dev, struct mlx5_flow_counter *cnt,
+uint32_t mlx5_counter_alloc(struct rte_eth_dev *dev);
+void mlx5_counter_free(struct rte_eth_dev *dev, uint32_t cnt);
+int mlx5_counter_query(struct rte_eth_dev *dev, uint32_t cnt,
 		       bool clear, uint64_t *pkts, uint64_t *bytes);
 int mlx5_flow_dev_dump(struct rte_eth_dev *dev, FILE *file,
 		       struct rte_flow_error *error);
diff --git a/drivers/net/mlx5/mlx5_flow.c b/drivers/net/mlx5/mlx5_flow.c
index d2e9cc4..3b358b6 100644
--- a/drivers/net/mlx5/mlx5_flow.c
+++ b/drivers/net/mlx5/mlx5_flow.c
@@ -5494,9 +5494,9 @@ struct mlx5_meter_domains_infos *
  *   Pointer to Ethernet device structure.
  *
  * @return
- *   Pointer to allocated counter  on success, NULL otherwise.
+ *   Index to allocated counter  on success, 0 otherwise.
  */
-struct mlx5_flow_counter *
+uint32_t
 mlx5_counter_alloc(struct rte_eth_dev *dev)
 {
 	const struct mlx5_flow_driver_ops *fops;
@@ -5509,7 +5509,7 @@ struct mlx5_flow_counter *
 	DRV_LOG(ERR,
 		"port %u counter allocate is not supported.",
 		 dev->data->port_id);
-	return NULL;
+	return 0;
 }
 
 /**
@@ -5518,10 +5518,10 @@ struct mlx5_flow_counter *
  * @param[in] dev
  *   Pointer to Ethernet device structure.
  * @param[in] cnt
- *   Pointer to counter to be free.
+ *   Index to counter to be free.
  */
 void
-mlx5_counter_free(struct rte_eth_dev *dev, struct mlx5_flow_counter *cnt)
+mlx5_counter_free(struct rte_eth_dev *dev, uint32_t cnt)
 {
 	const struct mlx5_flow_driver_ops *fops;
 	struct rte_flow_attr attr = { .transfer = 0 };
@@ -5542,7 +5542,7 @@ struct mlx5_flow_counter *
  * @param[in] dev
  *   Pointer to Ethernet device structure.
  * @param[in] cnt
- *   Pointer to counter to query.
+ *   Index to counter to query.
  * @param[in] clear
  *   Set to clear counter statistics.
  * @param[out] pkts
@@ -5554,7 +5554,7 @@ struct mlx5_flow_counter *
  *   0 on success, a negative errno value otherwise.
  */
 int
-mlx5_counter_query(struct rte_eth_dev *dev, struct mlx5_flow_counter *cnt,
+mlx5_counter_query(struct rte_eth_dev *dev, uint32_t cnt,
 		   bool clear, uint64_t *pkts, uint64_t *bytes)
 {
 	const struct mlx5_flow_driver_ops *fops;
diff --git a/drivers/net/mlx5/mlx5_flow.h b/drivers/net/mlx5/mlx5_flow.h
index 0f0e59d..daa1f84 100644
--- a/drivers/net/mlx5/mlx5_flow.h
+++ b/drivers/net/mlx5/mlx5_flow.h
@@ -632,7 +632,7 @@ struct mlx5_flow {
 
 /* Meter policer statistics */
 struct mlx5_flow_policer_stats {
-	struct mlx5_flow_counter *cnt[RTE_COLORS + 1];
+	uint32_t cnt[RTE_COLORS + 1];
 	/**< Color counter, extra for drop. */
 	uint64_t stats_mask;
 	/**< Statistics mask for the colors. */
@@ -729,7 +729,7 @@ struct rte_flow {
 	TAILQ_ENTRY(rte_flow) next; /**< Pointer to the next flow structure. */
 	enum mlx5_flow_drv_type drv_type; /**< Driver type. */
 	struct mlx5_flow_rss rss; /**< RSS context. */
-	struct mlx5_flow_counter *counter; /**< Holds flow counter. */
+	uint32_t counter; /**< Holds flow counter. */
 	struct mlx5_flow_mreg_copy_resource *mreg_copy;
 	/**< pointer to metadata register copy table resource. */
 	struct mlx5_flow_meter *meter; /**< Holds flow meter. */
@@ -780,12 +780,12 @@ typedef int (*mlx5_flow_destroy_policer_rules_t)
 					(struct rte_eth_dev *dev,
 					 const struct mlx5_flow_meter *fm,
 					 const struct rte_flow_attr *attr);
-typedef struct mlx5_flow_counter * (*mlx5_flow_counter_alloc_t)
+typedef uint32_t (*mlx5_flow_counter_alloc_t)
 				   (struct rte_eth_dev *dev);
 typedef void (*mlx5_flow_counter_free_t)(struct rte_eth_dev *dev,
-					 struct mlx5_flow_counter *cnt);
+					 uint32_t cnt);
 typedef int (*mlx5_flow_counter_query_t)(struct rte_eth_dev *dev,
-					 struct mlx5_flow_counter *cnt,
+					 uint32_t cnt,
 					 bool clear, uint64_t *pkts,
 					 uint64_t *bytes);
 struct mlx5_flow_driver_ops {
diff --git a/drivers/net/mlx5/mlx5_flow_dv.c b/drivers/net/mlx5/mlx5_flow_dv.c
index 91e130c..b7daa8f 100644
--- a/drivers/net/mlx5/mlx5_flow_dv.c
+++ b/drivers/net/mlx5/mlx5_flow_dv.c
@@ -3892,7 +3892,7 @@ struct field_modify_info modify_tcp[] = {
  * @param[in] dev
  *   Pointer to the Ethernet device structure.
  * @param[in] counter
- *   Pointer to the counter handler.
+ *   Index to the counter handler.
  */
 static void
 flow_dv_counter_release_fallback(struct rte_eth_dev *dev __rte_unused,
@@ -4384,9 +4384,9 @@ struct field_modify_info modify_tcp[] = {
  *   Counter flow group.
  *
  * @return
- *   pointer to flow counter on success, NULL otherwise and rte_errno is set.
+ *   Index to flow counter on success, 0 otherwise and rte_errno is set.
  */
-static struct mlx5_flow_counter *
+static uint32_t
 flow_dv_counter_alloc(struct rte_eth_dev *dev, uint32_t shared, uint32_t id,
 		      uint16_t group)
 {
@@ -4408,24 +4408,24 @@ struct field_modify_info modify_tcp[] = {
 
 	if (!priv->config.devx) {
 		rte_errno = ENOTSUP;
-		return NULL;
+		return 0;
 	}
 	if (shared) {
 		cnt_free = flow_dv_counter_shared_search(cont, id, &pool);
 		if (cnt_free) {
 			if (cnt_free->ref_cnt + 1 == 0) {
 				rte_errno = E2BIG;
-				return NULL;
+				return 0;
 			}
 			cnt_free->ref_cnt++;
 			cnt_idx = pool->index * MLX5_COUNTERS_PER_POOL +
 				  (cnt_free - pool->counters_raw) + 1;
-			return (struct mlx5_flow_counter *)(uintptr_t)cnt_idx;
+			return cnt_idx;
 		}
 	}
 	if (priv->counter_fallback)
-		return (struct mlx5_flow_counter *)(uintptr_t)
-		       flow_dv_counter_alloc_fallback(dev, shared, id);
+		return flow_dv_counter_alloc_fallback(dev, shared, id);
+
 	/* Pools which has a free counters are in the start. */
 	TAILQ_FOREACH(pool, &cont->pool_list, next) {
 		/*
@@ -4446,7 +4446,7 @@ struct field_modify_info modify_tcp[] = {
 	if (!cnt_free) {
 		cont = flow_dv_counter_pool_prepare(dev, &cnt_free, batch);
 		if (!cont)
-			return NULL;
+			return 0;
 		pool = TAILQ_FIRST(&cont->pool_list);
 	}
 	cnt_free->batch = batch;
@@ -4466,13 +4466,13 @@ struct field_modify_info modify_tcp[] = {
 					(dcs->obj, offset);
 		if (!cnt_free->action) {
 			rte_errno = errno;
-			return NULL;
+			return 0;
 		}
 	}
 	/* Update the counter reset values. */
 	if (_flow_dv_query_count(dev, cnt_free, &cnt_free->hits,
 				 &cnt_free->bytes))
-		return NULL;
+		return 0;
 	cnt_free->shared = shared;
 	cnt_free->ref_cnt = 1;
 	cnt_free->id = id;
@@ -4488,7 +4488,7 @@ struct field_modify_info modify_tcp[] = {
 	cnt_idx = MLX5_MAKE_CNT_IDX(pool->index,
 				    (cnt_free - pool->counters_raw));
 	cnt_idx += batch * MLX5_CNT_BATCH_OFFSET;
-	return (struct mlx5_flow_counter *)(uintptr_t)cnt_idx;
+	return cnt_idx;
 }
 
 /**
@@ -4497,11 +4497,10 @@ struct field_modify_info modify_tcp[] = {
  * @param[in] dev
  *   Pointer to the Ethernet device structure.
  * @param[in] counter
- *   Pointer to the counter handler.
+ *   Index to the counter handler.
  */
 static void
-flow_dv_counter_release(struct rte_eth_dev *dev,
-			struct mlx5_flow_counter *counter)
+flow_dv_counter_release(struct rte_eth_dev *dev, uint32_t counter)
 {
 	struct mlx5_priv *priv = dev->data->dev_private;
 	struct mlx5_flow_counter_pool *pool;
@@ -4509,7 +4508,7 @@ struct field_modify_info modify_tcp[] = {
 
 	if (!counter)
 		return;
-	cnt = flow_dv_counter_get_by_idx(dev, (uintptr_t)counter, &pool);
+	cnt = flow_dv_counter_get_by_idx(dev, counter, &pool);
 	if (priv->counter_fallback) {
 		flow_dv_counter_release_fallback(dev, cnt);
 		return;
@@ -7588,11 +7587,11 @@ struct field_modify_info modify_tcp[] = {
 							count->shared,
 							count->id,
 							dev_flow->dv.group);
-			if (flow->counter == NULL)
+			if (!flow->counter)
 				goto cnt_err;
 			dev_flow->dv.actions[actions_n++] =
 				  (flow_dv_counter_get_by_idx(dev,
-				  (uintptr_t)flow->counter, NULL))->action;
+				  flow->counter, NULL))->action;
 			action_flags |= MLX5_FLOW_ACTION_COUNT;
 			break;
 cnt_err:
@@ -8465,7 +8464,7 @@ struct field_modify_info modify_tcp[] = {
 	__flow_dv_remove(dev, flow);
 	if (flow->counter) {
 		flow_dv_counter_release(dev, flow->counter);
-		flow->counter = NULL;
+		flow->counter = 0;
 	}
 	if (flow->meter) {
 		mlx5_flow_meter_detach(flow->meter);
@@ -8524,7 +8523,7 @@ struct field_modify_info modify_tcp[] = {
 		uint64_t pkts, bytes;
 		struct mlx5_flow_counter *cnt;
 
-		cnt = flow_dv_counter_get_by_idx(dev, (uintptr_t)flow->counter,
+		cnt = flow_dv_counter_get_by_idx(dev, flow->counter,
 						 NULL);
 		int err = _flow_dv_query_count(dev, cnt, &pkts,
 					       &bytes);
@@ -8793,7 +8792,7 @@ struct field_modify_info modify_tcp[] = {
 		if (!fm->policer_stats.cnt[i])
 			continue;
 		cnt = flow_dv_counter_get_by_idx(dev,
-		      (uintptr_t)fm->policer_stats.cnt[i], NULL);
+		      fm->policer_stats.cnt[i], NULL);
 		mtb->count_actns[i] = cnt->action;
 	}
 	/* Create drop action. */
@@ -9013,7 +9012,7 @@ struct field_modify_info modify_tcp[] = {
  * @param[in] dev
  *   Pointer to the Ethernet device structure.
  * @param[in] cnt
- *   Pointer to the flow counter.
+ *   Index to the flow counter.
  * @param[in] clear
  *   Set to clear the counter statistics.
  * @param[out] pkts
@@ -9025,8 +9024,7 @@ struct field_modify_info modify_tcp[] = {
  *   0 on success, otherwise return -1.
  */
 static int
-flow_dv_counter_query(struct rte_eth_dev *dev,
-		      struct mlx5_flow_counter *counter, bool clear,
+flow_dv_counter_query(struct rte_eth_dev *dev, uint32_t counter, bool clear,
 		      uint64_t *pkts, uint64_t *bytes)
 {
 	struct mlx5_priv *priv = dev->data->dev_private;
@@ -9037,7 +9035,7 @@ struct field_modify_info modify_tcp[] = {
 	if (!priv->config.devx)
 		return -1;
 
-	cnt = flow_dv_counter_get_by_idx(dev, (uintptr_t)counter, NULL);
+	cnt = flow_dv_counter_get_by_idx(dev, counter, NULL);
 	ret = _flow_dv_query_count(dev, cnt, &inn_pkts, &inn_bytes);
 	if (ret)
 		return -1;
@@ -9110,10 +9108,10 @@ struct field_modify_info modify_tcp[] = {
 /*
  * Mutex-protected thunk to lock-free flow_dv_counter_alloc().
  */
-static struct mlx5_flow_counter *
+static uint32_t
 flow_dv_counter_allocate(struct rte_eth_dev *dev)
 {
-	struct mlx5_flow_counter *cnt;
+	uint32_t cnt;
 
 	flow_dv_shared_lock(dev);
 	cnt = flow_dv_counter_alloc(dev, 0, 0, 1);
@@ -9125,7 +9123,7 @@ struct field_modify_info modify_tcp[] = {
  * Mutex-protected thunk to lock-free flow_dv_counter_release().
  */
 static void
-flow_dv_counter_free(struct rte_eth_dev *dev, struct mlx5_flow_counter *cnt)
+flow_dv_counter_free(struct rte_eth_dev *dev, uint32_t cnt)
 {
 	flow_dv_shared_lock(dev);
 	flow_dv_counter_release(dev, cnt);
diff --git a/drivers/net/mlx5/mlx5_flow_verbs.c b/drivers/net/mlx5/mlx5_flow_verbs.c
index c053778..227f963 100644
--- a/drivers/net/mlx5/mlx5_flow_verbs.c
+++ b/drivers/net/mlx5/mlx5_flow_verbs.c
@@ -145,9 +145,9 @@
  *   Counter identifier.
  *
  * @return
- *   A pointer to the counter, NULL otherwise and rte_errno is set.
+ *   Index to the counter, 0 otherwise and rte_errno is set.
  */
-static struct mlx5_flow_counter *
+static uint32_t
 flow_verbs_counter_new(struct rte_eth_dev *dev, uint32_t shared, uint32_t id)
 {
 	struct mlx5_priv *priv = dev->data->dev_private;
@@ -166,9 +166,7 @@
 				cnt = &pool->counters_raw[i];
 				if (cnt->shared && cnt->id == id) {
 					cnt->ref_cnt++;
-					return (struct mlx5_flow_counter *)
-					       (uintptr_t)
-					       MLX5_MAKE_CNT_IDX(pool_idx, i);
+					return MLX5_MAKE_CNT_IDX(pool_idx, i);
 				}
 			}
 		}
@@ -191,7 +189,7 @@
 				     (n_valid + MLX5_CNT_CONTAINER_RESIZE);
 			pools = rte_zmalloc(__func__, size, 0);
 			if (!pools)
-				return NULL;
+				return 0;
 			if (n_valid) {
 				memcpy(pools, cont->pools,
 				       sizeof(struct mlx5_flow_counter_pool *) *
@@ -205,7 +203,7 @@
 		size = sizeof(*pool) + sizeof(*cnt) * MLX5_COUNTERS_PER_POOL;
 		pool = rte_calloc(__func__, 1, size, 0);
 		if (!pool)
-			return NULL;
+			return 0;
 		for (i = 0; i < MLX5_COUNTERS_PER_POOL; ++i) {
 			cnt = &pool->counters_raw[i];
 			TAILQ_INSERT_HEAD(&pool->counters, cnt, next);
@@ -225,12 +223,11 @@
 	ret = flow_verbs_counter_create(dev, cnt);
 	if (!ret) {
 		TAILQ_REMOVE(&pool->counters, cnt, next);
-		return (struct mlx5_flow_counter *)(uintptr_t)
-		       MLX5_MAKE_CNT_IDX(pool_idx, (cnt - pool->counters_raw));
+		return MLX5_MAKE_CNT_IDX(pool_idx, (cnt - pool->counters_raw));
 	}
 	/* Some error occurred in Verbs library. */
 	rte_errno = -ret;
-	return NULL;
+	return 0;
 }
 
 /**
@@ -239,18 +236,17 @@
  * @param[in] dev
  *   Pointer to the Ethernet device structure.
  * @param[in] counter
- *   Pointer to the counter handler.
+ *   Index to the counter handler.
  */
 static void
-flow_verbs_counter_release(struct rte_eth_dev *dev,
-			   struct mlx5_flow_counter *counter)
+flow_verbs_counter_release(struct rte_eth_dev *dev, uint32_t counter)
 {
 	struct mlx5_flow_counter_pool *pool;
 	struct mlx5_flow_counter *cnt;
 
-	cnt = flow_verbs_counter_get_by_idx(dev, (uintptr_t)(void *)counter,
+	cnt = flow_verbs_counter_get_by_idx(dev, counter,
 					    &pool);
-	if (--counter->ref_cnt == 0) {
+	if (--cnt->ref_cnt == 0) {
 #if defined(HAVE_IBV_DEVICE_COUNTERS_SET_V42)
 		claim_zero(mlx5_glue->destroy_counter_set(cnt->cs));
 		cnt->cs = NULL;
@@ -275,10 +271,9 @@
 {
 #if defined(HAVE_IBV_DEVICE_COUNTERS_SET_V42) || \
 	defined(HAVE_IBV_DEVICE_COUNTERS_SET_V45)
-	if (flow->counter && flow->counter->cs) {
+	if (flow->counter) {
 		struct mlx5_flow_counter *cnt = flow_verbs_counter_get_by_idx
-						(dev, (uintptr_t)(void *)
-						flow->counter, NULL);
+						(dev, flow->counter, NULL);
 		struct rte_flow_query_count *qc = data;
 		uint64_t counters[2] = {0, 0};
 #if defined(HAVE_IBV_DEVICE_COUNTERS_SET_V42)
@@ -1082,8 +1077,7 @@
 						  "cannot get counter"
 						  " context.");
 	}
-	cnt = flow_verbs_counter_get_by_idx(dev, (uintptr_t)(void *)
-					    flow->counter, NULL);
+	cnt = flow_verbs_counter_get_by_idx(dev, flow->counter, NULL);
 #if defined(HAVE_IBV_DEVICE_COUNTERS_SET_V42)
 	counter.counter_set_handle = cnt->cs->handle;
 	flow_verbs_spec_add(&dev_flow->verbs, &counter, size);
@@ -1775,7 +1769,7 @@
 	}
 	if (flow->counter) {
 		flow_verbs_counter_release(dev, flow->counter);
-		flow->counter = NULL;
+		flow->counter = 0;
 	}
 }
 
-- 
1.8.3.1


  parent reply	other threads:[~2020-04-07  4:00 UTC|newest]

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-04-07  3:59 [dpdk-dev] [PATCH 0/8] net/mlx5 counter optimize Suanming Mou
2020-04-07  3:59 ` [dpdk-dev] [PATCH 1/8] net/mlx5: fix incorrect counter container usage Suanming Mou
2020-04-07  3:59 ` [dpdk-dev] [PATCH 2/8] net/mlx5: optimize counter release query generation Suanming Mou
2020-04-07  3:59 ` [dpdk-dev] [PATCH 3/8] net/mlx5: change verbs counter allocator to indexed Suanming Mou
2020-04-07  3:59 ` [dpdk-dev] [PATCH 4/8] common/mlx5: add batch counter id offset Suanming Mou
2020-04-07  3:59 ` [dpdk-dev] [PATCH 5/8] net/mlx5: change Direct Verbs counter to indexed Suanming Mou
2020-04-07  3:59 ` Suanming Mou [this message]
2020-04-07  3:59 ` [dpdk-dev] [PATCH 7/8] net/mlx5: split the counter struct Suanming Mou
2020-04-07  3:59 ` [dpdk-dev] [PATCH 8/8] net/mlx5: reorganize fallback counter management Suanming Mou
2020-04-08 12:52 ` [dpdk-dev] [PATCH 0/8] net/mlx5 counter optimize 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=1586231987-338112-7-git-send-email-suanmingm@mellanox.com \
    --to=suanmingm@mellanox.com \
    --cc=dev@dpdk.org \
    --cc=matan@mellanox.com \
    --cc=rasland@mellanox.com \
    --cc=shahafs@mellanox.com \
    --cc=viacheslavo@mellanox.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).