Subject: Re: [PATCH net] failover: eliminate callback hell

From: Stephen Hemminger <[email-masked]>
Date: Tue, 5 Jun 2018 10:45:10 -0700

On Tue, 5 Jun 2018 10:22:13 -0700
"Samudrala, Sridhar" <[email-masked]> wrote:

> On 6/4/2018 8:42 PM, Stephen Hemminger wrote:
> > The net failover should be a simple library, not a virtual
> > object with function callbacks (see callback hell).
> > The code is simpler is smaller both for the netvsc and virtio use case.  
> 
> I quickly tried this patch and it breaks virtio-net in standby mode.
> I don't see failover netdev, unloading virtio-net causes a crash.

I said it wasn't tested. Not surprising. Don't have a version of KVM
that supports standby (and not going to build KVM from scratch for this).

> 
> With these changes, there is very minimal code that is shared between
> netvsc and virtio-net. The notifier and event handling code and the
> lookup_bymac routines are now duplicated in both the drivers. I thought
> we wanted to keep this code common between the 2 drivers and we went through
> multiple revisions to make sure that it works with both netvsc's 2 netdev
> and virtio-net's 3 netdev models.

The sharing is what needs to be shared; it turns out not much
is common really. The total code size is less with this version.
Also, since even with nested virtualization, both drivers will not
be present on same guest at same time.

> The reason for the indirect ops is to support these 2 different models and
> i am not sure if the overhead of the callbacks is that significant considering
> that they are not called in the hot path.

The callbacks invert the functionality. It makes everything bottom up.