| Message ID | 20150617132349.6560.80324.stgit@bahia.lab.toulouse-stg.fr.ibm.com |
|---|---|
| State | New |
| Headers | show |
Hi, On Wed, 17 Jun 2015 15:23:49 +0200 Greg Kurz <gkurz@linux.vnet.ibm.com> wrote: > The default behaviour for TAP/MACVTAP is to consider vnet as native endian. > > This patch handles the cases when this is not true: > - virtio 1.0: always little-endian > - legacy cross-endian > > Signed-off-by: Greg Kurz <gkurz@linux.vnet.ibm.com> > --- > hw/net/vhost_net.c | 33 ++++++++++++++++++++++++++++++++- > 1 file changed, 32 insertions(+), 1 deletion(-) > > diff --git a/hw/net/vhost_net.c b/hw/net/vhost_net.c > index 1c55517e3611..8cbb2f618c1c 100644 > --- a/hw/net/vhost_net.c > +++ b/hw/net/vhost_net.c ... > @@ -365,6 +394,8 @@ 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); > } Putting the vhost_net_set_vnet_endian() within the assert statement looks somewhat wrong to me. assert() gets defined to nothing in case NDEBUG is defined, so the call would then simply be dropped. I guess you rather want something like this here: r = vhost_net_set_vnet_endian(dev, ncs[0].peer, false); assert(r >= 0); ? Thomas
On Fri, 19 Jun 2015 11:16:35 +0200 Thomas Huth <thuth@redhat.com> wrote: > > Hi, > > On Wed, 17 Jun 2015 15:23:49 +0200 > Greg Kurz <gkurz@linux.vnet.ibm.com> wrote: > > > The default behaviour for TAP/MACVTAP is to consider vnet as native endian. > > > > This patch handles the cases when this is not true: > > - virtio 1.0: always little-endian > > - legacy cross-endian > > > > Signed-off-by: Greg Kurz <gkurz@linux.vnet.ibm.com> > > --- > > hw/net/vhost_net.c | 33 ++++++++++++++++++++++++++++++++- > > 1 file changed, 32 insertions(+), 1 deletion(-) > > > > diff --git a/hw/net/vhost_net.c b/hw/net/vhost_net.c > > index 1c55517e3611..8cbb2f618c1c 100644 > > --- a/hw/net/vhost_net.c > > +++ b/hw/net/vhost_net.c > ... > > @@ -365,6 +394,8 @@ 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); > > } > > Putting the vhost_net_set_vnet_endian() within the assert statement > looks somewhat wrong to me. assert() gets defined to nothing in case > NDEBUG is defined, so the call would then simply be dropped. > I guess you rather want something like this here: > > r = vhost_net_set_vnet_endian(dev, ncs[0].peer, false); > assert(r >= 0); > > ? > > Thomas > Oops you're right... I'll send a fix. -- Greg
diff --git a/hw/net/vhost_net.c b/hw/net/vhost_net.c index 1c55517e3611..8cbb2f618c1c 100644 --- a/hw/net/vhost_net.c +++ b/hw/net/vhost_net.c @@ -38,6 +38,7 @@ #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; @@ -197,6 +198,27 @@ 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_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) { @@ -314,6 +336,11 @@ 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); } @@ -321,7 +348,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; + goto err_endian; } for (i = 0; i < total_queues; i++) { @@ -343,6 +370,8 @@ 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; } @@ -365,6 +394,8 @@ 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)
The default behaviour for TAP/MACVTAP is to consider vnet as native endian. This patch handles the cases when this is not true: - virtio 1.0: always little-endian - legacy cross-endian Signed-off-by: Greg Kurz <gkurz@linux.vnet.ibm.com> --- hw/net/vhost_net.c | 33 ++++++++++++++++++++++++++++++++- 1 file changed, 32 insertions(+), 1 deletion(-)