[PATCH 22.11] net/mlx5: fix flow counter cache starvation

[PATCH 22.11] net/mlx5: fix flow counter cache starvation

[Dariusz Sosnowski]

[ upstream commit d755221b77c29549be4025e547cf907ad3a0abcf ]

mlx5 PMD maintains a global counter pool and per-queue counter cache,
which are used to allocate COUNT flow action objects.
Whenever an empty cache is accessed, it is replenished
with a pre-defined number of counters.

If number of configured counters was sufficiently small, then
it might have happened that caches associated with some queues
could get starved because all counters were fetched on other queues.

This patch fixes that by disabling cache at runtime
if number of configured counters is not sufficient to avoid
such starvation.

Fixes: 4d368e1da3a4 ("net/mlx5: support flow counter action for HWS")
Cc: stable@dpdk.org

Signed-off-by: Dariusz Sosnowski <dsosnowski@nvidia.com>
Acked-by: Ori Kam <orika@nvidia.com>
Acked-by: Bing Zhao <bingz@nvidia.com>
---
 drivers/net/mlx5/mlx5_flow_hw.c | 10 +++--
 drivers/net/mlx5/mlx5_hws_cnt.c | 72 ++++++++++++++++++++++++---------
 drivers/net/mlx5/mlx5_hws_cnt.h | 26 ++++++++++++
 3 files changed, 86 insertions(+), 22 deletions(-)

diff --git a/drivers/net/mlx5/mlx5_flow_hw.c b/drivers/net/mlx5/mlx5_flow_hw.c
index 20fa4eee0c..bb4693c2b4 100644
--- a/drivers/net/mlx5/mlx5_flow_hw.c
+++ b/drivers/net/mlx5/mlx5_flow_hw.c
@@ -2170,6 +2170,7 @@ flow_hw_actions_construct(struct rte_eth_dev *dev,
 		struct mlx5_hrxq *hrxq;
 		uint32_t ct_idx;
 		cnt_id_t cnt_id;
+		uint32_t *cnt_queue;
 		uint32_t mtr_id;
 
 		action = &actions[act_data->action_src];
@@ -2321,8 +2322,8 @@ flow_hw_actions_construct(struct rte_eth_dev *dev,
 				break;
 			/* Fall-through. */
 		case RTE_FLOW_ACTION_TYPE_COUNT:
-			ret = mlx5_hws_cnt_pool_get(priv->hws_cpool, &queue,
-						    &cnt_id, age_idx);
+			cnt_queue = mlx5_hws_cnt_get_queue(priv, &queue);
+			ret = mlx5_hws_cnt_pool_get(priv->hws_cpool, cnt_queue, &cnt_id, age_idx);
 			if (ret != 0)
 				return ret;
 			ret = mlx5_hws_cnt_pool_get_action_offset
@@ -2654,6 +2655,8 @@ flow_hw_age_count_release(struct mlx5_priv *priv, uint32_t queue,
 			  struct rte_flow_hw *flow,
 			  struct rte_flow_error *error)
 {
+	uint32_t *cnt_queue;
+
 	if (mlx5_hws_cnt_is_shared(priv->hws_cpool, flow->cnt_id)) {
 		if (flow->age_idx && !mlx5_hws_age_is_indirect(flow->age_idx)) {
 			/* Remove this AGE parameter from indirect counter. */
@@ -2664,8 +2667,9 @@ flow_hw_age_count_release(struct mlx5_priv *priv, uint32_t queue,
 		}
 		return;
 	}
+	cnt_queue = mlx5_hws_cnt_get_queue(priv, &queue);
 	/* Put the counter first to reduce the race risk in BG thread. */
-	mlx5_hws_cnt_pool_put(priv->hws_cpool, &queue, &flow->cnt_id);
+	mlx5_hws_cnt_pool_put(priv->hws_cpool, cnt_queue, &flow->cnt_id);
 	flow->cnt_id = 0;
 	if (flow->age_idx) {
 		if (mlx5_hws_age_is_indirect(flow->age_idx)) {
diff --git a/drivers/net/mlx5/mlx5_hws_cnt.c b/drivers/net/mlx5/mlx5_hws_cnt.c
index 791fde4458..885effe3a1 100644
--- a/drivers/net/mlx5/mlx5_hws_cnt.c
+++ b/drivers/net/mlx5/mlx5_hws_cnt.c
@@ -333,6 +333,55 @@ mlx5_hws_cnt_svc(void *opaque)
 	return NULL;
 }
 
+static bool
+mlx5_hws_cnt_should_enable_cache(const struct mlx5_hws_cnt_pool_cfg *pcfg,
+				 const struct mlx5_hws_cache_param *ccfg)
+{
+	/*
+	 * Enable cache if and only if there are enough counters requested
+	 * to populate all of the caches.
+	 */
+	return pcfg->request_num >= ccfg->q_num * ccfg->size;
+}
+
+static struct mlx5_hws_cnt_pool_caches *
+mlx5_hws_cnt_cache_init(const struct mlx5_hws_cnt_pool_cfg *pcfg,
+			const struct mlx5_hws_cache_param *ccfg)
+{
+	struct mlx5_hws_cnt_pool_caches *cache;
+	char mz_name[RTE_MEMZONE_NAMESIZE];
+	uint32_t qidx;
+
+	/* If counter pool is big enough, setup the counter pool cache. */
+	cache = mlx5_malloc(MLX5_MEM_ANY | MLX5_MEM_ZERO,
+			sizeof(*cache) +
+			sizeof(((struct mlx5_hws_cnt_pool_caches *)0)->qcache[0])
+				* ccfg->q_num, 0, SOCKET_ID_ANY);
+	if (cache == NULL)
+		return NULL;
+	/* Store the necessary cache parameters. */
+	cache->fetch_sz = ccfg->fetch_sz;
+	cache->preload_sz = ccfg->preload_sz;
+	cache->threshold = ccfg->threshold;
+	cache->q_num = ccfg->q_num;
+	for (qidx = 0; qidx < ccfg->q_num; qidx++) {
+		snprintf(mz_name, sizeof(mz_name), "%s_qc/%x", pcfg->name, qidx);
+		cache->qcache[qidx] = rte_ring_create(mz_name, ccfg->size,
+				SOCKET_ID_ANY,
+				RING_F_SP_ENQ | RING_F_SC_DEQ |
+				RING_F_EXACT_SZ);
+		if (cache->qcache[qidx] == NULL)
+			goto error;
+	}
+	return cache;
+
+error:
+	while (qidx--)
+		rte_ring_free(cache->qcache[qidx]);
+	mlx5_free(cache);
+	return NULL;
+}
+
 struct mlx5_hws_cnt_pool *
 mlx5_hws_cnt_pool_init(struct mlx5_dev_ctx_shared *sh,
 		       const struct mlx5_hws_cnt_pool_cfg *pcfg,
@@ -341,7 +390,6 @@ mlx5_hws_cnt_pool_init(struct mlx5_dev_ctx_shared *sh,
 	char mz_name[RTE_MEMZONE_NAMESIZE];
 	struct mlx5_hws_cnt_pool *cntp;
 	uint64_t cnt_num = 0;
-	uint32_t qidx;
 
 	MLX5_ASSERT(pcfg);
 	MLX5_ASSERT(ccfg);
@@ -351,17 +399,6 @@ mlx5_hws_cnt_pool_init(struct mlx5_dev_ctx_shared *sh,
 		return NULL;
 
 	cntp->cfg = *pcfg;
-	cntp->cache = mlx5_malloc(MLX5_MEM_ANY | MLX5_MEM_ZERO,
-			sizeof(*cntp->cache) +
-			sizeof(((struct mlx5_hws_cnt_pool_caches *)0)->qcache[0])
-				* ccfg->q_num, 0, SOCKET_ID_ANY);
-	if (cntp->cache == NULL)
-		goto error;
-	 /* store the necessary cache parameters. */
-	cntp->cache->fetch_sz = ccfg->fetch_sz;
-	cntp->cache->preload_sz = ccfg->preload_sz;
-	cntp->cache->threshold = ccfg->threshold;
-	cntp->cache->q_num = ccfg->q_num;
 	if (pcfg->request_num > sh->hws_max_nb_counters) {
 		DRV_LOG(ERR, "Counter number %u "
 			"is greater than the maximum supported (%u).",
@@ -408,13 +445,10 @@ mlx5_hws_cnt_pool_init(struct mlx5_dev_ctx_shared *sh,
 		DRV_LOG(ERR, "failed to create reuse list ring");
 		goto error;
 	}
-	for (qidx = 0; qidx < ccfg->q_num; qidx++) {
-		snprintf(mz_name, sizeof(mz_name), "%s_qc/%x", pcfg->name, qidx);
-		cntp->cache->qcache[qidx] = rte_ring_create(mz_name, ccfg->size,
-				SOCKET_ID_ANY,
-				RING_F_SP_ENQ | RING_F_SC_DEQ |
-				RING_F_EXACT_SZ);
-		if (cntp->cache->qcache[qidx] == NULL)
+	/* Allocate counter cache only if needed. */
+	if (mlx5_hws_cnt_should_enable_cache(pcfg, ccfg)) {
+		cntp->cache = mlx5_hws_cnt_cache_init(pcfg, ccfg);
+		if (cntp->cache == NULL)
 			goto error;
 	}
 	/* Initialize the time for aging-out calculation. */
diff --git a/drivers/net/mlx5/mlx5_hws_cnt.h b/drivers/net/mlx5/mlx5_hws_cnt.h
index b5c19a8e2c..72751f3330 100644
--- a/drivers/net/mlx5/mlx5_hws_cnt.h
+++ b/drivers/net/mlx5/mlx5_hws_cnt.h
@@ -533,6 +533,32 @@ mlx5_hws_cnt_pool_get(struct mlx5_hws_cnt_pool *cpool, uint32_t *queue,
 	return 0;
 }
 
+/**
+ * Decide if the given queue can be used to perform counter allocation/deallcation
+ * based on counter configuration
+ *
+ * @param[in] priv
+ *   Pointer to the port private data structure.
+ * @param[in] queue
+ *   Pointer to the queue index.
+ *
+ * @return
+ *   @p queue if cache related to the queue can be used. NULL otherwise.
+ */
+static __rte_always_inline uint32_t *
+mlx5_hws_cnt_get_queue(struct mlx5_priv *priv, uint32_t *queue)
+{
+	if (priv && priv->hws_cpool) {
+		/* Do not use queue cache if counter cache is disabled. */
+		if (priv->hws_cpool->cache == NULL)
+			return NULL;
+		return queue;
+	}
+	/* This case should not be reached if counter pool was successfully configured. */
+	MLX5_ASSERT(false);
+	return NULL;
+}
+
 static __rte_always_inline unsigned int
 mlx5_hws_cnt_pool_get_size(struct mlx5_hws_cnt_pool *cpool)
 {
-- 
2.39.2

Re: [PATCH 22.11] net/mlx5: fix flow counter cache starvation

[Luca Boccassi]

On Thu, 14 Mar 2024 at 13:30, Dariusz Sosnowski <dsosnowski@nvidia.com> wrote:
>
> [ upstream commit d755221b77c29549be4025e547cf907ad3a0abcf ]
>
> mlx5 PMD maintains a global counter pool and per-queue counter cache,
> which are used to allocate COUNT flow action objects.
> Whenever an empty cache is accessed, it is replenished
> with a pre-defined number of counters.
>
> If number of configured counters was sufficiently small, then
> it might have happened that caches associated with some queues
> could get starved because all counters were fetched on other queues.
>
> This patch fixes that by disabling cache at runtime
> if number of configured counters is not sufficient to avoid
> such starvation.
>
> Fixes: 4d368e1da3a4 ("net/mlx5: support flow counter action for HWS")
> Cc: stable@dpdk.org
>
> Signed-off-by: Dariusz Sosnowski <dsosnowski@nvidia.com>
> Acked-by: Ori Kam <orika@nvidia.com>
> Acked-by: Bing Zhao <bingz@nvidia.com>
> ---
>  drivers/net/mlx5/mlx5_flow_hw.c | 10 +++--
>  drivers/net/mlx5/mlx5_hws_cnt.c | 72 ++++++++++++++++++++++++---------
>  drivers/net/mlx5/mlx5_hws_cnt.h | 26 ++++++++++++
>  3 files changed, 86 insertions(+), 22 deletions(-)

Thanks, applied