Re: [Qemu-devel] [RFC] net: Peer with existing NIC in netdev_add

[Stefan Hajnoczi]

On Thu, Nov 1, 2012 at 12:31 PM, Michael S. Tsirkin <address@hidden> wrote:
> On Thu, Nov 01, 2012 at 10:53:52AM +0100, Stefan Hajnoczi wrote:
>> On Wed, Oct 31, 2012 at 06:34:07PM +0200, Michael S. Tsirkin wrote:
>> > On Wed, Oct 31, 2012 at 03:51:08PM +0100, Stefan Hajnoczi wrote:
>> > > On Wed, Oct 31, 2012 at 10:57:24AM +0200, Michael S. Tsirkin wrote:
>> > > > On Wed, Oct 31, 2012 at 09:07:27AM +0100, Stefan Hajnoczi wrote:
>> > > > > On Tue, Oct 30, 2012 at 05:24:06PM +0200, Michael S. Tsirkin wrote:
>> > > > > > On Wed, Oct 24, 2012 at 02:49:21PM +0200, Stefan Hajnoczi wrote:
>> > > > > > > Allow netdev_del followed by netdev_add to re-peer a NIC and its 
>> > > > > > > netdev:
>> > > > > > >
>> > > > > > >   (qemu) info network
>> > > > > > >   virtio-net-pci.0: 
>> > > > > > > type=nic,model=virtio-net-pci,macaddr=52:54:00:12:34:56
>> > > > > > >    \ netdev0: type=user,net=10.0.2.0,restrict=off
>> > > > > > >
>> > > > > > >   (qemu) netdev_del netdev0
>> > > > > > >
>> > > > > > >   (qemu) netdev_add socket,id=netdev0,listen=:1234
>> > > > > > >
>> > > > > > >   (qemu) info network
>> > > > > > >   virtio-net-pci.0: 
>> > > > > > > type=nic,model=virtio-net-pci,macaddr=52:54:00:12:34:56
>> > > > > > >    \ netdev0: type=socket,
>> > > > > > >
>> > > > > > > This makes it possible to switch netdev while the guest is 
>> > > > > > > running.  It
>> > > > > > > is not necessary to reset the NIC.
>> > > > > > >
>> > > > > > > Note that the NIC's link goes down in netdev_del and back up 
>> > > > > > > again in
>> > > > > > > netdev_add.  Therefore the guest becomes aware that the network 
>> > > > > > > has
>> > > > > > > changed, although this depends on the emulated NIC model 
>> > > > > > > providing link
>> > > > > > > status change interrupts.
>> > > > > > >
>> > > > > > > Signed-off-by: Stefan Hajnoczi <address@hidden>
>> > > > > >
>> > > > > > I'd be surprised if this patch worked when one or both backends 
>> > > > > > are tap.
>> > > > > > tap supports offloads but slirp doesn't, since guest
>> > > > > > probes offloads at startup, it assumes it can use offloads.
>> > > > > > We also program tap during device operation e.g. on set features.
>> > > > > > vhost operation could also be interesting, have not looked into it.
>> > > > >
>> > > > > Yes, I left a TODO in the RFC patch and described the issue below.
>> > > > > We'll have to reject incompatible netdevs.
>> > > >
>> > > > Ideally, we'd probe all backend capabilities at init time.
>> > > > However, looks like we allowed netdev and device creation in any order.
>> > > > Can we change this and require netdev always be there before device?
>> > >
>> > > I don't think the order is a problem.  The relaxed order is only
>> > > relevant during startup from main() - but in that case we have no
>> > > constraints yet anyway.
>> > > The problem only occurs when netdev_add is used to create an
>> > > incompatible netdev after devices have initialized.  We should be able
>> > > to check and error out in the code that my RFC patch modifies.  If
>> > > constraints are violated then netdev_add can fail with an error (the new
>> > > netdev is not created and the QMP client needs to try again with a
>> > > compatible netdev configuration).
>> > >
>> > > Maybe I'm misunderstanding your point?
>> > >
>> > > Stefan
>> >
>> > OK so if we basically require same type backend then I think it's mostly
>> > fine.  I was trying to think of a way to allow changing backend type,
>> > this becomes messy very quickly.  In partuclar macvtap probably
>> > shouldn't be swapped with tap even though they are the same type
>> > formally.
>>
>> As long as they are offload-compatible, I think they can be swapped.
>> It's up to the user or the management stack to make sure switching
>> netdevs makes "sense".  So the network may be different and the guest
>> needs to DHCP again, but that's the user's problem.
>
> I think a simple rule like "use same backend type" is better than
> an opaque one "are offload-compatible" - user has no idea
> which offloads do each of the frontends and backends support.
> Also if in future we add offloads to backend X suddenly we
> break ability to swap with backend Y.
> Let's keep it simple.

Okay, that's a safe constraint that we can start with.  If users
request more freedom later we can get fancy.

Stefan