[Qemu-devel] [V5 PATCH 0/4] Send gratuitous packets by guest

[Qemu-devel] [V5 PATCH 0/4] Send gratuitous packets by guest

[Jason Wang]

This an update of series that let guest and qemu to be co-operated to
send gratuitous packets when needed such as after migration, loadvm
and continuing.

As it's hard for qemu to track the network configuration in guest such
as bondings, vlans or ipv6. So current gratuitous may not work under
those situations.

The series first introduce a model specific function in order to let
nic models to use a device specific way to announce the link
presence. With this, virtio-net backend were modified to notify the
guest (through config update interrupt) and let guest send the
gratuitous packet when needed.

Changes from V4:

- keep the old behavior that send the gratuitous packets only after
  migration
- decide whether to send gratuitous packets by previous runstate
  instead of a dedicated parameter
- check virtio_net_started() instead of VIRTIO_NET_S_LINK_UP before
  issue the config update interrupt
- move VIRTIO_NET_S_ANNOUNCE to 0x100 and supress guest config write to RO bits
- cleanups suggested by Michael

---

Jason Wang (4):
      net: announce self after vm start
      net: model specific announcing support
      virtio-net: notify guest to annouce itself
      virtio-net: compat guest announce support.


 hw/pc_piix.c    |   35 +++++++++++++++++++++++++++++++++++
 hw/virtio-net.c |   19 +++++++++++++++++++
 hw/virtio-net.h |    3 +++
 migration.c     |    1 -
 net.h           |    2 ++
 savevm.c        |    8 +++++---
 vl.c            |    4 ++++
 7 files changed, 68 insertions(+), 4 deletions(-)

-- 
Jason Wang

[Qemu-devel] [V5 PATCH 1/4] net: announce self after vm start

[Jason Wang]

qemu_announce_self() were moved to vm_start(). This is because we may
want to let guest to send the gratuitous packets. After this change,
we need to check the previous run state (RUN_STATE_INMIGRATE) to
decide whether an announcement is needed.

Signed-off-by: Jason Wang <jasowang@redhat.com>
---
 migration.c |    1 -
 vl.c        |    4 ++++
 2 files changed, 4 insertions(+), 1 deletions(-)

diff --git a/migration.c b/migration.c
index 00fa1e3..1ce6b5c 100644
--- a/migration.c
+++ b/migration.c
@@ -88,7 +88,6 @@ void process_incoming_migration(QEMUFile *f)
         fprintf(stderr, "load of migration failed\n");
         exit(0);
     }
-    qemu_announce_self();
     DPRINTF("successfully loaded vm state\n");
 
     /* Make sure all file formats flush their mutable metadata */
diff --git a/vl.c b/vl.c
index 65f11f2..4742b1b 100644
--- a/vl.c
+++ b/vl.c
@@ -1261,11 +1261,15 @@ void vm_state_notify(int running, RunState state)
 void vm_start(void)
 {
     if (!runstate_is_running()) {
+        RunState prev_run_state = current_run_state;
         cpu_enable_ticks();
         runstate_set(RUN_STATE_RUNNING);
         vm_state_notify(1, RUN_STATE_RUNNING);
         resume_all_vcpus();
         monitor_protocol_event(QEVENT_RESUME, NULL);
+        if (prev_run_state == RUN_STATE_INMIGRATE) {
+            qemu_announce_self();
+        }
     }
 }
 

[Qemu-devel] [V5 PATCH 2/4] net: model specific announcing support

[Jason Wang]

This patch introduces a function pointer in NetClientInfo which is
called during self announcement. With this, each kind of card can
announce the link with a specific way. The old method is still kept
for cards that have not implemented this or old guest. The first user
would be virtio-net.

Signed-off-by: Jason Wang <jasowang@redhat.com>
---
 net.h    |    2 ++
 savevm.c |    8 +++++---
 2 files changed, 7 insertions(+), 3 deletions(-)

diff --git a/net.h b/net.h
index 75a8c15..7195bfc 100644
--- a/net.h
+++ b/net.h
@@ -48,6 +48,7 @@ typedef ssize_t (NetReceive)(VLANClientState *, const uint8_t *, size_t);
 typedef ssize_t (NetReceiveIOV)(VLANClientState *, const struct iovec *, int);
 typedef void (NetCleanup) (VLANClientState *);
 typedef void (LinkStatusChanged)(VLANClientState *);
+typedef int (NetAnnounce)(VLANClientState *);
 
 typedef struct NetClientInfo {
     net_client_type type;
@@ -59,6 +60,7 @@ typedef struct NetClientInfo {
     NetCleanup *cleanup;
     LinkStatusChanged *link_status_changed;
     NetPoll *poll;
+    NetAnnounce *announce;
 } NetClientInfo;
 
 struct VLANClientState {
diff --git a/savevm.c b/savevm.c
index 80be1ff..7558c1d 100644
--- a/savevm.c
+++ b/savevm.c
@@ -123,10 +123,12 @@ static void qemu_announce_self_iter(NICState *nic, void *opaque)
 {
     uint8_t buf[60];
     int len;
+    NetAnnounce *func = nic->nc.info->announce;
 
-    len = announce_self_create(buf, nic->conf->macaddr.a);
-
-    qemu_send_packet_raw(&nic->nc, buf, len);
+    if (!func || func(&nic->nc) != 0) {
+        len = announce_self_create(buf, nic->conf->macaddr.a);
+        qemu_send_packet_raw(&nic->nc, buf, len);
+    }
 }
 
 

[Qemu-devel] [V5 PATCH 3/4] virtio-net: notify guest to annouce itself

[Jason Wang]

It's hard to track all mac addresses and their usage (vlan, bondings,
ipv6) in qemu to send proper gratuitous packet. The better choice is
to let guest to send them.

So, this patch introduces a new rw config status bit of virtio-net,
VIRTIO_NET_S_ANNOUNCE which is used to notify guest to announce
presence of its link through config update interrupt. When gust have
done the announcement, it should clear that bit. The feature is negotiated by
a new feature bit VIRTIO_NET_F_ANNOUNCE.

Signed-off-by: Jason Wang <jasowang@redhat.com>
---
 hw/virtio-net.c |   19 +++++++++++++++++++
 hw/virtio-net.h |    3 +++
 2 files changed, 22 insertions(+), 0 deletions(-)

diff --git a/hw/virtio-net.c b/hw/virtio-net.c
index bc5e3a8..c1dbd49 100644
--- a/hw/virtio-net.c
+++ b/hw/virtio-net.c
@@ -95,6 +95,10 @@ static void virtio_net_set_config(VirtIODevice *vdev, const uint8_t *config)
         memcpy(n->mac, netcfg.mac, ETH_ALEN);
         qemu_format_nic_info_str(&n->nic->nc, n->mac);
     }
+
+    /* ignore write to RO byte */
+    memcpy((uint8_t *)&n->status + 1, (uint8_t *)&netcfg.status + 1,
+           sizeof(uint8_t));
 }
 
 static bool virtio_net_started(VirtIONet *n, uint8_t status)
@@ -983,6 +987,20 @@ static void virtio_net_cleanup(VLANClientState *nc)
     n->nic = NULL;
 }
 
+static int virtio_net_announce(VLANClientState *nc)
+{
+    VirtIONet *n = DO_UPCAST(NICState, nc, nc)->opaque;
+
+    if (n->vdev.guest_features & (0x1 << VIRTIO_NET_F_GUEST_ANNOUNCE)
+        && virtio_net_started(n, n->vdev.status)) {
+        n->status |= VIRTIO_NET_S_ANNOUNCE;
+        virtio_notify_config(&n->vdev);
+        return 0;
+    }
+
+    return -1;
+}
+
 static NetClientInfo net_virtio_info = {
     .type = NET_CLIENT_TYPE_NIC,
     .size = sizeof(NICState),
@@ -990,6 +1008,7 @@ static NetClientInfo net_virtio_info = {
     .receive = virtio_net_receive,
         .cleanup = virtio_net_cleanup,
     .link_status_changed = virtio_net_set_link_status,
+    .announce = virtio_net_announce,
 };
 
 VirtIODevice *virtio_net_init(DeviceState *dev, NICConf *conf,
diff --git a/hw/virtio-net.h b/hw/virtio-net.h
index 4468741..f3acbd1 100644
--- a/hw/virtio-net.h
+++ b/hw/virtio-net.h
@@ -44,8 +44,10 @@
 #define VIRTIO_NET_F_CTRL_RX    18      /* Control channel RX mode support */
 #define VIRTIO_NET_F_CTRL_VLAN  19      /* Control channel VLAN filtering */
 #define VIRTIO_NET_F_CTRL_RX_EXTRA 20   /* Extra RX mode control support */
+#define VIRTIO_NET_F_GUEST_ANNOUNCE 21  /* Guest can announce itself */
 
 #define VIRTIO_NET_S_LINK_UP    1       /* Link is up */
+#define VIRTIO_NET_S_ANNOUNCE   0x100   /* Announcement is needed */
 
 #define TX_TIMER_INTERVAL 150000 /* 150 us */
 
@@ -176,6 +178,7 @@ struct virtio_net_ctrl_mac {
         DEFINE_PROP_BIT("guest_tso6", _state, _field, VIRTIO_NET_F_GUEST_TSO6, true), \
         DEFINE_PROP_BIT("guest_ecn", _state, _field, VIRTIO_NET_F_GUEST_ECN, true), \
         DEFINE_PROP_BIT("guest_ufo", _state, _field, VIRTIO_NET_F_GUEST_UFO, true), \
+        DEFINE_PROP_BIT("guest_announce", _state, _field, VIRTIO_NET_F_GUEST_ANNOUNCE, true), \
         DEFINE_PROP_BIT("host_tso4", _state, _field, VIRTIO_NET_F_HOST_TSO4, true), \
         DEFINE_PROP_BIT("host_tso6", _state, _field, VIRTIO_NET_F_HOST_TSO6, true), \
         DEFINE_PROP_BIT("host_ecn", _state, _field, VIRTIO_NET_F_HOST_ECN, true), \

[Qemu-devel] [V5 PATCH 4/4] virtio-net: compat guest announce support.

[Jason Wang]

Disable guest announce for compat machine types.

Signed-off-by: Jason Wang <jasowang@redhat.com>
---
 hw/pc_piix.c |   35 +++++++++++++++++++++++++++++++++++
 1 files changed, 35 insertions(+), 0 deletions(-)

diff --git a/hw/pc_piix.c b/hw/pc_piix.c
index 6c5c40f..780b607 100644
--- a/hw/pc_piix.c
+++ b/hw/pc_piix.c
@@ -389,6 +389,11 @@ static QEMUMachine pc_machine_v1_0 = {
             .property = "check_media_rate",
             .value    = "off",
         },
+        {
+            .driver   = "virtio-net-pci",
+            .property = "guest_announce",
+            .value    = "off",
+        },
         { /* end of list */ }
     },
 };
@@ -408,6 +413,11 @@ static QEMUMachine pc_machine_v0_15 = {
             .property = "check_media_rate",
             .value    = "off",
         },
+        {
+            .driver   = "virtio-net-pci",
+            .property = "guest_announce",
+            .value    = "off",
+        },
         { /* end of list */ }
     },
 };
@@ -452,6 +462,11 @@ static QEMUMachine pc_machine_v0_14 = {
             .property = "rom_only",
             .value    = stringify(1),
         },
+        {
+            .driver   = "virtio-net-pci",
+            .property = "guest_announce",
+            .value    = "off",
+        },
         { /* end of list */ }
     },
 };
@@ -508,6 +523,11 @@ static QEMUMachine pc_machine_v0_13 = {
             .property = "rom_only",
             .value    = stringify(1),
         },
+        {
+            .driver   = "virtio-net-pci",
+            .property = "guest_announce",
+            .value    = "off",
+        },
         { /* end of list */ }
     },
 };
@@ -568,6 +588,11 @@ static QEMUMachine pc_machine_v0_12 = {
             .property = "rom_only",
             .value    = stringify(1),
         },
+        {
+            .driver   = "virtio-net-pci",
+            .property = "guest_announce",
+            .value    = "off",
+        },
         { /* end of list */ }
     }
 };
@@ -636,6 +661,11 @@ static QEMUMachine pc_machine_v0_11 = {
             .property = "rom_only",
             .value    = stringify(1),
         },
+        {
+            .driver   = "virtio-net-pci",
+            .property = "guest_announce",
+            .value    = "off",
+        },
         { /* end of list */ }
     }
 };
@@ -716,6 +746,11 @@ static QEMUMachine pc_machine_v0_10 = {
             .property = "rom_only",
             .value    = stringify(1),
         },
+        {
+            .driver   = "virtio-net-pci",
+            .property = "guest_announce",
+            .value    = "off",
+        },
         { /* end of list */ }
     },
 };

Re: [Qemu-devel] [V5 PATCH 1/4] net: announce self after vm start

[Paolo Bonzini]

Il 16/03/2012 09:54, Jason Wang ha scritto:
> qemu_announce_self() were moved to vm_start(). This is because we may
> want to let guest to send the gratuitous packets. After this change,
> we need to check the previous run state (RUN_STATE_INMIGRATE) to
> decide whether an announcement is needed.
> 
> Signed-off-by: Jason Wang <jasowang@redhat.com>
> ---
>  migration.c |    1 -
>  vl.c        |    4 ++++
>  2 files changed, 4 insertions(+), 1 deletions(-)
> 
> diff --git a/migration.c b/migration.c
> index 00fa1e3..1ce6b5c 100644
> --- a/migration.c
> +++ b/migration.c
> @@ -88,7 +88,6 @@ void process_incoming_migration(QEMUFile *f)
>          fprintf(stderr, "load of migration failed\n");
>          exit(0);
>      }
> -    qemu_announce_self();
>      DPRINTF("successfully loaded vm state\n");
>  
>      /* Make sure all file formats flush their mutable metadata */
> diff --git a/vl.c b/vl.c
> index 65f11f2..4742b1b 100644
> --- a/vl.c
> +++ b/vl.c
> @@ -1261,11 +1261,15 @@ void vm_state_notify(int running, RunState state)
>  void vm_start(void)
>  {
>      if (!runstate_is_running()) {
> +        RunState prev_run_state = current_run_state;
>          cpu_enable_ticks();
>          runstate_set(RUN_STATE_RUNNING);
>          vm_state_notify(1, RUN_STATE_RUNNING);
>          resume_all_vcpus();
>          monitor_protocol_event(QEVENT_RESUME, NULL);
> +        if (prev_run_state == RUN_STATE_INMIGRATE) {
> +            qemu_announce_self();
> +        }
>      }
>  }
>  
> 

I tihnk this won't work with -S, did you test it?  Perhaps it's possible
simply to change

    if (autostart) {
        vm_start();
    } else {
        runstate_set(RUN_STATE_PRELAUNCH);
    }

to remain in INMIGRATE state:

    if (autostart) {
        vm_start();
    }

Otherwise looks good.

Paolo

Re: [Qemu-devel] [V5 PATCH 1/4] net: announce self after vm start

[Jason Wang]

On 03/16/2012 05:43 PM, Paolo Bonzini wrote:
> Il 16/03/2012 09:54, Jason Wang ha scritto:
>> qemu_announce_self() were moved to vm_start(). This is because we may
>> want to let guest to send the gratuitous packets. After this change,
>> we need to check the previous run state (RUN_STATE_INMIGRATE) to
>> decide whether an announcement is needed.
>>
>> Signed-off-by: Jason Wang<jasowang@redhat.com>
>> ---
>>   migration.c |    1 -
>>   vl.c        |    4 ++++
>>   2 files changed, 4 insertions(+), 1 deletions(-)
>>
>> diff --git a/migration.c b/migration.c
>> index 00fa1e3..1ce6b5c 100644
>> --- a/migration.c
>> +++ b/migration.c
>> @@ -88,7 +88,6 @@ void process_incoming_migration(QEMUFile *f)
>>           fprintf(stderr, "load of migration failed\n");
>>           exit(0);
>>       }
>> -    qemu_announce_self();
>>       DPRINTF("successfully loaded vm state\n");
>>
>>       /* Make sure all file formats flush their mutable metadata */
>> diff --git a/vl.c b/vl.c
>> index 65f11f2..4742b1b 100644
>> --- a/vl.c
>> +++ b/vl.c
>> @@ -1261,11 +1261,15 @@ void vm_state_notify(int running, RunState state)
>>   void vm_start(void)
>>   {
>>       if (!runstate_is_running()) {
>> +        RunState prev_run_state = current_run_state;
>>           cpu_enable_ticks();
>>           runstate_set(RUN_STATE_RUNNING);
>>           vm_state_notify(1, RUN_STATE_RUNNING);
>>           resume_all_vcpus();
>>           monitor_protocol_event(QEVENT_RESUME, NULL);
>> +        if (prev_run_state == RUN_STATE_INMIGRATE) {
>> +            qemu_announce_self();
>> +        }
>>       }
>>   }
>>
>>
> I tihnk this won't work with -S, did you test it?  Perhaps it's possible
> simply to change

Yes, it does not work.
>
>      if (autostart) {
>          vm_start();
>      } else {
>          runstate_set(RUN_STATE_PRELAUNCH);
>      }
>
> to remain in INMIGRATE state:
>
>      if (autostart) {
>          vm_start();
>      }
>
> Otherwise looks good.
>
> Paolo

The problem with staying in the INMIGRATE is that we can not figure out 
when the migration is completed when using '-S', so this kind of 
transition were forbidden by qmp_cont().

Looks like we need a new state such as RUN_STATE_MIGRATE_PRELAUNCH?

Re: [Qemu-devel] [V5 PATCH 1/4] net: announce self after vm start

[Paolo Bonzini]

Il 16/03/2012 11:13, Jason Wang ha scritto:
> The problem with staying in the INMIGRATE is that we can not figure out
> when the migration is completed when using '-S', so this kind of
> transition were forbidden by qmp_cont().
> 
> Looks like we need a new state such as RUN_STATE_MIGRATE_PRELAUNCH?

Or just a global need_announce instead of looking at the runstate.

Paolo

Re: [Qemu-devel] [V5 PATCH 1/4] net: announce self after vm start

[Jason Wang]

On 03/16/2012 06:45 PM, Paolo Bonzini wrote:
> Il 16/03/2012 11:13, Jason Wang ha scritto:
>> >  The problem with staying in the INMIGRATE is that we can not figure out
>> >  when the migration is completed when using '-S', so this kind of
>> >  transition were forbidden by qmp_cont().
>> >  
>> >  Looks like we need a new state such as RUN_STATE_MIGRATE_PRELAUNCH?
> Or just a global need_announce instead of looking at the runstate.
>
> Paolo
>
Then I think it's better for us introduce a parameter for vm_start() 
like what we've done in V4.

Re: [Qemu-devel] [V5 PATCH 1/4] net: announce self after vm start

[Paolo Bonzini]

Il 16/03/2012 16:23, Jason Wang ha scritto:
>>>
>> Or just a global need_announce instead of looking at the runstate.
>>
>> Paolo
>>
> Then I think it's better for us introduce a parameter for vm_start()
> like what we've done in V4.

But that didn't work because you ended up changing the "cont" semantics.

There are two possibilities.

1) Changing those is okay, in which case you only need to check more
runstates;

2) Changing those is not okay, in which case you need something like
this in qemu_announce_self()

void qemu_announce_self()
{
    if (!runstate_is_running()) {
        need_announce = true;
        return;
    }

    need_announce = false;
    ...
}

and then you just check need_announce in vm_start.  Nothing to change in
all the invocations of vm_start, you just mark that you need to do work
later.

Paolo

Re: [Qemu-devel] [V5 PATCH 1/4] net: announce self after vm start

[Jason Wang]

On 03/17/2012 12:31 AM, Paolo Bonzini wrote:
> Il 16/03/2012 16:23, Jason Wang ha scritto:
>>> Or just a global need_announce instead of looking at the runstate.
>>>
>>> Paolo
>>>
>> Then I think it's better for us introduce a parameter for vm_start()
>> like what we've done in V4.
> But that didn't work because you ended up changing the "cont" semantics.
>
> There are two possibilities.
>
> 1) Changing those is okay, in which case you only need to check more
> runstates;
>
> 2) Changing those is not okay, in which case you need something like
> this in qemu_announce_self()
>
> void qemu_announce_self()
> {
>      if (!runstate_is_running()) {
>          need_announce = true;
>          return;
>      }
>
>      need_announce = false;
>      ...
> }
>
> and then you just check need_announce in vm_start.  Nothing to change in
> all the invocations of vm_start, you just mark that you need to do work
> later.
>
> Paolo

Right, I would replace the qemu_announce_self() with a "need_announce = 
true" in process_incoming_migration() and check it later in vm_start().

Thanks

Re: [Qemu-devel] [V5 PATCH 0/4] Send gratuitous packets by guest

[Anthony Liguori]

On 03/16/2012 03:54 AM, Jason Wang wrote:
> This an update of series that let guest and qemu to be co-operated to
> send gratuitous packets when needed such as after migration, loadvm
> and continuing.
>
> As it's hard for qemu to track the network configuration in guest such
> as bondings, vlans or ipv6. So current gratuitous may not work under
> those situations.

Can you be more specific about the failure scenarios?

Does this mean that migration cannot work today with guests using ipv6?  I don't 
think just pushing this to the guest is an acceptable solution in the short term.

Are there scenarios we cannot handle no matter what in the host?  Does this mean 
that for emulated drivers, we're completely out of luck?

Regards,

Anthony Liguori

Re: [Qemu-devel] [V5 PATCH 0/4] Send gratuitous packets by guest

[Jason Wang]

On 03/27/2012 06:10 AM, Anthony Liguori wrote:
> On 03/16/2012 03:54 AM, Jason Wang wrote:
>> This an update of series that let guest and qemu to be co-operated to
>> send gratuitous packets when needed such as after migration, loadvm
>> and continuing.
>>
>> As it's hard for qemu to track the network configuration in guest such
>> as bondings, vlans or ipv6. So current gratuitous may not work under
>> those situations.
>
> Can you be more specific about the failure scenarios?

The failure can happen when:

- Guest does not use primary mac address. Current qemu only send rarp 
packets for primary mac address. This looks could be solved by iterating 
nic with mac address table, but their size are limited and guest could 
use all-uni/promisc mode to use more mac addresses. So it's almost 
impossible to track all addresses in qemu side.

- Guest have some network configuration such as 802.1Q vlan, in this 
case, we need to send tagged gratuitous packet which qemu can't handle.

The point is qemu does not know the network configuration in guest or 
how mac addresses are used, so it's better for us to let guest choose 
the correct way to do this if they can. If guest could not do the 
announcement, the rarp packets would be sent as in the past.
>
> Does this mean that migration cannot work today with guests using 
> ipv6?  I don't think just pushing this to the guest is an acceptable 
> solution in the short term.
>

I haven't check, but w/o this patch a ipv4 rarp would be sent even guest 
are using ipv6; w/ this patch, a ipv6 neighbor advertisement would be 
sent by guest which looks pretty reasonable.

Did you see any drawbacks of this method? Xen and hyper-v also let guest 
to send gratuitous packet for their para-virtualized network card.
> Are there scenarios we cannot handle no matter what in the host?  Does 
> this mean that for emulated drivers, we're completely out of luck?
>

A possible improvement but not a final solution is to send garp for all 
address in the mac table I think.
> Regards,
>
> Anthony Liguori

Re: [Qemu-devel] [V5 PATCH 0/4] Send gratuitous packets by guest

[Michael S. Tsirkin]

On Mon, Mar 26, 2012 at 05:10:15PM -0500, Anthony Liguori wrote:
> On 03/16/2012 03:54 AM, Jason Wang wrote:
> >This an update of series that let guest and qemu to be co-operated to
> >send gratuitous packets when needed such as after migration, loadvm
> >and continuing.
> >
> >As it's hard for qemu to track the network configuration in guest such
> >as bondings, vlans or ipv6. So current gratuitous may not work under
> >those situations.
> 
> Can you be more specific about the failure scenarios?
> 
> Does this mean that migration cannot work today with guests using
> ipv6?  I don't think just pushing this to the guest is an acceptable
> solution in the short term.
> 
> Are there scenarios we cannot handle no matter what in the host?

Consider a nested virt scenario. It seems clear that you either
must notify the guest about migration, or learn nested
guest macs.

> Does this mean that for emulated drivers, we're completely out of
> luck?
> 
> Regards,
> 
> Anthony Liguori

For these, I think we can cause announcements by sending link up event
to the guest.

-- 
MST