[dpdk-dev,v7,2/4] virtio: Introduce config RTE_VIRTIO_INC_VECTOR
Commit Message
- 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.
Disabling RTE_VIRTIO_INC_VECTOR config for :
- i686 arch as i686 target config says:
config/defconfig_i686-native-linuxapp-gcc says "Vectorized PMD is not
supported on 32-bit".
- armv7/v8 arch.
Signed-off-by: Santosh Shukla <sshukla@mvista.com>
---
config/common_linuxapp | 1 +
config/defconfig_arm-armv7a-linuxapp-gcc | 4 +++-
config/defconfig_arm64-armv8a-linuxapp-gcc | 4 +++-
config/defconfig_i686-native-linuxapp-gcc | 1 +
config/defconfig_i686-native-linuxapp-icc | 1 +
drivers/net/virtio/Makefile | 2 +-
drivers/net/virtio/virtio_rxtx.c | 16 +++++++++++++++-
drivers/net/virtio/virtio_rxtx.h | 2 ++
8 files changed, 27 insertions(+), 4 deletions(-)
Comments
2016-02-07 19:21, Santosh Shukla:
> - 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.
>
> Disabling RTE_VIRTIO_INC_VECTOR config for :
>
> - i686 arch as i686 target config says:
> config/defconfig_i686-native-linuxapp-gcc says "Vectorized PMD is not
> supported on 32-bit".
>
> - armv7/v8 arch.
Yes it can be useful to disable vector optimizations, but it should done
at runtime, not a compilation option. I know it is already wrongly configured
at compilation for other drivers, we should fix them.
Here, you want to avoid SSE/AVX code on ARM. So we should just add the
appropriate ifdefs. Adding a compilation option does not prevent from enabling
it on ARM or old x86 which do not support these instructions.
Please virtio maintainers, we need to fix this code. Thanks
On Mon, Feb 8, 2016 at 2:55 AM, Thomas Monjalon
<thomas.monjalon@6wind.com> wrote:
> 2016-02-07 19:21, Santosh Shukla:
>> - 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.
>>
>> Disabling RTE_VIRTIO_INC_VECTOR config for :
>>
>> - i686 arch as i686 target config says:
>> config/defconfig_i686-native-linuxapp-gcc says "Vectorized PMD is not
>> supported on 32-bit".
>>
>> - armv7/v8 arch.
>
> Yes it can be useful to disable vector optimizations, but it should done
> at runtime, not a compilation option. I know it is already wrongly configured
> at compilation for other drivers, we should fix them.
>
Can't we consider this separate topic. My intent is virtio works for arm.
> Here, you want to avoid SSE/AVX code on ARM. So we should just add the
> appropriate ifdefs. Adding a compilation option does not prevent from enabling
> it on ARM or old x86 which do not support these instructions.
>
By disabling VIRTIO_INC_VEC, compiler wont build
virtio_recv_pkts_vec(), so wont generate SSE/AVX code. Adding ifdef
for other arch example arm, is next step. Vector instruction for arm
are not fully supported, Its a todolist (Pl. refer my early v1/2
cover-letter), We'll add that after virtio functionally works for arm.
> Please virtio maintainers, we need to fix this code. Thanks
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?
Huawei, since it's your patch introduced such issue, mind to fix
it?
--yliu
>
> Does this make sense?
>
> Thanks
> > --yliu
> >>
> >> Thanks
> >>
> >> On Mon, Feb 8, 2016 at 11:15 AM, Santosh Shukla <sshukla@mvista.com> wrote:
> >> > On Mon, Feb 8, 2016 at 2:55 AM, Thomas Monjalon
> >> > <thomas.monjalon@6wind.com> wrote:
> >> >> 2016-02-07 19:21, Santosh Shukla:
> >> >>> - 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.
> >> >>>
> >> >>> Disabling RTE_VIRTIO_INC_VECTOR config for :
> >> >>>
> >> >>> - i686 arch as i686 target config says:
> >> >>> config/defconfig_i686-native-linuxapp-gcc says "Vectorized PMD is not
> >> >>> supported on 32-bit".
> >> >>>
> >> >>> - armv7/v8 arch.
> >> >>
> >> >> Yes it can be useful to disable vector optimizations, but it should done
> >> >> at runtime, not a compilation option. I know it is already wrongly configured
> >> >> at compilation for other drivers, we should fix them.
> >> >>
> >> >
> >> > Can't we consider this separate topic. My intent is virtio works for arm.
> >> >
> >> >> Here, you want to avoid SSE/AVX code on ARM. So we should just add the
> >> >> appropriate ifdefs. Adding a compilation option does not prevent from enabling
> >> >> it on ARM or old x86 which do not support these instructions.
> >> >>
> >> >
> >> > By disabling VIRTIO_INC_VEC, compiler wont build
> >> > virtio_recv_pkts_vec(), so wont generate SSE/AVX code. Adding ifdef
> >> > for other arch example arm, is next step. Vector instruction for arm
> >> > are not fully supported, Its a todolist (Pl. refer my early v1/2
> >> > cover-letter), We'll add that after virtio functionally works for arm.
> >> >
> >> >> Please virtio maintainers, we need to fix this code. Thanks
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.
This patch is blocking virtio-for-arm entry which is floating close to
month or so, if no taker for this topic then pl. let me know, I'll
propose a patch. Thanks!
>
> Huawei, since it's your patch introduced such issue, mind to fix
> it?
>
> --yliu
>>
>> Does this make sense?
>>
>> Thanks
>> > --yliu
>> >>
>> >> Thanks
>> >>
>> >> On Mon, Feb 8, 2016 at 11:15 AM, Santosh Shukla <sshukla@mvista.com> wrote:
>> >> > On Mon, Feb 8, 2016 at 2:55 AM, Thomas Monjalon
>> >> > <thomas.monjalon@6wind.com> wrote:
>> >> >> 2016-02-07 19:21, Santosh Shukla:
>> >> >>> - 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.
>> >> >>>
>> >> >>> Disabling RTE_VIRTIO_INC_VECTOR config for :
>> >> >>>
>> >> >>> - i686 arch as i686 target config says:
>> >> >>> config/defconfig_i686-native-linuxapp-gcc says "Vectorized PMD is not
>> >> >>> supported on 32-bit".
>> >> >>>
>> >> >>> - armv7/v8 arch.
>> >> >>
>> >> >> Yes it can be useful to disable vector optimizations, but it should done
>> >> >> at runtime, not a compilation option. I know it is already wrongly configured
>> >> >> at compilation for other drivers, we should fix them.
>> >> >>
>> >> >
>> >> > Can't we consider this separate topic. My intent is virtio works for arm.
>> >> >
>> >> >> Here, you want to avoid SSE/AVX code on ARM. So we should just add the
>> >> >> appropriate ifdefs. Adding a compilation option does not prevent from enabling
>> >> >> it on ARM or old x86 which do not support these instructions.
>> >> >>
>> >> >
>> >> > By disabling VIRTIO_INC_VEC, compiler wont build
>> >> > virtio_recv_pkts_vec(), so wont generate SSE/AVX code. Adding ifdef
>> >> > for other arch example arm, is next step. Vector instruction for arm
>> >> > are not fully supported, Its a todolist (Pl. refer my early v1/2
>> >> > cover-letter), We'll add that after virtio functionally works for arm.
>> >> >
>> >> >> Please virtio maintainers, we need to fix this code. Thanks
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
On 2/19/2016 2:42 PM, Yuanhan Liu wrote:
> 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
>
This issue doesn't apply to virtio driver only but to all other PMDs,
unless they are assumed to run on only one arch. As we are close to
release, for the time being, i prefer to use RTE_MACHINE_CPUFLAG_. Later
we look for other elegant solutions, like moving different arch specific
optimizations into the arch directory under driver/virtio/ directory?
Other thoughts?
On Mon, Feb 22, 2016 at 7:33 AM, Xie, Huawei <huawei.xie@intel.com> wrote:
> On 2/19/2016 2:42 PM, Yuanhan Liu wrote:
>> 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
>>
>
> This issue doesn't apply to virtio driver only but to all other PMDs,
> unless they are assumed to run on only one arch. As we are close to
> release, for the time being, i prefer to use RTE_MACHINE_CPUFLAG_. Later
> we look for other elegant solutions, like moving different arch specific
> optimizations into the arch directory under driver/virtio/ directory?
> Other thoughts?
>
Creating arch specifics files in driver/virtio/: approach look okay to
me. It look alike to my proposal except eal. I choose eal so that one
api and its implementation stays in arch files, no ifdef clutter. I
guess - Same doable in virtio directory too, create arch files and
keep arch specific implementation their.
so, +1 to approach.
>
>
2016-02-22 09:44, Santosh Shukla:
> On Mon, Feb 22, 2016 at 7:33 AM, Xie, Huawei <huawei.xie@intel.com> wrote:
> > On 2/19/2016 2:42 PM, Yuanhan Liu wrote:
> >> 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:
> >>>>>> 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.
> >
> > This issue doesn't apply to virtio driver only but to all other PMDs,
> > unless they are assumed to run on only one arch. As we are close to
> > release, for the time being, i prefer to use RTE_MACHINE_CPUFLAG_.
Yes the obvious fix is to use some CPU flags.
In ACL the flags are checked on runtime to allow using some optimizations
after a "default" build. It can be considered later for virtio.
> > Later
> > we look for other elegant solutions, like moving different arch specific
> > optimizations into the arch directory under driver/virtio/ directory?
> > Other thoughts?
>
> Creating arch specifics files in driver/virtio/: approach look okay to
> me. It look alike to my proposal except eal. I choose eal so that one
> api and its implementation stays in arch files, no ifdef clutter. I
> guess - Same doable in virtio directory too, create arch files and
> keep arch specific implementation their.
>
> so, +1 to approach.
If there are some basic functions which can be re-used in other libs, there
must be in EAL. For virtio-specific functions, you can have some arch-specific
files in virtio.
@@ -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
@@ -43,6 +43,9 @@ CONFIG_RTE_FORCE_INTRINSICS=y
CONFIG_RTE_TOOLCHAIN="gcc"
CONFIG_RTE_TOOLCHAIN_GCC=y
+# Disable VIRTIO VECTOR support
+CONFIG_RTE_VIRTIO_INC_VECTOR=n
+
# ARM doesn't have support for vmware TSC map
CONFIG_RTE_LIBRTE_EAL_VMWARE_TSC_MAP_SUPPORT=n
@@ -70,7 +73,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
@@ -45,8 +45,10 @@ CONFIG_RTE_TOOLCHAIN_GCC=y
CONFIG_RTE_CACHE_LINE_SIZE=64
+# Disable VIRTIO VECTOR support
+CONFIG_RTE_VIRTIO_INC_VECTOR=n
+
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
@@ -49,3 +49,4 @@ CONFIG_RTE_LIBRTE_KNI=n
# Vectorized PMD is not supported on 32-bit
#
CONFIG_RTE_IXGBE_INC_VECTOR=n
+CONFIG_RTE_VIRTIO_INC_VECTOR=n
@@ -49,3 +49,4 @@ CONFIG_RTE_LIBRTE_KNI=n
# Vectorized PMD is not supported on 32-bit
#
CONFIG_RTE_IXGBE_INC_VECTOR=n
+CONFIG_RTE_VIRTIO_INC_VECTOR=n
@@ -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
@@ -67,7 +67,9 @@
#define VIRTIO_SIMPLE_FLAGS ((uint32_t)ETH_TXQ_FLAGS_NOMULTSEGS | \
ETH_TXQ_FLAGS_NOOFFLOADS)
+#ifdef RTE_VIRTIO_INC_VECTOR
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_VIRTIO_INC_VECTOR
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_VIRTIO_INC_VECTOR
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_VIRTIO_INC_VECTOR
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_VIRTIO_INC_VECTOR
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_VIRTIO_INC_VECTOR
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_VIRTIO_INC_VECTOR
/* 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);
@@ -33,7 +33,9 @@
#define RTE_PMD_VIRTIO_RX_MAX_BURST 64
+#ifdef RTE_VIRTIO_INC_VECTOR
int virtio_rxq_vec_setup(struct virtqueue *rxq);
int virtqueue_enqueue_recv_refill_simple(struct virtqueue *vq,
struct rte_mbuf *m);
+#endif