Re: [PATCH net-next v10 2/4] net: Introduce generic failover module

["Samudrala, Sridhar" <sridhar.samudrala@intel.com>]

On 5/7/2018 4:59 PM, Stephen Hemminger wrote:
> On Mon,  7 May 2018 15:10:44 -0700
> Sridhar Samudrala <sridhar.samudrala@intel.com> wrote:
>
> > +	if (netif_running(failover_dev)) {
> > +		err = dev_open(slave_dev);
> > +		if (err && (err != -EBUSY)) {
> > +			netdev_err(failover_dev, "Opening slave %s failed err:%d\n",
> > +				   slave_dev->name, err);
> > +			goto err_dev_open;
> > +		}
> > +	}
> > +
> > +	netif_addr_lock_bh(failover_dev);
> > +	dev_uc_sync_multiple(slave_dev, failover_dev);
> > +	dev_uc_sync_multiple(slave_dev, failover_dev);
> > +	netif_addr_unlock_bh(failover_dev);
> > +
> The order of these is backwards, you want to sync addresses before bringing up.
> Also, doing it this way does not allow udev/systemd the chance to rename VF devices.

During my testing, i noticed that dev_open() may fail with EBUSY in certain scenarios,
If so, the opening of the slave is handled after the rename via the NETDEV_CHANGENAME
event handler.

> The complexity of this whole failover mechanism does not make life easier,
> more reliable, or safer for netvsc. I though that was the whole reason for having
> common code.

netvsc doesn't go through this code.

	if (nfo_ops && nfo_ops->slave_register)
                return nfo_ops->slave_register(slave_dev, failover_dev);

So there is no change in event handling for netvsc 2-netdev model.