[v6,2/9] net/virtio: enable vectorized path

Message ID 20200416222431.114184-3-yong.liu@intel.com (mailing list archive)
State Superseded, archived
Delegated to: Maxime Coquelin
Headers
Series add packed ring vectorized path |

Checks

Context Check Description
ci/checkpatch success coding style OK
ci/Intel-compilation success Compilation OK

Commit Message

Marvin Liu April 16, 2020, 10:24 p.m. UTC
  Previously, virtio split ring vectorized path is enabled as default.
This is not suitable for everyone because of that path not follow virtio
spec. Add new config for virtio vectorized path selection. By default
vectorized path is enabled.

Signed-off-by: Marvin Liu <yong.liu@intel.com>
  

Comments

Maxime Coquelin April 20, 2020, 2:08 p.m. UTC | #1
Hi Marvin,

On 4/17/20 12:24 AM, Marvin Liu wrote:
> Previously, virtio split ring vectorized path is enabled as default.
> This is not suitable for everyone because of that path not follow virtio
> spec. Add new config for virtio vectorized path selection. By default
> vectorized path is enabled.

It should be disabled by default if not following spec. Also, it means
it will always be enabled with Meson, which is not acceptable.

I think we should have a devarg, so that it is built by default but
disabled. User would specify explicitly he wants to enable vector
support when probing the device.

Thanks,
Maxime

> Signed-off-by: Marvin Liu <yong.liu@intel.com>
> 
> diff --git a/config/common_base b/config/common_base
> index c31175f9d..5901a94f7 100644
> --- a/config/common_base
> +++ b/config/common_base
> @@ -449,6 +449,7 @@ CONFIG_RTE_LIBRTE_VIRTIO_PMD=y
>  CONFIG_RTE_LIBRTE_VIRTIO_DEBUG_RX=n
>  CONFIG_RTE_LIBRTE_VIRTIO_DEBUG_TX=n
>  CONFIG_RTE_LIBRTE_VIRTIO_DEBUG_DUMP=n
> +CONFIG_RTE_LIBRTE_VIRTIO_INC_VECTOR=y
>  
>  #
>  # Compile virtio device emulation inside virtio PMD driver
> diff --git a/drivers/net/virtio/Makefile b/drivers/net/virtio/Makefile
> index efdcb0d93..9ef445bc9 100644
> --- a/drivers/net/virtio/Makefile
> +++ b/drivers/net/virtio/Makefile
> @@ -29,6 +29,7 @@ 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
>  
> +ifeq ($(CONFIG_RTE_LIBRTE_VIRTIO_INC_VECTOR),y)
>  ifeq ($(CONFIG_RTE_ARCH_X86),y)
>  SRCS-$(CONFIG_RTE_LIBRTE_VIRTIO_PMD) += virtio_rxtx_simple_sse.c
>  else ifeq ($(CONFIG_RTE_ARCH_PPC_64),y)
> @@ -36,6 +37,7 @@ SRCS-$(CONFIG_RTE_LIBRTE_VIRTIO_PMD) += virtio_rxtx_simple_altivec.c
>  else ifneq ($(filter y,$(CONFIG_RTE_ARCH_ARM) $(CONFIG_RTE_ARCH_ARM64)),)
>  SRCS-$(CONFIG_RTE_LIBRTE_VIRTIO_PMD) += virtio_rxtx_simple_neon.c
>  endif
> +endif
>  
>  ifeq ($(CONFIG_RTE_VIRTIO_USER),y)
>  SRCS-$(CONFIG_RTE_LIBRTE_VIRTIO_PMD) += virtio_user/vhost_user.c
> diff --git a/drivers/net/virtio/meson.build b/drivers/net/virtio/meson.build
> index 5e7ca855c..f9619a108 100644
> --- a/drivers/net/virtio/meson.build
> +++ b/drivers/net/virtio/meson.build
> @@ -9,12 +9,14 @@ sources += files('virtio_ethdev.c',
>  	'virtqueue.c')
>  deps += ['kvargs', 'bus_pci']
>  
> -if arch_subdir == 'x86'
> -	sources += files('virtio_rxtx_simple_sse.c')
> -elif arch_subdir == 'ppc'
> -	sources += files('virtio_rxtx_simple_altivec.c')
> -elif arch_subdir == 'arm' and host_machine.cpu_family().startswith('aarch64')
> -	sources += files('virtio_rxtx_simple_neon.c')
> +if dpdk_conf.has('RTE_LIBRTE_VIRTIO_INC_VECTOR')
> +	if arch_subdir == 'x86'
> +		sources += files('virtio_rxtx_simple_sse.c')
> +	elif arch_subdir == 'ppc'
> +		sources += files('virtio_rxtx_simple_altivec.c')
> +	elif arch_subdir == 'arm' and host_machine.cpu_family().startswith('aarch64')
> +		sources += files('virtio_rxtx_simple_neon.c')
> +	endif
>  endif
>  
>  if is_linux
>
  
Marvin Liu April 21, 2020, 6:43 a.m. UTC | #2
> -----Original Message-----
> From: Maxime Coquelin <maxime.coquelin@redhat.com>
> Sent: Monday, April 20, 2020 10:08 PM
> To: Liu, Yong <yong.liu@intel.com>; Ye, Xiaolong <xiaolong.ye@intel.com>;
> Wang, Zhihong <zhihong.wang@intel.com>
> Cc: dev@dpdk.org
> Subject: Re: [PATCH v6 2/9] net/virtio: enable vectorized path
> 
> Hi Marvin,
> 
> On 4/17/20 12:24 AM, Marvin Liu wrote:
> > Previously, virtio split ring vectorized path is enabled as default.
> > This is not suitable for everyone because of that path not follow virtio
> > spec. Add new config for virtio vectorized path selection. By default
> > vectorized path is enabled.
> 
> It should be disabled by default if not following spec. Also, it means
> it will always be enabled with Meson, which is not acceptable.
> 
> I think we should have a devarg, so that it is built by default but
> disabled. User would specify explicitly he wants to enable vector
> support when probing the device.
> 

Thanks, Maxime. Will change to disable as default in next version.

> Thanks,
> Maxime
> 
> > Signed-off-by: Marvin Liu <yong.liu@intel.com>
> >
> > diff --git a/config/common_base b/config/common_base
> > index c31175f9d..5901a94f7 100644
> > --- a/config/common_base
> > +++ b/config/common_base
> > @@ -449,6 +449,7 @@ CONFIG_RTE_LIBRTE_VIRTIO_PMD=y
> >  CONFIG_RTE_LIBRTE_VIRTIO_DEBUG_RX=n
> >  CONFIG_RTE_LIBRTE_VIRTIO_DEBUG_TX=n
> >  CONFIG_RTE_LIBRTE_VIRTIO_DEBUG_DUMP=n
> > +CONFIG_RTE_LIBRTE_VIRTIO_INC_VECTOR=y
> >
> >  #
> >  # Compile virtio device emulation inside virtio PMD driver
> > diff --git a/drivers/net/virtio/Makefile b/drivers/net/virtio/Makefile
> > index efdcb0d93..9ef445bc9 100644
> > --- a/drivers/net/virtio/Makefile
> > +++ b/drivers/net/virtio/Makefile
> > @@ -29,6 +29,7 @@ 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
> >
> > +ifeq ($(CONFIG_RTE_LIBRTE_VIRTIO_INC_VECTOR),y)
> >  ifeq ($(CONFIG_RTE_ARCH_X86),y)
> >  SRCS-$(CONFIG_RTE_LIBRTE_VIRTIO_PMD) += virtio_rxtx_simple_sse.c
> >  else ifeq ($(CONFIG_RTE_ARCH_PPC_64),y)
> > @@ -36,6 +37,7 @@ SRCS-$(CONFIG_RTE_LIBRTE_VIRTIO_PMD) +=
> virtio_rxtx_simple_altivec.c
> >  else ifneq ($(filter y,$(CONFIG_RTE_ARCH_ARM)
> $(CONFIG_RTE_ARCH_ARM64)),)
> >  SRCS-$(CONFIG_RTE_LIBRTE_VIRTIO_PMD) += virtio_rxtx_simple_neon.c
> >  endif
> > +endif
> >
> >  ifeq ($(CONFIG_RTE_VIRTIO_USER),y)
> >  SRCS-$(CONFIG_RTE_LIBRTE_VIRTIO_PMD) += virtio_user/vhost_user.c
> > diff --git a/drivers/net/virtio/meson.build
> b/drivers/net/virtio/meson.build
> > index 5e7ca855c..f9619a108 100644
> > --- a/drivers/net/virtio/meson.build
> > +++ b/drivers/net/virtio/meson.build
> > @@ -9,12 +9,14 @@ sources += files('virtio_ethdev.c',
> >  	'virtqueue.c')
> >  deps += ['kvargs', 'bus_pci']
> >
> > -if arch_subdir == 'x86'
> > -	sources += files('virtio_rxtx_simple_sse.c')
> > -elif arch_subdir == 'ppc'
> > -	sources += files('virtio_rxtx_simple_altivec.c')
> > -elif arch_subdir == 'arm' and
> host_machine.cpu_family().startswith('aarch64')
> > -	sources += files('virtio_rxtx_simple_neon.c')
> > +if dpdk_conf.has('RTE_LIBRTE_VIRTIO_INC_VECTOR')
> > +	if arch_subdir == 'x86'
> > +		sources += files('virtio_rxtx_simple_sse.c')
> > +	elif arch_subdir == 'ppc'
> > +		sources += files('virtio_rxtx_simple_altivec.c')
> > +	elif arch_subdir == 'arm' and
> host_machine.cpu_family().startswith('aarch64')
> > +		sources += files('virtio_rxtx_simple_neon.c')
> > +	endif
> >  endif
> >
> >  if is_linux
> >
  
Marvin Liu April 22, 2020, 8:07 a.m. UTC | #3
> -----Original Message-----
> From: Liu, Yong
> Sent: Tuesday, April 21, 2020 2:43 PM
> To: 'Maxime Coquelin' <maxime.coquelin@redhat.com>; Ye, Xiaolong
> <xiaolong.ye@intel.com>; Wang, Zhihong <zhihong.wang@intel.com>
> Cc: dev@dpdk.org
> Subject: RE: [PATCH v6 2/9] net/virtio: enable vectorized path
> 
> 
> 
> > -----Original Message-----
> > From: Maxime Coquelin <maxime.coquelin@redhat.com>
> > Sent: Monday, April 20, 2020 10:08 PM
> > To: Liu, Yong <yong.liu@intel.com>; Ye, Xiaolong <xiaolong.ye@intel.com>;
> > Wang, Zhihong <zhihong.wang@intel.com>
> > Cc: dev@dpdk.org
> > Subject: Re: [PATCH v6 2/9] net/virtio: enable vectorized path
> >
> > Hi Marvin,
> >
> > On 4/17/20 12:24 AM, Marvin Liu wrote:
> > > Previously, virtio split ring vectorized path is enabled as default.
> > > This is not suitable for everyone because of that path not follow virtio
> > > spec. Add new config for virtio vectorized path selection. By default
> > > vectorized path is enabled.
> >
> > It should be disabled by default if not following spec. Also, it means
> > it will always be enabled with Meson, which is not acceptable.
> >
> > I think we should have a devarg, so that it is built by default but
> > disabled. User would specify explicitly he wants to enable vector
> > support when probing the device.
> >
> 

Hi Maxime,
There's one new parameter "vectorized" in devarg which allow user specific whether enable or disable vectorized path.
By now this parameter depend on RTE_LIBRTE_VIRTIO_INC_VECTOR,  parameter won't be used if INC_VECTOR option is disable. 

Regards,
Marvin

> Thanks, Maxime. Will change to disable as default in next version.
> 
> > Thanks,
> > Maxime
> >
> > > Signed-off-by: Marvin Liu <yong.liu@intel.com>
> > >
> > > diff --git a/config/common_base b/config/common_base
> > > index c31175f9d..5901a94f7 100644
> > > --- a/config/common_base
> > > +++ b/config/common_base
> > > @@ -449,6 +449,7 @@ CONFIG_RTE_LIBRTE_VIRTIO_PMD=y
> > >  CONFIG_RTE_LIBRTE_VIRTIO_DEBUG_RX=n
> > >  CONFIG_RTE_LIBRTE_VIRTIO_DEBUG_TX=n
> > >  CONFIG_RTE_LIBRTE_VIRTIO_DEBUG_DUMP=n
> > > +CONFIG_RTE_LIBRTE_VIRTIO_INC_VECTOR=y
> > >
> > >  #
> > >  # Compile virtio device emulation inside virtio PMD driver
> > > diff --git a/drivers/net/virtio/Makefile b/drivers/net/virtio/Makefile
> > > index efdcb0d93..9ef445bc9 100644
> > > --- a/drivers/net/virtio/Makefile
> > > +++ b/drivers/net/virtio/Makefile
> > > @@ -29,6 +29,7 @@ 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
> > >
> > > +ifeq ($(CONFIG_RTE_LIBRTE_VIRTIO_INC_VECTOR),y)
> > >  ifeq ($(CONFIG_RTE_ARCH_X86),y)
> > >  SRCS-$(CONFIG_RTE_LIBRTE_VIRTIO_PMD) += virtio_rxtx_simple_sse.c
> > >  else ifeq ($(CONFIG_RTE_ARCH_PPC_64),y)
> > > @@ -36,6 +37,7 @@ SRCS-$(CONFIG_RTE_LIBRTE_VIRTIO_PMD) +=
> > virtio_rxtx_simple_altivec.c
> > >  else ifneq ($(filter y,$(CONFIG_RTE_ARCH_ARM)
> > $(CONFIG_RTE_ARCH_ARM64)),)
> > >  SRCS-$(CONFIG_RTE_LIBRTE_VIRTIO_PMD) += virtio_rxtx_simple_neon.c
> > >  endif
> > > +endif
> > >
> > >  ifeq ($(CONFIG_RTE_VIRTIO_USER),y)
> > >  SRCS-$(CONFIG_RTE_LIBRTE_VIRTIO_PMD) += virtio_user/vhost_user.c
> > > diff --git a/drivers/net/virtio/meson.build
> > b/drivers/net/virtio/meson.build
> > > index 5e7ca855c..f9619a108 100644
> > > --- a/drivers/net/virtio/meson.build
> > > +++ b/drivers/net/virtio/meson.build
> > > @@ -9,12 +9,14 @@ sources += files('virtio_ethdev.c',
> > >  	'virtqueue.c')
> > >  deps += ['kvargs', 'bus_pci']
> > >
> > > -if arch_subdir == 'x86'
> > > -	sources += files('virtio_rxtx_simple_sse.c')
> > > -elif arch_subdir == 'ppc'
> > > -	sources += files('virtio_rxtx_simple_altivec.c')
> > > -elif arch_subdir == 'arm' and
> > host_machine.cpu_family().startswith('aarch64')
> > > -	sources += files('virtio_rxtx_simple_neon.c')
> > > +if dpdk_conf.has('RTE_LIBRTE_VIRTIO_INC_VECTOR')
> > > +	if arch_subdir == 'x86'
> > > +		sources += files('virtio_rxtx_simple_sse.c')
> > > +	elif arch_subdir == 'ppc'
> > > +		sources += files('virtio_rxtx_simple_altivec.c')
> > > +	elif arch_subdir == 'arm' and
> > host_machine.cpu_family().startswith('aarch64')
> > > +		sources += files('virtio_rxtx_simple_neon.c')
> > > +	endif
> > >  endif
> > >
> > >  if is_linux
> > >
  

Patch

diff --git a/config/common_base b/config/common_base
index c31175f9d..5901a94f7 100644
--- a/config/common_base
+++ b/config/common_base
@@ -449,6 +449,7 @@  CONFIG_RTE_LIBRTE_VIRTIO_PMD=y
 CONFIG_RTE_LIBRTE_VIRTIO_DEBUG_RX=n
 CONFIG_RTE_LIBRTE_VIRTIO_DEBUG_TX=n
 CONFIG_RTE_LIBRTE_VIRTIO_DEBUG_DUMP=n
+CONFIG_RTE_LIBRTE_VIRTIO_INC_VECTOR=y
 
 #
 # Compile virtio device emulation inside virtio PMD driver
diff --git a/drivers/net/virtio/Makefile b/drivers/net/virtio/Makefile
index efdcb0d93..9ef445bc9 100644
--- a/drivers/net/virtio/Makefile
+++ b/drivers/net/virtio/Makefile
@@ -29,6 +29,7 @@  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
 
+ifeq ($(CONFIG_RTE_LIBRTE_VIRTIO_INC_VECTOR),y)
 ifeq ($(CONFIG_RTE_ARCH_X86),y)
 SRCS-$(CONFIG_RTE_LIBRTE_VIRTIO_PMD) += virtio_rxtx_simple_sse.c
 else ifeq ($(CONFIG_RTE_ARCH_PPC_64),y)
@@ -36,6 +37,7 @@  SRCS-$(CONFIG_RTE_LIBRTE_VIRTIO_PMD) += virtio_rxtx_simple_altivec.c
 else ifneq ($(filter y,$(CONFIG_RTE_ARCH_ARM) $(CONFIG_RTE_ARCH_ARM64)),)
 SRCS-$(CONFIG_RTE_LIBRTE_VIRTIO_PMD) += virtio_rxtx_simple_neon.c
 endif
+endif
 
 ifeq ($(CONFIG_RTE_VIRTIO_USER),y)
 SRCS-$(CONFIG_RTE_LIBRTE_VIRTIO_PMD) += virtio_user/vhost_user.c
diff --git a/drivers/net/virtio/meson.build b/drivers/net/virtio/meson.build
index 5e7ca855c..f9619a108 100644
--- a/drivers/net/virtio/meson.build
+++ b/drivers/net/virtio/meson.build
@@ -9,12 +9,14 @@  sources += files('virtio_ethdev.c',
 	'virtqueue.c')
 deps += ['kvargs', 'bus_pci']
 
-if arch_subdir == 'x86'
-	sources += files('virtio_rxtx_simple_sse.c')
-elif arch_subdir == 'ppc'
-	sources += files('virtio_rxtx_simple_altivec.c')
-elif arch_subdir == 'arm' and host_machine.cpu_family().startswith('aarch64')
-	sources += files('virtio_rxtx_simple_neon.c')
+if dpdk_conf.has('RTE_LIBRTE_VIRTIO_INC_VECTOR')
+	if arch_subdir == 'x86'
+		sources += files('virtio_rxtx_simple_sse.c')
+	elif arch_subdir == 'ppc'
+		sources += files('virtio_rxtx_simple_altivec.c')
+	elif arch_subdir == 'arm' and host_machine.cpu_family().startswith('aarch64')
+		sources += files('virtio_rxtx_simple_neon.c')
+	endif
 endif
 
 if is_linux