[dpdk-dev] [PATCH 0/2] net/mlx5: copy the item flags from prefix flow

[dpdk-dev] [PATCH 0/2] net/mlx5: copy the item flags from prefix flow

[Suanming Mou]

For flow split to several subflows, the match items maybe in the prefix
flow, and the actions are split to the suffix flow.

In this case, for the actions need the user defined match item will not
create correctly.

Copy the item layers flags to the suffix flow from prefix flow to fix
the issue.

This patch series should be applied after:
https://patches.dpdk.org/project/dpdk/list/?series=8613

Suanming Mou (2):
  net/mlx5: fix layer flags missing in metadata
  net/mlx5: fix lack of match information in meter

 drivers/net/mlx5/mlx5_flow.c    | 41 +++++++++++++++++++-------
 drivers/net/mlx5/mlx5_flow_dv.c | 64 +++++++++++++++++++++++++++++++----------
 2 files changed, 79 insertions(+), 26 deletions(-)

-- 
1.8.3.1

[dpdk-dev] [PATCH 1/2] net/mlx5: fix layer flags missing in metadata

[Suanming Mou]

Metadata suffix subflow inherits the RSS needed hash_fields from the
prefix subflow as the suffix subflow only has the tag match item unable
to generate the full original hash_fields for RSS action.

Unfortunately, hash_fields will only be generated if flow has RSS action.
So it means the prefix flow won't generate the hash_fields as the RSS
action has been split to the suffix flow.

Copy the layer flags from prefix subflow to suffix subflow to help the
suffix subflow to generate the correct hash_fields itself.

Fixes: 71e254bc0294 ("net/mlx5: split Rx flows to provide metadata copy")
Cc: viacheslavo@mellanox.com
Cc: stable@dpdk.org

Signed-off-by: Suanming Mou <suanmingm@mellanox.com>
---
 drivers/net/mlx5/mlx5_flow.c    | 27 +++++++++++++++++----------
 drivers/net/mlx5/mlx5_flow_dv.c |  2 +-
 2 files changed, 18 insertions(+), 11 deletions(-)

diff --git a/drivers/net/mlx5/mlx5_flow.c b/drivers/net/mlx5/mlx5_flow.c
index 2548201..2f57759 100644
--- a/drivers/net/mlx5/mlx5_flow.c
+++ b/drivers/net/mlx5/mlx5_flow.c
@@ -3406,6 +3406,8 @@ uint32_t mlx5_flow_adjust_priority(struct rte_eth_dev *dev, int32_t priority,
  *   Parent flow structure pointer.
  * @param[in, out] sub_flow
  *   Pointer to return the created subflow, may be NULL.
+ * @param[in] prefix_flow
+ *   Pointer to the created prefix subflow, may be NULL.
  * @param[in] attr
  *   Flow rule attributes.
  * @param[in] items
@@ -3423,6 +3425,7 @@ uint32_t mlx5_flow_adjust_priority(struct rte_eth_dev *dev, int32_t priority,
 flow_create_split_inner(struct rte_eth_dev *dev,
 			struct rte_flow *flow,
 			struct mlx5_flow **sub_flow,
+			struct mlx5_flow *prefix_flow,
 			const struct rte_flow_attr *attr,
 			const struct rte_flow_item items[],
 			const struct rte_flow_action actions[],
@@ -3437,6 +3440,8 @@ uint32_t mlx5_flow_adjust_priority(struct rte_eth_dev *dev, int32_t priority,
 	dev_flow->external = external;
 	/* Subflow object was created, we must include one in the list. */
 	LIST_INSERT_HEAD(&flow->dev_flows, dev_flow, next);
+	if (prefix_flow)
+		dev_flow->layers = prefix_flow->layers;
 	if (sub_flow)
 		*sub_flow = dev_flow;
 	return flow_drv_translate(dev, dev_flow, attr, items, actions, error);
@@ -3768,8 +3773,9 @@ uint32_t mlx5_flow_adjust_priority(struct rte_eth_dev *dev, int32_t priority,
 	if (!config->dv_flow_en ||
 	    config->dv_xmeta_en == MLX5_XMETA_MODE_LEGACY ||
 	    !mlx5_flow_ext_mreg_supported(dev))
-		return flow_create_split_inner(dev, flow, NULL, attr, items,
-					       actions, external, error);
+		return flow_create_split_inner(dev, flow, NULL, NULL,
+					       attr, items, actions, external,
+					       error);
 	actions_n = flow_parse_qrss_action(actions, &qrss);
 	if (qrss) {
 		/* Exclude hairpin flows from splitting. */
@@ -3850,9 +3856,9 @@ uint32_t mlx5_flow_adjust_priority(struct rte_eth_dev *dev, int32_t priority,
 			goto exit;
 	}
 	/* Add the unmodified original or prefix subflow. */
-	ret = flow_create_split_inner(dev, flow, &dev_flow, attr, items,
-				      ext_actions ? ext_actions : actions,
-				      external, error);
+	ret = flow_create_split_inner(dev, flow, &dev_flow, NULL, attr,
+				      items, ext_actions ? ext_actions :
+				      actions, external, error);
 	if (ret < 0)
 		goto exit;
 	MLX5_ASSERT(dev_flow);
@@ -3886,7 +3892,7 @@ uint32_t mlx5_flow_adjust_priority(struct rte_eth_dev *dev, int32_t priority,
 				.type = RTE_FLOW_ACTION_TYPE_END,
 			},
 		};
-		uint64_t hash_fields = dev_flow->hash_fields;
+		struct mlx5_flow *prefix_flow;
 
 		/*
 		 * Configure the tag item only if there is no meter subflow.
@@ -3911,16 +3917,16 @@ uint32_t mlx5_flow_adjust_priority(struct rte_eth_dev *dev, int32_t priority,
 				goto exit;
 			q_tag_spec.id = ret;
 		}
+		prefix_flow = dev_flow;
 		dev_flow = NULL;
 		/* Add suffix subflow to execute Q/RSS. */
-		ret = flow_create_split_inner(dev, flow, &dev_flow,
+		ret = flow_create_split_inner(dev, flow, &dev_flow, prefix_flow,
 					      &q_attr, mtr_sfx ? items :
 					      q_items, q_actions,
 					      external, error);
 		if (ret < 0)
 			goto exit;
 		MLX5_ASSERT(dev_flow);
-		dev_flow->hash_fields = hash_fields;
 	}
 
 exit:
@@ -4009,8 +4015,9 @@ uint32_t mlx5_flow_adjust_priority(struct rte_eth_dev *dev, int32_t priority,
 			goto exit;
 		}
 		/* Add the prefix subflow. */
-		ret = flow_create_split_inner(dev, flow, &dev_flow, attr, items,
-						  pre_actions, external, error);
+		ret = flow_create_split_inner(dev, flow, &dev_flow, NULL, attr,
+					      items, pre_actions, external,
+					      error);
 		if (ret) {
 			ret = -rte_errno;
 			goto exit;
diff --git a/drivers/net/mlx5/mlx5_flow_dv.c b/drivers/net/mlx5/mlx5_flow_dv.c
index d038f7b..c958fca 100644
--- a/drivers/net/mlx5/mlx5_flow_dv.c
+++ b/drivers/net/mlx5/mlx5_flow_dv.c
@@ -7813,7 +7813,7 @@ struct field_modify_info modify_tcp[] = {
 	MLX5_ASSERT(!flow_dv_check_valid_spec(matcher.mask.buf,
 					      dev_flow->dv.value.buf));
 #endif
-	dev_flow->layers = item_flags;
+	dev_flow->layers |= item_flags;
 	if (action_flags & MLX5_FLOW_ACTION_RSS)
 		flow_dv_hashfields_set(dev_flow);
 	/* Register matcher. */
-- 
1.8.3.1

[dpdk-dev] [PATCH 2/2] net/mlx5: fix lack of match information in meter

[Suanming Mou]

As meter flows are split into three subflows each, the prefix subflow
with meter action color the packet, the meter subflow filters out the
colored packets, the suffix subflow applies all the remaining actions
to the passed packets.

Currently, all the user defined items are matched in the prefix flow.
Flow id tag match item is the only item added to the meter suffix
subflow. Some of the remaining actions to be applied in the suffix
subflow require more information in the match item, or the suffix
subflow will not be created successfully.

Actions require the L3/L4 type in the match items as below:
RTE_FLOW_ACTION_TYPE_SET_TP_SRC
RTE_FLOW_ACTION_TYPE_SET_TP_DST
RTE_FLOW_ACTION_TYPE_DEC_TTL
RTE_FLOW_ACTION_TYPE_SET_TTL
RTE_FLOW_ACTION_TYPE_RSS
RTE_FLOW_ACTION_TYPE_QUEUE

Inherit the match item flags from meter prefix subflow to make actions
in suffix subflow get sufficient information.

Fixes: 9ea9b049a960 ("net/mlx5: split meter flow")
Cc: stable@dpdk.org

Signed-off-by: Suanming Mou <suanmingm@mellanox.com>
---
 drivers/net/mlx5/mlx5_flow.c    | 22 +++++++++++----
 drivers/net/mlx5/mlx5_flow_dv.c | 62 +++++++++++++++++++++++++++++++----------
 2 files changed, 65 insertions(+), 19 deletions(-)

diff --git a/drivers/net/mlx5/mlx5_flow.c b/drivers/net/mlx5/mlx5_flow.c
index 2f57759..f533f78 100644
--- a/drivers/net/mlx5/mlx5_flow.c
+++ b/drivers/net/mlx5/mlx5_flow.c
@@ -3440,8 +3440,18 @@ uint32_t mlx5_flow_adjust_priority(struct rte_eth_dev *dev, int32_t priority,
 	dev_flow->external = external;
 	/* Subflow object was created, we must include one in the list. */
 	LIST_INSERT_HEAD(&flow->dev_flows, dev_flow, next);
-	if (prefix_flow)
+	if (prefix_flow) {
+		/*
+		 * Some suffix subflow need the item flags and decap action
+		 * flag to know if actions in suffix flow should be applied
+		 * to new outer layer or not.
+		 * Action flag will be overwritten in tanslate after using.
+		 * No need to copy the hash_fields, as it will be generated
+		 * by RSS action with the item layer flags.
+		 */
 		dev_flow->layers = prefix_flow->layers;
+		dev_flow->actions = prefix_flow->actions;
+	}
 	if (sub_flow)
 		*sub_flow = dev_flow;
 	return flow_drv_translate(dev, dev_flow, attr, items, actions, error);
@@ -3737,6 +3747,8 @@ uint32_t mlx5_flow_adjust_priority(struct rte_eth_dev *dev, int32_t priority,
  *   Pointer to Ethernet device.
  * @param[in] flow
  *   Parent flow structure pointer.
+ * @param[in] prefix_flow
+ *   Prefix flow structure pointer.
  * @param[in] attr
  *   Flow rule attributes.
  * @param[in] items
@@ -3753,6 +3765,7 @@ uint32_t mlx5_flow_adjust_priority(struct rte_eth_dev *dev, int32_t priority,
 static int
 flow_create_split_metadata(struct rte_eth_dev *dev,
 			   struct rte_flow *flow,
+			   struct mlx5_flow *prefix_flow,
 			   const struct rte_flow_attr *attr,
 			   const struct rte_flow_item items[],
 			   const struct rte_flow_action actions[],
@@ -3773,7 +3786,7 @@ uint32_t mlx5_flow_adjust_priority(struct rte_eth_dev *dev, int32_t priority,
 	if (!config->dv_flow_en ||
 	    config->dv_xmeta_en == MLX5_XMETA_MODE_LEGACY ||
 	    !mlx5_flow_ext_mreg_supported(dev))
-		return flow_create_split_inner(dev, flow, NULL, NULL,
+		return flow_create_split_inner(dev, flow, NULL, prefix_flow,
 					       attr, items, actions, external,
 					       error);
 	actions_n = flow_parse_qrss_action(actions, &qrss);
@@ -3856,7 +3869,7 @@ uint32_t mlx5_flow_adjust_priority(struct rte_eth_dev *dev, int32_t priority,
 			goto exit;
 	}
 	/* Add the unmodified original or prefix subflow. */
-	ret = flow_create_split_inner(dev, flow, &dev_flow, NULL, attr,
+	ret = flow_create_split_inner(dev, flow, &dev_flow, prefix_flow, attr,
 				      items, ext_actions ? ext_actions :
 				      actions, external, error);
 	if (ret < 0)
@@ -3892,7 +3905,6 @@ uint32_t mlx5_flow_adjust_priority(struct rte_eth_dev *dev, int32_t priority,
 				.type = RTE_FLOW_ACTION_TYPE_END,
 			},
 		};
-		struct mlx5_flow *prefix_flow;
 
 		/*
 		 * Configure the tag item only if there is no meter subflow.
@@ -4052,7 +4064,7 @@ uint32_t mlx5_flow_adjust_priority(struct rte_eth_dev *dev, int32_t priority,
 				 MLX5_FLOW_TABLE_LEVEL_SUFFIX;
 	}
 	/* Add the prefix subflow. */
-	ret = flow_create_split_metadata(dev, flow, &sfx_attr,
+	ret = flow_create_split_metadata(dev, flow, dev_flow, &sfx_attr,
 					 sfx_items ? sfx_items : items,
 					 sfx_actions ? sfx_actions : actions,
 					 external, error);
diff --git a/drivers/net/mlx5/mlx5_flow_dv.c b/drivers/net/mlx5/mlx5_flow_dv.c
index c958fca..359a037 100644
--- a/drivers/net/mlx5/mlx5_flow_dv.c
+++ b/drivers/net/mlx5/mlx5_flow_dv.c
@@ -85,16 +85,44 @@
  *   Pointer to item specification.
  * @param[out] attr
  *   Pointer to flow attributes structure.
+ * @param[in] dev_flow
+ *   Pointer to the sub flow.
  * @param[in] tunnel_decap
  *   Whether action is after tunnel decapsulation.
  */
 static void
 flow_dv_attr_init(const struct rte_flow_item *item, union flow_dv_attr *attr,
-		  bool tunnel_decap)
+		  struct mlx5_flow *dev_flow, bool tunnel_decap)
 {
+	if (dev_flow->layers) {
+		if (dev_flow->actions & MLX5_FLOW_ACTION_DECAP) {
+			if (dev_flow->layers & MLX5_FLOW_LAYER_INNER_L3_IPV4)
+				attr->ipv4 = 1;
+			else if (dev_flow->layers &
+				 MLX5_FLOW_LAYER_INNER_L3_IPV6)
+				attr->ipv6 = 1;
+			if (dev_flow->layers & MLX5_FLOW_LAYER_INNER_L4_TCP)
+				attr->tcp = 1;
+			else if (dev_flow->layers &
+				 MLX5_FLOW_LAYER_INNER_L4_UDP)
+				attr->udp = 1;
+		} else {
+			if (dev_flow->layers & MLX5_FLOW_LAYER_OUTER_L3_IPV4)
+				attr->ipv4 = 1;
+			else if (dev_flow->layers &
+				 MLX5_FLOW_LAYER_OUTER_L3_IPV6)
+				attr->ipv6 = 1;
+			if (dev_flow->layers & MLX5_FLOW_LAYER_OUTER_L4_TCP)
+				attr->tcp = 1;
+			else if (dev_flow->layers &
+				 MLX5_FLOW_LAYER_OUTER_L4_UDP)
+				attr->udp = 1;
+		}
+		attr->valid = 1;
+		return;
+	}
 	for (; item->type != RTE_FLOW_ITEM_TYPE_END; item++) {
 		uint8_t next_protocol = 0xff;
-
 		switch (item->type) {
 		case RTE_FLOW_ITEM_TYPE_GRE:
 		case RTE_FLOW_ITEM_TYPE_NVGRE:
@@ -640,6 +668,8 @@ struct field_modify_info modify_tcp[] = {
  *   Pointer to rte_flow_item objects list.
  * @param[in] attr
  *   Pointer to flow attributes structure.
+ * @param[in] dev_flow
+ *   Pointer to the sub flow.
  * @param[in] tunnel_decap
  *   Whether action is after tunnel decapsulation.
  * @param[out] error
@@ -653,8 +683,8 @@ struct field_modify_info modify_tcp[] = {
 			(struct mlx5_flow_dv_modify_hdr_resource *resource,
 			 const struct rte_flow_action *action,
 			 const struct rte_flow_item *items,
-			 union flow_dv_attr *attr, bool tunnel_decap,
-			 struct rte_flow_error *error)
+			 union flow_dv_attr *attr, struct mlx5_flow *dev_flow,
+			 bool tunnel_decap, struct rte_flow_error *error)
 {
 	const struct rte_flow_action_set_tp *conf =
 		(const struct rte_flow_action_set_tp *)(action->conf);
@@ -666,7 +696,7 @@ struct field_modify_info modify_tcp[] = {
 	struct field_modify_info *field;
 
 	if (!attr->valid)
-		flow_dv_attr_init(items, attr, tunnel_decap);
+		flow_dv_attr_init(items, attr, dev_flow, tunnel_decap);
 	if (attr->udp) {
 		memset(&udp, 0, sizeof(udp));
 		memset(&udp_mask, 0, sizeof(udp_mask));
@@ -716,6 +746,8 @@ struct field_modify_info modify_tcp[] = {
  *   Pointer to rte_flow_item objects list.
  * @param[in] attr
  *   Pointer to flow attributes structure.
+ * @param[in] dev_flow
+ *   Pointer to the sub flow.
  * @param[in] tunnel_decap
  *   Whether action is after tunnel decapsulation.
  * @param[out] error
@@ -729,8 +761,8 @@ struct field_modify_info modify_tcp[] = {
 			(struct mlx5_flow_dv_modify_hdr_resource *resource,
 			 const struct rte_flow_action *action,
 			 const struct rte_flow_item *items,
-			 union flow_dv_attr *attr, bool tunnel_decap,
-			 struct rte_flow_error *error)
+			 union flow_dv_attr *attr, struct mlx5_flow *dev_flow,
+			 bool tunnel_decap, struct rte_flow_error *error)
 {
 	const struct rte_flow_action_set_ttl *conf =
 		(const struct rte_flow_action_set_ttl *)(action->conf);
@@ -742,7 +774,7 @@ struct field_modify_info modify_tcp[] = {
 	struct field_modify_info *field;
 
 	if (!attr->valid)
-		flow_dv_attr_init(items, attr, tunnel_decap);
+		flow_dv_attr_init(items, attr, dev_flow, tunnel_decap);
 	if (attr->ipv4) {
 		memset(&ipv4, 0, sizeof(ipv4));
 		memset(&ipv4_mask, 0, sizeof(ipv4_mask));
@@ -778,6 +810,8 @@ struct field_modify_info modify_tcp[] = {
  *   Pointer to rte_flow_item objects list.
  * @param[in] attr
  *   Pointer to flow attributes structure.
+ * @param[in] dev_flow
+ *   Pointer to the sub flow.
  * @param[in] tunnel_decap
  *   Whether action is after tunnel decapsulation.
  * @param[out] error
@@ -790,8 +824,8 @@ struct field_modify_info modify_tcp[] = {
 flow_dv_convert_action_modify_dec_ttl
 			(struct mlx5_flow_dv_modify_hdr_resource *resource,
 			 const struct rte_flow_item *items,
-			 union flow_dv_attr *attr, bool tunnel_decap,
-			 struct rte_flow_error *error)
+			 union flow_dv_attr *attr, struct mlx5_flow *dev_flow,
+			 bool tunnel_decap, struct rte_flow_error *error)
 {
 	struct rte_flow_item item;
 	struct rte_flow_item_ipv4 ipv4;
@@ -801,7 +835,7 @@ struct field_modify_info modify_tcp[] = {
 	struct field_modify_info *field;
 
 	if (!attr->valid)
-		flow_dv_attr_init(items, attr, tunnel_decap);
+		flow_dv_attr_init(items, attr, dev_flow, tunnel_decap);
 	if (attr->ipv4) {
 		memset(&ipv4, 0, sizeof(ipv4));
 		memset(&ipv4_mask, 0, sizeof(ipv4_mask));
@@ -7505,7 +7539,7 @@ struct field_modify_info modify_tcp[] = {
 		case RTE_FLOW_ACTION_TYPE_SET_TP_DST:
 			if (flow_dv_convert_action_modify_tp
 					(mhdr_res, actions, items,
-					 &flow_attr, !!(action_flags &
+					 &flow_attr, dev_flow, !!(action_flags &
 					 MLX5_FLOW_ACTION_DECAP), error))
 				return -rte_errno;
 			action_flags |= actions->type ==
@@ -7515,7 +7549,7 @@ struct field_modify_info modify_tcp[] = {
 			break;
 		case RTE_FLOW_ACTION_TYPE_DEC_TTL:
 			if (flow_dv_convert_action_modify_dec_ttl
-					(mhdr_res, items, &flow_attr,
+					(mhdr_res, items, &flow_attr, dev_flow,
 					 !!(action_flags &
 					 MLX5_FLOW_ACTION_DECAP), error))
 				return -rte_errno;
@@ -7524,7 +7558,7 @@ struct field_modify_info modify_tcp[] = {
 		case RTE_FLOW_ACTION_TYPE_SET_TTL:
 			if (flow_dv_convert_action_modify_ttl
 					(mhdr_res, actions, items, &flow_attr,
-					 !!(action_flags &
+					 dev_flow, !!(action_flags &
 					 MLX5_FLOW_ACTION_DECAP), error))
 				return -rte_errno;
 			action_flags |= MLX5_FLOW_ACTION_SET_TTL;
-- 
1.8.3.1

Re: [dpdk-dev] [PATCH 2/2] net/mlx5: fix lack of match information in meter

[Matan Azrad]

Hi Mou

One comment inline.

From: Suanming Mou
> As meter flows are split into three subflows each, the prefix subflow with
> meter action color the packet, the meter subflow filters out the colored
> packets, the suffix subflow applies all the remaining actions to the passed
> packets.
> 
> Currently, all the user defined items are matched in the prefix flow.
> Flow id tag match item is the only item added to the meter suffix subflow.
> Some of the remaining actions to be applied in the suffix subflow require
> more information in the match item, or the suffix subflow will not be created
> successfully.
> 
> Actions require the L3/L4 type in the match items as below:
> RTE_FLOW_ACTION_TYPE_SET_TP_SRC
> RTE_FLOW_ACTION_TYPE_SET_TP_DST
> RTE_FLOW_ACTION_TYPE_DEC_TTL
> RTE_FLOW_ACTION_TYPE_SET_TTL
> RTE_FLOW_ACTION_TYPE_RSS
> RTE_FLOW_ACTION_TYPE_QUEUE
> 
> Inherit the match item flags from meter prefix subflow to make actions in
> suffix subflow get sufficient information.
> 
> Fixes: 9ea9b049a960 ("net/mlx5: split meter flow")
> Cc: stable@dpdk.org
> 
> Signed-off-by: Suanming Mou <suanmingm@mellanox.com>
> ---
>  drivers/net/mlx5/mlx5_flow.c    | 22 +++++++++++----
>  drivers/net/mlx5/mlx5_flow_dv.c | 62
> +++++++++++++++++++++++++++++++----------
>  2 files changed, 65 insertions(+), 19 deletions(-)
> 
> diff --git a/drivers/net/mlx5/mlx5_flow.c b/drivers/net/mlx5/mlx5_flow.c
> index 2f57759..f533f78 100644
> --- a/drivers/net/mlx5/mlx5_flow.c
> +++ b/drivers/net/mlx5/mlx5_flow.c
> @@ -3440,8 +3440,18 @@ uint32_t mlx5_flow_adjust_priority(struct
> rte_eth_dev *dev, int32_t priority,
>  	dev_flow->external = external;
>  	/* Subflow object was created, we must include one in the list. */
>  	LIST_INSERT_HEAD(&flow->dev_flows, dev_flow, next);
> -	if (prefix_flow)
> +	if (prefix_flow) {
> +		/*
> +		 * Some suffix subflow need the item flags and decap action
> +		 * flag to know if actions in suffix flow should be applied
> +		 * to new outer layer or not.
> +		 * Action flag will be overwritten in tanslate after using.
> +		 * No need to copy the hash_fields, as it will be generated
> +		 * by RSS action with the item layer flags.
> +		 */
>  		dev_flow->layers = prefix_flow->layers;
> +		dev_flow->actions = prefix_flow->actions;

I don't think it is good to move the actions too.
You just need to remove the outer layers here if you see decap, no?
Now we have only one needed param, it is prefix layers only.
please change all the commits to move to prefix_layers instead prefix_flow.
in case of decap:
Rss need the inner layers only and modify_actions need the inner layers only.
so you can just zero the outer layers.
please add comment on it in translate - somethink like "layers can be initiated by the user according to the prefix sub-flow in case of meter and metadata splitters".

> +	}
>  	if (sub_flow)
>  		*sub_flow = dev_flow;
>  	return flow_drv_translate(dev, dev_flow, attr, items, actions, error);
> @@ -3737,6 +3747,8 @@ uint32_t mlx5_flow_adjust_priority(struct
> rte_eth_dev *dev, int32_t priority,
>   *   Pointer to Ethernet device.
>   * @param[in] flow
>   *   Parent flow structure pointer.
> + * @param[in] prefix_flow
> + *   Prefix flow structure pointer.
>   * @param[in] attr
>   *   Flow rule attributes.
>   * @param[in] items
> @@ -3753,6 +3765,7 @@ uint32_t mlx5_flow_adjust_priority(struct
> rte_eth_dev *dev, int32_t priority,  static int
> flow_create_split_metadata(struct rte_eth_dev *dev,
>  			   struct rte_flow *flow,
> +			   struct mlx5_flow *prefix_flow,
>  			   const struct rte_flow_attr *attr,
>  			   const struct rte_flow_item items[],
>  			   const struct rte_flow_action actions[], @@ -3773,7
> +3786,7 @@ uint32_t mlx5_flow_adjust_priority(struct rte_eth_dev *dev,
> int32_t priority,
>  	if (!config->dv_flow_en ||
>  	    config->dv_xmeta_en == MLX5_XMETA_MODE_LEGACY ||
>  	    !mlx5_flow_ext_mreg_supported(dev))
> -		return flow_create_split_inner(dev, flow, NULL, NULL,
> +		return flow_create_split_inner(dev, flow, NULL, prefix_flow,
>  					       attr, items, actions, external,
>  					       error);
>  	actions_n = flow_parse_qrss_action(actions, &qrss); @@ -3856,7
> +3869,7 @@ uint32_t mlx5_flow_adjust_priority(struct rte_eth_dev *dev,
> int32_t priority,
>  			goto exit;
>  	}
>  	/* Add the unmodified original or prefix subflow. */
> -	ret = flow_create_split_inner(dev, flow, &dev_flow, NULL, attr,
> +	ret = flow_create_split_inner(dev, flow, &dev_flow, prefix_flow,
> attr,
>  				      items, ext_actions ? ext_actions :
>  				      actions, external, error);
>  	if (ret < 0)
> @@ -3892,7 +3905,6 @@ uint32_t mlx5_flow_adjust_priority(struct
> rte_eth_dev *dev, int32_t priority,
>  				.type = RTE_FLOW_ACTION_TYPE_END,
>  			},
>  		};
> -		struct mlx5_flow *prefix_flow;
> 
>  		/*
>  		 * Configure the tag item only if there is no meter subflow.
> @@ -4052,7 +4064,7 @@ uint32_t mlx5_flow_adjust_priority(struct
> rte_eth_dev *dev, int32_t priority,
>  				 MLX5_FLOW_TABLE_LEVEL_SUFFIX;
>  	}
>  	/* Add the prefix subflow. */
> -	ret = flow_create_split_metadata(dev, flow, &sfx_attr,
> +	ret = flow_create_split_metadata(dev, flow, dev_flow, &sfx_attr,
>  					 sfx_items ? sfx_items : items,
>  					 sfx_actions ? sfx_actions : actions,
>  					 external, error);
> diff --git a/drivers/net/mlx5/mlx5_flow_dv.c
> b/drivers/net/mlx5/mlx5_flow_dv.c index c958fca..359a037 100644
> --- a/drivers/net/mlx5/mlx5_flow_dv.c
> +++ b/drivers/net/mlx5/mlx5_flow_dv.c
> @@ -85,16 +85,44 @@
>   *   Pointer to item specification.
>   * @param[out] attr
>   *   Pointer to flow attributes structure.
> + * @param[in] dev_flow
> + *   Pointer to the sub flow.
>   * @param[in] tunnel_decap
>   *   Whether action is after tunnel decapsulation.
>   */
>  static void
>  flow_dv_attr_init(const struct rte_flow_item *item, union flow_dv_attr
> *attr,
> -		  bool tunnel_decap)
> +		  struct mlx5_flow *dev_flow, bool tunnel_decap)
>  {
> +	if (dev_flow->layers) {
> +		if (dev_flow->actions & MLX5_FLOW_ACTION_DECAP) {
> +			if (dev_flow->layers &
> MLX5_FLOW_LAYER_INNER_L3_IPV4)
> +				attr->ipv4 = 1;
> +			else if (dev_flow->layers &
> +				 MLX5_FLOW_LAYER_INNER_L3_IPV6)
> +				attr->ipv6 = 1;
> +			if (dev_flow->layers &
> MLX5_FLOW_LAYER_INNER_L4_TCP)
> +				attr->tcp = 1;
> +			else if (dev_flow->layers &
> +				 MLX5_FLOW_LAYER_INNER_L4_UDP)
> +				attr->udp = 1;
> +		} else {
> +			if (dev_flow->layers &
> MLX5_FLOW_LAYER_OUTER_L3_IPV4)
> +				attr->ipv4 = 1;
> +			else if (dev_flow->layers &
> +				 MLX5_FLOW_LAYER_OUTER_L3_IPV6)
> +				attr->ipv6 = 1;
> +			if (dev_flow->layers &
> MLX5_FLOW_LAYER_OUTER_L4_TCP)
> +				attr->tcp = 1;
> +			else if (dev_flow->layers &
> +				 MLX5_FLOW_LAYER_OUTER_L4_UDP)
> +				attr->udp = 1;
> +		}
> +		attr->valid = 1;
> +		return;
> +	}
>  	for (; item->type != RTE_FLOW_ITEM_TYPE_END; item++) {
>  		uint8_t next_protocol = 0xff;
> -
>  		switch (item->type) {
>  		case RTE_FLOW_ITEM_TYPE_GRE:
>  		case RTE_FLOW_ITEM_TYPE_NVGRE:
> @@ -640,6 +668,8 @@ struct field_modify_info modify_tcp[] = {
>   *   Pointer to rte_flow_item objects list.
>   * @param[in] attr
>   *   Pointer to flow attributes structure.
> + * @param[in] dev_flow
> + *   Pointer to the sub flow.
>   * @param[in] tunnel_decap
>   *   Whether action is after tunnel decapsulation.
>   * @param[out] error
> @@ -653,8 +683,8 @@ struct field_modify_info modify_tcp[] = {
>  			(struct mlx5_flow_dv_modify_hdr_resource
> *resource,
>  			 const struct rte_flow_action *action,
>  			 const struct rte_flow_item *items,
> -			 union flow_dv_attr *attr, bool tunnel_decap,
> -			 struct rte_flow_error *error)
> +			 union flow_dv_attr *attr, struct mlx5_flow
> *dev_flow,
> +			 bool tunnel_decap, struct rte_flow_error *error)
>  {
>  	const struct rte_flow_action_set_tp *conf =
>  		(const struct rte_flow_action_set_tp *)(action->conf); @@ -
> 666,7 +696,7 @@ struct field_modify_info modify_tcp[] = {
>  	struct field_modify_info *field;
> 
>  	if (!attr->valid)
> -		flow_dv_attr_init(items, attr, tunnel_decap);
> +		flow_dv_attr_init(items, attr, dev_flow, tunnel_decap);
>  	if (attr->udp) {
>  		memset(&udp, 0, sizeof(udp));
>  		memset(&udp_mask, 0, sizeof(udp_mask)); @@ -716,6
> +746,8 @@ struct field_modify_info modify_tcp[] = {
>   *   Pointer to rte_flow_item objects list.
>   * @param[in] attr
>   *   Pointer to flow attributes structure.
> + * @param[in] dev_flow
> + *   Pointer to the sub flow.
>   * @param[in] tunnel_decap
>   *   Whether action is after tunnel decapsulation.
>   * @param[out] error
> @@ -729,8 +761,8 @@ struct field_modify_info modify_tcp[] = {
>  			(struct mlx5_flow_dv_modify_hdr_resource
> *resource,
>  			 const struct rte_flow_action *action,
>  			 const struct rte_flow_item *items,
> -			 union flow_dv_attr *attr, bool tunnel_decap,
> -			 struct rte_flow_error *error)
> +			 union flow_dv_attr *attr, struct mlx5_flow
> *dev_flow,
> +			 bool tunnel_decap, struct rte_flow_error *error)
>  {
>  	const struct rte_flow_action_set_ttl *conf =
>  		(const struct rte_flow_action_set_ttl *)(action->conf); @@ -
> 742,7 +774,7 @@ struct field_modify_info modify_tcp[] = {
>  	struct field_modify_info *field;
> 
>  	if (!attr->valid)
> -		flow_dv_attr_init(items, attr, tunnel_decap);
> +		flow_dv_attr_init(items, attr, dev_flow, tunnel_decap);
>  	if (attr->ipv4) {
>  		memset(&ipv4, 0, sizeof(ipv4));
>  		memset(&ipv4_mask, 0, sizeof(ipv4_mask)); @@ -778,6
> +810,8 @@ struct field_modify_info modify_tcp[] = {
>   *   Pointer to rte_flow_item objects list.
>   * @param[in] attr
>   *   Pointer to flow attributes structure.
> + * @param[in] dev_flow
> + *   Pointer to the sub flow.
>   * @param[in] tunnel_decap
>   *   Whether action is after tunnel decapsulation.
>   * @param[out] error
> @@ -790,8 +824,8 @@ struct field_modify_info modify_tcp[] = {
> flow_dv_convert_action_modify_dec_ttl
>  			(struct mlx5_flow_dv_modify_hdr_resource
> *resource,
>  			 const struct rte_flow_item *items,
> -			 union flow_dv_attr *attr, bool tunnel_decap,
> -			 struct rte_flow_error *error)
> +			 union flow_dv_attr *attr, struct mlx5_flow
> *dev_flow,
> +			 bool tunnel_decap, struct rte_flow_error *error)
>  {
>  	struct rte_flow_item item;
>  	struct rte_flow_item_ipv4 ipv4;
> @@ -801,7 +835,7 @@ struct field_modify_info modify_tcp[] = {
>  	struct field_modify_info *field;
> 
>  	if (!attr->valid)
> -		flow_dv_attr_init(items, attr, tunnel_decap);
> +		flow_dv_attr_init(items, attr, dev_flow, tunnel_decap);
>  	if (attr->ipv4) {
>  		memset(&ipv4, 0, sizeof(ipv4));
>  		memset(&ipv4_mask, 0, sizeof(ipv4_mask)); @@ -7505,7
> +7539,7 @@ struct field_modify_info modify_tcp[] = {
>  		case RTE_FLOW_ACTION_TYPE_SET_TP_DST:
>  			if (flow_dv_convert_action_modify_tp
>  					(mhdr_res, actions, items,
> -					 &flow_attr, !!(action_flags &
> +					 &flow_attr, dev_flow, !!(action_flags
> &
>  					 MLX5_FLOW_ACTION_DECAP),
> error))
>  				return -rte_errno;
>  			action_flags |= actions->type ==
> @@ -7515,7 +7549,7 @@ struct field_modify_info modify_tcp[] = {
>  			break;
>  		case RTE_FLOW_ACTION_TYPE_DEC_TTL:
>  			if (flow_dv_convert_action_modify_dec_ttl
> -					(mhdr_res, items, &flow_attr,
> +					(mhdr_res, items, &flow_attr,
> dev_flow,
>  					 !!(action_flags &
>  					 MLX5_FLOW_ACTION_DECAP),
> error))
>  				return -rte_errno;
> @@ -7524,7 +7558,7 @@ struct field_modify_info modify_tcp[] = {
>  		case RTE_FLOW_ACTION_TYPE_SET_TTL:
>  			if (flow_dv_convert_action_modify_ttl
>  					(mhdr_res, actions, items,
> &flow_attr,
> -					 !!(action_flags &
> +					 dev_flow, !!(action_flags &
>  					 MLX5_FLOW_ACTION_DECAP),
> error))
>  				return -rte_errno;
>  			action_flags |= MLX5_FLOW_ACTION_SET_TTL;
> --
> 1.8.3.1

Re: [dpdk-dev] [PATCH 2/2] net/mlx5: fix lack of match information in meter

[Suanming Mou]

> -----Original Message-----
> From: Matan Azrad <matan@mellanox.com>
> Sent: Wednesday, February 19, 2020 11:31 PM
> To: Suanming Mou <suanmingm@mellanox.com>; Slava Ovsiienko
> <viacheslavo@mellanox.com>
> Cc: dev@dpdk.org; Raslan Darawsheh <rasland@mellanox.com>;
> stable@dpdk.org
> Subject: RE: [PATCH 2/2] net/mlx5: fix lack of match information in meter
> 
> 
> Hi Mou
> 
> One comment inline.
> 
> From: Suanming Mou
> > As meter flows are split into three subflows each, the prefix subflow
> > with meter action color the packet, the meter subflow filters out the
> > colored packets, the suffix subflow applies all the remaining actions
> > to the passed packets.
> >
> > Currently, all the user defined items are matched in the prefix flow.
> > Flow id tag match item is the only item added to the meter suffix subflow.
> > Some of the remaining actions to be applied in the suffix subflow
> > require more information in the match item, or the suffix subflow will
> > not be created successfully.
> >
> > Actions require the L3/L4 type in the match items as below:
> > RTE_FLOW_ACTION_TYPE_SET_TP_SRC
> > RTE_FLOW_ACTION_TYPE_SET_TP_DST
> > RTE_FLOW_ACTION_TYPE_DEC_TTL
> > RTE_FLOW_ACTION_TYPE_SET_TTL
> > RTE_FLOW_ACTION_TYPE_RSS
> > RTE_FLOW_ACTION_TYPE_QUEUE
> >
> > Inherit the match item flags from meter prefix subflow to make actions
> > in suffix subflow get sufficient information.
> >
> > Fixes: 9ea9b049a960 ("net/mlx5: split meter flow")
> > Cc: stable@dpdk.org
> >
> > Signed-off-by: Suanming Mou <suanmingm@mellanox.com>
> > ---
> >  drivers/net/mlx5/mlx5_flow.c    | 22 +++++++++++----
> >  drivers/net/mlx5/mlx5_flow_dv.c | 62
> > +++++++++++++++++++++++++++++++----------
> >  2 files changed, 65 insertions(+), 19 deletions(-)
> >
> > diff --git a/drivers/net/mlx5/mlx5_flow.c
> > b/drivers/net/mlx5/mlx5_flow.c index 2f57759..f533f78 100644
> > --- a/drivers/net/mlx5/mlx5_flow.c
> > +++ b/drivers/net/mlx5/mlx5_flow.c
> > @@ -3440,8 +3440,18 @@ uint32_t mlx5_flow_adjust_priority(struct
> > rte_eth_dev *dev, int32_t priority,
> >  	dev_flow->external = external;
> >  	/* Subflow object was created, we must include one in the list. */
> >  	LIST_INSERT_HEAD(&flow->dev_flows, dev_flow, next);
> > -	if (prefix_flow)
> > +	if (prefix_flow) {
> > +		/*
> > +		 * Some suffix subflow need the item flags and decap action
> > +		 * flag to know if actions in suffix flow should be applied
> > +		 * to new outer layer or not.
> > +		 * Action flag will be overwritten in tanslate after using.
> > +		 * No need to copy the hash_fields, as it will be generated
> > +		 * by RSS action with the item layer flags.
> > +		 */
> >  		dev_flow->layers = prefix_flow->layers;
> > +		dev_flow->actions = prefix_flow->actions;
> 
> I don't think it is good to move the actions too.
> You just need to remove the outer layers here if you see decap, no?
> Now we have only one needed param, it is prefix layers only.
> please change all the commits to move to prefix_layers instead prefix_flow.
> in case of decap:
> Rss need the inner layers only and modify_actions need the inner layers only.
> so you can just zero the outer layers.
> please add comment on it in translate - somethink like "layers can be initiated
> by the user according to the prefix sub-flow in case of meter and metadata
> splitters".
Good suggestion, will update the V2 version.

> 
> > +	}
> >  	if (sub_flow)
> >  		*sub_flow = dev_flow;
> >  	return flow_drv_translate(dev, dev_flow, attr, items, actions,
> > error); @@ -3737,6 +3747,8 @@ uint32_t
> > mlx5_flow_adjust_priority(struct rte_eth_dev *dev, int32_t priority,
> >   *   Pointer to Ethernet device.
> >   * @param[in] flow
> >   *   Parent flow structure pointer.
> > + * @param[in] prefix_flow
> > + *   Prefix flow structure pointer.
> >   * @param[in] attr
> >   *   Flow rule attributes.
> >   * @param[in] items
> > @@ -3753,6 +3765,7 @@ uint32_t mlx5_flow_adjust_priority(struct
> > rte_eth_dev *dev, int32_t priority,  static int
> > flow_create_split_metadata(struct rte_eth_dev *dev,
> >  			   struct rte_flow *flow,
> > +			   struct mlx5_flow *prefix_flow,
> >  			   const struct rte_flow_attr *attr,
> >  			   const struct rte_flow_item items[],
> >  			   const struct rte_flow_action actions[], @@ -3773,7
> > +3786,7 @@ uint32_t mlx5_flow_adjust_priority(struct rte_eth_dev *dev,
> > int32_t priority,
> >  	if (!config->dv_flow_en ||
> >  	    config->dv_xmeta_en == MLX5_XMETA_MODE_LEGACY ||
> >  	    !mlx5_flow_ext_mreg_supported(dev))
> > -		return flow_create_split_inner(dev, flow, NULL, NULL,
> > +		return flow_create_split_inner(dev, flow, NULL, prefix_flow,
> >  					       attr, items, actions, external,
> >  					       error);
> >  	actions_n = flow_parse_qrss_action(actions, &qrss); @@ -3856,7
> > +3869,7 @@ uint32_t mlx5_flow_adjust_priority(struct rte_eth_dev *dev,
> > int32_t priority,
> >  			goto exit;
> >  	}
> >  	/* Add the unmodified original or prefix subflow. */
> > -	ret = flow_create_split_inner(dev, flow, &dev_flow, NULL, attr,
> > +	ret = flow_create_split_inner(dev, flow, &dev_flow, prefix_flow,
> > attr,
> >  				      items, ext_actions ? ext_actions :
> >  				      actions, external, error);
> >  	if (ret < 0)
> > @@ -3892,7 +3905,6 @@ uint32_t mlx5_flow_adjust_priority(struct
> > rte_eth_dev *dev, int32_t priority,
> >  				.type = RTE_FLOW_ACTION_TYPE_END,
> >  			},
> >  		};
> > -		struct mlx5_flow *prefix_flow;
> >
> >  		/*
> >  		 * Configure the tag item only if there is no meter subflow.
> > @@ -4052,7 +4064,7 @@ uint32_t mlx5_flow_adjust_priority(struct
> > rte_eth_dev *dev, int32_t priority,
> >  				 MLX5_FLOW_TABLE_LEVEL_SUFFIX;
> >  	}
> >  	/* Add the prefix subflow. */
> > -	ret = flow_create_split_metadata(dev, flow, &sfx_attr,
> > +	ret = flow_create_split_metadata(dev, flow, dev_flow, &sfx_attr,
> >  					 sfx_items ? sfx_items : items,
> >  					 sfx_actions ? sfx_actions : actions,
> >  					 external, error);
> > diff --git a/drivers/net/mlx5/mlx5_flow_dv.c
> > b/drivers/net/mlx5/mlx5_flow_dv.c index c958fca..359a037 100644
> > --- a/drivers/net/mlx5/mlx5_flow_dv.c
> > +++ b/drivers/net/mlx5/mlx5_flow_dv.c
> > @@ -85,16 +85,44 @@
> >   *   Pointer to item specification.
> >   * @param[out] attr
> >   *   Pointer to flow attributes structure.
> > + * @param[in] dev_flow
> > + *   Pointer to the sub flow.
> >   * @param[in] tunnel_decap
> >   *   Whether action is after tunnel decapsulation.
> >   */
> >  static void
> >  flow_dv_attr_init(const struct rte_flow_item *item, union
> > flow_dv_attr *attr,
> > -		  bool tunnel_decap)
> > +		  struct mlx5_flow *dev_flow, bool tunnel_decap)
> >  {
> > +	if (dev_flow->layers) {
> > +		if (dev_flow->actions & MLX5_FLOW_ACTION_DECAP) {
> > +			if (dev_flow->layers &
> > MLX5_FLOW_LAYER_INNER_L3_IPV4)
> > +				attr->ipv4 = 1;
> > +			else if (dev_flow->layers &
> > +				 MLX5_FLOW_LAYER_INNER_L3_IPV6)
> > +				attr->ipv6 = 1;
> > +			if (dev_flow->layers &
> > MLX5_FLOW_LAYER_INNER_L4_TCP)
> > +				attr->tcp = 1;
> > +			else if (dev_flow->layers &
> > +				 MLX5_FLOW_LAYER_INNER_L4_UDP)
> > +				attr->udp = 1;
> > +		} else {
> > +			if (dev_flow->layers &
> > MLX5_FLOW_LAYER_OUTER_L3_IPV4)
> > +				attr->ipv4 = 1;
> > +			else if (dev_flow->layers &
> > +				 MLX5_FLOW_LAYER_OUTER_L3_IPV6)
> > +				attr->ipv6 = 1;
> > +			if (dev_flow->layers &
> > MLX5_FLOW_LAYER_OUTER_L4_TCP)
> > +				attr->tcp = 1;
> > +			else if (dev_flow->layers &
> > +				 MLX5_FLOW_LAYER_OUTER_L4_UDP)
> > +				attr->udp = 1;
> > +		}
> > +		attr->valid = 1;
> > +		return;
> > +	}
> >  	for (; item->type != RTE_FLOW_ITEM_TYPE_END; item++) {
> >  		uint8_t next_protocol = 0xff;
> > -
> >  		switch (item->type) {
> >  		case RTE_FLOW_ITEM_TYPE_GRE:
> >  		case RTE_FLOW_ITEM_TYPE_NVGRE:
> > @@ -640,6 +668,8 @@ struct field_modify_info modify_tcp[] = {
> >   *   Pointer to rte_flow_item objects list.
> >   * @param[in] attr
> >   *   Pointer to flow attributes structure.
> > + * @param[in] dev_flow
> > + *   Pointer to the sub flow.
> >   * @param[in] tunnel_decap
> >   *   Whether action is after tunnel decapsulation.
> >   * @param[out] error
> > @@ -653,8 +683,8 @@ struct field_modify_info modify_tcp[] = {
> >  			(struct mlx5_flow_dv_modify_hdr_resource
> *resource,
> >  			 const struct rte_flow_action *action,
> >  			 const struct rte_flow_item *items,
> > -			 union flow_dv_attr *attr, bool tunnel_decap,
> > -			 struct rte_flow_error *error)
> > +			 union flow_dv_attr *attr, struct mlx5_flow
> > *dev_flow,
> > +			 bool tunnel_decap, struct rte_flow_error *error)
> >  {
> >  	const struct rte_flow_action_set_tp *conf =
> >  		(const struct rte_flow_action_set_tp *)(action->conf); @@ -
> > 666,7 +696,7 @@ struct field_modify_info modify_tcp[] = {
> >  	struct field_modify_info *field;
> >
> >  	if (!attr->valid)
> > -		flow_dv_attr_init(items, attr, tunnel_decap);
> > +		flow_dv_attr_init(items, attr, dev_flow, tunnel_decap);
> >  	if (attr->udp) {
> >  		memset(&udp, 0, sizeof(udp));
> >  		memset(&udp_mask, 0, sizeof(udp_mask)); @@ -716,6
> > +746,8 @@ struct field_modify_info modify_tcp[] = {
> >   *   Pointer to rte_flow_item objects list.
> >   * @param[in] attr
> >   *   Pointer to flow attributes structure.
> > + * @param[in] dev_flow
> > + *   Pointer to the sub flow.
> >   * @param[in] tunnel_decap
> >   *   Whether action is after tunnel decapsulation.
> >   * @param[out] error
> > @@ -729,8 +761,8 @@ struct field_modify_info modify_tcp[] = {
> >  			(struct mlx5_flow_dv_modify_hdr_resource
> *resource,
> >  			 const struct rte_flow_action *action,
> >  			 const struct rte_flow_item *items,
> > -			 union flow_dv_attr *attr, bool tunnel_decap,
> > -			 struct rte_flow_error *error)
> > +			 union flow_dv_attr *attr, struct mlx5_flow
> > *dev_flow,
> > +			 bool tunnel_decap, struct rte_flow_error *error)
> >  {
> >  	const struct rte_flow_action_set_ttl *conf =
> >  		(const struct rte_flow_action_set_ttl *)(action->conf); @@ -
> > 742,7 +774,7 @@ struct field_modify_info modify_tcp[] = {
> >  	struct field_modify_info *field;
> >
> >  	if (!attr->valid)
> > -		flow_dv_attr_init(items, attr, tunnel_decap);
> > +		flow_dv_attr_init(items, attr, dev_flow, tunnel_decap);
> >  	if (attr->ipv4) {
> >  		memset(&ipv4, 0, sizeof(ipv4));
> >  		memset(&ipv4_mask, 0, sizeof(ipv4_mask)); @@ -778,6
> > +810,8 @@ struct field_modify_info modify_tcp[] = {
> >   *   Pointer to rte_flow_item objects list.
> >   * @param[in] attr
> >   *   Pointer to flow attributes structure.
> > + * @param[in] dev_flow
> > + *   Pointer to the sub flow.
> >   * @param[in] tunnel_decap
> >   *   Whether action is after tunnel decapsulation.
> >   * @param[out] error
> > @@ -790,8 +824,8 @@ struct field_modify_info modify_tcp[] = {
> > flow_dv_convert_action_modify_dec_ttl
> >  			(struct mlx5_flow_dv_modify_hdr_resource
> *resource,
> >  			 const struct rte_flow_item *items,
> > -			 union flow_dv_attr *attr, bool tunnel_decap,
> > -			 struct rte_flow_error *error)
> > +			 union flow_dv_attr *attr, struct mlx5_flow
> > *dev_flow,
> > +			 bool tunnel_decap, struct rte_flow_error *error)
> >  {
> >  	struct rte_flow_item item;
> >  	struct rte_flow_item_ipv4 ipv4;
> > @@ -801,7 +835,7 @@ struct field_modify_info modify_tcp[] = {
> >  	struct field_modify_info *field;
> >
> >  	if (!attr->valid)
> > -		flow_dv_attr_init(items, attr, tunnel_decap);
> > +		flow_dv_attr_init(items, attr, dev_flow, tunnel_decap);
> >  	if (attr->ipv4) {
> >  		memset(&ipv4, 0, sizeof(ipv4));
> >  		memset(&ipv4_mask, 0, sizeof(ipv4_mask)); @@ -7505,7
> > +7539,7 @@ struct field_modify_info modify_tcp[] = {
> >  		case RTE_FLOW_ACTION_TYPE_SET_TP_DST:
> >  			if (flow_dv_convert_action_modify_tp
> >  					(mhdr_res, actions, items,
> > -					 &flow_attr, !!(action_flags &
> > +					 &flow_attr, dev_flow, !!(action_flags
> > &
> >  					 MLX5_FLOW_ACTION_DECAP),
> > error))
> >  				return -rte_errno;
> >  			action_flags |= actions->type ==
> > @@ -7515,7 +7549,7 @@ struct field_modify_info modify_tcp[] = {
> >  			break;
> >  		case RTE_FLOW_ACTION_TYPE_DEC_TTL:
> >  			if (flow_dv_convert_action_modify_dec_ttl
> > -					(mhdr_res, items, &flow_attr,
> > +					(mhdr_res, items, &flow_attr,
> > dev_flow,
> >  					 !!(action_flags &
> >  					 MLX5_FLOW_ACTION_DECAP),
> > error))
> >  				return -rte_errno;
> > @@ -7524,7 +7558,7 @@ struct field_modify_info modify_tcp[] = {
> >  		case RTE_FLOW_ACTION_TYPE_SET_TTL:
> >  			if (flow_dv_convert_action_modify_ttl
> >  					(mhdr_res, actions, items,
> > &flow_attr,
> > -					 !!(action_flags &
> > +					 dev_flow, !!(action_flags &
> >  					 MLX5_FLOW_ACTION_DECAP),
> > error))
> >  				return -rte_errno;
> >  			action_flags |= MLX5_FLOW_ACTION_SET_TTL;
> > --
> > 1.8.3.1

[dpdk-dev] [PATCH v2 0/2] net/mlx5: copy the item flags from prefix flow

[Suanming Mou]

For flow split to several subflows, the match items maybe in the prefix
flow, and the actions are split to the suffix flow.

In this case, for the actions need the user defined match item will not
create correctly.

Copy the item layers flags to the suffix flow from prefix flow to fix
the issue.

This patch series should be applied after:
https://patches.dpdk.org/project/dpdk/list/?series=8613

v2:
Remove the actions flag inherit. Get the layer flags accordingly
based on the actions in prefix flow.

Suanming Mou (2):
  net/mlx5: fix layer flags missing in metadata
  net/mlx5: fix lack of match information in meter

 drivers/net/mlx5/mlx5_flow.c    | 73 +++++++++++++++++++++++++++++++++++------
 drivers/net/mlx5/mlx5_flow_dv.c | 59 ++++++++++++++++++++++++---------
 2 files changed, 107 insertions(+), 25 deletions(-)

-- 
1.8.3.1

[dpdk-dev] [PATCH v2 1/2] net/mlx5: fix layer flags missing in metadata

[Suanming Mou]

Metadata suffix subflow inherits the RSS needed hash_fields from the
prefix subflow as the suffix subflow only has the tag match item unable
to generate the full original hash_fields for RSS action.

Unfortunately, hash_fields will only be generated if flow has RSS action.
So it means the prefix flow won't generate the hash_fields as the RSS
action has been split to the suffix flow.

Copy the layer flags from prefix subflow to suffix subflow to help the
suffix subflow to generate the correct hash_fields itself.

Fixes: 71e254bc0294 ("net/mlx5: split Rx flows to provide metadata copy")
Cc: viacheslavo@mellanox.com
Cc: stable@dpdk.org

Signed-off-by: Suanming Mou <suanmingm@mellanox.com>
---
 drivers/net/mlx5/mlx5_flow.c    | 66 +++++++++++++++++++++++++++++++++++------
 drivers/net/mlx5/mlx5_flow_dv.c |  6 +++-
 2 files changed, 62 insertions(+), 10 deletions(-)

diff --git a/drivers/net/mlx5/mlx5_flow.c b/drivers/net/mlx5/mlx5_flow.c
index 2548201..175e999 100644
--- a/drivers/net/mlx5/mlx5_flow.c
+++ b/drivers/net/mlx5/mlx5_flow.c
@@ -2727,6 +2727,43 @@ uint32_t mlx5_flow_adjust_priority(struct rte_eth_dev *dev, int32_t priority,
 }
 
 /**
+ *  Get layer flags from the prefix flow.
+ *
+ *  Some flows may be split to several subflows, the prefix subflow gets the
+ *  match items and the suffix sub flow gets the actions.
+ *  Some actions need the user defined match item flags to get the detail for
+ *  the action.
+ *  This function helps the suffix flow to get the item layer flags from prefix
+ *  subflow.
+ *
+ * @param[in] dev_flow
+ *   Pointer the created preifx subflow.
+ *
+ * @return
+ *   The layers get from prefix subflow.
+ */
+static inline uint64_t
+flow_get_prefix_layer_flags(struct mlx5_flow *dev_flow)
+{
+	uint64_t layers = 0;
+
+	/* If no decap actions, use the layers directly. */
+	if (!(dev_flow->actions & MLX5_FLOW_ACTION_DECAP))
+		return dev_flow->layers;
+	/* Convert L3 layers with decap action. */
+	if (dev_flow->layers & MLX5_FLOW_LAYER_INNER_L3_IPV4)
+		layers |= MLX5_FLOW_LAYER_OUTER_L3_IPV4;
+	else if (dev_flow->layers & MLX5_FLOW_LAYER_INNER_L3_IPV6)
+		layers |= MLX5_FLOW_LAYER_OUTER_L3_IPV6;
+	/* Convert L4 layers with decap action.  */
+	if (dev_flow->layers & MLX5_FLOW_LAYER_INNER_L4_TCP)
+		layers |= MLX5_FLOW_LAYER_OUTER_L4_TCP;
+	else if (dev_flow->layers & MLX5_FLOW_LAYER_INNER_L4_UDP)
+		layers |= MLX5_FLOW_LAYER_OUTER_L4_UDP;
+	return layers;
+}
+
+/**
  * Get QUEUE/RSS action from the action list.
  *
  * @param[in] actions
@@ -3406,6 +3443,8 @@ uint32_t mlx5_flow_adjust_priority(struct rte_eth_dev *dev, int32_t priority,
  *   Parent flow structure pointer.
  * @param[in, out] sub_flow
  *   Pointer to return the created subflow, may be NULL.
+ * @param[in] prefix_layers
+ *   Prefix subflow layers, may be 0.
  * @param[in] attr
  *   Flow rule attributes.
  * @param[in] items
@@ -3423,6 +3462,7 @@ uint32_t mlx5_flow_adjust_priority(struct rte_eth_dev *dev, int32_t priority,
 flow_create_split_inner(struct rte_eth_dev *dev,
 			struct rte_flow *flow,
 			struct mlx5_flow **sub_flow,
+			uint64_t prefix_layers,
 			const struct rte_flow_attr *attr,
 			const struct rte_flow_item items[],
 			const struct rte_flow_action actions[],
@@ -3437,6 +3477,12 @@ uint32_t mlx5_flow_adjust_priority(struct rte_eth_dev *dev, int32_t priority,
 	dev_flow->external = external;
 	/* Subflow object was created, we must include one in the list. */
 	LIST_INSERT_HEAD(&flow->dev_flows, dev_flow, next);
+	/*
+	 * If dev_flow is as one of the suffix flow, some actions in suffix
+	 * flow may need some user defined item layer flags.
+	 */
+	if (prefix_layers)
+		dev_flow->layers = prefix_layers;
 	if (sub_flow)
 		*sub_flow = dev_flow;
 	return flow_drv_translate(dev, dev_flow, attr, items, actions, error);
@@ -3768,8 +3814,9 @@ uint32_t mlx5_flow_adjust_priority(struct rte_eth_dev *dev, int32_t priority,
 	if (!config->dv_flow_en ||
 	    config->dv_xmeta_en == MLX5_XMETA_MODE_LEGACY ||
 	    !mlx5_flow_ext_mreg_supported(dev))
-		return flow_create_split_inner(dev, flow, NULL, attr, items,
-					       actions, external, error);
+		return flow_create_split_inner(dev, flow, NULL, 0,
+					       attr, items, actions, external,
+					       error);
 	actions_n = flow_parse_qrss_action(actions, &qrss);
 	if (qrss) {
 		/* Exclude hairpin flows from splitting. */
@@ -3850,9 +3897,9 @@ uint32_t mlx5_flow_adjust_priority(struct rte_eth_dev *dev, int32_t priority,
 			goto exit;
 	}
 	/* Add the unmodified original or prefix subflow. */
-	ret = flow_create_split_inner(dev, flow, &dev_flow, attr, items,
-				      ext_actions ? ext_actions : actions,
-				      external, error);
+	ret = flow_create_split_inner(dev, flow, &dev_flow, 0, attr,
+				      items, ext_actions ? ext_actions :
+				      actions, external, error);
 	if (ret < 0)
 		goto exit;
 	MLX5_ASSERT(dev_flow);
@@ -3886,7 +3933,6 @@ uint32_t mlx5_flow_adjust_priority(struct rte_eth_dev *dev, int32_t priority,
 				.type = RTE_FLOW_ACTION_TYPE_END,
 			},
 		};
-		uint64_t hash_fields = dev_flow->hash_fields;
 
 		/*
 		 * Configure the tag item only if there is no meter subflow.
@@ -3914,13 +3960,14 @@ uint32_t mlx5_flow_adjust_priority(struct rte_eth_dev *dev, int32_t priority,
 		dev_flow = NULL;
 		/* Add suffix subflow to execute Q/RSS. */
 		ret = flow_create_split_inner(dev, flow, &dev_flow,
+					      flow_get_prefix_layer_flags
+					      (dev_flow),
 					      &q_attr, mtr_sfx ? items :
 					      q_items, q_actions,
 					      external, error);
 		if (ret < 0)
 			goto exit;
 		MLX5_ASSERT(dev_flow);
-		dev_flow->hash_fields = hash_fields;
 	}
 
 exit:
@@ -4009,8 +4056,9 @@ uint32_t mlx5_flow_adjust_priority(struct rte_eth_dev *dev, int32_t priority,
 			goto exit;
 		}
 		/* Add the prefix subflow. */
-		ret = flow_create_split_inner(dev, flow, &dev_flow, attr, items,
-						  pre_actions, external, error);
+		ret = flow_create_split_inner(dev, flow, &dev_flow, 0, attr,
+					      items, pre_actions, external,
+					      error);
 		if (ret) {
 			ret = -rte_errno;
 			goto exit;
diff --git a/drivers/net/mlx5/mlx5_flow_dv.c b/drivers/net/mlx5/mlx5_flow_dv.c
index d038f7b..5171d13 100644
--- a/drivers/net/mlx5/mlx5_flow_dv.c
+++ b/drivers/net/mlx5/mlx5_flow_dv.c
@@ -7813,7 +7813,11 @@ struct field_modify_info modify_tcp[] = {
 	MLX5_ASSERT(!flow_dv_check_valid_spec(matcher.mask.buf,
 					      dev_flow->dv.value.buf));
 #endif
-	dev_flow->layers = item_flags;
+	/*
+	 * Layers may be already initialized from prefix flow if this dev_flow
+	 * is the suffix flow.
+	 */
+	dev_flow->layers |= item_flags;
 	if (action_flags & MLX5_FLOW_ACTION_RSS)
 		flow_dv_hashfields_set(dev_flow);
 	/* Register matcher. */
-- 
1.8.3.1

[dpdk-dev] [PATCH v2 2/2] net/mlx5: fix lack of match information in meter

[Suanming Mou]

As meter flows are split into three subflows each, the prefix subflow
with meter action color the packet, the meter subflow filters out the
colored packets, the suffix subflow applies all the remaining actions
to the passed packets.

Currently, all the user defined items are matched in the prefix flow.
Flow id tag match item is the only item added to the meter suffix
subflow. Some of the remaining actions to be applied in the suffix
subflow require more information in the match item, or the suffix
subflow will not be created successfully.

Actions require the L3/L4 type in the match items as below:
RTE_FLOW_ACTION_TYPE_SET_TP_SRC
RTE_FLOW_ACTION_TYPE_SET_TP_DST
RTE_FLOW_ACTION_TYPE_DEC_TTL
RTE_FLOW_ACTION_TYPE_SET_TTL
RTE_FLOW_ACTION_TYPE_RSS
RTE_FLOW_ACTION_TYPE_QUEUE

Inherit the match item flags from meter prefix subflow to make actions
in suffix subflow get sufficient information.

Fixes: 9ea9b049a960 ("net/mlx5: split meter flow")
Cc: stable@dpdk.org

Signed-off-by: Suanming Mou <suanmingm@mellanox.com>
---
 drivers/net/mlx5/mlx5_flow.c    | 11 ++++++---
 drivers/net/mlx5/mlx5_flow_dv.c | 53 ++++++++++++++++++++++++++++++-----------
 2 files changed, 47 insertions(+), 17 deletions(-)

diff --git a/drivers/net/mlx5/mlx5_flow.c b/drivers/net/mlx5/mlx5_flow.c
index 175e999..e5d2e64 100644
--- a/drivers/net/mlx5/mlx5_flow.c
+++ b/drivers/net/mlx5/mlx5_flow.c
@@ -3778,6 +3778,8 @@ uint32_t mlx5_flow_adjust_priority(struct rte_eth_dev *dev, int32_t priority,
  *   Pointer to Ethernet device.
  * @param[in] flow
  *   Parent flow structure pointer.
+ * @param[in] prefix_layers
+ *   Prefix flow layer flags.
  * @param[in] attr
  *   Flow rule attributes.
  * @param[in] items
@@ -3794,6 +3796,7 @@ uint32_t mlx5_flow_adjust_priority(struct rte_eth_dev *dev, int32_t priority,
 static int
 flow_create_split_metadata(struct rte_eth_dev *dev,
 			   struct rte_flow *flow,
+			   uint64_t prefix_layers,
 			   const struct rte_flow_attr *attr,
 			   const struct rte_flow_item items[],
 			   const struct rte_flow_action actions[],
@@ -3814,7 +3817,7 @@ uint32_t mlx5_flow_adjust_priority(struct rte_eth_dev *dev, int32_t priority,
 	if (!config->dv_flow_en ||
 	    config->dv_xmeta_en == MLX5_XMETA_MODE_LEGACY ||
 	    !mlx5_flow_ext_mreg_supported(dev))
-		return flow_create_split_inner(dev, flow, NULL, 0,
+		return flow_create_split_inner(dev, flow, NULL, prefix_layers,
 					       attr, items, actions, external,
 					       error);
 	actions_n = flow_parse_qrss_action(actions, &qrss);
@@ -3897,7 +3900,7 @@ uint32_t mlx5_flow_adjust_priority(struct rte_eth_dev *dev, int32_t priority,
 			goto exit;
 	}
 	/* Add the unmodified original or prefix subflow. */
-	ret = flow_create_split_inner(dev, flow, &dev_flow, 0, attr,
+	ret = flow_create_split_inner(dev, flow, &dev_flow, prefix_layers, attr,
 				      items, ext_actions ? ext_actions :
 				      actions, external, error);
 	if (ret < 0)
@@ -4093,7 +4096,9 @@ uint32_t mlx5_flow_adjust_priority(struct rte_eth_dev *dev, int32_t priority,
 				 MLX5_FLOW_TABLE_LEVEL_SUFFIX;
 	}
 	/* Add the prefix subflow. */
-	ret = flow_create_split_metadata(dev, flow, &sfx_attr,
+	ret = flow_create_split_metadata(dev, flow, dev_flow ?
+					 flow_get_prefix_layer_flags(dev_flow) :
+					 0, &sfx_attr,
 					 sfx_items ? sfx_items : items,
 					 sfx_actions ? sfx_actions : actions,
 					 external, error);
diff --git a/drivers/net/mlx5/mlx5_flow_dv.c b/drivers/net/mlx5/mlx5_flow_dv.c
index 5171d13..9e1454d 100644
--- a/drivers/net/mlx5/mlx5_flow_dv.c
+++ b/drivers/net/mlx5/mlx5_flow_dv.c
@@ -85,16 +85,35 @@
  *   Pointer to item specification.
  * @param[out] attr
  *   Pointer to flow attributes structure.
+ * @param[in] dev_flow
+ *   Pointer to the sub flow.
  * @param[in] tunnel_decap
  *   Whether action is after tunnel decapsulation.
  */
 static void
 flow_dv_attr_init(const struct rte_flow_item *item, union flow_dv_attr *attr,
-		  bool tunnel_decap)
+		  struct mlx5_flow *dev_flow, bool tunnel_decap)
 {
+	/*
+	 * If layers is already initialized, it means this dev_flow is the
+	 * suffix flow, the layers flags is set by the prefix flow. Need to
+	 * use the layer flags from prefix flow as the suffix flow may not
+	 * have the user defined items as the flow is split.
+	 */
+	if (dev_flow->layers) {
+		if (dev_flow->layers & MLX5_FLOW_LAYER_OUTER_L3_IPV4)
+			attr->ipv4 = 1;
+		else if (dev_flow->layers & MLX5_FLOW_LAYER_OUTER_L3_IPV6)
+			attr->ipv6 = 1;
+		if (dev_flow->layers & MLX5_FLOW_LAYER_OUTER_L4_TCP)
+			attr->tcp = 1;
+		else if (dev_flow->layers & MLX5_FLOW_LAYER_OUTER_L4_UDP)
+			attr->udp = 1;
+		attr->valid = 1;
+		return;
+	}
 	for (; item->type != RTE_FLOW_ITEM_TYPE_END; item++) {
 		uint8_t next_protocol = 0xff;
-
 		switch (item->type) {
 		case RTE_FLOW_ITEM_TYPE_GRE:
 		case RTE_FLOW_ITEM_TYPE_NVGRE:
@@ -640,6 +659,8 @@ struct field_modify_info modify_tcp[] = {
  *   Pointer to rte_flow_item objects list.
  * @param[in] attr
  *   Pointer to flow attributes structure.
+ * @param[in] dev_flow
+ *   Pointer to the sub flow.
  * @param[in] tunnel_decap
  *   Whether action is after tunnel decapsulation.
  * @param[out] error
@@ -653,8 +674,8 @@ struct field_modify_info modify_tcp[] = {
 			(struct mlx5_flow_dv_modify_hdr_resource *resource,
 			 const struct rte_flow_action *action,
 			 const struct rte_flow_item *items,
-			 union flow_dv_attr *attr, bool tunnel_decap,
-			 struct rte_flow_error *error)
+			 union flow_dv_attr *attr, struct mlx5_flow *dev_flow,
+			 bool tunnel_decap, struct rte_flow_error *error)
 {
 	const struct rte_flow_action_set_tp *conf =
 		(const struct rte_flow_action_set_tp *)(action->conf);
@@ -666,7 +687,7 @@ struct field_modify_info modify_tcp[] = {
 	struct field_modify_info *field;
 
 	if (!attr->valid)
-		flow_dv_attr_init(items, attr, tunnel_decap);
+		flow_dv_attr_init(items, attr, dev_flow, tunnel_decap);
 	if (attr->udp) {
 		memset(&udp, 0, sizeof(udp));
 		memset(&udp_mask, 0, sizeof(udp_mask));
@@ -716,6 +737,8 @@ struct field_modify_info modify_tcp[] = {
  *   Pointer to rte_flow_item objects list.
  * @param[in] attr
  *   Pointer to flow attributes structure.
+ * @param[in] dev_flow
+ *   Pointer to the sub flow.
  * @param[in] tunnel_decap
  *   Whether action is after tunnel decapsulation.
  * @param[out] error
@@ -729,8 +752,8 @@ struct field_modify_info modify_tcp[] = {
 			(struct mlx5_flow_dv_modify_hdr_resource *resource,
 			 const struct rte_flow_action *action,
 			 const struct rte_flow_item *items,
-			 union flow_dv_attr *attr, bool tunnel_decap,
-			 struct rte_flow_error *error)
+			 union flow_dv_attr *attr, struct mlx5_flow *dev_flow,
+			 bool tunnel_decap, struct rte_flow_error *error)
 {
 	const struct rte_flow_action_set_ttl *conf =
 		(const struct rte_flow_action_set_ttl *)(action->conf);
@@ -742,7 +765,7 @@ struct field_modify_info modify_tcp[] = {
 	struct field_modify_info *field;
 
 	if (!attr->valid)
-		flow_dv_attr_init(items, attr, tunnel_decap);
+		flow_dv_attr_init(items, attr, dev_flow, tunnel_decap);
 	if (attr->ipv4) {
 		memset(&ipv4, 0, sizeof(ipv4));
 		memset(&ipv4_mask, 0, sizeof(ipv4_mask));
@@ -778,6 +801,8 @@ struct field_modify_info modify_tcp[] = {
  *   Pointer to rte_flow_item objects list.
  * @param[in] attr
  *   Pointer to flow attributes structure.
+ * @param[in] dev_flow
+ *   Pointer to the sub flow.
  * @param[in] tunnel_decap
  *   Whether action is after tunnel decapsulation.
  * @param[out] error
@@ -790,8 +815,8 @@ struct field_modify_info modify_tcp[] = {
 flow_dv_convert_action_modify_dec_ttl
 			(struct mlx5_flow_dv_modify_hdr_resource *resource,
 			 const struct rte_flow_item *items,
-			 union flow_dv_attr *attr, bool tunnel_decap,
-			 struct rte_flow_error *error)
+			 union flow_dv_attr *attr, struct mlx5_flow *dev_flow,
+			 bool tunnel_decap, struct rte_flow_error *error)
 {
 	struct rte_flow_item item;
 	struct rte_flow_item_ipv4 ipv4;
@@ -801,7 +826,7 @@ struct field_modify_info modify_tcp[] = {
 	struct field_modify_info *field;
 
 	if (!attr->valid)
-		flow_dv_attr_init(items, attr, tunnel_decap);
+		flow_dv_attr_init(items, attr, dev_flow, tunnel_decap);
 	if (attr->ipv4) {
 		memset(&ipv4, 0, sizeof(ipv4));
 		memset(&ipv4_mask, 0, sizeof(ipv4_mask));
@@ -7505,7 +7530,7 @@ struct field_modify_info modify_tcp[] = {
 		case RTE_FLOW_ACTION_TYPE_SET_TP_DST:
 			if (flow_dv_convert_action_modify_tp
 					(mhdr_res, actions, items,
-					 &flow_attr, !!(action_flags &
+					 &flow_attr, dev_flow, !!(action_flags &
 					 MLX5_FLOW_ACTION_DECAP), error))
 				return -rte_errno;
 			action_flags |= actions->type ==
@@ -7515,7 +7540,7 @@ struct field_modify_info modify_tcp[] = {
 			break;
 		case RTE_FLOW_ACTION_TYPE_DEC_TTL:
 			if (flow_dv_convert_action_modify_dec_ttl
-					(mhdr_res, items, &flow_attr,
+					(mhdr_res, items, &flow_attr, dev_flow,
 					 !!(action_flags &
 					 MLX5_FLOW_ACTION_DECAP), error))
 				return -rte_errno;
@@ -7524,7 +7549,7 @@ struct field_modify_info modify_tcp[] = {
 		case RTE_FLOW_ACTION_TYPE_SET_TTL:
 			if (flow_dv_convert_action_modify_ttl
 					(mhdr_res, actions, items, &flow_attr,
-					 !!(action_flags &
+					 dev_flow, !!(action_flags &
 					 MLX5_FLOW_ACTION_DECAP), error))
 				return -rte_errno;
 			action_flags |= MLX5_FLOW_ACTION_SET_TTL;
-- 
1.8.3.1

Re: [dpdk-dev] [PATCH v2 1/2] net/mlx5: fix layer flags missing in metadata

[Matan Azrad]

From: Suanming Mou
> Metadata suffix subflow inherits the RSS needed hash_fields from the prefix
> subflow as the suffix subflow only has the tag match item unable to generate
> the full original hash_fields for RSS action.
> 
> Unfortunately, hash_fields will only be generated if flow has RSS action.
> So it means the prefix flow won't generate the hash_fields as the RSS action
> has been split to the suffix flow.
> 
> Copy the layer flags from prefix subflow to suffix subflow to help the suffix
> subflow to generate the correct hash_fields itself.
> 
> Fixes: 71e254bc0294 ("net/mlx5: split Rx flows to provide metadata copy")
> Cc: viacheslavo@mellanox.com
> Cc: stable@dpdk.org
> 
> Signed-off-by: Suanming Mou <suanmingm@mellanox.com>
Acked-by: Matan Azrad <matan@mellanox.com>

Re: [dpdk-dev] [PATCH v2 2/2] net/mlx5: fix lack of match information in meter

[Matan Azrad]

From: Suanming Mou
> As meter flows are split into three subflows each, the prefix subflow with
> meter action color the packet, the meter subflow filters out the colored
> packets, the suffix subflow applies all the remaining actions to the passed
> packets.
> 
> Currently, all the user defined items are matched in the prefix flow.
> Flow id tag match item is the only item added to the meter suffix subflow.
> Some of the remaining actions to be applied in the suffix subflow require
> more information in the match item, or the suffix subflow will not be created
> successfully.
> 
> Actions require the L3/L4 type in the match items as below:
> RTE_FLOW_ACTION_TYPE_SET_TP_SRC
> RTE_FLOW_ACTION_TYPE_SET_TP_DST
> RTE_FLOW_ACTION_TYPE_DEC_TTL
> RTE_FLOW_ACTION_TYPE_SET_TTL
> RTE_FLOW_ACTION_TYPE_RSS
> RTE_FLOW_ACTION_TYPE_QUEUE
> 
> Inherit the match item flags from meter prefix subflow to make actions in
> suffix subflow get sufficient information.
> 
> Fixes: 9ea9b049a960 ("net/mlx5: split meter flow")
> Cc: stable@dpdk.org
> 
> Signed-off-by: Suanming Mou <suanmingm@mellanox.com>
Acked-by: Matan Azrad <matan@mellanox.com>

Re: [dpdk-dev] [PATCH 0/2] net/mlx5: copy the item flags from prefix flow

[Raslan Darawsheh]

Hi,

> -----Original Message-----
> From: Suanming Mou <suanmingm@mellanox.com>
> Sent: Wednesday, February 19, 2020 4:31 PM
> To: Slava Ovsiienko <viacheslavo@mellanox.com>; Matan Azrad
> <matan@mellanox.com>
> Cc: dev@dpdk.org; Raslan Darawsheh <rasland@mellanox.com>
> Subject: [PATCH 0/2] net/mlx5: copy the item flags from prefix flow
> 
> For flow split to several subflows, the match items maybe in the prefix
> flow, and the actions are split to the suffix flow.
> 
> In this case, for the actions need the user defined match item will not
> create correctly.
> 
> Copy the item layers flags to the suffix flow from prefix flow to fix
> the issue.
> 
> This patch series should be applied after:
> https://eur03.safelinks.protection.outlook.com/?url=https%3A%2F%2Fpatch
> es.dpdk.org%2Fproject%2Fdpdk%2Flist%2F%3Fseries%3D8613&amp;data=0
> 2%7C01%7Crasland%40mellanox.com%7C18e60c79e62d40313b6008d7b5485c
> df%7Ca652971c7d2e4d9ba6a4d149256f461b%7C0%7C0%7C6371771947191293
> 09&amp;sdata=QE%2FeFbc3rp1m5KgmtmR%2Bfu5%2B2v%2BeBANO1u80R2
> PdS9A%3D&amp;reserved=0
> 
> Suanming Mou (2):
>   net/mlx5: fix layer flags missing in metadata
>   net/mlx5: fix lack of match information in meter
> 
>  drivers/net/mlx5/mlx5_flow.c    | 41 +++++++++++++++++++-------
>  drivers/net/mlx5/mlx5_flow_dv.c | 64
> +++++++++++++++++++++++++++++++----------
>  2 files changed, 79 insertions(+), 26 deletions(-)
> 
> --
> 1.8.3.1


Series applied to next-net-mlx,

Kindest regards,
Raslan Darawsheh

Re: [dpdk-dev] [PATCH 0/2] net/mlx5: copy the item flags from prefix flow

[Raslan Darawsheh]

Hi,

Taking this series out of next-net-mlx,
Waiting for a v2, we are seeing a Seg. Fault due to the first patch of this series.


Kindest regards,
Raslan Darawsheh

> -----Original Message-----
> From: Raslan Darawsheh
> Sent: Thursday, February 20, 2020 2:13 PM
> To: Suanming Mou <suanmingm@mellanox.com>; Slava Ovsiienko
> <viacheslavo@mellanox.com>; Matan Azrad <matan@mellanox.com>
> Cc: dev@dpdk.org
> Subject: RE: [PATCH 0/2] net/mlx5: copy the item flags from prefix flow
> 
> Hi,
> 
> > -----Original Message-----
> > From: Suanming Mou <suanmingm@mellanox.com>
> > Sent: Wednesday, February 19, 2020 4:31 PM
> > To: Slava Ovsiienko <viacheslavo@mellanox.com>; Matan Azrad
> > <matan@mellanox.com>
> > Cc: dev@dpdk.org; Raslan Darawsheh <rasland@mellanox.com>
> > Subject: [PATCH 0/2] net/mlx5: copy the item flags from prefix flow
> >
> > For flow split to several subflows, the match items maybe in the prefix
> > flow, and the actions are split to the suffix flow.
> >
> > In this case, for the actions need the user defined match item will not
> > create correctly.
> >
> > Copy the item layers flags to the suffix flow from prefix flow to fix
> > the issue.
> >
> > This patch series should be applied after:
> >
> https://eur03.safelinks.protection.outlook.com/?url=https%3A%2F%2Fpatch
> >
> es.dpdk.org%2Fproject%2Fdpdk%2Flist%2F%3Fseries%3D8613&amp;data=0
> >
> 2%7C01%7Crasland%40mellanox.com%7C18e60c79e62d40313b6008d7b5485c
> >
> df%7Ca652971c7d2e4d9ba6a4d149256f461b%7C0%7C0%7C6371771947191293
> >
> 09&amp;sdata=QE%2FeFbc3rp1m5KgmtmR%2Bfu5%2B2v%2BeBANO1u80R2
> > PdS9A%3D&amp;reserved=0
> >
> > Suanming Mou (2):
> >   net/mlx5: fix layer flags missing in metadata
> >   net/mlx5: fix lack of match information in meter
> >
> >  drivers/net/mlx5/mlx5_flow.c    | 41 +++++++++++++++++++-------
> >  drivers/net/mlx5/mlx5_flow_dv.c | 64
> > +++++++++++++++++++++++++++++++----------
> >  2 files changed, 79 insertions(+), 26 deletions(-)
> >
> > --
> > 1.8.3.1
> 
> 
> Series applied to next-net-mlx,
> 
> Kindest regards,
> Raslan Darawsheh

[dpdk-dev] [PATCH v3 0/2] net/mlx5: copy the item flags from prefix flow

[Suanming Mou]

For flow split to several subflows, the match items maybe in the prefix
flow, and the actions are split to the suffix flow.

In this case, for the actions need the user defined match item will not
create correctly.

Copy the item layers flags to the suffix flow from prefix flow to fix
the issue.

This patch series should be applied after:
https://patches.dpdk.org/project/dpdk/list/?series=8613

v2:
Remove the actions flag inherit. Get the layer flags accordingly
based on the actions in prefix flow.

v3:
Fix NULL pointer issue.

Suanming Mou (2):
  net/mlx5: fix layer flags missing in metadata
  net/mlx5: fix lack of match information in meter

 drivers/net/mlx5/mlx5_flow.c    | 74 +++++++++++++++++++++++++++++++++++------
 drivers/net/mlx5/mlx5_flow_dv.c | 59 +++++++++++++++++++++++---------
 2 files changed, 107 insertions(+), 26 deletions(-)

-- 
1.8.3.1

[dpdk-dev] [PATCH v3 1/2] net/mlx5: fix layer flags missing in metadata

[Suanming Mou]

Metadata suffix subflow inherits the RSS needed hash_fields from the
prefix subflow as the suffix subflow only has the tag match item unable
to generate the full original hash_fields for RSS action.

Unfortunately, hash_fields will only be generated if flow has RSS action.
So it means the prefix flow won't generate the hash_fields as the RSS
action has been split to the suffix flow.

Copy the layer flags from prefix subflow to suffix subflow to help the
suffix subflow to generate the correct hash_fields itself.

Fixes: 71e254bc0294 ("net/mlx5: split Rx flows to provide metadata copy")
Cc: viacheslavo@mellanox.com
Cc: stable@dpdk.org

Signed-off-by: Suanming Mou <suanmingm@mellanox.com>
Acked-by: Matan Azrad <matan@mellanox.com>
---
 drivers/net/mlx5/mlx5_flow.c    | 67 +++++++++++++++++++++++++++++++++++------
 drivers/net/mlx5/mlx5_flow_dv.c |  6 +++-
 2 files changed, 62 insertions(+), 11 deletions(-)

diff --git a/drivers/net/mlx5/mlx5_flow.c b/drivers/net/mlx5/mlx5_flow.c
index ce5aded..9267858 100644
--- a/drivers/net/mlx5/mlx5_flow.c
+++ b/drivers/net/mlx5/mlx5_flow.c
@@ -2727,6 +2727,43 @@ uint32_t mlx5_flow_adjust_priority(struct rte_eth_dev *dev, int32_t priority,
 }
 
 /**
+ *  Get layer flags from the prefix flow.
+ *
+ *  Some flows may be split to several subflows, the prefix subflow gets the
+ *  match items and the suffix sub flow gets the actions.
+ *  Some actions need the user defined match item flags to get the detail for
+ *  the action.
+ *  This function helps the suffix flow to get the item layer flags from prefix
+ *  subflow.
+ *
+ * @param[in] dev_flow
+ *   Pointer the created preifx subflow.
+ *
+ * @return
+ *   The layers get from prefix subflow.
+ */
+static inline uint64_t
+flow_get_prefix_layer_flags(struct mlx5_flow *dev_flow)
+{
+	uint64_t layers = 0;
+
+	/* If no decap actions, use the layers directly. */
+	if (!(dev_flow->actions & MLX5_FLOW_ACTION_DECAP))
+		return dev_flow->layers;
+	/* Convert L3 layers with decap action. */
+	if (dev_flow->layers & MLX5_FLOW_LAYER_INNER_L3_IPV4)
+		layers |= MLX5_FLOW_LAYER_OUTER_L3_IPV4;
+	else if (dev_flow->layers & MLX5_FLOW_LAYER_INNER_L3_IPV6)
+		layers |= MLX5_FLOW_LAYER_OUTER_L3_IPV6;
+	/* Convert L4 layers with decap action.  */
+	if (dev_flow->layers & MLX5_FLOW_LAYER_INNER_L4_TCP)
+		layers |= MLX5_FLOW_LAYER_OUTER_L4_TCP;
+	else if (dev_flow->layers & MLX5_FLOW_LAYER_INNER_L4_UDP)
+		layers |= MLX5_FLOW_LAYER_OUTER_L4_UDP;
+	return layers;
+}
+
+/**
  * Get QUEUE/RSS action from the action list.
  *
  * @param[in] actions
@@ -3406,6 +3443,8 @@ uint32_t mlx5_flow_adjust_priority(struct rte_eth_dev *dev, int32_t priority,
  *   Parent flow structure pointer.
  * @param[in, out] sub_flow
  *   Pointer to return the created subflow, may be NULL.
+ * @param[in] prefix_layers
+ *   Prefix subflow layers, may be 0.
  * @param[in] attr
  *   Flow rule attributes.
  * @param[in] items
@@ -3423,6 +3462,7 @@ uint32_t mlx5_flow_adjust_priority(struct rte_eth_dev *dev, int32_t priority,
 flow_create_split_inner(struct rte_eth_dev *dev,
 			struct rte_flow *flow,
 			struct mlx5_flow **sub_flow,
+			uint64_t prefix_layers,
 			const struct rte_flow_attr *attr,
 			const struct rte_flow_item items[],
 			const struct rte_flow_action actions[],
@@ -3437,6 +3477,12 @@ uint32_t mlx5_flow_adjust_priority(struct rte_eth_dev *dev, int32_t priority,
 	dev_flow->external = external;
 	/* Subflow object was created, we must include one in the list. */
 	LIST_INSERT_HEAD(&flow->dev_flows, dev_flow, next);
+	/*
+	 * If dev_flow is as one of the suffix flow, some actions in suffix
+	 * flow may need some user defined item layer flags.
+	 */
+	if (prefix_layers)
+		dev_flow->layers = prefix_layers;
 	if (sub_flow)
 		*sub_flow = dev_flow;
 	return flow_drv_translate(dev, dev_flow, attr, items, actions, error);
@@ -3768,8 +3814,9 @@ uint32_t mlx5_flow_adjust_priority(struct rte_eth_dev *dev, int32_t priority,
 	if (!config->dv_flow_en ||
 	    config->dv_xmeta_en == MLX5_XMETA_MODE_LEGACY ||
 	    !mlx5_flow_ext_mreg_supported(dev))
-		return flow_create_split_inner(dev, flow, NULL, attr, items,
-					       actions, external, error);
+		return flow_create_split_inner(dev, flow, NULL, 0,
+					       attr, items, actions, external,
+					       error);
 	actions_n = flow_parse_qrss_action(actions, &qrss);
 	if (qrss) {
 		/* Exclude hairpin flows from splitting. */
@@ -3850,9 +3897,9 @@ uint32_t mlx5_flow_adjust_priority(struct rte_eth_dev *dev, int32_t priority,
 			goto exit;
 	}
 	/* Add the unmodified original or prefix subflow. */
-	ret = flow_create_split_inner(dev, flow, &dev_flow, attr, items,
-				      ext_actions ? ext_actions : actions,
-				      external, error);
+	ret = flow_create_split_inner(dev, flow, &dev_flow, 0, attr,
+				      items, ext_actions ? ext_actions :
+				      actions, external, error);
 	if (ret < 0)
 		goto exit;
 	MLX5_ASSERT(dev_flow);
@@ -3886,7 +3933,7 @@ uint32_t mlx5_flow_adjust_priority(struct rte_eth_dev *dev, int32_t priority,
 				.type = RTE_FLOW_ACTION_TYPE_END,
 			},
 		};
-		uint64_t hash_fields = dev_flow->hash_fields;
+		uint64_t layers = flow_get_prefix_layer_flags(dev_flow);
 
 		/*
 		 * Configure the tag item only if there is no meter subflow.
@@ -3913,14 +3960,13 @@ uint32_t mlx5_flow_adjust_priority(struct rte_eth_dev *dev, int32_t priority,
 		}
 		dev_flow = NULL;
 		/* Add suffix subflow to execute Q/RSS. */
-		ret = flow_create_split_inner(dev, flow, &dev_flow,
+		ret = flow_create_split_inner(dev, flow, &dev_flow, layers,
 					      &q_attr, mtr_sfx ? items :
 					      q_items, q_actions,
 					      external, error);
 		if (ret < 0)
 			goto exit;
 		MLX5_ASSERT(dev_flow);
-		dev_flow->hash_fields = hash_fields;
 	}
 
 exit:
@@ -4009,8 +4055,9 @@ uint32_t mlx5_flow_adjust_priority(struct rte_eth_dev *dev, int32_t priority,
 			goto exit;
 		}
 		/* Add the prefix subflow. */
-		ret = flow_create_split_inner(dev, flow, &dev_flow, attr, items,
-						  pre_actions, external, error);
+		ret = flow_create_split_inner(dev, flow, &dev_flow, 0, attr,
+					      items, pre_actions, external,
+					      error);
 		if (ret) {
 			ret = -rte_errno;
 			goto exit;
diff --git a/drivers/net/mlx5/mlx5_flow_dv.c b/drivers/net/mlx5/mlx5_flow_dv.c
index b6e50b1..19f5b61 100644
--- a/drivers/net/mlx5/mlx5_flow_dv.c
+++ b/drivers/net/mlx5/mlx5_flow_dv.c
@@ -7813,7 +7813,11 @@ struct field_modify_info modify_tcp[] = {
 	MLX5_ASSERT(!flow_dv_check_valid_spec(matcher.mask.buf,
 					      dev_flow->dv.value.buf));
 #endif
-	dev_flow->layers = item_flags;
+	/*
+	 * Layers may be already initialized from prefix flow if this dev_flow
+	 * is the suffix flow.
+	 */
+	dev_flow->layers |= item_flags;
 	if (action_flags & MLX5_FLOW_ACTION_RSS)
 		flow_dv_hashfields_set(dev_flow);
 	/* Register matcher. */
-- 
1.8.3.1

[dpdk-dev] [PATCH v3 2/2] net/mlx5: fix lack of match information in meter

[Suanming Mou]

As meter flows are split into three subflows each, the prefix subflow
with meter action color the packet, the meter subflow filters out the
colored packets, the suffix subflow applies all the remaining actions
to the passed packets.

Currently, all the user defined items are matched in the prefix flow.
Flow id tag match item is the only item added to the meter suffix
subflow. Some of the remaining actions to be applied in the suffix
subflow require more information in the match item, or the suffix
subflow will not be created successfully.

Actions require the L3/L4 type in the match items as below:
RTE_FLOW_ACTION_TYPE_SET_TP_SRC
RTE_FLOW_ACTION_TYPE_SET_TP_DST
RTE_FLOW_ACTION_TYPE_DEC_TTL
RTE_FLOW_ACTION_TYPE_SET_TTL
RTE_FLOW_ACTION_TYPE_RSS
RTE_FLOW_ACTION_TYPE_QUEUE

Inherit the match item flags from meter prefix subflow to make actions
in suffix subflow get sufficient information.

Fixes: 9ea9b049a960 ("net/mlx5: split meter flow")
Cc: stable@dpdk.org

Signed-off-by: Suanming Mou <suanmingm@mellanox.com>
Acked-by: Matan Azrad <matan@mellanox.com>
---
 drivers/net/mlx5/mlx5_flow.c    | 11 ++++++---
 drivers/net/mlx5/mlx5_flow_dv.c | 53 ++++++++++++++++++++++++++++++-----------
 2 files changed, 47 insertions(+), 17 deletions(-)

diff --git a/drivers/net/mlx5/mlx5_flow.c b/drivers/net/mlx5/mlx5_flow.c
index 9267858..da60ce7 100644
--- a/drivers/net/mlx5/mlx5_flow.c
+++ b/drivers/net/mlx5/mlx5_flow.c
@@ -3778,6 +3778,8 @@ uint32_t mlx5_flow_adjust_priority(struct rte_eth_dev *dev, int32_t priority,
  *   Pointer to Ethernet device.
  * @param[in] flow
  *   Parent flow structure pointer.
+ * @param[in] prefix_layers
+ *   Prefix flow layer flags.
  * @param[in] attr
  *   Flow rule attributes.
  * @param[in] items
@@ -3794,6 +3796,7 @@ uint32_t mlx5_flow_adjust_priority(struct rte_eth_dev *dev, int32_t priority,
 static int
 flow_create_split_metadata(struct rte_eth_dev *dev,
 			   struct rte_flow *flow,
+			   uint64_t prefix_layers,
 			   const struct rte_flow_attr *attr,
 			   const struct rte_flow_item items[],
 			   const struct rte_flow_action actions[],
@@ -3814,7 +3817,7 @@ uint32_t mlx5_flow_adjust_priority(struct rte_eth_dev *dev, int32_t priority,
 	if (!config->dv_flow_en ||
 	    config->dv_xmeta_en == MLX5_XMETA_MODE_LEGACY ||
 	    !mlx5_flow_ext_mreg_supported(dev))
-		return flow_create_split_inner(dev, flow, NULL, 0,
+		return flow_create_split_inner(dev, flow, NULL, prefix_layers,
 					       attr, items, actions, external,
 					       error);
 	actions_n = flow_parse_qrss_action(actions, &qrss);
@@ -3897,7 +3900,7 @@ uint32_t mlx5_flow_adjust_priority(struct rte_eth_dev *dev, int32_t priority,
 			goto exit;
 	}
 	/* Add the unmodified original or prefix subflow. */
-	ret = flow_create_split_inner(dev, flow, &dev_flow, 0, attr,
+	ret = flow_create_split_inner(dev, flow, &dev_flow, prefix_layers, attr,
 				      items, ext_actions ? ext_actions :
 				      actions, external, error);
 	if (ret < 0)
@@ -4092,7 +4095,9 @@ uint32_t mlx5_flow_adjust_priority(struct rte_eth_dev *dev, int32_t priority,
 				 MLX5_FLOW_TABLE_LEVEL_SUFFIX;
 	}
 	/* Add the prefix subflow. */
-	ret = flow_create_split_metadata(dev, flow, &sfx_attr,
+	ret = flow_create_split_metadata(dev, flow, dev_flow ?
+					 flow_get_prefix_layer_flags(dev_flow) :
+					 0, &sfx_attr,
 					 sfx_items ? sfx_items : items,
 					 sfx_actions ? sfx_actions : actions,
 					 external, error);
diff --git a/drivers/net/mlx5/mlx5_flow_dv.c b/drivers/net/mlx5/mlx5_flow_dv.c
index 19f5b61..ebd7a93 100644
--- a/drivers/net/mlx5/mlx5_flow_dv.c
+++ b/drivers/net/mlx5/mlx5_flow_dv.c
@@ -85,16 +85,35 @@
  *   Pointer to item specification.
  * @param[out] attr
  *   Pointer to flow attributes structure.
+ * @param[in] dev_flow
+ *   Pointer to the sub flow.
  * @param[in] tunnel_decap
  *   Whether action is after tunnel decapsulation.
  */
 static void
 flow_dv_attr_init(const struct rte_flow_item *item, union flow_dv_attr *attr,
-		  bool tunnel_decap)
+		  struct mlx5_flow *dev_flow, bool tunnel_decap)
 {
+	/*
+	 * If layers is already initialized, it means this dev_flow is the
+	 * suffix flow, the layers flags is set by the prefix flow. Need to
+	 * use the layer flags from prefix flow as the suffix flow may not
+	 * have the user defined items as the flow is split.
+	 */
+	if (dev_flow->layers) {
+		if (dev_flow->layers & MLX5_FLOW_LAYER_OUTER_L3_IPV4)
+			attr->ipv4 = 1;
+		else if (dev_flow->layers & MLX5_FLOW_LAYER_OUTER_L3_IPV6)
+			attr->ipv6 = 1;
+		if (dev_flow->layers & MLX5_FLOW_LAYER_OUTER_L4_TCP)
+			attr->tcp = 1;
+		else if (dev_flow->layers & MLX5_FLOW_LAYER_OUTER_L4_UDP)
+			attr->udp = 1;
+		attr->valid = 1;
+		return;
+	}
 	for (; item->type != RTE_FLOW_ITEM_TYPE_END; item++) {
 		uint8_t next_protocol = 0xff;
-
 		switch (item->type) {
 		case RTE_FLOW_ITEM_TYPE_GRE:
 		case RTE_FLOW_ITEM_TYPE_NVGRE:
@@ -640,6 +659,8 @@ struct field_modify_info modify_tcp[] = {
  *   Pointer to rte_flow_item objects list.
  * @param[in] attr
  *   Pointer to flow attributes structure.
+ * @param[in] dev_flow
+ *   Pointer to the sub flow.
  * @param[in] tunnel_decap
  *   Whether action is after tunnel decapsulation.
  * @param[out] error
@@ -653,8 +674,8 @@ struct field_modify_info modify_tcp[] = {
 			(struct mlx5_flow_dv_modify_hdr_resource *resource,
 			 const struct rte_flow_action *action,
 			 const struct rte_flow_item *items,
-			 union flow_dv_attr *attr, bool tunnel_decap,
-			 struct rte_flow_error *error)
+			 union flow_dv_attr *attr, struct mlx5_flow *dev_flow,
+			 bool tunnel_decap, struct rte_flow_error *error)
 {
 	const struct rte_flow_action_set_tp *conf =
 		(const struct rte_flow_action_set_tp *)(action->conf);
@@ -666,7 +687,7 @@ struct field_modify_info modify_tcp[] = {
 	struct field_modify_info *field;
 
 	if (!attr->valid)
-		flow_dv_attr_init(items, attr, tunnel_decap);
+		flow_dv_attr_init(items, attr, dev_flow, tunnel_decap);
 	if (attr->udp) {
 		memset(&udp, 0, sizeof(udp));
 		memset(&udp_mask, 0, sizeof(udp_mask));
@@ -716,6 +737,8 @@ struct field_modify_info modify_tcp[] = {
  *   Pointer to rte_flow_item objects list.
  * @param[in] attr
  *   Pointer to flow attributes structure.
+ * @param[in] dev_flow
+ *   Pointer to the sub flow.
  * @param[in] tunnel_decap
  *   Whether action is after tunnel decapsulation.
  * @param[out] error
@@ -729,8 +752,8 @@ struct field_modify_info modify_tcp[] = {
 			(struct mlx5_flow_dv_modify_hdr_resource *resource,
 			 const struct rte_flow_action *action,
 			 const struct rte_flow_item *items,
-			 union flow_dv_attr *attr, bool tunnel_decap,
-			 struct rte_flow_error *error)
+			 union flow_dv_attr *attr, struct mlx5_flow *dev_flow,
+			 bool tunnel_decap, struct rte_flow_error *error)
 {
 	const struct rte_flow_action_set_ttl *conf =
 		(const struct rte_flow_action_set_ttl *)(action->conf);
@@ -742,7 +765,7 @@ struct field_modify_info modify_tcp[] = {
 	struct field_modify_info *field;
 
 	if (!attr->valid)
-		flow_dv_attr_init(items, attr, tunnel_decap);
+		flow_dv_attr_init(items, attr, dev_flow, tunnel_decap);
 	if (attr->ipv4) {
 		memset(&ipv4, 0, sizeof(ipv4));
 		memset(&ipv4_mask, 0, sizeof(ipv4_mask));
@@ -778,6 +801,8 @@ struct field_modify_info modify_tcp[] = {
  *   Pointer to rte_flow_item objects list.
  * @param[in] attr
  *   Pointer to flow attributes structure.
+ * @param[in] dev_flow
+ *   Pointer to the sub flow.
  * @param[in] tunnel_decap
  *   Whether action is after tunnel decapsulation.
  * @param[out] error
@@ -790,8 +815,8 @@ struct field_modify_info modify_tcp[] = {
 flow_dv_convert_action_modify_dec_ttl
 			(struct mlx5_flow_dv_modify_hdr_resource *resource,
 			 const struct rte_flow_item *items,
-			 union flow_dv_attr *attr, bool tunnel_decap,
-			 struct rte_flow_error *error)
+			 union flow_dv_attr *attr, struct mlx5_flow *dev_flow,
+			 bool tunnel_decap, struct rte_flow_error *error)
 {
 	struct rte_flow_item item;
 	struct rte_flow_item_ipv4 ipv4;
@@ -801,7 +826,7 @@ struct field_modify_info modify_tcp[] = {
 	struct field_modify_info *field;
 
 	if (!attr->valid)
-		flow_dv_attr_init(items, attr, tunnel_decap);
+		flow_dv_attr_init(items, attr, dev_flow, tunnel_decap);
 	if (attr->ipv4) {
 		memset(&ipv4, 0, sizeof(ipv4));
 		memset(&ipv4_mask, 0, sizeof(ipv4_mask));
@@ -7505,7 +7530,7 @@ struct field_modify_info modify_tcp[] = {
 		case RTE_FLOW_ACTION_TYPE_SET_TP_DST:
 			if (flow_dv_convert_action_modify_tp
 					(mhdr_res, actions, items,
-					 &flow_attr, !!(action_flags &
+					 &flow_attr, dev_flow, !!(action_flags &
 					 MLX5_FLOW_ACTION_DECAP), error))
 				return -rte_errno;
 			action_flags |= actions->type ==
@@ -7515,7 +7540,7 @@ struct field_modify_info modify_tcp[] = {
 			break;
 		case RTE_FLOW_ACTION_TYPE_DEC_TTL:
 			if (flow_dv_convert_action_modify_dec_ttl
-					(mhdr_res, items, &flow_attr,
+					(mhdr_res, items, &flow_attr, dev_flow,
 					 !!(action_flags &
 					 MLX5_FLOW_ACTION_DECAP), error))
 				return -rte_errno;
@@ -7524,7 +7549,7 @@ struct field_modify_info modify_tcp[] = {
 		case RTE_FLOW_ACTION_TYPE_SET_TTL:
 			if (flow_dv_convert_action_modify_ttl
 					(mhdr_res, actions, items, &flow_attr,
-					 !!(action_flags &
+					 dev_flow, !!(action_flags &
 					 MLX5_FLOW_ACTION_DECAP), error))
 				return -rte_errno;
 			action_flags |= MLX5_FLOW_ACTION_SET_TTL;
-- 
1.8.3.1

Re: [dpdk-dev] [PATCH v3 0/2] net/mlx5: copy the item flags from prefix flow

[Raslan Darawsheh]

Hi,

Series applied to next-net-mlx,

Kindest regards,
Raslan Darawsheh

> -----Original Message-----
> From: Suanming Mou <suanmingm@mellanox.com>
> Sent: Thursday, February 20, 2020 3:54 PM
> To: Matan Azrad <matan@mellanox.com>; Slava Ovsiienko
> <viacheslavo@mellanox.com>
> Cc: dev@dpdk.org; Raslan Darawsheh <rasland@mellanox.com>
> Subject: [PATCH v3 0/2] net/mlx5: copy the item flags from prefix flow
> 
> For flow split to several subflows, the match items maybe in the prefix
> flow, and the actions are split to the suffix flow.
> 
> In this case, for the actions need the user defined match item will not
> create correctly.
> 
> Copy the item layers flags to the suffix flow from prefix flow to fix
> the issue.
> 
> This patch series should be applied after:
> https://eur03.safelinks.protection.outlook.com/?url=https%3A%2F%2Fpatch
> es.dpdk.org%2Fproject%2Fdpdk%2Flist%2F%3Fseries%3D8613&amp;data=0
> 2%7C01%7Crasland%40mellanox.com%7Cf520b39d2d0548ae5d2808d7b60c50
> 1d%7Ca652971c7d2e4d9ba6a4d149256f461b%7C0%7C0%7C637178036316184
> 939&amp;sdata=8ZXhPrT27gbXwseF3TAwVbAslh7nOnt3%2FuzilxJblGE%3D&
> amp;reserved=0
> 
> v2:
> Remove the actions flag inherit. Get the layer flags accordingly
> based on the actions in prefix flow.
> 
> v3:
> Fix NULL pointer issue.
> 
> Suanming Mou (2):
>   net/mlx5: fix layer flags missing in metadata
>   net/mlx5: fix lack of match information in meter
> 
>  drivers/net/mlx5/mlx5_flow.c    | 74
> +++++++++++++++++++++++++++++++++++------
>  drivers/net/mlx5/mlx5_flow_dv.c | 59 +++++++++++++++++++++++---------
>  2 files changed, 107 insertions(+), 26 deletions(-)
> 
> --
> 1.8.3.1