From patchwork Tue Jan 22 14:38:14 2013 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Amos Kong X-Patchwork-Id: 214567 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Received: from lists.gnu.org (lists.gnu.org [208.118.235.17]) (using TLSv1 with cipher AES256-SHA (256/256 bits)) (Client did not present a certificate) by ozlabs.org (Postfix) with ESMTPS id EF6112C007E for ; Wed, 23 Jan 2013 01:39:12 +1100 (EST) Received: from localhost ([::1]:58736 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Txf0I-0005QN-VG for incoming@patchwork.ozlabs.org; Tue, 22 Jan 2013 09:39:10 -0500 Received: from eggs.gnu.org ([208.118.235.92]:55488) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Txf07-0005Pa-1c for qemu-devel@nongnu.org; Tue, 22 Jan 2013 09:39:05 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1Txf05-0003Wr-LJ for qemu-devel@nongnu.org; Tue, 22 Jan 2013 09:38:58 -0500 Received: from mx1.redhat.com ([209.132.183.28]:40766) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Txf05-0003Wk-Ck for qemu-devel@nongnu.org; Tue, 22 Jan 2013 09:38:57 -0500 Received: from int-mx02.intmail.prod.int.phx2.redhat.com (int-mx02.intmail.prod.int.phx2.redhat.com [10.5.11.12]) by mx1.redhat.com (8.14.4/8.14.4) with ESMTP id r0MEcs8H028590 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=OK); Tue, 22 Jan 2013 09:38:55 -0500 Received: from localhost (vpn1-114-71.nay.redhat.com [10.66.114.71]) by int-mx02.intmail.prod.int.phx2.redhat.com (8.13.8/8.13.8) with ESMTP id r0MEcqiM001630; Tue, 22 Jan 2013 09:38:53 -0500 Date: Tue, 22 Jan 2013 22:38:14 +0800 From: Amos Kong To: Stefan Hajnoczi Message-ID: <20130122143814.GB4066@t430s.nay.redhat.com> References: <1358560468-10865-1-git-send-email-akong@redhat.com> <1358560468-10865-2-git-send-email-akong@redhat.com> <20130121160330.GC24473@stefanha-thinkpad.redhat.com> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <20130121160330.GC24473@stefanha-thinkpad.redhat.com> User-Agent: Mutt/1.5.21 (2010-09-15) X-Scanned-By: MIMEDefang 2.67 on 10.5.11.12 X-detected-operating-system: by eggs.gnu.org: GNU/Linux 3.x X-Received-From: 209.132.183.28 Cc: virtualization@lists.linux-foundation.org, stefanha@redhat.com, qemu-devel@nongnu.org, kvm@vger.kernel.org, mst@redhat.com Subject: Re: [Qemu-devel] [QEMU PATCH v4 1/3] virtio-net: remove layout assumptions for ctrl vq X-BeenThere: qemu-devel@nongnu.org X-Mailman-Version: 2.1.14 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org Sender: qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org On Mon, Jan 21, 2013 at 05:03:30PM +0100, Stefan Hajnoczi wrote: > On Sat, Jan 19, 2013 at 09:54:26AM +0800, akong@redhat.com wrote: > > From: "Michael S. Tsirkin" > > > > Virtio-net code makes assumption about virtqueue descriptor layout > > (e.g. sg[0] is the header, sg[1] is the data buffer). > > > > This patch makes code not rely on the layout of descriptors. > > > > Signed-off-by: Michael S. Tsirkin > > Signed-off-by: Amos Kong > > --- > > hw/virtio-net.c | 128 ++++++++++++++++++++++++++++++++------------------------ > > 1 file changed, 74 insertions(+), 54 deletions(-) > > > > diff --git a/hw/virtio-net.c b/hw/virtio-net.c > > index 3bb01b1..113e194 100644 > > --- a/hw/virtio-net.c > > +++ b/hw/virtio-net.c > > @@ -315,44 +315,44 @@ static void virtio_net_set_features(VirtIODevice *vdev, uint32_t features) > > } > > > > static int virtio_net_handle_rx_mode(VirtIONet *n, uint8_t cmd, > > - VirtQueueElement *elem) > > + struct iovec *iov, unsigned int iov_cnt) > > { > > uint8_t on; > > + size_t s; > > > > - if (elem->out_num != 2 || elem->out_sg[1].iov_len != sizeof(on)) { > > - error_report("virtio-net ctrl invalid rx mode command"); > > - exit(1); > > + s = iov_to_buf(iov, iov_cnt, 0, &on, sizeof(on)); > > + if (s != sizeof(on)) { > > + return VIRTIO_NET_ERR; > > } > > > > - on = ldub_p(elem->out_sg[1].iov_base); > > - > > - if (cmd == VIRTIO_NET_CTRL_RX_MODE_PROMISC) > > + if (cmd == VIRTIO_NET_CTRL_RX_MODE_PROMISC) { > > n->promisc = on; > > - else if (cmd == VIRTIO_NET_CTRL_RX_MODE_ALLMULTI) > > + } else if (cmd == VIRTIO_NET_CTRL_RX_MODE_ALLMULTI) { > > n->allmulti = on; > > - else if (cmd == VIRTIO_NET_CTRL_RX_MODE_ALLUNI) > > + } else if (cmd == VIRTIO_NET_CTRL_RX_MODE_ALLUNI) { > > n->alluni = on; > > - else if (cmd == VIRTIO_NET_CTRL_RX_MODE_NOMULTI) > > + } else if (cmd == VIRTIO_NET_CTRL_RX_MODE_NOMULTI) { > > n->nomulti = on; > > - else if (cmd == VIRTIO_NET_CTRL_RX_MODE_NOUNI) > > + } else if (cmd == VIRTIO_NET_CTRL_RX_MODE_NOUNI) { > > n->nouni = on; > > - else if (cmd == VIRTIO_NET_CTRL_RX_MODE_NOBCAST) > > + } else if (cmd == VIRTIO_NET_CTRL_RX_MODE_NOBCAST) { > > n->nobcast = on; > > - else > > + } else { > > return VIRTIO_NET_ERR; > > + } > > > > return VIRTIO_NET_OK; > > } > > > > static int virtio_net_handle_mac(VirtIONet *n, uint8_t cmd, > > - VirtQueueElement *elem) > > + struct iovec *iov, unsigned int iov_cnt) > > { > > struct virtio_net_ctrl_mac mac_data; > > + size_t s; > > > > - if (cmd != VIRTIO_NET_CTRL_MAC_TABLE_SET || elem->out_num != 3 || > > - elem->out_sg[1].iov_len < sizeof(mac_data) || > > - elem->out_sg[2].iov_len < sizeof(mac_data)) > > + if (cmd != VIRTIO_NET_CTRL_MAC_TABLE_SET) { > > return VIRTIO_NET_ERR; > > + } > > > > n->mac_table.in_use = 0; > > n->mac_table.first_multi = 0; > > @@ -360,54 +360,71 @@ static int virtio_net_handle_mac(VirtIONet *n, uint8_t cmd, > > n->mac_table.multi_overflow = 0; > > memset(n->mac_table.macs, 0, MAC_TABLE_ENTRIES * ETH_ALEN); > > > > - mac_data.entries = ldl_p(elem->out_sg[1].iov_base); > > + s = iov_to_buf(iov, iov_cnt, 0, &mac_data.entries, > > + sizeof(mac_data.entries)); Hi Stefan, can we adjust the endianness after each iov_to_buf() copy? diff --git a/hw/virtio-net.c b/hw/virtio-net.c index 72d7857..0088d6c 100644 --- a/hw/virtio-net.c +++ b/hw/virtio-net.c @@ -321,6 +321,7 @@ static int virtio_net_handle_rx_mode(VirtIONet *n, uint8_t cmd, size_t s; s = iov_to_buf(iov, iov_cnt, 0, &on, sizeof(on)); + on = ldub_p(&on); if (s != sizeof(on)) { return VIRTIO_NET_ERR; } @@ -362,7 +363,7 @@ static int virtio_net_handle_mac(VirtIONet *n, uint8_t cmd, s = iov_to_buf(iov, iov_cnt, 0, &mac_data.entries, sizeof(mac_data.entries)); - + mac_data.entries = ldl_p(&mac_data.entries); if (s != sizeof(mac_data.entries)) { return VIRTIO_NET_ERR; } @@ -389,7 +390,7 @@ static int virtio_net_handle_mac(VirtIONet *n, uint8_t cmd, s = iov_to_buf(iov, iov_cnt, 0, &mac_data.entries, sizeof(mac_data.entries)); - + mac_data.entries = ldl_p(&mac_data.entries); if (s != sizeof(mac_data.entries)) { return VIRTIO_NET_ERR; } @@ -421,6 +422,7 @@ static int virtio_net_handle_vlan_table(VirtIONet *n, uint8_t cmd, size_t s; s = iov_to_buf(iov, iov_cnt, 0, &vid, sizeof(vid)); + vid = lduw_p(&vid); if (s != sizeof(vid)) { return VIRTIO_NET_ERR; } @@ -458,6 +460,8 @@ static void virtio_net_handle_ctrl(VirtIODevice *vdev, VirtQueue *vq) iov = elem.out_sg; iov_cnt = elem.out_num; s = iov_to_buf(iov, iov_cnt, 0, &ctrl, sizeof(ctrl)); + ctrl.class = ldub_p(&ctrl.class); + ctrl.cmd = ldub_p(&ctrl.class + sizeof(ctrl.class)); iov_discard_front(&iov, &iov_cnt, sizeof(ctrl)); if (s != sizeof(ctrl)) { status = VIRTIO_NET_ERR;