From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mga02.intel.com (mga02.intel.com [134.134.136.20]) by dpdk.org (Postfix) with ESMTP id 23A19594B for ; Fri, 15 Jan 2016 07:49:42 +0100 (CET) Received: from fmsmga004.fm.intel.com ([10.253.24.48]) by orsmga101.jf.intel.com with ESMTP; 14 Jan 2016 22:49:42 -0800 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.22,298,1449561600"; d="scan'208";a="29761187" Received: from yliu-dev.sh.intel.com (HELO yliu-dev) ([10.239.66.49]) by fmsmga004.fm.intel.com with ESMTP; 14 Jan 2016 22:49:40 -0800 Date: Fri, 15 Jan 2016 14:51:07 +0800 From: Yuanhan Liu To: Santosh Shukla Message-ID: <20160115065107.GV19531@yliu-dev.sh.intel.com> References: <1452778117-30178-1-git-send-email-sshukla@mvista.com> <1452778117-30178-2-git-send-email-sshukla@mvista.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <1452778117-30178-2-git-send-email-sshukla@mvista.com> User-Agent: Mutt/1.5.23 (2014-03-12) Cc: dev@dpdk.org Subject: Re: [dpdk-dev] [PATCH v4 01/14] virtio: Introduce config RTE_VIRTIO_INC_VECTOR X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: patches and discussions about DPDK List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Fri, 15 Jan 2016 06:49:43 -0000 On Thu, Jan 14, 2016 at 06:58:24PM +0530, Santosh Shukla wrote: > virtio_recv_pkts_vec and other virtio vector friend apis are written for sse/avx > instructions. For arm64 in particular, virtio vector implementation does not > exist(todo). > > So virtio pmd driver wont build for targets like i686, arm64. By making > RTE_VIRTIO_INC_VECTOR=n, Driver can build for non-sse/avx targets and will work > in non-vectored virtio mode. While revisiting this patch, I'm thinking you may squash both patch 2 and patch 11 into this one. > > Signed-off-by: Santosh Shukla > --- > config/common_linuxapp | 1 + > drivers/net/virtio/Makefile | 2 +- > drivers/net/virtio/virtio_rxtx.c | 7 +++++++ > 3 files changed, 9 insertions(+), 1 deletion(-) > > diff --git a/config/common_linuxapp b/config/common_linuxapp > index 74bc515..8677697 100644 > --- a/config/common_linuxapp > +++ b/config/common_linuxapp > @@ -274,6 +274,7 @@ CONFIG_RTE_LIBRTE_VIRTIO_DEBUG_RX=n > CONFIG_RTE_LIBRTE_VIRTIO_DEBUG_TX=n > CONFIG_RTE_LIBRTE_VIRTIO_DEBUG_DRIVER=n > CONFIG_RTE_LIBRTE_VIRTIO_DEBUG_DUMP=n > +CONFIG_RTE_VIRTIO_INC_VECTOR=y > > # > # Compile burst-oriented VMXNET3 PMD driver > diff --git a/drivers/net/virtio/Makefile b/drivers/net/virtio/Makefile > index 43835ba..25a842d 100644 > --- a/drivers/net/virtio/Makefile > +++ b/drivers/net/virtio/Makefile > @@ -50,7 +50,7 @@ SRCS-$(CONFIG_RTE_LIBRTE_VIRTIO_PMD) += virtqueue.c > SRCS-$(CONFIG_RTE_LIBRTE_VIRTIO_PMD) += virtio_pci.c > SRCS-$(CONFIG_RTE_LIBRTE_VIRTIO_PMD) += virtio_rxtx.c > SRCS-$(CONFIG_RTE_LIBRTE_VIRTIO_PMD) += virtio_ethdev.c > -SRCS-$(CONFIG_RTE_LIBRTE_VIRTIO_PMD) += virtio_rxtx_simple.c > +SRCS-$(CONFIG_RTE_VIRTIO_INC_VECTOR) += virtio_rxtx_simple.c > > # this lib depends upon: > DEPDIRS-$(CONFIG_RTE_LIBRTE_VIRTIO_PMD) += lib/librte_eal lib/librte_ether > diff --git a/drivers/net/virtio/virtio_rxtx.c b/drivers/net/virtio/virtio_rxtx.c > index 74b39ef..23be1ff 100644 > --- a/drivers/net/virtio/virtio_rxtx.c > +++ b/drivers/net/virtio/virtio_rxtx.c > @@ -438,7 +438,9 @@ virtio_dev_rx_queue_setup(struct rte_eth_dev *dev, > > dev->data->rx_queues[queue_idx] = vq; > > +#ifdef RTE_VIRTIO_INC_VECTOR > virtio_rxq_vec_setup(vq); > +#endif You should put such macros for the declaration in virtio_rxtx.h as well. And note that you may miss one: 325 /****************************************** 326 * Enqueue allocated buffers * 327 *******************************************/ 328 if (use_simple_rxtx) ==> 329 error = virtqueue_enqueue_recv_refill_simple(vq, m); 330 else 331 error = virtqueue_enqueue_recv_refill(vq, m); 332 if (error) { 333 rte_pktmbuf_free(m); 334 break; 335 } virtqueue_enqueue_recv_refill_simple() is defined inside virtio_rxtx_simple.c, which is built only when CONFIG_RTE_VIRTIO_INC_VECTOR is set. But I see no such check here. Note that this will not break the build, as gcc just ignores it, for use_simple_rxtx is 0 by default, thus the "if" part code is not compiled at all. Even for that, I think it's better to put an explicit macro check here. --yliu