On Wed, 14 Oct 2015 12:26:58 +0300 "Michael S. Tsirkin" <m...@redhat.com> wrote:
> commit 5be7d9f1b1452613b95c6ba70b8d7ad3d0797991 > vhost-net: tell tap backend about the vnet endianness > makes vhost net always try to set LE - even if that matches the > native endian-ness. > > This makes it fail on older kernels on x86 without TUNSETVNETLE support. > Since qemu_set_vnet_le() is only called from vhost_net_set_vnet_endian(): 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"); } and virtio_legacy_is_cross_endian() is { return false; } on x86, I guess this is about virtio 1.0, right ? > To fix, make qemu_set_vnet_le/qemu_set_vnet_be skip the > ioctl if it matches the host endian-ness. > The qemu_set_vnet_le() change indeed makes sense since it fixes a bug. I am not so sure about qemu_set_vnet_be() since it is only called in the case we have a ppc64le host and a legacy ppc64 guest, in which case HOST_WORDS_BIGENDIAN is not defined... IMHO it is better to rework the logic so that we only call qemu_set_vnet_* when it is needed. There are only 3 cases actually: - BE host with virtio 1 needs vnet_le - BE host with legacy LE needs vnet_le - LE host with legacy BE needs vnet_be Something like: static inline bool vhost_net_needs_vnet_le(VirtIODevice *dev) { #if defined(HOST_WORDS_BIGENDIAN) return virtio_vdev_has_feature(dev, VIRTIO_F_VERSION_1) || !virtio_is_big_endian(dev); #else return false; #endif } static inline bool vhost_net_needs_vnet_be(VirtIODevice *dev) { #if defined(HOST_WORDS_BIGENDIAN) return false; #else return virtio_is_big_endian(dev); #endif } static int vhost_net_set_vnet_endian(VirtIODevice *dev, NetClientState *peer, bool set) { int r = 0; if (vhost_net_needs_vnet_le(dev)) { r = qemu_set_vnet_le(peer, set); if (r) { error_report("backend does not support LE vnet headers"); } } else if (vhost_net_needs_vnet_be(dev)) { r = qemu_set_vnet_be(peer, set); if (r) { error_report("backend does not support BE vnet headers"); } } return r; } Cheers. -- Greg > Reported-by: Marcel Apfelbaum <mar...@redhat.com> > Cc: Greg Kurz <gk...@linux.vnet.ibm.com> > Cc: qemu-sta...@nongnu.org > Signed-off-by: Michael S. Tsirkin <m...@redhat.com> > --- > net/net.c | 8 ++++++++ > 1 file changed, 8 insertions(+) > > diff --git a/net/net.c b/net/net.c > index 28a5597..8e96011 100644 > --- a/net/net.c > +++ b/net/net.c > @@ -517,20 +517,28 @@ void qemu_set_vnet_hdr_len(NetClientState *nc, int len) > > int qemu_set_vnet_le(NetClientState *nc, bool is_le) > { > +#ifdef HOST_WORDS_BIGENDIAN > if (!nc || !nc->info->set_vnet_le) { > return -ENOSYS; > } > > return nc->info->set_vnet_le(nc, is_le); > +#else > + return 0; > +#endif > } > > int qemu_set_vnet_be(NetClientState *nc, bool is_be) > { > +#ifdef HOST_WORDS_BIGENDIAN > + return 0; > +#else > if (!nc || !nc->info->set_vnet_be) { > return -ENOSYS; > } > > return nc->info->set_vnet_be(nc, is_be); > +#endif > } > > int qemu_can_send_packet(NetClientState *sender)