Re: [patch net-next 24/26] mlxsw: spectrum: Implement LAG port join/leave

Andy Gospodarek
2015-12-02T14:26:32Z

On Wed, Dec 02, 2015 at 03:10:22PM +0100, Jiri Pirko wrote:
> Wed, Dec 02, 2015 at 03:01:32PM CET, gospo@cumulusnetworks.com wrote:
> >On Tue, Dec 01, 2015 at 02:49:02PM +0100, Jiri Pirko wrote:
> >> From: Jiri Pirko <jiri@mellanox.com>
> >>
> >> Implement basic procedures for joining/leaving port to/from LAG. That
> >> includes HW setup of collector, core LAG mapping setup.
> >>
> >[...]
> >> @@ -1937,6 +2153,18 @@ static int mlxsw_sp_netdevice_event(struct notifier_block *unused,
> >> mlxsw_sp_port->bridged = 0;
> >> mlxsw_sp_master_bridge_dec(mlxsw_sp, upper_dev);
> >> }
> >> + } else if (netif_is_lag_master(upper_dev)) {
> >> + if (info->linking) {
> >> + err = mlxsw_sp_port_lag_join(mlxsw_sp_port,
> >> + upper_dev);
> >> + if (err)
> >> + netdev_err(dev, "Failed to join link aggregation\n");
> >> + } else {
> >> + err = mlxsw_sp_port_lag_leave(mlxsw_sp_port,
> >> + upper_dev);
> >> + if (err)
> >> + netdev_err(dev, "Failed to leave link aggregation\n");
> >> + }
> >> }
> >> break;
> >> }
> >
> >If I'm reading this correctly, moving an offloaded port into a bond/team
> >or changing configuration options isn't synchronous. It seems like the
> >lack of immediate feedback to the caller is not desired.
>
> This is actually something we have prepared and wanted to push as a
> follow-up. To allow driver to return NOTIBY_BAD here and let changeupper
> to fail back to bond/team code.

I would like to see these additional changes proposed as part of this
set. The failure case needs to be synchronous and users need to be
notified immediately to consider this feature complete.