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

[Yuanhan Liu <yuanhan.liu@linux.intel.com>]

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'