From mboxrd@z Thu Jan 1 00:00:00 1970 From: "Michael S. Tsirkin" Subject: Re: [RFC PATCH 4/4] vhost_net: byteswap virtio_net header Date: Mon, 3 Nov 2014 18:25:25 +0200 Message-ID: <20141103162525.GC24877@redhat.com> References: <1414571925-16918-1-git-send-email-clg@fr.ibm.com> <1414571925-16918-5-git-send-email-clg@fr.ibm.com> Mime-Version: 1.0 Content-Type: text/plain; charset=iso-8859-1 Content-Transfer-Encoding: QUOTED-PRINTABLE Cc: kvm@vger.kernel.org, kvm-ppc@vger.kernel.org, agraf@suse.de, paulus@samba.org, gkurz@linux.vnet.ibm.com.patch, aik@ozlabs.ru To: =?iso-8859-1?Q?C=E9dric?= Le Goater Return-path: Received: from mx1.redhat.com ([209.132.183.28]:43881 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752023AbaKCQZo (ORCPT ); Mon, 3 Nov 2014 11:25:44 -0500 Content-Disposition: inline In-Reply-To: <1414571925-16918-5-git-send-email-clg@fr.ibm.com> Sender: kvm-owner@vger.kernel.org List-ID: On Wed, Oct 29, 2014 at 09:38:45AM +0100, C=E9dric Le Goater wrote: > Signed-off-by: C=E9dric Le Goater This patch casts userspace pointers to void *, this is generally unsafe. In particular sparse will warn. > --- > drivers/vhost/net.c | 39 ++++++++++++++++++++++++++++++--------- > 1 file changed, 30 insertions(+), 9 deletions(-) >=20 > diff --git a/drivers/vhost/net.c b/drivers/vhost/net.c > index 8dae2f724a35..f2d5f585dae9 100644 > --- a/drivers/vhost/net.c > +++ b/drivers/vhost/net.c > @@ -18,6 +18,7 @@ > #include > #include > #include > +#include > =20 > #include > #include > @@ -329,6 +330,14 @@ static void vhost_zerocopy_callback(struct ubuf_= info *ubuf, bool success) > rcu_read_unlock_bh(); > } > =20 > +static void virtio_net_hdr_swap(struct virtio_net_hdr *hdr) > +{ > + hdr->hdr_len =3D swab16(hdr->hdr_len); > + hdr->gso_size =3D swab16(hdr->gso_size); > + hdr->csum_start =3D swab16(hdr->csum_start); > + hdr->csum_offset =3D swab16(hdr->csum_offset); > +} > + > /* Expects to be always run from workqueue - which acts as > * read-size critical section for our kind of RCU. */ > static void handle_tx(struct vhost_net *net) > @@ -346,7 +355,7 @@ static void handle_tx(struct vhost_net *net) > .msg_flags =3D MSG_DONTWAIT, > }; > size_t len, total_len =3D 0; > - int err; > + int err, has_vnet_hdr; > size_t hdr_size; > struct socket *sock; > struct vhost_net_ubuf_ref *uninitialized_var(ubufs); > @@ -359,6 +368,7 @@ static void handle_tx(struct vhost_net *net) > =20 > vhost_disable_notify(&net->dev, vq); > =20 > + has_vnet_hdr =3D vhost_has_feature(vq, VHOST_NET_F_VIRTIO_NET_HDR); > hdr_size =3D nvq->vhost_hlen; > zcopy =3D nvq->ubufs; > =20 > @@ -406,6 +416,8 @@ static void handle_tx(struct vhost_net *net) > break; > } > =20 > + if (!has_vnet_hdr && vq->byteswap) > + virtio_net_hdr_swap((void *) vq->iov[0].iov_base); > zcopy_used =3D zcopy && len >=3D VHOST_GOODCOPY_LEN > && (nvq->upend_idx + 1) % UIO_MAXIOV !=3D > nvq->done_idx > @@ -570,7 +582,7 @@ static void handle_rx(struct vhost_net *net) > .hdr.gso_type =3D VIRTIO_NET_HDR_GSO_NONE > }; > size_t total_len =3D 0; > - int err, mergeable; > + int err, mergeable, has_vnet_hdr; > s16 headcount; > size_t vhost_hlen, sock_hlen; > size_t vhost_len, sock_len; > @@ -588,6 +600,7 @@ static void handle_rx(struct vhost_net *net) > vq_log =3D unlikely(vhost_has_feature(vq, VHOST_F_LOG_ALL)) ? > vq->log : NULL; > mergeable =3D vhost_has_feature(vq, VIRTIO_NET_F_MRG_RXBUF); > + has_vnet_hdr =3D vhost_has_feature(vq, VHOST_NET_F_VIRTIO_NET_HDR); > =20 > while ((sock_len =3D peek_head_len(sock->sk))) { > sock_len +=3D sock_hlen; > @@ -638,6 +651,9 @@ static void handle_rx(struct vhost_net *net) > vhost_discard_vq_desc(vq, headcount); > continue; > } > + > + if (!has_vnet_hdr && vq->byteswap) > + virtio_net_hdr_swap((void *) vq->iov[0].iov_base); > if (unlikely(vhost_hlen) && > memcpy_toiovecend(nvq->hdr, (unsigned char *)&hdr, 0, > vhost_hlen)) { > @@ -646,13 +662,18 @@ static void handle_rx(struct vhost_net *net) > break; > } > /* TODO: Should check and handle checksum. */ > - if (likely(mergeable) && > - memcpy_toiovecend(nvq->hdr, (unsigned char *)&headcount, > - offsetof(typeof(hdr), num_buffers), > - sizeof hdr.num_buffers)) { > - vq_err(vq, "Failed num_buffers write"); > - vhost_discard_vq_desc(vq, headcount); > - break; > + if (likely(mergeable)) { > + __u16 tmp =3D headcount; > + > + if (vq->byteswap) > + tmp =3D swab16(headcount); > + if (memcpy_toiovecend(nvq->hdr, (unsigned char *)&tmp, > + offsetof(typeof(hdr), num_buffers), > + sizeof(hdr.num_buffers))) { > + vq_err(vq, "Failed num_buffers write"); > + vhost_discard_vq_desc(vq, headcount); > + break; > + } > } > vhost_add_used_and_signal_n(&net->dev, vq, vq->heads, > headcount); > --=20 > 1.7.10.4