Hi,
> -----Original Message-----
> From: Bing Zhao <bi...@nvidia.com>
> Sent: Monday, November 13, 2023 9:51 PM
> To: Suanming Mou <suanmi...@nvidia.com>; Matan Azrad
> <ma...@nvidia.com>; Slava Ovsiienko <viachesl...@nvidia.com>; Raslan
> Darawsheh <rasl...@nvidia.com>; Ori Kam <or...@nvidia.com>
> Cc: dev@dpdk.org; Dariusz Sosnowski <dsosnow...@nvidia.com>;
> sta...@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 <suanmi...@nvidia.com>
> > Sent: Monday, November 13, 2023 9:14 PM
> > To: Bing Zhao <bi...@nvidia.com>; Matan Azrad <ma...@nvidia.com>;
> > Slava Ovsiienko <viachesl...@nvidia.com>; Raslan Darawsheh
> > <rasl...@nvidia.com>; Ori Kam <or...@nvidia.com>
> > Cc: dev@dpdk.org; Dariusz Sosnowski <dsosnow...@nvidia.com>;
> > sta...@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 <bi...@nvidia.com>
> > > Sent: Monday, November 13, 2023 8:42 PM
> > > To: Matan Azrad <ma...@nvidia.com>; Slava Ovsiienko
> > > <viachesl...@nvidia.com>; Raslan Darawsheh <rasl...@nvidia.com>;
> > > Suanming Mou <suanmi...@nvidia.com>; Ori Kam <or...@nvidia.com>
> > > Cc: dev@dpdk.org; Dariusz Sosnowski <dsosnow...@nvidia.com>;
> > > sta...@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: dsosnow...@nvidia.com
> > > Cc: sta...@dpdk.org
> > >
> > > Signed-off-by: Bing Zhao <bi...@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 <suanmi...@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
