On Wed, Jul 15, 2015 at 03:56:07PM +0800, Jason Wang wrote: > Commit 032a74a1c0fcdd5fd1c69e56126b4c857ee36611 > ("virtio-net: byteswap virtio-net header") breaks any layout by > requiring out_sg[0].iov_len >= n->guest_hdr_len. Fixing this by > copying header to temporary buffer if swap is needed, and then use > this buffer as part of out_sg. > > Fixes 032a74a1c0fcdd5fd1c69e56126b4c857ee36611 > ("virtio-net: byteswap virtio-net header") > Cc: qemu-sta...@nongnu.org > Cc: c...@fr.ibm.com > Signed-off-by: Jason Wang <jasow...@redhat.com> > --- > Changes from V1: > - avoid header copying if there's no need to do header swap > - don't write the header back > --- > hw/net/virtio-net.c | 17 ++++++++++++++--- > include/hw/virtio/virtio-access.h | 9 +++++++++ > 2 files changed, 23 insertions(+), 3 deletions(-) > > diff --git a/hw/net/virtio-net.c b/hw/net/virtio-net.c > index e3c2db3..12322bd 100644 > --- a/hw/net/virtio-net.c > +++ b/hw/net/virtio-net.c > @@ -1142,7 +1142,8 @@ static int32_t virtio_net_flush_tx(VirtIONetQueue *q) > ssize_t ret, len; > unsigned int out_num = elem.out_num; > struct iovec *out_sg = &elem.out_sg[0]; > - struct iovec sg[VIRTQUEUE_MAX_SIZE]; > + struct iovec sg[VIRTQUEUE_MAX_SIZE], sg2[VIRTQUEUE_MAX_SIZE]; > + struct virtio_net_hdr hdr; > > if (out_num < 1) { > error_report("virtio-net header not in first element"); > @@ -1150,11 +1151,21 @@ static int32_t virtio_net_flush_tx(VirtIONetQueue *q) > } > > if (n->has_vnet_hdr) { > - if (out_sg[0].iov_len < n->guest_hdr_len) { > + if (iov_size(out_sg, out_num) < n->guest_hdr_len) { > error_report("virtio-net header incorrect"); > exit(1); > }
this scans the iov unnecessarily. How about checking return code from iov_to_buf instead? > - virtio_net_hdr_swap(vdev, (void *) out_sg[0].iov_base); > + if (virtio_needs_swap(vdev)) { > + iov_to_buf(out_sg, out_num, 0, &hdr, sizeof(hdr)); > + virtio_net_hdr_swap(vdev, (void *) &hdr); > + sg2[0].iov_base = &hdr; > + sg2[0].iov_len = sizeof(hdr); > + out_num = iov_copy(&sg2[1], ARRAY_SIZE(sg2) - 1, > + out_sg, out_num, > + sizeof(hdr), -1); This might truncate packet if it does not fit in 1024 anymore. It might be better to just drop it. > + out_num += 1; > + out_sg = sg2; > + } > } > > /* > diff --git a/include/hw/virtio/virtio-access.h > b/include/hw/virtio/virtio-access.h > index cee5dd7..1ec1dfd 100644 > --- a/include/hw/virtio/virtio-access.h > +++ b/include/hw/virtio/virtio-access.h > @@ -143,6 +143,15 @@ 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 > -- > 2.1.4