Re: [virtio-dev] [PATCH v4] content: Introduce VIRTIO_NET_F_STANDBY feature

["Michael S. Tsirkin" <mst@redhat.com>]

On Tue, Sep 18, 2018 at 10:13:37AM -0500, Venu Busireddy wrote:
> On 2018-09-18 09:35:48 -0400, Michael S. Tsirkin wrote:
> > On Tue, Sep 18, 2018 at 12:20:52PM +0200, Cornelia Huck wrote:
> > > On Wed, 12 Sep 2018 11:22:12 -0400
> > > "Michael S. Tsirkin" <mst@redhat.com> wrote:
> > > 
> > > > On Wed, Sep 12, 2018 at 08:17:45AM -0700, Samudrala, Sridhar wrote:
> > > > > 
> > > > > 
> > > > > On 9/7/2018 2:34 PM, Michael S. Tsirkin wrote:  
> > > > > > On Wed, Aug 15, 2018 at 11:49:15AM -0700, Sridhar Samudrala wrote:  
> > > > > > > VIRTIO_NET_F_STANDBY feature enables hypervisor to indicate virtio_net
> > > > > > > device to act as a standby for another device with the same MAC address.
> > > > > > > 
> > > > > > > Signed-off-by: Sridhar Samudrala <sridhar.samudrala@intel.com>
> > > > > > > Acked-by: Cornelia Huck <cohuck@redhat.com>
> > > > > > > Fixes: https://github.com/oasis-tcs/virtio-spec/issues/18  
> > > > > > Applied but when do you plan to add documentation as pointed
> > > > > > out by Jan and Halil?  
> > > > > 
> > > > > I thought additional documentation will be done as part of the Qemu enablement
> > > > > patches and i hope someone in RH is looking into it.
> > > > > 
> > > > > Does it make sense to add a link to to the kernel documentation of this feature in
> > > > > the spec
> > > > >  https://www.kernel.org/doc/html/latest/networking/net_failover.html  
> > > > 
> > > > 
> > > > I do not think this will address the comments posted.  Specifically we
> > > > should probably include documentation for what is a standby and primary:
> > > > what is expected of driver (maintain configuration on standby, support
> > > > primary coming and going, transmit on standby only if there is no
> > > > primary) and of device (have same mac for standby as for standby).
> > > 
> > > Yes, we need some definitive statements of what a driver and a device
> > > is supposed to do in order to conform; it might make sense to discuss
> > > this in conjunction with discussion on any QEMU patches (have not
> > > checked whether anything has been posted, just returned from vacation).
> > > 
> > > I assume that we still stick with the plan to implement/document
> > > MAC-based handling first and then enhance with other methods later?
> > 
> > I'm fine with that at least. If someone wants to work on
> > other methods straight away, that's also fine by me.
> 
> Patch set [1] implements the failover-group-id mechanism. Are you
> thinking of some other method?
> 
> Venu
> 
> [1] https://lists.oasis-open.org/archives/virtio-dev/201806/msg00384.html
> 

Yes, the grouping mechanism seems fine to me (I don't remember
about the implementation, it's been a while).

It is not by itself sufficient though, is it?

MAC is assumed to be shared to avoid things like ARP/neighboor
rediscovery, right?
If true that implies that to avoid guest confusion visibility of the
primary needs to be controlled by standby's driver.
This makes this patchset incomplete.

For this work to be complete what is needed is:
- hypervisor: add control of primary's visibility to guest
- guest: add support for this grouping to the failover driver

We also need
- spec: document matching rules based on the pci bridge

and it's helpful to have a spec proposal with implementation, but I
would say at least proposed patches to one of the above 2 would be
helpful before we include this in spec.

-- 
MST