Re: [Qemu-devel] [PATCH v2] net: Add support of VIRTIO_NET_F_GUEST_ANNOUNCE for vhost-net/vhost-user

[Thibaut Collet]

Hi,

My understanding of gratuitous packet with virtio for any backend (vhost user or other):

- When the VM is loaded (first start or migration) the virtio net interfaces are loaded ( virtio_net_load_device function in hw/net/virtio-net.c)

- If the guest has the VIRTIO_NET_F_GUEST_ANNOUNCE capability, request to send gratuitous packet is done.

1. To enable gratuitous packet through this mechanism I have added VIRTIO_NET_F_GUEST_ANNOUNCE capability to hw/net/vhost_net.c. So host and guest can negotiate this feature when vhost-user is used.

2. self announce occurs in case of live migration. During a live migration a GARP is sent to all net backend through a queue dedicated to the net backend.

   But for vhost-user:

       - this operation is not possible (vhost-user has no queue)

       - it is already done with the previous mechanism.

   Rather to define a queue to vhost user and notify twice the guest to send gratuitous packet I have disable GARP from self announce and use only the first mechanism for that.

I have tested my modifications with guest that supports VIRTIO_NET_F_GUEST_ANNOUNCE and vhost-user on the host. After a live migration I have the GARP from the guest.

Regards.

On Mon, Jun 8, 2015 at 7:55 AM, Jason Wang <address@hidden> wrote:

On 06/05/2015 09:24 PM, Thibaut Collet wrote:
> Add VIRTIO_NET_F_GUEST_ANNOUNCE capability to vhost-net when netdev backend is
> vhost-user.
>
> For netdev backend using virtio-net NIC the self announce is managed directly
> by the virtio-net NIC and not by the netdev backend itself.
> To avoid duplication of announces (once from the guest and once from QEMU) a
> bitfield is added in the NetClientState structure.
> If this bit is set self announce does not send message to the guest to request
> gratuitous ARP but let virtio-net NIC set the VIRTIO_NET_S_ANNOUNCE for
> gratuitous ARP.
>
> Signed-off-by: Thibaut Collet <address@hidden>
> ---
> v2: do not discard anymore packets send to vhost-user: it is GARP request after
>     live migration.
>     As suggested by S. Hajnoczi qemu_announce_self skips virtio-net NIC that
>     already send GARP.
>
>  hw/net/vhost_net.c |    2 ++
>  include/net/net.h  |    1 +
>  net/vhost-user.c   |    2 ++
>  savevm.c           |   11 ++++++++---
>  4 files changed, 13 insertions(+), 3 deletions(-)
>
> diff --git a/hw/net/vhost_net.c b/hw/net/vhost_net.c
> index 426b23e..a745f97 100644
> --- a/hw/net/vhost_net.c
> +++ b/hw/net/vhost_net.c
> @@ -82,6 +82,8 @@ static const int user_feature_bits[] = {
>      VIRTIO_NET_F_CTRL_MAC_ADDR,
>      VIRTIO_NET_F_CTRL_GUEST_OFFLOADS,
>
> +    VIRTIO_NET_F_GUEST_ANNOUNCE,
> +
>      VIRTIO_NET_F_MQ,
>
>      VHOST_INVALID_FEATURE_BIT
> diff --git a/include/net/net.h b/include/net/net.h
> index e66ca03..a78e9df 100644
> --- a/include/net/net.h
> +++ b/include/net/net.h
> @@ -85,6 +85,7 @@ struct NetClientState {
>      char *name;
>      char info_str[256];
>      unsigned receive_disabled : 1;
> +    unsigned self_announce_disabled : 1;
>      NetClientDestructor *destructor;
>      unsigned int queue_index;
>      unsigned rxfilter_notify_enabled:1;
> diff --git a/net/vhost-user.c b/net/vhost-user.c
> index 8d26728..b345446 100644
> --- a/net/vhost-user.c
> +++ b/net/vhost-user.c
> @@ -147,6 +147,8 @@ static int net_vhost_user_init(NetClientState *peer, const char *device,
>
>          s = DO_UPCAST(VhostUserState, nc, nc);
>
> +        /* Self announce is managed directly by virtio-net NIC */
> +        s->nc.self_announce_disabled = 1;

Shouldn't we do this in set_features consider it needs guest support or
you want to disable gratuitous packet for vhost-user at all?

>          /* We don't provide a receive callback */
>          s->nc.receive_disabled = 1;
>          s->chr = chr;
> diff --git a/savevm.c b/savevm.c
> index 3b0e222..7a134b1 100644
> --- a/savevm.c
> +++ b/savevm.c
> @@ -80,11 +80,16 @@ static void qemu_announce_self_iter(NICState *nic, void *opaque)
>  {
>      uint8_t buf[60];
>      int len;
> +    NetClientState *nc;
>
> -    trace_qemu_announce_self_iter(qemu_ether_ntoa(&nic->conf->macaddr));
> -    len = announce_self_create(buf, nic->conf->macaddr.a);
> +    nc = qemu_get_queue(nic);
>
> -    qemu_send_packet_raw(qemu_get_queue(nic), buf, len);
> +    if (!nc->peer->self_announce_disabled) {
> +        trace_qemu_announce_self_iter(qemu_ether_ntoa(&nic->conf->macaddr));
> +        len = announce_self_create(buf, nic->conf->macaddr.a);
> +
> +        qemu_send_packet_raw(nc, buf, len);
> +    }
>  }
>
>