Re: [dpdk-dev] [PATCH v7 2/4] virtio: Introduce config RTE_VIRTIO_INC_VECTOR

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

On Fri, Feb 19, 2016 at 10:16:42AM +0530, Santosh Shukla wrote:
> On Tue, Feb 16, 2016 at 8:35 AM, Yuanhan Liu
> <yuanhan.liu@linux.intel.com> wrote:
> > On Mon, Feb 15, 2016 at 04:48:36PM +0530, Santosh Shukla wrote:
> >> Hi Yuanhan,
> >>
> >> On Mon, Feb 15, 2016 at 4:27 PM, Yuanhan Liu
> >> <yuanhan.liu@linux.intel.com> wrote:
> >> > On Mon, Feb 15, 2016 at 03:22:11PM +0530, Santosh Shukla wrote:
> >> >> Hi Yuanhan,
> >> >>
> >> >> I guess you are back from vacation.
> >> >>
> >> >> Can you pl. review this patch, Except this patch, rest of patches
> >> >> received ack-by:
> >> >
> >> > I had a quick glimpse of the comments from Thomas: he made a good point.
> >> > I will have a deeper thought tomorrow, to see what I can do to fix it.
> >> >
> >>
> >> I agree to what Thomas pointed out about runtime mode switch (vectored
> >> vs non-vectored). I have a proposal in my mind and Like to know you
> >> opinion:
> >>
> >> - need for apis like is_arch_support_vec().
> >>
> >> if (is_arch_support_vec())
> >>          simpple_xxxx = 1 /* Switch code path to vector mode */
> >> else
> >>          simple_xxxx = 0  /* Switch code path to non-vector mode */
> >>
> >> That api should reside to arch file. i.e.. arch like i686/arm{for
> >> implementation not exist so say no supported} will return 0 and for
> >> x86_64 = 1
> >
> > I was thinking that Thomas meant to something like below (like what
> > we did at rte_memcpy.h):
> >
> >     #ifdef RTE_MACHINE_CPUFLAG_SSE (or whatever)
> >
> >         /* with vec here */
> >
> >     #else
> >
> >         /* without vec here */
> >
> >     #endif
> >
> > I mean, you have to bypass the build first; otherwise, you can't
> > go that further to runtime, right?
> >
> 
> I meant: move virtio_recv_pkt_vec() implementation in
> lib/libeal_rte/xx/include/arch/xx/virtio_vec.h. virtio driver to check
> for CPUFLAG supported or not and then use _recv_pkt() call back
> function from arch files. This approach will avoid #ifdef ARCH
> clutter.

Moving virtio stuff to eal looks wrong to me.

Huawei, please comment on this.

	--yliu