On Mon, 2026-03-30 at 19:08 -0700, Jakub Kicinski wrote:
> This is an AI-generated review of your patch. The human sending this
> email has considered the AI review valid, or at least plausible.
> ---
> net/mlx5: qos: Support cross-device tx scheduling
> 
> This commit relaxes the restriction that rate groups can only contain
> vports
> from the same E-Switch. It enables cross-device TX scheduling if the
> hardware
> supports it, LAG is enabled, and the E-Switches share a devlink
> device.
> 
> > diff --git a/drivers/net/ethernet/mellanox/mlx5/core/esw/qos.c
> > b/drivers/net/ethernet/mellanox/mlx5/core/esw/qos.c
> > --- a/drivers/net/ethernet/mellanox/mlx5/core/esw/qos.c
> > +++ b/drivers/net/ethernet/mellanox/mlx5/core/esw/qos.c
> 
> [ ... ]
> 
> > @@ -1738,18 +1761,44 @@ int mlx5_esw_devlink_rate_node_del(struct
> > devlink_rate *rate_node, void *priv,
> >     return 0;
> >  }
> >  
> > +static int
> > +mlx5_esw_validate_cross_esw_scheduling(struct mlx5_eswitch *esw,
> > +                                  struct mlx5_esw_sched_node
> > *parent,
> > +                                  struct netlink_ext_ack
> > *extack)
> > +{
> 
> [ ... ]
> 
> > +   if (!mlx5_lag_is_active(esw->dev)) {
> > +           NL_SET_ERR_MSG_MOD(extack,
> > +                              "Cross E-Switch scheduling
> > requires LAG to be activated");
> > +           return -EOPNOTSUPP;
> > +   }
> 
> What happens to the cross-device rate scheduling if LAG is
> deactivated
> after the configuration is applied?
> 
> When LAG is deactivated, the LAG state is torn down in hardware, but
> there
> does not appear to be a corresponding cleanup or update triggered for
> the
> cross-eswitch QoS scheduling elements.
> 
> Could this result in a regression where the hardware TSAR tree and
> devlink
> rate configuration remain pointing to a now-invalid VHCA association,
> bypassing the state validation and causing traffic drops or firmware
> errors?

This has been considered, and FW rejects LAG deactivation while there
are VFs present on any of the ports. VFs need to be removed first, and
removing all VF will also remove the QoS tree entirely.

SW could perhaps make this nicer by detecting that the QoS tree spans
multiple devices and issuing a warning before attempting to invoke the
DESTROY_LAG FW command. But this appears to be a significant effort
currently, since the LAG code isn't structured in a way that would make
this easy.

> 
> > +
> > +   return 0;
> > +}

Cosmin.

Reply via email to