Re: [Qemu-devel] [PATCH v3 1/2] vhost user: add support of live migration

["Michael S. Tsirkin" <mst@redhat.com>]

On Wed, Jun 10, 2015 at 03:43:02PM +0200, Thibaut Collet wrote:
> Some vhost client/backend are able to support live migration.
> To provide this service the following features must be added:
> 1. GARP from guest after live migration:
>    the VIRTIO_NET_F_GUEST_ANNOUNCE capability is added to vhost-net when netdev
>    backend is vhost-user to let virtio-net NIC manages GARP after migration.
> 2. RARP on vhost-user:
>    After a live migration RARP are automatically sent to any interfaces. A
>    receive callback is added to vhost-user to catch this packet. If guest
>    supports VIRTIO_NET_F_GUEST_ANNOUNCE this packet is discarded to avoid
>    message duplication (already done by virtio-net NIC). For legacy guest
>    without VIRTIO_NET_F_GUEST_ANNOUNCE a warning message is displayed to
>    alert the user. RARP must be sent to the vhost client/backend.
> 
> Signed-off-by: Thibaut Collet <thibaut.collet@6wind.com>
> ---
>  hw/net/vhost_net.c      |   15 +++++++++++++++
>  include/net/vhost_net.h |    1 +
>  net/vhost-user.c        |   21 +++++++++++++++++++--
>  3 files changed, 35 insertions(+), 2 deletions(-)
> 
> diff --git a/hw/net/vhost_net.c b/hw/net/vhost_net.c
> index 426b23e..4a42325 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
> @@ -365,6 +367,15 @@ void vhost_net_stop(VirtIODevice *dev, NetClientState *ncs,
>      assert(r >= 0);
>  }
>  
> +void vhost_net_inject_rarp(struct vhost_net *net, const uint8_t *buf, size_t size)
> +{
> +    if ((net->dev.acked_features & (1 << VIRTIO_NET_F_GUEST_ANNOUNCE)) == 0) {
> +        fprintf(stderr,
> +                "Warning: Guest with no VIRTIO_NET_F_GUEST_ANNOUNCE support. RARP must be sent by vhost-user backend\n");
> +        fflush(stderr);

And maybe it does, then you are just filling log up with useless
warnings. Pls add some ifdef so this isn't normally compiled in.

> +    }

Can you move this out to vhost-user? It's not a generic function, is it?

> +}
> +
>  void vhost_net_cleanup(struct vhost_net *net)
>  {
>      vhost_dev_cleanup(&net->dev);
> @@ -427,6 +438,10 @@ void vhost_net_stop(VirtIODevice *dev,
>  {
>  }
>  
> +void vhost_net_inject_rarp(struct vhost_net *net, const uint8_t *buf, size_t size);
> +{
> +}
> +
>  void vhost_net_cleanup(struct vhost_net *net)
>  {
>  }
> diff --git a/include/net/vhost_net.h b/include/net/vhost_net.h
> index b1c18a3..e82a0f9 100644
> --- a/include/net/vhost_net.h
> +++ b/include/net/vhost_net.h
> @@ -20,6 +20,7 @@ bool vhost_net_query(VHostNetState *net, VirtIODevice *dev);
>  int vhost_net_start(VirtIODevice *dev, NetClientState *ncs, int total_queues);
>  void vhost_net_stop(VirtIODevice *dev, NetClientState *ncs, int total_queues);
>  
> +void vhost_net_inject_rarp(VHostNetState *net, const uint8_t *buf, size_t size);
>  void vhost_net_cleanup(VHostNetState *net);
>  
>  unsigned vhost_net_get_features(VHostNetState *net, unsigned features);
> diff --git a/net/vhost-user.c b/net/vhost-user.c
> index 8d26728..69c2313 100644
> --- a/net/vhost-user.c
> +++ b/net/vhost-user.c
> @@ -66,6 +66,24 @@ static void vhost_user_stop(VhostUserState *s)
>      s->vhost_net = 0;
>  }
>  
> +static ssize_t vhost_user_receive(NetClientState *nc, const uint8_t *buf,
> +                                  size_t size)
> +{
> +    if (size == 60) {
> +        /* Assume it is a RARP request sent automatically after a
> +         * live migration */
> +        /* The RARP must be sent if guest does not support
> +         * VIRTIO_NET_F_GUEST_ANNOUNCE */
> +        VhostUserState *s = DO_UPCAST(VhostUserState, nc, nc);
> +
> +        vhost_net_inject_rarp(s->vhost_net, buf, size);
> +    } else {
> +        fprintf(stderr,"Vhost user receives unexpected packets\n");
> +        fflush(stderr);
> +    }
> +    return size;
> +}
> +
>  static void vhost_user_cleanup(NetClientState *nc)
>  {
>      VhostUserState *s = DO_UPCAST(VhostUserState, nc, nc);
> @@ -91,6 +109,7 @@ static bool vhost_user_has_ufo(NetClientState *nc)
>  static NetClientInfo net_vhost_user_info = {
>          .type = NET_CLIENT_OPTIONS_KIND_VHOST_USER,
>          .size = sizeof(VhostUserState),
> +        .receive = vhost_user_receive,
>          .cleanup = vhost_user_cleanup,
>          .has_vnet_hdr = vhost_user_has_vnet_hdr,
>          .has_ufo = vhost_user_has_ufo,
> @@ -147,8 +166,6 @@ static int net_vhost_user_init(NetClientState *peer, const char *device,
>  
>          s = DO_UPCAST(VhostUserState, nc, nc);
>  
> -        /* We don't provide a receive callback */
> -        s->nc.receive_disabled = 1;
>          s->chr = chr;
>          s->nc.queue_index = i;
>  
> -- 
> 1.7.10.4