[PATCH 1/2] net/mlx5: embed HWS counter pool name in the pool

[PATCH 1/2] net/mlx5: embed HWS counter pool name in the pool

[Maayan Kashani]

The pool name is a small fixed string, but it was allocated separately
and the pointer was stored in the pool config. Destroy always freed
that pointer. The create error path freed it again, which logged
"Invalid memory".

Keep the name in the pool config. The string is copied with the config,
so create and destroy no longer allocate or free it.

Fixes: 9e34fcc81c00 ("net/mlx5: fix leak in HWS flow counter action")
Cc: stable@dpdk.org

Signed-off-by: Maayan Kashani <mkashani@nvidia.com>
Acked-by: Dariusz Sosnowski <dsosnowski@nvidia.com>
---
 drivers/net/mlx5/mlx5_hws_cnt.c | 13 ++-----------
 drivers/net/mlx5/mlx5_hws_cnt.h |  3 ++-
 2 files changed, 4 insertions(+), 12 deletions(-)

diff --git a/drivers/net/mlx5/mlx5_hws_cnt.c b/drivers/net/mlx5/mlx5_hws_cnt.c
index d0c4ead71b7..d6f713fd9bd 100644
--- a/drivers/net/mlx5/mlx5_hws_cnt.c
+++ b/drivers/net/mlx5/mlx5_hws_cnt.c
@@ -730,18 +730,11 @@ mlx5_hws_cnt_pool_create(struct rte_eth_dev *dev,
 	struct mlx5_priv *priv = dev->data->dev_private;
 	struct mlx5_hws_cache_param cparam = {0};
 	struct mlx5_hws_cnt_pool_cfg pcfg = {0};
-	char *mp_name;
 	int ret = 0;
 	size_t sz;
 
-	mp_name = mlx5_malloc(MLX5_MEM_ZERO, RTE_MEMZONE_NAMESIZE, 0, SOCKET_ID_ANY);
-	if (mp_name == NULL) {
-		ret = rte_flow_error_set(error, ENOMEM, RTE_FLOW_ERROR_TYPE_UNSPECIFIED, NULL,
-					 "failed to allocate counter pool name prefix");
-		goto error;
-	}
-	snprintf(mp_name, RTE_MEMZONE_NAMESIZE, "MLX5_HWS_CNT_P_%x", dev->data->port_id);
-	pcfg.name = mp_name;
+	snprintf(pcfg.name, sizeof(pcfg.name), "MLX5_HWS_CNT_P_%x",
+		 dev->data->port_id);
 	pcfg.request_num = nb_counters;
 	pcfg.alloc_factor = HWS_CNT_ALLOC_FACTOR_DEFAULT;
 	if (chost) {
@@ -814,7 +807,6 @@ mlx5_hws_cnt_pool_create(struct rte_eth_dev *dev,
 	MLX5_ASSERT(ret);
 	mlx5_hws_cnt_pool_destroy(priv->sh, cpool);
 	priv->hws_cpool = NULL;
-	mlx5_free(mp_name);
 	return ret;
 }
 
@@ -842,7 +834,6 @@ mlx5_hws_cnt_pool_destroy(struct mlx5_dev_ctx_shared *sh,
 		mlx5_hws_cnt_pool_dcs_free(cpool);
 		mlx5_hws_cnt_raw_data_free(cpool->raw_mng);
 	}
-	mlx5_free((void *)cpool->cfg.name);
 	mlx5_hws_cnt_pool_deinit(cpool);
 }
 
diff --git a/drivers/net/mlx5/mlx5_hws_cnt.h b/drivers/net/mlx5/mlx5_hws_cnt.h
index 63a14f4f7a0..5a55820abec 100644
--- a/drivers/net/mlx5/mlx5_hws_cnt.h
+++ b/drivers/net/mlx5/mlx5_hws_cnt.h
@@ -5,6 +5,7 @@
 #ifndef _MLX5_HWS_CNT_H_
 #define _MLX5_HWS_CNT_H_
 
+#include <rte_memzone.h>
 #include <rte_ring.h>
 #include "mlx5_utils.h"
 #include "mlx5_flow.h"
@@ -87,7 +88,7 @@ struct mlx5_hws_cache_param {
 };
 
 struct mlx5_hws_cnt_pool_cfg {
-	char *name;
+	char name[RTE_MEMZONE_NAMESIZE];
 	uint32_t request_num;
 	uint32_t alloc_factor;
 	struct mlx5_hws_cnt_pool *host_cpool;
-- 
2.25.1

[PATCH 2/2] net/mlx5: report HWS counter pool allocation failure

[Maayan Kashani]

Non-template HWS pre-allocates half of the device counter capacity
from hugepage memory. When that allocation fails, the PMD replaced
the specific error with "counters pool not initialized".

Log the failing allocation and return the original error. Document
that this pre-allocation can exhaust hugepages, and that the
application can reserve more hugepages or configure fewer counters.

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

Signed-off-by: Maayan Kashani <mkashani@nvidia.com>
Acked-by: Dariusz Sosnowski <dsosnowski@nvidia.com>
---
 doc/guides/nics/mlx5.rst        | 10 ++++++++++
 drivers/net/mlx5/mlx5_flow_hw.c | 11 ++++-------
 drivers/net/mlx5/mlx5_hws_cnt.c | 20 ++++++++++++++++++--
 3 files changed, 32 insertions(+), 9 deletions(-)

diff --git a/doc/guides/nics/mlx5.rst b/doc/guides/nics/mlx5.rst
index 45d1a25ec18..64cd09c9944 100644
--- a/doc/guides/nics/mlx5.rst
+++ b/doc/guides/nics/mlx5.rst
@@ -2992,6 +2992,16 @@ With :ref:`HW steering <mlx5_hws>`:
    - Linux kernel >= 6.4 and rdma-core >= 60.0 for upstream drivers/libraries or,
    - DOCA >= 3.2.0 for drivers/libraries provided by NVIDIA.
 
+#. The non-template API pre-allocates half of the NIC counter capacity
+   from hugepage memory when the first ``COUNT`` action is created.
+   Other allocations, such as large mbuf pools, can use up the hugepage
+   heap first. Counter creation then fails.
+
+   Reserve more hugepages, or call ``rte_flow_configure()`` with a
+   smaller ``nb_counters`` before creating the first ``COUNT`` action.
+   An explicit configuration allocates only the requested number of
+   counters.
+
 
 .. _mlx5_age:
 
diff --git a/drivers/net/mlx5/mlx5_flow_hw.c b/drivers/net/mlx5/mlx5_flow_hw.c
index 8b74f66770b..481b01a21e3 100644
--- a/drivers/net/mlx5/mlx5_flow_hw.c
+++ b/drivers/net/mlx5/mlx5_flow_hw.c
@@ -12768,13 +12768,10 @@ flow_hw_action_handle_validate(struct rte_eth_dev *dev, uint32_t queue,
 		}
 		break;
 	case RTE_FLOW_ACTION_TYPE_COUNT:
-		if (!priv->hws_cpool) {
-			if (flow_hw_allocate_actions(dev, MLX5_FLOW_ACTION_COUNT,
-						     error))
-				return rte_flow_error_set
-					(error, EINVAL, RTE_FLOW_ERROR_TYPE_ACTION,
-					 NULL, "counters pool not initialized");
-		}
+		if (!priv->hws_cpool)
+			return flow_hw_allocate_actions(dev,
+							MLX5_FLOW_ACTION_COUNT,
+							error);
 		break;
 	case RTE_FLOW_ACTION_TYPE_CONNTRACK:
 		if (priv->hws_ctpool == NULL) {
diff --git a/drivers/net/mlx5/mlx5_hws_cnt.c b/drivers/net/mlx5/mlx5_hws_cnt.c
index d6f713fd9bd..c2aa75c865e 100644
--- a/drivers/net/mlx5/mlx5_hws_cnt.c
+++ b/drivers/net/mlx5/mlx5_hws_cnt.c
@@ -31,8 +31,11 @@ __hws_cnt_id_load(struct mlx5_hws_cnt_pool *cpool)
 
 	cnt_arr = mlx5_malloc(MLX5_MEM_ANY | MLX5_MEM_ZERO,
 			      cnt_num * sizeof(cnt_id_t), 0, SOCKET_ID_ANY);
-	if (cnt_arr == NULL)
+	if (cnt_arr == NULL) {
+		DRV_LOG(ERR, "Failed to allocate counter ID array: %zu bytes.",
+			cnt_num * sizeof(cnt_id_t));
 		return -ENOMEM;
+	}
 	/*
 	 * Counter ID order is important for tracking the max number of in used
 	 * counter for querying, which means counter internal index order must
@@ -290,6 +293,8 @@ mlx5_hws_cnt_raw_data_alloc(struct mlx5_dev_ctx_shared *sh, uint32_t n,
 	mng->raw = mlx5_malloc(MLX5_MEM_ANY | MLX5_MEM_ZERO, sz, pgsz,
 			SOCKET_ID_ANY);
 	if (mng->raw == NULL) {
+		DRV_LOG(ERR, "Failed to allocate raw counter memory: %zu bytes.",
+			sz);
 		rte_flow_error_set(error, ENOMEM,
 				   RTE_FLOW_ERROR_TYPE_UNSPECIFIED,
 				   NULL, "failed to allocate raw counters memory");
@@ -469,6 +474,9 @@ mlx5_hws_cnt_pool_init(struct mlx5_dev_ctx_shared *sh,
 				 sizeof(struct mlx5_hws_cnt) * cnt_num,
 				 0, SOCKET_ID_ANY);
 	if (cntp->pool == NULL) {
+		DRV_LOG(ERR,
+			"Failed to allocate counter pool: %zu bytes for %" PRIu64 " counters.",
+			sizeof(struct mlx5_hws_cnt) * (size_t)cnt_num, cnt_num);
 		rte_flow_error_set(error, ENOMEM,
 				   RTE_FLOW_ERROR_TYPE_UNSPECIFIED, NULL,
 				   "failed to allocate counter pool context");
@@ -773,8 +781,12 @@ mlx5_hws_cnt_pool_create(struct rte_eth_dev *dev,
 		goto error;
 	}
 	ret = __hws_cnt_id_load(cpool);
-	if (ret != 0)
+	if (ret != 0) {
+		rte_flow_error_set(error, ENOMEM,
+				   RTE_FLOW_ERROR_TYPE_UNSPECIFIED, NULL,
+				   "failed to allocate counter ID array");
 		goto error;
+	}
 	/*
 	 * Bump query gen right after pool create so the
 	 * pre-loaded counters can be used directly
@@ -805,6 +817,10 @@ mlx5_hws_cnt_pool_create(struct rte_eth_dev *dev,
 	return 0;
 error:
 	MLX5_ASSERT(ret);
+	DRV_LOG(ERR, "Port %u failed to create counter pool for %u counters: %s",
+		dev->data->port_id, nb_counters,
+		(error != NULL && error->message != NULL) ?
+		error->message : "unspecified error");
 	mlx5_hws_cnt_pool_destroy(priv->sh, cpool);
 	priv->hws_cpool = NULL;
 	return ret;
-- 
2.25.1

Re: [PATCH 1/2] net/mlx5: embed HWS counter pool name in the pool

[Raslan Darawsheh]

Patch applied to next-net-mlx.

Thanks,
Raslan Darawsheh

Re: [PATCH 2/2] net/mlx5: report HWS counter pool allocation failure

[Raslan Darawsheh]

Patch applied to next-net-mlx.

Thanks,
Raslan Darawsheh