[PATCH 3/3] net/mlx5: allow FDB RSS

[Bing Zhao]

Hi



> -----Original Message-----
> From: Suanming Mou <suanmingm at nvidia.com>
> Sent: Wednesday, February 26, 2025 9:43 PM
> To: Bing Zhao <bingz at nvidia.com>; Dariusz Sosnowski
> <dsosnowski at nvidia.com>; Slava Ovsiienko <viacheslavo at nvidia.com>; Ori Kam
> <orika at nvidia.com>; Matan Azrad <matan at nvidia.com>
> Cc: dev at dpdk.org; Raslan Darawsheh <rasland at nvidia.com>
> Subject: RE: [PATCH 3/3] net/mlx5: allow FDB RSS
> 
> 
> 
> > -----Original Message-----
> > From: Bing Zhao <bingz at nvidia.com>
> > Sent: Wednesday, February 26, 2025 6:50 PM
> > To: Suanming Mou <suanmingm at nvidia.com>; Dariusz Sosnowski
> > <dsosnowski at nvidia.com>; Slava Ovsiienko <viacheslavo at nvidia.com>; Ori
> > Kam <orika at nvidia.com>; Matan Azrad <matan at nvidia.com>
> > Cc: dev at dpdk.org; Raslan Darawsheh <rasland at nvidia.com>
> > Subject: RE: [PATCH 3/3] net/mlx5: allow FDB RSS
> >
> >
> >
> > > -----Original Message-----
> > > From: Suanming Mou <suanmingm at nvidia.com>
> > > Sent: Tuesday, February 25, 2025 8:45 AM
> > > To: Dariusz Sosnowski <dsosnowski at nvidia.com>; Slava Ovsiienko
> > > <viacheslavo at nvidia.com>; Bing Zhao <bingz at nvidia.com>; Ori Kam
> > > <orika at nvidia.com>; Matan Azrad <matan at nvidia.com>
> > > Cc: dev at dpdk.org; Raslan Darawsheh <rasland at nvidia.com>
> > > Subject: [PATCH 3/3] net/mlx5: allow FDB RSS
> > >
> > > RSS can be used in FDB Rx rules when JUMP_FDB_RX action to allow
> > > jump from FDB Tx to FDB Rx table.
> > >
> > > Different with NIC RSS, FDB RSS will not do the internal implicit
> > > metadata copy.
> > >
> > > This commit enables the FDB RSS if JUMP_FDB_RX is supported.
> > >
> > > Signed-off-by: Suanming Mou <suanmingm at nvidia.com>
> > > ---
> > >  drivers/net/mlx5/mlx5_flow.c    |  4 ++--
> > >  drivers/net/mlx5/mlx5_flow_dv.c | 32
> > > +++++++++++++++++--------------- drivers/net/mlx5/mlx5_flow_hw.c |
> > > 19 ++++++++++++++-----
> > >  3 files changed, 33 insertions(+), 22 deletions(-)
> > >
> > > diff --git a/drivers/net/mlx5/mlx5_flow.c
> > > b/drivers/net/mlx5/mlx5_flow.c index 3fbe89a9d4..9c6a4f39fb 100644
> > > --- a/drivers/net/mlx5/mlx5_flow.c
> > > +++ b/drivers/net/mlx5/mlx5_flow.c
> > > @@ -1977,9 +1977,9 @@ mlx5_flow_validate_action_mark(struct
> > > rte_eth_dev *dev,
> > >  					  RTE_FLOW_ERROR_TYPE_ATTR_EGRESS, NULL,
> > >  					  "mark action not supported for "
> > >  					  "egress");
> > > -	if (attr->transfer && mlx5_hws_active(dev))
> > > +	if (attr->transfer && !mlx5_hws_active(dev))
> > >  		return rte_flow_error_set(error, ENOTSUP,
> > > -					  RTE_FLOW_ERROR_TYPE_ATTR_EGRESS, NULL,
> > > +					  RTE_FLOW_ERROR_TYPE_ATTR_TRANSFER, NULL,
> > >  					  "non-template mark action not supported
> for
> > transfer");
> > >  	return 0;
> > >  }
> > > diff --git a/drivers/net/mlx5/mlx5_flow_dv.c
> > > b/drivers/net/mlx5/mlx5_flow_dv.c index 633c41e358..61d3101ce8
> > 100644
> > > --- a/drivers/net/mlx5/mlx5_flow_dv.c
> > > +++ b/drivers/net/mlx5/mlx5_flow_dv.c
> > > @@ -8939,21 +8939,23 @@ flow_dv_validate(struct rte_eth_dev *dev,
> > > const struct rte_flow_attr *attr,
> > >  						  RTE_FLOW_ERROR_TYPE_ACTION,
> > >  						  NULL,
> > >  						  "unsupported action MARK");
> > > -		if (action_flags & MLX5_FLOW_ACTION_QUEUE)
> > > -			return rte_flow_error_set(error, ENOTSUP,
> > > -						  RTE_FLOW_ERROR_TYPE_ACTION,
> > > -						  NULL,
> > > -						  "unsupported action QUEUE");
> > > -		if (action_flags & MLX5_FLOW_ACTION_RSS)
> > > -			return rte_flow_error_set(error, ENOTSUP,
> > > -						  RTE_FLOW_ERROR_TYPE_ACTION,
> > > -						  NULL,
> > > -						  "unsupported action RSS");
> > > -		if (!(action_flags & MLX5_FLOW_FATE_ESWITCH_ACTIONS))
> > > -			return rte_flow_error_set(error, EINVAL,
> > > -						  RTE_FLOW_ERROR_TYPE_ACTION,
> > > -						  actions,
> > > -						  "no fate action is found");
> > > +		if (!priv->jump_fdb_rx_en) {
> > > +			if (action_flags & MLX5_FLOW_ACTION_QUEUE)
> > > +				return rte_flow_error_set(error, ENOTSUP,
> > > +							  RTE_FLOW_ERROR_TYPE_ACTION,
> > > +							  NULL,
> > > +							  "unsupported action QUEUE");
> > > +			if (action_flags & MLX5_FLOW_ACTION_RSS)
> > > +				return rte_flow_error_set(error, ENOTSUP,
> > > +							  RTE_FLOW_ERROR_TYPE_ACTION,
> > > +							  NULL,
> > > +							  "unsupported action RSS");
> > > +			if (!(action_flags & MLX5_FLOW_FATE_ESWITCH_ACTIONS))
> > > +				return rte_flow_error_set(error, EINVAL,
> > > +							  RTE_FLOW_ERROR_TYPE_ACTION,
> > > +							  actions,
> > > +							  "no fate action is found");
> > > +		}
> > >  	} else {
> > >  		if (!(action_flags & MLX5_FLOW_FATE_ACTIONS) && attr->ingress)
> > >  			return rte_flow_error_set(error, EINVAL, diff --git
> > > a/drivers/net/mlx5/mlx5_flow_hw.c b/drivers/net/mlx5/mlx5_flow_hw.c
> > > index 83f55ed3e8..a063e5ac9c 100644
> > > --- a/drivers/net/mlx5/mlx5_flow_hw.c
> > > +++ b/drivers/net/mlx5/mlx5_flow_hw.c
> > > @@ -7026,6 +7026,7 @@ mlx5_hw_validate_action_queue(struct
> > rte_eth_dev
> > > *dev,
> > >  			      uint64_t action_flags,
> > >  			      struct rte_flow_error *error)  {
> > > +	struct mlx5_priv *priv = dev->data->dev_private;
> > >  	const struct rte_flow_action_queue *queue_mask = template_mask-
> > > >conf;
> > >  	const struct rte_flow_attr attr = {
> > >  		.ingress = template_attr->ingress, @@ -7034,7 +7035,7 @@
> > > mlx5_hw_validate_action_queue(struct
> > rte_eth_dev
> > > *dev,
> > >  	};
> > >  	bool masked = queue_mask != NULL && queue_mask->index;
> > >
> > > -	if (template_attr->egress || template_attr->transfer)
> > > +	if (template_attr->egress || (template_attr->transfer &&
> > > +!priv->jump_fdb_rx_en))
> > >  		return rte_flow_error_set(error, EINVAL,
> > >  					  RTE_FLOW_ERROR_TYPE_ATTR, NULL,
> > >  					  "QUEUE action supported for ingress
> only"); @@
> > -7053,9
> > > +7054,10 @@ mlx5_hw_validate_action_rss(struct
> > > rte_eth_dev *dev,
> > >  			      __rte_unused uint64_t action_flags,
> > >  			      struct rte_flow_error *error)  {
> > > +	struct mlx5_priv *priv = dev->data->dev_private;
> > >  	const struct rte_flow_action_rss *mask = template_mask->conf;
> > >
> > > -	if (template_attr->egress || template_attr->transfer)
> > > +	if (template_attr->egress || (template_attr->transfer &&
> > > +!priv->jump_fdb_rx_en))
> > >  		return rte_flow_error_set(error, EINVAL,
> > >  					  RTE_FLOW_ERROR_TYPE_ATTR, NULL,
> > >  					  "RSS action supported for ingress only");
> @@
> > -8104,6 +8106,7
> > > @@ __flow_hw_actions_template_create(struct rte_eth_dev *dev,
> > >  	}
> > >  	if (priv->sh->config.dv_xmeta_en ==
> > MLX5_XMETA_MODE_META32_HWS &&
> > >  	    priv->sh->config.dv_esw_en &&
> > > +	    !attr->transfer &&
> > >  	    (action_flags & (MLX5_FLOW_ACTION_QUEUE |
> > > MLX5_FLOW_ACTION_RSS))) {
> > >  		/* Insert META copy */
> > >  		mf_actions[expand_mf_num] = rx_meta_copy_action; @@
> > -12112,23
> > > +12115,29 @@ __flow_hw_configure(struct rte_eth_dev *dev,
> > >  	/* Add global actions. */
> > >  	for (i = 0; i < MLX5_HW_ACTION_FLAG_MAX; i++) {
> > >  		uint32_t act_flags = 0;
> > > +		uint32_t tag_flags = mlx5_hw_act_flag[i][0];
> > >
> > >  		act_flags = mlx5_hw_act_flag[i][MLX5DR_TABLE_TYPE_NIC_RX] |
> > >  			    mlx5_hw_act_flag[i][MLX5DR_TABLE_TYPE_NIC_TX];
> > >  		if (is_proxy) {
> > > -			if (unified_fdb)
> > > +			if (unified_fdb) {
> > >  				act_flags |=
> > >
> > > 	(mlx5_hw_act_flag[i][MLX5DR_TABLE_TYPE_FDB_RX] |
> > >
> > > mlx5_hw_act_flag[i][MLX5DR_TABLE_TYPE_FDB_TX] |
> > >
> > > mlx5_hw_act_flag[i][MLX5DR_TABLE_TYPE_FDB_UNIFIED]);
> > > -			else
> > > +				if (i == MLX5_HW_ACTION_FLAG_NONE_ROOT)
> > > +					tag_flags |=
> > > mlx5_hw_act_flag[i][MLX5DR_TABLE_TYPE_FDB_RX];
> > > +			} else {
> > >  				act_flags |=
> > > mlx5_hw_act_flag[i][MLX5DR_TABLE_TYPE_FDB];
> > > +				if (i == MLX5_HW_ACTION_FLAG_NONE_ROOT)
> > > +					tag_flags |=
> > > mlx5_hw_act_flag[i][MLX5DR_TABLE_TYPE_FDB];
> > > +			}
> > >  		}
> >
> > One small question about the TAG action flags in this case. If unified
> > FDB is supported, should only FDB_RX sub-domain of all FDB sub-domains
> > support the TAG? Or FDB_UNI also supports it. And if no unified FDB,
> > will the TAG also be supported in the legacy FDB domain? Can we add
> > some code comments here and update the limitation?
> Yes, it is allowed only in unified FDB Rx domain. Let's do this.
> 
> >
> > >  		priv->hw_drop[i] = mlx5dr_action_create_dest_drop(priv-
> > > >dr_ctx, act_flags);
> > >  		if (!priv->hw_drop[i])
> > >  			goto err;
> > >  		priv->hw_tag[i] = mlx5dr_action_create_tag
> > > -			(priv->dr_ctx, mlx5_hw_act_flag[i][0]);
> > > +			(priv->dr_ctx, tag_flags);
> > >  		if (!priv->hw_tag[i])
> > >  			goto err;
> > >  	}
> > > --
> > > 2.34.1

This should be with V2 version prefix.
Series Acked-by: Bing Zhao <bingz at nvidia.com>