From mboxrd@z Thu Jan  1 00:00:00 1970
From: "Samudrala, Sridhar" <sridhar.samudrala@intel.com>
Subject: Re: [PATCH net-next v10 2/4] net: Introduce generic failover module
Date: Mon, 7 May 2018 17:11:21 -0700
Message-ID: <d4b8dcae-3d21-3b8b-5881-1fc15617d18a@intel.com>
References: <1525731046-10989-1-git-send-email-sridhar.samudrala@intel.com>
 <1525731046-10989-3-git-send-email-sridhar.samudrala@intel.com>
 <20180507165911.45b14c58@xeon-e3>
Mime-Version: 1.0
Content-Type: text/plain; charset=utf-8; format=flowed
Content-Transfer-Encoding: 8bit
Cc: mst@redhat.com, davem@davemloft.net, netdev@vger.kernel.org,
        virtualization@lists.linux-foundation.org,
        virtio-dev@lists.oasis-open.org, jesse.brandeburg@intel.com,
        alexander.h.duyck@intel.com, kubakici@wp.pl, jasowang@redhat.com,
        loseweigh@gmail.com, jiri@resnulli.us, aaron.f.brown@intel.com
To: Stephen Hemminger <stephen@networkplumber.org>
Return-path: <netdev-owner@vger.kernel.org>
Received: from mga12.intel.com ([192.55.52.136]:47612 "EHLO mga12.intel.com"
        rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP
        id S1753435AbeEHALW (ORCPT <rfc822;netdev@vger.kernel.org>);
        Mon, 7 May 2018 20:11:22 -0400
In-Reply-To: <20180507165911.45b14c58@xeon-e3>
Content-Language: en-US
Sender: netdev-owner@vger.kernel.org
List-ID: <netdev.vger.kernel.org>

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.