patches for DPDK stable branches
 help / color / mirror / Atom feed
* [dpdk-stable] [PATCH] net/mlx5: fix E-Switch egress mirror flow validation
@ 2021-02-03  8:29 Jiawei Wang
  2021-02-04 14:41 ` Raslan Darawsheh
  0 siblings, 1 reply; 2+ messages in thread
From: Jiawei Wang @ 2021-02-03  8:29 UTC (permalink / raw)
  To: viacheslavo, matan, orika, thomas; +Cc: dev, rasland, stable

The stored metadata in all registers C were lost in E-Switch egress
mirroring flows due to HW limitation. The register C0 keeps the
source vport index that also was used as one of the flow matcher.

While sample action and jump action (jump to table X) was in the
E-Switch egress flow, the flow in the next table X wasn't hit since
source vport value lost.

The modify actions after sample action should be applied to the packet
on normal path, not to the sampled packet. In order to support this
mlx5 PMD splits the flow into sub flows and jump action is engaged
implicitly, causing malfunction due to registers corruption.

This patch adds the validation the for E-Switch mirroring jump egress
flow, and checks for this hidden jump as well and reject the flows with
modify actions after sampling.

Fixes: 6a951567c159 ("net/mlx5: support E-Switch mirroring and jump in one flow")
Cc: stable@dpdk.org

Signed-off-by: Jiawei Wang <jiaweiw@nvidia.com>
Acked-by: Viacheslav Ovsiienko <viacheslavo@nvidia.com>
---
 drivers/net/mlx5/mlx5_flow_dv.c | 47 +++++++++++++++++++++++++++++++++++++++++
 1 file changed, 47 insertions(+)

diff --git a/drivers/net/mlx5/mlx5_flow_dv.c b/drivers/net/mlx5/mlx5_flow_dv.c
index 8c11ac3..ee1c4f4 100644
--- a/drivers/net/mlx5/mlx5_flow_dv.c
+++ b/drivers/net/mlx5/mlx5_flow_dv.c
@@ -5081,6 +5081,8 @@ struct mlx5_hlist_entry *
  *   Pointer to the RSS action.
  * @param[out] sample_rss
  *   Pointer to the RSS action in sample action list.
+ * @param[out] fdb_mirror_limit
+ *   Pointer to the FDB mirror limitation flag.
  * @param[out] error
  *   Pointer to error structure.
  *
@@ -5095,6 +5097,7 @@ struct mlx5_hlist_entry *
 			       uint64_t item_flags,
 			       const struct rte_flow_action_rss *rss,
 			       const struct rte_flow_action_rss **sample_rss,
+			       int *fdb_mirror_limit,
 			       struct rte_flow_error *error)
 {
 	struct mlx5_priv *priv = dev->data->dev_private;
@@ -5269,6 +5272,9 @@ struct mlx5_hlist_entry *
 						  NULL,
 						  "E-Switch must has a dest "
 						  "port for mirroring");
+		if (!priv->config.hca_attr.reg_c_preserve &&
+		     priv->representor_id != -1)
+			*fdb_mirror_limit = 1;
 	}
 	/* Continue validation for Xcap actions.*/
 	if ((sub_action_flags & MLX5_FLOW_XCAP_ACTIONS) &&
@@ -6010,6 +6016,8 @@ struct mlx5_hlist_entry *
 	uint16_t ether_type = 0;
 	int actions_n = 0;
 	uint8_t item_ipv6_proto = 0;
+	int fdb_mirror_limit = 0;
+	int modify_after_mirror = 0;
 	const struct rte_flow_item *geneve_item = NULL;
 	const struct rte_flow_item *gre_item = NULL;
 	const struct rte_flow_item *gtp_item = NULL;
@@ -6429,6 +6437,9 @@ struct mlx5_hlist_entry *
 					++actions_n;
 				action_flags |= MLX5_FLOW_ACTION_FLAG |
 						MLX5_FLOW_ACTION_MARK_EXT;
+				if (action_flags & MLX5_FLOW_ACTION_SAMPLE)
+					modify_after_mirror = 1;
+
 			} else {
 				action_flags |= MLX5_FLOW_ACTION_FLAG;
 				++actions_n;
@@ -6448,6 +6459,8 @@ struct mlx5_hlist_entry *
 					++actions_n;
 				action_flags |= MLX5_FLOW_ACTION_MARK |
 						MLX5_FLOW_ACTION_MARK_EXT;
+				if (action_flags & MLX5_FLOW_ACTION_SAMPLE)
+					modify_after_mirror = 1;
 			} else {
 				action_flags |= MLX5_FLOW_ACTION_MARK;
 				++actions_n;
@@ -6463,6 +6476,8 @@ struct mlx5_hlist_entry *
 			/* Count all modify-header actions as one action. */
 			if (!(action_flags & MLX5_FLOW_MODIFY_HDR_ACTIONS))
 				++actions_n;
+			if (action_flags & MLX5_FLOW_ACTION_SAMPLE)
+				modify_after_mirror = 1;
 			action_flags |= MLX5_FLOW_ACTION_SET_META;
 			rw_act_num += MLX5_ACT_NUM_SET_META;
 			break;
@@ -6475,6 +6490,8 @@ struct mlx5_hlist_entry *
 			/* Count all modify-header actions as one action. */
 			if (!(action_flags & MLX5_FLOW_MODIFY_HDR_ACTIONS))
 				++actions_n;
+			if (action_flags & MLX5_FLOW_ACTION_SAMPLE)
+				modify_after_mirror = 1;
 			action_flags |= MLX5_FLOW_ACTION_SET_TAG;
 			rw_act_num += MLX5_ACT_NUM_SET_TAG;
 			break;
@@ -6635,6 +6652,8 @@ struct mlx5_hlist_entry *
 					RTE_FLOW_ACTION_TYPE_SET_MAC_SRC ?
 						MLX5_FLOW_ACTION_SET_MAC_SRC :
 						MLX5_FLOW_ACTION_SET_MAC_DST;
+			if (action_flags & MLX5_FLOW_ACTION_SAMPLE)
+				modify_after_mirror = 1;
 			/*
 			 * Even if the source and destination MAC addresses have
 			 * overlap in the header with 4B alignment, the convert
@@ -6655,6 +6674,8 @@ struct mlx5_hlist_entry *
 			/* Count all modify-header actions as one action. */
 			if (!(action_flags & MLX5_FLOW_MODIFY_HDR_ACTIONS))
 				++actions_n;
+			if (action_flags & MLX5_FLOW_ACTION_SAMPLE)
+				modify_after_mirror = 1;
 			action_flags |= actions->type ==
 					RTE_FLOW_ACTION_TYPE_SET_IPV4_SRC ?
 						MLX5_FLOW_ACTION_SET_IPV4_SRC :
@@ -6678,6 +6699,8 @@ struct mlx5_hlist_entry *
 			/* Count all modify-header actions as one action. */
 			if (!(action_flags & MLX5_FLOW_MODIFY_HDR_ACTIONS))
 				++actions_n;
+			if (action_flags & MLX5_FLOW_ACTION_SAMPLE)
+				modify_after_mirror = 1;
 			action_flags |= actions->type ==
 					RTE_FLOW_ACTION_TYPE_SET_IPV6_SRC ?
 						MLX5_FLOW_ACTION_SET_IPV6_SRC :
@@ -6695,6 +6718,8 @@ struct mlx5_hlist_entry *
 			/* Count all modify-header actions as one action. */
 			if (!(action_flags & MLX5_FLOW_MODIFY_HDR_ACTIONS))
 				++actions_n;
+			if (action_flags & MLX5_FLOW_ACTION_SAMPLE)
+				modify_after_mirror = 1;
 			action_flags |= actions->type ==
 					RTE_FLOW_ACTION_TYPE_SET_TP_SRC ?
 						MLX5_FLOW_ACTION_SET_TP_SRC :
@@ -6712,6 +6737,8 @@ struct mlx5_hlist_entry *
 			/* Count all modify-header actions as one action. */
 			if (!(action_flags & MLX5_FLOW_MODIFY_HDR_ACTIONS))
 				++actions_n;
+			if (action_flags & MLX5_FLOW_ACTION_SAMPLE)
+				modify_after_mirror = 1;
 			action_flags |= actions->type ==
 					RTE_FLOW_ACTION_TYPE_SET_TTL ?
 						MLX5_FLOW_ACTION_SET_TTL :
@@ -6725,6 +6752,12 @@ struct mlx5_hlist_entry *
 							   error);
 			if (ret)
 				return ret;
+			if ((action_flags & MLX5_FLOW_ACTION_SAMPLE) &&
+			    fdb_mirror_limit)
+				return rte_flow_error_set(error, EINVAL,
+						  RTE_FLOW_ERROR_TYPE_ACTION,
+						  NULL,
+						  "sample and jump action combination is not supported");
 			++actions_n;
 			action_flags |= MLX5_FLOW_ACTION_JUMP;
 			break;
@@ -6740,6 +6773,8 @@ struct mlx5_hlist_entry *
 			/* Count all modify-header actions as one action. */
 			if (!(action_flags & MLX5_FLOW_MODIFY_HDR_ACTIONS))
 				++actions_n;
+			if (action_flags & MLX5_FLOW_ACTION_SAMPLE)
+				modify_after_mirror = 1;
 			action_flags |= actions->type ==
 					RTE_FLOW_ACTION_TYPE_INC_TCP_SEQ ?
 						MLX5_FLOW_ACTION_INC_TCP_SEQ :
@@ -6758,6 +6793,8 @@ struct mlx5_hlist_entry *
 			/* Count all modify-header actions as one action. */
 			if (!(action_flags & MLX5_FLOW_MODIFY_HDR_ACTIONS))
 				++actions_n;
+			if (action_flags & MLX5_FLOW_ACTION_SAMPLE)
+				modify_after_mirror = 1;
 			action_flags |= actions->type ==
 					RTE_FLOW_ACTION_TYPE_INC_TCP_ACK ?
 						MLX5_FLOW_ACTION_INC_TCP_ACK :
@@ -6811,6 +6848,8 @@ struct mlx5_hlist_entry *
 			/* Count all modify-header actions as one action. */
 			if (!(action_flags & MLX5_FLOW_MODIFY_HDR_ACTIONS))
 				++actions_n;
+			if (action_flags & MLX5_FLOW_ACTION_SAMPLE)
+				modify_after_mirror = 1;
 			action_flags |= MLX5_FLOW_ACTION_SET_IPV4_DSCP;
 			rw_act_num += MLX5_ACT_NUM_SET_DSCP;
 			break;
@@ -6825,6 +6864,8 @@ struct mlx5_hlist_entry *
 			/* Count all modify-header actions as one action. */
 			if (!(action_flags & MLX5_FLOW_MODIFY_HDR_ACTIONS))
 				++actions_n;
+			if (action_flags & MLX5_FLOW_ACTION_SAMPLE)
+				modify_after_mirror = 1;
 			action_flags |= MLX5_FLOW_ACTION_SET_IPV6_DSCP;
 			rw_act_num += MLX5_ACT_NUM_SET_DSCP;
 			break;
@@ -6833,6 +6874,7 @@ struct mlx5_hlist_entry *
 							     actions, dev,
 							     attr, item_flags,
 							     rss, &sample_rss,
+							     &fdb_mirror_limit,
 							     error);
 			if (ret < 0)
 				return ret;
@@ -7031,6 +7073,11 @@ struct mlx5_hlist_entry *
 					  NULL, "too many header modify"
 					  " actions to support");
 	}
+	/* Eswitch egress mirror and modify flow has limitation on CX5 */
+	if (fdb_mirror_limit && modify_after_mirror)
+		return rte_flow_error_set(error, EINVAL,
+				RTE_FLOW_ERROR_TYPE_ACTION, NULL,
+				"sample before modify action is not supported");
 	return 0;
 }
 
-- 
1.8.3.1


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

* Re: [dpdk-stable] [PATCH] net/mlx5: fix E-Switch egress mirror flow validation
  2021-02-03  8:29 [dpdk-stable] [PATCH] net/mlx5: fix E-Switch egress mirror flow validation Jiawei Wang
@ 2021-02-04 14:41 ` Raslan Darawsheh
  0 siblings, 0 replies; 2+ messages in thread
From: Raslan Darawsheh @ 2021-02-04 14:41 UTC (permalink / raw)
  To: Jiawei(Jonny) Wang, Slava Ovsiienko, Matan Azrad, Ori Kam,
	NBU-Contact-Thomas Monjalon
  Cc: dev, stable

Hi,

> -----Original Message-----
> From: Jiawei Wang <jiaweiw@nvidia.com>
> Sent: Wednesday, February 3, 2021 10:29 AM
> To: Slava Ovsiienko <viacheslavo@nvidia.com>; Matan Azrad
> <matan@nvidia.com>; Ori Kam <orika@nvidia.com>; NBU-Contact-Thomas
> Monjalon <thomas@monjalon.net>
> Cc: dev@dpdk.org; Raslan Darawsheh <rasland@nvidia.com>;
> stable@dpdk.org
> Subject: [PATCH] net/mlx5: fix E-Switch egress mirror flow validation
> 
> The stored metadata in all registers C were lost in E-Switch egress
> mirroring flows due to HW limitation. The register C0 keeps the
> source vport index that also was used as one of the flow matcher.
> 
> While sample action and jump action (jump to table X) was in the
> E-Switch egress flow, the flow in the next table X wasn't hit since
> source vport value lost.
> 
> The modify actions after sample action should be applied to the packet
> on normal path, not to the sampled packet. In order to support this
> mlx5 PMD splits the flow into sub flows and jump action is engaged
> implicitly, causing malfunction due to registers corruption.
> 
> This patch adds the validation the for E-Switch mirroring jump egress
> flow, and checks for this hidden jump as well and reject the flows with
> modify actions after sampling.
> 
> Fixes: 6a951567c159 ("net/mlx5: support E-Switch mirroring and jump in one
> flow")
> Cc: stable@dpdk.org
> 
> Signed-off-by: Jiawei Wang <jiaweiw@nvidia.com>
> Acked-by: Viacheslav Ovsiienko <viacheslavo@nvidia.com>
> ---
Patch applied to next-net-mlx,


Kindest regards,
Raslan Darawsheh

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

end of thread, other threads:[~2021-02-04 14:41 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-02-03  8:29 [dpdk-stable] [PATCH] net/mlx5: fix E-Switch egress mirror flow validation Jiawei Wang
2021-02-04 14:41 ` 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).