[PATCH 23.11 0/3] net/mlx5: rebased fixes from 24.03

[PATCH 23.11 0/3] net/mlx5: rebased fixes from 24.03

[Dariusz Sosnowski]

Rebased fixes from 24.03 release which did not apply cleanly
on 23.11 LTS branch.

Dariusz Sosnowski (2):
  net/mlx5: fix async flow create error handling
  net/mlx5: fix rollback on failed flow configure

Erez Shitrit (1):
  net/mlx5/hws: fix port ID for root table

 drivers/net/mlx5/hws/mlx5dr_matcher.c |  17 ++++
 drivers/net/mlx5/hws/mlx5dr_rule.c    |  18 ++++
 drivers/net/mlx5/mlx5_flow.h          |  22 +++++
 drivers/net/mlx5/mlx5_flow_hw.c       | 124 ++++++++++++++++++--------
 4 files changed, 142 insertions(+), 39 deletions(-)

--
2.39.2

[PATCH 23.11 1/3] net/mlx5/hws: fix port ID for root table

[Dariusz Sosnowski]

From: Erez Shitrit <erezsh@nvidia.com>

[ upstream commit 572fe9ef2f461b7e7e195a3a4da5bf0c11f35949 ]

In root tables matcher and rule need to have their port-id, otherwise
the translate function that done in dpdk layer will not get the right
attributes.
For that whenever the matcher is matching the source-port we need to get
the relevant port-id before calling the translate function.

Fixes: 405242c52dd5 ("net/mlx5/hws: add rule object")
Cc: stable@dpdk.org

Signed-off-by: Erez Shitrit <erezsh@nvidia.com>
Signed-off-by: Yevgeny Kliteynik <kliteyn@nvidia.com>
Acked-by: Matan Azrad <matan@nvidia.com>
---
 drivers/net/mlx5/hws/mlx5dr_matcher.c | 17 +++++++++++++++++
 drivers/net/mlx5/hws/mlx5dr_rule.c    | 18 ++++++++++++++++++
 drivers/net/mlx5/mlx5_flow.h          | 22 ++++++++++++++++++++++
 3 files changed, 57 insertions(+)

diff --git a/drivers/net/mlx5/hws/mlx5dr_matcher.c b/drivers/net/mlx5/hws/mlx5dr_matcher.c
index 4ea161eae6..71478dab33 100644
--- a/drivers/net/mlx5/hws/mlx5dr_matcher.c
+++ b/drivers/net/mlx5/hws/mlx5dr_matcher.c
@@ -1143,6 +1143,7 @@ static int mlx5dr_matcher_init_root(struct mlx5dr_matcher *matcher)
 	struct mlx5dv_flow_match_parameters *mask;
 	struct mlx5_flow_attr flow_attr = {0};
 	struct rte_flow_error rte_error;
+	struct rte_flow_item *item;
 	uint8_t match_criteria;
 	int ret;
 
@@ -1171,6 +1172,22 @@ static int mlx5dr_matcher_init_root(struct mlx5dr_matcher *matcher)
 		return rte_errno;
 	}
 
+	/* We need the port id in case of matching representor */
+	item = matcher->mt[0].items;
+	while (item->type != RTE_FLOW_ITEM_TYPE_END) {
+		if (item->type == RTE_FLOW_ITEM_TYPE_PORT_REPRESENTOR ||
+		    item->type == RTE_FLOW_ITEM_TYPE_REPRESENTED_PORT) {
+			ret = flow_hw_get_port_id_from_ctx(ctx, &flow_attr.port_id);
+			if (ret) {
+				DR_LOG(ERR, "Failed to get port id for dev %s",
+				       ctx->ibv_ctx->device->name);
+				rte_errno = EINVAL;
+				return rte_errno;
+			}
+		}
+		++item;
+	}
+
 	mask = simple_calloc(1, MLX5_ST_SZ_BYTES(fte_match_param) +
 			     offsetof(struct mlx5dv_flow_match_parameters, match_buf));
 	if (!mask) {
diff --git a/drivers/net/mlx5/hws/mlx5dr_rule.c b/drivers/net/mlx5/hws/mlx5dr_rule.c
index 77245ad97d..02d57409ab 100644
--- a/drivers/net/mlx5/hws/mlx5dr_rule.c
+++ b/drivers/net/mlx5/hws/mlx5dr_rule.c
@@ -594,10 +594,28 @@ static int mlx5dr_rule_create_root(struct mlx5dr_rule *rule,
 	struct mlx5dv_flow_match_parameters *value;
 	struct mlx5_flow_attr flow_attr = {0};
 	struct mlx5dv_flow_action_attr *attr;
+	const struct rte_flow_item *cur_item;
 	struct rte_flow_error error;
 	uint8_t match_criteria;
 	int ret;
 
+	/* We need the port id in case of matching representor */
+	cur_item = items;
+	while (cur_item->type != RTE_FLOW_ITEM_TYPE_END) {
+		if (cur_item->type == RTE_FLOW_ITEM_TYPE_PORT_REPRESENTOR ||
+		    cur_item->type == RTE_FLOW_ITEM_TYPE_REPRESENTED_PORT) {
+			ret = flow_hw_get_port_id_from_ctx(rule->matcher->tbl->ctx,
+							   &flow_attr.port_id);
+			if (ret) {
+				DR_LOG(ERR, "Failed to get port id for dev %s",
+				       rule->matcher->tbl->ctx->ibv_ctx->device->name);
+				rte_errno = EINVAL;
+				return rte_errno;
+			}
+		}
+		++cur_item;
+	}
+
 	attr = simple_calloc(num_actions, sizeof(*attr));
 	if (!attr) {
 		rte_errno = ENOMEM;
diff --git a/drivers/net/mlx5/mlx5_flow.h b/drivers/net/mlx5/mlx5_flow.h
index 7a5e334a83..bde7dc43a8 100644
--- a/drivers/net/mlx5/mlx5_flow.h
+++ b/drivers/net/mlx5/mlx5_flow.h
@@ -1759,6 +1759,28 @@ flow_hw_get_reg_id_from_ctx(void *dr_ctx,
 	return REG_NON;
 }
 
+static __rte_always_inline int
+flow_hw_get_port_id_from_ctx(void *dr_ctx, uint32_t *port_val)
+{
+#if defined(HAVE_IBV_FLOW_DV_SUPPORT) || !defined(HAVE_INFINIBAND_VERBS_H)
+	uint32_t port;
+
+	MLX5_ETH_FOREACH_DEV(port, NULL) {
+		struct mlx5_priv *priv;
+		priv = rte_eth_devices[port].data->dev_private;
+
+		if (priv->dr_ctx == dr_ctx) {
+			*port_val = port;
+			return 0;
+		}
+	}
+#else
+	RTE_SET_USED(dr_ctx);
+	RTE_SET_USED(port_val);
+#endif
+	return -EINVAL;
+}
+
 void flow_hw_set_port_info(struct rte_eth_dev *dev);
 void flow_hw_clear_port_info(struct rte_eth_dev *dev);
 int flow_hw_create_vport_action(struct rte_eth_dev *dev);
-- 
2.39.2

[PATCH 23.11 2/3] net/mlx5: fix async flow create error handling

[Dariusz Sosnowski]

[ upstream commit 5ecc8df4fad3411a53c20406f99b59dc736a6d1e ]

Whenever processing of asynchronous flow rule create operation failed,
but after some dynamic flow actions had already been allocated,
these actions were not freed during error handling flow.
That behavior lead to leaks e.g., RSS/QUEUE action objects were leaked
which triggered assertions during device cleanup.

This patch adds flow rule cleanup handling in case of an error
during async flow rule creation.

Fixes: 3a2f674b6aa8 ("net/mlx5: add queue and RSS HW steering action")
Cc: stable@dpdk.org

Signed-off-by: Dariusz Sosnowski <dsosnowski@nvidia.com>
Acked-by: Ori Kam <orika@nvidia.com>
---
 drivers/net/mlx5/mlx5_flow_hw.c | 71 +++++++++++++++++++++++----------
 1 file changed, 51 insertions(+), 20 deletions(-)

diff --git a/drivers/net/mlx5/mlx5_flow_hw.c b/drivers/net/mlx5/mlx5_flow_hw.c
index 47fbbd0818..b51a4bb837 100644
--- a/drivers/net/mlx5/mlx5_flow_hw.c
+++ b/drivers/net/mlx5/mlx5_flow_hw.c
@@ -111,6 +111,10 @@ flow_hw_action_job_init(struct mlx5_priv *priv, uint32_t queue,
 			enum mlx5_hw_job_type type,
 			enum mlx5_hw_indirect_type indirect_type,
 			struct rte_flow_error *error);
+static void
+flow_hw_age_count_release(struct mlx5_priv *priv, uint32_t queue, struct rte_flow_hw *flow,
+			  struct rte_flow_error *error);
+
 static int
 mlx5_tbl_multi_pattern_process(struct rte_eth_dev *dev,
 			       struct rte_flow_template_table *tbl,
@@ -2865,6 +2869,30 @@ flow_hw_modify_field_construct(struct mlx5_hw_q_job *job,
 	return 0;
 }
 
+/**
+ * Release any actions allocated for the flow rule during actions construction.
+ *
+ * @param[in] flow
+ *   Pointer to flow structure.
+ */
+static void
+flow_hw_release_actions(struct rte_eth_dev *dev,
+			uint32_t queue,
+			struct rte_flow_hw *flow)
+{
+	struct mlx5_priv *priv = dev->data->dev_private;
+	struct mlx5_aso_mtr_pool *pool = priv->hws_mpool;
+
+	if (flow->fate_type == MLX5_FLOW_FATE_JUMP)
+		flow_hw_jump_release(dev, flow->jump);
+	else if (flow->fate_type == MLX5_FLOW_FATE_QUEUE)
+		mlx5_hrxq_obj_release(dev, flow->hrxq);
+	if (mlx5_hws_cnt_id_valid(flow->cnt_id))
+		flow_hw_age_count_release(priv, queue, flow, NULL);
+	if (flow->mtr_id)
+		mlx5_ipool_free(pool->idx_pool, flow->mtr_id);
+}
+
 /**
  * Construct flow action array.
  *
@@ -2980,7 +3008,7 @@ flow_hw_actions_construct(struct rte_eth_dev *dev,
 					(dev, queue, action, table, it_idx,
 					 at->action_flags, job->flow,
 					 &rule_acts[act_data->action_dst]))
-				return -1;
+				goto error;
 			break;
 		case RTE_FLOW_ACTION_TYPE_VOID:
 			break;
@@ -3000,7 +3028,7 @@ flow_hw_actions_construct(struct rte_eth_dev *dev,
 			jump = flow_hw_jump_action_register
 				(dev, &table->cfg, jump_group, NULL);
 			if (!jump)
-				return -1;
+				goto error;
 			rule_acts[act_data->action_dst].action =
 			(!!attr.group) ? jump->hws_action : jump->root_action;
 			job->flow->jump = jump;
@@ -3012,7 +3040,7 @@ flow_hw_actions_construct(struct rte_eth_dev *dev,
 					ft_flag,
 					action);
 			if (!hrxq)
-				return -1;
+				goto error;
 			rule_acts[act_data->action_dst].action = hrxq->action;
 			job->flow->hrxq = hrxq;
 			job->flow->fate_type = MLX5_FLOW_FATE_QUEUE;
@@ -3022,19 +3050,19 @@ flow_hw_actions_construct(struct rte_eth_dev *dev,
 			if (flow_hw_shared_action_get
 				(dev, act_data, item_flags,
 				 &rule_acts[act_data->action_dst]))
-				return -1;
+				goto error;
 			break;
 		case RTE_FLOW_ACTION_TYPE_VXLAN_ENCAP:
 			enc_item = ((const struct rte_flow_action_vxlan_encap *)
 				   action->conf)->definition;
 			if (flow_dv_convert_encap_data(enc_item, buf, &encap_len, NULL))
-				return -1;
+				goto error;
 			break;
 		case RTE_FLOW_ACTION_TYPE_NVGRE_ENCAP:
 			enc_item = ((const struct rte_flow_action_nvgre_encap *)
 				   action->conf)->definition;
 			if (flow_dv_convert_encap_data(enc_item, buf, &encap_len, NULL))
-				return -1;
+				goto error;
 			break;
 		case RTE_FLOW_ACTION_TYPE_RAW_ENCAP:
 			raw_encap_data =
@@ -3063,12 +3091,12 @@ flow_hw_actions_construct(struct rte_eth_dev *dev,
 								     hw_acts,
 								     action);
 			if (ret)
-				return -1;
+				goto error;
 			break;
 		case RTE_FLOW_ACTION_TYPE_REPRESENTED_PORT:
 			port_action = action->conf;
 			if (!priv->hw_vport[port_action->port_id])
-				return -1;
+				goto error;
 			rule_acts[act_data->action_dst].action =
 					priv->hw_vport[port_action->port_id];
 			break;
@@ -3088,7 +3116,7 @@ flow_hw_actions_construct(struct rte_eth_dev *dev,
 			jump = flow_hw_jump_action_register
 				(dev, &table->cfg, aso_mtr->fm.group, NULL);
 			if (!jump)
-				return -1;
+				goto error;
 			MLX5_ASSERT
 				(!rule_acts[act_data->action_dst + 1].action);
 			rule_acts[act_data->action_dst + 1].action =
@@ -3097,7 +3125,7 @@ flow_hw_actions_construct(struct rte_eth_dev *dev,
 			job->flow->jump = jump;
 			job->flow->fate_type = MLX5_FLOW_FATE_JUMP;
 			if (mlx5_aso_mtr_wait(priv, aso_mtr, true))
-				return -1;
+				goto error;
 			break;
 		case RTE_FLOW_ACTION_TYPE_AGE:
 			age = action->conf;
@@ -3112,7 +3140,7 @@ flow_hw_actions_construct(struct rte_eth_dev *dev,
 							     job->flow->res_idx,
 							     error);
 			if (age_idx == 0)
-				return -rte_errno;
+				goto error;
 			job->flow->age_idx = age_idx;
 			if (at->action_flags & MLX5_FLOW_ACTION_INDIRECT_COUNT)
 				/*
@@ -3126,7 +3154,7 @@ flow_hw_actions_construct(struct rte_eth_dev *dev,
 			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;
+				goto error;
 			ret = mlx5_hws_cnt_pool_get_action_offset
 				(priv->hws_cpool,
 				 cnt_id,
@@ -3134,7 +3162,7 @@ flow_hw_actions_construct(struct rte_eth_dev *dev,
 				 &rule_acts[act_data->action_dst].counter.offset
 				 );
 			if (ret != 0)
-				return ret;
+				goto error;
 			job->flow->cnt_id = cnt_id;
 			break;
 		case MLX5_RTE_FLOW_ACTION_TYPE_COUNT:
@@ -3145,7 +3173,7 @@ flow_hw_actions_construct(struct rte_eth_dev *dev,
 				 &rule_acts[act_data->action_dst].counter.offset
 				 );
 			if (ret != 0)
-				return ret;
+				goto error;
 			job->flow->cnt_id = act_data->shared_counter.id;
 			break;
 		case RTE_FLOW_ACTION_TYPE_CONNTRACK:
@@ -3153,7 +3181,7 @@ flow_hw_actions_construct(struct rte_eth_dev *dev,
 				 ((uint32_t)(uintptr_t)action->conf);
 			if (flow_hw_ct_compile(dev, queue, ct_idx,
 					       &rule_acts[act_data->action_dst]))
-				return -1;
+				goto error;
 			break;
 		case MLX5_RTE_FLOW_ACTION_TYPE_METER_MARK:
 			mtr_id = act_data->shared_meter.id &
@@ -3161,7 +3189,7 @@ flow_hw_actions_construct(struct rte_eth_dev *dev,
 			/* Find ASO object. */
 			aso_mtr = mlx5_ipool_get(pool->idx_pool, mtr_id);
 			if (!aso_mtr)
-				return -1;
+				goto error;
 			rule_acts[act_data->action_dst].action =
 							pool->action;
 			rule_acts[act_data->action_dst].aso_meter.offset =
@@ -3176,7 +3204,7 @@ flow_hw_actions_construct(struct rte_eth_dev *dev,
 				act_data->action_dst, action,
 				rule_acts, &job->flow->mtr_id, MLX5_HW_INV_QUEUE);
 			if (ret != 0)
-				return ret;
+				goto error;
 			break;
 		default:
 			break;
@@ -3214,6 +3242,11 @@ flow_hw_actions_construct(struct rte_eth_dev *dev,
 	if (mlx5_hws_cnt_id_valid(hw_acts->cnt_id))
 		job->flow->cnt_id = hw_acts->cnt_id;
 	return 0;
+
+error:
+	flow_hw_release_actions(dev, queue, job->flow);
+	rte_errno = EINVAL;
+	return -rte_errno;
 }
 
 static const struct rte_flow_item *
@@ -3363,10 +3396,8 @@ flow_hw_async_flow_create(struct rte_eth_dev *dev,
 	if (flow_hw_actions_construct(dev, job,
 				      &table->ats[action_template_index],
 				      pattern_template_index, actions,
-				      rule_acts, queue, error)) {
-		rte_errno = EINVAL;
+				      rule_acts, queue, error))
 		goto error;
-	}
 	rule_items = flow_hw_get_rule_items(dev, table, items,
 					    pattern_template_index, job);
 	if (!rule_items)
-- 
2.39.2

[PATCH 23.11 3/3] net/mlx5: fix rollback on failed flow configure

[Dariusz Sosnowski]

[ upstream commit 727283742a372359241fafec07f3648d3261e763 ]

If rte_flow_configure() failed, then some port resources
were either not freed, nor reset to the default state.
As a result, assumptions in other places in PMD were invalidated
and that lead to segmentation faults during release of HW Steering
resources when port was closed.

This patch adds missing resource release to rollback procedure
in mlx5 PMD implementation of rte_flow_configure().
Whole rollback procedure is reordered for clarity, to resemble
reverse order of resource allocation.

Fixes: 1939eb6f660c ("net/mlx5: support flow port action with HWS")
Fixes: 773ca0e91ba1 ("net/mlx5: support VLAN push/pop/modify with HWS")
Fixes: 04a4de756e14 ("net/mlx5: support flow age action with HWS")
Cc: stable@dpdk.org

Signed-off-by: Dariusz Sosnowski <dsosnowski@nvidia.com>
Acked-by: Ori Kam <orika@nvidia.com>
---
 drivers/net/mlx5/mlx5_flow_hw.c | 53 +++++++++++++++++++++------------
 1 file changed, 34 insertions(+), 19 deletions(-)

diff --git a/drivers/net/mlx5/mlx5_flow_hw.c b/drivers/net/mlx5/mlx5_flow_hw.c
index b51a4bb837..2f4b2d774e 100644
--- a/drivers/net/mlx5/mlx5_flow_hw.c
+++ b/drivers/net/mlx5/mlx5_flow_hw.c
@@ -9705,6 +9705,14 @@ flow_hw_configure(struct rte_eth_dev *dev,
 		priv->hws_strict_queue = 1;
 	return 0;
 err:
+	priv->hws_strict_queue = 0;
+	flow_hw_destroy_vlan(dev);
+	if (priv->hws_age_req)
+		mlx5_hws_age_pool_destroy(priv);
+	if (priv->hws_cpool) {
+		mlx5_hws_cnt_pool_destroy(priv->sh, priv->hws_cpool);
+		priv->hws_cpool = NULL;
+	}
 	if (priv->hws_ctpool) {
 		flow_hw_ct_pool_destroy(dev, priv->hws_ctpool);
 		priv->hws_ctpool = NULL;
@@ -9713,27 +9721,37 @@ flow_hw_configure(struct rte_eth_dev *dev,
 		flow_hw_ct_mng_destroy(dev, priv->ct_mng);
 		priv->ct_mng = NULL;
 	}
-	if (priv->hws_age_req)
-		mlx5_hws_age_pool_destroy(priv);
-	if (priv->hws_cpool) {
-		mlx5_hws_cnt_pool_destroy(priv->sh, priv->hws_cpool);
-		priv->hws_cpool = NULL;
-	}
-	mlx5_flow_quota_destroy(dev);
 	flow_hw_destroy_send_to_kernel_action(priv);
 	flow_hw_cleanup_ctrl_fdb_tables(dev);
 	flow_hw_free_vport_actions(priv);
+	if (priv->hw_def_miss) {
+		mlx5dr_action_destroy(priv->hw_def_miss);
+		priv->hw_def_miss = NULL;
+	}
+	flow_hw_cleanup_tx_repr_tagging(dev);
 	for (i = 0; i < MLX5_HW_ACTION_FLAG_MAX; i++) {
-		if (priv->hw_drop[i])
+		if (priv->hw_drop[i]) {
 			mlx5dr_action_destroy(priv->hw_drop[i]);
-		if (priv->hw_tag[i])
+			priv->hw_drop[i] = NULL;
+		}
+		if (priv->hw_tag[i]) {
 			mlx5dr_action_destroy(priv->hw_tag[i]);
+			priv->hw_tag[i] = NULL;
+		}
 	}
-	if (priv->hw_def_miss)
-		mlx5dr_action_destroy(priv->hw_def_miss);
-	flow_hw_destroy_vlan(dev);
-	if (dr_ctx)
+	mlx5_flow_meter_uninit(dev);
+	mlx5_flow_quota_destroy(dev);
+	flow_hw_cleanup_ctrl_rx_tables(dev);
+	if (dr_ctx) {
 		claim_zero(mlx5dr_context_close(dr_ctx));
+		priv->dr_ctx = NULL;
+	}
+	if (priv->shared_host) {
+		struct mlx5_priv *host_priv = priv->shared_host->data->dev_private;
+
+		__atomic_fetch_sub(&host_priv->shared_refcnt, 1, __ATOMIC_RELAXED);
+		priv->shared_host = NULL;
+	}
 	if (priv->hw_q) {
 		for (i = 0; i < nb_q_updated; i++) {
 			rte_ring_free(priv->hw_q[i].indir_iq);
@@ -9746,14 +9764,11 @@ flow_hw_configure(struct rte_eth_dev *dev,
 		mlx5_ipool_destroy(priv->acts_ipool);
 		priv->acts_ipool = NULL;
 	}
-	if (_queue_attr)
-		mlx5_free(_queue_attr);
-	if (priv->shared_host) {
-		__atomic_fetch_sub(&host_priv->shared_refcnt, 1, __ATOMIC_RELAXED);
-		priv->shared_host = NULL;
-	}
 	mlx5_free(priv->hw_attr);
 	priv->hw_attr = NULL;
+	priv->nb_queue = 0;
+	if (_queue_attr)
+		mlx5_free(_queue_attr);
 	/* Do not overwrite the internal errno information. */
 	if (ret)
 		return ret;
-- 
2.39.2

Re: [PATCH 23.11 0/3] net/mlx5: rebased fixes from 24.03

[Xueming Li]

[-- Attachment #1: Type: text/plain, Size: 902 bytes --]

Hi Dariusz,

Thanks for the backporting, 3 patches applied to 23.11.1 release candidate queue.

________________________________
From: Dariusz Sosnowski <dsosnowski@nvidia.com>
Sent: Tuesday, April 23, 2024 12:50 AM
Cc: stable@dpdk.org <stable@dpdk.org>
Subject: [PATCH 23.11 0/3] net/mlx5: rebased fixes from 24.03

Rebased fixes from 24.03 release which did not apply cleanly
on 23.11 LTS branch.

Dariusz Sosnowski (2):
  net/mlx5: fix async flow create error handling
  net/mlx5: fix rollback on failed flow configure

Erez Shitrit (1):
  net/mlx5/hws: fix port ID for root table

 drivers/net/mlx5/hws/mlx5dr_matcher.c |  17 ++++
 drivers/net/mlx5/hws/mlx5dr_rule.c    |  18 ++++
 drivers/net/mlx5/mlx5_flow.h          |  22 +++++
 drivers/net/mlx5/mlx5_flow_hw.c       | 124 ++++++++++++++++++--------
 4 files changed, 142 insertions(+), 39 deletions(-)

--
2.39.2


[-- Attachment #2: Type: text/html, Size: 2430 bytes --]