[PATCH] net/mlx5: relax hws meter validation

[PATCH] net/mlx5: relax hws meter validation

[Alexander Kozyrev]

The policies array is not required for the new METER_MARK action.
It is only required for the old METER action implementation.
Furthermore, the profiles array is optional for the METER_MARK action
since a profile can be found using the rte_mtr_meter_profile_get().
Do not enforce the preallocation of meter profiles and policies.

Signed-off-by: Alexander Kozyrev <akozyrev@nvidia.com>
---
 drivers/net/mlx5/mlx5_flow_hw.c    | 60 ++++++++++++++++--------------
 drivers/net/mlx5/mlx5_flow_meter.c | 32 ++--------------
 2 files changed, 36 insertions(+), 56 deletions(-)

diff --git a/drivers/net/mlx5/mlx5_flow_hw.c b/drivers/net/mlx5/mlx5_flow_hw.c
index 9392727aa0..fba76311fd 100644
--- a/drivers/net/mlx5/mlx5_flow_hw.c
+++ b/drivers/net/mlx5/mlx5_flow_hw.c
@@ -7261,7 +7261,7 @@ flow_hw_configure(struct rte_eth_dev *dev,
 	}
 	/* Initialize meter library*/
 	if (port_attr->nb_meters || (host_priv && host_priv->hws_mpool))
-		if (mlx5_flow_meter_init(dev, port_attr->nb_meters, 1, 1, nb_q_updated))
+		if (mlx5_flow_meter_init(dev, port_attr->nb_meters, 0, 0, nb_q_updated))
 			goto err;
 	/* Add global actions. */
 	for (i = 0; i < MLX5_HW_ACTION_FLAG_MAX; i++) {
@@ -9650,7 +9650,7 @@ mlx5_flow_meter_init(struct rte_eth_dev *dev,
 		.type = "mlx5_hw_mtr_mark_action",
 	};
 
-	if (!nb_meters || !nb_meter_profiles || !nb_meter_policies) {
+	if (!nb_meters) {
 		ret = ENOTSUP;
 		rte_flow_error_set(&error, ENOMEM,
 				  RTE_FLOW_ERROR_TYPE_UNSPECIFIED,
@@ -9751,33 +9751,37 @@ mlx5_flow_meter_init(struct rte_eth_dev *dev,
 		cfg.per_core_cache = MLX5_HW_IPOOL_CACHE_MIN;
 	}
 	priv->hws_mpool->idx_pool = mlx5_ipool_create(&cfg);
-	priv->mtr_config.nb_meter_profiles = nb_meter_profiles;
-	priv->mtr_profile_arr =
-		mlx5_malloc(MLX5_MEM_ZERO,
-			    sizeof(struct mlx5_flow_meter_profile) *
-			    nb_meter_profiles,
-			    RTE_CACHE_LINE_SIZE,
-			    SOCKET_ID_ANY);
-	if (!priv->mtr_profile_arr) {
-		ret = ENOMEM;
-		rte_flow_error_set(&error, ENOMEM,
-				  RTE_FLOW_ERROR_TYPE_UNSPECIFIED,
-				  NULL, "Meter profile allocation failed.");
-		goto err;
+	if (nb_meter_profiles) {
+		priv->mtr_config.nb_meter_profiles = nb_meter_profiles;
+		priv->mtr_profile_arr =
+			mlx5_malloc(MLX5_MEM_ZERO,
+				    sizeof(struct mlx5_flow_meter_profile) *
+				    nb_meter_profiles,
+				    RTE_CACHE_LINE_SIZE,
+				    SOCKET_ID_ANY);
+		if (!priv->mtr_profile_arr) {
+			ret = ENOMEM;
+			rte_flow_error_set(&error, ENOMEM,
+					   RTE_FLOW_ERROR_TYPE_UNSPECIFIED,
+					   NULL, "Meter profile allocation failed.");
+			goto err;
+		}
 	}
-	priv->mtr_config.nb_meter_policies = nb_meter_policies;
-	priv->mtr_policy_arr =
-		mlx5_malloc(MLX5_MEM_ZERO,
-			    sizeof(struct mlx5_flow_meter_policy) *
-			    nb_meter_policies,
-			    RTE_CACHE_LINE_SIZE,
-			    SOCKET_ID_ANY);
-	if (!priv->mtr_policy_arr) {
-		ret = ENOMEM;
-		rte_flow_error_set(&error, ENOMEM,
-				  RTE_FLOW_ERROR_TYPE_UNSPECIFIED,
-				  NULL, "Meter policy allocation failed.");
-		goto err;
+	if (nb_meter_policies) {
+		priv->mtr_config.nb_meter_policies = nb_meter_policies;
+		priv->mtr_policy_arr =
+			mlx5_malloc(MLX5_MEM_ZERO,
+				    sizeof(struct mlx5_flow_meter_policy) *
+				    nb_meter_policies,
+				    RTE_CACHE_LINE_SIZE,
+				    SOCKET_ID_ANY);
+		if (!priv->mtr_policy_arr) {
+			ret = ENOMEM;
+			rte_flow_error_set(&error, ENOMEM,
+					   RTE_FLOW_ERROR_TYPE_UNSPECIFIED,
+					   NULL, "Meter policy allocation failed.");
+			goto err;
+		}
 	}
 	return 0;
 err:
diff --git a/drivers/net/mlx5/mlx5_flow_meter.c b/drivers/net/mlx5/mlx5_flow_meter.c
index 08f8aad70a..735669e572 100644
--- a/drivers/net/mlx5/mlx5_flow_meter.c
+++ b/drivers/net/mlx5/mlx5_flow_meter.c
@@ -644,9 +644,7 @@ mlx5_flow_meter_profile_hws_add(struct rte_eth_dev *dev,
 	int ret;
 
 	if (!priv->mtr_profile_arr)
-		return -rte_mtr_error_set(error, ENOTSUP,
-					  RTE_MTR_ERROR_TYPE_UNSPECIFIED,
-					  NULL, "Meter profile array is not allocated");
+		return mlx5_flow_meter_profile_add(dev, meter_profile_id, profile, error);
 	/* Check input params. */
 	ret = mlx5_flow_meter_profile_validate(dev, meter_profile_id,
 					       profile, error);
@@ -683,15 +681,7 @@ mlx5_flow_meter_profile_hws_delete(struct rte_eth_dev *dev,
 	struct mlx5_flow_meter_profile *fmp;
 
 	if (!priv->mtr_profile_arr)
-		return -rte_mtr_error_set(error, ENOTSUP,
-					  RTE_MTR_ERROR_TYPE_UNSPECIFIED,
-					  NULL, "Meter profile array is not allocated");
-	/* Meter id must be valid. */
-	if (meter_profile_id >= priv->mtr_config.nb_meter_profiles)
-		return -rte_mtr_error_set(error, EINVAL,
-					  RTE_MTR_ERROR_TYPE_METER_PROFILE_ID,
-					  &meter_profile_id,
-					  "Meter profile id not valid.");
+		return mlx5_flow_meter_profile_delete(dev, meter_profile_id, error);
 	/* Meter profile must exist. */
 	fmp = mlx5_flow_meter_profile_find(priv, meter_profile_id);
 	if (!fmp->initialized)
@@ -1238,15 +1228,7 @@ mlx5_flow_meter_policy_hws_delete(struct rte_eth_dev *dev,
 	struct rte_flow_op_result result[RTE_COLORS * MLX5_MTR_DOMAIN_MAX];
 
 	if (!priv->mtr_policy_arr)
-		return -rte_mtr_error_set(error, ENOTSUP,
-					  RTE_MTR_ERROR_TYPE_UNSPECIFIED,
-					  NULL, "Meter policy array is not allocated");
-	/* Meter id must be valid. */
-	if (policy_id >= priv->mtr_config.nb_meter_policies)
-		return -rte_mtr_error_set(error, EINVAL,
-					  RTE_MTR_ERROR_TYPE_METER_POLICY_ID,
-					  &policy_id,
-					  "Meter policy id not valid.");
+		return mlx5_flow_meter_policy_delete(dev, policy_id, error);
 	/* Meter policy must exist. */
 	mtr_policy = mlx5_flow_meter_policy_find(dev, policy_id, NULL);
 	if (!mtr_policy->initialized)
@@ -1358,13 +1340,7 @@ mlx5_flow_meter_policy_hws_add(struct rte_eth_dev *dev,
 	};
 
 	if (!priv->mtr_policy_arr)
-		return -rte_mtr_error_set(error, ENOTSUP,
-					  RTE_MTR_ERROR_TYPE_METER_POLICY,
-					  NULL, "Meter policy array is not allocated.");
-	if (policy_id >= priv->mtr_config.nb_meter_policies)
-		return -rte_mtr_error_set(error, ENOTSUP,
-					  RTE_MTR_ERROR_TYPE_METER_POLICY_ID,
-					  NULL, "Meter policy id not valid.");
+		return mlx5_flow_meter_policy_add(dev, policy_id, policy, error);
 	mtr_policy = mlx5_flow_meter_policy_find(dev, policy_id, NULL);
 	if (mtr_policy->initialized)
 		return -rte_mtr_error_set(error, EEXIST,
-- 
2.18.2

RE: [PATCH] net/mlx5: relax hws meter validation

[Matan Azrad]

From: Alexander Kozyrev
> The policies array is not required for the new METER_MARK action.
> It is only required for the old METER action implementation.
> Furthermore, the profiles array is optional for the METER_MARK action since
> a profile can be found using the rte_mtr_meter_profile_get().
> Do not enforce the preallocation of meter profiles and policies.
> 
> Signed-off-by: Alexander Kozyrev <akozyrev@nvidia.com>
Acked-by: Matan Azrad <matan@nvidia.com>

RE: [PATCH] net/mlx5: relax hws meter validation

[Raslan Darawsheh]

Hi,

> -----Original Message-----
> From: Alexander Kozyrev <akozyrev@nvidia.com>
> Sent: Monday, March 13, 2023 8:15 PM
> To: dev@dpdk.org
> Cc: Raslan Darawsheh <rasland@nvidia.com>; Slava Ovsiienko
> <viacheslavo@nvidia.com>; Matan Azrad <matan@nvidia.com>; Ori Kam
> <orika@nvidia.com>
> Subject: [PATCH] net/mlx5: relax hws meter validation
> 
> The policies array is not required for the new METER_MARK action.
> It is only required for the old METER action implementation.
> Furthermore, the profiles array is optional for the METER_MARK action
> since a profile can be found using the rte_mtr_meter_profile_get().
> Do not enforce the preallocation of meter profiles and policies.
> 
> Signed-off-by: Alexander Kozyrev <akozyrev@nvidia.com>

Patch applied to next-net-mlx,

Kindest regards,
Raslan Darawsheh