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

[Bing Zhao]

Hi Suanming,

My answers are inline.

> -----Original Message-----
> From: Suanming Mou <suanmingm at nvidia.com>
> Sent: Monday, November 13, 2023 9:14 PM
> To: Bing Zhao <bingz at nvidia.com>; Matan Azrad <matan at nvidia.com>; Slava
> Ovsiienko <viacheslavo at nvidia.com>; Raslan Darawsheh
> <rasland at nvidia.com>; Ori Kam <orika at nvidia.com>
> Cc: dev at dpdk.org; Dariusz Sosnowski <dsosnowski at nvidia.com>;
> stable at 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 at nvidia.com>
> > Sent: Monday, November 13, 2023 8:42 PM
> > To: Matan Azrad <matan at nvidia.com>; Slava Ovsiienko
> > <viacheslavo at nvidia.com>; Raslan Darawsheh <rasland at nvidia.com>;
> > Suanming Mou <suanmingm at nvidia.com>; Ori Kam <orika at nvidia.com>
> > Cc: dev at dpdk.org; Dariusz Sosnowski <dsosnowski at nvidia.com>;
> > stable at 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 at nvidia.com
> > Cc: stable at dpdk.org
> >
> > Signed-off-by: Bing Zhao <bingz at 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 at 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