Re: [PATCHv4 0/6] qemu-kvm: vhost net support

[Anthony Liguori <anthony@codemonkey.ws>]

Hi Michael,

I'll reserve individual patch review until they're in a mergable state, 
but I do have some comments about the overall integration architecture.

Generally speaking, I think the integration unnecessarily invasive.  It 
adds things to the virtio infrastructure that shouldn't be there like 
the irqfd/queuefd bindings.  It also sneaks in things like raw backend 
support which really isn't needed.

I think we can do better.  Here's what I suggest:

The long term goal should be to have a NetDevice interface that looks 
very much like virtio-net but as an API, not an ABI.  Roughly, it would 
look something like:

struct NetDevice {
   int add_xmit(NetDevice *dev, struct iovec *iov, int iovcnt, void *token);
   int add recv(NetDevice *dev, struct iovec *iov, int iovcnt, void *token);

   void *get_xmit(NetDevice *dev);
   void *get_recv(NetDevice *dev);

   void kick(NetDevice *dev);

   ...
};

That gives us a better API for use with virtio-net, e1000, etc.

Assuming we had this interface, I think a natural extension would be:

int add_ring(NetDevice *dev, void *address);
int add_kickfd(NetDevice *dev, int fd);

For slot management, it really should happen outside of the NetDevice 
structure.  We'll need a slot notifier mechanism such that we can keep 
this up to date as things change.

vhost-net because a NetDevice.  It can support things like the e1000 by 
doing ring translation behind the scenes. virtio-net can be fast pathed 
in the case that we're using KVM but otherwise, it would also rely on 
the ring translation.  N.B. in the case vhost-net is fast pathed, it 
requires a different device in QEMU that uses a separate virtio 
transport.  We should reuse as much code as possible obviously.  It 
doesn't make sense to have all of the virtio-pci code and virtio-net 
code in place when we aren't using it.

All this said, I'm *not* suggesting you have to implement all of this to 
get vhost-net merged.  Rather, I'm suggesting that we should try to 
structure the current vhost-net implementation to complement this 
architecture assuming we all agree this is the sane thing to do.  That 
means I would make the following changes to your series:

 - move vhost-net support to a VLANClientState backend.
 - do not introduce a raw socket backend
 - if for some reason you want to back to tap and raw, those should be 
options to the vhost-net backend.
 - when fast pathing with vhost-net, we should introduce interfaces to 
VLANClientState similar to add_ring and add_kickfd.  They'll be very 
specific to vhost-net for now, but that's okay.
 - sort out the layering of vhost-net within the virtio infrastructure.  
vhost-net should really be it's own qdev device.  I don't see very much 
code reuse happening right now so I don't understand why it's not that 
way currently.

Regards,

Anthony Liguori