Hi
> -----Original Message-----
> From: Suanming Mou <suanmi...@nvidia.com>
> Sent: Wednesday, February 26, 2025 9:43 PM
> To: Bing Zhao <bi...@nvidia.com>; Dariusz Sosnowski
> <dsosnow...@nvidia.com>; Slava Ovsiienko <viachesl...@nvidia.com>; Ori Kam
> <or...@nvidia.com>; Matan Azrad <ma...@nvidia.com>
> Cc: dev@dpdk.org; Raslan Darawsheh <rasl...@nvidia.com>
> Subject: RE: [PATCH 3/3] net/mlx5: allow FDB RSS
>
>
>
> > -----Original Message-----
> > From: Bing Zhao <bi...@nvidia.com>
> > Sent: Wednesday, February 26, 2025 6:50 PM
> > To: Suanming Mou <suanmi...@nvidia.com>; Dariusz Sosnowski
> > <dsosnow...@nvidia.com>; Slava Ovsiienko <viachesl...@nvidia.com>; Ori
> > Kam <or...@nvidia.com>; Matan Azrad <ma...@nvidia.com>
> > Cc: dev@dpdk.org; Raslan Darawsheh <rasl...@nvidia.com>
> > Subject: RE: [PATCH 3/3] net/mlx5: allow FDB RSS
> >
> >
> >
> > > -----Original Message-----
> > > From: Suanming Mou <suanmi...@nvidia.com>
> > > Sent: Tuesday, February 25, 2025 8:45 AM
> > > To: Dariusz Sosnowski <dsosnow...@nvidia.com>; Slava Ovsiienko
> > > <viachesl...@nvidia.com>; Bing Zhao <bi...@nvidia.com>; Ori Kam
> > > <or...@nvidia.com>; Matan Azrad <ma...@nvidia.com>
> > > Cc: dev@dpdk.org; Raslan Darawsheh <rasl...@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 <suanmi...@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 <bi...@nvidia.com>
