[PATCH] net/mlx5: fix the LACP redirection in Rx domain

[PATCH] net/mlx5: fix the LACP redirection in Rx domain

[Bing Zhao]

When the "lacp_by_user" is not set from the application in bond
mode, the LACP traffic should be handled by the kernel driver by
default.

This commit adds the missing support in the template API when
"dv_flow_en=2". The behavior will be the same as that in the DV
mode with "dv_flow_en=1". The LACP packets will be redirected to the
kernel when starting the steering in the NIC Rx domain.

With this commit, the DEFAULT_MISS action usage is refactored a bit.
In the HWS, one unique action can be created with supported bits set
in the "flag" per port. The *ROOT_FDB and *HWS_FDB flag bits will
only be set when the port is in switchdev mode and working as the
E-Switch manager proxy port. The SF/VF and all other representors
won't have the FDB flag bits when creating the DEFAULT_MISS action.

Fixes: 9fa7c1cddb85 ("net/mlx5: create control flow rules with HWS")
Cc: dsosnowski@nvidia.com
Cc: stable@dpdk.org

Signed-off-by: Bing Zhao <bingz@nvidia.com>
---
 drivers/net/mlx5/linux/mlx5_os.c |   8 +-
 drivers/net/mlx5/mlx5.h          |   1 +
 drivers/net/mlx5/mlx5_flow.h     |   1 +
 drivers/net/mlx5/mlx5_flow_hw.c  | 249 +++++++++++++++++++++++++++++--
 drivers/net/mlx5/mlx5_trigger.c  |   3 +
 5 files changed, 249 insertions(+), 13 deletions(-)

diff --git a/drivers/net/mlx5/linux/mlx5_os.c b/drivers/net/mlx5/linux/mlx5_os.c
index 07f31de5ae..ae82e1e5d8 100644
--- a/drivers/net/mlx5/linux/mlx5_os.c
+++ b/drivers/net/mlx5/linux/mlx5_os.c
@@ -474,6 +474,10 @@ mlx5_alloc_shared_dr(struct mlx5_priv *priv)
 	err = mlx5_alloc_table_hash_list(priv);
 	if (err)
 		goto error;
+	sh->default_miss_action =
+			mlx5_glue->dr_create_flow_action_default_miss();
+	if (!sh->default_miss_action)
+		DRV_LOG(WARNING, "Default miss action is not supported.");
 	/* The resources below are only valid with DV support. */
 #ifdef HAVE_IBV_FLOW_DV_SUPPORT
 	/* Init shared flex parsers list, no need lcore_share */
@@ -600,10 +604,6 @@ mlx5_alloc_shared_dr(struct mlx5_priv *priv)
 
 	__mlx5_discovery_misc5_cap(priv);
 #endif /* HAVE_MLX5DV_DR */
-	sh->default_miss_action =
-			mlx5_glue->dr_create_flow_action_default_miss();
-	if (!sh->default_miss_action)
-		DRV_LOG(WARNING, "Default miss action is not supported.");
 	LIST_INIT(&sh->shared_rxqs);
 	return 0;
 error:
diff --git a/drivers/net/mlx5/mlx5.h b/drivers/net/mlx5/mlx5.h
index 795748eddc..f0d63a0ba5 100644
--- a/drivers/net/mlx5/mlx5.h
+++ b/drivers/net/mlx5/mlx5.h
@@ -1860,6 +1860,7 @@ struct mlx5_priv {
 	struct rte_flow_template_table *hw_esw_sq_miss_tbl;
 	struct rte_flow_template_table *hw_esw_zero_tbl;
 	struct rte_flow_template_table *hw_tx_meta_cpy_tbl;
+	struct rte_flow_template_table *hw_lacp_rx_tbl;
 	struct rte_flow_pattern_template *hw_tx_repr_tagging_pt;
 	struct rte_flow_actions_template *hw_tx_repr_tagging_at;
 	struct rte_flow_template_table *hw_tx_repr_tagging_tbl;
diff --git a/drivers/net/mlx5/mlx5_flow.h b/drivers/net/mlx5/mlx5_flow.h
index 8c0b9a4b60..6dde9de688 100644
--- a/drivers/net/mlx5/mlx5_flow.h
+++ b/drivers/net/mlx5/mlx5_flow.h
@@ -2880,6 +2880,7 @@ int mlx5_flow_hw_esw_destroy_sq_miss_flow(struct rte_eth_dev *dev,
 int mlx5_flow_hw_esw_create_default_jump_flow(struct rte_eth_dev *dev);
 int mlx5_flow_hw_create_tx_default_mreg_copy_flow(struct rte_eth_dev *dev);
 int mlx5_flow_hw_tx_repr_matching_flow(struct rte_eth_dev *dev, uint32_t sqn, bool external);
+int mlx5_flow_hw_lacp_rx_flow(struct rte_eth_dev *dev);
 int mlx5_flow_actions_validate(struct rte_eth_dev *dev,
 		const struct rte_flow_actions_template_attr *attr,
 		const struct rte_flow_action actions[],
diff --git a/drivers/net/mlx5/mlx5_flow_hw.c b/drivers/net/mlx5/mlx5_flow_hw.c
index c35064518a..d72f0a66fb 100644
--- a/drivers/net/mlx5/mlx5_flow_hw.c
+++ b/drivers/net/mlx5/mlx5_flow_hw.c
@@ -2454,6 +2454,15 @@ __flow_hw_actions_translate(struct rte_eth_dev *dev,
 								     dr_pos))
 				goto err;
 			break;
+		case MLX5_RTE_FLOW_ACTION_TYPE_DEFAULT_MISS:
+			/* Internal, can be skipped. */
+			if (!!attr->group) {
+				DRV_LOG(ERR, "DEFAULT MISS action is only"
+					" supported in root table.");
+				goto err;
+			}
+			acts->rule_acts[dr_pos].action = priv->hw_def_miss;
+			break;
 		case RTE_FLOW_ACTION_TYPE_END:
 			actions_end = true;
 			break;
@@ -5531,6 +5540,34 @@ flow_hw_validate_action_push_vlan(struct rte_eth_dev *dev,
 #undef X_FIELD
 }
 
+static int
+flow_hw_validate_action_default_miss(struct rte_eth_dev *dev,
+				     const struct rte_flow_actions_template_attr *attr,
+				     uint64_t action_flags,
+				     struct rte_flow_error *error)
+{
+	/*
+	 * The private DEFAULT_MISS action is used internally for LACP in control
+	 * flows. So this validation can be ignored. It can be kept right now since
+	 * the validation will be done only once.
+	 */
+	struct mlx5_priv *priv = dev->data->dev_private;
+
+	if (!attr->ingress || attr->egress || attr->transfer)
+		return rte_flow_error_set(error, EINVAL,
+					  RTE_FLOW_ERROR_TYPE_ACTION, NULL,
+					  "DEFAULT MISS is only supported in ingress.");
+	if (!priv->hw_def_miss)
+		return rte_flow_error_set(error, EINVAL,
+					  RTE_FLOW_ERROR_TYPE_ACTION, NULL,
+					  "DEFAULT MISS action does not exist.");
+	if (action_flags & MLX5_FLOW_FATE_ACTIONS)
+		return rte_flow_error_set(error, EINVAL,
+					  RTE_FLOW_ERROR_TYPE_ACTION, NULL,
+					  "DEFAULT MISS should be the only termination.");
+	return 0;
+}
+
 static int
 mlx5_flow_hw_actions_validate(struct rte_eth_dev *dev,
 			      const struct rte_flow_actions_template_attr *attr,
@@ -5568,7 +5605,7 @@ mlx5_flow_hw_actions_validate(struct rte_eth_dev *dev,
 						  RTE_FLOW_ERROR_TYPE_ACTION,
 						  action,
 						  "mask type does not match action type");
-		switch (action->type) {
+		switch ((int)action->type) {
 		case RTE_FLOW_ACTION_TYPE_VOID:
 			break;
 		case RTE_FLOW_ACTION_TYPE_INDIRECT_LIST:
@@ -5735,6 +5772,13 @@ mlx5_flow_hw_actions_validate(struct rte_eth_dev *dev,
 		case RTE_FLOW_ACTION_TYPE_END:
 			actions_end = true;
 			break;
+		case MLX5_RTE_FLOW_ACTION_TYPE_DEFAULT_MISS:
+			ret = flow_hw_validate_action_default_miss(dev, attr,
+								   action_flags, error);
+			if (ret < 0)
+				return ret;
+			action_flags |= MLX5_FLOW_ACTION_DEFAULT_MISS;
+			break;
 		default:
 			return rte_flow_error_set(error, ENOTSUP,
 						  RTE_FLOW_ERROR_TYPE_ACTION,
@@ -5754,8 +5798,7 @@ flow_hw_actions_validate(struct rte_eth_dev *dev,
 			 const struct rte_flow_action masks[],
 			 struct rte_flow_error *error)
 {
-	return mlx5_flow_hw_actions_validate(dev, attr, actions, masks, NULL,
-					     error);
+	return mlx5_flow_hw_actions_validate(dev, attr, actions, masks, NULL, error);
 }
 
 
@@ -5907,7 +5950,7 @@ flow_hw_dr_actions_template_create(struct rte_eth_dev *dev,
 
 		if (curr_off >= MLX5_HW_MAX_ACTS)
 			goto err_actions_num;
-		switch (at->actions[i].type) {
+		switch ((int)at->actions[i].type) {
 		case RTE_FLOW_ACTION_TYPE_VOID:
 			break;
 		case RTE_FLOW_ACTION_TYPE_INDIRECT_LIST:
@@ -5998,6 +6041,10 @@ flow_hw_dr_actions_template_create(struct rte_eth_dev *dev,
 			}
 			at->dr_off[i] = cnt_off;
 			break;
+		case MLX5_RTE_FLOW_ACTION_TYPE_DEFAULT_MISS:
+			at->dr_off[i] = curr_off;
+			action_types[curr_off++] = MLX5DR_ACTION_TYP_MISS;
+			break;
 		default:
 			type = mlx5_hw_dr_action_types[at->actions[i].type];
 			at->dr_off[i] = curr_off;
@@ -7773,6 +7820,42 @@ flow_hw_create_tx_default_mreg_copy_pattern_template(struct rte_eth_dev *dev,
 	return flow_hw_pattern_template_create(dev, &tx_pa_attr, eth_all, error);
 }
 
+/*
+ * Creating a flow pattern template with all LACP packets matching, only for NIC
+ * ingress domain.
+ *
+ * @param dev
+ *   Pointer to Ethernet device.
+ * @param error
+ *   Pointer to error structure.
+ *
+ * @return
+ *   Pointer to flow pattern template on success, NULL otherwise.
+ */
+static struct rte_flow_pattern_template *
+flow_hw_create_lacp_rx_pattern_template(struct rte_eth_dev *dev, struct rte_flow_error *error)
+{
+	struct rte_flow_pattern_template_attr pa_attr = {
+		.relaxed_matching = 0,
+		.ingress = 1,
+	};
+	struct rte_flow_item_eth lacp_mask = {
+		.dst.addr_bytes = "\x00\x00\x00\x00\x00\x00",
+		.src.addr_bytes = "\x00\x00\x00\x00\x00\x00",
+		.type = 0xFFFF,
+	};
+	struct rte_flow_item eth_all[] = {
+		[0] = {
+			.type = RTE_FLOW_ITEM_TYPE_ETH,
+			.mask = &lacp_mask,
+		},
+		[1] = {
+			.type = RTE_FLOW_ITEM_TYPE_END,
+		},
+	};
+	return flow_hw_pattern_template_create(dev, &pa_attr, eth_all, error);
+}
+
 /**
  * Creates a flow actions template with modify field action and masked jump action.
  * Modify field action sets the least significant bit of REG_C_0 (usable by user-space)
@@ -8042,6 +8125,38 @@ flow_hw_create_tx_default_mreg_copy_actions_template(struct rte_eth_dev *dev,
 					       masks, error);
 }
 
+/*
+ * Creating an actions template to use default miss to re-route packets to the
+ * kernel driver stack.
+ * On root table, only DEFAULT_MISS action can be used.
+ *
+ * @param dev
+ *   Pointer to Ethernet device.
+ * @param error
+ *   Pointer to error structure.
+ *
+ * @return
+ *   Pointer to flow actions template on success, NULL otherwise.
+ */
+static struct rte_flow_actions_template *
+flow_hw_create_lacp_rx_actions_template(struct rte_eth_dev *dev, struct rte_flow_error *error)
+{
+	struct rte_flow_actions_template_attr act_attr = {
+		.ingress = 1,
+	};
+	const struct rte_flow_action actions[] = {
+		[0] = {
+			.type = (enum rte_flow_action_type)
+				MLX5_RTE_FLOW_ACTION_TYPE_DEFAULT_MISS,
+		},
+		[1] = {
+			.type = RTE_FLOW_ACTION_TYPE_END,
+		},
+	};
+
+	return flow_hw_actions_template_create(dev, &act_attr, actions, actions, error);
+}
+
 /**
  * Creates a control flow table used to transfer traffic from E-Switch Manager
  * and TX queues from group 0 to group 1.
@@ -8200,6 +8315,43 @@ flow_hw_create_ctrl_jump_table(struct rte_eth_dev *dev,
 	return flow_hw_table_create(dev, &cfg, &it, 1, &at, 1, error);
 }
 
+/*
+ * Create a table on the root group to for the LACP traffic redirecting.
+ *
+ * @param dev
+ *   Pointer to Ethernet device.
+ * @param it
+ *   Pointer to flow pattern template.
+ * @param at
+ *   Pointer to flow actions template.
+ *
+ * @return
+ *   Pointer to flow table on success, NULL otherwise.
+ */
+static struct rte_flow_template_table *
+flow_hw_create_lacp_rx_table(struct rte_eth_dev *dev,
+			     struct rte_flow_pattern_template *it,
+			     struct rte_flow_actions_template *at,
+			     struct rte_flow_error *error)
+{
+	struct rte_flow_template_table_attr attr = {
+		.flow_attr = {
+			.group = 0,
+			.priority = 0,
+			.ingress = 1,
+			.egress = 0,
+			.transfer = 0,
+		},
+		.nb_flows = 1,
+	};
+	struct mlx5_flow_template_table_cfg cfg = {
+		.attr = attr,
+		.external = false,
+	};
+
+	return flow_hw_table_create(dev, &cfg, &it, 1, &at, 1, error);
+}
+
 /**
  * Creates a set of flow tables used to create control flows used
  * when E-Switch is engaged.
@@ -8220,10 +8372,12 @@ flow_hw_create_ctrl_tables(struct rte_eth_dev *dev, struct rte_flow_error *error
 	struct rte_flow_pattern_template *regc_sq_items_tmpl = NULL;
 	struct rte_flow_pattern_template *port_items_tmpl = NULL;
 	struct rte_flow_pattern_template *tx_meta_items_tmpl = NULL;
+	struct rte_flow_pattern_template *lacp_rx_items_tmpl = NULL;
 	struct rte_flow_actions_template *regc_jump_actions_tmpl = NULL;
 	struct rte_flow_actions_template *port_actions_tmpl = NULL;
 	struct rte_flow_actions_template *jump_one_actions_tmpl = NULL;
 	struct rte_flow_actions_template *tx_meta_actions_tmpl = NULL;
+	struct rte_flow_actions_template *lacp_rx_actions_tmpl = NULL;
 	uint32_t xmeta = priv->sh->config.dv_xmeta_en;
 	uint32_t repr_matching = priv->sh->config.repr_matching;
 	int ret;
@@ -8319,6 +8473,28 @@ flow_hw_create_ctrl_tables(struct rte_eth_dev *dev, struct rte_flow_error *error
 			goto err;
 		}
 	}
+	/* Create LACP default miss table. */
+	if (!priv->sh->config.lacp_by_user && priv->pf_bond >= 0) {
+		lacp_rx_items_tmpl = flow_hw_create_lacp_rx_pattern_template(dev, error);
+		if (!lacp_rx_items_tmpl) {
+			DRV_LOG(ERR, "port %u failed to create pattern template"
+				" for LACP Rx traffic", dev->data->port_id);
+			goto err;
+		}
+		lacp_rx_actions_tmpl = flow_hw_create_lacp_rx_actions_template(dev, error);
+		if (!lacp_rx_actions_tmpl) {
+			DRV_LOG(ERR, "port %u failed to create actions template"
+				" for LACP Rx traffic", dev->data->port_id);
+			goto err;
+		}
+		priv->hw_lacp_rx_tbl = flow_hw_create_lacp_rx_table(dev, lacp_rx_items_tmpl,
+								    lacp_rx_actions_tmpl, error);
+		if (!priv->hw_lacp_rx_tbl) {
+			DRV_LOG(ERR, "port %u failed to create template table for"
+				" for LACP Rx traffic", dev->data->port_id);
+			goto err;
+		}
+	}
 	return 0;
 err:
 	/* Do not overwrite the rte_errno. */
@@ -8327,6 +8503,10 @@ flow_hw_create_ctrl_tables(struct rte_eth_dev *dev, struct rte_flow_error *error
 		ret = rte_flow_error_set(error, EINVAL,
 					 RTE_FLOW_ERROR_TYPE_UNSPECIFIED, NULL,
 					 "Failed to create control tables.");
+	if (priv->hw_tx_meta_cpy_tbl) {
+		flow_hw_table_destroy(dev, priv->hw_tx_meta_cpy_tbl, NULL);
+		priv->hw_tx_meta_cpy_tbl = NULL;
+	}
 	if (priv->hw_esw_zero_tbl) {
 		flow_hw_table_destroy(dev, priv->hw_esw_zero_tbl, NULL);
 		priv->hw_esw_zero_tbl = NULL;
@@ -8339,6 +8519,8 @@ flow_hw_create_ctrl_tables(struct rte_eth_dev *dev, struct rte_flow_error *error
 		flow_hw_table_destroy(dev, priv->hw_esw_sq_miss_root_tbl, NULL);
 		priv->hw_esw_sq_miss_root_tbl = NULL;
 	}
+	if (lacp_rx_actions_tmpl)
+		flow_hw_actions_template_destroy(dev, lacp_rx_actions_tmpl, NULL);
 	if (tx_meta_actions_tmpl)
 		flow_hw_actions_template_destroy(dev, tx_meta_actions_tmpl, NULL);
 	if (jump_one_actions_tmpl)
@@ -8347,6 +8529,8 @@ flow_hw_create_ctrl_tables(struct rte_eth_dev *dev, struct rte_flow_error *error
 		flow_hw_actions_template_destroy(dev, port_actions_tmpl, NULL);
 	if (regc_jump_actions_tmpl)
 		flow_hw_actions_template_destroy(dev, regc_jump_actions_tmpl, NULL);
+	if (lacp_rx_items_tmpl)
+		flow_hw_pattern_template_destroy(dev, lacp_rx_items_tmpl, NULL);
 	if (tx_meta_items_tmpl)
 		flow_hw_pattern_template_destroy(dev, tx_meta_items_tmpl, NULL);
 	if (port_items_tmpl)
@@ -8998,6 +9182,7 @@ flow_hw_configure(struct rte_eth_dev *dev,
 	struct rte_flow_queue_attr ctrl_queue_attr = {0};
 	bool is_proxy = !!(priv->sh->config.dv_esw_en && priv->master);
 	int ret = 0;
+	uint32_t action_flags;
 
 	if (!port_attr || !nb_queue || !queue_attr) {
 		rte_errno = EINVAL;
@@ -9229,12 +9414,21 @@ flow_hw_configure(struct rte_eth_dev *dev,
 		if (ret)
 			goto err;
 	}
+	/*
+	 * DEFAULT_MISS action have different behaviors in different domains.
+	 * In FDB, it will steering the packets to the E-switch manager.
+	 * In NIC Rx root, it will steering the packet to the kernel driver stack.
+	 * An action with all bits set in the flag can be created and the HWS
+	 * layer will translate it properly when being used in different rules.
+	 */
+	action_flags = MLX5DR_ACTION_FLAG_ROOT_RX | MLX5DR_ACTION_FLAG_HWS_RX |
+		       MLX5DR_ACTION_FLAG_ROOT_TX | MLX5DR_ACTION_FLAG_HWS_TX;
+	if (is_proxy)
+		action_flags |= (MLX5DR_ACTION_FLAG_ROOT_FDB | MLX5DR_ACTION_FLAG_HWS_FDB);
+	priv->hw_def_miss = mlx5dr_action_create_default_miss(priv->dr_ctx, action_flags);
+	if (!priv->hw_def_miss)
+		goto err;
 	if (is_proxy) {
-		/* Only supported on proxy port. */
-		priv->hw_def_miss = mlx5dr_action_create_default_miss
-			(priv->dr_ctx, MLX5DR_ACTION_FLAG_HWS_FDB);
-		if (!priv->hw_def_miss)
-			goto err;
 		ret = flow_hw_create_vport_actions(priv);
 		if (ret) {
 			rte_flow_error_set(error, -ret, RTE_FLOW_ERROR_TYPE_UNSPECIFIED,
@@ -11956,6 +12150,43 @@ mlx5_flow_hw_tx_repr_matching_flow(struct rte_eth_dev *dev, uint32_t sqn, bool e
 					items, 0, actions, 0, &flow_info, external);
 }
 
+int
+mlx5_flow_hw_lacp_rx_flow(struct rte_eth_dev *dev)
+{
+	struct mlx5_priv *priv = dev->data->dev_private;
+	struct rte_flow_item_eth lacp_item = {
+		.type = RTE_BE16(RTE_ETHER_TYPE_SLOW),
+	};
+	struct rte_flow_item eth_lacp[] = {
+		[0] = {
+			.type = RTE_FLOW_ITEM_TYPE_ETH,
+			.spec = &lacp_item,
+			.mask = &lacp_item,
+		},
+		[1] = {
+			.type = RTE_FLOW_ITEM_TYPE_END,
+		},
+	};
+	struct rte_flow_action miss_action[] = {
+		[0] = {
+			.type = (enum rte_flow_action_type)
+				MLX5_RTE_FLOW_ACTION_TYPE_DEFAULT_MISS,
+		},
+		[1] = {
+			.type = RTE_FLOW_ACTION_TYPE_END,
+		},
+	};
+	struct mlx5_hw_ctrl_flow_info flow_info = {
+		.type = MLX5_HW_CTRL_FLOW_TYPE_LACP_RX,
+	};
+
+	MLX5_ASSERT(priv->master);
+	if (!priv->dr_ctx || !priv->hw_lacp_rx_tbl)
+		return 0;
+	return flow_hw_create_ctrl_flow(dev, dev, priv->hw_lacp_rx_tbl, eth_lacp, 0,
+					miss_action, 0, &flow_info, false);
+}
+
 static uint32_t
 __calc_pattern_flags(const enum mlx5_flow_ctrl_rx_eth_pattern_type eth_pattern_type)
 {
diff --git a/drivers/net/mlx5/mlx5_trigger.c b/drivers/net/mlx5/mlx5_trigger.c
index 35733b0604..5ac25d7e2d 100644
--- a/drivers/net/mlx5/mlx5_trigger.c
+++ b/drivers/net/mlx5/mlx5_trigger.c
@@ -1524,6 +1524,9 @@ mlx5_traffic_enable_hws(struct rte_eth_dev *dev)
 	}
 	if (priv->isolated)
 		return 0;
+	if (!priv->sh->config.lacp_by_user && priv->pf_bond >= 0)
+		if (mlx5_flow_hw_lacp_rx_flow(dev))
+			goto error;
 	if (dev->data->promiscuous)
 		flags |= MLX5_CTRL_PROMISCUOUS;
 	if (dev->data->all_multicast)
-- 
2.34.1

RE: [PATCH] net/mlx5: fix the LACP redirection in Rx domain

[Suanming Mou]

Hi,

One small comment, v2 is needed.

> -----Original Message-----
> From: Bing Zhao <bingz@nvidia.com>
> Sent: Monday, November 13, 2023 8:42 PM
> To: Matan Azrad <matan@nvidia.com>; Slava Ovsiienko
> <viacheslavo@nvidia.com>; Raslan Darawsheh <rasland@nvidia.com>; Suanming
> Mou <suanmingm@nvidia.com>; Ori Kam <orika@nvidia.com>
> Cc: dev@dpdk.org; Dariusz Sosnowski <dsosnowski@nvidia.com>;
> stable@dpdk.org
> Subject: [PATCH] net/mlx5: fix the LACP redirection in Rx domain
> 
> When the "lacp_by_user" is not set from the application in bond mode, the LACP
> traffic should be handled by the kernel driver by default.
> 
> This commit adds the missing support in the template API when "dv_flow_en=2".
> The behavior will be the same as that in the DV mode with "dv_flow_en=1". The
> LACP packets will be redirected to the kernel when starting the steering in the
> NIC Rx domain.
> 
> With this commit, the DEFAULT_MISS action usage is refactored a bit.
> In the HWS, one unique action can be created with supported bits set in the "flag"
> per port. The *ROOT_FDB and *HWS_FDB flag bits will only be set when the port
> is in switchdev mode and working as the E-Switch manager proxy port. The SF/VF
> and all other representors won't have the FDB flag bits when creating the
> DEFAULT_MISS action.
> 
> Fixes: 9fa7c1cddb85 ("net/mlx5: create control flow rules with HWS")
> Cc: dsosnowski@nvidia.com
> Cc: stable@dpdk.org
> 
> Signed-off-by: Bing Zhao <bingz@nvidia.com>
> ---
>  drivers/net/mlx5/linux/mlx5_os.c |   8 +-
>  drivers/net/mlx5/mlx5.h          |   1 +
>  drivers/net/mlx5/mlx5_flow.h     |   1 +
>  drivers/net/mlx5/mlx5_flow_hw.c  | 249 +++++++++++++++++++++++++++++--
>  drivers/net/mlx5/mlx5_trigger.c  |   3 +
>  5 files changed, 249 insertions(+), 13 deletions(-)
> 
> diff --git a/drivers/net/mlx5/linux/mlx5_os.c b/drivers/net/mlx5/linux/mlx5_os.c

[snip]

> @@ -8327,6 +8503,10 @@ flow_hw_create_ctrl_tables(struct rte_eth_dev *dev,
> struct rte_flow_error *error
>  		ret = rte_flow_error_set(error, EINVAL,
> 
> RTE_FLOW_ERROR_TYPE_UNSPECIFIED, NULL,
>  					 "Failed to create control tables.");
> +	if (priv->hw_tx_meta_cpy_tbl) {

I assume it should be hw_lacp_rx_tbl here.  Can you please send v2 for that?
And LGTM for left code.  It's OK to add my ack for v2.
Acked-by: Suanming Mou <suanmingm@nvidia.com>


> +		flow_hw_table_destroy(dev, priv->hw_tx_meta_cpy_tbl, NULL);
> +		priv->hw_tx_meta_cpy_tbl = NULL;
> +	}

[snip]

> 2.34.1

RE: [PATCH] net/mlx5: fix the LACP redirection in Rx domain

[Bing Zhao]

Hi Suanming,

My answers are inline.

> -----Original Message-----
> From: Suanming Mou <suanmingm@nvidia.com>
> Sent: Monday, November 13, 2023 9:14 PM
> To: Bing Zhao <bingz@nvidia.com>; Matan Azrad <matan@nvidia.com>; Slava
> Ovsiienko <viacheslavo@nvidia.com>; Raslan Darawsheh
> <rasland@nvidia.com>; Ori Kam <orika@nvidia.com>
> Cc: dev@dpdk.org; Dariusz Sosnowski <dsosnowski@nvidia.com>;
> stable@dpdk.org
> Subject: RE: [PATCH] net/mlx5: fix the LACP redirection in Rx domain
> 
> Hi,
> 
> One small comment, v2 is needed.
> 
> > -----Original Message-----
> > From: Bing Zhao <bingz@nvidia.com>
> > Sent: Monday, November 13, 2023 8:42 PM
> > To: Matan Azrad <matan@nvidia.com>; Slava Ovsiienko
> > <viacheslavo@nvidia.com>; Raslan Darawsheh <rasland@nvidia.com>;
> > Suanming Mou <suanmingm@nvidia.com>; Ori Kam <orika@nvidia.com>
> > Cc: dev@dpdk.org; Dariusz Sosnowski <dsosnowski@nvidia.com>;
> > stable@dpdk.org
> > Subject: [PATCH] net/mlx5: fix the LACP redirection in Rx domain
> >
> > When the "lacp_by_user" is not set from the application in bond mode,
> > the LACP traffic should be handled by the kernel driver by default.
> >
> > This commit adds the missing support in the template API when
> "dv_flow_en=2".
> > The behavior will be the same as that in the DV mode with
> > "dv_flow_en=1". The LACP packets will be redirected to the kernel when
> > starting the steering in the NIC Rx domain.
> >
> > With this commit, the DEFAULT_MISS action usage is refactored a bit.
> > In the HWS, one unique action can be created with supported bits set in the
> "flag"
> > per port. The *ROOT_FDB and *HWS_FDB flag bits will only be set when
> > the port is in switchdev mode and working as the E-Switch manager
> > proxy port. The SF/VF and all other representors won't have the FDB
> > flag bits when creating the DEFAULT_MISS action.
> >
> > Fixes: 9fa7c1cddb85 ("net/mlx5: create control flow rules with HWS")
> > Cc: dsosnowski@nvidia.com
> > Cc: stable@dpdk.org
> >
> > Signed-off-by: Bing Zhao <bingz@nvidia.com>
> > ---
> >  drivers/net/mlx5/linux/mlx5_os.c |   8 +-
> >  drivers/net/mlx5/mlx5.h          |   1 +
> >  drivers/net/mlx5/mlx5_flow.h     |   1 +
> >  drivers/net/mlx5/mlx5_flow_hw.c  | 249
> +++++++++++++++++++++++++++++--
> >  drivers/net/mlx5/mlx5_trigger.c  |   3 +
> >  5 files changed, 249 insertions(+), 13 deletions(-)
> >
> > diff --git a/drivers/net/mlx5/linux/mlx5_os.c
> > b/drivers/net/mlx5/linux/mlx5_os.c
> 
> [snip]
> 
> > @@ -8327,6 +8503,10 @@ flow_hw_create_ctrl_tables(struct rte_eth_dev
> > *dev, struct rte_flow_error *error
> >  		ret = rte_flow_error_set(error, EINVAL,
> >
> > RTE_FLOW_ERROR_TYPE_UNSPECIFIED, NULL,
> >  					 "Failed to create control tables.");
> > +	if (priv->hw_tx_meta_cpy_tbl) {
> 
> I assume it should be hw_lacp_rx_tbl here.  Can you please send v2 for that?
> And LGTM for left code.  It's OK to add my ack for v2.
> Acked-by: Suanming Mou <suanmingm@nvidia.com>

Since it is a "goto" label, when arriving here from the LACP error branch, the previous tables were created successfully.
The LACP table is the last one from the code and it will never be NULL when reaching here. What should be rollbaked is the previous one.

This is some disadvantage of using goto in the code, not quite straightforward.

> 
> 
> > +		flow_hw_table_destroy(dev, priv->hw_tx_meta_cpy_tbl,
> NULL);
> > +		priv->hw_tx_meta_cpy_tbl = NULL;
> > +	}
> 
> [snip]
> 
> > 2.34.1

BR. Bing

RE: [PATCH] net/mlx5: fix the LACP redirection in Rx domain

[Suanming Mou]

Hi,

> -----Original Message-----
> From: Bing Zhao <bingz@nvidia.com>
> Sent: Monday, November 13, 2023 9:51 PM
> To: Suanming Mou <suanmingm@nvidia.com>; Matan Azrad
> <matan@nvidia.com>; Slava Ovsiienko <viacheslavo@nvidia.com>; Raslan
> Darawsheh <rasland@nvidia.com>; Ori Kam <orika@nvidia.com>
> Cc: dev@dpdk.org; Dariusz Sosnowski <dsosnowski@nvidia.com>;
> stable@dpdk.org
> Subject: RE: [PATCH] net/mlx5: fix the LACP redirection in Rx domain
> 
> Hi Suanming,
> 
> My answers are inline.
> 
> > -----Original Message-----
> > From: Suanming Mou <suanmingm@nvidia.com>
> > Sent: Monday, November 13, 2023 9:14 PM
> > To: Bing Zhao <bingz@nvidia.com>; Matan Azrad <matan@nvidia.com>;
> > Slava Ovsiienko <viacheslavo@nvidia.com>; Raslan Darawsheh
> > <rasland@nvidia.com>; Ori Kam <orika@nvidia.com>
> > Cc: dev@dpdk.org; Dariusz Sosnowski <dsosnowski@nvidia.com>;
> > stable@dpdk.org
> > Subject: RE: [PATCH] net/mlx5: fix the LACP redirection in Rx domain
> >
> > Hi,
> >
> > One small comment, v2 is needed.
> >
> > > -----Original Message-----
> > > From: Bing Zhao <bingz@nvidia.com>
> > > Sent: Monday, November 13, 2023 8:42 PM
> > > To: Matan Azrad <matan@nvidia.com>; Slava Ovsiienko
> > > <viacheslavo@nvidia.com>; Raslan Darawsheh <rasland@nvidia.com>;
> > > Suanming Mou <suanmingm@nvidia.com>; Ori Kam <orika@nvidia.com>
> > > Cc: dev@dpdk.org; Dariusz Sosnowski <dsosnowski@nvidia.com>;
> > > stable@dpdk.org
> > > Subject: [PATCH] net/mlx5: fix the LACP redirection in Rx domain
> > >
> > > When the "lacp_by_user" is not set from the application in bond
> > > mode, the LACP traffic should be handled by the kernel driver by default.
> > >
> > > This commit adds the missing support in the template API when
> > "dv_flow_en=2".
> > > The behavior will be the same as that in the DV mode with
> > > "dv_flow_en=1". The LACP packets will be redirected to the kernel
> > > when starting the steering in the NIC Rx domain.
> > >
> > > With this commit, the DEFAULT_MISS action usage is refactored a bit.
> > > In the HWS, one unique action can be created with supported bits set
> > > in the
> > "flag"
> > > per port. The *ROOT_FDB and *HWS_FDB flag bits will only be set when
> > > the port is in switchdev mode and working as the E-Switch manager
> > > proxy port. The SF/VF and all other representors won't have the FDB
> > > flag bits when creating the DEFAULT_MISS action.
> > >
> > > Fixes: 9fa7c1cddb85 ("net/mlx5: create control flow rules with HWS")
> > > Cc: dsosnowski@nvidia.com
> > > Cc: stable@dpdk.org
> > >
> > > Signed-off-by: Bing Zhao <bingz@nvidia.com>
> > > ---
> > >  drivers/net/mlx5/linux/mlx5_os.c |   8 +-
> > >  drivers/net/mlx5/mlx5.h          |   1 +
> > >  drivers/net/mlx5/mlx5_flow.h     |   1 +
> > >  drivers/net/mlx5/mlx5_flow_hw.c  | 249
> > +++++++++++++++++++++++++++++--
> > >  drivers/net/mlx5/mlx5_trigger.c  |   3 +
> > >  5 files changed, 249 insertions(+), 13 deletions(-)
> > >
> > > diff --git a/drivers/net/mlx5/linux/mlx5_os.c
> > > b/drivers/net/mlx5/linux/mlx5_os.c
> >
> > [snip]
> >
> > > @@ -8327,6 +8503,10 @@ flow_hw_create_ctrl_tables(struct rte_eth_dev
> > > *dev, struct rte_flow_error *error
> > >  		ret = rte_flow_error_set(error, EINVAL,
> > >
> > > RTE_FLOW_ERROR_TYPE_UNSPECIFIED, NULL,
> > >  					 "Failed to create control tables.");
> > > +	if (priv->hw_tx_meta_cpy_tbl) {
> >
> > I assume it should be hw_lacp_rx_tbl here.  Can you please send v2 for that?
> > And LGTM for left code.  It's OK to add my ack for v2.
> > Acked-by: Suanming Mou <suanmingm@nvidia.com>
> 
> Since it is a "goto" label, when arriving here from the LACP error branch, the
> previous tables were created successfully.
> The LACP table is the last one from the code and it will never be NULL when
> reaching here. What should be rollbaked is the previous one.
> 
> This is some disadvantage of using goto in the code, not quite straightforward.

OK, got it. Then no need for v2.  Thanks.

> 
> >
> >
> > > +		flow_hw_table_destroy(dev, priv->hw_tx_meta_cpy_tbl,
> > NULL);
> > > +		priv->hw_tx_meta_cpy_tbl = NULL;
> > > +	}
> >
> > [snip]
> >
> > > 2.34.1
> 
> BR. Bing

RE: [PATCH] net/mlx5: fix the LACP redirection in Rx domain

[Raslan Darawsheh]

Hi,

> -----Original Message-----
> From: Bing Zhao <bingz@nvidia.com>
> Sent: Monday, November 13, 2023 2:42 PM
> To: Matan Azrad <matan@nvidia.com>; Slava Ovsiienko
> <viacheslavo@nvidia.com>; Raslan Darawsheh <rasland@nvidia.com>;
> Suanming Mou <suanmingm@nvidia.com>; Ori Kam <orika@nvidia.com>
> Cc: dev@dpdk.org; Dariusz Sosnowski <dsosnowski@nvidia.com>;
> stable@dpdk.org
> Subject: [PATCH] net/mlx5: fix the LACP redirection in Rx domain
> 
> When the "lacp_by_user" is not set from the application in bond
> mode, the LACP traffic should be handled by the kernel driver by
> default.
> 
> This commit adds the missing support in the template API when
> "dv_flow_en=2". The behavior will be the same as that in the DV
> mode with "dv_flow_en=1". The LACP packets will be redirected to the
> kernel when starting the steering in the NIC Rx domain.
> 
> With this commit, the DEFAULT_MISS action usage is refactored a bit.
> In the HWS, one unique action can be created with supported bits set
> in the "flag" per port. The *ROOT_FDB and *HWS_FDB flag bits will
> only be set when the port is in switchdev mode and working as the
> E-Switch manager proxy port. The SF/VF and all other representors
> won't have the FDB flag bits when creating the DEFAULT_MISS action.
> 
> Fixes: 9fa7c1cddb85 ("net/mlx5: create control flow rules with HWS")
> Cc: dsosnowski@nvidia.com
> Cc: stable@dpdk.org
> 
> Signed-off-by: Bing Zhao <bingz@nvidia.com>

Patch applied to next-net-mlx,

Kindest regards
Raslan Darawsheh