DPDK patches and discussions
 help / color / mirror / Atom feed
From: Yuanhan Liu <yuanhan.liu@linux.intel.com>
To: Santosh Shukla <sshukla@mvista.com>
Cc: dev@dpdk.org
Subject: Re: [dpdk-dev] [PATCH v4 01/14] virtio: Introduce config RTE_VIRTIO_INC_VECTOR
Date: Fri, 15 Jan 2016 14:51:07 +0800	[thread overview]
Message-ID: <20160115065107.GV19531@yliu-dev.sh.intel.com> (raw)
In-Reply-To: <1452778117-30178-2-git-send-email-sshukla@mvista.com>

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 <sshukla@mvista.com>
> ---
>  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

  reply	other threads:[~2016-01-15  6:49 UTC|newest]

Thread overview: 36+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-01-14 13:28 [dpdk-dev] [PATCH v4 00/14] Add virtio support for arm/arm64 Santosh Shukla
2016-01-14 13:28 ` [dpdk-dev] [PATCH v4 01/14] virtio: Introduce config RTE_VIRTIO_INC_VECTOR Santosh Shukla
2016-01-15  6:51   ` Yuanhan Liu [this message]
2016-01-16  6:18     ` Santosh Shukla
2016-01-14 13:28 ` [dpdk-dev] [PATCH v4 02/14] config: i686: set RTE_VIRTIO_INC_VECTOR=n Santosh Shukla
2016-01-14 13:28 ` [dpdk-dev] [PATCH v4 03/14] linuxapp: eal: arm: Always return 0 for rte_eal_iopl_init() Santosh Shukla
2016-01-14 13:28 ` [dpdk-dev] [PATCH v4 04/14] linuxapp/vfio: ignore mapping for ioport region Santosh Shukla
2016-01-14 13:28 ` [dpdk-dev] [PATCH v4 05/14] virtio_pci.h: build fix for sys/io.h for non-x86 arch Santosh Shukla
2016-01-14 13:28 ` [dpdk-dev] [PATCH v4 06/14] eal: pci: vfio: add rd/wr func for pci bar space Santosh Shukla
2016-01-15  5:48   ` Yuanhan Liu
2016-01-16  8:06     ` Santosh Shukla
2016-01-14 13:28 ` [dpdk-dev] [PATCH v4 07/14] virtio: vfio: add api support to rd/wr ioport bar Santosh Shukla
2016-01-15  6:03   ` Yuanhan Liu
2016-01-16  8:53     ` Santosh Shukla
2016-01-14 13:28 ` [dpdk-dev] [PATCH v4 08/14] virtio: pci: extend virtio pci rw api for vfio interface Santosh Shukla
2016-01-15  6:27   ` Yuanhan Liu
2016-01-15 12:43     ` Santosh Shukla
2016-01-15 13:42       ` Santosh Shukla
2016-01-18  6:11         ` Yuanhan Liu
2016-01-18  6:45           ` Santosh Shukla
2016-01-18  7:17             ` Yuanhan Liu
2016-01-18 13:09               ` Santosh Shukla
2016-01-18  6:59   ` Jason Wang
2016-01-18  7:39     ` Santosh Shukla
2016-01-14 13:28 ` [dpdk-dev] [PATCH v4 09/14] virtio: ethdev: check " Santosh Shukla
2016-01-15  6:35   ` Yuanhan Liu
2016-01-15 12:37     ` Santosh Shukla
2016-01-14 13:28 ` [dpdk-dev] [PATCH v4 10/14] virtio: pci: add dummy func definition for in/outb for non-x86 arch Santosh Shukla
2016-01-14 13:28 ` [dpdk-dev] [PATCH v4 11/14] config: armv7/v8: Enable RTE_LIBRTE_VIRTIO_PMD Santosh Shukla
2016-01-15  6:37   ` Yuanhan Liu
2016-01-15 12:45     ` Santosh Shukla
2016-01-14 13:28 ` [dpdk-dev] [PATCH v4 12/14] eal: pci: export pci_[un]map_device Santosh Shukla
2016-01-14 13:28 ` [dpdk-dev] [PATCH v4 13/14] virtio: enable vfio in pmd driver Santosh Shukla
2016-01-14 13:28 ` [dpdk-dev] [PATCH v4 14/14] vfio: Support for no-IOMMU mode Santosh Shukla
2016-01-29  7:27 ` [dpdk-dev] [PATCH v4 00/14] Add virtio support for arm/arm64 Xie, Huawei
2016-01-29  9:19   ` Santosh Shukla

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20160115065107.GV19531@yliu-dev.sh.intel.com \
    --to=yuanhan.liu@linux.intel.com \
    --cc=dev@dpdk.org \
    --cc=sshukla@mvista.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).