DPDK patches and discussions
 help / color / mirror / Atom feed
* [PATCH 1/4] net/mlx5: reorganize main structures
@ 2024-06-02 10:29 Maayan Kashani
  2024-06-03  8:16 ` [PATCH v2 22/34] " Maayan Kashani
  2024-06-03 10:54 ` [PATCH v3 1/4] net/mlx5: reorganize main structures Maayan Kashani
  0 siblings, 2 replies; 15+ messages in thread
From: Maayan Kashani @ 2024-06-02 10:29 UTC (permalink / raw)
  To: dev
  Cc: mkashani, dsosnowski, rasland, Viacheslav Ovsiienko, Ori Kam,
	Suanming Mou, Matan Azrad

Reorganize rte_flow_hw and rte_flow_nt2hws
structures for better performance, and removed packed.

Signed-off-by: Maayan Kashani <mkashani@nvidia.com>
---
 drivers/net/mlx5/mlx5_flow.h | 26 ++++++++++++--------------
 1 file changed, 12 insertions(+), 14 deletions(-)

diff --git a/drivers/net/mlx5/mlx5_flow.h b/drivers/net/mlx5/mlx5_flow.h
index 7e0f005741..5a3f047968 100644
--- a/drivers/net/mlx5/mlx5_flow.h
+++ b/drivers/net/mlx5/mlx5_flow.h
@@ -1328,12 +1328,12 @@ struct rte_flow_nt2hws {
 	struct rte_flow_hw_aux *flow_aux;
 	/** Modify header pointer. */
 	struct mlx5_flow_dv_modify_hdr_resource *modify_hdr;
+	/** Chain NTA flows. */
+	SLIST_ENTRY(rte_flow_hw) next;
 	/** Encap/decap index. */
 	uint32_t rix_encap_decap;
 	uint8_t chaned_flow;
-	/** Chain NTA flows. */
-	SLIST_ENTRY(rte_flow_hw) next;
-} __rte_packed;
+};
 
 /** HWS flow struct. */
 struct rte_flow_hw {
@@ -1345,6 +1345,12 @@ struct rte_flow_hw {
 	};
 	/** Application's private data passed to enqueued flow operation. */
 	void *user_data;
+	union {
+		/** Jump action. */
+		struct mlx5_hw_jump_action *jump;
+		/** TIR action. */
+		struct mlx5_hrxq *hrxq;
+	};
 	/** Flow index from indexed pool. */
 	uint32_t idx;
 	/** Resource index from indexed pool. */
@@ -1353,20 +1359,12 @@ struct rte_flow_hw {
 	uint32_t rule_idx;
 	/** Which flow fields (inline or in auxiliary struct) are used. */
 	uint32_t flags;
+	/** COUNT action index. */
+	cnt_id_t cnt_id;
 	/** Ongoing flow operation type. */
 	uint8_t operation_type;
 	/** Index of pattern template this flow is based on. */
 	uint8_t mt_idx;
-
-	/** COUNT action index. */
-	cnt_id_t cnt_id;
-	union {
-		/** Jump action. */
-		struct mlx5_hw_jump_action *jump;
-		/** TIR action. */
-		struct mlx5_hrxq *hrxq;
-	};
-
 	/** Equals true if it is non template rule. */
 	bool nt_rule;
 	/**
@@ -1377,7 +1375,7 @@ struct rte_flow_hw {
 	uint8_t padding[9];
 	/** HWS layer data struct. */
 	uint8_t rule[];
-} __rte_packed;
+};
 
 /** Auxiliary data fields that are updatable. */
 struct rte_flow_hw_aux_fields {
-- 
2.25.1


From 662c1b56b66c3d50eb3ee9911bc35b63811ae0af Mon Sep 17 00:00:00 2001
From: Maayan Kashani <mkashani@nvidia.com>
Date: Wed, 13 Mar 2024 08:45:19 +0200
Subject: [PATCH 2/4] net/mlx5: set modify header as shared action

In current implementation, in non template mode,
modify header action is not set as always shared.
Align to HWS implementation, setting modify header action as always shared.
Optimize mask initialization.

Signed-off-by: Maayan Kashani <mkashani@nvidia.com>
---
 drivers/net/mlx5/mlx5_flow.h    |  3 --
 drivers/net/mlx5/mlx5_flow_dv.c | 10 ++--
 drivers/net/mlx5/mlx5_flow_hw.c | 91 +++++++++++++++++++--------------
 3 files changed, 59 insertions(+), 45 deletions(-)

diff --git a/drivers/net/mlx5/mlx5_flow.h b/drivers/net/mlx5/mlx5_flow.h
index 5a3f047968..f5bb01616e 100644
--- a/drivers/net/mlx5/mlx5_flow.h
+++ b/drivers/net/mlx5/mlx5_flow.h
@@ -669,9 +669,6 @@ struct mlx5_flow_dv_modify_hdr_resource {
 	struct mlx5_list_entry entry;
 	void *action; /**< Modify header action object. */
 	uint32_t idx;
-#ifdef HAVE_MLX5_HWS_SUPPORT
-	void *mh_dr_pattern; /**< Modify header DR pattern(HWS only). */
-#endif
 	uint64_t flags; /**< Flags for RDMA API(HWS only). */
 	/* Key area for hash list matching: */
 	uint8_t ft_type; /**< Flow table type, Rx or Tx. */
diff --git a/drivers/net/mlx5/mlx5_flow_dv.c b/drivers/net/mlx5/mlx5_flow_dv.c
index 3611ffa4a1..94af391894 100644
--- a/drivers/net/mlx5/mlx5_flow_dv.c
+++ b/drivers/net/mlx5/mlx5_flow_dv.c
@@ -6219,9 +6219,7 @@ flow_modify_create_cb(void *tool_ctx, void *cb_ctx)
 	uint32_t data_len = ref->actions_num * sizeof(ref->actions[0]);
 	uint32_t key_len = sizeof(*ref) - offsetof(typeof(*ref), ft_type);
 	uint32_t idx;
-	struct mlx5_tbl_multi_pattern_ctx *mpctx;
 
-	typeof(mpctx->mh) *mh_dr_pattern = ref->mh_dr_pattern;
 	if (unlikely(!ipool)) {
 		rte_flow_error_set(ctx->error, ENOMEM,
 				   RTE_FLOW_ERROR_TYPE_UNSPECIFIED,
@@ -6240,9 +6238,13 @@ flow_modify_create_cb(void *tool_ctx, void *cb_ctx)
 			key_len + data_len);
 	if (sh->config.dv_flow_en == 2) {
 #ifdef HAVE_MLX5_HWS_SUPPORT
+		struct mlx5dr_action_mh_pattern pattern = {
+			.sz = data_len,
+			.data = (__be64 *)ref->actions
+		};
 		entry->action = mlx5dr_action_create_modify_header(ctx->data2,
-			mh_dr_pattern->elements_num,
-			mh_dr_pattern->pattern, 0, ref->flags);
+			1,
+			&pattern, 0, ref->flags);
 		if (!entry->action)
 			ret = -1;
 #else
diff --git a/drivers/net/mlx5/mlx5_flow_hw.c b/drivers/net/mlx5/mlx5_flow_hw.c
index 43bcaab592..134a035f41 100644
--- a/drivers/net/mlx5/mlx5_flow_hw.c
+++ b/drivers/net/mlx5/mlx5_flow_hw.c
@@ -1439,13 +1439,11 @@ flow_hw_converted_mhdr_cmds_append(struct mlx5_hw_modify_header_action *mhdr,
 
 static __rte_always_inline void
 flow_hw_modify_field_init(struct mlx5_hw_modify_header_action *mhdr,
-			  struct rte_flow_actions_template *at,
-			  bool nt_mode)
+			  struct rte_flow_actions_template *at)
 {
 	memset(mhdr, 0, sizeof(*mhdr));
 	/* Modify header action without any commands is shared by default. */
-	if (!(nt_mode))
-		mhdr->shared = true;
+	mhdr->shared = true;
 	mhdr->pos = at->mhdr_off;
 }
 
@@ -2212,10 +2210,6 @@ mlx5_tbl_translate_modify_header(struct rte_eth_dev *dev,
 				 struct mlx5_hw_modify_header_action *mhdr,
 				 struct rte_flow_error *error)
 {
-	struct mlx5_priv *priv = dev->data->dev_private;
-	const struct rte_flow_template_table_attr *table_attr = &cfg->attr;
-	const struct rte_flow_attr *attr = &table_attr->flow_attr;
-	enum mlx5dr_table_type tbl_type = get_mlx5dr_table_type(attr);
 	uint16_t mhdr_ix = mhdr->pos;
 	struct mlx5dr_action_mh_pattern pattern = {
 		.sz = sizeof(struct mlx5_modification_cmd) * mhdr->mhdr_cmds_num
@@ -2232,20 +2226,8 @@ mlx5_tbl_translate_modify_header(struct rte_eth_dev *dev,
 					  RTE_FLOW_ERROR_TYPE_UNSPECIFIED,
 					  NULL, "translate modify_header: no memory for modify header context");
 	rte_memcpy(acts->mhdr, mhdr, sizeof(*mhdr));
-	pattern.data = (__be64 *)acts->mhdr->mhdr_cmds;
-	if (mhdr->shared) {
-		uint32_t flags = mlx5_hw_act_flag[!!attr->group][tbl_type] |
-				 MLX5DR_ACTION_FLAG_SHARED;
-
-		acts->mhdr->action = mlx5dr_action_create_modify_header
-						(priv->dr_ctx, 1, &pattern, 0,
-						 flags);
-		if (!acts->mhdr->action)
-			return rte_flow_error_set(error, rte_errno,
-						  RTE_FLOW_ERROR_TYPE_UNSPECIFIED,
-						  NULL, "translate modify_header: failed to create DR action");
-		acts->rule_acts[mhdr_ix].action = acts->mhdr->action;
-	} else {
+	if (!mhdr->shared) {
+		pattern.data = (__be64 *)acts->mhdr->mhdr_cmds;
 		typeof(mp_ctx->mh) *mh = &mp_ctx->mh;
 		uint32_t idx = mh->elements_num;
 		mh->pattern[mh->elements_num++] = pattern;
@@ -2256,6 +2238,32 @@ mlx5_tbl_translate_modify_header(struct rte_eth_dev *dev,
 	return 0;
 }
 
+static int
+mlx5_tbl_ensure_shared_modify_header(struct rte_eth_dev *dev,
+				     const struct mlx5_flow_template_table_cfg *cfg,
+				     struct mlx5_hw_actions *acts,
+				     struct rte_flow_error *error)
+{
+	struct mlx5_priv *priv = dev->data->dev_private;
+	const struct rte_flow_template_table_attr *table_attr = &cfg->attr;
+	const struct rte_flow_attr *attr = &table_attr->flow_attr;
+	enum mlx5dr_table_type tbl_type = get_mlx5dr_table_type(attr);
+	struct mlx5dr_action_mh_pattern pattern = {
+		.sz = sizeof(struct mlx5_modification_cmd) * acts->mhdr->mhdr_cmds_num
+	};
+	uint16_t mhdr_ix = acts->mhdr->pos;
+	uint32_t flags = mlx5_hw_act_flag[!!attr->group][tbl_type] | MLX5DR_ACTION_FLAG_SHARED;
+
+	pattern.data = (__be64 *)acts->mhdr->mhdr_cmds;
+	acts->mhdr->action = mlx5dr_action_create_modify_header(priv->dr_ctx, 1,
+								&pattern, 0, flags);
+	if (!acts->mhdr->action)
+		return rte_flow_error_set(error, rte_errno,
+					  RTE_FLOW_ERROR_TYPE_UNSPECIFIED, NULL,
+					  "translate modify_header: failed to create DR action");
+	acts->rule_acts[mhdr_ix].action = acts->mhdr->action;
+	return 0;
+}
 
 static int
 mlx5_create_ipv6_ext_reformat(struct rte_eth_dev *dev,
@@ -2393,7 +2401,7 @@ __flow_hw_translate_actions_template(struct rte_eth_dev *dev,
 	uint32_t target_grp = 0;
 	int table_type;
 
-	flow_hw_modify_field_init(&mhdr, at, nt_mode);
+	flow_hw_modify_field_init(&mhdr, at);
 	if (attr->transfer)
 		type = MLX5DR_TABLE_TYPE_FDB;
 	else if (attr->egress)
@@ -2780,10 +2788,14 @@ __flow_hw_translate_actions_template(struct rte_eth_dev *dev,
 		}
 	}
 	if (mhdr.pos != UINT16_MAX) {
-		ret = mlx5_tbl_translate_modify_header(dev, cfg, acts, mp_ctx,
-						       &mhdr, error);
+		ret = mlx5_tbl_translate_modify_header(dev, cfg, acts, mp_ctx, &mhdr, error);
 		if (ret)
 			goto err;
+		if (!nt_mode && mhdr.shared) {
+			ret = mlx5_tbl_ensure_shared_modify_header(dev, cfg, acts, error);
+			if (ret)
+				goto err;
+		}
 	}
 	if (reformat_used) {
 		ret = mlx5_tbl_translate_reformat(priv, table_attr, acts, at,
@@ -12348,8 +12360,8 @@ static int flow_hw_prepare(struct rte_eth_dev *dev,
 	/*TODO: consider if other allocation is needed for actions translate. */
 	return 0;
 }
-#define FLOW_HW_SET_DV_FIELDS(flow_attr, root, flags)						\
-{												\
+
+#define FLOW_HW_SET_DV_FIELDS(flow_attr, root, flags, dv_resource) {				\
 	typeof(flow_attr) _flow_attr = (flow_attr);						\
 	if (_flow_attr->transfer)								\
 		dv_resource.ft_type = MLX5DV_FLOW_TABLE_TYPE_FDB;				\
@@ -12370,28 +12382,31 @@ flow_hw_modify_hdr_resource_register
 {
 	struct rte_flow_attr *attr = &table->cfg.attr.flow_attr;
 	struct mlx5_flow_dv_modify_hdr_resource *dv_resource_ptr = NULL;
-	struct mlx5_flow_dv_modify_hdr_resource dv_resource;
-	struct mlx5_tbl_multi_pattern_ctx *mpctx = &table->mpctx;
+	union {
+		struct mlx5_flow_dv_modify_hdr_resource dv_resource;
+		uint8_t data[sizeof(struct mlx5_flow_dv_modify_hdr_resource) +
+			     sizeof(struct mlx5_modification_cmd) * MLX5_MHDR_MAX_CMD];
+	} dummy;
 	int ret;
 
 	if (hw_acts->mhdr) {
-		dv_resource.actions_num = hw_acts->mhdr->mhdr_cmds_num;
-		memcpy(dv_resource.actions, hw_acts->mhdr->mhdr_cmds,
-			sizeof(struct mlx5_modification_cmd) * dv_resource.actions_num);
+		dummy.dv_resource.actions_num = hw_acts->mhdr->mhdr_cmds_num;
+		memcpy(dummy.dv_resource.actions, hw_acts->mhdr->mhdr_cmds,
+			sizeof(struct mlx5_modification_cmd) * dummy.dv_resource.actions_num);
 	} else {
 		return 0;
 	}
-	FLOW_HW_SET_DV_FIELDS(attr, dv_resource.root, dv_resource.flags);
-	/* Save a pointer to the pattern needed for DR layer created on actions translate. */
-	dv_resource.mh_dr_pattern = &table->mpctx.mh;
-	ret = __flow_modify_hdr_resource_register(dev, &dv_resource,
+	FLOW_HW_SET_DV_FIELDS(attr, dummy.dv_resource.root, dummy.dv_resource.flags,
+			      dummy.dv_resource);
+	dummy.dv_resource.flags |= MLX5DR_ACTION_FLAG_SHARED;
+	ret = __flow_modify_hdr_resource_register(dev, &dummy.dv_resource,
 		&dv_resource_ptr, error);
 	if (ret)
 		return ret;
 	MLX5_ASSERT(dv_resource_ptr);
 	dev_flow->nt2hws->modify_hdr = dv_resource_ptr;
 	/* keep action for the rule construction. */
-	mpctx->segments[0].mhdr_action = dv_resource_ptr->action;
+	hw_acts->rule_acts[hw_acts->mhdr->pos].action = dv_resource_ptr->action;
 	/* Bulk size is 1, so index is 1. */
 	dev_flow->res_idx = 1;
 	return 0;
@@ -12426,7 +12441,7 @@ flow_hw_encap_decap_resource_register
 		return rte_flow_error_set(error, EINVAL, RTE_FLOW_ERROR_TYPE_ACTION,
 				   NULL, "No reformat action exist in the table.");
 	dv_resource.size = reformat->reformat_hdr->sz;
-	FLOW_HW_SET_DV_FIELDS(attr, is_root, dv_resource.flags);
+	FLOW_HW_SET_DV_FIELDS(attr, is_root, dv_resource.flags, dv_resource);
 	MLX5_ASSERT(dv_resource.size <= MLX5_ENCAP_MAX_LEN);
 	memcpy(dv_resource.buf, reformat->reformat_hdr->data, dv_resource.size);
 	ret = __flow_encap_decap_resource_register(dev, &dv_resource, is_root,
-- 
2.25.1


From 3acb029275b5b606834fb07ac1b036e1645a309d Mon Sep 17 00:00:00 2001
From: Maayan Kashani <mkashani@nvidia.com>
Date: Wed, 13 Mar 2024 22:59:03 +0200
Subject: [PATCH 3/4] net/mlx5: set encap as shared action

In current implementation, in non template mode,
encap action is not set as non shared(according to given masks).
By masking the relevant fields, encap is now used as shared.
decap remained unshared and cannot be shared according to
current implementation.
Optimize encap action mask initialization and fixed requested
number of reformat actions to one.

Signed-off-by: Maayan Kashani <mkashani@nvidia.com>
---
 drivers/net/mlx5/mlx5_flow_dv.c |   2 +-
 drivers/net/mlx5/mlx5_flow_hw.c | 119 +++++++++++++++++++++++---------
 2 files changed, 86 insertions(+), 35 deletions(-)

diff --git a/drivers/net/mlx5/mlx5_flow_dv.c b/drivers/net/mlx5/mlx5_flow_dv.c
index 94af391894..1484531418 100644
--- a/drivers/net/mlx5/mlx5_flow_dv.c
+++ b/drivers/net/mlx5/mlx5_flow_dv.c
@@ -4317,7 +4317,7 @@ flow_encap_decap_create_cb(void *tool_ctx, void *cb_ctx)
 		hdr.data = ctx_resource->buf;
 		resource->action = mlx5dr_action_create_reformat
 		(ctx->data2, (enum mlx5dr_action_type)ctx_resource->reformat_type, 1,
-			&hdr, 1, ctx_resource->flags);
+			&hdr, 0, ctx_resource->flags);
 		if (!resource->action)
 			ret = -1;
 #else
diff --git a/drivers/net/mlx5/mlx5_flow_hw.c b/drivers/net/mlx5/mlx5_flow_hw.c
index 134a035f41..61b6a71bbf 100644
--- a/drivers/net/mlx5/mlx5_flow_hw.c
+++ b/drivers/net/mlx5/mlx5_flow_hw.c
@@ -2124,9 +2124,17 @@ table_template_translate_indirect_list(struct rte_eth_dev *dev,
 	return ret;
 }
 
+static void
+mlx5_set_reformat_header(struct mlx5dr_action_reformat_header *hdr,
+			 uint8_t *encap_data,
+			 size_t data_size)
+{
+	hdr->sz = data_size;
+	hdr->data = encap_data;
+}
+
 static int
 mlx5_tbl_translate_reformat(struct mlx5_priv *priv,
-			    const struct rte_flow_template_table_attr *table_attr,
 			    struct mlx5_hw_actions *acts,
 			    struct rte_flow_actions_template *at,
 			    const struct rte_flow_item *enc_item,
@@ -2138,8 +2146,6 @@ mlx5_tbl_translate_reformat(struct mlx5_priv *priv,
 			    struct rte_flow_error *error)
 {
 	int mp_reformat_ix = mlx5_multi_pattern_reformat_to_index(refmt_type);
-	const struct rte_flow_attr *attr = &table_attr->flow_attr;
-	enum mlx5dr_table_type tbl_type = get_mlx5dr_table_type(attr);
 	struct mlx5dr_action_reformat_header hdr;
 	uint8_t buf[MLX5_ENCAP_MAX_LEN];
 	bool shared_rfmt = false;
@@ -2164,19 +2170,16 @@ mlx5_tbl_translate_reformat(struct mlx5_priv *priv,
 		return rte_flow_error_set(error, ENOMEM,
 					  RTE_FLOW_ERROR_TYPE_UNSPECIFIED,
 					  NULL, "no memory for reformat context");
-	hdr.sz = data_size;
-	hdr.data = encap_data;
+	acts->encap_decap_pos = at->reformat_off;
+	acts->encap_decap->data_size = data_size;
+	acts->encap_decap->action_type = refmt_type;
 	if (shared_rfmt || mp_reformat_ix < 0) {
 		uint16_t reformat_ix = at->reformat_off;
-		uint32_t flags = mlx5_hw_act_flag[!!attr->group][tbl_type] |
-				 MLX5DR_ACTION_FLAG_SHARED;
-
-		acts->encap_decap->action =
-			mlx5dr_action_create_reformat(priv->dr_ctx, refmt_type,
-						      1, &hdr, 0, flags);
-		if (!acts->encap_decap->action)
-			return -rte_errno;
-		acts->rule_acts[reformat_ix].action = acts->encap_decap->action;
+		/*
+		 * This copy is only needed in non template mode.
+		 * In order to create the action later.
+		 */
+		memcpy(acts->encap_decap->data, encap_data, data_size);
 		acts->rule_acts[reformat_ix].reformat.data = acts->encap_decap->data;
 		acts->rule_acts[reformat_ix].reformat.offset = 0;
 		acts->encap_decap->shared = true;
@@ -2184,14 +2187,11 @@ mlx5_tbl_translate_reformat(struct mlx5_priv *priv,
 		uint32_t ix;
 		typeof(mp_ctx->reformat[0]) *reformat = mp_ctx->reformat +
 							mp_reformat_ix;
-
+		mlx5_set_reformat_header(&hdr, encap_data, data_size);
 		ix = reformat->elements_num++;
 		reformat->reformat_hdr[ix] = hdr;
 		acts->rule_acts[at->reformat_off].reformat.hdr_idx = ix;
-		acts->encap_decap_pos = at->reformat_off;
 		acts->encap_decap->multi_pattern = 1;
-		acts->encap_decap->data_size = data_size;
-		acts->encap_decap->action_type = refmt_type;
 		ret = __flow_hw_act_data_encap_append
 			(priv, acts, (at->actions + reformat_src)->type,
 			 reformat_src, at->reformat_off, data_size);
@@ -2202,6 +2202,32 @@ mlx5_tbl_translate_reformat(struct mlx5_priv *priv,
 	return 0;
 }
 
+static int
+mlx5_tbl_create_reformat_action(struct mlx5_priv *priv,
+				const struct rte_flow_template_table_attr *table_attr,
+				struct mlx5_hw_actions *acts,
+				struct rte_flow_actions_template *at,
+				uint8_t *encap_data,
+				size_t data_size,
+				enum mlx5dr_action_type refmt_type)
+{
+	const struct rte_flow_attr *attr = &table_attr->flow_attr;
+	enum mlx5dr_table_type tbl_type = get_mlx5dr_table_type(attr);
+	struct mlx5dr_action_reformat_header hdr;
+
+	mlx5_set_reformat_header(&hdr, encap_data, data_size);
+	uint16_t reformat_ix = at->reformat_off;
+	uint32_t flags = mlx5_hw_act_flag[!!attr->group][tbl_type] |
+				MLX5DR_ACTION_FLAG_SHARED;
+
+	acts->encap_decap->action = mlx5dr_action_create_reformat(priv->dr_ctx, refmt_type,
+							   1, &hdr, 0, flags);
+	if (!acts->encap_decap->action)
+		return -rte_errno;
+	acts->rule_acts[reformat_ix].action = acts->encap_decap->action;
+	return 0;
+}
+
 static int
 mlx5_tbl_translate_modify_header(struct rte_eth_dev *dev,
 				 const struct mlx5_flow_template_table_cfg *cfg,
@@ -2798,7 +2824,7 @@ __flow_hw_translate_actions_template(struct rte_eth_dev *dev,
 		}
 	}
 	if (reformat_used) {
-		ret = mlx5_tbl_translate_reformat(priv, table_attr, acts, at,
+		ret = mlx5_tbl_translate_reformat(priv, acts, at,
 						  enc_item, enc_item_m,
 						  encap_data, encap_data_m,
 						  mp_ctx, data_size,
@@ -2806,6 +2832,13 @@ __flow_hw_translate_actions_template(struct rte_eth_dev *dev,
 						  refmt_type, error);
 		if (ret)
 			goto err;
+		if (!nt_mode && acts->encap_decap->shared) {
+			ret = mlx5_tbl_create_reformat_action(priv, table_attr, acts, at,
+							      encap_data, data_size,
+							      refmt_type);
+			if (ret)
+				goto err;
+		}
 	}
 	if (recom_used) {
 		MLX5_ASSERT(at->recom_off != UINT16_MAX);
@@ -12361,7 +12394,7 @@ static int flow_hw_prepare(struct rte_eth_dev *dev,
 	return 0;
 }
 
-#define FLOW_HW_SET_DV_FIELDS(flow_attr, root, flags, dv_resource) {				\
+#define FLOW_HW_SET_DV_FIELDS(flow_attr, root, dv_resource) {					\
 	typeof(flow_attr) _flow_attr = (flow_attr);						\
 	if (_flow_attr->transfer)								\
 		dv_resource.ft_type = MLX5DV_FLOW_TABLE_TYPE_FDB;				\
@@ -12369,7 +12402,8 @@ static int flow_hw_prepare(struct rte_eth_dev *dev,
 		dv_resource.ft_type = _flow_attr->egress ? MLX5DV_FLOW_TABLE_TYPE_NIC_TX :	\
 					     MLX5DV_FLOW_TABLE_TYPE_NIC_RX;			\
 	root = _flow_attr->group ? 0 : 1;							\
-	flags = mlx5_hw_act_flag[!!_flow_attr->group][get_mlx5dr_table_type(_flow_attr)];	\
+	dv_resource.flags =									\
+		mlx5_hw_act_flag[!!_flow_attr->group][get_mlx5dr_table_type(_flow_attr)];	\
 }
 
 static int
@@ -12396,8 +12430,7 @@ flow_hw_modify_hdr_resource_register
 	} else {
 		return 0;
 	}
-	FLOW_HW_SET_DV_FIELDS(attr, dummy.dv_resource.root, dummy.dv_resource.flags,
-			      dummy.dv_resource);
+	FLOW_HW_SET_DV_FIELDS(attr, dummy.dv_resource.root, dummy.dv_resource);
 	dummy.dv_resource.flags |= MLX5DR_ACTION_FLAG_SHARED;
 	ret = __flow_modify_hdr_resource_register(dev, &dummy.dv_resource,
 		&dv_resource_ptr, error);
@@ -12432,18 +12465,25 @@ flow_hw_encap_decap_resource_register
 		dv_resource.reformat_type = hw_acts->encap_decap->action_type;
 	else
 		return 0;
+	FLOW_HW_SET_DV_FIELDS(attr, is_root, dv_resource);
 	ix = mlx5_bwc_multi_pattern_reformat_to_index((enum mlx5dr_action_type)
-		dv_resource.reformat_type);
+			dv_resource.reformat_type);
 	if (ix < 0)
 		return ix;
-	typeof(mpctx->reformat[0]) *reformat = mpctx->reformat + ix;
-	if (!reformat->elements_num)
-		return rte_flow_error_set(error, EINVAL, RTE_FLOW_ERROR_TYPE_ACTION,
-				   NULL, "No reformat action exist in the table.");
-	dv_resource.size = reformat->reformat_hdr->sz;
-	FLOW_HW_SET_DV_FIELDS(attr, is_root, dv_resource.flags, dv_resource);
-	MLX5_ASSERT(dv_resource.size <= MLX5_ENCAP_MAX_LEN);
-	memcpy(dv_resource.buf, reformat->reformat_hdr->data, dv_resource.size);
+	if (hw_acts->encap_decap->shared) {
+		dv_resource.size = hw_acts->encap_decap->data_size;
+		MLX5_ASSERT(dv_resource.size <= MLX5_ENCAP_MAX_LEN);
+		memcpy(&dv_resource.buf, hw_acts->encap_decap->data, dv_resource.size);
+		dv_resource.flags |= MLX5DR_ACTION_FLAG_SHARED;
+	} else {
+		typeof(mpctx->reformat[0]) *reformat = mpctx->reformat + ix;
+		if (!reformat->elements_num)
+			return rte_flow_error_set(error, EINVAL, RTE_FLOW_ERROR_TYPE_ACTION,
+					NULL, "No reformat action exist in the table.");
+		dv_resource.size = reformat->reformat_hdr->sz;
+		MLX5_ASSERT(dv_resource.size <= MLX5_ENCAP_MAX_LEN);
+		memcpy(&dv_resource.buf, reformat->reformat_hdr->data, dv_resource.size);
+	}
 	ret = __flow_encap_decap_resource_register(dev, &dv_resource, is_root,
 		&dv_resource_ptr, error);
 	if (ret)
@@ -12451,7 +12491,10 @@ flow_hw_encap_decap_resource_register
 	MLX5_ASSERT(dv_resource_ptr);
 	dev_flow->nt2hws->rix_encap_decap = dv_resource_ptr->idx;
 	/* keep action for the rule construction. */
-	mpctx->segments[0].reformat_action[ix] = dv_resource_ptr->action;
+	if (hw_acts->encap_decap->shared)
+		hw_acts->rule_acts[hw_acts->encap_decap_pos].action = dv_resource_ptr->action;
+	else
+		mpctx->segments[0].reformat_action[ix] = dv_resource_ptr->action;
 	/* Bulk size is 1, so index is 1. */
 	dev_flow->res_idx = 1;
 	return 0;
@@ -12570,6 +12613,15 @@ flow_hw_translate_flow_actions(struct rte_eth_dev *dev,
 	RTE_SET_USED(action_flags);
 	memset(masks, 0, sizeof(masks));
 	memset(mask_conf, 0, sizeof(mask_conf));
+	/*
+	 * Notice All direct actions will be unmasked,
+	 * except for modify header and encap,
+	 * and therefore will be parsed as part of action construct.
+	 * Modify header is always shared in HWS,
+	 * encap is masked such that it will be treated as shared.
+	 * shared actions will be parsed as part of template translation
+	 * and not during action construct.
+	 */
 	flow_nta_build_template_mask(actions, masks, mask_conf);
 	/* The group in the attribute translation was done in advance. */
 	ret = __translate_group(dev, attr, external, attr->group, &src_group, error);
@@ -12587,7 +12639,6 @@ flow_hw_translate_flow_actions(struct rte_eth_dev *dev,
 	if (!table)
 		return rte_flow_error_set(error, ENOMEM, RTE_FLOW_ERROR_TYPE_ACTION,
 				   actions, "Failed to allocate dummy table");
-	/* Notice All actions will be unmasked. */
 	at = __flow_hw_actions_template_create(dev, &template_attr, actions, masks, true, error);
 	if (!at) {
 		ret = -rte_errno;
-- 
2.25.1


From 5c134e8f32d33b2e347b822e7bf7c3e5fee7ec57 Mon Sep 17 00:00:00 2001
From: Maayan Kashani <mkashani@nvidia.com>
Date: Thu, 14 Mar 2024 08:51:27 +0200
Subject: [PATCH 4/4] net/mlx5: clean up TODO comments

review and cleanup unneeded TODO comments.

Signed-off-by: Maayan Kashani <mkashani@nvidia.com>
---
 drivers/net/mlx5/mlx5_flow_hw.c | 32 +++++++++-----------------------
 1 file changed, 9 insertions(+), 23 deletions(-)

diff --git a/drivers/net/mlx5/mlx5_flow_hw.c b/drivers/net/mlx5/mlx5_flow_hw.c
index 61b6a71bbf..d9e43c25c3 100644
--- a/drivers/net/mlx5/mlx5_flow_hw.c
+++ b/drivers/net/mlx5/mlx5_flow_hw.c
@@ -12390,7 +12390,6 @@ static int flow_hw_prepare(struct rte_eth_dev *dev,
 		return rte_flow_error_set(error, ENOMEM,
 				RTE_FLOW_ERROR_TYPE_UNSPECIFIED, NULL,
 				"cannot allocate flow aux memory");
-	/*TODO: consider if other allocation is needed for actions translate. */
 	return 0;
 }
 
@@ -12633,9 +12632,8 @@ flow_hw_translate_flow_actions(struct rte_eth_dev *dev,
 		table_type = MLX5DR_TABLE_TYPE_NIC_TX;
 	else
 		table_type = MLX5DR_TABLE_TYPE_NIC_RX;
-	/* TODO: consider add flag if using only non template mode to reduce table struct size. */
+	/* TODO: consider to reuse the workspace per thread. */
 	table = mlx5_malloc(MLX5_MEM_ZERO, sizeof(*table), 0, SOCKET_ID_ANY);
-	/* TODO: consider sending only relevant fields to construct. */
 	if (!table)
 		return rte_flow_error_set(error, ENOMEM, RTE_FLOW_ERROR_TYPE_ACTION,
 				   actions, "Failed to allocate dummy table");
@@ -12833,9 +12831,7 @@ flow_hw_allocate_actions(struct rte_eth_dev *dev,
 				  NULL, "fail to allocate actions");
 }
 
-/* TODO: remove dev if not used */
-static int flow_hw_apply(struct rte_eth_dev *dev __rte_unused,
-			 const struct rte_flow_item items[],
+static int flow_hw_apply(const struct rte_flow_item items[],
 			 struct mlx5dr_rule_action rule_actions[],
 			 struct rte_flow_hw *flow,
 			 struct rte_flow_error *error)
@@ -12901,16 +12897,7 @@ flow_hw_create_flow(struct rte_eth_dev *dev, enum mlx5_flow_type type,
 		.group = attr->group,
 		.priority = attr->priority,
 		.rss_level = 0,
-		/*
-		 * TODO: currently only mlx5_flow_lacp_miss rule is relevant:
-		 * action type=(enum rte_flow_action_type) MLX5_RTE_FLOW_ACTION_TYPE_DEFAULT_MISS.
-		 * I don't want to waist time going over all actions for this corner case.
-		 * Needs to use another preparation code to update this action flags.
-		 * if (action_type == (enum rte_flow_action_type)
-		 * MLX5_RTE_FLOW_ACTION_TYPE_DEFAULT_MISS)
-		 *     act_flags |= MLX5_FLOW_ACTION_DEFAULT_MISS;
-		 */
-		.act_flags = 0, /*TODO update*/
+		.act_flags = action_flags,
 		.tbl_type = 0,
 		};
 
@@ -12958,11 +12945,6 @@ flow_hw_create_flow(struct rte_eth_dev *dev, enum mlx5_flow_type type,
 	if (ret)
 		goto error;
 
-	/*
-	 * TODO: check regarding release: CT index is not saved per rule,
-	 * the index is in the conf of given action.
-	 */
-
 	/*
 	 * If the flow is external (from application) OR device is started,
 	 * OR mreg discover, then apply immediately.
@@ -12970,7 +12952,7 @@ flow_hw_create_flow(struct rte_eth_dev *dev, enum mlx5_flow_type type,
 	if (external || dev->data->dev_started ||
 	    (attr->group == MLX5_FLOW_MREG_CP_TABLE_GROUP &&
 	     attr->priority == MLX5_FLOW_LOWEST_PRIO_INDICATOR)) {
-		ret = flow_hw_apply(dev, items, hw_act.rule_acts, *flow, error);
+		ret = flow_hw_apply(items, hw_act.rule_acts, *flow, error);
 		if (ret)
 			goto error;
 	}
@@ -13012,7 +12994,7 @@ flow_hw_destroy(struct rte_eth_dev *dev, struct rte_flow_hw *flow)
 			DRV_LOG(ERR, "bwc rule destroy failed");
 	}
 	flow->operation_type = MLX5_FLOW_HW_FLOW_OP_TYPE_DESTROY;
-	/* TODO: notice this function does not handle shared/static actions. */
+	/* Notice this function does not handle shared/static actions. */
 	hw_cmpl_flow_update_or_destroy(dev, flow, 0, NULL);
 
 	/**
@@ -13108,6 +13090,10 @@ static uintptr_t flow_hw_list_create(struct rte_eth_dev *dev,
 	uint64_t item_flags = flow_hw_matching_item_flags_get(items);
 	uint64_t action_flags = flow_hw_action_flags_get(actions, error);
 
+	/*
+	 * TODO: add a call to flow_hw_validate function once it exist.
+	 * and update mlx5_flow_hw_drv_ops accordingly.
+	 */
 
 	if (action_flags & MLX5_FLOW_ACTION_RSS) {
 		const struct rte_flow_action_rss
-- 
2.25.1


^ permalink raw reply	[flat|nested] 15+ messages in thread

* [PATCH v2 22/34] net/mlx5: reorganize main structures
  2024-06-02 10:29 [PATCH 1/4] net/mlx5: reorganize main structures Maayan Kashani
@ 2024-06-03  8:16 ` Maayan Kashani
  2024-06-03  8:16   ` [PATCH v2 23/34] net/mlx5: set modify header as shared action Maayan Kashani
                     ` (2 more replies)
  2024-06-03 10:54 ` [PATCH v3 1/4] net/mlx5: reorganize main structures Maayan Kashani
  1 sibling, 3 replies; 15+ messages in thread
From: Maayan Kashani @ 2024-06-03  8:16 UTC (permalink / raw)
  To: dev
  Cc: mkashani, dsosnowski, rasland, Viacheslav Ovsiienko, Ori Kam,
	Suanming Mou, Matan Azrad

Reorganize rte_flow_hw and rte_flow_nt2hws
structures for better performance, and removed packed.

Signed-off-by: Maayan Kashani <mkashani@nvidia.com>
---
 drivers/net/mlx5/mlx5_flow.h | 26 ++++++++++++--------------
 1 file changed, 12 insertions(+), 14 deletions(-)

diff --git a/drivers/net/mlx5/mlx5_flow.h b/drivers/net/mlx5/mlx5_flow.h
index 7e0f005741..5a3f047968 100644
--- a/drivers/net/mlx5/mlx5_flow.h
+++ b/drivers/net/mlx5/mlx5_flow.h
@@ -1328,12 +1328,12 @@ struct rte_flow_nt2hws {
 	struct rte_flow_hw_aux *flow_aux;
 	/** Modify header pointer. */
 	struct mlx5_flow_dv_modify_hdr_resource *modify_hdr;
+	/** Chain NTA flows. */
+	SLIST_ENTRY(rte_flow_hw) next;
 	/** Encap/decap index. */
 	uint32_t rix_encap_decap;
 	uint8_t chaned_flow;
-	/** Chain NTA flows. */
-	SLIST_ENTRY(rte_flow_hw) next;
-} __rte_packed;
+};
 
 /** HWS flow struct. */
 struct rte_flow_hw {
@@ -1345,6 +1345,12 @@ struct rte_flow_hw {
 	};
 	/** Application's private data passed to enqueued flow operation. */
 	void *user_data;
+	union {
+		/** Jump action. */
+		struct mlx5_hw_jump_action *jump;
+		/** TIR action. */
+		struct mlx5_hrxq *hrxq;
+	};
 	/** Flow index from indexed pool. */
 	uint32_t idx;
 	/** Resource index from indexed pool. */
@@ -1353,20 +1359,12 @@ struct rte_flow_hw {
 	uint32_t rule_idx;
 	/** Which flow fields (inline or in auxiliary struct) are used. */
 	uint32_t flags;
+	/** COUNT action index. */
+	cnt_id_t cnt_id;
 	/** Ongoing flow operation type. */
 	uint8_t operation_type;
 	/** Index of pattern template this flow is based on. */
 	uint8_t mt_idx;
-
-	/** COUNT action index. */
-	cnt_id_t cnt_id;
-	union {
-		/** Jump action. */
-		struct mlx5_hw_jump_action *jump;
-		/** TIR action. */
-		struct mlx5_hrxq *hrxq;
-	};
-
 	/** Equals true if it is non template rule. */
 	bool nt_rule;
 	/**
@@ -1377,7 +1375,7 @@ struct rte_flow_hw {
 	uint8_t padding[9];
 	/** HWS layer data struct. */
 	uint8_t rule[];
-} __rte_packed;
+};
 
 /** Auxiliary data fields that are updatable. */
 struct rte_flow_hw_aux_fields {
-- 
2.25.1


^ permalink raw reply	[flat|nested] 15+ messages in thread

* [PATCH v2 23/34] net/mlx5: set modify header as shared action
  2024-06-03  8:16 ` [PATCH v2 22/34] " Maayan Kashani
@ 2024-06-03  8:16   ` Maayan Kashani
  2024-06-03  8:16   ` [PATCH v2 24/34] net/mlx5: set encap " Maayan Kashani
  2024-06-03  8:16   ` [PATCH v2 25/34] net/mlx5: clean up TODO comments Maayan Kashani
  2 siblings, 0 replies; 15+ messages in thread
From: Maayan Kashani @ 2024-06-03  8:16 UTC (permalink / raw)
  To: dev
  Cc: mkashani, dsosnowski, rasland, Viacheslav Ovsiienko, Ori Kam,
	Suanming Mou, Matan Azrad

In current implementation, in non template mode,
modify header action is not set as always shared.
Align to HWS implementation, setting modify header action as always shared.
Optimize mask initialization.

Signed-off-by: Maayan Kashani <mkashani@nvidia.com>
---
 drivers/net/mlx5/mlx5_flow.h    |  3 --
 drivers/net/mlx5/mlx5_flow_dv.c | 10 ++--
 drivers/net/mlx5/mlx5_flow_hw.c | 91 +++++++++++++++++++--------------
 3 files changed, 59 insertions(+), 45 deletions(-)

diff --git a/drivers/net/mlx5/mlx5_flow.h b/drivers/net/mlx5/mlx5_flow.h
index 5a3f047968..f5bb01616e 100644
--- a/drivers/net/mlx5/mlx5_flow.h
+++ b/drivers/net/mlx5/mlx5_flow.h
@@ -669,9 +669,6 @@ struct mlx5_flow_dv_modify_hdr_resource {
 	struct mlx5_list_entry entry;
 	void *action; /**< Modify header action object. */
 	uint32_t idx;
-#ifdef HAVE_MLX5_HWS_SUPPORT
-	void *mh_dr_pattern; /**< Modify header DR pattern(HWS only). */
-#endif
 	uint64_t flags; /**< Flags for RDMA API(HWS only). */
 	/* Key area for hash list matching: */
 	uint8_t ft_type; /**< Flow table type, Rx or Tx. */
diff --git a/drivers/net/mlx5/mlx5_flow_dv.c b/drivers/net/mlx5/mlx5_flow_dv.c
index 3611ffa4a1..94af391894 100644
--- a/drivers/net/mlx5/mlx5_flow_dv.c
+++ b/drivers/net/mlx5/mlx5_flow_dv.c
@@ -6219,9 +6219,7 @@ flow_modify_create_cb(void *tool_ctx, void *cb_ctx)
 	uint32_t data_len = ref->actions_num * sizeof(ref->actions[0]);
 	uint32_t key_len = sizeof(*ref) - offsetof(typeof(*ref), ft_type);
 	uint32_t idx;
-	struct mlx5_tbl_multi_pattern_ctx *mpctx;
 
-	typeof(mpctx->mh) *mh_dr_pattern = ref->mh_dr_pattern;
 	if (unlikely(!ipool)) {
 		rte_flow_error_set(ctx->error, ENOMEM,
 				   RTE_FLOW_ERROR_TYPE_UNSPECIFIED,
@@ -6240,9 +6238,13 @@ flow_modify_create_cb(void *tool_ctx, void *cb_ctx)
 			key_len + data_len);
 	if (sh->config.dv_flow_en == 2) {
 #ifdef HAVE_MLX5_HWS_SUPPORT
+		struct mlx5dr_action_mh_pattern pattern = {
+			.sz = data_len,
+			.data = (__be64 *)ref->actions
+		};
 		entry->action = mlx5dr_action_create_modify_header(ctx->data2,
-			mh_dr_pattern->elements_num,
-			mh_dr_pattern->pattern, 0, ref->flags);
+			1,
+			&pattern, 0, ref->flags);
 		if (!entry->action)
 			ret = -1;
 #else
diff --git a/drivers/net/mlx5/mlx5_flow_hw.c b/drivers/net/mlx5/mlx5_flow_hw.c
index 43bcaab592..134a035f41 100644
--- a/drivers/net/mlx5/mlx5_flow_hw.c
+++ b/drivers/net/mlx5/mlx5_flow_hw.c
@@ -1439,13 +1439,11 @@ flow_hw_converted_mhdr_cmds_append(struct mlx5_hw_modify_header_action *mhdr,
 
 static __rte_always_inline void
 flow_hw_modify_field_init(struct mlx5_hw_modify_header_action *mhdr,
-			  struct rte_flow_actions_template *at,
-			  bool nt_mode)
+			  struct rte_flow_actions_template *at)
 {
 	memset(mhdr, 0, sizeof(*mhdr));
 	/* Modify header action without any commands is shared by default. */
-	if (!(nt_mode))
-		mhdr->shared = true;
+	mhdr->shared = true;
 	mhdr->pos = at->mhdr_off;
 }
 
@@ -2212,10 +2210,6 @@ mlx5_tbl_translate_modify_header(struct rte_eth_dev *dev,
 				 struct mlx5_hw_modify_header_action *mhdr,
 				 struct rte_flow_error *error)
 {
-	struct mlx5_priv *priv = dev->data->dev_private;
-	const struct rte_flow_template_table_attr *table_attr = &cfg->attr;
-	const struct rte_flow_attr *attr = &table_attr->flow_attr;
-	enum mlx5dr_table_type tbl_type = get_mlx5dr_table_type(attr);
 	uint16_t mhdr_ix = mhdr->pos;
 	struct mlx5dr_action_mh_pattern pattern = {
 		.sz = sizeof(struct mlx5_modification_cmd) * mhdr->mhdr_cmds_num
@@ -2232,20 +2226,8 @@ mlx5_tbl_translate_modify_header(struct rte_eth_dev *dev,
 					  RTE_FLOW_ERROR_TYPE_UNSPECIFIED,
 					  NULL, "translate modify_header: no memory for modify header context");
 	rte_memcpy(acts->mhdr, mhdr, sizeof(*mhdr));
-	pattern.data = (__be64 *)acts->mhdr->mhdr_cmds;
-	if (mhdr->shared) {
-		uint32_t flags = mlx5_hw_act_flag[!!attr->group][tbl_type] |
-				 MLX5DR_ACTION_FLAG_SHARED;
-
-		acts->mhdr->action = mlx5dr_action_create_modify_header
-						(priv->dr_ctx, 1, &pattern, 0,
-						 flags);
-		if (!acts->mhdr->action)
-			return rte_flow_error_set(error, rte_errno,
-						  RTE_FLOW_ERROR_TYPE_UNSPECIFIED,
-						  NULL, "translate modify_header: failed to create DR action");
-		acts->rule_acts[mhdr_ix].action = acts->mhdr->action;
-	} else {
+	if (!mhdr->shared) {
+		pattern.data = (__be64 *)acts->mhdr->mhdr_cmds;
 		typeof(mp_ctx->mh) *mh = &mp_ctx->mh;
 		uint32_t idx = mh->elements_num;
 		mh->pattern[mh->elements_num++] = pattern;
@@ -2256,6 +2238,32 @@ mlx5_tbl_translate_modify_header(struct rte_eth_dev *dev,
 	return 0;
 }
 
+static int
+mlx5_tbl_ensure_shared_modify_header(struct rte_eth_dev *dev,
+				     const struct mlx5_flow_template_table_cfg *cfg,
+				     struct mlx5_hw_actions *acts,
+				     struct rte_flow_error *error)
+{
+	struct mlx5_priv *priv = dev->data->dev_private;
+	const struct rte_flow_template_table_attr *table_attr = &cfg->attr;
+	const struct rte_flow_attr *attr = &table_attr->flow_attr;
+	enum mlx5dr_table_type tbl_type = get_mlx5dr_table_type(attr);
+	struct mlx5dr_action_mh_pattern pattern = {
+		.sz = sizeof(struct mlx5_modification_cmd) * acts->mhdr->mhdr_cmds_num
+	};
+	uint16_t mhdr_ix = acts->mhdr->pos;
+	uint32_t flags = mlx5_hw_act_flag[!!attr->group][tbl_type] | MLX5DR_ACTION_FLAG_SHARED;
+
+	pattern.data = (__be64 *)acts->mhdr->mhdr_cmds;
+	acts->mhdr->action = mlx5dr_action_create_modify_header(priv->dr_ctx, 1,
+								&pattern, 0, flags);
+	if (!acts->mhdr->action)
+		return rte_flow_error_set(error, rte_errno,
+					  RTE_FLOW_ERROR_TYPE_UNSPECIFIED, NULL,
+					  "translate modify_header: failed to create DR action");
+	acts->rule_acts[mhdr_ix].action = acts->mhdr->action;
+	return 0;
+}
 
 static int
 mlx5_create_ipv6_ext_reformat(struct rte_eth_dev *dev,
@@ -2393,7 +2401,7 @@ __flow_hw_translate_actions_template(struct rte_eth_dev *dev,
 	uint32_t target_grp = 0;
 	int table_type;
 
-	flow_hw_modify_field_init(&mhdr, at, nt_mode);
+	flow_hw_modify_field_init(&mhdr, at);
 	if (attr->transfer)
 		type = MLX5DR_TABLE_TYPE_FDB;
 	else if (attr->egress)
@@ -2780,10 +2788,14 @@ __flow_hw_translate_actions_template(struct rte_eth_dev *dev,
 		}
 	}
 	if (mhdr.pos != UINT16_MAX) {
-		ret = mlx5_tbl_translate_modify_header(dev, cfg, acts, mp_ctx,
-						       &mhdr, error);
+		ret = mlx5_tbl_translate_modify_header(dev, cfg, acts, mp_ctx, &mhdr, error);
 		if (ret)
 			goto err;
+		if (!nt_mode && mhdr.shared) {
+			ret = mlx5_tbl_ensure_shared_modify_header(dev, cfg, acts, error);
+			if (ret)
+				goto err;
+		}
 	}
 	if (reformat_used) {
 		ret = mlx5_tbl_translate_reformat(priv, table_attr, acts, at,
@@ -12348,8 +12360,8 @@ static int flow_hw_prepare(struct rte_eth_dev *dev,
 	/*TODO: consider if other allocation is needed for actions translate. */
 	return 0;
 }
-#define FLOW_HW_SET_DV_FIELDS(flow_attr, root, flags)						\
-{												\
+
+#define FLOW_HW_SET_DV_FIELDS(flow_attr, root, flags, dv_resource) {				\
 	typeof(flow_attr) _flow_attr = (flow_attr);						\
 	if (_flow_attr->transfer)								\
 		dv_resource.ft_type = MLX5DV_FLOW_TABLE_TYPE_FDB;				\
@@ -12370,28 +12382,31 @@ flow_hw_modify_hdr_resource_register
 {
 	struct rte_flow_attr *attr = &table->cfg.attr.flow_attr;
 	struct mlx5_flow_dv_modify_hdr_resource *dv_resource_ptr = NULL;
-	struct mlx5_flow_dv_modify_hdr_resource dv_resource;
-	struct mlx5_tbl_multi_pattern_ctx *mpctx = &table->mpctx;
+	union {
+		struct mlx5_flow_dv_modify_hdr_resource dv_resource;
+		uint8_t data[sizeof(struct mlx5_flow_dv_modify_hdr_resource) +
+			     sizeof(struct mlx5_modification_cmd) * MLX5_MHDR_MAX_CMD];
+	} dummy;
 	int ret;
 
 	if (hw_acts->mhdr) {
-		dv_resource.actions_num = hw_acts->mhdr->mhdr_cmds_num;
-		memcpy(dv_resource.actions, hw_acts->mhdr->mhdr_cmds,
-			sizeof(struct mlx5_modification_cmd) * dv_resource.actions_num);
+		dummy.dv_resource.actions_num = hw_acts->mhdr->mhdr_cmds_num;
+		memcpy(dummy.dv_resource.actions, hw_acts->mhdr->mhdr_cmds,
+			sizeof(struct mlx5_modification_cmd) * dummy.dv_resource.actions_num);
 	} else {
 		return 0;
 	}
-	FLOW_HW_SET_DV_FIELDS(attr, dv_resource.root, dv_resource.flags);
-	/* Save a pointer to the pattern needed for DR layer created on actions translate. */
-	dv_resource.mh_dr_pattern = &table->mpctx.mh;
-	ret = __flow_modify_hdr_resource_register(dev, &dv_resource,
+	FLOW_HW_SET_DV_FIELDS(attr, dummy.dv_resource.root, dummy.dv_resource.flags,
+			      dummy.dv_resource);
+	dummy.dv_resource.flags |= MLX5DR_ACTION_FLAG_SHARED;
+	ret = __flow_modify_hdr_resource_register(dev, &dummy.dv_resource,
 		&dv_resource_ptr, error);
 	if (ret)
 		return ret;
 	MLX5_ASSERT(dv_resource_ptr);
 	dev_flow->nt2hws->modify_hdr = dv_resource_ptr;
 	/* keep action for the rule construction. */
-	mpctx->segments[0].mhdr_action = dv_resource_ptr->action;
+	hw_acts->rule_acts[hw_acts->mhdr->pos].action = dv_resource_ptr->action;
 	/* Bulk size is 1, so index is 1. */
 	dev_flow->res_idx = 1;
 	return 0;
@@ -12426,7 +12441,7 @@ flow_hw_encap_decap_resource_register
 		return rte_flow_error_set(error, EINVAL, RTE_FLOW_ERROR_TYPE_ACTION,
 				   NULL, "No reformat action exist in the table.");
 	dv_resource.size = reformat->reformat_hdr->sz;
-	FLOW_HW_SET_DV_FIELDS(attr, is_root, dv_resource.flags);
+	FLOW_HW_SET_DV_FIELDS(attr, is_root, dv_resource.flags, dv_resource);
 	MLX5_ASSERT(dv_resource.size <= MLX5_ENCAP_MAX_LEN);
 	memcpy(dv_resource.buf, reformat->reformat_hdr->data, dv_resource.size);
 	ret = __flow_encap_decap_resource_register(dev, &dv_resource, is_root,
-- 
2.25.1


^ permalink raw reply	[flat|nested] 15+ messages in thread

* [PATCH v2 24/34] net/mlx5: set encap as shared action
  2024-06-03  8:16 ` [PATCH v2 22/34] " Maayan Kashani
  2024-06-03  8:16   ` [PATCH v2 23/34] net/mlx5: set modify header as shared action Maayan Kashani
@ 2024-06-03  8:16   ` Maayan Kashani
  2024-06-03  8:16   ` [PATCH v2 25/34] net/mlx5: clean up TODO comments Maayan Kashani
  2 siblings, 0 replies; 15+ messages in thread
From: Maayan Kashani @ 2024-06-03  8:16 UTC (permalink / raw)
  To: dev
  Cc: mkashani, dsosnowski, rasland, Viacheslav Ovsiienko, Ori Kam,
	Suanming Mou, Matan Azrad

In current implementation, in non template mode,
encap action is not set as non shared(according to given masks).
By masking the relevant fields, encap is now used as shared.
decap remained unshared and cannot be shared according to
current implementation.
Optimize encap action mask initialization and fixed requested
number of reformat actions to one.

Signed-off-by: Maayan Kashani <mkashani@nvidia.com>
---
 drivers/net/mlx5/mlx5_flow_dv.c |   2 +-
 drivers/net/mlx5/mlx5_flow_hw.c | 119 +++++++++++++++++++++++---------
 2 files changed, 86 insertions(+), 35 deletions(-)

diff --git a/drivers/net/mlx5/mlx5_flow_dv.c b/drivers/net/mlx5/mlx5_flow_dv.c
index 94af391894..1484531418 100644
--- a/drivers/net/mlx5/mlx5_flow_dv.c
+++ b/drivers/net/mlx5/mlx5_flow_dv.c
@@ -4317,7 +4317,7 @@ flow_encap_decap_create_cb(void *tool_ctx, void *cb_ctx)
 		hdr.data = ctx_resource->buf;
 		resource->action = mlx5dr_action_create_reformat
 		(ctx->data2, (enum mlx5dr_action_type)ctx_resource->reformat_type, 1,
-			&hdr, 1, ctx_resource->flags);
+			&hdr, 0, ctx_resource->flags);
 		if (!resource->action)
 			ret = -1;
 #else
diff --git a/drivers/net/mlx5/mlx5_flow_hw.c b/drivers/net/mlx5/mlx5_flow_hw.c
index 134a035f41..61b6a71bbf 100644
--- a/drivers/net/mlx5/mlx5_flow_hw.c
+++ b/drivers/net/mlx5/mlx5_flow_hw.c
@@ -2124,9 +2124,17 @@ table_template_translate_indirect_list(struct rte_eth_dev *dev,
 	return ret;
 }
 
+static void
+mlx5_set_reformat_header(struct mlx5dr_action_reformat_header *hdr,
+			 uint8_t *encap_data,
+			 size_t data_size)
+{
+	hdr->sz = data_size;
+	hdr->data = encap_data;
+}
+
 static int
 mlx5_tbl_translate_reformat(struct mlx5_priv *priv,
-			    const struct rte_flow_template_table_attr *table_attr,
 			    struct mlx5_hw_actions *acts,
 			    struct rte_flow_actions_template *at,
 			    const struct rte_flow_item *enc_item,
@@ -2138,8 +2146,6 @@ mlx5_tbl_translate_reformat(struct mlx5_priv *priv,
 			    struct rte_flow_error *error)
 {
 	int mp_reformat_ix = mlx5_multi_pattern_reformat_to_index(refmt_type);
-	const struct rte_flow_attr *attr = &table_attr->flow_attr;
-	enum mlx5dr_table_type tbl_type = get_mlx5dr_table_type(attr);
 	struct mlx5dr_action_reformat_header hdr;
 	uint8_t buf[MLX5_ENCAP_MAX_LEN];
 	bool shared_rfmt = false;
@@ -2164,19 +2170,16 @@ mlx5_tbl_translate_reformat(struct mlx5_priv *priv,
 		return rte_flow_error_set(error, ENOMEM,
 					  RTE_FLOW_ERROR_TYPE_UNSPECIFIED,
 					  NULL, "no memory for reformat context");
-	hdr.sz = data_size;
-	hdr.data = encap_data;
+	acts->encap_decap_pos = at->reformat_off;
+	acts->encap_decap->data_size = data_size;
+	acts->encap_decap->action_type = refmt_type;
 	if (shared_rfmt || mp_reformat_ix < 0) {
 		uint16_t reformat_ix = at->reformat_off;
-		uint32_t flags = mlx5_hw_act_flag[!!attr->group][tbl_type] |
-				 MLX5DR_ACTION_FLAG_SHARED;
-
-		acts->encap_decap->action =
-			mlx5dr_action_create_reformat(priv->dr_ctx, refmt_type,
-						      1, &hdr, 0, flags);
-		if (!acts->encap_decap->action)
-			return -rte_errno;
-		acts->rule_acts[reformat_ix].action = acts->encap_decap->action;
+		/*
+		 * This copy is only needed in non template mode.
+		 * In order to create the action later.
+		 */
+		memcpy(acts->encap_decap->data, encap_data, data_size);
 		acts->rule_acts[reformat_ix].reformat.data = acts->encap_decap->data;
 		acts->rule_acts[reformat_ix].reformat.offset = 0;
 		acts->encap_decap->shared = true;
@@ -2184,14 +2187,11 @@ mlx5_tbl_translate_reformat(struct mlx5_priv *priv,
 		uint32_t ix;
 		typeof(mp_ctx->reformat[0]) *reformat = mp_ctx->reformat +
 							mp_reformat_ix;
-
+		mlx5_set_reformat_header(&hdr, encap_data, data_size);
 		ix = reformat->elements_num++;
 		reformat->reformat_hdr[ix] = hdr;
 		acts->rule_acts[at->reformat_off].reformat.hdr_idx = ix;
-		acts->encap_decap_pos = at->reformat_off;
 		acts->encap_decap->multi_pattern = 1;
-		acts->encap_decap->data_size = data_size;
-		acts->encap_decap->action_type = refmt_type;
 		ret = __flow_hw_act_data_encap_append
 			(priv, acts, (at->actions + reformat_src)->type,
 			 reformat_src, at->reformat_off, data_size);
@@ -2202,6 +2202,32 @@ mlx5_tbl_translate_reformat(struct mlx5_priv *priv,
 	return 0;
 }
 
+static int
+mlx5_tbl_create_reformat_action(struct mlx5_priv *priv,
+				const struct rte_flow_template_table_attr *table_attr,
+				struct mlx5_hw_actions *acts,
+				struct rte_flow_actions_template *at,
+				uint8_t *encap_data,
+				size_t data_size,
+				enum mlx5dr_action_type refmt_type)
+{
+	const struct rte_flow_attr *attr = &table_attr->flow_attr;
+	enum mlx5dr_table_type tbl_type = get_mlx5dr_table_type(attr);
+	struct mlx5dr_action_reformat_header hdr;
+
+	mlx5_set_reformat_header(&hdr, encap_data, data_size);
+	uint16_t reformat_ix = at->reformat_off;
+	uint32_t flags = mlx5_hw_act_flag[!!attr->group][tbl_type] |
+				MLX5DR_ACTION_FLAG_SHARED;
+
+	acts->encap_decap->action = mlx5dr_action_create_reformat(priv->dr_ctx, refmt_type,
+							   1, &hdr, 0, flags);
+	if (!acts->encap_decap->action)
+		return -rte_errno;
+	acts->rule_acts[reformat_ix].action = acts->encap_decap->action;
+	return 0;
+}
+
 static int
 mlx5_tbl_translate_modify_header(struct rte_eth_dev *dev,
 				 const struct mlx5_flow_template_table_cfg *cfg,
@@ -2798,7 +2824,7 @@ __flow_hw_translate_actions_template(struct rte_eth_dev *dev,
 		}
 	}
 	if (reformat_used) {
-		ret = mlx5_tbl_translate_reformat(priv, table_attr, acts, at,
+		ret = mlx5_tbl_translate_reformat(priv, acts, at,
 						  enc_item, enc_item_m,
 						  encap_data, encap_data_m,
 						  mp_ctx, data_size,
@@ -2806,6 +2832,13 @@ __flow_hw_translate_actions_template(struct rte_eth_dev *dev,
 						  refmt_type, error);
 		if (ret)
 			goto err;
+		if (!nt_mode && acts->encap_decap->shared) {
+			ret = mlx5_tbl_create_reformat_action(priv, table_attr, acts, at,
+							      encap_data, data_size,
+							      refmt_type);
+			if (ret)
+				goto err;
+		}
 	}
 	if (recom_used) {
 		MLX5_ASSERT(at->recom_off != UINT16_MAX);
@@ -12361,7 +12394,7 @@ static int flow_hw_prepare(struct rte_eth_dev *dev,
 	return 0;
 }
 
-#define FLOW_HW_SET_DV_FIELDS(flow_attr, root, flags, dv_resource) {				\
+#define FLOW_HW_SET_DV_FIELDS(flow_attr, root, dv_resource) {					\
 	typeof(flow_attr) _flow_attr = (flow_attr);						\
 	if (_flow_attr->transfer)								\
 		dv_resource.ft_type = MLX5DV_FLOW_TABLE_TYPE_FDB;				\
@@ -12369,7 +12402,8 @@ static int flow_hw_prepare(struct rte_eth_dev *dev,
 		dv_resource.ft_type = _flow_attr->egress ? MLX5DV_FLOW_TABLE_TYPE_NIC_TX :	\
 					     MLX5DV_FLOW_TABLE_TYPE_NIC_RX;			\
 	root = _flow_attr->group ? 0 : 1;							\
-	flags = mlx5_hw_act_flag[!!_flow_attr->group][get_mlx5dr_table_type(_flow_attr)];	\
+	dv_resource.flags =									\
+		mlx5_hw_act_flag[!!_flow_attr->group][get_mlx5dr_table_type(_flow_attr)];	\
 }
 
 static int
@@ -12396,8 +12430,7 @@ flow_hw_modify_hdr_resource_register
 	} else {
 		return 0;
 	}
-	FLOW_HW_SET_DV_FIELDS(attr, dummy.dv_resource.root, dummy.dv_resource.flags,
-			      dummy.dv_resource);
+	FLOW_HW_SET_DV_FIELDS(attr, dummy.dv_resource.root, dummy.dv_resource);
 	dummy.dv_resource.flags |= MLX5DR_ACTION_FLAG_SHARED;
 	ret = __flow_modify_hdr_resource_register(dev, &dummy.dv_resource,
 		&dv_resource_ptr, error);
@@ -12432,18 +12465,25 @@ flow_hw_encap_decap_resource_register
 		dv_resource.reformat_type = hw_acts->encap_decap->action_type;
 	else
 		return 0;
+	FLOW_HW_SET_DV_FIELDS(attr, is_root, dv_resource);
 	ix = mlx5_bwc_multi_pattern_reformat_to_index((enum mlx5dr_action_type)
-		dv_resource.reformat_type);
+			dv_resource.reformat_type);
 	if (ix < 0)
 		return ix;
-	typeof(mpctx->reformat[0]) *reformat = mpctx->reformat + ix;
-	if (!reformat->elements_num)
-		return rte_flow_error_set(error, EINVAL, RTE_FLOW_ERROR_TYPE_ACTION,
-				   NULL, "No reformat action exist in the table.");
-	dv_resource.size = reformat->reformat_hdr->sz;
-	FLOW_HW_SET_DV_FIELDS(attr, is_root, dv_resource.flags, dv_resource);
-	MLX5_ASSERT(dv_resource.size <= MLX5_ENCAP_MAX_LEN);
-	memcpy(dv_resource.buf, reformat->reformat_hdr->data, dv_resource.size);
+	if (hw_acts->encap_decap->shared) {
+		dv_resource.size = hw_acts->encap_decap->data_size;
+		MLX5_ASSERT(dv_resource.size <= MLX5_ENCAP_MAX_LEN);
+		memcpy(&dv_resource.buf, hw_acts->encap_decap->data, dv_resource.size);
+		dv_resource.flags |= MLX5DR_ACTION_FLAG_SHARED;
+	} else {
+		typeof(mpctx->reformat[0]) *reformat = mpctx->reformat + ix;
+		if (!reformat->elements_num)
+			return rte_flow_error_set(error, EINVAL, RTE_FLOW_ERROR_TYPE_ACTION,
+					NULL, "No reformat action exist in the table.");
+		dv_resource.size = reformat->reformat_hdr->sz;
+		MLX5_ASSERT(dv_resource.size <= MLX5_ENCAP_MAX_LEN);
+		memcpy(&dv_resource.buf, reformat->reformat_hdr->data, dv_resource.size);
+	}
 	ret = __flow_encap_decap_resource_register(dev, &dv_resource, is_root,
 		&dv_resource_ptr, error);
 	if (ret)
@@ -12451,7 +12491,10 @@ flow_hw_encap_decap_resource_register
 	MLX5_ASSERT(dv_resource_ptr);
 	dev_flow->nt2hws->rix_encap_decap = dv_resource_ptr->idx;
 	/* keep action for the rule construction. */
-	mpctx->segments[0].reformat_action[ix] = dv_resource_ptr->action;
+	if (hw_acts->encap_decap->shared)
+		hw_acts->rule_acts[hw_acts->encap_decap_pos].action = dv_resource_ptr->action;
+	else
+		mpctx->segments[0].reformat_action[ix] = dv_resource_ptr->action;
 	/* Bulk size is 1, so index is 1. */
 	dev_flow->res_idx = 1;
 	return 0;
@@ -12570,6 +12613,15 @@ flow_hw_translate_flow_actions(struct rte_eth_dev *dev,
 	RTE_SET_USED(action_flags);
 	memset(masks, 0, sizeof(masks));
 	memset(mask_conf, 0, sizeof(mask_conf));
+	/*
+	 * Notice All direct actions will be unmasked,
+	 * except for modify header and encap,
+	 * and therefore will be parsed as part of action construct.
+	 * Modify header is always shared in HWS,
+	 * encap is masked such that it will be treated as shared.
+	 * shared actions will be parsed as part of template translation
+	 * and not during action construct.
+	 */
 	flow_nta_build_template_mask(actions, masks, mask_conf);
 	/* The group in the attribute translation was done in advance. */
 	ret = __translate_group(dev, attr, external, attr->group, &src_group, error);
@@ -12587,7 +12639,6 @@ flow_hw_translate_flow_actions(struct rte_eth_dev *dev,
 	if (!table)
 		return rte_flow_error_set(error, ENOMEM, RTE_FLOW_ERROR_TYPE_ACTION,
 				   actions, "Failed to allocate dummy table");
-	/* Notice All actions will be unmasked. */
 	at = __flow_hw_actions_template_create(dev, &template_attr, actions, masks, true, error);
 	if (!at) {
 		ret = -rte_errno;
-- 
2.25.1


^ permalink raw reply	[flat|nested] 15+ messages in thread

* [PATCH v2 25/34] net/mlx5: clean up TODO comments
  2024-06-03  8:16 ` [PATCH v2 22/34] " Maayan Kashani
  2024-06-03  8:16   ` [PATCH v2 23/34] net/mlx5: set modify header as shared action Maayan Kashani
  2024-06-03  8:16   ` [PATCH v2 24/34] net/mlx5: set encap " Maayan Kashani
@ 2024-06-03  8:16   ` Maayan Kashani
  2 siblings, 0 replies; 15+ messages in thread
From: Maayan Kashani @ 2024-06-03  8:16 UTC (permalink / raw)
  To: dev
  Cc: mkashani, dsosnowski, rasland, Viacheslav Ovsiienko, Ori Kam,
	Suanming Mou, Matan Azrad

review and cleanup unneeded TODO comments.

Signed-off-by: Maayan Kashani <mkashani@nvidia.com>
---
 drivers/net/mlx5/mlx5_flow_hw.c | 32 +++++++++-----------------------
 1 file changed, 9 insertions(+), 23 deletions(-)

diff --git a/drivers/net/mlx5/mlx5_flow_hw.c b/drivers/net/mlx5/mlx5_flow_hw.c
index 61b6a71bbf..d9e43c25c3 100644
--- a/drivers/net/mlx5/mlx5_flow_hw.c
+++ b/drivers/net/mlx5/mlx5_flow_hw.c
@@ -12390,7 +12390,6 @@ static int flow_hw_prepare(struct rte_eth_dev *dev,
 		return rte_flow_error_set(error, ENOMEM,
 				RTE_FLOW_ERROR_TYPE_UNSPECIFIED, NULL,
 				"cannot allocate flow aux memory");
-	/*TODO: consider if other allocation is needed for actions translate. */
 	return 0;
 }
 
@@ -12633,9 +12632,8 @@ flow_hw_translate_flow_actions(struct rte_eth_dev *dev,
 		table_type = MLX5DR_TABLE_TYPE_NIC_TX;
 	else
 		table_type = MLX5DR_TABLE_TYPE_NIC_RX;
-	/* TODO: consider add flag if using only non template mode to reduce table struct size. */
+	/* TODO: consider to reuse the workspace per thread. */
 	table = mlx5_malloc(MLX5_MEM_ZERO, sizeof(*table), 0, SOCKET_ID_ANY);
-	/* TODO: consider sending only relevant fields to construct. */
 	if (!table)
 		return rte_flow_error_set(error, ENOMEM, RTE_FLOW_ERROR_TYPE_ACTION,
 				   actions, "Failed to allocate dummy table");
@@ -12833,9 +12831,7 @@ flow_hw_allocate_actions(struct rte_eth_dev *dev,
 				  NULL, "fail to allocate actions");
 }
 
-/* TODO: remove dev if not used */
-static int flow_hw_apply(struct rte_eth_dev *dev __rte_unused,
-			 const struct rte_flow_item items[],
+static int flow_hw_apply(const struct rte_flow_item items[],
 			 struct mlx5dr_rule_action rule_actions[],
 			 struct rte_flow_hw *flow,
 			 struct rte_flow_error *error)
@@ -12901,16 +12897,7 @@ flow_hw_create_flow(struct rte_eth_dev *dev, enum mlx5_flow_type type,
 		.group = attr->group,
 		.priority = attr->priority,
 		.rss_level = 0,
-		/*
-		 * TODO: currently only mlx5_flow_lacp_miss rule is relevant:
-		 * action type=(enum rte_flow_action_type) MLX5_RTE_FLOW_ACTION_TYPE_DEFAULT_MISS.
-		 * I don't want to waist time going over all actions for this corner case.
-		 * Needs to use another preparation code to update this action flags.
-		 * if (action_type == (enum rte_flow_action_type)
-		 * MLX5_RTE_FLOW_ACTION_TYPE_DEFAULT_MISS)
-		 *     act_flags |= MLX5_FLOW_ACTION_DEFAULT_MISS;
-		 */
-		.act_flags = 0, /*TODO update*/
+		.act_flags = action_flags,
 		.tbl_type = 0,
 		};
 
@@ -12958,11 +12945,6 @@ flow_hw_create_flow(struct rte_eth_dev *dev, enum mlx5_flow_type type,
 	if (ret)
 		goto error;
 
-	/*
-	 * TODO: check regarding release: CT index is not saved per rule,
-	 * the index is in the conf of given action.
-	 */
-
 	/*
 	 * If the flow is external (from application) OR device is started,
 	 * OR mreg discover, then apply immediately.
@@ -12970,7 +12952,7 @@ flow_hw_create_flow(struct rte_eth_dev *dev, enum mlx5_flow_type type,
 	if (external || dev->data->dev_started ||
 	    (attr->group == MLX5_FLOW_MREG_CP_TABLE_GROUP &&
 	     attr->priority == MLX5_FLOW_LOWEST_PRIO_INDICATOR)) {
-		ret = flow_hw_apply(dev, items, hw_act.rule_acts, *flow, error);
+		ret = flow_hw_apply(items, hw_act.rule_acts, *flow, error);
 		if (ret)
 			goto error;
 	}
@@ -13012,7 +12994,7 @@ flow_hw_destroy(struct rte_eth_dev *dev, struct rte_flow_hw *flow)
 			DRV_LOG(ERR, "bwc rule destroy failed");
 	}
 	flow->operation_type = MLX5_FLOW_HW_FLOW_OP_TYPE_DESTROY;
-	/* TODO: notice this function does not handle shared/static actions. */
+	/* Notice this function does not handle shared/static actions. */
 	hw_cmpl_flow_update_or_destroy(dev, flow, 0, NULL);
 
 	/**
@@ -13108,6 +13090,10 @@ static uintptr_t flow_hw_list_create(struct rte_eth_dev *dev,
 	uint64_t item_flags = flow_hw_matching_item_flags_get(items);
 	uint64_t action_flags = flow_hw_action_flags_get(actions, error);
 
+	/*
+	 * TODO: add a call to flow_hw_validate function once it exist.
+	 * and update mlx5_flow_hw_drv_ops accordingly.
+	 */
 
 	if (action_flags & MLX5_FLOW_ACTION_RSS) {
 		const struct rte_flow_action_rss
-- 
2.25.1


^ permalink raw reply	[flat|nested] 15+ messages in thread

* [PATCH v3 1/4] net/mlx5: reorganize main structures
  2024-06-02 10:29 [PATCH 1/4] net/mlx5: reorganize main structures Maayan Kashani
  2024-06-03  8:16 ` [PATCH v2 22/34] " Maayan Kashani
@ 2024-06-03 10:54 ` Maayan Kashani
  2024-06-03 10:54   ` [PATCH v3 2/4] net/mlx5: set modify header as shared action Maayan Kashani
                     ` (3 more replies)
  1 sibling, 4 replies; 15+ messages in thread
From: Maayan Kashani @ 2024-06-03 10:54 UTC (permalink / raw)
  To: dev
  Cc: mkashani, dsosnowski, rasland, Viacheslav Ovsiienko, Ori Kam,
	Suanming Mou, Matan Azrad

Reorganize rte_flow_hw and rte_flow_nt2hws
structures for better performance, and removed packed.

Signed-off-by: Maayan Kashani <mkashani@nvidia.com>
---
 drivers/net/mlx5/mlx5_flow.h | 26 ++++++++++++--------------
 1 file changed, 12 insertions(+), 14 deletions(-)

diff --git a/drivers/net/mlx5/mlx5_flow.h b/drivers/net/mlx5/mlx5_flow.h
index 7e0f005741..5a3f047968 100644
--- a/drivers/net/mlx5/mlx5_flow.h
+++ b/drivers/net/mlx5/mlx5_flow.h
@@ -1328,12 +1328,12 @@ struct rte_flow_nt2hws {
 	struct rte_flow_hw_aux *flow_aux;
 	/** Modify header pointer. */
 	struct mlx5_flow_dv_modify_hdr_resource *modify_hdr;
+	/** Chain NTA flows. */
+	SLIST_ENTRY(rte_flow_hw) next;
 	/** Encap/decap index. */
 	uint32_t rix_encap_decap;
 	uint8_t chaned_flow;
-	/** Chain NTA flows. */
-	SLIST_ENTRY(rte_flow_hw) next;
-} __rte_packed;
+};
 
 /** HWS flow struct. */
 struct rte_flow_hw {
@@ -1345,6 +1345,12 @@ struct rte_flow_hw {
 	};
 	/** Application's private data passed to enqueued flow operation. */
 	void *user_data;
+	union {
+		/** Jump action. */
+		struct mlx5_hw_jump_action *jump;
+		/** TIR action. */
+		struct mlx5_hrxq *hrxq;
+	};
 	/** Flow index from indexed pool. */
 	uint32_t idx;
 	/** Resource index from indexed pool. */
@@ -1353,20 +1359,12 @@ struct rte_flow_hw {
 	uint32_t rule_idx;
 	/** Which flow fields (inline or in auxiliary struct) are used. */
 	uint32_t flags;
+	/** COUNT action index. */
+	cnt_id_t cnt_id;
 	/** Ongoing flow operation type. */
 	uint8_t operation_type;
 	/** Index of pattern template this flow is based on. */
 	uint8_t mt_idx;
-
-	/** COUNT action index. */
-	cnt_id_t cnt_id;
-	union {
-		/** Jump action. */
-		struct mlx5_hw_jump_action *jump;
-		/** TIR action. */
-		struct mlx5_hrxq *hrxq;
-	};
-
 	/** Equals true if it is non template rule. */
 	bool nt_rule;
 	/**
@@ -1377,7 +1375,7 @@ struct rte_flow_hw {
 	uint8_t padding[9];
 	/** HWS layer data struct. */
 	uint8_t rule[];
-} __rte_packed;
+};
 
 /** Auxiliary data fields that are updatable. */
 struct rte_flow_hw_aux_fields {
-- 
2.25.1


^ permalink raw reply	[flat|nested] 15+ messages in thread

* [PATCH v3 2/4] net/mlx5: set modify header as shared action
  2024-06-03 10:54 ` [PATCH v3 1/4] net/mlx5: reorganize main structures Maayan Kashani
@ 2024-06-03 10:54   ` Maayan Kashani
  2024-06-03 10:54   ` [PATCH v3 3/4] net/mlx5: set encap " Maayan Kashani
                     ` (2 subsequent siblings)
  3 siblings, 0 replies; 15+ messages in thread
From: Maayan Kashani @ 2024-06-03 10:54 UTC (permalink / raw)
  To: dev
  Cc: mkashani, dsosnowski, rasland, Viacheslav Ovsiienko, Ori Kam,
	Suanming Mou, Matan Azrad

In current implementation, in non template mode,
modify header action is not set as always shared.
Align to HWS implementation, setting modify header action as always shared.
Optimize mask initialization.

Signed-off-by: Maayan Kashani <mkashani@nvidia.com>
---
 drivers/net/mlx5/mlx5_flow.h    |  3 --
 drivers/net/mlx5/mlx5_flow_dv.c | 10 ++--
 drivers/net/mlx5/mlx5_flow_hw.c | 91 +++++++++++++++++++--------------
 3 files changed, 59 insertions(+), 45 deletions(-)

diff --git a/drivers/net/mlx5/mlx5_flow.h b/drivers/net/mlx5/mlx5_flow.h
index 5a3f047968..f5bb01616e 100644
--- a/drivers/net/mlx5/mlx5_flow.h
+++ b/drivers/net/mlx5/mlx5_flow.h
@@ -669,9 +669,6 @@ struct mlx5_flow_dv_modify_hdr_resource {
 	struct mlx5_list_entry entry;
 	void *action; /**< Modify header action object. */
 	uint32_t idx;
-#ifdef HAVE_MLX5_HWS_SUPPORT
-	void *mh_dr_pattern; /**< Modify header DR pattern(HWS only). */
-#endif
 	uint64_t flags; /**< Flags for RDMA API(HWS only). */
 	/* Key area for hash list matching: */
 	uint8_t ft_type; /**< Flow table type, Rx or Tx. */
diff --git a/drivers/net/mlx5/mlx5_flow_dv.c b/drivers/net/mlx5/mlx5_flow_dv.c
index 3611ffa4a1..94af391894 100644
--- a/drivers/net/mlx5/mlx5_flow_dv.c
+++ b/drivers/net/mlx5/mlx5_flow_dv.c
@@ -6219,9 +6219,7 @@ flow_modify_create_cb(void *tool_ctx, void *cb_ctx)
 	uint32_t data_len = ref->actions_num * sizeof(ref->actions[0]);
 	uint32_t key_len = sizeof(*ref) - offsetof(typeof(*ref), ft_type);
 	uint32_t idx;
-	struct mlx5_tbl_multi_pattern_ctx *mpctx;
 
-	typeof(mpctx->mh) *mh_dr_pattern = ref->mh_dr_pattern;
 	if (unlikely(!ipool)) {
 		rte_flow_error_set(ctx->error, ENOMEM,
 				   RTE_FLOW_ERROR_TYPE_UNSPECIFIED,
@@ -6240,9 +6238,13 @@ flow_modify_create_cb(void *tool_ctx, void *cb_ctx)
 			key_len + data_len);
 	if (sh->config.dv_flow_en == 2) {
 #ifdef HAVE_MLX5_HWS_SUPPORT
+		struct mlx5dr_action_mh_pattern pattern = {
+			.sz = data_len,
+			.data = (__be64 *)ref->actions
+		};
 		entry->action = mlx5dr_action_create_modify_header(ctx->data2,
-			mh_dr_pattern->elements_num,
-			mh_dr_pattern->pattern, 0, ref->flags);
+			1,
+			&pattern, 0, ref->flags);
 		if (!entry->action)
 			ret = -1;
 #else
diff --git a/drivers/net/mlx5/mlx5_flow_hw.c b/drivers/net/mlx5/mlx5_flow_hw.c
index 43bcaab592..134a035f41 100644
--- a/drivers/net/mlx5/mlx5_flow_hw.c
+++ b/drivers/net/mlx5/mlx5_flow_hw.c
@@ -1439,13 +1439,11 @@ flow_hw_converted_mhdr_cmds_append(struct mlx5_hw_modify_header_action *mhdr,
 
 static __rte_always_inline void
 flow_hw_modify_field_init(struct mlx5_hw_modify_header_action *mhdr,
-			  struct rte_flow_actions_template *at,
-			  bool nt_mode)
+			  struct rte_flow_actions_template *at)
 {
 	memset(mhdr, 0, sizeof(*mhdr));
 	/* Modify header action without any commands is shared by default. */
-	if (!(nt_mode))
-		mhdr->shared = true;
+	mhdr->shared = true;
 	mhdr->pos = at->mhdr_off;
 }
 
@@ -2212,10 +2210,6 @@ mlx5_tbl_translate_modify_header(struct rte_eth_dev *dev,
 				 struct mlx5_hw_modify_header_action *mhdr,
 				 struct rte_flow_error *error)
 {
-	struct mlx5_priv *priv = dev->data->dev_private;
-	const struct rte_flow_template_table_attr *table_attr = &cfg->attr;
-	const struct rte_flow_attr *attr = &table_attr->flow_attr;
-	enum mlx5dr_table_type tbl_type = get_mlx5dr_table_type(attr);
 	uint16_t mhdr_ix = mhdr->pos;
 	struct mlx5dr_action_mh_pattern pattern = {
 		.sz = sizeof(struct mlx5_modification_cmd) * mhdr->mhdr_cmds_num
@@ -2232,20 +2226,8 @@ mlx5_tbl_translate_modify_header(struct rte_eth_dev *dev,
 					  RTE_FLOW_ERROR_TYPE_UNSPECIFIED,
 					  NULL, "translate modify_header: no memory for modify header context");
 	rte_memcpy(acts->mhdr, mhdr, sizeof(*mhdr));
-	pattern.data = (__be64 *)acts->mhdr->mhdr_cmds;
-	if (mhdr->shared) {
-		uint32_t flags = mlx5_hw_act_flag[!!attr->group][tbl_type] |
-				 MLX5DR_ACTION_FLAG_SHARED;
-
-		acts->mhdr->action = mlx5dr_action_create_modify_header
-						(priv->dr_ctx, 1, &pattern, 0,
-						 flags);
-		if (!acts->mhdr->action)
-			return rte_flow_error_set(error, rte_errno,
-						  RTE_FLOW_ERROR_TYPE_UNSPECIFIED,
-						  NULL, "translate modify_header: failed to create DR action");
-		acts->rule_acts[mhdr_ix].action = acts->mhdr->action;
-	} else {
+	if (!mhdr->shared) {
+		pattern.data = (__be64 *)acts->mhdr->mhdr_cmds;
 		typeof(mp_ctx->mh) *mh = &mp_ctx->mh;
 		uint32_t idx = mh->elements_num;
 		mh->pattern[mh->elements_num++] = pattern;
@@ -2256,6 +2238,32 @@ mlx5_tbl_translate_modify_header(struct rte_eth_dev *dev,
 	return 0;
 }
 
+static int
+mlx5_tbl_ensure_shared_modify_header(struct rte_eth_dev *dev,
+				     const struct mlx5_flow_template_table_cfg *cfg,
+				     struct mlx5_hw_actions *acts,
+				     struct rte_flow_error *error)
+{
+	struct mlx5_priv *priv = dev->data->dev_private;
+	const struct rte_flow_template_table_attr *table_attr = &cfg->attr;
+	const struct rte_flow_attr *attr = &table_attr->flow_attr;
+	enum mlx5dr_table_type tbl_type = get_mlx5dr_table_type(attr);
+	struct mlx5dr_action_mh_pattern pattern = {
+		.sz = sizeof(struct mlx5_modification_cmd) * acts->mhdr->mhdr_cmds_num
+	};
+	uint16_t mhdr_ix = acts->mhdr->pos;
+	uint32_t flags = mlx5_hw_act_flag[!!attr->group][tbl_type] | MLX5DR_ACTION_FLAG_SHARED;
+
+	pattern.data = (__be64 *)acts->mhdr->mhdr_cmds;
+	acts->mhdr->action = mlx5dr_action_create_modify_header(priv->dr_ctx, 1,
+								&pattern, 0, flags);
+	if (!acts->mhdr->action)
+		return rte_flow_error_set(error, rte_errno,
+					  RTE_FLOW_ERROR_TYPE_UNSPECIFIED, NULL,
+					  "translate modify_header: failed to create DR action");
+	acts->rule_acts[mhdr_ix].action = acts->mhdr->action;
+	return 0;
+}
 
 static int
 mlx5_create_ipv6_ext_reformat(struct rte_eth_dev *dev,
@@ -2393,7 +2401,7 @@ __flow_hw_translate_actions_template(struct rte_eth_dev *dev,
 	uint32_t target_grp = 0;
 	int table_type;
 
-	flow_hw_modify_field_init(&mhdr, at, nt_mode);
+	flow_hw_modify_field_init(&mhdr, at);
 	if (attr->transfer)
 		type = MLX5DR_TABLE_TYPE_FDB;
 	else if (attr->egress)
@@ -2780,10 +2788,14 @@ __flow_hw_translate_actions_template(struct rte_eth_dev *dev,
 		}
 	}
 	if (mhdr.pos != UINT16_MAX) {
-		ret = mlx5_tbl_translate_modify_header(dev, cfg, acts, mp_ctx,
-						       &mhdr, error);
+		ret = mlx5_tbl_translate_modify_header(dev, cfg, acts, mp_ctx, &mhdr, error);
 		if (ret)
 			goto err;
+		if (!nt_mode && mhdr.shared) {
+			ret = mlx5_tbl_ensure_shared_modify_header(dev, cfg, acts, error);
+			if (ret)
+				goto err;
+		}
 	}
 	if (reformat_used) {
 		ret = mlx5_tbl_translate_reformat(priv, table_attr, acts, at,
@@ -12348,8 +12360,8 @@ static int flow_hw_prepare(struct rte_eth_dev *dev,
 	/*TODO: consider if other allocation is needed for actions translate. */
 	return 0;
 }
-#define FLOW_HW_SET_DV_FIELDS(flow_attr, root, flags)						\
-{												\
+
+#define FLOW_HW_SET_DV_FIELDS(flow_attr, root, flags, dv_resource) {				\
 	typeof(flow_attr) _flow_attr = (flow_attr);						\
 	if (_flow_attr->transfer)								\
 		dv_resource.ft_type = MLX5DV_FLOW_TABLE_TYPE_FDB;				\
@@ -12370,28 +12382,31 @@ flow_hw_modify_hdr_resource_register
 {
 	struct rte_flow_attr *attr = &table->cfg.attr.flow_attr;
 	struct mlx5_flow_dv_modify_hdr_resource *dv_resource_ptr = NULL;
-	struct mlx5_flow_dv_modify_hdr_resource dv_resource;
-	struct mlx5_tbl_multi_pattern_ctx *mpctx = &table->mpctx;
+	union {
+		struct mlx5_flow_dv_modify_hdr_resource dv_resource;
+		uint8_t data[sizeof(struct mlx5_flow_dv_modify_hdr_resource) +
+			     sizeof(struct mlx5_modification_cmd) * MLX5_MHDR_MAX_CMD];
+	} dummy;
 	int ret;
 
 	if (hw_acts->mhdr) {
-		dv_resource.actions_num = hw_acts->mhdr->mhdr_cmds_num;
-		memcpy(dv_resource.actions, hw_acts->mhdr->mhdr_cmds,
-			sizeof(struct mlx5_modification_cmd) * dv_resource.actions_num);
+		dummy.dv_resource.actions_num = hw_acts->mhdr->mhdr_cmds_num;
+		memcpy(dummy.dv_resource.actions, hw_acts->mhdr->mhdr_cmds,
+			sizeof(struct mlx5_modification_cmd) * dummy.dv_resource.actions_num);
 	} else {
 		return 0;
 	}
-	FLOW_HW_SET_DV_FIELDS(attr, dv_resource.root, dv_resource.flags);
-	/* Save a pointer to the pattern needed for DR layer created on actions translate. */
-	dv_resource.mh_dr_pattern = &table->mpctx.mh;
-	ret = __flow_modify_hdr_resource_register(dev, &dv_resource,
+	FLOW_HW_SET_DV_FIELDS(attr, dummy.dv_resource.root, dummy.dv_resource.flags,
+			      dummy.dv_resource);
+	dummy.dv_resource.flags |= MLX5DR_ACTION_FLAG_SHARED;
+	ret = __flow_modify_hdr_resource_register(dev, &dummy.dv_resource,
 		&dv_resource_ptr, error);
 	if (ret)
 		return ret;
 	MLX5_ASSERT(dv_resource_ptr);
 	dev_flow->nt2hws->modify_hdr = dv_resource_ptr;
 	/* keep action for the rule construction. */
-	mpctx->segments[0].mhdr_action = dv_resource_ptr->action;
+	hw_acts->rule_acts[hw_acts->mhdr->pos].action = dv_resource_ptr->action;
 	/* Bulk size is 1, so index is 1. */
 	dev_flow->res_idx = 1;
 	return 0;
@@ -12426,7 +12441,7 @@ flow_hw_encap_decap_resource_register
 		return rte_flow_error_set(error, EINVAL, RTE_FLOW_ERROR_TYPE_ACTION,
 				   NULL, "No reformat action exist in the table.");
 	dv_resource.size = reformat->reformat_hdr->sz;
-	FLOW_HW_SET_DV_FIELDS(attr, is_root, dv_resource.flags);
+	FLOW_HW_SET_DV_FIELDS(attr, is_root, dv_resource.flags, dv_resource);
 	MLX5_ASSERT(dv_resource.size <= MLX5_ENCAP_MAX_LEN);
 	memcpy(dv_resource.buf, reformat->reformat_hdr->data, dv_resource.size);
 	ret = __flow_encap_decap_resource_register(dev, &dv_resource, is_root,
-- 
2.25.1


^ permalink raw reply	[flat|nested] 15+ messages in thread

* [PATCH v3 3/4] net/mlx5: set encap as shared action
  2024-06-03 10:54 ` [PATCH v3 1/4] net/mlx5: reorganize main structures Maayan Kashani
  2024-06-03 10:54   ` [PATCH v3 2/4] net/mlx5: set modify header as shared action Maayan Kashani
@ 2024-06-03 10:54   ` Maayan Kashani
  2024-06-03 10:54   ` [PATCH v3 4/4] net/mlx5: clean up TODO comments Maayan Kashani
  2024-06-06 10:06   ` [PATCH v4 0/4] non template pmd code changes Maayan Kashani
  3 siblings, 0 replies; 15+ messages in thread
From: Maayan Kashani @ 2024-06-03 10:54 UTC (permalink / raw)
  To: dev
  Cc: mkashani, dsosnowski, rasland, Viacheslav Ovsiienko, Ori Kam,
	Suanming Mou, Matan Azrad

In current implementation, in non template mode,
encap action is not set as non shared(according to given masks).
By masking the relevant fields, encap is now used as shared.
decap remained unshared and cannot be shared according to
current implementation.
Optimize encap action mask initialization and fixed requested
number of reformat actions to one.

Signed-off-by: Maayan Kashani <mkashani@nvidia.com>
---
 drivers/net/mlx5/mlx5_flow_dv.c |   2 +-
 drivers/net/mlx5/mlx5_flow_hw.c | 119 +++++++++++++++++++++++---------
 2 files changed, 86 insertions(+), 35 deletions(-)

diff --git a/drivers/net/mlx5/mlx5_flow_dv.c b/drivers/net/mlx5/mlx5_flow_dv.c
index 94af391894..1484531418 100644
--- a/drivers/net/mlx5/mlx5_flow_dv.c
+++ b/drivers/net/mlx5/mlx5_flow_dv.c
@@ -4317,7 +4317,7 @@ flow_encap_decap_create_cb(void *tool_ctx, void *cb_ctx)
 		hdr.data = ctx_resource->buf;
 		resource->action = mlx5dr_action_create_reformat
 		(ctx->data2, (enum mlx5dr_action_type)ctx_resource->reformat_type, 1,
-			&hdr, 1, ctx_resource->flags);
+			&hdr, 0, ctx_resource->flags);
 		if (!resource->action)
 			ret = -1;
 #else
diff --git a/drivers/net/mlx5/mlx5_flow_hw.c b/drivers/net/mlx5/mlx5_flow_hw.c
index 134a035f41..61b6a71bbf 100644
--- a/drivers/net/mlx5/mlx5_flow_hw.c
+++ b/drivers/net/mlx5/mlx5_flow_hw.c
@@ -2124,9 +2124,17 @@ table_template_translate_indirect_list(struct rte_eth_dev *dev,
 	return ret;
 }
 
+static void
+mlx5_set_reformat_header(struct mlx5dr_action_reformat_header *hdr,
+			 uint8_t *encap_data,
+			 size_t data_size)
+{
+	hdr->sz = data_size;
+	hdr->data = encap_data;
+}
+
 static int
 mlx5_tbl_translate_reformat(struct mlx5_priv *priv,
-			    const struct rte_flow_template_table_attr *table_attr,
 			    struct mlx5_hw_actions *acts,
 			    struct rte_flow_actions_template *at,
 			    const struct rte_flow_item *enc_item,
@@ -2138,8 +2146,6 @@ mlx5_tbl_translate_reformat(struct mlx5_priv *priv,
 			    struct rte_flow_error *error)
 {
 	int mp_reformat_ix = mlx5_multi_pattern_reformat_to_index(refmt_type);
-	const struct rte_flow_attr *attr = &table_attr->flow_attr;
-	enum mlx5dr_table_type tbl_type = get_mlx5dr_table_type(attr);
 	struct mlx5dr_action_reformat_header hdr;
 	uint8_t buf[MLX5_ENCAP_MAX_LEN];
 	bool shared_rfmt = false;
@@ -2164,19 +2170,16 @@ mlx5_tbl_translate_reformat(struct mlx5_priv *priv,
 		return rte_flow_error_set(error, ENOMEM,
 					  RTE_FLOW_ERROR_TYPE_UNSPECIFIED,
 					  NULL, "no memory for reformat context");
-	hdr.sz = data_size;
-	hdr.data = encap_data;
+	acts->encap_decap_pos = at->reformat_off;
+	acts->encap_decap->data_size = data_size;
+	acts->encap_decap->action_type = refmt_type;
 	if (shared_rfmt || mp_reformat_ix < 0) {
 		uint16_t reformat_ix = at->reformat_off;
-		uint32_t flags = mlx5_hw_act_flag[!!attr->group][tbl_type] |
-				 MLX5DR_ACTION_FLAG_SHARED;
-
-		acts->encap_decap->action =
-			mlx5dr_action_create_reformat(priv->dr_ctx, refmt_type,
-						      1, &hdr, 0, flags);
-		if (!acts->encap_decap->action)
-			return -rte_errno;
-		acts->rule_acts[reformat_ix].action = acts->encap_decap->action;
+		/*
+		 * This copy is only needed in non template mode.
+		 * In order to create the action later.
+		 */
+		memcpy(acts->encap_decap->data, encap_data, data_size);
 		acts->rule_acts[reformat_ix].reformat.data = acts->encap_decap->data;
 		acts->rule_acts[reformat_ix].reformat.offset = 0;
 		acts->encap_decap->shared = true;
@@ -2184,14 +2187,11 @@ mlx5_tbl_translate_reformat(struct mlx5_priv *priv,
 		uint32_t ix;
 		typeof(mp_ctx->reformat[0]) *reformat = mp_ctx->reformat +
 							mp_reformat_ix;
-
+		mlx5_set_reformat_header(&hdr, encap_data, data_size);
 		ix = reformat->elements_num++;
 		reformat->reformat_hdr[ix] = hdr;
 		acts->rule_acts[at->reformat_off].reformat.hdr_idx = ix;
-		acts->encap_decap_pos = at->reformat_off;
 		acts->encap_decap->multi_pattern = 1;
-		acts->encap_decap->data_size = data_size;
-		acts->encap_decap->action_type = refmt_type;
 		ret = __flow_hw_act_data_encap_append
 			(priv, acts, (at->actions + reformat_src)->type,
 			 reformat_src, at->reformat_off, data_size);
@@ -2202,6 +2202,32 @@ mlx5_tbl_translate_reformat(struct mlx5_priv *priv,
 	return 0;
 }
 
+static int
+mlx5_tbl_create_reformat_action(struct mlx5_priv *priv,
+				const struct rte_flow_template_table_attr *table_attr,
+				struct mlx5_hw_actions *acts,
+				struct rte_flow_actions_template *at,
+				uint8_t *encap_data,
+				size_t data_size,
+				enum mlx5dr_action_type refmt_type)
+{
+	const struct rte_flow_attr *attr = &table_attr->flow_attr;
+	enum mlx5dr_table_type tbl_type = get_mlx5dr_table_type(attr);
+	struct mlx5dr_action_reformat_header hdr;
+
+	mlx5_set_reformat_header(&hdr, encap_data, data_size);
+	uint16_t reformat_ix = at->reformat_off;
+	uint32_t flags = mlx5_hw_act_flag[!!attr->group][tbl_type] |
+				MLX5DR_ACTION_FLAG_SHARED;
+
+	acts->encap_decap->action = mlx5dr_action_create_reformat(priv->dr_ctx, refmt_type,
+							   1, &hdr, 0, flags);
+	if (!acts->encap_decap->action)
+		return -rte_errno;
+	acts->rule_acts[reformat_ix].action = acts->encap_decap->action;
+	return 0;
+}
+
 static int
 mlx5_tbl_translate_modify_header(struct rte_eth_dev *dev,
 				 const struct mlx5_flow_template_table_cfg *cfg,
@@ -2798,7 +2824,7 @@ __flow_hw_translate_actions_template(struct rte_eth_dev *dev,
 		}
 	}
 	if (reformat_used) {
-		ret = mlx5_tbl_translate_reformat(priv, table_attr, acts, at,
+		ret = mlx5_tbl_translate_reformat(priv, acts, at,
 						  enc_item, enc_item_m,
 						  encap_data, encap_data_m,
 						  mp_ctx, data_size,
@@ -2806,6 +2832,13 @@ __flow_hw_translate_actions_template(struct rte_eth_dev *dev,
 						  refmt_type, error);
 		if (ret)
 			goto err;
+		if (!nt_mode && acts->encap_decap->shared) {
+			ret = mlx5_tbl_create_reformat_action(priv, table_attr, acts, at,
+							      encap_data, data_size,
+							      refmt_type);
+			if (ret)
+				goto err;
+		}
 	}
 	if (recom_used) {
 		MLX5_ASSERT(at->recom_off != UINT16_MAX);
@@ -12361,7 +12394,7 @@ static int flow_hw_prepare(struct rte_eth_dev *dev,
 	return 0;
 }
 
-#define FLOW_HW_SET_DV_FIELDS(flow_attr, root, flags, dv_resource) {				\
+#define FLOW_HW_SET_DV_FIELDS(flow_attr, root, dv_resource) {					\
 	typeof(flow_attr) _flow_attr = (flow_attr);						\
 	if (_flow_attr->transfer)								\
 		dv_resource.ft_type = MLX5DV_FLOW_TABLE_TYPE_FDB;				\
@@ -12369,7 +12402,8 @@ static int flow_hw_prepare(struct rte_eth_dev *dev,
 		dv_resource.ft_type = _flow_attr->egress ? MLX5DV_FLOW_TABLE_TYPE_NIC_TX :	\
 					     MLX5DV_FLOW_TABLE_TYPE_NIC_RX;			\
 	root = _flow_attr->group ? 0 : 1;							\
-	flags = mlx5_hw_act_flag[!!_flow_attr->group][get_mlx5dr_table_type(_flow_attr)];	\
+	dv_resource.flags =									\
+		mlx5_hw_act_flag[!!_flow_attr->group][get_mlx5dr_table_type(_flow_attr)];	\
 }
 
 static int
@@ -12396,8 +12430,7 @@ flow_hw_modify_hdr_resource_register
 	} else {
 		return 0;
 	}
-	FLOW_HW_SET_DV_FIELDS(attr, dummy.dv_resource.root, dummy.dv_resource.flags,
-			      dummy.dv_resource);
+	FLOW_HW_SET_DV_FIELDS(attr, dummy.dv_resource.root, dummy.dv_resource);
 	dummy.dv_resource.flags |= MLX5DR_ACTION_FLAG_SHARED;
 	ret = __flow_modify_hdr_resource_register(dev, &dummy.dv_resource,
 		&dv_resource_ptr, error);
@@ -12432,18 +12465,25 @@ flow_hw_encap_decap_resource_register
 		dv_resource.reformat_type = hw_acts->encap_decap->action_type;
 	else
 		return 0;
+	FLOW_HW_SET_DV_FIELDS(attr, is_root, dv_resource);
 	ix = mlx5_bwc_multi_pattern_reformat_to_index((enum mlx5dr_action_type)
-		dv_resource.reformat_type);
+			dv_resource.reformat_type);
 	if (ix < 0)
 		return ix;
-	typeof(mpctx->reformat[0]) *reformat = mpctx->reformat + ix;
-	if (!reformat->elements_num)
-		return rte_flow_error_set(error, EINVAL, RTE_FLOW_ERROR_TYPE_ACTION,
-				   NULL, "No reformat action exist in the table.");
-	dv_resource.size = reformat->reformat_hdr->sz;
-	FLOW_HW_SET_DV_FIELDS(attr, is_root, dv_resource.flags, dv_resource);
-	MLX5_ASSERT(dv_resource.size <= MLX5_ENCAP_MAX_LEN);
-	memcpy(dv_resource.buf, reformat->reformat_hdr->data, dv_resource.size);
+	if (hw_acts->encap_decap->shared) {
+		dv_resource.size = hw_acts->encap_decap->data_size;
+		MLX5_ASSERT(dv_resource.size <= MLX5_ENCAP_MAX_LEN);
+		memcpy(&dv_resource.buf, hw_acts->encap_decap->data, dv_resource.size);
+		dv_resource.flags |= MLX5DR_ACTION_FLAG_SHARED;
+	} else {
+		typeof(mpctx->reformat[0]) *reformat = mpctx->reformat + ix;
+		if (!reformat->elements_num)
+			return rte_flow_error_set(error, EINVAL, RTE_FLOW_ERROR_TYPE_ACTION,
+					NULL, "No reformat action exist in the table.");
+		dv_resource.size = reformat->reformat_hdr->sz;
+		MLX5_ASSERT(dv_resource.size <= MLX5_ENCAP_MAX_LEN);
+		memcpy(&dv_resource.buf, reformat->reformat_hdr->data, dv_resource.size);
+	}
 	ret = __flow_encap_decap_resource_register(dev, &dv_resource, is_root,
 		&dv_resource_ptr, error);
 	if (ret)
@@ -12451,7 +12491,10 @@ flow_hw_encap_decap_resource_register
 	MLX5_ASSERT(dv_resource_ptr);
 	dev_flow->nt2hws->rix_encap_decap = dv_resource_ptr->idx;
 	/* keep action for the rule construction. */
-	mpctx->segments[0].reformat_action[ix] = dv_resource_ptr->action;
+	if (hw_acts->encap_decap->shared)
+		hw_acts->rule_acts[hw_acts->encap_decap_pos].action = dv_resource_ptr->action;
+	else
+		mpctx->segments[0].reformat_action[ix] = dv_resource_ptr->action;
 	/* Bulk size is 1, so index is 1. */
 	dev_flow->res_idx = 1;
 	return 0;
@@ -12570,6 +12613,15 @@ flow_hw_translate_flow_actions(struct rte_eth_dev *dev,
 	RTE_SET_USED(action_flags);
 	memset(masks, 0, sizeof(masks));
 	memset(mask_conf, 0, sizeof(mask_conf));
+	/*
+	 * Notice All direct actions will be unmasked,
+	 * except for modify header and encap,
+	 * and therefore will be parsed as part of action construct.
+	 * Modify header is always shared in HWS,
+	 * encap is masked such that it will be treated as shared.
+	 * shared actions will be parsed as part of template translation
+	 * and not during action construct.
+	 */
 	flow_nta_build_template_mask(actions, masks, mask_conf);
 	/* The group in the attribute translation was done in advance. */
 	ret = __translate_group(dev, attr, external, attr->group, &src_group, error);
@@ -12587,7 +12639,6 @@ flow_hw_translate_flow_actions(struct rte_eth_dev *dev,
 	if (!table)
 		return rte_flow_error_set(error, ENOMEM, RTE_FLOW_ERROR_TYPE_ACTION,
 				   actions, "Failed to allocate dummy table");
-	/* Notice All actions will be unmasked. */
 	at = __flow_hw_actions_template_create(dev, &template_attr, actions, masks, true, error);
 	if (!at) {
 		ret = -rte_errno;
-- 
2.25.1


^ permalink raw reply	[flat|nested] 15+ messages in thread

* [PATCH v3 4/4] net/mlx5: clean up TODO comments
  2024-06-03 10:54 ` [PATCH v3 1/4] net/mlx5: reorganize main structures Maayan Kashani
  2024-06-03 10:54   ` [PATCH v3 2/4] net/mlx5: set modify header as shared action Maayan Kashani
  2024-06-03 10:54   ` [PATCH v3 3/4] net/mlx5: set encap " Maayan Kashani
@ 2024-06-03 10:54   ` Maayan Kashani
  2024-06-06 10:06   ` [PATCH v4 0/4] non template pmd code changes Maayan Kashani
  3 siblings, 0 replies; 15+ messages in thread
From: Maayan Kashani @ 2024-06-03 10:54 UTC (permalink / raw)
  To: dev
  Cc: mkashani, dsosnowski, rasland, Viacheslav Ovsiienko, Ori Kam,
	Suanming Mou, Matan Azrad

review and cleanup unneeded TODO comments.

Signed-off-by: Maayan Kashani <mkashani@nvidia.com>
---
 drivers/net/mlx5/mlx5_flow_hw.c | 32 +++++++++-----------------------
 1 file changed, 9 insertions(+), 23 deletions(-)

diff --git a/drivers/net/mlx5/mlx5_flow_hw.c b/drivers/net/mlx5/mlx5_flow_hw.c
index 61b6a71bbf..d9e43c25c3 100644
--- a/drivers/net/mlx5/mlx5_flow_hw.c
+++ b/drivers/net/mlx5/mlx5_flow_hw.c
@@ -12390,7 +12390,6 @@ static int flow_hw_prepare(struct rte_eth_dev *dev,
 		return rte_flow_error_set(error, ENOMEM,
 				RTE_FLOW_ERROR_TYPE_UNSPECIFIED, NULL,
 				"cannot allocate flow aux memory");
-	/*TODO: consider if other allocation is needed for actions translate. */
 	return 0;
 }
 
@@ -12633,9 +12632,8 @@ flow_hw_translate_flow_actions(struct rte_eth_dev *dev,
 		table_type = MLX5DR_TABLE_TYPE_NIC_TX;
 	else
 		table_type = MLX5DR_TABLE_TYPE_NIC_RX;
-	/* TODO: consider add flag if using only non template mode to reduce table struct size. */
+	/* TODO: consider to reuse the workspace per thread. */
 	table = mlx5_malloc(MLX5_MEM_ZERO, sizeof(*table), 0, SOCKET_ID_ANY);
-	/* TODO: consider sending only relevant fields to construct. */
 	if (!table)
 		return rte_flow_error_set(error, ENOMEM, RTE_FLOW_ERROR_TYPE_ACTION,
 				   actions, "Failed to allocate dummy table");
@@ -12833,9 +12831,7 @@ flow_hw_allocate_actions(struct rte_eth_dev *dev,
 				  NULL, "fail to allocate actions");
 }
 
-/* TODO: remove dev if not used */
-static int flow_hw_apply(struct rte_eth_dev *dev __rte_unused,
-			 const struct rte_flow_item items[],
+static int flow_hw_apply(const struct rte_flow_item items[],
 			 struct mlx5dr_rule_action rule_actions[],
 			 struct rte_flow_hw *flow,
 			 struct rte_flow_error *error)
@@ -12901,16 +12897,7 @@ flow_hw_create_flow(struct rte_eth_dev *dev, enum mlx5_flow_type type,
 		.group = attr->group,
 		.priority = attr->priority,
 		.rss_level = 0,
-		/*
-		 * TODO: currently only mlx5_flow_lacp_miss rule is relevant:
-		 * action type=(enum rte_flow_action_type) MLX5_RTE_FLOW_ACTION_TYPE_DEFAULT_MISS.
-		 * I don't want to waist time going over all actions for this corner case.
-		 * Needs to use another preparation code to update this action flags.
-		 * if (action_type == (enum rte_flow_action_type)
-		 * MLX5_RTE_FLOW_ACTION_TYPE_DEFAULT_MISS)
-		 *     act_flags |= MLX5_FLOW_ACTION_DEFAULT_MISS;
-		 */
-		.act_flags = 0, /*TODO update*/
+		.act_flags = action_flags,
 		.tbl_type = 0,
 		};
 
@@ -12958,11 +12945,6 @@ flow_hw_create_flow(struct rte_eth_dev *dev, enum mlx5_flow_type type,
 	if (ret)
 		goto error;
 
-	/*
-	 * TODO: check regarding release: CT index is not saved per rule,
-	 * the index is in the conf of given action.
-	 */
-
 	/*
 	 * If the flow is external (from application) OR device is started,
 	 * OR mreg discover, then apply immediately.
@@ -12970,7 +12952,7 @@ flow_hw_create_flow(struct rte_eth_dev *dev, enum mlx5_flow_type type,
 	if (external || dev->data->dev_started ||
 	    (attr->group == MLX5_FLOW_MREG_CP_TABLE_GROUP &&
 	     attr->priority == MLX5_FLOW_LOWEST_PRIO_INDICATOR)) {
-		ret = flow_hw_apply(dev, items, hw_act.rule_acts, *flow, error);
+		ret = flow_hw_apply(items, hw_act.rule_acts, *flow, error);
 		if (ret)
 			goto error;
 	}
@@ -13012,7 +12994,7 @@ flow_hw_destroy(struct rte_eth_dev *dev, struct rte_flow_hw *flow)
 			DRV_LOG(ERR, "bwc rule destroy failed");
 	}
 	flow->operation_type = MLX5_FLOW_HW_FLOW_OP_TYPE_DESTROY;
-	/* TODO: notice this function does not handle shared/static actions. */
+	/* Notice this function does not handle shared/static actions. */
 	hw_cmpl_flow_update_or_destroy(dev, flow, 0, NULL);
 
 	/**
@@ -13108,6 +13090,10 @@ static uintptr_t flow_hw_list_create(struct rte_eth_dev *dev,
 	uint64_t item_flags = flow_hw_matching_item_flags_get(items);
 	uint64_t action_flags = flow_hw_action_flags_get(actions, error);
 
+	/*
+	 * TODO: add a call to flow_hw_validate function once it exist.
+	 * and update mlx5_flow_hw_drv_ops accordingly.
+	 */
 
 	if (action_flags & MLX5_FLOW_ACTION_RSS) {
 		const struct rte_flow_action_rss
-- 
2.25.1


^ permalink raw reply	[flat|nested] 15+ messages in thread

* [PATCH v4 0/4] non template pmd code changes
  2024-06-03 10:54 ` [PATCH v3 1/4] net/mlx5: reorganize main structures Maayan Kashani
                     ` (2 preceding siblings ...)
  2024-06-03 10:54   ` [PATCH v3 4/4] net/mlx5: clean up TODO comments Maayan Kashani
@ 2024-06-06 10:06   ` Maayan Kashani
  2024-06-06 10:06     ` [PATCH v4 1/4] net/mlx5: reorganize main structures Maayan Kashani
                       ` (4 more replies)
  3 siblings, 5 replies; 15+ messages in thread
From: Maayan Kashani @ 2024-06-06 10:06 UTC (permalink / raw)
  To: dev; +Cc: mkashani, dsosnowski, rasland

Code changes applied after review.

Maayan Kashani (4):
  net/mlx5: reorganize main structures
  net/mlx5: set modify header as shared action
  net/mlx5: set encap as shared action
  net/mlx5: clean up TODO comments

 drivers/net/mlx5/mlx5_flow.h    |  29 ++--
 drivers/net/mlx5/mlx5_flow_dv.c |  12 +-
 drivers/net/mlx5/mlx5_flow_hw.c | 234 +++++++++++++++++++-------------
 3 files changed, 162 insertions(+), 113 deletions(-)

-- 
2.21.0


^ permalink raw reply	[flat|nested] 15+ messages in thread

* [PATCH v4 1/4] net/mlx5: reorganize main structures
  2024-06-06 10:06   ` [PATCH v4 0/4] non template pmd code changes Maayan Kashani
@ 2024-06-06 10:06     ` Maayan Kashani
  2024-06-06 10:06     ` [PATCH v4 2/4] net/mlx5: set modify header as shared action Maayan Kashani
                       ` (3 subsequent siblings)
  4 siblings, 0 replies; 15+ messages in thread
From: Maayan Kashani @ 2024-06-06 10:06 UTC (permalink / raw)
  To: dev
  Cc: mkashani, dsosnowski, rasland, Viacheslav Ovsiienko, Ori Kam,
	Suanming Mou, Matan Azrad

Reorganize rte_flow_hw and rte_flow_nt2hws
structures for better performance, and removed packed.

Signed-off-by: Maayan Kashani <mkashani@nvidia.com>
Acked-by: Dariusz Sosnowski <dsosnowski@nvidia.com>
---
 drivers/net/mlx5/mlx5_flow.h | 26 ++++++++++++--------------
 1 file changed, 12 insertions(+), 14 deletions(-)

diff --git a/drivers/net/mlx5/mlx5_flow.h b/drivers/net/mlx5/mlx5_flow.h
index 7e0f005741..5a3f047968 100644
--- a/drivers/net/mlx5/mlx5_flow.h
+++ b/drivers/net/mlx5/mlx5_flow.h
@@ -1328,12 +1328,12 @@ struct rte_flow_nt2hws {
 	struct rte_flow_hw_aux *flow_aux;
 	/** Modify header pointer. */
 	struct mlx5_flow_dv_modify_hdr_resource *modify_hdr;
+	/** Chain NTA flows. */
+	SLIST_ENTRY(rte_flow_hw) next;
 	/** Encap/decap index. */
 	uint32_t rix_encap_decap;
 	uint8_t chaned_flow;
-	/** Chain NTA flows. */
-	SLIST_ENTRY(rte_flow_hw) next;
-} __rte_packed;
+};
 
 /** HWS flow struct. */
 struct rte_flow_hw {
@@ -1345,6 +1345,12 @@ struct rte_flow_hw {
 	};
 	/** Application's private data passed to enqueued flow operation. */
 	void *user_data;
+	union {
+		/** Jump action. */
+		struct mlx5_hw_jump_action *jump;
+		/** TIR action. */
+		struct mlx5_hrxq *hrxq;
+	};
 	/** Flow index from indexed pool. */
 	uint32_t idx;
 	/** Resource index from indexed pool. */
@@ -1353,20 +1359,12 @@ struct rte_flow_hw {
 	uint32_t rule_idx;
 	/** Which flow fields (inline or in auxiliary struct) are used. */
 	uint32_t flags;
+	/** COUNT action index. */
+	cnt_id_t cnt_id;
 	/** Ongoing flow operation type. */
 	uint8_t operation_type;
 	/** Index of pattern template this flow is based on. */
 	uint8_t mt_idx;
-
-	/** COUNT action index. */
-	cnt_id_t cnt_id;
-	union {
-		/** Jump action. */
-		struct mlx5_hw_jump_action *jump;
-		/** TIR action. */
-		struct mlx5_hrxq *hrxq;
-	};
-
 	/** Equals true if it is non template rule. */
 	bool nt_rule;
 	/**
@@ -1377,7 +1375,7 @@ struct rte_flow_hw {
 	uint8_t padding[9];
 	/** HWS layer data struct. */
 	uint8_t rule[];
-} __rte_packed;
+};
 
 /** Auxiliary data fields that are updatable. */
 struct rte_flow_hw_aux_fields {
-- 
2.21.0


^ permalink raw reply	[flat|nested] 15+ messages in thread

* [PATCH v4 2/4] net/mlx5: set modify header as shared action
  2024-06-06 10:06   ` [PATCH v4 0/4] non template pmd code changes Maayan Kashani
  2024-06-06 10:06     ` [PATCH v4 1/4] net/mlx5: reorganize main structures Maayan Kashani
@ 2024-06-06 10:06     ` Maayan Kashani
  2024-06-06 10:06     ` [PATCH v4 3/4] net/mlx5: set encap " Maayan Kashani
                       ` (2 subsequent siblings)
  4 siblings, 0 replies; 15+ messages in thread
From: Maayan Kashani @ 2024-06-06 10:06 UTC (permalink / raw)
  To: dev
  Cc: mkashani, dsosnowski, rasland, Viacheslav Ovsiienko, Ori Kam,
	Suanming Mou, Matan Azrad

In current implementation, in non template mode,
modify header action is not set as always shared.
Align to HWS implementation, setting modify header action as always shared.
Optimize mask initialization.

Signed-off-by: Maayan Kashani <mkashani@nvidia.com>
Acked-by: Dariusz Sosnowski <dsosnowski@nvidia.com>
---
 drivers/net/mlx5/mlx5_flow.h    |  3 --
 drivers/net/mlx5/mlx5_flow_dv.c | 10 ++--
 drivers/net/mlx5/mlx5_flow_hw.c | 91 +++++++++++++++++++--------------
 3 files changed, 59 insertions(+), 45 deletions(-)

diff --git a/drivers/net/mlx5/mlx5_flow.h b/drivers/net/mlx5/mlx5_flow.h
index 5a3f047968..f5bb01616e 100644
--- a/drivers/net/mlx5/mlx5_flow.h
+++ b/drivers/net/mlx5/mlx5_flow.h
@@ -669,9 +669,6 @@ struct mlx5_flow_dv_modify_hdr_resource {
 	struct mlx5_list_entry entry;
 	void *action; /**< Modify header action object. */
 	uint32_t idx;
-#ifdef HAVE_MLX5_HWS_SUPPORT
-	void *mh_dr_pattern; /**< Modify header DR pattern(HWS only). */
-#endif
 	uint64_t flags; /**< Flags for RDMA API(HWS only). */
 	/* Key area for hash list matching: */
 	uint8_t ft_type; /**< Flow table type, Rx or Tx. */
diff --git a/drivers/net/mlx5/mlx5_flow_dv.c b/drivers/net/mlx5/mlx5_flow_dv.c
index 3611ffa4a1..94af391894 100644
--- a/drivers/net/mlx5/mlx5_flow_dv.c
+++ b/drivers/net/mlx5/mlx5_flow_dv.c
@@ -6219,9 +6219,7 @@ flow_modify_create_cb(void *tool_ctx, void *cb_ctx)
 	uint32_t data_len = ref->actions_num * sizeof(ref->actions[0]);
 	uint32_t key_len = sizeof(*ref) - offsetof(typeof(*ref), ft_type);
 	uint32_t idx;
-	struct mlx5_tbl_multi_pattern_ctx *mpctx;
 
-	typeof(mpctx->mh) *mh_dr_pattern = ref->mh_dr_pattern;
 	if (unlikely(!ipool)) {
 		rte_flow_error_set(ctx->error, ENOMEM,
 				   RTE_FLOW_ERROR_TYPE_UNSPECIFIED,
@@ -6240,9 +6238,13 @@ flow_modify_create_cb(void *tool_ctx, void *cb_ctx)
 			key_len + data_len);
 	if (sh->config.dv_flow_en == 2) {
 #ifdef HAVE_MLX5_HWS_SUPPORT
+		struct mlx5dr_action_mh_pattern pattern = {
+			.sz = data_len,
+			.data = (__be64 *)ref->actions
+		};
 		entry->action = mlx5dr_action_create_modify_header(ctx->data2,
-			mh_dr_pattern->elements_num,
-			mh_dr_pattern->pattern, 0, ref->flags);
+			1,
+			&pattern, 0, ref->flags);
 		if (!entry->action)
 			ret = -1;
 #else
diff --git a/drivers/net/mlx5/mlx5_flow_hw.c b/drivers/net/mlx5/mlx5_flow_hw.c
index 43bcaab592..134a035f41 100644
--- a/drivers/net/mlx5/mlx5_flow_hw.c
+++ b/drivers/net/mlx5/mlx5_flow_hw.c
@@ -1439,13 +1439,11 @@ flow_hw_converted_mhdr_cmds_append(struct mlx5_hw_modify_header_action *mhdr,
 
 static __rte_always_inline void
 flow_hw_modify_field_init(struct mlx5_hw_modify_header_action *mhdr,
-			  struct rte_flow_actions_template *at,
-			  bool nt_mode)
+			  struct rte_flow_actions_template *at)
 {
 	memset(mhdr, 0, sizeof(*mhdr));
 	/* Modify header action without any commands is shared by default. */
-	if (!(nt_mode))
-		mhdr->shared = true;
+	mhdr->shared = true;
 	mhdr->pos = at->mhdr_off;
 }
 
@@ -2212,10 +2210,6 @@ mlx5_tbl_translate_modify_header(struct rte_eth_dev *dev,
 				 struct mlx5_hw_modify_header_action *mhdr,
 				 struct rte_flow_error *error)
 {
-	struct mlx5_priv *priv = dev->data->dev_private;
-	const struct rte_flow_template_table_attr *table_attr = &cfg->attr;
-	const struct rte_flow_attr *attr = &table_attr->flow_attr;
-	enum mlx5dr_table_type tbl_type = get_mlx5dr_table_type(attr);
 	uint16_t mhdr_ix = mhdr->pos;
 	struct mlx5dr_action_mh_pattern pattern = {
 		.sz = sizeof(struct mlx5_modification_cmd) * mhdr->mhdr_cmds_num
@@ -2232,20 +2226,8 @@ mlx5_tbl_translate_modify_header(struct rte_eth_dev *dev,
 					  RTE_FLOW_ERROR_TYPE_UNSPECIFIED,
 					  NULL, "translate modify_header: no memory for modify header context");
 	rte_memcpy(acts->mhdr, mhdr, sizeof(*mhdr));
-	pattern.data = (__be64 *)acts->mhdr->mhdr_cmds;
-	if (mhdr->shared) {
-		uint32_t flags = mlx5_hw_act_flag[!!attr->group][tbl_type] |
-				 MLX5DR_ACTION_FLAG_SHARED;
-
-		acts->mhdr->action = mlx5dr_action_create_modify_header
-						(priv->dr_ctx, 1, &pattern, 0,
-						 flags);
-		if (!acts->mhdr->action)
-			return rte_flow_error_set(error, rte_errno,
-						  RTE_FLOW_ERROR_TYPE_UNSPECIFIED,
-						  NULL, "translate modify_header: failed to create DR action");
-		acts->rule_acts[mhdr_ix].action = acts->mhdr->action;
-	} else {
+	if (!mhdr->shared) {
+		pattern.data = (__be64 *)acts->mhdr->mhdr_cmds;
 		typeof(mp_ctx->mh) *mh = &mp_ctx->mh;
 		uint32_t idx = mh->elements_num;
 		mh->pattern[mh->elements_num++] = pattern;
@@ -2256,6 +2238,32 @@ mlx5_tbl_translate_modify_header(struct rte_eth_dev *dev,
 	return 0;
 }
 
+static int
+mlx5_tbl_ensure_shared_modify_header(struct rte_eth_dev *dev,
+				     const struct mlx5_flow_template_table_cfg *cfg,
+				     struct mlx5_hw_actions *acts,
+				     struct rte_flow_error *error)
+{
+	struct mlx5_priv *priv = dev->data->dev_private;
+	const struct rte_flow_template_table_attr *table_attr = &cfg->attr;
+	const struct rte_flow_attr *attr = &table_attr->flow_attr;
+	enum mlx5dr_table_type tbl_type = get_mlx5dr_table_type(attr);
+	struct mlx5dr_action_mh_pattern pattern = {
+		.sz = sizeof(struct mlx5_modification_cmd) * acts->mhdr->mhdr_cmds_num
+	};
+	uint16_t mhdr_ix = acts->mhdr->pos;
+	uint32_t flags = mlx5_hw_act_flag[!!attr->group][tbl_type] | MLX5DR_ACTION_FLAG_SHARED;
+
+	pattern.data = (__be64 *)acts->mhdr->mhdr_cmds;
+	acts->mhdr->action = mlx5dr_action_create_modify_header(priv->dr_ctx, 1,
+								&pattern, 0, flags);
+	if (!acts->mhdr->action)
+		return rte_flow_error_set(error, rte_errno,
+					  RTE_FLOW_ERROR_TYPE_UNSPECIFIED, NULL,
+					  "translate modify_header: failed to create DR action");
+	acts->rule_acts[mhdr_ix].action = acts->mhdr->action;
+	return 0;
+}
 
 static int
 mlx5_create_ipv6_ext_reformat(struct rte_eth_dev *dev,
@@ -2393,7 +2401,7 @@ __flow_hw_translate_actions_template(struct rte_eth_dev *dev,
 	uint32_t target_grp = 0;
 	int table_type;
 
-	flow_hw_modify_field_init(&mhdr, at, nt_mode);
+	flow_hw_modify_field_init(&mhdr, at);
 	if (attr->transfer)
 		type = MLX5DR_TABLE_TYPE_FDB;
 	else if (attr->egress)
@@ -2780,10 +2788,14 @@ __flow_hw_translate_actions_template(struct rte_eth_dev *dev,
 		}
 	}
 	if (mhdr.pos != UINT16_MAX) {
-		ret = mlx5_tbl_translate_modify_header(dev, cfg, acts, mp_ctx,
-						       &mhdr, error);
+		ret = mlx5_tbl_translate_modify_header(dev, cfg, acts, mp_ctx, &mhdr, error);
 		if (ret)
 			goto err;
+		if (!nt_mode && mhdr.shared) {
+			ret = mlx5_tbl_ensure_shared_modify_header(dev, cfg, acts, error);
+			if (ret)
+				goto err;
+		}
 	}
 	if (reformat_used) {
 		ret = mlx5_tbl_translate_reformat(priv, table_attr, acts, at,
@@ -12348,8 +12360,8 @@ static int flow_hw_prepare(struct rte_eth_dev *dev,
 	/*TODO: consider if other allocation is needed for actions translate. */
 	return 0;
 }
-#define FLOW_HW_SET_DV_FIELDS(flow_attr, root, flags)						\
-{												\
+
+#define FLOW_HW_SET_DV_FIELDS(flow_attr, root, flags, dv_resource) {				\
 	typeof(flow_attr) _flow_attr = (flow_attr);						\
 	if (_flow_attr->transfer)								\
 		dv_resource.ft_type = MLX5DV_FLOW_TABLE_TYPE_FDB;				\
@@ -12370,28 +12382,31 @@ flow_hw_modify_hdr_resource_register
 {
 	struct rte_flow_attr *attr = &table->cfg.attr.flow_attr;
 	struct mlx5_flow_dv_modify_hdr_resource *dv_resource_ptr = NULL;
-	struct mlx5_flow_dv_modify_hdr_resource dv_resource;
-	struct mlx5_tbl_multi_pattern_ctx *mpctx = &table->mpctx;
+	union {
+		struct mlx5_flow_dv_modify_hdr_resource dv_resource;
+		uint8_t data[sizeof(struct mlx5_flow_dv_modify_hdr_resource) +
+			     sizeof(struct mlx5_modification_cmd) * MLX5_MHDR_MAX_CMD];
+	} dummy;
 	int ret;
 
 	if (hw_acts->mhdr) {
-		dv_resource.actions_num = hw_acts->mhdr->mhdr_cmds_num;
-		memcpy(dv_resource.actions, hw_acts->mhdr->mhdr_cmds,
-			sizeof(struct mlx5_modification_cmd) * dv_resource.actions_num);
+		dummy.dv_resource.actions_num = hw_acts->mhdr->mhdr_cmds_num;
+		memcpy(dummy.dv_resource.actions, hw_acts->mhdr->mhdr_cmds,
+			sizeof(struct mlx5_modification_cmd) * dummy.dv_resource.actions_num);
 	} else {
 		return 0;
 	}
-	FLOW_HW_SET_DV_FIELDS(attr, dv_resource.root, dv_resource.flags);
-	/* Save a pointer to the pattern needed for DR layer created on actions translate. */
-	dv_resource.mh_dr_pattern = &table->mpctx.mh;
-	ret = __flow_modify_hdr_resource_register(dev, &dv_resource,
+	FLOW_HW_SET_DV_FIELDS(attr, dummy.dv_resource.root, dummy.dv_resource.flags,
+			      dummy.dv_resource);
+	dummy.dv_resource.flags |= MLX5DR_ACTION_FLAG_SHARED;
+	ret = __flow_modify_hdr_resource_register(dev, &dummy.dv_resource,
 		&dv_resource_ptr, error);
 	if (ret)
 		return ret;
 	MLX5_ASSERT(dv_resource_ptr);
 	dev_flow->nt2hws->modify_hdr = dv_resource_ptr;
 	/* keep action for the rule construction. */
-	mpctx->segments[0].mhdr_action = dv_resource_ptr->action;
+	hw_acts->rule_acts[hw_acts->mhdr->pos].action = dv_resource_ptr->action;
 	/* Bulk size is 1, so index is 1. */
 	dev_flow->res_idx = 1;
 	return 0;
@@ -12426,7 +12441,7 @@ flow_hw_encap_decap_resource_register
 		return rte_flow_error_set(error, EINVAL, RTE_FLOW_ERROR_TYPE_ACTION,
 				   NULL, "No reformat action exist in the table.");
 	dv_resource.size = reformat->reformat_hdr->sz;
-	FLOW_HW_SET_DV_FIELDS(attr, is_root, dv_resource.flags);
+	FLOW_HW_SET_DV_FIELDS(attr, is_root, dv_resource.flags, dv_resource);
 	MLX5_ASSERT(dv_resource.size <= MLX5_ENCAP_MAX_LEN);
 	memcpy(dv_resource.buf, reformat->reformat_hdr->data, dv_resource.size);
 	ret = __flow_encap_decap_resource_register(dev, &dv_resource, is_root,
-- 
2.21.0


^ permalink raw reply	[flat|nested] 15+ messages in thread

* [PATCH v4 3/4] net/mlx5: set encap as shared action
  2024-06-06 10:06   ` [PATCH v4 0/4] non template pmd code changes Maayan Kashani
  2024-06-06 10:06     ` [PATCH v4 1/4] net/mlx5: reorganize main structures Maayan Kashani
  2024-06-06 10:06     ` [PATCH v4 2/4] net/mlx5: set modify header as shared action Maayan Kashani
@ 2024-06-06 10:06     ` Maayan Kashani
  2024-06-06 10:06     ` [PATCH v4 4/4] net/mlx5: clean up TODO comments Maayan Kashani
  2024-06-11 11:16     ` [PATCH v4 0/4] non template pmd code changes Raslan Darawsheh
  4 siblings, 0 replies; 15+ messages in thread
From: Maayan Kashani @ 2024-06-06 10:06 UTC (permalink / raw)
  To: dev
  Cc: mkashani, dsosnowski, rasland, Viacheslav Ovsiienko, Ori Kam,
	Suanming Mou, Matan Azrad

In current implementation, in non template mode,
encap action is not set as non shared(according to given masks).
By masking the relevant fields, encap is now used as shared.
decap remained unshared and cannot be shared according to
current implementation.
Optimize encap action mask initialization and fixed requested
number of reformat actions to one.

Signed-off-by: Maayan Kashani <mkashani@nvidia.com>
Acked-by: Dariusz Sosnowski <dsosnowski@nvidia.com>
---
 drivers/net/mlx5/mlx5_flow_dv.c |   2 +-
 drivers/net/mlx5/mlx5_flow_hw.c | 119 +++++++++++++++++++++++---------
 2 files changed, 86 insertions(+), 35 deletions(-)

diff --git a/drivers/net/mlx5/mlx5_flow_dv.c b/drivers/net/mlx5/mlx5_flow_dv.c
index 94af391894..1484531418 100644
--- a/drivers/net/mlx5/mlx5_flow_dv.c
+++ b/drivers/net/mlx5/mlx5_flow_dv.c
@@ -4317,7 +4317,7 @@ flow_encap_decap_create_cb(void *tool_ctx, void *cb_ctx)
 		hdr.data = ctx_resource->buf;
 		resource->action = mlx5dr_action_create_reformat
 		(ctx->data2, (enum mlx5dr_action_type)ctx_resource->reformat_type, 1,
-			&hdr, 1, ctx_resource->flags);
+			&hdr, 0, ctx_resource->flags);
 		if (!resource->action)
 			ret = -1;
 #else
diff --git a/drivers/net/mlx5/mlx5_flow_hw.c b/drivers/net/mlx5/mlx5_flow_hw.c
index 134a035f41..61b6a71bbf 100644
--- a/drivers/net/mlx5/mlx5_flow_hw.c
+++ b/drivers/net/mlx5/mlx5_flow_hw.c
@@ -2124,9 +2124,17 @@ table_template_translate_indirect_list(struct rte_eth_dev *dev,
 	return ret;
 }
 
+static void
+mlx5_set_reformat_header(struct mlx5dr_action_reformat_header *hdr,
+			 uint8_t *encap_data,
+			 size_t data_size)
+{
+	hdr->sz = data_size;
+	hdr->data = encap_data;
+}
+
 static int
 mlx5_tbl_translate_reformat(struct mlx5_priv *priv,
-			    const struct rte_flow_template_table_attr *table_attr,
 			    struct mlx5_hw_actions *acts,
 			    struct rte_flow_actions_template *at,
 			    const struct rte_flow_item *enc_item,
@@ -2138,8 +2146,6 @@ mlx5_tbl_translate_reformat(struct mlx5_priv *priv,
 			    struct rte_flow_error *error)
 {
 	int mp_reformat_ix = mlx5_multi_pattern_reformat_to_index(refmt_type);
-	const struct rte_flow_attr *attr = &table_attr->flow_attr;
-	enum mlx5dr_table_type tbl_type = get_mlx5dr_table_type(attr);
 	struct mlx5dr_action_reformat_header hdr;
 	uint8_t buf[MLX5_ENCAP_MAX_LEN];
 	bool shared_rfmt = false;
@@ -2164,19 +2170,16 @@ mlx5_tbl_translate_reformat(struct mlx5_priv *priv,
 		return rte_flow_error_set(error, ENOMEM,
 					  RTE_FLOW_ERROR_TYPE_UNSPECIFIED,
 					  NULL, "no memory for reformat context");
-	hdr.sz = data_size;
-	hdr.data = encap_data;
+	acts->encap_decap_pos = at->reformat_off;
+	acts->encap_decap->data_size = data_size;
+	acts->encap_decap->action_type = refmt_type;
 	if (shared_rfmt || mp_reformat_ix < 0) {
 		uint16_t reformat_ix = at->reformat_off;
-		uint32_t flags = mlx5_hw_act_flag[!!attr->group][tbl_type] |
-				 MLX5DR_ACTION_FLAG_SHARED;
-
-		acts->encap_decap->action =
-			mlx5dr_action_create_reformat(priv->dr_ctx, refmt_type,
-						      1, &hdr, 0, flags);
-		if (!acts->encap_decap->action)
-			return -rte_errno;
-		acts->rule_acts[reformat_ix].action = acts->encap_decap->action;
+		/*
+		 * This copy is only needed in non template mode.
+		 * In order to create the action later.
+		 */
+		memcpy(acts->encap_decap->data, encap_data, data_size);
 		acts->rule_acts[reformat_ix].reformat.data = acts->encap_decap->data;
 		acts->rule_acts[reformat_ix].reformat.offset = 0;
 		acts->encap_decap->shared = true;
@@ -2184,14 +2187,11 @@ mlx5_tbl_translate_reformat(struct mlx5_priv *priv,
 		uint32_t ix;
 		typeof(mp_ctx->reformat[0]) *reformat = mp_ctx->reformat +
 							mp_reformat_ix;
-
+		mlx5_set_reformat_header(&hdr, encap_data, data_size);
 		ix = reformat->elements_num++;
 		reformat->reformat_hdr[ix] = hdr;
 		acts->rule_acts[at->reformat_off].reformat.hdr_idx = ix;
-		acts->encap_decap_pos = at->reformat_off;
 		acts->encap_decap->multi_pattern = 1;
-		acts->encap_decap->data_size = data_size;
-		acts->encap_decap->action_type = refmt_type;
 		ret = __flow_hw_act_data_encap_append
 			(priv, acts, (at->actions + reformat_src)->type,
 			 reformat_src, at->reformat_off, data_size);
@@ -2202,6 +2202,32 @@ mlx5_tbl_translate_reformat(struct mlx5_priv *priv,
 	return 0;
 }
 
+static int
+mlx5_tbl_create_reformat_action(struct mlx5_priv *priv,
+				const struct rte_flow_template_table_attr *table_attr,
+				struct mlx5_hw_actions *acts,
+				struct rte_flow_actions_template *at,
+				uint8_t *encap_data,
+				size_t data_size,
+				enum mlx5dr_action_type refmt_type)
+{
+	const struct rte_flow_attr *attr = &table_attr->flow_attr;
+	enum mlx5dr_table_type tbl_type = get_mlx5dr_table_type(attr);
+	struct mlx5dr_action_reformat_header hdr;
+
+	mlx5_set_reformat_header(&hdr, encap_data, data_size);
+	uint16_t reformat_ix = at->reformat_off;
+	uint32_t flags = mlx5_hw_act_flag[!!attr->group][tbl_type] |
+				MLX5DR_ACTION_FLAG_SHARED;
+
+	acts->encap_decap->action = mlx5dr_action_create_reformat(priv->dr_ctx, refmt_type,
+							   1, &hdr, 0, flags);
+	if (!acts->encap_decap->action)
+		return -rte_errno;
+	acts->rule_acts[reformat_ix].action = acts->encap_decap->action;
+	return 0;
+}
+
 static int
 mlx5_tbl_translate_modify_header(struct rte_eth_dev *dev,
 				 const struct mlx5_flow_template_table_cfg *cfg,
@@ -2798,7 +2824,7 @@ __flow_hw_translate_actions_template(struct rte_eth_dev *dev,
 		}
 	}
 	if (reformat_used) {
-		ret = mlx5_tbl_translate_reformat(priv, table_attr, acts, at,
+		ret = mlx5_tbl_translate_reformat(priv, acts, at,
 						  enc_item, enc_item_m,
 						  encap_data, encap_data_m,
 						  mp_ctx, data_size,
@@ -2806,6 +2832,13 @@ __flow_hw_translate_actions_template(struct rte_eth_dev *dev,
 						  refmt_type, error);
 		if (ret)
 			goto err;
+		if (!nt_mode && acts->encap_decap->shared) {
+			ret = mlx5_tbl_create_reformat_action(priv, table_attr, acts, at,
+							      encap_data, data_size,
+							      refmt_type);
+			if (ret)
+				goto err;
+		}
 	}
 	if (recom_used) {
 		MLX5_ASSERT(at->recom_off != UINT16_MAX);
@@ -12361,7 +12394,7 @@ static int flow_hw_prepare(struct rte_eth_dev *dev,
 	return 0;
 }
 
-#define FLOW_HW_SET_DV_FIELDS(flow_attr, root, flags, dv_resource) {				\
+#define FLOW_HW_SET_DV_FIELDS(flow_attr, root, dv_resource) {					\
 	typeof(flow_attr) _flow_attr = (flow_attr);						\
 	if (_flow_attr->transfer)								\
 		dv_resource.ft_type = MLX5DV_FLOW_TABLE_TYPE_FDB;				\
@@ -12369,7 +12402,8 @@ static int flow_hw_prepare(struct rte_eth_dev *dev,
 		dv_resource.ft_type = _flow_attr->egress ? MLX5DV_FLOW_TABLE_TYPE_NIC_TX :	\
 					     MLX5DV_FLOW_TABLE_TYPE_NIC_RX;			\
 	root = _flow_attr->group ? 0 : 1;							\
-	flags = mlx5_hw_act_flag[!!_flow_attr->group][get_mlx5dr_table_type(_flow_attr)];	\
+	dv_resource.flags =									\
+		mlx5_hw_act_flag[!!_flow_attr->group][get_mlx5dr_table_type(_flow_attr)];	\
 }
 
 static int
@@ -12396,8 +12430,7 @@ flow_hw_modify_hdr_resource_register
 	} else {
 		return 0;
 	}
-	FLOW_HW_SET_DV_FIELDS(attr, dummy.dv_resource.root, dummy.dv_resource.flags,
-			      dummy.dv_resource);
+	FLOW_HW_SET_DV_FIELDS(attr, dummy.dv_resource.root, dummy.dv_resource);
 	dummy.dv_resource.flags |= MLX5DR_ACTION_FLAG_SHARED;
 	ret = __flow_modify_hdr_resource_register(dev, &dummy.dv_resource,
 		&dv_resource_ptr, error);
@@ -12432,18 +12465,25 @@ flow_hw_encap_decap_resource_register
 		dv_resource.reformat_type = hw_acts->encap_decap->action_type;
 	else
 		return 0;
+	FLOW_HW_SET_DV_FIELDS(attr, is_root, dv_resource);
 	ix = mlx5_bwc_multi_pattern_reformat_to_index((enum mlx5dr_action_type)
-		dv_resource.reformat_type);
+			dv_resource.reformat_type);
 	if (ix < 0)
 		return ix;
-	typeof(mpctx->reformat[0]) *reformat = mpctx->reformat + ix;
-	if (!reformat->elements_num)
-		return rte_flow_error_set(error, EINVAL, RTE_FLOW_ERROR_TYPE_ACTION,
-				   NULL, "No reformat action exist in the table.");
-	dv_resource.size = reformat->reformat_hdr->sz;
-	FLOW_HW_SET_DV_FIELDS(attr, is_root, dv_resource.flags, dv_resource);
-	MLX5_ASSERT(dv_resource.size <= MLX5_ENCAP_MAX_LEN);
-	memcpy(dv_resource.buf, reformat->reformat_hdr->data, dv_resource.size);
+	if (hw_acts->encap_decap->shared) {
+		dv_resource.size = hw_acts->encap_decap->data_size;
+		MLX5_ASSERT(dv_resource.size <= MLX5_ENCAP_MAX_LEN);
+		memcpy(&dv_resource.buf, hw_acts->encap_decap->data, dv_resource.size);
+		dv_resource.flags |= MLX5DR_ACTION_FLAG_SHARED;
+	} else {
+		typeof(mpctx->reformat[0]) *reformat = mpctx->reformat + ix;
+		if (!reformat->elements_num)
+			return rte_flow_error_set(error, EINVAL, RTE_FLOW_ERROR_TYPE_ACTION,
+					NULL, "No reformat action exist in the table.");
+		dv_resource.size = reformat->reformat_hdr->sz;
+		MLX5_ASSERT(dv_resource.size <= MLX5_ENCAP_MAX_LEN);
+		memcpy(&dv_resource.buf, reformat->reformat_hdr->data, dv_resource.size);
+	}
 	ret = __flow_encap_decap_resource_register(dev, &dv_resource, is_root,
 		&dv_resource_ptr, error);
 	if (ret)
@@ -12451,7 +12491,10 @@ flow_hw_encap_decap_resource_register
 	MLX5_ASSERT(dv_resource_ptr);
 	dev_flow->nt2hws->rix_encap_decap = dv_resource_ptr->idx;
 	/* keep action for the rule construction. */
-	mpctx->segments[0].reformat_action[ix] = dv_resource_ptr->action;
+	if (hw_acts->encap_decap->shared)
+		hw_acts->rule_acts[hw_acts->encap_decap_pos].action = dv_resource_ptr->action;
+	else
+		mpctx->segments[0].reformat_action[ix] = dv_resource_ptr->action;
 	/* Bulk size is 1, so index is 1. */
 	dev_flow->res_idx = 1;
 	return 0;
@@ -12570,6 +12613,15 @@ flow_hw_translate_flow_actions(struct rte_eth_dev *dev,
 	RTE_SET_USED(action_flags);
 	memset(masks, 0, sizeof(masks));
 	memset(mask_conf, 0, sizeof(mask_conf));
+	/*
+	 * Notice All direct actions will be unmasked,
+	 * except for modify header and encap,
+	 * and therefore will be parsed as part of action construct.
+	 * Modify header is always shared in HWS,
+	 * encap is masked such that it will be treated as shared.
+	 * shared actions will be parsed as part of template translation
+	 * and not during action construct.
+	 */
 	flow_nta_build_template_mask(actions, masks, mask_conf);
 	/* The group in the attribute translation was done in advance. */
 	ret = __translate_group(dev, attr, external, attr->group, &src_group, error);
@@ -12587,7 +12639,6 @@ flow_hw_translate_flow_actions(struct rte_eth_dev *dev,
 	if (!table)
 		return rte_flow_error_set(error, ENOMEM, RTE_FLOW_ERROR_TYPE_ACTION,
 				   actions, "Failed to allocate dummy table");
-	/* Notice All actions will be unmasked. */
 	at = __flow_hw_actions_template_create(dev, &template_attr, actions, masks, true, error);
 	if (!at) {
 		ret = -rte_errno;
-- 
2.21.0


^ permalink raw reply	[flat|nested] 15+ messages in thread

* [PATCH v4 4/4] net/mlx5: clean up TODO comments
  2024-06-06 10:06   ` [PATCH v4 0/4] non template pmd code changes Maayan Kashani
                       ` (2 preceding siblings ...)
  2024-06-06 10:06     ` [PATCH v4 3/4] net/mlx5: set encap " Maayan Kashani
@ 2024-06-06 10:06     ` Maayan Kashani
  2024-06-11 11:16     ` [PATCH v4 0/4] non template pmd code changes Raslan Darawsheh
  4 siblings, 0 replies; 15+ messages in thread
From: Maayan Kashani @ 2024-06-06 10:06 UTC (permalink / raw)
  To: dev
  Cc: mkashani, dsosnowski, rasland, Viacheslav Ovsiienko, Ori Kam,
	Suanming Mou, Matan Azrad

review and cleanup unneeded TODO comments.

Signed-off-by: Maayan Kashani <mkashani@nvidia.com>
Acked-by: Dariusz Sosnowski <dsosnowski@nvidia.com>
---
 drivers/net/mlx5/mlx5_flow_hw.c | 32 +++++++++-----------------------
 1 file changed, 9 insertions(+), 23 deletions(-)

diff --git a/drivers/net/mlx5/mlx5_flow_hw.c b/drivers/net/mlx5/mlx5_flow_hw.c
index 61b6a71bbf..d9e43c25c3 100644
--- a/drivers/net/mlx5/mlx5_flow_hw.c
+++ b/drivers/net/mlx5/mlx5_flow_hw.c
@@ -12390,7 +12390,6 @@ static int flow_hw_prepare(struct rte_eth_dev *dev,
 		return rte_flow_error_set(error, ENOMEM,
 				RTE_FLOW_ERROR_TYPE_UNSPECIFIED, NULL,
 				"cannot allocate flow aux memory");
-	/*TODO: consider if other allocation is needed for actions translate. */
 	return 0;
 }
 
@@ -12633,9 +12632,8 @@ flow_hw_translate_flow_actions(struct rte_eth_dev *dev,
 		table_type = MLX5DR_TABLE_TYPE_NIC_TX;
 	else
 		table_type = MLX5DR_TABLE_TYPE_NIC_RX;
-	/* TODO: consider add flag if using only non template mode to reduce table struct size. */
+	/* TODO: consider to reuse the workspace per thread. */
 	table = mlx5_malloc(MLX5_MEM_ZERO, sizeof(*table), 0, SOCKET_ID_ANY);
-	/* TODO: consider sending only relevant fields to construct. */
 	if (!table)
 		return rte_flow_error_set(error, ENOMEM, RTE_FLOW_ERROR_TYPE_ACTION,
 				   actions, "Failed to allocate dummy table");
@@ -12833,9 +12831,7 @@ flow_hw_allocate_actions(struct rte_eth_dev *dev,
 				  NULL, "fail to allocate actions");
 }
 
-/* TODO: remove dev if not used */
-static int flow_hw_apply(struct rte_eth_dev *dev __rte_unused,
-			 const struct rte_flow_item items[],
+static int flow_hw_apply(const struct rte_flow_item items[],
 			 struct mlx5dr_rule_action rule_actions[],
 			 struct rte_flow_hw *flow,
 			 struct rte_flow_error *error)
@@ -12901,16 +12897,7 @@ flow_hw_create_flow(struct rte_eth_dev *dev, enum mlx5_flow_type type,
 		.group = attr->group,
 		.priority = attr->priority,
 		.rss_level = 0,
-		/*
-		 * TODO: currently only mlx5_flow_lacp_miss rule is relevant:
-		 * action type=(enum rte_flow_action_type) MLX5_RTE_FLOW_ACTION_TYPE_DEFAULT_MISS.
-		 * I don't want to waist time going over all actions for this corner case.
-		 * Needs to use another preparation code to update this action flags.
-		 * if (action_type == (enum rte_flow_action_type)
-		 * MLX5_RTE_FLOW_ACTION_TYPE_DEFAULT_MISS)
-		 *     act_flags |= MLX5_FLOW_ACTION_DEFAULT_MISS;
-		 */
-		.act_flags = 0, /*TODO update*/
+		.act_flags = action_flags,
 		.tbl_type = 0,
 		};
 
@@ -12958,11 +12945,6 @@ flow_hw_create_flow(struct rte_eth_dev *dev, enum mlx5_flow_type type,
 	if (ret)
 		goto error;
 
-	/*
-	 * TODO: check regarding release: CT index is not saved per rule,
-	 * the index is in the conf of given action.
-	 */
-
 	/*
 	 * If the flow is external (from application) OR device is started,
 	 * OR mreg discover, then apply immediately.
@@ -12970,7 +12952,7 @@ flow_hw_create_flow(struct rte_eth_dev *dev, enum mlx5_flow_type type,
 	if (external || dev->data->dev_started ||
 	    (attr->group == MLX5_FLOW_MREG_CP_TABLE_GROUP &&
 	     attr->priority == MLX5_FLOW_LOWEST_PRIO_INDICATOR)) {
-		ret = flow_hw_apply(dev, items, hw_act.rule_acts, *flow, error);
+		ret = flow_hw_apply(items, hw_act.rule_acts, *flow, error);
 		if (ret)
 			goto error;
 	}
@@ -13012,7 +12994,7 @@ flow_hw_destroy(struct rte_eth_dev *dev, struct rte_flow_hw *flow)
 			DRV_LOG(ERR, "bwc rule destroy failed");
 	}
 	flow->operation_type = MLX5_FLOW_HW_FLOW_OP_TYPE_DESTROY;
-	/* TODO: notice this function does not handle shared/static actions. */
+	/* Notice this function does not handle shared/static actions. */
 	hw_cmpl_flow_update_or_destroy(dev, flow, 0, NULL);
 
 	/**
@@ -13108,6 +13090,10 @@ static uintptr_t flow_hw_list_create(struct rte_eth_dev *dev,
 	uint64_t item_flags = flow_hw_matching_item_flags_get(items);
 	uint64_t action_flags = flow_hw_action_flags_get(actions, error);
 
+	/*
+	 * TODO: add a call to flow_hw_validate function once it exist.
+	 * and update mlx5_flow_hw_drv_ops accordingly.
+	 */
 
 	if (action_flags & MLX5_FLOW_ACTION_RSS) {
 		const struct rte_flow_action_rss
-- 
2.21.0


^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH v4 0/4] non template pmd code changes
  2024-06-06 10:06   ` [PATCH v4 0/4] non template pmd code changes Maayan Kashani
                       ` (3 preceding siblings ...)
  2024-06-06 10:06     ` [PATCH v4 4/4] net/mlx5: clean up TODO comments Maayan Kashani
@ 2024-06-11 11:16     ` Raslan Darawsheh
  4 siblings, 0 replies; 15+ messages in thread
From: Raslan Darawsheh @ 2024-06-11 11:16 UTC (permalink / raw)
  To: Maayan Kashani, dev; +Cc: Dariusz Sosnowski

Hi,

From: Maayan Kashani <mkashani@nvidia.com>
Sent: Thursday, June 6, 2024 1:06 PM
To: dev@dpdk.org
Cc: Maayan Kashani; Dariusz Sosnowski; Raslan Darawsheh
Subject: [PATCH v4 0/4] non template pmd code changes

Code changes applied after review.

Maayan Kashani (4):
  net/mlx5: reorganize main structures
  net/mlx5: set modify header as shared action
  net/mlx5: set encap as shared action
  net/mlx5: clean up TODO comments

 drivers/net/mlx5/mlx5_flow.h    |  29 ++--
 drivers/net/mlx5/mlx5_flow_dv.c |  12 +-
 drivers/net/mlx5/mlx5_flow_hw.c | 234 +++++++++++++++++++-------------
 3 files changed, 162 insertions(+), 113 deletions(-)


Series applied to next-net-mlx,

Kindest regards,
Raslan Darawsheh


^ permalink raw reply	[flat|nested] 15+ messages in thread

end of thread, other threads:[~2024-06-11 11:16 UTC | newest]

Thread overview: 15+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2024-06-02 10:29 [PATCH 1/4] net/mlx5: reorganize main structures Maayan Kashani
2024-06-03  8:16 ` [PATCH v2 22/34] " Maayan Kashani
2024-06-03  8:16   ` [PATCH v2 23/34] net/mlx5: set modify header as shared action Maayan Kashani
2024-06-03  8:16   ` [PATCH v2 24/34] net/mlx5: set encap " Maayan Kashani
2024-06-03  8:16   ` [PATCH v2 25/34] net/mlx5: clean up TODO comments Maayan Kashani
2024-06-03 10:54 ` [PATCH v3 1/4] net/mlx5: reorganize main structures Maayan Kashani
2024-06-03 10:54   ` [PATCH v3 2/4] net/mlx5: set modify header as shared action Maayan Kashani
2024-06-03 10:54   ` [PATCH v3 3/4] net/mlx5: set encap " Maayan Kashani
2024-06-03 10:54   ` [PATCH v3 4/4] net/mlx5: clean up TODO comments Maayan Kashani
2024-06-06 10:06   ` [PATCH v4 0/4] non template pmd code changes Maayan Kashani
2024-06-06 10:06     ` [PATCH v4 1/4] net/mlx5: reorganize main structures Maayan Kashani
2024-06-06 10:06     ` [PATCH v4 2/4] net/mlx5: set modify header as shared action Maayan Kashani
2024-06-06 10:06     ` [PATCH v4 3/4] net/mlx5: set encap " Maayan Kashani
2024-06-06 10:06     ` [PATCH v4 4/4] net/mlx5: clean up TODO comments Maayan Kashani
2024-06-11 11:16     ` [PATCH v4 0/4] non template pmd code changes Raslan Darawsheh

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).