[v4,1/6] virtio-net: use the backend cross-endian capabilities

Commit Message

Greg Kurz Feb. 5, 2016, 10:43 a.m. UTC

When running a fully emulated device in cross-endian conditions, including
a virtio 1.0 device offered to a big endian guest, we need to fix the vnet
headers. This is currently handled by the virtio_net_hdr_swap() function
in the core virtio-net code but it should actually be handled by the net
backend.

With this patch, virtio-net now tries to configure the backend to do the
endian fixing when the device starts (i.e. drivers sets the CONFIG_OK bit).
If the backend cannot support the requested endiannes, we have to fallback
onto virtio_net_hdr_swap(): this is recorded in the needs_vnet_hdr_swap flag,
to be used in the TX and RX paths.

Note that we reset the backend to the default behaviour (guest native
endianness) when the device stops (i.e. device status had CONFIG_OK bit and
driver unsets it). This is needed, with the linux tap backend at least,
otherwise the guest may lose network connectivity if rebooted into a
different endianness.

The current vhost-net code also tries to configure net backends. This will
be no more needed and will be reverted in a subsequent patch.

Reviewed-by: Cornelia Huck <[e-mail]>
Reviewed-by: Laurent Vivier <[e-mail]>
Signed-off-by: Greg Kurz <[e-mail]>
---
v4:
  - fix bug with multiqueue
  - rename helper to virtio_net_vnet_endian_status, for better clarity on
    what it is used for
---
 hw/net/virtio-net.c               |   68 ++++++++++++++++++++++++++++++++++++-
 include/hw/virtio/virtio-access.h |    9 -----
 include/hw/virtio/virtio-net.h    |    1 +
 3 files changed, 67 insertions(+), 11 deletions(-)

Comments

Cornelia Huck Feb. 5, 2016, 11:33 a.m. UTC | #1

On Fri, 05 Feb 2016 11:43:11 +0100
Greg Kurz <[e-mail]> wrote:

> +static bool virtio_net_set_vnet_endian(VirtIODevice *vdev, NetClientState *ncs,
> +                                       int queues, bool enable)

You might consider adding a comment like "returns whether endianness
handling needs to fall back to the device" or so, as it's not
immediately obvious what true/false is supposed to mean. But the patch
is fine.

> +{
> +    int i;
> +
> +    for (i = 0; i < queues; i++) {
> +        if (virtio_net_set_vnet_endian_one(vdev, ncs[i].peer, enable) < 0 &&
> +            enable) {
> +            while (--i >= 0) {
> +                virtio_net_set_vnet_endian_one(vdev, ncs[i].peer, false);
> +            }
> +
> +            return true;
> +        }
> +    }
> +
> +    return false;
> +}

Greg Kurz Feb. 5, 2016, 3:41 p.m. UTC | #2

On Fri, 5 Feb 2016 12:33:44 +0100
Cornelia Huck <[e-mail]> wrote:

> On Fri, 05 Feb 2016 11:43:11 +0100
> Greg Kurz <[e-mail]> wrote:
> 
> > +static bool virtio_net_set_vnet_endian(VirtIODevice *vdev, NetClientState *ncs,
> > +                                       int queues, bool enable)  
> 
> You might consider adding a comment like "returns whether endianness
> handling needs to fall back to the device" or so, as it's not
> immediately obvious what true/false is supposed to mean. But the patch
> is fine.
> 

I agree it is not that obvious. If Michael agrees, I'll send a followup
patch when these series has been applied.

Thanks.

--
Greg

> > +{
> > +    int i;
> > +
> > +    for (i = 0; i < queues; i++) {
> > +        if (virtio_net_set_vnet_endian_one(vdev, ncs[i].peer, enable) < 0 &&
> > +            enable) {
> > +            while (--i >= 0) {
> > +                virtio_net_set_vnet_endian_one(vdev, ncs[i].peer, false);
> > +            }
> > +
> > +            return true;
> > +        }
> > +    }
> > +
> > +    return false;
> > +}

Patch

diff --git a/hw/net/virtio-net.c b/hw/net/virtio-net.c
index de696e8dd0d6..5798f87d8ea2 100644
--- a/hw/net/virtio-net.c
+++ b/hw/net/virtio-net.c
@@ -129,6 +129,13 @@  static void virtio_net_vhost_status(VirtIONet *n, uint8_t status)
     if (!n->vhost_started) {
         int r, i;
 
+        if (n->needs_vnet_hdr_swap) {
+            error_report("backend does not support %s vnet headers; "
+                         "falling back on userspace virtio",
+                         virtio_is_big_endian(vdev) ? "BE" : "LE");
+            return;
+        }
+
         /* Any packets outstanding? Purge them to avoid touching rings
          * when vhost is running.
          */
@@ -153,6 +160,59 @@  static void virtio_net_vhost_status(VirtIONet *n, uint8_t status)
     }
 }
 
+static int virtio_net_set_vnet_endian_one(VirtIODevice *vdev,
+                                          NetClientState *peer,
+                                          bool enable)
+{
+    if (virtio_is_big_endian(vdev)) {
+        return qemu_set_vnet_be(peer, enable);
+    } else {
+        return qemu_set_vnet_le(peer, enable);
+    }
+}
+
+static bool virtio_net_set_vnet_endian(VirtIODevice *vdev, NetClientState *ncs,
+                                       int queues, bool enable)
+{
+    int i;
+
+    for (i = 0; i < queues; i++) {
+        if (virtio_net_set_vnet_endian_one(vdev, ncs[i].peer, enable) < 0 &&
+            enable) {
+            while (--i >= 0) {
+                virtio_net_set_vnet_endian_one(vdev, ncs[i].peer, false);
+            }
+
+            return true;
+        }
+    }
+
+    return false;
+}
+
+static void virtio_net_vnet_endian_status(VirtIONet *n, uint8_t status)
+{
+    VirtIODevice *vdev = VIRTIO_DEVICE(n);
+    int queues = n->multiqueue ? n->max_queues : 1;
+
+    if (virtio_net_started(n, status)) {
+        /* Before using the device, we tell the network backend about the
+         * endianness to use when parsing vnet headers. If the backend
+         * can't do it, we fallback onto fixing the headers in the core
+         * virtio-net code.
+         */
+        n->needs_vnet_hdr_swap = virtio_net_set_vnet_endian(vdev, n->nic->ncs,
+                                                            queues, true);
+    } else if (virtio_net_started(n, vdev->status)) {
+        /* After using the device, we need to reset the network backend to
+         * the default (guest native endianness), otherwise the guest may
+         * lose network connectivity if it is rebooted into a different
+         * endianness.
+         */
+        virtio_net_set_vnet_endian(vdev, n->nic->ncs, queues, false);
+    }
+}
+
 static void virtio_net_set_status(struct VirtIODevice *vdev, uint8_t status)
 {
     VirtIONet *n = VIRTIO_NET(vdev);
@@ -160,6 +220,7 @@  static void virtio_net_set_status(struct VirtIODevice *vdev, uint8_t status)
     int i;
     uint8_t queue_status;
 
+    virtio_net_vnet_endian_status(n, status);
     virtio_net_vhost_status(n, status);
 
     for (i = 0; i < n->max_queues; i++) {
@@ -963,7 +1024,10 @@  static void receive_header(VirtIONet *n, const struct iovec *iov, int iov_cnt,
         void *wbuf = (void *)buf;
         work_around_broken_dhclient(wbuf, wbuf + n->host_hdr_len,
                                     size - n->host_hdr_len);
-        virtio_net_hdr_swap(VIRTIO_DEVICE(n), wbuf);
+
+        if (n->needs_vnet_hdr_swap) {
+            virtio_net_hdr_swap(VIRTIO_DEVICE(n), wbuf);
+        }
         iov_from_buf(iov, iov_cnt, 0, buf, sizeof(struct virtio_net_hdr));
     } else {
         struct virtio_net_hdr hdr = {
@@ -1184,7 +1248,7 @@  static int32_t virtio_net_flush_tx(VirtIONetQueue *q)
                 error_report("virtio-net header incorrect");
                 exit(1);
             }
-            if (virtio_needs_swap(vdev)) {
+            if (n->needs_vnet_hdr_swap) {
                 virtio_net_hdr_swap(vdev, (void *) &mhdr);
                 sg2[0].iov_base = &mhdr;
                 sg2[0].iov_len = n->guest_hdr_len;
diff --git a/include/hw/virtio/virtio-access.h b/include/hw/virtio/virtio-access.h
index 8aec843c8ff3..a01fff2e51d7 100644
--- a/include/hw/virtio/virtio-access.h
+++ b/include/hw/virtio/virtio-access.h
@@ -143,15 +143,6 @@  static inline uint64_t virtio_ldq_p(VirtIODevice *vdev, const void *ptr)
     }
 }
 
-static inline bool virtio_needs_swap(VirtIODevice *vdev)
-{
-#ifdef HOST_WORDS_BIGENDIAN
-    return virtio_access_is_big_endian(vdev) ? false : true;
-#else
-    return virtio_access_is_big_endian(vdev) ? true : false;
-#endif
-}
-
 static inline uint16_t virtio_tswap16(VirtIODevice *vdev, uint16_t s)
 {
 #ifdef HOST_WORDS_BIGENDIAN
diff --git a/include/hw/virtio/virtio-net.h b/include/hw/virtio/virtio-net.h
index 2ce3b03bd448..0cabdb682241 100644
--- a/include/hw/virtio/virtio-net.h
+++ b/include/hw/virtio/virtio-net.h
@@ -94,6 +94,7 @@  typedef struct VirtIONet {
     uint64_t curr_guest_offloads;
     QEMUTimer *announce_timer;
     int announce_counter;
+    bool needs_vnet_hdr_swap;
 } VirtIONet;
 
 void virtio_net_set_netclient_name(VirtIONet *n, const char *name,