diff mbox

[v7,16/24] vhost user: add rarp sending after live migration for legacy guest

Message ID 1443720248-15482-17-git-send-email-marcandre.lureau@redhat.com
State New
Headers show

Commit Message

Marc-André Lureau Oct. 1, 2015, 5:24 p.m. UTC 
From: Thibaut Collet <thibaut.collet@6wind.com>

A new vhost user message is added to allow QEMU to ask to vhost user backend to
broadcast a fake RARP after live migration for guest without GUEST_ANNOUNCE
capability.

This new message is sent only if the backend supports the new
VHOST_USER_PROTOCOL_F_RARP protocol feature.
The payload of this new message is the MAC address of the guest (not known by
the backend). The MAC address is copied in the first 6 bytes of a u64 to avoid
to create a new payload message type.

This new message has no equivalent ioctl so a new callback is added in the
userOps structure to send the request.

Upon reception of this new message the vhost user backend must generate and
broadcast a fake RARP request to notify the migration is terminated.

Signed-off-by: Thibaut Collet <thibaut.collet@6wind.com>
[Rebased and fixed checkpatch errors - Marc-André]
Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
---
 docs/specs/vhost-user.txt         | 15 +++++++++++++++
 hw/net/vhost_net.c                | 17 +++++++++++++++++
 hw/virtio/vhost-user.c            | 32 +++++++++++++++++++++++++++++++-
 include/hw/virtio/vhost-backend.h |  3 +++
 include/net/vhost_net.h           |  1 +
 net/vhost-user.c                  | 24 ++++++++++++++++++++++--
 6 files changed, 89 insertions(+), 3 deletions(-)

Comments

Michael S. Tsirkin Oct. 2, 2015, 1:37 p.m. UTC | #1 
On Thu, Oct 01, 2015 at 07:24:00PM +0200, marcandre.lureau@redhat.com wrote:
> diff --git a/hw/net/vhost_net.c b/hw/net/vhost_net.c
> index 840f443..da66b64 100644
> --- a/hw/net/vhost_net.c
> +++ b/hw/net/vhost_net.c
> @@ -388,6 +388,18 @@ void vhost_net_cleanup(struct vhost_net *net)
>      g_free(net);
>  }
>  
> +int vhost_net_notify_migration_done(struct vhost_net *net, char* mac_addr)
> +{
> +    const VhostOps *vhost_ops = net->dev.vhost_ops;
> +    int r = -1;
> +
> +    if (vhost_ops->vhost_migration_done) {
> +        r = vhost_ops->vhost_migration_done(&net->dev, mac_addr);
> +    }
> +
> +    return r;
> +}
> +
>  bool vhost_net_virtqueue_pending(VHostNetState *net, int idx)
>  {
>      return vhost_virtqueue_pending(&net->dev, idx);
> @@ -479,6 +491,11 @@ void vhost_net_virtqueue_mask(VHostNetState *net, VirtIODevice *dev,
>  {
>  }
>  
> +int vhost_net_notify_migration_done(struct vhost_net *net)
> +{
> +    return -1;
> +}
> +
>  VHostNetState *get_vhost_net(NetClientState *nc)
>  {
>      return 0;

This signature does not fit the one above.
How was this tested?
Marc-Andre Lureau Oct. 2, 2015, 1:55 p.m. UTC | #2 
----- Original Message -----
> On Thu, Oct 01, 2015 at 07:24:00PM +0200, marcandre.lureau@redhat.com wrote:
> > diff --git a/hw/net/vhost_net.c b/hw/net/vhost_net.c
> > index 840f443..da66b64 100644
> > --- a/hw/net/vhost_net.c
> > +++ b/hw/net/vhost_net.c
> > @@ -388,6 +388,18 @@ void vhost_net_cleanup(struct vhost_net *net)
> >      g_free(net);
> >  }
> >  
> > +int vhost_net_notify_migration_done(struct vhost_net *net, char* mac_addr)
> > +{
> > +    const VhostOps *vhost_ops = net->dev.vhost_ops;
> > +    int r = -1;
> > +
> > +    if (vhost_ops->vhost_migration_done) {
> > +        r = vhost_ops->vhost_migration_done(&net->dev, mac_addr);
> > +    }
> > +
> > +    return r;
> > +}
> > +
> >  bool vhost_net_virtqueue_pending(VHostNetState *net, int idx)
> >  {
> >      return vhost_virtqueue_pending(&net->dev, idx);
> > @@ -479,6 +491,11 @@ void vhost_net_virtqueue_mask(VHostNetState *net,
> > VirtIODevice *dev,
> >  {
> >  }
> >  
> > +int vhost_net_notify_migration_done(struct vhost_net *net)
> > +{
> > +    return -1;
> > +}
> > +
> >  VHostNetState *get_vhost_net(NetClientState *nc)
> >  {
> >      return 0;
> 
> This signature does not fit the one above.
> How was this tested?
> 

Good question, I totally missed that. It has been there since Thibaut v6 series.

I guess we all compile with CONFIG_VHOST_NET, and adding the missing char* mac_addr is enough to fix this.
Michael S. Tsirkin Oct. 2, 2015, 2:02 p.m. UTC | #3 
On Fri, Oct 02, 2015 at 09:55:01AM -0400, Marc-André Lureau wrote:
> 
> 
> ----- Original Message -----
> > On Thu, Oct 01, 2015 at 07:24:00PM +0200, marcandre.lureau@redhat.com wrote:
> > > diff --git a/hw/net/vhost_net.c b/hw/net/vhost_net.c
> > > index 840f443..da66b64 100644
> > > --- a/hw/net/vhost_net.c
> > > +++ b/hw/net/vhost_net.c
> > > @@ -388,6 +388,18 @@ void vhost_net_cleanup(struct vhost_net *net)
> > >      g_free(net);
> > >  }
> > >  
> > > +int vhost_net_notify_migration_done(struct vhost_net *net, char* mac_addr)
> > > +{
> > > +    const VhostOps *vhost_ops = net->dev.vhost_ops;
> > > +    int r = -1;
> > > +
> > > +    if (vhost_ops->vhost_migration_done) {
> > > +        r = vhost_ops->vhost_migration_done(&net->dev, mac_addr);
> > > +    }
> > > +
> > > +    return r;
> > > +}
> > > +
> > >  bool vhost_net_virtqueue_pending(VHostNetState *net, int idx)
> > >  {
> > >      return vhost_virtqueue_pending(&net->dev, idx);
> > > @@ -479,6 +491,11 @@ void vhost_net_virtqueue_mask(VHostNetState *net,
> > > VirtIODevice *dev,
> > >  {
> > >  }
> > >  
> > > +int vhost_net_notify_migration_done(struct vhost_net *net)
> > > +{
> > > +    return -1;
> > > +}
> > > +
> > >  VHostNetState *get_vhost_net(NetClientState *nc)
> > >  {
> > >      return 0;
> > 
> > This signature does not fit the one above.
> > How was this tested?
> > 
> 
> Good question, I totally missed that. It has been there since Thibaut v6 series.
> 
> I guess we all compile with CONFIG_VHOST_NET, and adding the missing char* mac_addr is enough to fix this.

You will catch this if you build and test all targets.
Thibaut Collet Oct. 2, 2015, 2:38 p.m. UTC | #4 
On Fri, Oct 2, 2015 at 4:02 PM, Michael S. Tsirkin <mst@redhat.com> wrote:
> On Fri, Oct 02, 2015 at 09:55:01AM -0400, Marc-André Lureau wrote:
>>
>>
>> ----- Original Message -----
>> > On Thu, Oct 01, 2015 at 07:24:00PM +0200, marcandre.lureau@redhat.com wrote:
>> > > diff --git a/hw/net/vhost_net.c b/hw/net/vhost_net.c
>> > > index 840f443..da66b64 100644
>> > > --- a/hw/net/vhost_net.c
>> > > +++ b/hw/net/vhost_net.c
>> > > @@ -388,6 +388,18 @@ void vhost_net_cleanup(struct vhost_net *net)
>> > >      g_free(net);
>> > >  }
>> > >
>> > > +int vhost_net_notify_migration_done(struct vhost_net *net, char* mac_addr)
>> > > +{
>> > > +    const VhostOps *vhost_ops = net->dev.vhost_ops;
>> > > +    int r = -1;
>> > > +
>> > > +    if (vhost_ops->vhost_migration_done) {
>> > > +        r = vhost_ops->vhost_migration_done(&net->dev, mac_addr);
>> > > +    }
>> > > +
>> > > +    return r;
>> > > +}
>> > > +
>> > >  bool vhost_net_virtqueue_pending(VHostNetState *net, int idx)
>> > >  {
>> > >      return vhost_virtqueue_pending(&net->dev, idx);
>> > > @@ -479,6 +491,11 @@ void vhost_net_virtqueue_mask(VHostNetState *net,
>> > > VirtIODevice *dev,
>> > >  {
>> > >  }
>> > >
>> > > +int vhost_net_notify_migration_done(struct vhost_net *net)
>> > > +{
>> > > +    return -1;
>> > > +}
>> > > +
>> > >  VHostNetState *get_vhost_net(NetClientState *nc)
>> > >  {
>> > >      return 0;
>> >
>> > This signature does not fit the one above.
>> > How was this tested?
>> >
>>
>> Good question, I totally missed that. It has been there since Thibaut v6 series.
>>
>> I guess we all compile with CONFIG_VHOST_NET, and adding the missing char* mac_addr is enough to fix this.
>
> You will catch this if you build and test all targets.
>
> --
> MST

Sorry for the omission.
I have tested some QEMU configuration but all of them to test my feature.
The add of the missing char* mac_addr is sufficient : in this case the
function will be compliant with its definition set in the
include/net/vhost_net.h file.

Next time I will build all targets to avoid this kind of errors.

Thibaut.
diff mbox

Patch

diff --git a/docs/specs/vhost-user.txt b/docs/specs/vhost-user.txt
index e0292a0..e0d71e2 100644
--- a/docs/specs/vhost-user.txt
+++ b/docs/specs/vhost-user.txt
@@ -194,6 +194,7 @@  Protocol features
 
 #define VHOST_USER_PROTOCOL_F_MQ             0
 #define VHOST_USER_PROTOCOL_F_LOG_SHMFD      1
+#define VHOST_USER_PROTOCOL_F_RARP           2
 
 Message types
 -------------
@@ -381,3 +382,17 @@  Message types
       Master payload: vring state description
 
       Signal slave to enable or disable corresponding vring.
+
+ * VHOST_USER_SEND_RARP
+
+      Id: 19
+      Equivalent ioctl: N/A
+      Master payload: u64
+
+      Ask vhost user backend to broadcast a fake RARP to notify the migration
+      is terminated for guest that does not support GUEST_ANNOUNCE.
+      Only legal if feature bit VHOST_USER_F_PROTOCOL_FEATURES is present in
+      VHOST_USER_GET_FEATURES and protocol feature bit VHOST_USER_PROTOCOL_F_RARP
+      is present in VHOST_USER_GET_PROTOCOL_FEATURES.
+      The first 6 bytes of the payload contain the mac address of the guest to
+      allow the vhost user backend to construct and broadcast the fake RARP.
diff --git a/hw/net/vhost_net.c b/hw/net/vhost_net.c
index 840f443..da66b64 100644
--- a/hw/net/vhost_net.c
+++ b/hw/net/vhost_net.c
@@ -388,6 +388,18 @@  void vhost_net_cleanup(struct vhost_net *net)
     g_free(net);
 }
 
+int vhost_net_notify_migration_done(struct vhost_net *net, char* mac_addr)
+{
+    const VhostOps *vhost_ops = net->dev.vhost_ops;
+    int r = -1;
+
+    if (vhost_ops->vhost_migration_done) {
+        r = vhost_ops->vhost_migration_done(&net->dev, mac_addr);
+    }
+
+    return r;
+}
+
 bool vhost_net_virtqueue_pending(VHostNetState *net, int idx)
 {
     return vhost_virtqueue_pending(&net->dev, idx);
@@ -479,6 +491,11 @@  void vhost_net_virtqueue_mask(VHostNetState *net, VirtIODevice *dev,
 {
 }
 
+int vhost_net_notify_migration_done(struct vhost_net *net)
+{
+    return -1;
+}
+
 VHostNetState *get_vhost_net(NetClientState *nc)
 {
     return 0;
diff --git a/hw/virtio/vhost-user.c b/hw/virtio/vhost-user.c
index cd84f0c..5ab888d 100644
--- a/hw/virtio/vhost-user.c
+++ b/hw/virtio/vhost-user.c
@@ -10,6 +10,7 @@ 
 
 #include "hw/virtio/vhost.h"
 #include "hw/virtio/vhost-backend.h"
+#include "hw/virtio/virtio-net.h"
 #include "sysemu/char.h"
 #include "sysemu/kvm.h"
 #include "qemu/error-report.h"
@@ -27,9 +28,10 @@ 
 #define VHOST_MEMORY_MAX_NREGIONS    8
 #define VHOST_USER_F_PROTOCOL_FEATURES 30
 
-#define VHOST_USER_PROTOCOL_FEATURE_MASK 0x3ULL
+#define VHOST_USER_PROTOCOL_FEATURE_MASK 0x7ULL
 #define VHOST_USER_PROTOCOL_F_MQ         0
 #define VHOST_USER_PROTOCOL_F_LOG_SHMFD  1
+#define VHOST_USER_PROTOCOL_F_RARP       2
 
 typedef enum VhostUserRequest {
     VHOST_USER_NONE = 0,
@@ -51,6 +53,7 @@  typedef enum VhostUserRequest {
     VHOST_USER_SET_PROTOCOL_FEATURES = 16,
     VHOST_USER_GET_QUEUE_NUM = 17,
     VHOST_USER_SET_VRING_ENABLE = 18,
+    VHOST_USER_SEND_RARP = 19,
     VHOST_USER_MAX
 } VhostUserRequest;
 
@@ -557,6 +560,32 @@  static bool vhost_user_requires_shm_log(struct vhost_dev *dev)
                               VHOST_USER_PROTOCOL_F_LOG_SHMFD);
 }
 
+static int vhost_user_migration_done(struct vhost_dev *dev, char* mac_addr)
+{
+    VhostUserMsg msg = { 0 };
+    int err;
+
+    assert(dev->vhost_ops->backend_type == VHOST_BACKEND_TYPE_USER);
+
+    /* If guest supports GUEST_ANNOUNCE do nothing */
+    if (virtio_has_feature(dev->acked_features, VIRTIO_NET_F_GUEST_ANNOUNCE)) {
+        return 0;
+    }
+
+    /* if backend supports VHOST_USER_PROTOCOL_F_RARP ask it to send the RARP */
+    if (virtio_has_feature(dev->protocol_features,
+                           VHOST_USER_PROTOCOL_F_RARP)) {
+        msg.request = VHOST_USER_SEND_RARP;
+        msg.flags = VHOST_USER_VERSION;
+        memcpy((char *)&msg.u64, mac_addr, 6);
+        msg.size = sizeof(m.u64);
+
+        err = vhost_user_write(dev, &msg, NULL, 0);
+        return err;
+    }
+    return -1;
+}
+
 const VhostOps user_ops = {
         .backend_type = VHOST_BACKEND_TYPE_USER,
         .vhost_backend_init = vhost_user_init,
@@ -578,4 +607,5 @@  const VhostOps user_ops = {
         .vhost_get_vq_index = vhost_user_get_vq_index,
         .vhost_set_vring_enable = vhost_user_set_vring_enable,
         .vhost_requires_shm_log = vhost_user_requires_shm_log,
+        .vhost_migration_done = vhost_user_migration_done,
 };
diff --git a/include/hw/virtio/vhost-backend.h b/include/hw/virtio/vhost-backend.h
index e07118c..5918c01 100644
--- a/include/hw/virtio/vhost-backend.h
+++ b/include/hw/virtio/vhost-backend.h
@@ -67,6 +67,8 @@  typedef int (*vhost_get_vq_index_op)(struct vhost_dev *dev, int idx);
 typedef int (*vhost_set_vring_enable_op)(struct vhost_dev *dev,
                                          int enable);
 typedef bool (*vhost_requires_shm_log_op)(struct vhost_dev *dev);
+typedef int (*vhost_migration_done_op)(struct vhost_dev *dev,
+                                       char *mac_addr);
 
 typedef struct VhostOps {
     VhostBackendType backend_type;
@@ -94,6 +96,7 @@  typedef struct VhostOps {
     vhost_get_vq_index_op vhost_get_vq_index;
     vhost_set_vring_enable_op vhost_set_vring_enable;
     vhost_requires_shm_log_op vhost_requires_shm_log;
+    vhost_migration_done_op vhost_migration_done;
 } VhostOps;
 
 extern const VhostOps user_ops;
diff --git a/include/net/vhost_net.h b/include/net/vhost_net.h
index 0188c4d..3389b41 100644
--- a/include/net/vhost_net.h
+++ b/include/net/vhost_net.h
@@ -27,6 +27,7 @@  void vhost_net_ack_features(VHostNetState *net, uint64_t features);
 bool vhost_net_virtqueue_pending(VHostNetState *net, int n);
 void vhost_net_virtqueue_mask(VHostNetState *net, VirtIODevice *dev,
                               int idx, bool mask);
+int vhost_net_notify_migration_done(VHostNetState *net, char* mac_addr);
 VHostNetState *get_vhost_net(NetClientState *nc);
 
 int vhost_set_vring_enable(NetClientState * nc, int enable);
diff --git a/net/vhost-user.c b/net/vhost-user.c
index 87917a5..cfe11b8 100644
--- a/net/vhost-user.c
+++ b/net/vhost-user.c
@@ -106,9 +106,29 @@  err:
 static ssize_t vhost_user_receive(NetClientState *nc, const uint8_t *buf,
                                   size_t size)
 {
-    /* Discard the request that is received and managed by backend
-     * by an other way.
+    /* In case of RARP (message size is 60) notify backup to send a fake RARP.
+       This fake RARP will be sent by backend only for guest
+       without GUEST_ANNOUNCE capability.
      */
+    if (size == 60) {
+        VhostUserState *s = DO_UPCAST(VhostUserState, nc, nc);
+        int r;
+        static int display_rarp_failure = 1;
+        char mac_addr[6];
+
+        /* extract guest mac address from the RARP message */
+        memcpy(mac_addr, &buf[6], 6);
+
+        r = vhost_net_notify_migration_done(s->vhost_net, mac_addr);
+
+        if ((r != 0) && (display_rarp_failure)) {
+            fprintf(stderr,
+                    "Vhost user backend fails to broadcast fake RARP\n");
+            fflush(stderr);
+            display_rarp_failure = 0;
+        }
+    }
+
     return size;
 }