[Qemu-devel] [PATCH v3 0/3] virtio-net/vhost-net: share cross-endian enablement

[Qemu-devel] [PATCH v3 0/3] virtio-net/vhost-net: share cross-endian enablement

[Greg Kurz]

Since QEMU 2.4.0, vhost-net uses the cross-endian support of TAP devices to
fix vnet headers. In fact, virtio-net can do the same instead of hackily
patching headers in virtio_net_hdr_swap().

This series moves the enablement of cross-endian support from vhost-net to
virtio-net: both vhost and full emulation can now benefit from it. Of course
we keep the current hack to fall back on when the backend doesn't support
cross-endian.

---

Greg Kurz (3):
      virtio-net: use the backend cross-endian capabilities
      Revert "vhost-net: tell tap backend about the vnet endianness"
      virtio: drop the virtio_needs_swap() helper


 hw/net/vhost_net.c                |   33 +------------------------------
 hw/net/virtio-net.c               |   40 +++++++++++++++++++++++++++++++++++--
 include/hw/virtio/virtio-access.h |    9 --------
 include/hw/virtio/virtio-net.h    |    1 +
 4 files changed, 40 insertions(+), 43 deletions(-)

[Qemu-devel] [PATCH v3 1/3] virtio-net: use the backend cross-endian capabilities

[Greg Kurz]

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. If the backend cannot support the
requested endiannes, we have to fall back on virtio_net_hdr_swap(): this
is recorded in the needs_vnet_hdr_swap flag, to be used in the TX and RX
paths.

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

Signed-off-by: Greg Kurz <gkurz@linux.vnet.ibm.com>
---
v3: - dropped extraneous peer_has_vnet_hdr() declaration
    - only rely on the flag to decide if swap is needed
---
 hw/net/virtio-net.c            |   33 +++++++++++++++++++++++++++++++--
 include/hw/virtio/virtio-net.h |    1 +
 2 files changed, 32 insertions(+), 2 deletions(-)

diff --git a/hw/net/virtio-net.c b/hw/net/virtio-net.c
index a877614e3e7a..d4cc94ea5e55 100644
--- a/hw/net/virtio-net.c
+++ b/hw/net/virtio-net.c
@@ -152,6 +152,31 @@ static void virtio_net_vhost_status(VirtIONet *n, uint8_t status)
     }
 }
 
+static void virtio_net_vnet_status(VirtIONet *n, uint8_t status)
+{
+    VirtIODevice *vdev = VIRTIO_DEVICE(n);
+    NetClientState *peer = qemu_get_queue(n->nic)->peer;
+
+    if (virtio_net_started(n, status)) {
+        int r;
+
+        if (virtio_is_big_endian(vdev)) {
+            r = qemu_set_vnet_be(peer, true);
+        } else {
+            r = qemu_set_vnet_le(peer, true);
+        }
+
+        n->needs_vnet_hdr_swap = !!r;
+    } else if (virtio_net_started(n, vdev->status) &&
+               !virtio_net_started(n, status)) {
+        if (virtio_is_big_endian(vdev)) {
+            qemu_set_vnet_be(peer, false);
+        } else {
+            qemu_set_vnet_le(peer, false);
+        }
+    }
+}
+
 static void virtio_net_set_status(struct VirtIODevice *vdev, uint8_t status)
 {
     VirtIONet *n = VIRTIO_NET(vdev);
@@ -159,6 +184,7 @@ static void virtio_net_set_status(struct VirtIODevice *vdev, uint8_t status)
     int i;
     uint8_t queue_status;
 
+    virtio_net_vnet_status(n, status);
     virtio_net_vhost_status(n, status);
 
     for (i = 0; i < n->max_queues; i++) {
@@ -957,7 +983,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 = {
@@ -1167,7 +1196,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-net.h b/include/hw/virtio/virtio-net.h
index f3cc25feca2b..27bc868fbc7d 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,

[Qemu-devel] [PATCH v3 2/3] Revert "vhost-net: tell tap backend about the vnet endianness"

[Greg Kurz]

This reverts commit 5be7d9f1b1452613b95c6ba70b8d7ad3d0797991.

Cross-endian is now configured by the core virtio-net code. We simply
fall back on full emulation if the net backend cannot support the
requested endianness for vnet headers.

Signed-off-by: Greg Kurz <gkurz@linux.vnet.ibm.com>
---
 hw/net/vhost_net.c  |   33 +--------------------------------
 hw/net/virtio-net.c |    7 +++++++
 2 files changed, 8 insertions(+), 32 deletions(-)

diff --git a/hw/net/vhost_net.c b/hw/net/vhost_net.c
index d91b7b155ea7..fd7ea47fd38d 100644
--- a/hw/net/vhost_net.c
+++ b/hw/net/vhost_net.c
@@ -38,7 +38,6 @@
 #include "standard-headers/linux/virtio_ring.h"
 #include "hw/virtio/vhost.h"
 #include "hw/virtio/virtio-bus.h"
-#include "hw/virtio/virtio-access.h"
 
 struct vhost_net {
     struct vhost_dev dev;
@@ -205,27 +204,6 @@ static void vhost_net_set_vq_index(struct vhost_net *net, int vq_index)
     net->dev.vq_index = vq_index;
 }
 
-static int vhost_net_set_vnet_endian(VirtIODevice *dev, NetClientState *peer,
-                                     bool set)
-{
-    int r = 0;
-
-    if (virtio_vdev_has_feature(dev, VIRTIO_F_VERSION_1) ||
-        (virtio_legacy_is_cross_endian(dev) && !virtio_is_big_endian(dev))) {
-        r = qemu_set_vnet_le(peer, set);
-        if (r) {
-            error_report("backend does not support LE vnet headers");
-        }
-    } else if (virtio_legacy_is_cross_endian(dev)) {
-        r = qemu_set_vnet_be(peer, set);
-        if (r) {
-            error_report("backend does not support BE vnet headers");
-        }
-    }
-
-    return r;
-}
-
 static int vhost_net_start_one(struct vhost_net *net,
                                VirtIODevice *dev)
 {
@@ -320,11 +298,6 @@ int vhost_net_start(VirtIODevice *dev, NetClientState *ncs,
         goto err;
     }
 
-    r = vhost_net_set_vnet_endian(dev, ncs[0].peer, true);
-    if (r < 0) {
-        goto err;
-    }
-
     for (i = 0; i < total_queues; i++) {
         vhost_net_set_vq_index(get_vhost_net(ncs[i].peer), i * 2);
     }
@@ -332,7 +305,7 @@ int vhost_net_start(VirtIODevice *dev, NetClientState *ncs,
     r = k->set_guest_notifiers(qbus->parent, total_queues * 2, true);
     if (r < 0) {
         error_report("Error binding guest notifier: %d", -r);
-        goto err_endian;
+        goto err;
     }
 
     for (i = 0; i < total_queues; i++) {
@@ -354,8 +327,6 @@ err_start:
         fprintf(stderr, "vhost guest notifier cleanup failed: %d\n", e);
         fflush(stderr);
     }
-err_endian:
-    vhost_net_set_vnet_endian(dev, ncs[0].peer, false);
 err:
     return r;
 }
@@ -378,8 +349,6 @@ void vhost_net_stop(VirtIODevice *dev, NetClientState *ncs,
         fflush(stderr);
     }
     assert(r >= 0);
-
-    assert(vhost_net_set_vnet_endian(dev, ncs[0].peer, false) >= 0);
 }
 
 void vhost_net_cleanup(struct vhost_net *net)
diff --git a/hw/net/virtio-net.c b/hw/net/virtio-net.c
index d4cc94ea5e55..5a0ab6ad5bb5 100644
--- a/hw/net/virtio-net.c
+++ b/hw/net/virtio-net.c
@@ -128,6 +128,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.
          */

[Qemu-devel] [PATCH v3 3/3] virtio: drop the virtio_needs_swap() helper

[Greg Kurz]

It is not used anymore.

Signed-off-by: Greg Kurz <gkurz@linux.vnet.ibm.com>
---
 include/hw/virtio/virtio-access.h |    9 ---------
 1 file changed, 9 deletions(-)

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

Re: [Qemu-devel] [PATCH v3 0/3] virtio-net/vhost-net: share cross-endian enablement

[Michael S. Tsirkin]

On Wed, Nov 18, 2015 at 05:23:00PM +0100, Greg Kurz wrote:
> Since QEMU 2.4.0, vhost-net uses the cross-endian support of TAP devices to
> fix vnet headers. In fact, virtio-net can do the same instead of hackily
> patching headers in virtio_net_hdr_swap().
> 
> This series moves the enablement of cross-endian support from vhost-net to
> virtio-net: both vhost and full emulation can now benefit from it. Of course
> we keep the current hack to fall back on when the backend doesn't support
> cross-endian.


Thanks!
This is an optimization rather than a bugfix, right?
As such I'd rather defer this until after 2.5.

> ---
> 
> Greg Kurz (3):
>       virtio-net: use the backend cross-endian capabilities
>       Revert "vhost-net: tell tap backend about the vnet endianness"
>       virtio: drop the virtio_needs_swap() helper
> 
> 
>  hw/net/vhost_net.c                |   33 +------------------------------
>  hw/net/virtio-net.c               |   40 +++++++++++++++++++++++++++++++++++--
>  include/hw/virtio/virtio-access.h |    9 --------
>  include/hw/virtio/virtio-net.h    |    1 +
>  4 files changed, 40 insertions(+), 43 deletions(-)

Re: [Qemu-devel] [PATCH v3 0/3] virtio-net/vhost-net: share cross-endian enablement

[Greg Kurz]

On Wed, 18 Nov 2015 22:48:06 +0200
"Michael S. Tsirkin" <mst@redhat.com> wrote:

> On Wed, Nov 18, 2015 at 05:23:00PM +0100, Greg Kurz wrote:
> > Since QEMU 2.4.0, vhost-net uses the cross-endian support of TAP devices to
> > fix vnet headers. In fact, virtio-net can do the same instead of hackily
> > patching headers in virtio_net_hdr_swap().
> > 
> > This series moves the enablement of cross-endian support from vhost-net to
> > virtio-net: both vhost and full emulation can now benefit from it. Of course
> > we keep the current hack to fall back on when the backend doesn't support
> > cross-endian.
> 
> 
> Thanks!
> This is an optimization rather than a bugfix, right?
> As such I'd rather defer this until after 2.5.
> 

Of course. I'll ping or repost later.

> > ---
> > 
> > Greg Kurz (3):
> >       virtio-net: use the backend cross-endian capabilities
> >       Revert "vhost-net: tell tap backend about the vnet endianness"
> >       virtio: drop the virtio_needs_swap() helper
> > 
> > 
> >  hw/net/vhost_net.c                |   33 +------------------------------
> >  hw/net/virtio-net.c               |   40 +++++++++++++++++++++++++++++++++++--
> >  include/hw/virtio/virtio-access.h |    9 --------
> >  include/hw/virtio/virtio-net.h    |    1 +
> >  4 files changed, 40 insertions(+), 43 deletions(-)
> 

Re: [Qemu-devel] [PATCH v3 0/3] virtio-net/vhost-net: share cross-endian enablement

[Greg Kurz]

On Wed, 18 Nov 2015 22:46:55 +0100
Greg Kurz <gkurz@linux.vnet.ibm.com> wrote:

> On Wed, 18 Nov 2015 22:48:06 +0200
> "Michael S. Tsirkin" <mst@redhat.com> wrote:
> 
> > On Wed, Nov 18, 2015 at 05:23:00PM +0100, Greg Kurz wrote:
> > > Since QEMU 2.4.0, vhost-net uses the cross-endian support of TAP devices to
> > > fix vnet headers. In fact, virtio-net can do the same instead of hackily
> > > patching headers in virtio_net_hdr_swap().
> > > 
> > > This series moves the enablement of cross-endian support from vhost-net to
> > > virtio-net: both vhost and full emulation can now benefit from it. Of course
> > > we keep the current hack to fall back on when the backend doesn't support
> > > cross-endian.
> > 
> > 
> > Thanks!
> > This is an optimization rather than a bugfix, right?
> > As such I'd rather defer this until after 2.5.
> > 
> 
> Of course. I'll ping or repost later.
> 

Hi Michael,

2.5 is now released and this series still applies cleanly. Should I repost ?

--
Greg

> > > ---
> > > 
> > > Greg Kurz (3):
> > >       virtio-net: use the backend cross-endian capabilities
> > >       Revert "vhost-net: tell tap backend about the vnet endianness"
> > >       virtio: drop the virtio_needs_swap() helper
> > > 
> > > 
> > >  hw/net/vhost_net.c                |   33 +------------------------------
> > >  hw/net/virtio-net.c               |   40 +++++++++++++++++++++++++++++++++++--
> > >  include/hw/virtio/virtio-access.h |    9 --------
> > >  include/hw/virtio/virtio-net.h    |    1 +
> > >  4 files changed, 40 insertions(+), 43 deletions(-)
> > 
> 
>