From mboxrd@z Thu Jan  1 00:00:00 1970
From: Yuanhan Liu <yuanhan.liu@linux.intel.com>
Subject: Re: [PATCH v1] virtio: Use cpuflag for vector api
Date: Tue, 1 Mar 2016 13:55:08 +0800
Message-ID: <20160301055508.GL14300@yliu-dev.sh.intel.com>
References: <1456476662-23081-1-git-send-email-sshukla@mvista.com>
 <20160229042721.GJ14300@yliu-dev.sh.intel.com>
 <CAAyOgsZcug46hyS331W81Hj7gHxMU2xwCqhYWzgYY99cPYc=0Q@mail.gmail.com>
Mime-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Cc: dpdk <dev@dpdk.org>
To: Santosh Shukla <sshukla@mvista.com>
Return-path: <dev-bounces@dpdk.org>
Received: from mga01.intel.com (mga01.intel.com [192.55.52.88])
 by dpdk.org (Postfix) with ESMTP id 36DCC11C5
 for <dev@dpdk.org>; Tue,  1 Mar 2016 06:54:25 +0100 (CET)
Content-Disposition: inline
In-Reply-To: <CAAyOgsZcug46hyS331W81Hj7gHxMU2xwCqhYWzgYY99cPYc=0Q@mail.gmail.com>
List-Id: patches and discussions about DPDK <dev.dpdk.org>
List-Unsubscribe: <http://dpdk.org/ml/options/dev>,
 <mailto:dev-request@dpdk.org?subject=unsubscribe>
List-Archive: <http://dpdk.org/ml/archives/dev/>
List-Post: <mailto:dev@dpdk.org>
List-Help: <mailto:dev-request@dpdk.org?subject=help>
List-Subscribe: <http://dpdk.org/ml/listinfo/dev>,
 <mailto:dev-request@dpdk.org?subject=subscribe>
Errors-To: dev-bounces@dpdk.org
Sender: "dev" <dev-bounces@dpdk.org>

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'