diff mbox

[v5,2/2] vhost user: add rarp sending after live migration for legacy guest

Message ID 1438593754-30005-3-git-send-email-thibaut.collet@6wind.com
State New
Headers show

Commit Message

Thibaut Collet Aug. 3, 2015, 9:22 a.m. UTC
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>
---
 docs/specs/vhost-user.txt         |   15 +++++++++++++++
 hw/net/vhost_net.c                |   16 ++++++++++++++++
 hw/virtio/vhost-user.c            |   32 ++++++++++++++++++++++++++++++--
 include/hw/virtio/vhost-backend.h |    2 ++
 include/net/vhost_net.h           |    1 +
 net/vhost-user.c                  |   18 ++++++++++++++++++
 6 files changed, 82 insertions(+), 2 deletions(-)

Comments

Jason Wang Aug. 4, 2015, 6:22 a.m. UTC | #1
On 08/03/2015 05:22 PM, Thibaut Collet wrote:
> 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>
> ---
>  docs/specs/vhost-user.txt         |   15 +++++++++++++++
>  hw/net/vhost_net.c                |   16 ++++++++++++++++
>  hw/virtio/vhost-user.c            |   32 ++++++++++++++++++++++++++++++--
>  include/hw/virtio/vhost-backend.h |    2 ++
>  include/net/vhost_net.h           |    1 +
>  net/vhost-user.c                  |   18 ++++++++++++++++++
>  6 files changed, 82 insertions(+), 2 deletions(-)
>
> diff --git a/docs/specs/vhost-user.txt b/docs/specs/vhost-user.txt
> index c2d2e2a..8c05301 100644
> --- a/docs/specs/vhost-user.txt
> +++ b/docs/specs/vhost-user.txt
> @@ -140,6 +140,7 @@ Protocol features
>  -----------------
>  
>  #define VHOST_USER_PROTOCOL_F_LOG_SHMFD 0
> +#define VHOST_USER_PROTOCOL_F_RARP      1
>  
>  Message types
>  -------------
> @@ -308,6 +309,20 @@ Message types
>        invalid FD flag. This flag is set when there is no file descriptor
>        in the ancillary data.
>  
> + * VHOST_USER_SEND_RARP
> +
> +      Id: 17
> +      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.
> +
>  Migration

If for some reason the announce packet was changed in the future, we may
then need another kind of message type. Just wonder whether allowing
qemu to inject packet directly to vhost-user is better.

>  ---------
>  
> diff --git a/hw/net/vhost_net.c b/hw/net/vhost_net.c
> index 9850520..c3b9664 100644
> --- a/hw/net/vhost_net.c
> +++ b/hw/net/vhost_net.c
> @@ -383,6 +383,17 @@ void vhost_net_cleanup(struct vhost_net *net)
>      g_free(net);
>  }
>  
> +int vhost_net_migrate(struct vhost_net *net, char* mac_addr)
> +{
> +    const VhostOps *vhost_ops = net->dev.vhost_ops;
> +    int r = -1;
> +
> +    if (vhost_ops->vhost_backend_migrate)
> +        r = vhost_ops->vhost_backend_migrate(&net->dev, mac_addr);
> +
> +    return r;
> +}
> +
>  bool vhost_net_virtqueue_pending(VHostNetState *net, int idx)
>  {
>      return vhost_virtqueue_pending(&net->dev, idx);
> @@ -456,6 +467,11 @@ void vhost_net_virtqueue_mask(VHostNetState *net, VirtIODevice *dev,
>  {
>  }
>  
> +int vhost_net_migrate(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 fe75618..693840f 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,8 +28,9 @@
>  
>  #define VHOST_USER_F_PROTOCOL_FEATURES 30
>  
> -#define VHOST_USER_PROTOCOL_FEATURE_MASK 0x1ULL
> +#define VHOST_USER_PROTOCOL_FEATURE_MASK 0x3ULL
>  #define VHOST_USER_PROTOCOL_F_LOG_SHMFD 0
> +#define VHOST_USER_PROTOCOL_F_RARP      1
>  
>  typedef enum VhostUserRequest {
>      VHOST_USER_NONE = 0,
> @@ -48,6 +50,7 @@ typedef enum VhostUserRequest {
>      VHOST_USER_SET_VRING_ERR = 14,
>      VHOST_USER_GET_PROTOCOL_FEATURES = 15,
>      VHOST_USER_SET_PROTOCOL_FEATURES = 16,
> +    VHOST_USER_SEND_RARP = 17,
>      VHOST_USER_MAX
>  } VhostUserRequest;
>  
> @@ -409,9 +412,34 @@ static int vhost_user_cleanup(struct vhost_dev *dev)
>      return 0;
>  }
>  
> +static int vhost_user_migrate(struct vhost_dev *dev, char* mac_addr)
> +{

Not sure migrate is a good name since migration was not done in this
function and it was called in vhost_user_receive().
Thibaut Collet Aug. 4, 2015, 9:27 a.m. UTC | #2
On Tue, Aug 4, 2015 at 8:22 AM, Jason Wang <jasowang@redhat.com> wrote:
>
>
> On 08/03/2015 05:22 PM, Thibaut Collet wrote:
>> 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>
>> ---
>>  docs/specs/vhost-user.txt         |   15 +++++++++++++++
>>  hw/net/vhost_net.c                |   16 ++++++++++++++++
>>  hw/virtio/vhost-user.c            |   32 ++++++++++++++++++++++++++++++--
>>  include/hw/virtio/vhost-backend.h |    2 ++
>>  include/net/vhost_net.h           |    1 +
>>  net/vhost-user.c                  |   18 ++++++++++++++++++
>>  6 files changed, 82 insertions(+), 2 deletions(-)
>>
>> diff --git a/docs/specs/vhost-user.txt b/docs/specs/vhost-user.txt
>> index c2d2e2a..8c05301 100644
>> --- a/docs/specs/vhost-user.txt
>> +++ b/docs/specs/vhost-user.txt
>> @@ -140,6 +140,7 @@ Protocol features
>>  -----------------
>>
>>  #define VHOST_USER_PROTOCOL_F_LOG_SHMFD 0
>> +#define VHOST_USER_PROTOCOL_F_RARP      1
>>
>>  Message types
>>  -------------
>> @@ -308,6 +309,20 @@ Message types
>>        invalid FD flag. This flag is set when there is no file descriptor
>>        in the ancillary data.
>>
>> + * VHOST_USER_SEND_RARP
>> +
>> +      Id: 17
>> +      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.
>> +
>>  Migration
>
> If for some reason the announce packet was changed in the future, we may
> then need another kind of message type. Just wonder whether allowing
> qemu to inject packet directly to vhost-user is better.
>

Purpose of the announce packet is to notified switches that MAC
address of the guest has moved to avoid network outage.
So only knowledge of the MAC address is needed to construct the
announce packet (whatever is format)
The new message provides the MAC address.  Vhost user backend can
create an announce packet that is not necessary the same of QEMU

Providing the announce packet directly to vhost user is possible but
needs a lot of additional change:
 - The announce packet can not be copy in the master payload (in the
future if the announce packet changed its size can changed too)
The master payload can be a u64 that gives the size of the message.
The message will be stored in a shared memory between QEMU and backend
and a fd to this shared memory must be provided too.
That can be interesting if QEMU has several type of packets to inject
to vhost user but it is not the case and I prefer to keep my solution
(less complex and risk of bugs or memory leakage is less)

>>  ---------
>>
>> diff --git a/hw/net/vhost_net.c b/hw/net/vhost_net.c
>> index 9850520..c3b9664 100644
>> --- a/hw/net/vhost_net.c
>> +++ b/hw/net/vhost_net.c
>> @@ -383,6 +383,17 @@ void vhost_net_cleanup(struct vhost_net *net)
>>      g_free(net);
>>  }
>>
>> +int vhost_net_migrate(struct vhost_net *net, char* mac_addr)
>> +{
>> +    const VhostOps *vhost_ops = net->dev.vhost_ops;
>> +    int r = -1;
>> +
>> +    if (vhost_ops->vhost_backend_migrate)
>> +        r = vhost_ops->vhost_backend_migrate(&net->dev, mac_addr);
>> +
>> +    return r;
>> +}
>> +
>>  bool vhost_net_virtqueue_pending(VHostNetState *net, int idx)
>>  {
>>      return vhost_virtqueue_pending(&net->dev, idx);
>> @@ -456,6 +467,11 @@ void vhost_net_virtqueue_mask(VHostNetState *net, VirtIODevice *dev,
>>  {
>>  }
>>
>> +int vhost_net_migrate(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 fe75618..693840f 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,8 +28,9 @@
>>
>>  #define VHOST_USER_F_PROTOCOL_FEATURES 30
>>
>> -#define VHOST_USER_PROTOCOL_FEATURE_MASK 0x1ULL
>> +#define VHOST_USER_PROTOCOL_FEATURE_MASK 0x3ULL
>>  #define VHOST_USER_PROTOCOL_F_LOG_SHMFD 0
>> +#define VHOST_USER_PROTOCOL_F_RARP      1
>>
>>  typedef enum VhostUserRequest {
>>      VHOST_USER_NONE = 0,
>> @@ -48,6 +50,7 @@ typedef enum VhostUserRequest {
>>      VHOST_USER_SET_VRING_ERR = 14,
>>      VHOST_USER_GET_PROTOCOL_FEATURES = 15,
>>      VHOST_USER_SET_PROTOCOL_FEATURES = 16,
>> +    VHOST_USER_SEND_RARP = 17,
>>      VHOST_USER_MAX
>>  } VhostUserRequest;
>>
>> @@ -409,9 +412,34 @@ static int vhost_user_cleanup(struct vhost_dev *dev)
>>      return 0;
>>  }
>>
>> +static int vhost_user_migrate(struct vhost_dev *dev, char* mac_addr)
>> +{
>
> Not sure migrate is a good name since migration was not done in this
> function and it was called in vhost_user_receive().
>

OK. I will rename it to vhost_user_notify_migration_done that is the
purpose of this function
diff mbox

Patch

diff --git a/docs/specs/vhost-user.txt b/docs/specs/vhost-user.txt
index c2d2e2a..8c05301 100644
--- a/docs/specs/vhost-user.txt
+++ b/docs/specs/vhost-user.txt
@@ -140,6 +140,7 @@  Protocol features
 -----------------
 
 #define VHOST_USER_PROTOCOL_F_LOG_SHMFD 0
+#define VHOST_USER_PROTOCOL_F_RARP      1
 
 Message types
 -------------
@@ -308,6 +309,20 @@  Message types
       invalid FD flag. This flag is set when there is no file descriptor
       in the ancillary data.
 
+ * VHOST_USER_SEND_RARP
+
+      Id: 17
+      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.
+
 Migration
 ---------
 
diff --git a/hw/net/vhost_net.c b/hw/net/vhost_net.c
index 9850520..c3b9664 100644
--- a/hw/net/vhost_net.c
+++ b/hw/net/vhost_net.c
@@ -383,6 +383,17 @@  void vhost_net_cleanup(struct vhost_net *net)
     g_free(net);
 }
 
+int vhost_net_migrate(struct vhost_net *net, char* mac_addr)
+{
+    const VhostOps *vhost_ops = net->dev.vhost_ops;
+    int r = -1;
+
+    if (vhost_ops->vhost_backend_migrate)
+        r = vhost_ops->vhost_backend_migrate(&net->dev, mac_addr);
+
+    return r;
+}
+
 bool vhost_net_virtqueue_pending(VHostNetState *net, int idx)
 {
     return vhost_virtqueue_pending(&net->dev, idx);
@@ -456,6 +467,11 @@  void vhost_net_virtqueue_mask(VHostNetState *net, VirtIODevice *dev,
 {
 }
 
+int vhost_net_migrate(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 fe75618..693840f 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,8 +28,9 @@ 
 
 #define VHOST_USER_F_PROTOCOL_FEATURES 30
 
-#define VHOST_USER_PROTOCOL_FEATURE_MASK 0x1ULL
+#define VHOST_USER_PROTOCOL_FEATURE_MASK 0x3ULL
 #define VHOST_USER_PROTOCOL_F_LOG_SHMFD 0
+#define VHOST_USER_PROTOCOL_F_RARP      1
 
 typedef enum VhostUserRequest {
     VHOST_USER_NONE = 0,
@@ -48,6 +50,7 @@  typedef enum VhostUserRequest {
     VHOST_USER_SET_VRING_ERR = 14,
     VHOST_USER_GET_PROTOCOL_FEATURES = 15,
     VHOST_USER_SET_PROTOCOL_FEATURES = 16,
+    VHOST_USER_SEND_RARP = 17,
     VHOST_USER_MAX
 } VhostUserRequest;
 
@@ -409,9 +412,34 @@  static int vhost_user_cleanup(struct vhost_dev *dev)
     return 0;
 }
 
+static int vhost_user_migrate(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_call = vhost_user_call,
         .vhost_backend_init = vhost_user_init,
-        .vhost_backend_cleanup = vhost_user_cleanup
+        .vhost_backend_cleanup = vhost_user_cleanup,
+        .vhost_backend_migrate = vhost_user_migrate
         };
diff --git a/include/hw/virtio/vhost-backend.h b/include/hw/virtio/vhost-backend.h
index e472f29..cafc7ec 100644
--- a/include/hw/virtio/vhost-backend.h
+++ b/include/hw/virtio/vhost-backend.h
@@ -24,12 +24,14 @@  typedef int (*vhost_call)(struct vhost_dev *dev, unsigned long int request,
              void *arg);
 typedef int (*vhost_backend_init)(struct vhost_dev *dev, void *opaque);
 typedef int (*vhost_backend_cleanup)(struct vhost_dev *dev);
+typedef int (*vhost_backend_migrate)(struct vhost_dev *dev, char* mac_addr);
 
 typedef struct VhostOps {
     VhostBackendType backend_type;
     vhost_call vhost_call;
     vhost_backend_init vhost_backend_init;
     vhost_backend_cleanup vhost_backend_cleanup;
+    vhost_backend_migrate vhost_backend_migrate;
 } VhostOps;
 
 extern const VhostOps user_ops;
diff --git a/include/net/vhost_net.h b/include/net/vhost_net.h
index 840d4b1..e5700fb 100644
--- a/include/net/vhost_net.h
+++ b/include/net/vhost_net.h
@@ -26,5 +26,6 @@  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_migrate(VHostNetState *net, char* mac_addr);
 VHostNetState *get_vhost_net(NetClientState *nc);
 #endif
diff --git a/net/vhost-user.c b/net/vhost-user.c
index 2290271..3cdfd35 100644
--- a/net/vhost-user.c
+++ b/net/vhost-user.c
@@ -84,6 +84,24 @@  static ssize_t vhost_user_receive(NetClientState *nc, const uint8_t *buf,
             display_trace = 0;
         }
     }
+    else {
+        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_migrate(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;
 }