[dpdk-dev] [PATCH v1] virtio: Use cpuflag for vector api

[dpdk-dev] [PATCH v1] virtio: Use cpuflag for vector api

[Santosh Shukla]

Check cpuflag macro before using vectored api.
-virtio_recv_pkts_vec() uses _sse3__ simd instruction for now so added cpuflag.
- Also wrap other vectored freind api ie..
1) virtqueue_enqueue_recv_refill_simple
2) virtio_rxq_vec_setup

todo:
1) Move virtio_recv_pkts_vec() implementation to
   drivers/virtio/virtio_vec_<arch>.h file.
2) Remove use_simple_rxtx flag, so that virtio/virtio_vec_<arch>.h
   files to provide vectored/non-vectored rx/tx apis.

Signed-off-by: Santosh Shukla <sshukla@mvista.com>
---
- v1: This is a rework of patch [1].
Note: This patch will let non-x86 arch to use virtio pmd.

[1] http://dpdk.org/dev/patchwork/patch/10429/

 drivers/net/virtio/virtio_rxtx.c        |   16 +++++++++++++++-
 drivers/net/virtio/virtio_rxtx.h        |    2 ++
 drivers/net/virtio/virtio_rxtx_simple.c |   11 ++++++++++-
 3 files changed, 27 insertions(+), 2 deletions(-)

diff --git a/drivers/net/virtio/virtio_rxtx.c b/drivers/net/virtio/virtio_rxtx.c
index 41a1366..ec0b8de 100644
--- a/drivers/net/virtio/virtio_rxtx.c
+++ b/drivers/net/virtio/virtio_rxtx.c
@@ -67,7 +67,9 @@
 #define VIRTIO_SIMPLE_FLAGS ((uint32_t)ETH_TXQ_FLAGS_NOMULTSEGS | \
 	ETH_TXQ_FLAGS_NOOFFLOADS)
 
+#ifdef RTE_MACHINE_CPUFLAG_SSSE3
 static int use_simple_rxtx;
+#endif
 
 static void
 vq_ring_free_chain(struct virtqueue *vq, uint16_t desc_idx)
@@ -307,12 +309,13 @@ virtio_dev_vring_start(struct virtqueue *vq, int queue_type)
 		nbufs = 0;
 		error = ENOSPC;
 
+#ifdef RTE_MACHINE_CPUFLAG_SSSE3
 		if (use_simple_rxtx)
 			for (i = 0; i < vq->vq_nentries; i++) {
 				vq->vq_ring.avail->ring[i] = i;
 				vq->vq_ring.desc[i].flags = VRING_DESC_F_WRITE;
 			}
-
+#endif
 		memset(&vq->fake_mbuf, 0, sizeof(vq->fake_mbuf));
 		for (i = 0; i < RTE_PMD_VIRTIO_RX_MAX_BURST; i++)
 			vq->sw_ring[vq->vq_nentries + i] = &vq->fake_mbuf;
@@ -325,9 +328,11 @@ virtio_dev_vring_start(struct virtqueue *vq, int queue_type)
 			/******************************************
 			*         Enqueue allocated buffers        *
 			*******************************************/
+#ifdef RTE_MACHINE_CPUFLAG_SSSE3
 			if (use_simple_rxtx)
 				error = virtqueue_enqueue_recv_refill_simple(vq, m);
 			else
+#endif
 				error = virtqueue_enqueue_recv_refill(vq, m);
 			if (error) {
 				rte_pktmbuf_free(m);
@@ -340,6 +345,7 @@ virtio_dev_vring_start(struct virtqueue *vq, int queue_type)
 
 		PMD_INIT_LOG(DEBUG, "Allocated %d bufs", nbufs);
 	} else if (queue_type == VTNET_TQ) {
+#ifdef RTE_MACHINE_CPUFLAG_SSSE3
 		if (use_simple_rxtx) {
 			int mid_idx  = vq->vq_nentries >> 1;
 			for (i = 0; i < mid_idx; i++) {
@@ -357,6 +363,7 @@ virtio_dev_vring_start(struct virtqueue *vq, int queue_type)
 			for (i = mid_idx; i < vq->vq_nentries; i++)
 				vq->vq_ring.avail->ring[i] = i;
 		}
+#endif
 	}
 }
 
@@ -423,7 +430,9 @@ virtio_dev_rx_queue_setup(struct rte_eth_dev *dev,
 
 	dev->data->rx_queues[queue_idx] = vq;
 
+#ifdef RTE_MACHINE_CPUFLAG_SSSE3
 	virtio_rxq_vec_setup(vq);
+#endif
 
 	return 0;
 }
@@ -449,7 +458,10 @@ virtio_dev_tx_queue_setup(struct rte_eth_dev *dev,
 			const struct rte_eth_txconf *tx_conf)
 {
 	uint8_t vtpci_queue_idx = 2 * queue_idx + VTNET_SQ_TQ_QUEUE_IDX;
+
+#ifdef RTE_MACHINE_CPUFLAG_SSSE3
 	struct virtio_hw *hw = dev->data->dev_private;
+#endif
 	struct virtqueue *vq;
 	uint16_t tx_free_thresh;
 	int ret;
@@ -462,6 +474,7 @@ virtio_dev_tx_queue_setup(struct rte_eth_dev *dev,
 		return -EINVAL;
 	}
 
+#ifdef RTE_MACHINE_CPUFLAG_SSSE3
 	/* Use simple rx/tx func if single segment and no offloads */
 	if ((tx_conf->txq_flags & VIRTIO_SIMPLE_FLAGS) == VIRTIO_SIMPLE_FLAGS &&
 	     !vtpci_with_feature(hw, VIRTIO_NET_F_MRG_RXBUF)) {
@@ -470,6 +483,7 @@ virtio_dev_tx_queue_setup(struct rte_eth_dev *dev,
 		dev->rx_pkt_burst = virtio_recv_pkts_vec;
 		use_simple_rxtx = 1;
 	}
+#endif
 
 	ret = virtio_dev_queue_setup(dev, VTNET_TQ, queue_idx, vtpci_queue_idx,
 			nb_desc, socket_id, &vq);
diff --git a/drivers/net/virtio/virtio_rxtx.h b/drivers/net/virtio/virtio_rxtx.h
index 831e492..a76c3e5 100644
--- a/drivers/net/virtio/virtio_rxtx.h
+++ b/drivers/net/virtio/virtio_rxtx.h
@@ -33,7 +33,9 @@
 
 #define RTE_PMD_VIRTIO_RX_MAX_BURST 64
 
+#ifdef RTE_MACHINE_CPUFLAG_SSSE3
 int virtio_rxq_vec_setup(struct virtqueue *rxq);
 
 int virtqueue_enqueue_recv_refill_simple(struct virtqueue *vq,
 	struct rte_mbuf *m);
+#endif
diff --git a/drivers/net/virtio/virtio_rxtx_simple.c b/drivers/net/virtio/virtio_rxtx_simple.c
index 3a1de9d..be51d7c 100644
--- a/drivers/net/virtio/virtio_rxtx_simple.c
+++ b/drivers/net/virtio/virtio_rxtx_simple.c
@@ -37,7 +37,9 @@
 #include <string.h>
 #include <errno.h>
 
-#include <tmmintrin.h>
+#ifdef __SSE3__
+#include <rte_vect.h>
+#endif
 
 #include <rte_cycles.h>
 #include <rte_memory.h>
@@ -66,6 +68,7 @@
 #pragma GCC diagnostic ignored "-Wcast-qual"
 #endif
 
+#ifdef RTE_MACHINE_CPUFLAG_SSSE3
 int __attribute__((cold))
 virtqueue_enqueue_recv_refill_simple(struct virtqueue *vq,
 	struct rte_mbuf *cookie)
@@ -90,6 +93,7 @@ virtqueue_enqueue_recv_refill_simple(struct virtqueue *vq,
 
 	return 0;
 }
+#endif
 
 static inline void
 virtio_rxq_rearm_vec(struct virtqueue *rxvq)
@@ -130,6 +134,7 @@ virtio_rxq_rearm_vec(struct virtqueue *rxvq)
 	vq_update_avail_idx(rxvq);
 }
 
+#ifdef RTE_MACHINE_CPUFLAG_SSSE3
 /* virtio vPMD receive routine, only accept(nb_pkts >= RTE_VIRTIO_DESC_PER_LOOP)
  *
  * This routine is for non-mergeable RX, one desc for each guest buffer.
@@ -291,6 +296,7 @@ virtio_recv_pkts_vec(void *rx_queue, struct rte_mbuf **rx_pkts,
 	rxvq->packets += nb_pkts_received;
 	return nb_pkts_received;
 }
+#endif
 
 #define VIRTIO_TX_FREE_THRESH 32
 #define VIRTIO_TX_MAX_FREE_BUF_SZ 32
@@ -398,6 +404,7 @@ virtio_xmit_pkts_simple(void *tx_queue, struct rte_mbuf **tx_pkts,
 	return nb_pkts;
 }
 
+#ifdef RTE_MACHINE_CPUFLAG_SSSE3
 int __attribute__((cold))
 virtio_rxq_vec_setup(struct virtqueue *rxq)
 {
@@ -416,3 +423,5 @@ virtio_rxq_vec_setup(struct virtqueue *rxq)
 
 	return 0;
 }
+#endif
+
-- 
1.7.9.5

Re: [dpdk-dev] [PATCH v1] virtio: Use cpuflag for vector api

[Yuanhan Liu]

On Fri, Feb 26, 2016 at 02:21:02PM +0530, Santosh Shukla wrote:
> Check cpuflag macro before using vectored api.
> -virtio_recv_pkts_vec() uses _sse3__ simd instruction for now so added cpuflag.
> - Also wrap other vectored freind api ie..
> 1) virtqueue_enqueue_recv_refill_simple
> 2) virtio_rxq_vec_setup
> 
...
> diff --git a/drivers/net/virtio/virtio_rxtx_simple.c b/drivers/net/virtio/virtio_rxtx_simple.c
> index 3a1de9d..be51d7c 100644
> --- a/drivers/net/virtio/virtio_rxtx_simple.c
> +++ b/drivers/net/virtio/virtio_rxtx_simple.c

Hmm, why not wrapping the whole file, instead of just few functions?

Or maybe better, do a compile time check at the Makefile, something
like:

    if has_CPUFLAG_xxx
        SRCS-$(CONFIG_RTE_LIBRTE_VIRTIO_PMD) += virtio_rxtx_simple.c
    endif


	--yliu

Re: [dpdk-dev] [PATCH v1] virtio: Use cpuflag for vector api

[Xie, Huawei]

On 2/29/2016 12:26 PM, Yuanhan Liu wrote:
> On Fri, Feb 26, 2016 at 02:21:02PM +0530, Santosh Shukla wrote:
>> Check cpuflag macro before using vectored api.
>> -virtio_recv_pkts_vec() uses _sse3__ simd instruction for now so added cpuflag.
>> - Also wrap other vectored freind api ie..
>> 1) virtqueue_enqueue_recv_refill_simple
>> 2) virtio_rxq_vec_setup
>>
> ...
>> diff --git a/drivers/net/virtio/virtio_rxtx_simple.c b/drivers/net/virtio/virtio_rxtx_simple.c
>> index 3a1de9d..be51d7c 100644
>> --- a/drivers/net/virtio/virtio_rxtx_simple.c
>> +++ b/drivers/net/virtio/virtio_rxtx_simple.c
> Hmm, why not wrapping the whole file, instead of just few functions?
>
> Or maybe better, do a compile time check at the Makefile, something
> like:
>
>     if has_CPUFLAG_xxx
>         SRCS-$(CONFIG_RTE_LIBRTE_VIRTIO_PMD) += virtio_rxtx_simple.c
>     endif
>
>
> 	--yliu
>
For next release, we could consider providing arch level framework for
different arch optimizations. It is more complicated for rte_memcpy. For
the time being, except the small issue, ok with the temporary solution
using CPUFLAG.

Re: [dpdk-dev] [PATCH v1] virtio: Use cpuflag for vector api

[Santosh Shukla]

On Mon, Feb 29, 2016 at 9:57 AM, Yuanhan Liu
<yuanhan.liu@linux.intel.com> wrote:
> On Fri, Feb 26, 2016 at 02:21:02PM +0530, Santosh Shukla wrote:
>> Check cpuflag macro before using vectored api.
>> -virtio_recv_pkts_vec() uses _sse3__ simd instruction for now so added cpuflag.
>> - Also wrap other vectored freind api ie..
>> 1) virtqueue_enqueue_recv_refill_simple
>> 2) virtio_rxq_vec_setup
>>
> ...
>> diff --git a/drivers/net/virtio/virtio_rxtx_simple.c b/drivers/net/virtio/virtio_rxtx_simple.c
>> index 3a1de9d..be51d7c 100644
>> --- a/drivers/net/virtio/virtio_rxtx_simple.c
>> +++ b/drivers/net/virtio/virtio_rxtx_simple.c
>
> Hmm, why not wrapping the whole file, instead of just few functions?
>

Better to refactor code and make arch specific. Current implementation
is temporary.
> Or maybe better, do a compile time check at the Makefile, something
> like:
>
>     if has_CPUFLAG_xxx
>         SRCS-$(CONFIG_RTE_LIBRTE_VIRTIO_PMD) += virtio_rxtx_simple.c
>     endif
>
Tried this approach but end up with link error,  If I try to fix below
link error then I will be ending up writing similar code,
linker error snap:

/work/santosh/thunder/nfs/dpdk/arm64-thunderx-linuxapp-gcc/lib/librte_pmd_virtio.a(virtio_rxtx.o):
In function `virtio_dev_rxtx_start':
virtio_rxtx.c:(.text+0x168c): undefined reference to
`virtqueue_enqueue_recv_refill_simple'
/work/santosh/thunder/nfs/dpdk/arm64-thunderx-linuxapp-gcc/lib/librte_pmd_virtio.a(virtio_rxtx.o):
In function `virtio_dev_rx_queue_setup':
virtio_rxtx.c:(.text+0x2364): undefined reference to `virtio_rxq_vec_setup'
/work/santosh/thunder/nfs/dpdk/arm64-thunderx-linuxapp-gcc/lib/librte_pmd_virtio.a(virtio_rxtx.o):
In function `virtio_dev_tx_queue_setup':
virtio_rxtx.c:(.text+0x2460): undefined reference to `virtio_xmit_pkts_simple'
virtio_rxtx.c:(.text+0x2464): undefined reference to `virtio_recv_pkts_vec'
virtio_rxtx.c:(.text+0x2468): undefined reference to `virtio_xmit_pkts_simple'
virtio_rxtx.c:(.text+0x246c): undefined reference to `virtio_recv_pkts_vec'
collect2: error: ld returned 1 exit status
make[5]: *** [test] Error 1
make[4]: *** [test] Error 2
make[3]: *** [app] Error 2

>
>         --yliu

[dpdk-dev] [PATCH v2] virtio: Use cpuflag for vector api

[Santosh Shukla]

Check cpuflag macro before using vectored api.
-virtio_recv_pkts_vec() uses _sse3__ simd instruction for now so added cpuflag.
- Also wrap other vectored freind api ie..
1) virtqueue_enqueue_recv_refill_simple
2) virtio_rxq_vec_setup

- removed VIRTIO_PMD=n from armv7/v8 config.

todo:
1) Move virtio_recv_pkts_vec() implementation to
   drivers/virtio/virtio_vec_<arch>.h file.
2) Remove use_simple_rxtx flag, so that virtio/virtio_vec_<arch>.h
   files to provide vectored/non-vectored rx/tx apis.

Signed-off-by: Santosh Shukla <sshukla@mvista.com>
---
- v2: Removed VIRTIO_PMD=n from arm v7/v8

- v1: This is a rework of patch [1].
Note: This patch will let non-x86 arch to use virtio pmd.

[1] http://dpdk.org/dev/patchwork/patch/10429/


 config/defconfig_arm-armv7a-linuxapp-gcc   |    1 -
 config/defconfig_arm64-armv8a-linuxapp-gcc |    1 -
 drivers/net/virtio/virtio_rxtx.c           |   16 +++++++++++++++-
 drivers/net/virtio/virtio_rxtx.h           |    2 ++
 drivers/net/virtio/virtio_rxtx_simple.c    |   11 ++++++++++-
 5 files changed, 27 insertions(+), 4 deletions(-)

diff --git a/config/defconfig_arm-armv7a-linuxapp-gcc b/config/defconfig_arm-armv7a-linuxapp-gcc
index cbebd64..4bfdfad 100644
--- a/config/defconfig_arm-armv7a-linuxapp-gcc
+++ b/config/defconfig_arm-armv7a-linuxapp-gcc
@@ -70,7 +70,6 @@ CONFIG_RTE_LIBRTE_I40E_PMD=n
 CONFIG_RTE_LIBRTE_IXGBE_PMD=n
 CONFIG_RTE_LIBRTE_MLX4_PMD=n
 CONFIG_RTE_LIBRTE_MPIPE_PMD=n
-CONFIG_RTE_LIBRTE_VIRTIO_PMD=n
 CONFIG_RTE_LIBRTE_VMXNET3_PMD=n
 CONFIG_RTE_LIBRTE_PMD_XENVIRT=n
 CONFIG_RTE_LIBRTE_PMD_BNX2X=n
diff --git a/config/defconfig_arm64-armv8a-linuxapp-gcc b/config/defconfig_arm64-armv8a-linuxapp-gcc
index eacd01c..f6f5d18 100644
--- a/config/defconfig_arm64-armv8a-linuxapp-gcc
+++ b/config/defconfig_arm64-armv8a-linuxapp-gcc
@@ -44,7 +44,6 @@ CONFIG_RTE_TOOLCHAIN="gcc"
 CONFIG_RTE_TOOLCHAIN_GCC=y
 
 CONFIG_RTE_IXGBE_INC_VECTOR=n
-CONFIG_RTE_LIBRTE_VIRTIO_PMD=n
 CONFIG_RTE_LIBRTE_IVSHMEM=n
 CONFIG_RTE_LIBRTE_FM10K_PMD=n
 CONFIG_RTE_LIBRTE_I40E_PMD=n
diff --git a/drivers/net/virtio/virtio_rxtx.c b/drivers/net/virtio/virtio_rxtx.c
index 41a1366..ec0b8de 100644
--- a/drivers/net/virtio/virtio_rxtx.c
+++ b/drivers/net/virtio/virtio_rxtx.c
@@ -67,7 +67,9 @@
 #define VIRTIO_SIMPLE_FLAGS ((uint32_t)ETH_TXQ_FLAGS_NOMULTSEGS | \
 	ETH_TXQ_FLAGS_NOOFFLOADS)
 
+#ifdef RTE_MACHINE_CPUFLAG_SSSE3
 static int use_simple_rxtx;
+#endif
 
 static void
 vq_ring_free_chain(struct virtqueue *vq, uint16_t desc_idx)
@@ -307,12 +309,13 @@ virtio_dev_vring_start(struct virtqueue *vq, int queue_type)
 		nbufs = 0;
 		error = ENOSPC;
 
+#ifdef RTE_MACHINE_CPUFLAG_SSSE3
 		if (use_simple_rxtx)
 			for (i = 0; i < vq->vq_nentries; i++) {
 				vq->vq_ring.avail->ring[i] = i;
 				vq->vq_ring.desc[i].flags = VRING_DESC_F_WRITE;
 			}
-
+#endif
 		memset(&vq->fake_mbuf, 0, sizeof(vq->fake_mbuf));
 		for (i = 0; i < RTE_PMD_VIRTIO_RX_MAX_BURST; i++)
 			vq->sw_ring[vq->vq_nentries + i] = &vq->fake_mbuf;
@@ -325,9 +328,11 @@ virtio_dev_vring_start(struct virtqueue *vq, int queue_type)
 			/******************************************
 			*         Enqueue allocated buffers        *
 			*******************************************/
+#ifdef RTE_MACHINE_CPUFLAG_SSSE3
 			if (use_simple_rxtx)
 				error = virtqueue_enqueue_recv_refill_simple(vq, m);
 			else
+#endif
 				error = virtqueue_enqueue_recv_refill(vq, m);
 			if (error) {
 				rte_pktmbuf_free(m);
@@ -340,6 +345,7 @@ virtio_dev_vring_start(struct virtqueue *vq, int queue_type)
 
 		PMD_INIT_LOG(DEBUG, "Allocated %d bufs", nbufs);
 	} else if (queue_type == VTNET_TQ) {
+#ifdef RTE_MACHINE_CPUFLAG_SSSE3
 		if (use_simple_rxtx) {
 			int mid_idx  = vq->vq_nentries >> 1;
 			for (i = 0; i < mid_idx; i++) {
@@ -357,6 +363,7 @@ virtio_dev_vring_start(struct virtqueue *vq, int queue_type)
 			for (i = mid_idx; i < vq->vq_nentries; i++)
 				vq->vq_ring.avail->ring[i] = i;
 		}
+#endif
 	}
 }
 
@@ -423,7 +430,9 @@ virtio_dev_rx_queue_setup(struct rte_eth_dev *dev,
 
 	dev->data->rx_queues[queue_idx] = vq;
 
+#ifdef RTE_MACHINE_CPUFLAG_SSSE3
 	virtio_rxq_vec_setup(vq);
+#endif
 
 	return 0;
 }
@@ -449,7 +458,10 @@ virtio_dev_tx_queue_setup(struct rte_eth_dev *dev,
 			const struct rte_eth_txconf *tx_conf)
 {
 	uint8_t vtpci_queue_idx = 2 * queue_idx + VTNET_SQ_TQ_QUEUE_IDX;
+
+#ifdef RTE_MACHINE_CPUFLAG_SSSE3
 	struct virtio_hw *hw = dev->data->dev_private;
+#endif
 	struct virtqueue *vq;
 	uint16_t tx_free_thresh;
 	int ret;
@@ -462,6 +474,7 @@ virtio_dev_tx_queue_setup(struct rte_eth_dev *dev,
 		return -EINVAL;
 	}
 
+#ifdef RTE_MACHINE_CPUFLAG_SSSE3
 	/* Use simple rx/tx func if single segment and no offloads */
 	if ((tx_conf->txq_flags & VIRTIO_SIMPLE_FLAGS) == VIRTIO_SIMPLE_FLAGS &&
 	     !vtpci_with_feature(hw, VIRTIO_NET_F_MRG_RXBUF)) {
@@ -470,6 +483,7 @@ virtio_dev_tx_queue_setup(struct rte_eth_dev *dev,
 		dev->rx_pkt_burst = virtio_recv_pkts_vec;
 		use_simple_rxtx = 1;
 	}
+#endif
 
 	ret = virtio_dev_queue_setup(dev, VTNET_TQ, queue_idx, vtpci_queue_idx,
 			nb_desc, socket_id, &vq);
diff --git a/drivers/net/virtio/virtio_rxtx.h b/drivers/net/virtio/virtio_rxtx.h
index 831e492..a76c3e5 100644
--- a/drivers/net/virtio/virtio_rxtx.h
+++ b/drivers/net/virtio/virtio_rxtx.h
@@ -33,7 +33,9 @@
 
 #define RTE_PMD_VIRTIO_RX_MAX_BURST 64
 
+#ifdef RTE_MACHINE_CPUFLAG_SSSE3
 int virtio_rxq_vec_setup(struct virtqueue *rxq);
 
 int virtqueue_enqueue_recv_refill_simple(struct virtqueue *vq,
 	struct rte_mbuf *m);
+#endif
diff --git a/drivers/net/virtio/virtio_rxtx_simple.c b/drivers/net/virtio/virtio_rxtx_simple.c
index 3a1de9d..be51d7c 100644
--- a/drivers/net/virtio/virtio_rxtx_simple.c
+++ b/drivers/net/virtio/virtio_rxtx_simple.c
@@ -37,7 +37,9 @@
 #include <string.h>
 #include <errno.h>
 
-#include <tmmintrin.h>
+#ifdef __SSE3__
+#include <rte_vect.h>
+#endif
 
 #include <rte_cycles.h>
 #include <rte_memory.h>
@@ -66,6 +68,7 @@
 #pragma GCC diagnostic ignored "-Wcast-qual"
 #endif
 
+#ifdef RTE_MACHINE_CPUFLAG_SSSE3
 int __attribute__((cold))
 virtqueue_enqueue_recv_refill_simple(struct virtqueue *vq,
 	struct rte_mbuf *cookie)
@@ -90,6 +93,7 @@ virtqueue_enqueue_recv_refill_simple(struct virtqueue *vq,
 
 	return 0;
 }
+#endif
 
 static inline void
 virtio_rxq_rearm_vec(struct virtqueue *rxvq)
@@ -130,6 +134,7 @@ virtio_rxq_rearm_vec(struct virtqueue *rxvq)
 	vq_update_avail_idx(rxvq);
 }
 
+#ifdef RTE_MACHINE_CPUFLAG_SSSE3
 /* virtio vPMD receive routine, only accept(nb_pkts >= RTE_VIRTIO_DESC_PER_LOOP)
  *
  * This routine is for non-mergeable RX, one desc for each guest buffer.
@@ -291,6 +296,7 @@ virtio_recv_pkts_vec(void *rx_queue, struct rte_mbuf **rx_pkts,
 	rxvq->packets += nb_pkts_received;
 	return nb_pkts_received;
 }
+#endif
 
 #define VIRTIO_TX_FREE_THRESH 32
 #define VIRTIO_TX_MAX_FREE_BUF_SZ 32
@@ -398,6 +404,7 @@ virtio_xmit_pkts_simple(void *tx_queue, struct rte_mbuf **tx_pkts,
 	return nb_pkts;
 }
 
+#ifdef RTE_MACHINE_CPUFLAG_SSSE3
 int __attribute__((cold))
 virtio_rxq_vec_setup(struct virtqueue *rxq)
 {
@@ -416,3 +423,5 @@ virtio_rxq_vec_setup(struct virtqueue *rxq)
 
 	return 0;
 }
+#endif
+
-- 
1.7.9.5

Re: [dpdk-dev] [PATCH v1] virtio: Use cpuflag for vector api

[Yuanhan Liu]

On Mon, Feb 29, 2016 at 06:01:38PM +0530, Santosh Shukla wrote:
> On Mon, Feb 29, 2016 at 9:57 AM, Yuanhan Liu
> <yuanhan.liu@linux.intel.com> wrote:
> > On Fri, Feb 26, 2016 at 02:21:02PM +0530, Santosh Shukla wrote:
> >> Check cpuflag macro before using vectored api.
> >> -virtio_recv_pkts_vec() uses _sse3__ simd instruction for now so added cpuflag.
> >> - Also wrap other vectored freind api ie..
> >> 1) virtqueue_enqueue_recv_refill_simple
> >> 2) virtio_rxq_vec_setup
> >>
> > ...
> >> diff --git a/drivers/net/virtio/virtio_rxtx_simple.c b/drivers/net/virtio/virtio_rxtx_simple.c
> >> index 3a1de9d..be51d7c 100644
> >> --- a/drivers/net/virtio/virtio_rxtx_simple.c
> >> +++ b/drivers/net/virtio/virtio_rxtx_simple.c
> >
> > Hmm, why not wrapping the whole file, instead of just few functions?
> >
> 
> Better to refactor code and make arch specific. Current implementation
> is temporary.

I'm okay to refactor, if it's a clean one. But moving virtio __driver__
stuff to EAL, sorry, it still doesn't make too much sense to me.

For this case, let's make it simple so far, and consider it when we
have another vec implementation, or better refactor comes out.

> > Or maybe better, do a compile time check at the Makefile, something
> > like:
> >
> >     if has_CPUFLAG_xxx
> >         SRCS-$(CONFIG_RTE_LIBRTE_VIRTIO_PMD) += virtio_rxtx_simple.c
> >     endif
> >
> Tried this approach but end up with link error,  If I try to fix below
> link error then I will be ending up writing similar code,

Sure you need the first part of your patch. Yes, it's similar code,
but it has fewer "#ifdef " conditions, and more importantly, it
leaves virtio_rxtx_simple.c alone.

	--yliu

> linker error snap:
> 
> /work/santosh/thunder/nfs/dpdk/arm64-thunderx-linuxapp-gcc/lib/librte_pmd_virtio.a(virtio_rxtx.o):
> In function `virtio_dev_rxtx_start':
> virtio_rxtx.c:(.text+0x168c): undefined reference to
> `virtqueue_enqueue_recv_refill_simple'
> /work/santosh/thunder/nfs/dpdk/arm64-thunderx-linuxapp-gcc/lib/librte_pmd_virtio.a(virtio_rxtx.o):
> In function `virtio_dev_rx_queue_setup':
> virtio_rxtx.c:(.text+0x2364): undefined reference to `virtio_rxq_vec_setup'
> /work/santosh/thunder/nfs/dpdk/arm64-thunderx-linuxapp-gcc/lib/librte_pmd_virtio.a(virtio_rxtx.o):
> In function `virtio_dev_tx_queue_setup':
> virtio_rxtx.c:(.text+0x2460): undefined reference to `virtio_xmit_pkts_simple'
> virtio_rxtx.c:(.text+0x2464): undefined reference to `virtio_recv_pkts_vec'
> virtio_rxtx.c:(.text+0x2468): undefined reference to `virtio_xmit_pkts_simple'
> virtio_rxtx.c:(.text+0x246c): undefined reference to `virtio_recv_pkts_vec'

Re: [dpdk-dev] [PATCH v2] virtio: Use cpuflag for vector api

[Yuanhan Liu]

On Mon, Feb 29, 2016 at 06:28:10PM +0530, Santosh Shukla wrote:
> Check cpuflag macro before using vectored api.
> -virtio_recv_pkts_vec() uses _sse3__ simd instruction for now so added cpuflag.
> - Also wrap other vectored freind api ie..
> 1) virtqueue_enqueue_recv_refill_simple
> 2) virtio_rxq_vec_setup
> 
> - removed VIRTIO_PMD=n from armv7/v8 config.
> 
> todo:
> 1) Move virtio_recv_pkts_vec() implementation to
>    drivers/virtio/virtio_vec_<arch>.h file.
> 2) Remove use_simple_rxtx flag, so that virtio/virtio_vec_<arch>.h
>    files to provide vectored/non-vectored rx/tx apis.
> 
> Signed-off-by: Santosh Shukla <sshukla@mvista.com>
> ---
> - v2: Removed VIRTIO_PMD=n from arm v7/v8

Firstly, I would not suggest you to send another new version, while there
still was discussions ongoing on old version.

And, you should not mix the ARM stuff here; this patch should only do
what the patch title tells. In generic, don't do two or more things in
one patch.

	--yliu

Re: [dpdk-dev] [PATCH v2] virtio: Use cpuflag for vector api

[Santosh Shukla]

On Tue, Mar 1, 2016 at 11:29 AM, Yuanhan Liu
<yuanhan.liu@linux.intel.com> wrote:
> On Mon, Feb 29, 2016 at 06:28:10PM +0530, Santosh Shukla wrote:
>> Check cpuflag macro before using vectored api.
>> -virtio_recv_pkts_vec() uses _sse3__ simd instruction for now so added cpuflag.
>> - Also wrap other vectored freind api ie..
>> 1) virtqueue_enqueue_recv_refill_simple
>> 2) virtio_rxq_vec_setup
>>
>> - removed VIRTIO_PMD=n from armv7/v8 config.
>>
>> todo:
>> 1) Move virtio_recv_pkts_vec() implementation to
>>    drivers/virtio/virtio_vec_<arch>.h file.
>> 2) Remove use_simple_rxtx flag, so that virtio/virtio_vec_<arch>.h
>>    files to provide vectored/non-vectored rx/tx apis.
>>
>> Signed-off-by: Santosh Shukla <sshukla@mvista.com>
>> ---
>> - v2: Removed VIRTIO_PMD=n from arm v7/v8
>
> Firstly, I would not suggest you to send another new version, while there
> still was discussions ongoing on old version.
>
> And, you should not mix the ARM stuff here; this patch should only do
> what the patch title tells. In generic, don't do two or more things in
> one patch.
>

w/o v2 patch, old version wont build for armv7/v8. Clubbing both in
v2, inspired from v7 virtio INC_VEC review comment/feedback [1].

[1] http://dpdk.org/dev/patchwork/patch/10429/

>         --yliu

Re: [dpdk-dev] [PATCH v1] virtio: Use cpuflag for vector api

[Santosh Shukla]

On Tue, Mar 1, 2016 at 11:25 AM, Yuanhan Liu
<yuanhan.liu@linux.intel.com> wrote:
> On Mon, Feb 29, 2016 at 06:01:38PM +0530, Santosh Shukla wrote:
>> On Mon, Feb 29, 2016 at 9:57 AM, Yuanhan Liu
>> <yuanhan.liu@linux.intel.com> wrote:
>> > On Fri, Feb 26, 2016 at 02:21:02PM +0530, Santosh Shukla wrote:
>> >> Check cpuflag macro before using vectored api.
>> >> -virtio_recv_pkts_vec() uses _sse3__ simd instruction for now so added cpuflag.
>> >> - Also wrap other vectored freind api ie..
>> >> 1) virtqueue_enqueue_recv_refill_simple
>> >> 2) virtio_rxq_vec_setup
>> >>
>> > ...
>> >> diff --git a/drivers/net/virtio/virtio_rxtx_simple.c b/drivers/net/virtio/virtio_rxtx_simple.c
>> >> index 3a1de9d..be51d7c 100644
>> >> --- a/drivers/net/virtio/virtio_rxtx_simple.c
>> >> +++ b/drivers/net/virtio/virtio_rxtx_simple.c
>> >
>> > Hmm, why not wrapping the whole file, instead of just few functions?
>> >
>>
>> Better to refactor code and make arch specific. Current implementation
>> is temporary.
>
> I'm okay to refactor, if it's a clean one. But moving virtio __driver__
> stuff to EAL, sorry, it still doesn't make too much sense to me.
>

You misunderstood my comment, my arch specific meaning
driver/net/virtio/virtio_vec_<arch>.h

> For this case, let's make it simple so far, and consider it when we
> have another vec implementation, or better refactor comes out.
>
>> > Or maybe better, do a compile time check at the Makefile, something
>> > like:
>> >
>> >     if has_CPUFLAG_xxx
>> >         SRCS-$(CONFIG_RTE_LIBRTE_VIRTIO_PMD) += virtio_rxtx_simple.c
>> >     endif
>> >
>> Tried this approach but end up with link error,  If I try to fix below
>> link error then I will be ending up writing similar code,
>
> Sure you need the first part of your patch. Yes, it's similar code,
> but it has fewer "#ifdef " conditions, and more importantly, it
> leaves virtio_rxtx_simple.c alone.
>

Ok.,
>         --yliu
>
>> linker error snap:
>>
>> /work/santosh/thunder/nfs/dpdk/arm64-thunderx-linuxapp-gcc/lib/librte_pmd_virtio.a(virtio_rxtx.o):
>> In function `virtio_dev_rxtx_start':
>> virtio_rxtx.c:(.text+0x168c): undefined reference to
>> `virtqueue_enqueue_recv_refill_simple'
>> /work/santosh/thunder/nfs/dpdk/arm64-thunderx-linuxapp-gcc/lib/librte_pmd_virtio.a(virtio_rxtx.o):
>> In function `virtio_dev_rx_queue_setup':
>> virtio_rxtx.c:(.text+0x2364): undefined reference to `virtio_rxq_vec_setup'
>> /work/santosh/thunder/nfs/dpdk/arm64-thunderx-linuxapp-gcc/lib/librte_pmd_virtio.a(virtio_rxtx.o):
>> In function `virtio_dev_tx_queue_setup':
>> virtio_rxtx.c:(.text+0x2460): undefined reference to `virtio_xmit_pkts_simple'
>> virtio_rxtx.c:(.text+0x2464): undefined reference to `virtio_recv_pkts_vec'
>> virtio_rxtx.c:(.text+0x2468): undefined reference to `virtio_xmit_pkts_simple'
>> virtio_rxtx.c:(.text+0x246c): undefined reference to `virtio_recv_pkts_vec'

Re: [dpdk-dev] [PATCH v1] virtio: Use cpuflag for vector api

[Yuanhan Liu]

On Tue, Mar 01, 2016 at 11:40:41AM +0530, Santosh Shukla wrote:
> On Tue, Mar 1, 2016 at 11:25 AM, Yuanhan Liu
> <yuanhan.liu@linux.intel.com> wrote:
> > On Mon, Feb 29, 2016 at 06:01:38PM +0530, Santosh Shukla wrote:
> >> On Mon, Feb 29, 2016 at 9:57 AM, Yuanhan Liu
> >> <yuanhan.liu@linux.intel.com> wrote:
> >> > On Fri, Feb 26, 2016 at 02:21:02PM +0530, Santosh Shukla wrote:
> >> >> Check cpuflag macro before using vectored api.
> >> >> -virtio_recv_pkts_vec() uses _sse3__ simd instruction for now so added cpuflag.
> >> >> - Also wrap other vectored freind api ie..
> >> >> 1) virtqueue_enqueue_recv_refill_simple
> >> >> 2) virtio_rxq_vec_setup
> >> >>
> >> > ...
> >> >> diff --git a/drivers/net/virtio/virtio_rxtx_simple.c b/drivers/net/virtio/virtio_rxtx_simple.c
> >> >> index 3a1de9d..be51d7c 100644
> >> >> --- a/drivers/net/virtio/virtio_rxtx_simple.c
> >> >> +++ b/drivers/net/virtio/virtio_rxtx_simple.c
> >> >
> >> > Hmm, why not wrapping the whole file, instead of just few functions?
> >> >
> >>
> >> Better to refactor code and make arch specific. Current implementation
> >> is temporary.
> >
> > I'm okay to refactor, if it's a clean one. But moving virtio __driver__
> > stuff to EAL, sorry, it still doesn't make too much sense to me.
> >
> 
> You misunderstood my comment, my arch specific meaning
> driver/net/virtio/virtio_vec_<arch>.h

Oh, sorry. Then, yes, that's the way to go, if refactor is really
needed.

	--yliu

Re: [dpdk-dev] [PATCH v2] virtio: Use cpuflag for vector api

[Yuanhan Liu]

On Tue, Mar 01, 2016 at 11:38:55AM +0530, Santosh Shukla wrote:
> On Tue, Mar 1, 2016 at 11:29 AM, Yuanhan Liu
> <yuanhan.liu@linux.intel.com> wrote:
> > On Mon, Feb 29, 2016 at 06:28:10PM +0530, Santosh Shukla wrote:
> >> Check cpuflag macro before using vectored api.
> >> -virtio_recv_pkts_vec() uses _sse3__ simd instruction for now so added cpuflag.
> >> - Also wrap other vectored freind api ie..
> >> 1) virtqueue_enqueue_recv_refill_simple
> >> 2) virtio_rxq_vec_setup
> >>
> >> - removed VIRTIO_PMD=n from armv7/v8 config.
> >>
> >> todo:
> >> 1) Move virtio_recv_pkts_vec() implementation to
> >>    drivers/virtio/virtio_vec_<arch>.h file.
> >> 2) Remove use_simple_rxtx flag, so that virtio/virtio_vec_<arch>.h
> >>    files to provide vectored/non-vectored rx/tx apis.
> >>
> >> Signed-off-by: Santosh Shukla <sshukla@mvista.com>
> >> ---
> >> - v2: Removed VIRTIO_PMD=n from arm v7/v8
> >
> > Firstly, I would not suggest you to send another new version, while there
> > still was discussions ongoing on old version.
> >
> > And, you should not mix the ARM stuff here; this patch should only do
> > what the patch title tells. In generic, don't do two or more things in
> > one patch.
> >
> 
> w/o v2 patch, old version wont build for armv7/v8. Clubbing both in
> v2, inspired from v7 virtio INC_VEC review comment/feedback [1].

Thinking it this way, that build won't work for ARM, with or without
this patch. And this patch just fix a build error for platforms that
doesn't has vec instructions (which could include old x86 platforms).

So, the right way to go is to separate the ARM stuff to another
standalone patch, claiming that we now supports ARM.

Makes sense to you?

BTW, is this the last piece of code to make virtio for ARM work?
I maybe wrong, but I remembered you have few more patches for virtio
in old versions. (Yeah, I'm aware of that the EAL parts have been
merged)

Anyway, here is a remind: don't forget to update release note:

    doc/guides/rel_notes/release_16_04.rst

	--yliu

Re: [dpdk-dev] [PATCH v1] virtio: Use cpuflag for vector api

[Qiu, Michael]

On 2/26/2016 4:53 PM, Santosh Shukla wrote:
> Check cpuflag macro before using vectored api.
> -virtio_recv_pkts_vec() uses _sse3__ simd instruction for now so added cpuflag.
> - Also wrap other vectored freind api ie..
> 1) virtqueue_enqueue_recv_refill_simple
> 2) virtio_rxq_vec_setup
>
> todo:
> 1) Move virtio_recv_pkts_vec() implementation to
>    drivers/virtio/virtio_vec_<arch>.h file.
> 2) Remove use_simple_rxtx flag, so that virtio/virtio_vec_<arch>.h
>    files to provide vectored/non-vectored rx/tx apis.
>
> Signed-off-by: Santosh Shukla <sshukla@mvista.com>
> ---
> - v1: This is a rework of patch [1].
> Note: This patch will let non-x86 arch to use virtio pmd.
>
> [1] http://dpdk.org/dev/patchwork/patch/10429/
>
>  drivers/net/virtio/virtio_rxtx.c        |   16 +++++++++++++++-
>  drivers/net/virtio/virtio_rxtx.h        |    2 ++
>  drivers/net/virtio/virtio_rxtx_simple.c |   11 ++++++++++-
>  3 files changed, 27 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/net/virtio/virtio_rxtx.c b/drivers/net/virtio/virtio_rxtx.c
> index 41a1366..ec0b8de 100644
> --- a/drivers/net/virtio/virtio_rxtx.c
> +++ b/drivers/net/virtio/virtio_rxtx.c
> @@ -67,7 +67,9 @@
>  #define VIRTIO_SIMPLE_FLAGS ((uint32_t)ETH_TXQ_FLAGS_NOMULTSEGS | \
>  	ETH_TXQ_FLAGS_NOOFFLOADS)
>  
> +#ifdef RTE_MACHINE_CPUFLAG_SSSE3
>  static int use_simple_rxtx;
> +#endif
>  
>

I don't think so much #ifdef ... #endif in *.c file is a good choice.
Would you consider let it only in header file like:

in drivers/net/virtio/virtio_rxtx.h

[...]

#ifdef RTE_MACHINE_CPUFLAG_SSSE3
int virtio_rxq_vec_setup(struct virtqueue *rxq);

int virtqueue_enqueue_recv_refill_simple(struct virtqueue *vq,
	struct rte_mbuf *m);
#else
int virtio_rxq_vec_setup(__rte_unused struct virtqueue *rxq) {return -1;}
int virtqueue_enqueue_recv_refill_simple(__rte_unused struct virtqueue *vq,
					 __rte_unused struct rte_mbuf *m) {
	return -1;
}
#endif

and remove most #ifdef ... #endif in *.c file.

Thanks,
Michael

Re: [dpdk-dev] [PATCH v1] virtio: Use cpuflag for vector api

[Santosh Shukla]

On Tue, Mar 1, 2016 at 2:41 PM, Qiu, Michael <michael.qiu@intel.com> wrote:
> On 2/26/2016 4:53 PM, Santosh Shukla wrote:
>> Check cpuflag macro before using vectored api.
>> -virtio_recv_pkts_vec() uses _sse3__ simd instruction for now so added cpuflag.
>> - Also wrap other vectored freind api ie..
>> 1) virtqueue_enqueue_recv_refill_simple
>> 2) virtio_rxq_vec_setup
>>
>> todo:
>> 1) Move virtio_recv_pkts_vec() implementation to
>>    drivers/virtio/virtio_vec_<arch>.h file.
>> 2) Remove use_simple_rxtx flag, so that virtio/virtio_vec_<arch>.h
>>    files to provide vectored/non-vectored rx/tx apis.
>>
>> Signed-off-by: Santosh Shukla <sshukla@mvista.com>
>> ---
>> - v1: This is a rework of patch [1].
>> Note: This patch will let non-x86 arch to use virtio pmd.
>>
>> [1] http://dpdk.org/dev/patchwork/patch/10429/
>>
>>  drivers/net/virtio/virtio_rxtx.c        |   16 +++++++++++++++-
>>  drivers/net/virtio/virtio_rxtx.h        |    2 ++
>>  drivers/net/virtio/virtio_rxtx_simple.c |   11 ++++++++++-
>>  3 files changed, 27 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/net/virtio/virtio_rxtx.c b/drivers/net/virtio/virtio_rxtx.c
>> index 41a1366..ec0b8de 100644
>> --- a/drivers/net/virtio/virtio_rxtx.c
>> +++ b/drivers/net/virtio/virtio_rxtx.c
>> @@ -67,7 +67,9 @@
>>  #define VIRTIO_SIMPLE_FLAGS ((uint32_t)ETH_TXQ_FLAGS_NOMULTSEGS | \
>>       ETH_TXQ_FLAGS_NOOFFLOADS)
>>
>> +#ifdef RTE_MACHINE_CPUFLAG_SSSE3
>>  static int use_simple_rxtx;
>> +#endif
>>
>>
>
> I don't think so much #ifdef ... #endif in *.c file is a good choice.
> Would you consider let it only in header file like:
>
> in drivers/net/virtio/virtio_rxtx.h
>
> [...]
>
> #ifdef RTE_MACHINE_CPUFLAG_SSSE3
> int virtio_rxq_vec_setup(struct virtqueue *rxq);
>
> int virtqueue_enqueue_recv_refill_simple(struct virtqueue *vq,
>         struct rte_mbuf *m);
> #else
> int virtio_rxq_vec_setup(__rte_unused struct virtqueue *rxq) {return -1;}
> int virtqueue_enqueue_recv_refill_simple(__rte_unused struct virtqueue *vq,
>                                          __rte_unused struct rte_mbuf *m) {
>         return -1;
> }
> #endif
>
> and remove most #ifdef ... #endif in *.c file.
>

I guess, above approach wont work for non-x86 arch, ad those func are
dummy, right? also code wont build for arm/non-86 arch because
tx/rx_pkt_burst callback will be using x86 specific virtio vec rx/tx
api.

> Thanks,
> Michael

[dpdk-dev] [PATCH v1 0/3] virtio vector and misc

[Santosh Shukla]

- 1st patch: let non-x86 arch use virtio pmd driver in non-vec
- 2nd patch: enable virtio arm support
- 3rd patch: update virtio for arm feature entry in release guide.

Thanks.

Santosh Shukla (3):
  virtio: use vector rx/tx for ssse cpuflag only
  config: enable virtio for armv7/v8
  guide/release: add virtio for arm feature info

 config/defconfig_arm-armv7a-linuxapp-gcc   |    1 -
 config/defconfig_arm64-armv8a-linuxapp-gcc |    1 -
 doc/guides/rel_notes/release_16_04.rst     |    5 +++++
 drivers/net/virtio/Makefile                |    3 +++
 drivers/net/virtio/virtio_rxtx.c           |   16 +++++++++++++++-
 drivers/net/virtio/virtio_rxtx.h           |    2 ++
 6 files changed, 25 insertions(+), 3 deletions(-)

-- 
1.7.9.5

[dpdk-dev] [PATCH v1 1/3] virtio: use vector rx/tx for ssse cpuflag only

[Santosh Shukla]

Temporary implementation to let virtio operate in non-vec mode for archs which
doesn't support _ssse_ cpuflag.

todo:
1) Move virtio_recv_pkts_vec() implementation to
   drivers/virtio/virtio_vec_<arch>.h file.
2) Remove use_simple_rxtx flag, so that virtio/virtio_vec_<arch>.h
   files to provide vectored/non-vectored rx/tx apis.

Signed-off-by: Santosh Shukla <sshukla@mvista.com>
---
v1: rework of this[1] patch
[1] http://dpdk.org/dev/patchwork/patch/10911/
- Removed ifdef from virtio_rxtx_simple.c

 drivers/net/virtio/Makefile      |    3 +++
 drivers/net/virtio/virtio_rxtx.c |   16 +++++++++++++++-
 drivers/net/virtio/virtio_rxtx.h |    2 ++
 3 files changed, 20 insertions(+), 1 deletion(-)

diff --git a/drivers/net/virtio/Makefile b/drivers/net/virtio/Makefile
index 43835ba..ef84f60 100644
--- a/drivers/net/virtio/Makefile
+++ b/drivers/net/virtio/Makefile
@@ -50,7 +50,10 @@ 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
+
+ifeq ($(findstring RTE_MACHINE_CPUFLAG_SSSE3,$(CFLAGS)),RTE_MACHINE_CPUFLAG_SSSE3)
 SRCS-$(CONFIG_RTE_LIBRTE_VIRTIO_PMD) += virtio_rxtx_simple.c
+endif
 
 # 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 41a1366..ec0b8de 100644
--- a/drivers/net/virtio/virtio_rxtx.c
+++ b/drivers/net/virtio/virtio_rxtx.c
@@ -67,7 +67,9 @@
 #define VIRTIO_SIMPLE_FLAGS ((uint32_t)ETH_TXQ_FLAGS_NOMULTSEGS | \
 	ETH_TXQ_FLAGS_NOOFFLOADS)
 
+#ifdef RTE_MACHINE_CPUFLAG_SSSE3
 static int use_simple_rxtx;
+#endif
 
 static void
 vq_ring_free_chain(struct virtqueue *vq, uint16_t desc_idx)
@@ -307,12 +309,13 @@ virtio_dev_vring_start(struct virtqueue *vq, int queue_type)
 		nbufs = 0;
 		error = ENOSPC;
 
+#ifdef RTE_MACHINE_CPUFLAG_SSSE3
 		if (use_simple_rxtx)
 			for (i = 0; i < vq->vq_nentries; i++) {
 				vq->vq_ring.avail->ring[i] = i;
 				vq->vq_ring.desc[i].flags = VRING_DESC_F_WRITE;
 			}
-
+#endif
 		memset(&vq->fake_mbuf, 0, sizeof(vq->fake_mbuf));
 		for (i = 0; i < RTE_PMD_VIRTIO_RX_MAX_BURST; i++)
 			vq->sw_ring[vq->vq_nentries + i] = &vq->fake_mbuf;
@@ -325,9 +328,11 @@ virtio_dev_vring_start(struct virtqueue *vq, int queue_type)
 			/******************************************
 			*         Enqueue allocated buffers        *
 			*******************************************/
+#ifdef RTE_MACHINE_CPUFLAG_SSSE3
 			if (use_simple_rxtx)
 				error = virtqueue_enqueue_recv_refill_simple(vq, m);
 			else
+#endif
 				error = virtqueue_enqueue_recv_refill(vq, m);
 			if (error) {
 				rte_pktmbuf_free(m);
@@ -340,6 +345,7 @@ virtio_dev_vring_start(struct virtqueue *vq, int queue_type)
 
 		PMD_INIT_LOG(DEBUG, "Allocated %d bufs", nbufs);
 	} else if (queue_type == VTNET_TQ) {
+#ifdef RTE_MACHINE_CPUFLAG_SSSE3
 		if (use_simple_rxtx) {
 			int mid_idx  = vq->vq_nentries >> 1;
 			for (i = 0; i < mid_idx; i++) {
@@ -357,6 +363,7 @@ virtio_dev_vring_start(struct virtqueue *vq, int queue_type)
 			for (i = mid_idx; i < vq->vq_nentries; i++)
 				vq->vq_ring.avail->ring[i] = i;
 		}
+#endif
 	}
 }
 
@@ -423,7 +430,9 @@ virtio_dev_rx_queue_setup(struct rte_eth_dev *dev,
 
 	dev->data->rx_queues[queue_idx] = vq;
 
+#ifdef RTE_MACHINE_CPUFLAG_SSSE3
 	virtio_rxq_vec_setup(vq);
+#endif
 
 	return 0;
 }
@@ -449,7 +458,10 @@ virtio_dev_tx_queue_setup(struct rte_eth_dev *dev,
 			const struct rte_eth_txconf *tx_conf)
 {
 	uint8_t vtpci_queue_idx = 2 * queue_idx + VTNET_SQ_TQ_QUEUE_IDX;
+
+#ifdef RTE_MACHINE_CPUFLAG_SSSE3
 	struct virtio_hw *hw = dev->data->dev_private;
+#endif
 	struct virtqueue *vq;
 	uint16_t tx_free_thresh;
 	int ret;
@@ -462,6 +474,7 @@ virtio_dev_tx_queue_setup(struct rte_eth_dev *dev,
 		return -EINVAL;
 	}
 
+#ifdef RTE_MACHINE_CPUFLAG_SSSE3
 	/* Use simple rx/tx func if single segment and no offloads */
 	if ((tx_conf->txq_flags & VIRTIO_SIMPLE_FLAGS) == VIRTIO_SIMPLE_FLAGS &&
 	     !vtpci_with_feature(hw, VIRTIO_NET_F_MRG_RXBUF)) {
@@ -470,6 +483,7 @@ virtio_dev_tx_queue_setup(struct rte_eth_dev *dev,
 		dev->rx_pkt_burst = virtio_recv_pkts_vec;
 		use_simple_rxtx = 1;
 	}
+#endif
 
 	ret = virtio_dev_queue_setup(dev, VTNET_TQ, queue_idx, vtpci_queue_idx,
 			nb_desc, socket_id, &vq);
diff --git a/drivers/net/virtio/virtio_rxtx.h b/drivers/net/virtio/virtio_rxtx.h
index 831e492..a76c3e5 100644
--- a/drivers/net/virtio/virtio_rxtx.h
+++ b/drivers/net/virtio/virtio_rxtx.h
@@ -33,7 +33,9 @@
 
 #define RTE_PMD_VIRTIO_RX_MAX_BURST 64
 
+#ifdef RTE_MACHINE_CPUFLAG_SSSE3
 int virtio_rxq_vec_setup(struct virtqueue *rxq);
 
 int virtqueue_enqueue_recv_refill_simple(struct virtqueue *vq,
 	struct rte_mbuf *m);
+#endif
-- 
1.7.9.5

[dpdk-dev] [PATCH v1 2/3] config: enable virtio for armv7/v8

[Santosh Shukla]

removed _VIRTIO_PMD=n from arch config and let arch to use _VIRTIO_PMD from
config/common_linuxapp.

Signed-off-by: Santosh Shukla <sshukla@mvista.com>
---
 config/defconfig_arm-armv7a-linuxapp-gcc   |    1 -
 config/defconfig_arm64-armv8a-linuxapp-gcc |    1 -
 2 files changed, 2 deletions(-)

diff --git a/config/defconfig_arm-armv7a-linuxapp-gcc b/config/defconfig_arm-armv7a-linuxapp-gcc
index cbebd64..4bfdfad 100644
--- a/config/defconfig_arm-armv7a-linuxapp-gcc
+++ b/config/defconfig_arm-armv7a-linuxapp-gcc
@@ -70,7 +70,6 @@ CONFIG_RTE_LIBRTE_I40E_PMD=n
 CONFIG_RTE_LIBRTE_IXGBE_PMD=n
 CONFIG_RTE_LIBRTE_MLX4_PMD=n
 CONFIG_RTE_LIBRTE_MPIPE_PMD=n
-CONFIG_RTE_LIBRTE_VIRTIO_PMD=n
 CONFIG_RTE_LIBRTE_VMXNET3_PMD=n
 CONFIG_RTE_LIBRTE_PMD_XENVIRT=n
 CONFIG_RTE_LIBRTE_PMD_BNX2X=n
diff --git a/config/defconfig_arm64-armv8a-linuxapp-gcc b/config/defconfig_arm64-armv8a-linuxapp-gcc
index eacd01c..f6f5d18 100644
--- a/config/defconfig_arm64-armv8a-linuxapp-gcc
+++ b/config/defconfig_arm64-armv8a-linuxapp-gcc
@@ -44,7 +44,6 @@ CONFIG_RTE_TOOLCHAIN="gcc"
 CONFIG_RTE_TOOLCHAIN_GCC=y
 
 CONFIG_RTE_IXGBE_INC_VECTOR=n
-CONFIG_RTE_LIBRTE_VIRTIO_PMD=n
 CONFIG_RTE_LIBRTE_IVSHMEM=n
 CONFIG_RTE_LIBRTE_FM10K_PMD=n
 CONFIG_RTE_LIBRTE_I40E_PMD=n
-- 
1.7.9.5

[dpdk-dev] [PATCH v1 3/3] guide/release: add virtio for arm feature info

[Santosh Shukla]

Signed-off-by: Santosh Shukla <sshukla@mvista.com>
---
 doc/guides/rel_notes/release_16_04.rst |    5 +++++
 1 file changed, 5 insertions(+)

diff --git a/doc/guides/rel_notes/release_16_04.rst b/doc/guides/rel_notes/release_16_04.rst
index 8273817..fc0b3bd 100644
--- a/doc/guides/rel_notes/release_16_04.rst
+++ b/doc/guides/rel_notes/release_16_04.rst
@@ -46,6 +46,11 @@ This section should contain new features added in this release. Sample format:
 
 * **Added vhost-user live migration support.**
 
+* **Added virtio for arm support.**
+  Enabled virtio support for armv7/v8. Tested for arm64. Virtio can work with
+  other non-x86 arch too like powerpc.
+  * virtio for arm support VFIO(-noiommu) mode only
+
 
 Resolved Issues
 ---------------
-- 
1.7.9.5

Re: [dpdk-dev] [PATCH v2] virtio: Use cpuflag for vector api

[Santosh Shukla]

On Tue, Mar 1, 2016 at 12:02 PM, Yuanhan Liu
<yuanhan.liu@linux.intel.com> wrote:
> On Tue, Mar 01, 2016 at 11:38:55AM +0530, Santosh Shukla wrote:
>> On Tue, Mar 1, 2016 at 11:29 AM, Yuanhan Liu
>> <yuanhan.liu@linux.intel.com> wrote:
>> > On Mon, Feb 29, 2016 at 06:28:10PM +0530, Santosh Shukla wrote:
>> >> Check cpuflag macro before using vectored api.
>> >> -virtio_recv_pkts_vec() uses _sse3__ simd instruction for now so added cpuflag.
>> >> - Also wrap other vectored freind api ie..
>> >> 1) virtqueue_enqueue_recv_refill_simple
>> >> 2) virtio_rxq_vec_setup
>> >>
>> >> - removed VIRTIO_PMD=n from armv7/v8 config.
>> >>
>> >> todo:
>> >> 1) Move virtio_recv_pkts_vec() implementation to
>> >>    drivers/virtio/virtio_vec_<arch>.h file.
>> >> 2) Remove use_simple_rxtx flag, so that virtio/virtio_vec_<arch>.h
>> >>    files to provide vectored/non-vectored rx/tx apis.
>> >>
>> >> Signed-off-by: Santosh Shukla <sshukla@mvista.com>
>> >> ---
>> >> - v2: Removed VIRTIO_PMD=n from arm v7/v8
>> >
>> > Firstly, I would not suggest you to send another new version, while there
>> > still was discussions ongoing on old version.
>> >
>> > And, you should not mix the ARM stuff here; this patch should only do
>> > what the patch title tells. In generic, don't do two or more things in
>> > one patch.
>> >
>>
>> w/o v2 patch, old version wont build for armv7/v8. Clubbing both in
>> v2, inspired from v7 virtio INC_VEC review comment/feedback [1].
>
> Thinking it this way, that build won't work for ARM, with or without
> this patch. And this patch just fix a build error for platforms that
> doesn't has vec instructions (which could include old x86 platforms).
>
> So, the right way to go is to separate the ARM stuff to another
> standalone patch, claiming that we now supports ARM.
>
> Makes sense to you?
>

Sent a new patch series. Incorporated comments in this thread
http://dpdk.org/dev/patchwork/patch/10945/
http://dpdk.org/dev/patchwork/patch/10946/

> BTW, is this the last piece of code to make virtio for ARM work?
> I maybe wrong, but I remembered you have few more patches for virtio
> in old versions. (Yeah, I'm aware of that the EAL parts have been
> merged)
>
> Anyway, here is a remind: don't forget to update release note:
>
>     doc/guides/rel_notes/release_16_04.rst
>
Posted a patch just now:
http://dpdk.org/dev/patchwork/patch/10947/

>         --yliu
>

Re: [dpdk-dev] [PATCH v1] virtio: Use cpuflag for vector api

[Qiu, Michael]

On 3/1/2016 5:46 PM, Santosh Shukla wrote:
> On Tue, Mar 1, 2016 at 2:41 PM, Qiu, Michael <michael.qiu@intel.com> wrote:
>> On 2/26/2016 4:53 PM, Santosh Shukla wrote:
>>> Check cpuflag macro before using vectored api.
>>> -virtio_recv_pkts_vec() uses _sse3__ simd instruction for now so added cpuflag.
>>> - Also wrap other vectored freind api ie..
>>> 1) virtqueue_enqueue_recv_refill_simple
>>> 2) virtio_rxq_vec_setup
>>>
>>> todo:
>>> 1) Move virtio_recv_pkts_vec() implementation to
>>>    drivers/virtio/virtio_vec_<arch>.h file.
>>> 2) Remove use_simple_rxtx flag, so that virtio/virtio_vec_<arch>.h
>>>    files to provide vectored/non-vectored rx/tx apis.
>>>
>>> Signed-off-by: Santosh Shukla <sshukla@mvista.com>
>>> ---
>>> - v1: This is a rework of patch [1].
>>> Note: This patch will let non-x86 arch to use virtio pmd.
>>>
>>> [1] http://dpdk.org/dev/patchwork/patch/10429/
>>>
>>>  drivers/net/virtio/virtio_rxtx.c        |   16 +++++++++++++++-
>>>  drivers/net/virtio/virtio_rxtx.h        |    2 ++
>>>  drivers/net/virtio/virtio_rxtx_simple.c |   11 ++++++++++-
>>>  3 files changed, 27 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/drivers/net/virtio/virtio_rxtx.c b/drivers/net/virtio/virtio_rxtx.c
>>> index 41a1366..ec0b8de 100644
>>> --- a/drivers/net/virtio/virtio_rxtx.c
>>> +++ b/drivers/net/virtio/virtio_rxtx.c
>>> @@ -67,7 +67,9 @@
>>>  #define VIRTIO_SIMPLE_FLAGS ((uint32_t)ETH_TXQ_FLAGS_NOMULTSEGS | \
>>>       ETH_TXQ_FLAGS_NOOFFLOADS)
>>>
>>> +#ifdef RTE_MACHINE_CPUFLAG_SSSE3
>>>  static int use_simple_rxtx;
>>> +#endif
>>>
>>>
>> I don't think so much #ifdef ... #endif in *.c file is a good choice.
>> Would you consider let it only in header file like:
>>
>> in drivers/net/virtio/virtio_rxtx.h
>>
>> [...]
>>
>> #ifdef RTE_MACHINE_CPUFLAG_SSSE3
>> int virtio_rxq_vec_setup(struct virtqueue *rxq);
>>
>> int virtqueue_enqueue_recv_refill_simple(struct virtqueue *vq,
>>         struct rte_mbuf *m);
>> #else
>> int virtio_rxq_vec_setup(__rte_unused struct virtqueue *rxq) {return -1;}
>> int virtqueue_enqueue_recv_refill_simple(__rte_unused struct virtqueue *vq,
>>                                          __rte_unused struct rte_mbuf *m) {
>>         return -1;
>> }
>> #endif
>>
>> and remove most #ifdef ... #endif in *.c file.
>>
> I guess, above approach wont work for non-x86 arch, ad those func are
> dummy, right? also code wont build for arm/non-86 arch because
> tx/rx_pkt_burst callback will be using x86 specific virtio vec rx/tx
> api.

You may right, but you really need to reduce the #ifdef in *.c files.

Thanks,
Michael

>> Thanks,
>> Michael

Re: [dpdk-dev] [PATCH v1] virtio: Use cpuflag for vector api

[Yuanhan Liu]

On Wed, Mar 02, 2016 at 02:10:14AM +0000, Qiu, Michael wrote:
> On 3/1/2016 5:46 PM, Santosh Shukla wrote:
> > On Tue, Mar 1, 2016 at 2:41 PM, Qiu, Michael <michael.qiu@intel.com> wrote:
> >> On 2/26/2016 4:53 PM, Santosh Shukla wrote:
> >>> Check cpuflag macro before using vectored api.
> >>> -virtio_recv_pkts_vec() uses _sse3__ simd instruction for now so added cpuflag.
> >>> - Also wrap other vectored freind api ie..
> >>> 1) virtqueue_enqueue_recv_refill_simple
> >>> 2) virtio_rxq_vec_setup
> >>>
> >>> todo:
> >>> 1) Move virtio_recv_pkts_vec() implementation to
> >>>    drivers/virtio/virtio_vec_<arch>.h file.
> >>> 2) Remove use_simple_rxtx flag, so that virtio/virtio_vec_<arch>.h
> >>>    files to provide vectored/non-vectored rx/tx apis.
> >>>
> >>> Signed-off-by: Santosh Shukla <sshukla@mvista.com>
> >>> ---
> >>> - v1: This is a rework of patch [1].
> >>> Note: This patch will let non-x86 arch to use virtio pmd.
> >>>
> >>> [1] http://dpdk.org/dev/patchwork/patch/10429/
> >>>
> >>>  drivers/net/virtio/virtio_rxtx.c        |   16 +++++++++++++++-
> >>>  drivers/net/virtio/virtio_rxtx.h        |    2 ++
> >>>  drivers/net/virtio/virtio_rxtx_simple.c |   11 ++++++++++-
> >>>  3 files changed, 27 insertions(+), 2 deletions(-)
> >>>
> >>> diff --git a/drivers/net/virtio/virtio_rxtx.c b/drivers/net/virtio/virtio_rxtx.c
> >>> index 41a1366..ec0b8de 100644
> >>> --- a/drivers/net/virtio/virtio_rxtx.c
> >>> +++ b/drivers/net/virtio/virtio_rxtx.c
> >>> @@ -67,7 +67,9 @@
> >>>  #define VIRTIO_SIMPLE_FLAGS ((uint32_t)ETH_TXQ_FLAGS_NOMULTSEGS | \
> >>>       ETH_TXQ_FLAGS_NOOFFLOADS)
> >>>
> >>> +#ifdef RTE_MACHINE_CPUFLAG_SSSE3
> >>>  static int use_simple_rxtx;
> >>> +#endif
> >>>
> >>>
> >> I don't think so much #ifdef ... #endif in *.c file is a good choice.
> >> Would you consider let it only in header file like:
> >>
> >> in drivers/net/virtio/virtio_rxtx.h
> >>
> >> [...]
> >>
> >> #ifdef RTE_MACHINE_CPUFLAG_SSSE3
> >> int virtio_rxq_vec_setup(struct virtqueue *rxq);
> >>
> >> int virtqueue_enqueue_recv_refill_simple(struct virtqueue *vq,
> >>         struct rte_mbuf *m);
> >> #else
> >> int virtio_rxq_vec_setup(__rte_unused struct virtqueue *rxq) {return -1;}
> >> int virtqueue_enqueue_recv_refill_simple(__rte_unused struct virtqueue *vq,
> >>                                          __rte_unused struct rte_mbuf *m) {
> >>         return -1;
> >> }
> >> #endif
> >>
> >> and remove most #ifdef ... #endif in *.c file.
> >>
> > I guess, above approach wont work for non-x86 arch, ad those func are
> > dummy, right? also code wont build for arm/non-86 arch because
> > tx/rx_pkt_burst callback will be using x86 specific virtio vec rx/tx
> > api.
> 
> You may right, but you really need to reduce the #ifdef in *.c files.

In general, yes. But for this case, no: those vec stuff are for
platforms that support it. For other platforms, we should not
invoke a dummy function like virtio_rxq_vec_setup() at all.

The right way to go is to add another wrapper beyond the vector
stuff, something like:

	virtio_rxq_setup()
	{

		if (has_vec_support)
			virtio_rxq_vec_setup();
		else
			virtio_rxq_generic_setup();
	}

Where virtio_rxq_vec_setup() could have a per-arch implementation,
say for X86, or ARM.

It touchs more code, but for now, I'd like to make it simple first.
With the virtio_rxtx_simple.c isolated from Makefile, there aren't
many #ifdef after all.

	--yliu

Re: [dpdk-dev] [PATCH v1 0/3] virtio vector and misc

[Yuanhan Liu]

On Tue, Mar 01, 2016 at 03:32:17PM +0530, Santosh Shukla wrote:
> - 1st patch: let non-x86 arch use virtio pmd driver in non-vec
> - 2nd patch: enable virtio arm support
> - 3rd patch: update virtio for arm feature entry in release guide.

Series looks good to me:

Acked-by: Yuanhan Liu <yuanhan.liu@linux.intel.com>

However, FYI, Thomas would like to see that the release note is
added __inside__ the patch that acutally enables it, but not in
another standalone patch. In another word, you should squash
patch 3 to patch 2.

I'm wondering Thomas could do that for you this time while applying
your pathces. But, this is just a kind remind, and you should not
do that next time.

	--yliu

Re: [dpdk-dev] [PATCH v1 0/3] virtio vector and misc

[Santosh Shukla]

On Wed, Mar 2, 2016 at 2:02 PM, Yuanhan Liu <yuanhan.liu@linux.intel.com> wrote:
> On Tue, Mar 01, 2016 at 03:32:17PM +0530, Santosh Shukla wrote:
>> - 1st patch: let non-x86 arch use virtio pmd driver in non-vec
>> - 2nd patch: enable virtio arm support
>> - 3rd patch: update virtio for arm feature entry in release guide.
>
> Series looks good to me:
>
> Acked-by: Yuanhan Liu <yuanhan.liu@linux.intel.com>
>
> However, FYI, Thomas would like to see that the release note is
> added __inside__ the patch that acutally enables it, but not in
> another standalone patch. In another word, you should squash
> patch 3 to patch 2.
>
> I'm wondering Thomas could do that for you this time while applying
> your pathces. But, this is just a kind remind, and you should not
> do that next time.
>

Thanks!,

Thomas, Can you please take care?

>         --yliu

Re: [dpdk-dev] [PATCH v1 0/3] virtio vector and misc

[Thomas Monjalon]

2016-03-02 14:11, Santosh Shukla:
> On Wed, Mar 2, 2016 at 2:02 PM, Yuanhan Liu <yuanhan.liu@linux.intel.com> wrote:
> > On Tue, Mar 01, 2016 at 03:32:17PM +0530, Santosh Shukla wrote:
> >> - 1st patch: let non-x86 arch use virtio pmd driver in non-vec
> >> - 2nd patch: enable virtio arm support
> >> - 3rd patch: update virtio for arm feature entry in release guide.
> >
> > Series looks good to me:
> >
> > Acked-by: Yuanhan Liu <yuanhan.liu@linux.intel.com>
> >
> > However, FYI, Thomas would like to see that the release note is
> > added __inside__ the patch that acutally enables it, but not in
> > another standalone patch. In another word, you should squash
> > patch 3 to patch 2.
> >
> > I'm wondering Thomas could do that for you this time while applying
> > your pathces. But, this is just a kind remind, and you should not
> > do that next time.
> >
> 
> Thanks!,
> 
> Thomas, Can you please take care?

I've squashed and reworded a bit the release note.
Applied, thanks.

Re: [dpdk-dev] [PATCH v1 0/3] virtio vector and misc

[Santosh Shukla]

On Thu, Mar 3, 2016 at 6:56 PM, Thomas Monjalon
<thomas.monjalon@6wind.com> wrote:
> 2016-03-02 14:11, Santosh Shukla:
>> On Wed, Mar 2, 2016 at 2:02 PM, Yuanhan Liu <yuanhan.liu@linux.intel.com> wrote:
>> > On Tue, Mar 01, 2016 at 03:32:17PM +0530, Santosh Shukla wrote:
>> >> - 1st patch: let non-x86 arch use virtio pmd driver in non-vec
>> >> - 2nd patch: enable virtio arm support
>> >> - 3rd patch: update virtio for arm feature entry in release guide.
>> >
>> > Series looks good to me:
>> >
>> > Acked-by: Yuanhan Liu <yuanhan.liu@linux.intel.com>
>> >
>> > However, FYI, Thomas would like to see that the release note is
>> > added __inside__ the patch that acutally enables it, but not in
>> > another standalone patch. In another word, you should squash
>> > patch 3 to patch 2.
>> >
>> > I'm wondering Thomas could do that for you this time while applying
>> > your pathces. But, this is just a kind remind, and you should not
>> > do that next time.
>> >
>>
>> Thanks!,
>>
>> Thomas, Can you please take care?
>
> I've squashed and reworded a bit the release note.
> Applied, thanks.

Thanks you!

Re: [dpdk-dev] [PATCH v1] virtio: Use cpuflag for vector api

[Qiu, Michael]

On 3/2/2016 10:48 AM, Yuanhan Liu wrote:
> On Wed, Mar 02, 2016 at 02:10:14AM +0000, Qiu, Michael wrote:
>> On 3/1/2016 5:46 PM, Santosh Shukla wrote:
>>> On Tue, Mar 1, 2016 at 2:41 PM, Qiu, Michael <michael.qiu@intel.com> wrote:
>>>> On 2/26/2016 4:53 PM, Santosh Shukla wrote:
>>>>> Check cpuflag macro before using vectored api.
>>>>> -virtio_recv_pkts_vec() uses _sse3__ simd instruction for now so added cpuflag.
>>>>> - Also wrap other vectored freind api ie..
>>>>> 1) virtqueue_enqueue_recv_refill_simple
>>>>> 2) virtio_rxq_vec_setup
>>>>>
>>>>> todo:
>>>>> 1) Move virtio_recv_pkts_vec() implementation to
>>>>>    drivers/virtio/virtio_vec_<arch>.h file.
>>>>> 2) Remove use_simple_rxtx flag, so that virtio/virtio_vec_<arch>.h
>>>>>    files to provide vectored/non-vectored rx/tx apis.
>>>>>
>>>>> Signed-off-by: Santosh Shukla <sshukla@mvista.com>
>>>>> ---
>>>>> - v1: This is a rework of patch [1].
>>>>> Note: This patch will let non-x86 arch to use virtio pmd.
>>>>>
>>>>> [1] http://dpdk.org/dev/patchwork/patch/10429/
>>>>>
>>>>>  drivers/net/virtio/virtio_rxtx.c        |   16 +++++++++++++++-
>>>>>  drivers/net/virtio/virtio_rxtx.h        |    2 ++
>>>>>  drivers/net/virtio/virtio_rxtx_simple.c |   11 ++++++++++-
>>>>>  3 files changed, 27 insertions(+), 2 deletions(-)
>>>>>
>>>>> diff --git a/drivers/net/virtio/virtio_rxtx.c b/drivers/net/virtio/virtio_rxtx.c
>>>>> index 41a1366..ec0b8de 100644
>>>>> --- a/drivers/net/virtio/virtio_rxtx.c
>>>>> +++ b/drivers/net/virtio/virtio_rxtx.c
>>>>> @@ -67,7 +67,9 @@
>>>>>  #define VIRTIO_SIMPLE_FLAGS ((uint32_t)ETH_TXQ_FLAGS_NOMULTSEGS | \
>>>>>       ETH_TXQ_FLAGS_NOOFFLOADS)
>>>>>
>>>>> +#ifdef RTE_MACHINE_CPUFLAG_SSSE3
>>>>>  static int use_simple_rxtx;
>>>>> +#endif
>>>>>
>>>>>
>>>> I don't think so much #ifdef ... #endif in *.c file is a good choice.
>>>> Would you consider let it only in header file like:
>>>>
>>>> in drivers/net/virtio/virtio_rxtx.h
>>>>
>>>> [...]
>>>>
>>>> #ifdef RTE_MACHINE_CPUFLAG_SSSE3
>>>> int virtio_rxq_vec_setup(struct virtqueue *rxq);
>>>>
>>>> int virtqueue_enqueue_recv_refill_simple(struct virtqueue *vq,
>>>>         struct rte_mbuf *m);
>>>> #else
>>>> int virtio_rxq_vec_setup(__rte_unused struct virtqueue *rxq) {return -1;}
>>>> int virtqueue_enqueue_recv_refill_simple(__rte_unused struct virtqueue *vq,
>>>>                                          __rte_unused struct rte_mbuf *m) {
>>>>         return -1;
>>>> }
>>>> #endif
>>>>
>>>> and remove most #ifdef ... #endif in *.c file.
>>>>
>>> I guess, above approach wont work for non-x86 arch, ad those func are
>>> dummy, right? also code wont build for arm/non-86 arch because
>>> tx/rx_pkt_burst callback will be using x86 specific virtio vec rx/tx
>>> api.
>> You may right, but you really need to reduce the #ifdef in *.c files.
> In general, yes. But for this case, no: those vec stuff are for
> platforms that support it. For other platforms, we should not
> invoke a dummy function like virtio_rxq_vec_setup() at all.
>
> The right way to go is to add another wrapper beyond the vector
> stuff, something like:
>
> 	virtio_rxq_setup()
> 	{
>
> 		if (has_vec_support)
> 			virtio_rxq_vec_setup();
> 		else
> 			virtio_rxq_generic_setup();
> 	}

Actually, we could call vec first and if set up failed, fall back to
simple mode. Thus we could use dummy func, and it could make lift simple.

Thanks,
Michael
> Where virtio_rxq_vec_setup() could have a per-arch implementation,
> say for X86, or ARM.
>
> It touchs more code, but for now, I'd like to make it simple first.
> With the virtio_rxtx_simple.c isolated from Makefile, there aren't
> many #ifdef after all.
>
> 	--yliu
>