diff mbox series

[v16,1/3] build: disable/enable drivers in Arm builds

Message ID 1612361037-12746-2-git-send-email-juraj.linkes@pantheon.tech (mailing list archive)
State Superseded
Delegated to: Thomas Monjalon
Headers show
Series Arm build options rework | expand

Checks

Context Check Description
ci/checkpatch warning coding style issues

Commit Message

Juraj Linkeš Feb. 3, 2021, 2:03 p.m. UTC
Add support for enabling or disabling drivers for Arm cross build. Do
not implement any enable/disable lists yet.

Enabling drivers is useful when building for an SoC where we only want
to build a few drivers. That way the list won't be too long.

Similarly, disabling drivers is useful when we want to disable only a
few drivers.

Both of these are advantageous mainly in aarch64 -> aarch64 (or arch ->
same arch) builds, where the build machine may have the required driver
dependencies, yet we don't want to build drivers for a speficic SoC.

By default, build all drivers for which dependencies are found. If
enabled_drivers is a non-empty list, build only those drivers.  If
disabled_drivers is non-empty list, build all drivers except those in
disabled_drivers. Error out if both are specified (i.e. do not support
that case).

Remove the old Makefile arm configutaion options which don't do anything
in Meson.

Signed-off-by: Juraj Linkeš <juraj.linkes@pantheon.tech>
Acked-by: Bruce Richardson <bruce.richardson@intel.com>
Reviewed-by: Honnappa Nagarahalli <honnappa.nagarahalli@arm.com>
---
 config/arm/meson.build                        |  4 --
 .../linux_gsg/cross_build_dpdk_for_arm64.rst  |  8 ++++
 drivers/meson.build                           | 41 ++++++++++++++++---
 meson.build                                   |  2 +
 4 files changed, 46 insertions(+), 9 deletions(-)

Comments

Juraj Linkeš Feb. 19, 2021, 10:38 a.m. UTC | #1
I believe this is the last patch in the series that needs some more review. I implemented both an allowlist and a blocklist, so please let me know if this would be usable. There's only the implementation, the actual allow/blocklists would have to be added by maintainers/SoC owners.

There's also an open question of whether we want to have the allow/blocklist be exclusive (only one of them supported at a time) or both could be used at the same time. More in TODO/QUERY below.

> ---
>  config/arm/meson.build                        |  4 --
>  .../linux_gsg/cross_build_dpdk_for_arm64.rst  |  8 ++++
>  drivers/meson.build                           | 41 ++++++++++++++++---
>  meson.build                                   |  2 +
>  4 files changed, 46 insertions(+), 9 deletions(-)
> 
> diff --git a/config/arm/meson.build b/config/arm/meson.build index
> f948768578..d279724dec 100644
> --- a/config/arm/meson.build
> +++ b/config/arm/meson.build
> @@ -16,9 +16,6 @@ flags_common = [
>  	#	['RTE_ARM64_MEMCPY_ALIGN_MASK', 0xF],
>  	#	['RTE_ARM64_MEMCPY_STRICT_ALIGN', false],
> 
> -	['RTE_NET_FM10K', false],
> -	['RTE_NET_AVP', false],
> -
>  	['RTE_SCHED_VECTOR', false],
>  	['RTE_ARM_USE_WFE', false],
>  	['RTE_ARCH_ARM64', true],
> @@ -125,7 +122,6 @@ implementer_cavium = {
>  				['RTE_MACHINE', '"octeontx2"'],
>  				['RTE_ARM_FEATURE_ATOMICS', true],
>  				['RTE_USE_C11_MEM_MODEL', true],
> -				['RTE_EAL_IGB_UIO', false],
>  				['RTE_MAX_LCORE', 36],
>  				['RTE_MAX_NUMA_NODES', 1]
>  			]
> diff --git a/doc/guides/linux_gsg/cross_build_dpdk_for_arm64.rst
> b/doc/guides/linux_gsg/cross_build_dpdk_for_arm64.rst
> index faaf24b95b..1504dbfef0 100644
> --- a/doc/guides/linux_gsg/cross_build_dpdk_for_arm64.rst
> +++ b/doc/guides/linux_gsg/cross_build_dpdk_for_arm64.rst
> @@ -234,3 +234,11 @@ There are other options you may specify in a cross file
> to tailor the build::
>        numa = false        # set to false to force building for a non-NUMA system
>           # if not set or set to true, the build system will build for a NUMA
>           # system only if libnuma is installed
> +
> +      disabled_drivers = ['bus/dpaa', 'crypto/*']  # add disabled drivers
> +         # valid values are dir/subdirs in the drivers directory
> +         # wildcards are allowed
> +
> +      enabled_drivers = ['common/*', 'bus/*']  # build only these drivers
> +         # valid values are dir/subdirs in the drivers directory
> +         # wildcards are allowed
> diff --git a/drivers/meson.build b/drivers/meson.build index
> fdf76120ac..70c1aa4e6c 100644
> --- a/drivers/meson.build
> +++ b/drivers/meson.build
> @@ -18,8 +18,36 @@ subdirs = [
>  	'baseband', # depends on common and bus.
>  ]
> 
> -disabled_drivers = run_command(list_dir_globs, get_option('disable_drivers'),
> -		).stdout().split()
> +if meson.is_cross_build()
> +	disabled_drivers += meson.get_cross_property('disabled_drivers', [])
> +	enabled_drivers += meson.get_cross_property('enabled_drivers', [])
> +endif
> +
> +# add cmdline disabled drivers (comma separated string) # and meson
> +disabled drivers (list) # together into a comma separated string
> +disabled_drivers = ','.join([get_option('disable_drivers'),
> +','.join(disabled_drivers)]).strip(',')
> +if disabled_drivers != ''
> +	disabled_drivers = run_command(list_dir_globs,
> +		disabled_drivers).stdout().split()
> +else
> +	disabled_drivers = []
> +endif
> +
> +if enabled_drivers != []
> +	enabled_drivers = run_command(list_dir_globs,
> +		','.join(enabled_drivers)).stdout().split()
> +endif
> +
> +if disabled_drivers != [] and enabled_drivers != []
> +	# TODO/QUERY we could support both:
> +	# first 'select' only drivers by enabled_drivers
> +	# then 'deselect' those in disabled_drivers
> +	# this would be useful if a directory is in enabled_drivers
> +	# and a driver from that directory is in disabled_drivers
> +	error('Simultaneous disabled drivers and enabled drivers ' +
> +	      'configuration is not supported.') endif
> 
>  default_cflags = machine_args
>  default_cflags += ['-DALLOW_EXPERIMENTAL_API'] @@ -48,7 +76,7 @@
> foreach subpath:subdirs
>  		dpdk_driver_classes += class
>  	endif
>  	# get already enabled drivers of the same class
> -	enabled_drivers = get_variable(class + '_drivers', [])
> +	enabled_class_drivers = get_variable(class + '_drivers', [])
> 
>  	foreach drv:drivers
>  		drv_path = join_paths(class, drv)
> @@ -76,6 +104,9 @@ foreach subpath:subdirs
>  		if disabled_drivers.contains(drv_path)
>  			build = false
>  			reason = 'explicitly disabled via build config'
> +		elif enabled_drivers.length() > 0 and not
> enabled_drivers.contains(drv_path)
> +			build = false
> +			reason = 'not in enabled drivers build config'
>  		else
>  			# pull in driver directory which should update all the
> local variables
>  			subdir(drv_path)
> @@ -108,7 +139,7 @@ foreach subpath:subdirs
>  						'_disable_reason', reason)
>  			endif
>  		else
> -			enabled_drivers += name
> +			enabled_class_drivers += name
>  			lib_name = '_'.join(['rte', class, name])
>  			dpdk_conf.set(lib_name.to_upper(), 1)
> 
> @@ -213,5 +244,5 @@ foreach subpath:subdirs
>  		endif # build
>  	endforeach
> 
> -	set_variable(class + '_drivers', enabled_drivers)
> +	set_variable(class + '_drivers', enabled_class_drivers)
>  endforeach
> diff --git a/meson.build b/meson.build
> index fcc4d4c900..ea7ccfdae3 100644
> --- a/meson.build
> +++ b/meson.build
> @@ -22,6 +22,8 @@ dpdk_drivers = []
>  dpdk_extra_ldflags = []
>  dpdk_libs_disabled = []
>  dpdk_drvs_disabled = []
> +disabled_drivers = []
> +enabled_drivers = []
>  abi_version_file = files('ABI_VERSION')
> 
>  if host_machine.cpu_family().startswith('x86')
> --
> 2.20.1
Juraj Linkeš March 9, 2021, 8:58 a.m. UTC | #2
Honnappa, Thomas, Bruce, Jerin, you've comments in the past. Do you have any further input?

I think we just need to agree on the allowlist/blocklist mechanism. The current commit allows specifying either an allowlist or a blocklist, but not both. However, it would possible to implement specifying both - first we'll allow what's in allowlist and then we'll remove from that set what's in blocklist. Thoughts?

Note that among the three commits remaining in this series, this commit is the only one that seems to be lacking in review. Please consider reviewing ASAP so that we can finally finish the series.

Thanks,
Juraj

> -----Original Message-----
> From: Juraj Linkeš <juraj.linkes@pantheon.tech>
> Sent: Friday, February 19, 2021 11:39 AM
> To: Juraj Linkeš <juraj.linkes@pantheon.tech>; bruce.richardson@intel.com;
> Ruifeng.Wang@arm.com; Honnappa.Nagarahalli@arm.com;
> Phil.Yang@arm.com; vcchunga@amazon.com; Dharmik.Thakkar@arm.com;
> jerinjacobk@gmail.com; hemant.agrawal@nxp.com;
> ajit.khaparde@broadcom.com; ferruh.yigit@intel.com; aboyer@pensando.io
> Cc: dev@dpdk.org
> Subject: RE: [PATCH v16 1/3] build: disable/enable drivers in Arm builds
> 
> I believe this is the last patch in the series that needs some more review. I
> implemented both an allowlist and a blocklist, so please let me know if this
> would be usable. There's only the implementation, the actual allow/blocklists
> would have to be added by maintainers/SoC owners.
> 
> There's also an open question of whether we want to have the allow/blocklist
> be exclusive (only one of them supported at a time) or both could be used at the
> same time. More in TODO/QUERY below.
> 
> > ---
> >  config/arm/meson.build                        |  4 --
> >  .../linux_gsg/cross_build_dpdk_for_arm64.rst  |  8 ++++
> >  drivers/meson.build                           | 41 ++++++++++++++++---
> >  meson.build                                   |  2 +
> >  4 files changed, 46 insertions(+), 9 deletions(-)
> >
> > diff --git a/config/arm/meson.build b/config/arm/meson.build index
> > f948768578..d279724dec 100644
> > --- a/config/arm/meson.build
> > +++ b/config/arm/meson.build
> > @@ -16,9 +16,6 @@ flags_common = [
> >  	#	['RTE_ARM64_MEMCPY_ALIGN_MASK', 0xF],
> >  	#	['RTE_ARM64_MEMCPY_STRICT_ALIGN', false],
> >
> > -	['RTE_NET_FM10K', false],
> > -	['RTE_NET_AVP', false],
> > -
> >  	['RTE_SCHED_VECTOR', false],
> >  	['RTE_ARM_USE_WFE', false],
> >  	['RTE_ARCH_ARM64', true],
> > @@ -125,7 +122,6 @@ implementer_cavium = {
> >  				['RTE_MACHINE', '"octeontx2"'],
> >  				['RTE_ARM_FEATURE_ATOMICS', true],
> >  				['RTE_USE_C11_MEM_MODEL', true],
> > -				['RTE_EAL_IGB_UIO', false],
> >  				['RTE_MAX_LCORE', 36],
> >  				['RTE_MAX_NUMA_NODES', 1]
> >  			]
> > diff --git a/doc/guides/linux_gsg/cross_build_dpdk_for_arm64.rst
> > b/doc/guides/linux_gsg/cross_build_dpdk_for_arm64.rst
> > index faaf24b95b..1504dbfef0 100644
> > --- a/doc/guides/linux_gsg/cross_build_dpdk_for_arm64.rst
> > +++ b/doc/guides/linux_gsg/cross_build_dpdk_for_arm64.rst
> > @@ -234,3 +234,11 @@ There are other options you may specify in a
> > cross file to tailor the build::
> >        numa = false        # set to false to force building for a non-NUMA system
> >           # if not set or set to true, the build system will build for a NUMA
> >           # system only if libnuma is installed
> > +
> > +      disabled_drivers = ['bus/dpaa', 'crypto/*']  # add disabled drivers
> > +         # valid values are dir/subdirs in the drivers directory
> > +         # wildcards are allowed
> > +
> > +      enabled_drivers = ['common/*', 'bus/*']  # build only these drivers
> > +         # valid values are dir/subdirs in the drivers directory
> > +         # wildcards are allowed
> > diff --git a/drivers/meson.build b/drivers/meson.build index
> > fdf76120ac..70c1aa4e6c 100644
> > --- a/drivers/meson.build
> > +++ b/drivers/meson.build
> > @@ -18,8 +18,36 @@ subdirs = [
> >  	'baseband', # depends on common and bus.
> >  ]
> >
> > -disabled_drivers = run_command(list_dir_globs, get_option('disable_drivers'),
> > -		).stdout().split()
> > +if meson.is_cross_build()
> > +	disabled_drivers += meson.get_cross_property('disabled_drivers', [])
> > +	enabled_drivers += meson.get_cross_property('enabled_drivers', [])
> > +endif
> > +
> > +# add cmdline disabled drivers (comma separated string) # and meson
> > +disabled drivers (list) # together into a comma separated string
> > +disabled_drivers = ','.join([get_option('disable_drivers'),
> > +','.join(disabled_drivers)]).strip(',')
> > +if disabled_drivers != ''
> > +	disabled_drivers = run_command(list_dir_globs,
> > +		disabled_drivers).stdout().split()
> > +else
> > +	disabled_drivers = []
> > +endif
> > +
> > +if enabled_drivers != []
> > +	enabled_drivers = run_command(list_dir_globs,
> > +		','.join(enabled_drivers)).stdout().split()
> > +endif
> > +
> > +if disabled_drivers != [] and enabled_drivers != []
> > +	# TODO/QUERY we could support both:
> > +	# first 'select' only drivers by enabled_drivers
> > +	# then 'deselect' those in disabled_drivers
> > +	# this would be useful if a directory is in enabled_drivers
> > +	# and a driver from that directory is in disabled_drivers
> > +	error('Simultaneous disabled drivers and enabled drivers ' +
> > +	      'configuration is not supported.') endif
> >
> >  default_cflags = machine_args
> >  default_cflags += ['-DALLOW_EXPERIMENTAL_API'] @@ -48,7 +76,7 @@
> > foreach subpath:subdirs
> >  		dpdk_driver_classes += class
> >  	endif
> >  	# get already enabled drivers of the same class
> > -	enabled_drivers = get_variable(class + '_drivers', [])
> > +	enabled_class_drivers = get_variable(class + '_drivers', [])
> >
> >  	foreach drv:drivers
> >  		drv_path = join_paths(class, drv)
> > @@ -76,6 +104,9 @@ foreach subpath:subdirs
> >  		if disabled_drivers.contains(drv_path)
> >  			build = false
> >  			reason = 'explicitly disabled via build config'
> > +		elif enabled_drivers.length() > 0 and not
> > enabled_drivers.contains(drv_path)
> > +			build = false
> > +			reason = 'not in enabled drivers build config'
> >  		else
> >  			# pull in driver directory which should update all the
> local
> > variables
> >  			subdir(drv_path)
> > @@ -108,7 +139,7 @@ foreach subpath:subdirs
> >  						'_disable_reason', reason)
> >  			endif
> >  		else
> > -			enabled_drivers += name
> > +			enabled_class_drivers += name
> >  			lib_name = '_'.join(['rte', class, name])
> >  			dpdk_conf.set(lib_name.to_upper(), 1)
> >
> > @@ -213,5 +244,5 @@ foreach subpath:subdirs
> >  		endif # build
> >  	endforeach
> >
> > -	set_variable(class + '_drivers', enabled_drivers)
> > +	set_variable(class + '_drivers', enabled_class_drivers)
> >  endforeach
> > diff --git a/meson.build b/meson.build index fcc4d4c900..ea7ccfdae3
> > 100644
> > --- a/meson.build
> > +++ b/meson.build
> > @@ -22,6 +22,8 @@ dpdk_drivers = []
> >  dpdk_extra_ldflags = []
> >  dpdk_libs_disabled = []
> >  dpdk_drvs_disabled = []
> > +disabled_drivers = []
> > +enabled_drivers = []
> >  abi_version_file = files('ABI_VERSION')
> >
> >  if host_machine.cpu_family().startswith('x86')
> > --
> > 2.20.1
Bruce Richardson March 9, 2021, 10:56 a.m. UTC | #3
On Tue, Mar 09, 2021 at 08:58:39AM +0000, Juraj Linkeš wrote:
> Honnappa, Thomas, Bruce, Jerin, you've comments in the past. Do you have any further input?
> 
> I think we just need to agree on the allowlist/blocklist mechanism. The current commit allows specifying either an allowlist or a blocklist, but not both. However, it would possible to implement specifying both - first we'll allow what's in allowlist and then we'll remove from that set what's in blocklist. Thoughts?
> 

If we have both, I think limiting to only one is by far the sanest option.
I'm not fully convinced by the need to have both, since the blocklist
allows wildcarding and exception cases. For example "net/[!i]*" will
blocklist all net drivers except those starting with an "i". Admittedly,
for usability purposes having an allowlist might work better.

One final thought, if we add a driver allowlist for cross files, should we
also add one as a top-level meson option also for consistency?

/Bruce
Juraj Linkeš March 9, 2021, 11:49 a.m. UTC | #4
> -----Original Message-----
> From: Bruce Richardson <bruce.richardson@intel.com>
> Sent: Tuesday, March 9, 2021 11:57 AM
> To: Juraj Linkeš <juraj.linkes@pantheon.tech>
> Cc: Ruifeng.Wang@arm.com; Honnappa.Nagarahalli@arm.com;
> Phil.Yang@arm.com; vcchunga@amazon.com; Dharmik.Thakkar@arm.com;
> jerinjacobk@gmail.com; hemant.agrawal@nxp.com;
> ajit.khaparde@broadcom.com; ferruh.yigit@intel.com; aboyer@pensando.io;
> lironh@marvell.com; dev@dpdk.org
> Subject: Re: [PATCH v16 1/3] build: disable/enable drivers in Arm builds
> 
> On Tue, Mar 09, 2021 at 08:58:39AM +0000, Juraj Linkeš wrote:
> > Honnappa, Thomas, Bruce, Jerin, you've comments in the past. Do you have
> any further input?
> >
> > I think we just need to agree on the allowlist/blocklist mechanism. The current
> commit allows specifying either an allowlist or a blocklist, but not both.
> However, it would possible to implement specifying both - first we'll allow
> what's in allowlist and then we'll remove from that set what's in blocklist.
> Thoughts?
> >
> 
> If we have both, I think limiting to only one is by far the sanest option.
> I'm not fully convinced by the need to have both, since the blocklist allows
> wildcarding and exception cases. For example "net/[!i]*" will blocklist all net
> drivers except those starting with an "i". Admittedly, for usability purposes
> having an allowlist might work better.
> 

If we only want to build a handful of drivers then the list could be very long (which was the original reason behind having an allowlist), such as here:
https://gerrit.fd.io/r/gitweb?p=vpp.git;a=blob;f=build/external/packages/dpdk.mk;h=c35ac84c27b19411a0cfdf9a3524fdf36024762c;hb=HEAD

An allowlist could also help with maintenance - users won't need to add new drivers to their blocklists (if that's what users need, like in the case of VPP).

> One final thought, if we add a driver allowlist for cross files, should we also add
> one as a top-level meson option also for consistency?
> 

This definitely makese sense. I was thinking about this and wasn't sure whether I should put it into this commit or a separate one. The commit evolved a bit and now that it's just an implementation of an allow/blocklist it makes sense to include a meson option in it I think - I'll put it into the next version.

> /Bruce
Jerin Jacob March 9, 2021, 12:57 p.m. UTC | #5
On Tue, Mar 9, 2021 at 5:19 PM Juraj Linkeš <juraj.linkes@pantheon.tech> wrote:
>
>
>
> > -----Original Message-----
> > From: Bruce Richardson <bruce.richardson@intel.com>
> > Sent: Tuesday, March 9, 2021 11:57 AM
> > To: Juraj Linkeš <juraj.linkes@pantheon.tech>
> > Cc: Ruifeng.Wang@arm.com; Honnappa.Nagarahalli@arm.com;
> > Phil.Yang@arm.com; vcchunga@amazon.com; Dharmik.Thakkar@arm.com;
> > jerinjacobk@gmail.com; hemant.agrawal@nxp.com;
> > ajit.khaparde@broadcom.com; ferruh.yigit@intel.com; aboyer@pensando.io;
> > lironh@marvell.com; dev@dpdk.org
> > Subject: Re: [PATCH v16 1/3] build: disable/enable drivers in Arm builds
> >
> > On Tue, Mar 09, 2021 at 08:58:39AM +0000, Juraj Linkeš wrote:
> > > Honnappa, Thomas, Bruce, Jerin, you've comments in the past. Do you have
> > any further input?
> > >
> > > I think we just need to agree on the allowlist/blocklist mechanism. The current
> > commit allows specifying either an allowlist or a blocklist, but not both.
> > However, it would possible to implement specifying both - first we'll allow
> > what's in allowlist and then we'll remove from that set what's in blocklist.
> > Thoughts?
> > >
> >
> > If we have both, I think limiting to only one is by far the sanest option.
> > I'm not fully convinced by the need to have both, since the blocklist allows
> > wildcarding and exception cases. For example "net/[!i]*" will blocklist all net
> > drivers except those starting with an "i". Admittedly, for usability purposes
> > having an allowlist might work better.
> >
>
> If we only want to build a handful of drivers then the list could be very long (which was the original reason behind having an allowlist), such as here:
> https://gerrit.fd.io/r/gitweb?p=vpp.git;a=blob;f=build/external/packages/dpdk.mk;h=c35ac84c27b19411a0cfdf9a3524fdf36024762c;hb=HEAD
>
> An allowlist could also help with maintenance - users won't need to add new drivers to their blocklists (if that's what users need, like in the case of VPP).

+1 for allowlist.


>
> > One final thought, if we add a driver allowlist for cross files, should we also add
> > one as a top-level meson option also for consistency?
> >
>
> This definitely makese sense. I was thinking about this and wasn't sure whether I should put it into this commit or a separate one. The commit evolved a bit and now that it's just an implementation of an allow/blocklist it makes sense to include a meson option in it I think - I'll put it into the next version.
>
> > /Bruce
>
Honnappa Nagarahalli March 9, 2021, 3:08 p.m. UTC | #6
<snip>

> > >
> > > On Tue, Mar 09, 2021 at 08:58:39AM +0000, Juraj Linkeš wrote:
> > > > Honnappa, Thomas, Bruce, Jerin, you've comments in the past. Do
> > > > you have
> > > any further input?
> > > >
> > > > I think we just need to agree on the allowlist/blocklist
> > > > mechanism. The current
> > > commit allows specifying either an allowlist or a blocklist, but not both.
> > > However, it would possible to implement specifying both - first
> > > we'll allow what's in allowlist and then we'll remove from that set what's
> in blocklist.
> > > Thoughts?
> > > >
> > >
> > > If we have both, I think limiting to only one is by far the sanest option.
+1

> > > I'm not fully convinced by the need to have both, since the
> > > blocklist allows wildcarding and exception cases. For example
> > > "net/[!i]*" will blocklist all net drivers except those starting
> > > with an "i". Admittedly, for usability purposes having an allowlist might
> work better.
> > >
> >
> > If we only want to build a handful of drivers then the list could be very long
> (which was the original reason behind having an allowlist), such as here:
> > https://gerrit.fd.io/r/gitweb?p=vpp.git;a=blob;f=build/external/packag
> > es/dpdk.mk;h=c35ac84c27b19411a0cfdf9a3524fdf36024762c;hb=HEAD
> >
> > An allowlist could also help with maintenance - users won't need to add
> new drivers to their blocklists (if that's what users need, like in the case of
> VPP).
> 
> +1 for allowlist.
May be I am missing something here. By creating an allowlist, does it mean drivers are disabled (from compilation) by default? For a server platform, where almost all the drivers can be compiled, does the allowlist contain all the drivers?

If we assume by default everything should compile on Arm platform, but allow for few exceptions (where things are really painful to fix, for ex: compiler needs to be fixed), having a blocklist should be shorter/better?

By having an allowlist, we will end up with a large part of the code that might not compile on Arm platforms.

> 
> >
> > > One final thought, if we add a driver allowlist for cross files,
> > > should we also add one as a top-level meson option also for consistency?
> > >
> >
> > This definitely makese sense. I was thinking about this and wasn't sure
> whether I should put it into this commit or a separate one. The commit
> evolved a bit and now that it's just an implementation of an allow/blocklist it
> makes sense to include a meson option in it I think - I'll put it into the next
> version.
> >
> > > /Bruce
> >
Honnappa Nagarahalli March 9, 2021, 3:10 p.m. UTC | #7
<snip>

> > >
> > > On Tue, Mar 09, 2021 at 08:58:39AM +0000, Juraj Linkeš wrote:
> > > > Honnappa, Thomas, Bruce, Jerin, you've comments in the past. Do
> > > > you have
> > > any further input?
> > > >
> > > > I think we just need to agree on the allowlist/blocklist
> > > > mechanism. The current
> > > commit allows specifying either an allowlist or a blocklist, but not both.
> > > However, it would possible to implement specifying both - first
> > > we'll allow what's in allowlist and then we'll remove from that set what's
> in blocklist.
> > > Thoughts?
> > > >
> > >
> > > If we have both, I think limiting to only one is by far the sanest option.
+1

> > > I'm not fully convinced by the need to have both, since the
> > > blocklist allows wildcarding and exception cases. For example
> > > "net/[!i]*" will blocklist all net drivers except those starting
> > > with an "i". Admittedly, for usability purposes having an allowlist might
> work better.
> > >
> >
> > If we only want to build a handful of drivers then the list could be very long
> (which was the original reason behind having an allowlist), such as here:
> > https://gerrit.fd.io/r/gitweb?p=vpp.git;a=blob;f=build/external/packag
> > es/dpdk.mk;h=c35ac84c27b19411a0cfdf9a3524fdf36024762c;hb=HEAD
> >
> > An allowlist could also help with maintenance - users won't need to add
> new drivers to their blocklists (if that's what users need, like in the case of
> VPP).
> 
> +1 for allowlist.
May be I am missing something here. By creating an allowlist, does it mean drivers are disabled (from compilation) by default? For a server platform, where almost all the drivers can be compiled, does the allowlist contain all the drivers?

If we assume by default everything should compile on Arm platform, but allow for few exceptions (where things are really painful to fix, for ex: compiler needs to be fixed), having a blocklist should be shorter/better?

By having an allowlist, we will end up with a large part of the code that might not compile on Arm platforms.

> 
> >
> > > One final thought, if we add a driver allowlist for cross files,
> > > should we also add one as a top-level meson option also for consistency?
> > >
> >
> > This definitely makese sense. I was thinking about this and wasn't sure
> whether I should put it into this commit or a separate one. The commit
> evolved a bit and now that it's just an implementation of an allow/blocklist it
> makes sense to include a meson option in it I think - I'll put it into the next
> version.
> >
> > > /Bruce
> >
Juraj Linkeš March 9, 2021, 3:49 p.m. UTC | #8
> -----Original Message-----
> From: Honnappa Nagarahalli <Honnappa.Nagarahalli@arm.com>
> Sent: Tuesday, March 9, 2021 4:09 PM
> To: Jerin Jacob <jerinjacobk@gmail.com>; Juraj Linkeš
> <juraj.linkes@pantheon.tech>
> Cc: Bruce Richardson <bruce.richardson@intel.com>; Ruifeng Wang
> <Ruifeng.Wang@arm.com>; vcchunga@amazon.com; Dharmik Thakkar
> <Dharmik.Thakkar@arm.com>; hemant.agrawal@nxp.com; Ajit Khaparde
> (ajit.khaparde@broadcom.com) <ajit.khaparde@broadcom.com>;
> ferruh.yigit@intel.com; aboyer@pensando.io; lironh@marvell.com;
> dev@dpdk.org; nd <nd@arm.com>; nd <nd@arm.com>
> Subject: RE: [PATCH v16 1/3] build: disable/enable drivers in Arm builds
> 
> <snip>
> 
> > > >
> > > > On Tue, Mar 09, 2021 at 08:58:39AM +0000, Juraj Linkeš wrote:
> > > > > Honnappa, Thomas, Bruce, Jerin, you've comments in the past. Do
> > > > > you have
> > > > any further input?
> > > > >
> > > > > I think we just need to agree on the allowlist/blocklist
> > > > > mechanism. The current
> > > > commit allows specifying either an allowlist or a blocklist, but not both.
> > > > However, it would possible to implement specifying both - first
> > > > we'll allow what's in allowlist and then we'll remove from that
> > > > set what's
> > in blocklist.
> > > > Thoughts?
> > > > >
> > > >
> > > > If we have both, I think limiting to only one is by far the sanest option.
> +1
> 
> > > > I'm not fully convinced by the need to have both, since the
> > > > blocklist allows wildcarding and exception cases. For example
> > > > "net/[!i]*" will blocklist all net drivers except those starting
> > > > with an "i". Admittedly, for usability purposes having an
> > > > allowlist might
> > work better.
> > > >
> > >
> > > If we only want to build a handful of drivers then the list could be
> > > very long
> > (which was the original reason behind having an allowlist), such as here:
> > > https://gerrit.fd.io/r/gitweb?p=vpp.git;a=blob;f=build/external/pack
> > > ag es/dpdk.mk;h=c35ac84c27b19411a0cfdf9a3524fdf36024762c;hb=HEAD
> > >
> > > An allowlist could also help with maintenance - users won't need to
> > > add
> > new drivers to their blocklists (if that's what users need, like in
> > the case of VPP).
> >
> > +1 for allowlist.
> May be I am missing something here. By creating an allowlist, does it mean
> drivers are disabled (from compilation) by default? For a server platform, where
> almost all the drivers can be compiled, does the allowlist contain all the drivers?
> 

If no allowlist is specified, then everythin will be built - nothing will be filtered.

> If we assume by default everything should compile on Arm platform, but allow
> for few exceptions (where things are really painful to fix, for ex: compiler needs
> to be fixed), having a blocklist should be shorter/better?
> 

The blocklist is, I think, agreed upon by everyone. The question is whether we want to support the allowlist alongside it and there seem to be enough reasons to do that.

> By having an allowlist, we will end up with a large part of the code that might
> not compile on Arm platforms.
> 
> >
> > >
> > > > One final thought, if we add a driver allowlist for cross files,
> > > > should we also add one as a top-level meson option also for consistency?
> > > >
> > >
> > > This definitely makese sense. I was thinking about this and wasn't
> > > sure
> > whether I should put it into this commit or a separate one. The commit
> > evolved a bit and now that it's just an implementation of an
> > allow/blocklist it makes sense to include a meson option in it I think
> > - I'll put it into the next version.
> > >
> > > > /Bruce
> > >
Honnappa Nagarahalli March 9, 2021, 4:04 p.m. UTC | #9
<snip>

> > > > >
> > > > > On Tue, Mar 09, 2021 at 08:58:39AM +0000, Juraj Linkeš wrote:
> > > > > > Honnappa, Thomas, Bruce, Jerin, you've comments in the past.
> > > > > > Do you have
> > > > > any further input?
> > > > > >
> > > > > > I think we just need to agree on the allowlist/blocklist
> > > > > > mechanism. The current
> > > > > commit allows specifying either an allowlist or a blocklist, but not
> both.
> > > > > However, it would possible to implement specifying both - first
> > > > > we'll allow what's in allowlist and then we'll remove from that
> > > > > set what's
> > > in blocklist.
> > > > > Thoughts?
> > > > > >
> > > > >
> > > > > If we have both, I think limiting to only one is by far the sanest option.
> > +1
> >
> > > > > I'm not fully convinced by the need to have both, since the
> > > > > blocklist allows wildcarding and exception cases. For example
> > > > > "net/[!i]*" will blocklist all net drivers except those starting
> > > > > with an "i". Admittedly, for usability purposes having an
> > > > > allowlist might
> > > work better.
> > > > >
> > > >
> > > > If we only want to build a handful of drivers then the list could
> > > > be very long
> > > (which was the original reason behind having an allowlist), such as here:
> > > > https://gerrit.fd.io/r/gitweb?p=vpp.git;a=blob;f=build/external/pa
> > > > ck ag
> > > > es/dpdk.mk;h=c35ac84c27b19411a0cfdf9a3524fdf36024762c;hb=HEAD
> > > >
> > > > An allowlist could also help with maintenance - users won't need
> > > > to add
> > > new drivers to their blocklists (if that's what users need, like in
> > > the case of VPP).
> > >
> > > +1 for allowlist.
> > May be I am missing something here. By creating an allowlist, does it
> > mean drivers are disabled (from compilation) by default? For a server
> > platform, where almost all the drivers can be compiled, does the allowlist
> contain all the drivers?
> >
> 
> If no allowlist is specified, then everythin will be built - nothing will be
> filtered.
That's confusing.
If a platform like bluefield has an allow list, a new PMD that gets added will not be compiled for that platform unless someone explicitly adds it to the allow list. Is my understanding correct?
Whereas if the bluefield platform had a block list, then the new PMD would be compiled for bluefield platform.

> 
> > If we assume by default everything should compile on Arm platform, but
> > allow for few exceptions (where things are really painful to fix, for
> > ex: compiler needs to be fixed), having a blocklist should be shorter/better?
> >
> 
> The blocklist is, I think, agreed upon by everyone. The question is whether we
> want to support the allowlist alongside it and there seem to be enough
> reasons to do that.
Sorry, may be this is answered already, but, what additional benefit does allowlist provide over the blocklist?

> 
> > By having an allowlist, we will end up with a large part of the code
> > that might not compile on Arm platforms.
> >
> > >
> > > >
> > > > > One final thought, if we add a driver allowlist for cross files,
> > > > > should we also add one as a top-level meson option also for
> consistency?
> > > > >
> > > >
> > > > This definitely makese sense. I was thinking about this and wasn't
> > > > sure
> > > whether I should put it into this commit or a separate one. The
> > > commit evolved a bit and now that it's just an implementation of an
> > > allow/blocklist it makes sense to include a meson option in it I
> > > think
> > > - I'll put it into the next version.
> > > >
> > > > > /Bruce
> > > >
Juraj Linkeš March 9, 2021, 6:09 p.m. UTC | #10
> -----Original Message-----
> From: Honnappa Nagarahalli <Honnappa.Nagarahalli@arm.com>
> Sent: Tuesday, March 9, 2021 5:05 PM
> To: Juraj Linkeš <juraj.linkes@pantheon.tech>; Jerin Jacob
> <jerinjacobk@gmail.com>
> Cc: Bruce Richardson <bruce.richardson@intel.com>; Ruifeng Wang
> <Ruifeng.Wang@arm.com>; vcchunga@amazon.com; Dharmik Thakkar
> <Dharmik.Thakkar@arm.com>; hemant.agrawal@nxp.com; Ajit Khaparde
> (ajit.khaparde@broadcom.com) <ajit.khaparde@broadcom.com>;
> ferruh.yigit@intel.com; aboyer@pensando.io; lironh@marvell.com;
> dev@dpdk.org; nd <nd@arm.com>; nd <nd@arm.com>
> Subject: RE: [PATCH v16 1/3] build: disable/enable drivers in Arm builds
> 
> <snip>
> 
> > > > > >
> > > > > > On Tue, Mar 09, 2021 at 08:58:39AM +0000, Juraj Linkeš wrote:
> > > > > > > Honnappa, Thomas, Bruce, Jerin, you've comments in the past.
> > > > > > > Do you have
> > > > > > any further input?
> > > > > > >
> > > > > > > I think we just need to agree on the allowlist/blocklist
> > > > > > > mechanism. The current
> > > > > > commit allows specifying either an allowlist or a blocklist,
> > > > > > but not
> > both.
> > > > > > However, it would possible to implement specifying both -
> > > > > > first we'll allow what's in allowlist and then we'll remove
> > > > > > from that set what's
> > > > in blocklist.
> > > > > > Thoughts?
> > > > > > >
> > > > > >
> > > > > > If we have both, I think limiting to only one is by far the sanest option.
> > > +1
> > >
> > > > > > I'm not fully convinced by the need to have both, since the
> > > > > > blocklist allows wildcarding and exception cases. For example
> > > > > > "net/[!i]*" will blocklist all net drivers except those
> > > > > > starting with an "i". Admittedly, for usability purposes
> > > > > > having an allowlist might
> > > > work better.
> > > > > >
> > > > >
> > > > > If we only want to build a handful of drivers then the list
> > > > > could be very long
> > > > (which was the original reason behind having an allowlist), such as here:
> > > > > https://gerrit.fd.io/r/gitweb?p=vpp.git;a=blob;f=build/external/
> > > > > pa
> > > > > ck ag
> > > > > es/dpdk.mk;h=c35ac84c27b19411a0cfdf9a3524fdf36024762c;hb=HEAD
> > > > >
> > > > > An allowlist could also help with maintenance - users won't need
> > > > > to add
> > > > new drivers to their blocklists (if that's what users need, like
> > > > in the case of VPP).
> > > >
> > > > +1 for allowlist.
> > > May be I am missing something here. By creating an allowlist, does
> > > it mean drivers are disabled (from compilation) by default? For a
> > > server platform, where almost all the drivers can be compiled, does
> > > the allowlist
> > contain all the drivers?
> > >
> >
> > If no allowlist is specified, then everythin will be built - nothing
> > will be filtered.
> That's confusing.
> If a platform like bluefield has an allow list, a new PMD that gets added will not
> be compiled for that platform unless someone explicitly adds it to the allow list.
> Is my understanding correct?

Yes.

> Whereas if the bluefield platform had a block list, then the new PMD would be
> compiled for bluefield platform.
> 

Again, yes.

Supporting both would give us the option to choose between the two behaviors.

> >
> > > If we assume by default everything should compile on Arm platform,
> > > but allow for few exceptions (where things are really painful to
> > > fix, for
> > > ex: compiler needs to be fixed), having a blocklist should be shorter/better?
> > >
> >
> > The blocklist is, I think, agreed upon by everyone. The question is
> > whether we want to support the allowlist alongside it and there seem
> > to be enough reasons to do that.
> Sorry, may be this is answered already, but, what additional benefit does
> allowlist provide over the blocklist?
> 

VPP could use it:
https://gerrit.fd.io/r/gitweb?p=vpp.git;a=blob;f=build/external/packages/dpdk.mk;h=c35ac84c27b19411a0cfdf9a3524fdf36024762c;hb=HEAD

They're disabling almost everything so an allowlist would fit there. And they won't need to update the list when a new driver is added (which they won't need).

I think it was Jerin who suggested the allowlist. I don't know of an Arm usecase for it, but maybe he has an example.

> >
> > > By having an allowlist, we will end up with a large part of the code
> > > that might not compile on Arm platforms.
> > >
> > > >
> > > > >
> > > > > > One final thought, if we add a driver allowlist for cross
> > > > > > files, should we also add one as a top-level meson option also
> > > > > > for
> > consistency?
> > > > > >
> > > > >
> > > > > This definitely makese sense. I was thinking about this and
> > > > > wasn't sure
> > > > whether I should put it into this commit or a separate one. The
> > > > commit evolved a bit and now that it's just an implementation of
> > > > an allow/blocklist it makes sense to include a meson option in it
> > > > I think
> > > > - I'll put it into the next version.
> > > > >
> > > > > > /Bruce
> > > > >
Honnappa Nagarahalli March 9, 2021, 7:54 p.m. UTC | #11
<snip>
> >
> > > > > > >
> > > > > > > On Tue, Mar 09, 2021 at 08:58:39AM +0000, Juraj Linkeš wrote:
> > > > > > > > Honnappa, Thomas, Bruce, Jerin, you've comments in the past.
> > > > > > > > Do you have
> > > > > > > any further input?
> > > > > > > >
> > > > > > > > I think we just need to agree on the allowlist/blocklist
> > > > > > > > mechanism. The current
> > > > > > > commit allows specifying either an allowlist or a blocklist,
> > > > > > > but not
> > > both.
> > > > > > > However, it would possible to implement specifying both -
> > > > > > > first we'll allow what's in allowlist and then we'll remove
> > > > > > > from that set what's
> > > > > in blocklist.
> > > > > > > Thoughts?
> > > > > > > >
> > > > > > >
> > > > > > > If we have both, I think limiting to only one is by far the sanest
> option.
> > > > +1
> > > >
> > > > > > > I'm not fully convinced by the need to have both, since the
> > > > > > > blocklist allows wildcarding and exception cases. For
> > > > > > > example "net/[!i]*" will blocklist all net drivers except
> > > > > > > those starting with an "i". Admittedly, for usability
> > > > > > > purposes having an allowlist might
> > > > > work better.
> > > > > > >
> > > > > >
> > > > > > If we only want to build a handful of drivers then the list
> > > > > > could be very long
> > > > > (which was the original reason behind having an allowlist), such as
> here:
> > > > > > https://gerrit.fd.io/r/gitweb?p=vpp.git;a=blob;f=build/externa
> > > > > > l/
> > > > > > pa
> > > > > > ck ag
> > > > > >
> es/dpdk.mk;h=c35ac84c27b19411a0cfdf9a3524fdf36024762c;hb=HEAD
> > > > > >
> > > > > > An allowlist could also help with maintenance - users won't
> > > > > > need to add
> > > > > new drivers to their blocklists (if that's what users need, like
> > > > > in the case of VPP).
> > > > >
> > > > > +1 for allowlist.
> > > > May be I am missing something here. By creating an allowlist, does
> > > > it mean drivers are disabled (from compilation) by default? For a
> > > > server platform, where almost all the drivers can be compiled,
> > > > does the allowlist
> > > contain all the drivers?
> > > >
> > >
> > > If no allowlist is specified, then everythin will be built - nothing
> > > will be filtered.
> > That's confusing.
> > If a platform like bluefield has an allow list, a new PMD that gets
> > added will not be compiled for that platform unless someone explicitly adds
> it to the allow list.
> > Is my understanding correct?
> 
> Yes.
With this it becomes very easy to skip compiling on a platform.

> 
> > Whereas if the bluefield platform had a block list, then the new PMD
> > would be compiled for bluefield platform.
> >
> 
> Again, yes.
> 
> Supporting both would give us the option to choose between the two
> behaviors.
> 
> > >
> > > > If we assume by default everything should compile on Arm platform,
> > > > but allow for few exceptions (where things are really painful to
> > > > fix, for
> > > > ex: compiler needs to be fixed), having a blocklist should be
> shorter/better?
> > > >
> > >
> > > The blocklist is, I think, agreed upon by everyone. The question is
> > > whether we want to support the allowlist alongside it and there seem
> > > to be enough reasons to do that.
> > Sorry, may be this is answered already, but, what additional benefit
> > does allowlist provide over the blocklist?
> >
> 
> VPP could use it:
> https://gerrit.fd.io/r/gitweb?p=vpp.git;a=blob;f=build/external/packages/dpdk
> .mk;h=c35ac84c27b19411a0cfdf9a3524fdf36024762c;hb=HEAD
> 
> They're disabling almost everything so an allowlist would fit there. And they
> won't need to update the list when a new driver is added (which they won't
> need).
This is different from how we started this discussion. The current discussion was for DPDK internal use. But the one you are referencing above is for users of DPDK. I am fine for providing the allow list for the users of DPDK. But for DPDK internal, I think block list is enough.

> 
> I think it was Jerin who suggested the allowlist. I don't know of an Arm
> usecase for it, but maybe he has an example.
> 
> > >
> > > > By having an allowlist, we will end up with a large part of the
> > > > code that might not compile on Arm platforms.
> > > >
> > > > >
> > > > > >
> > > > > > > One final thought, if we add a driver allowlist for cross
> > > > > > > files, should we also add one as a top-level meson option
> > > > > > > also for
> > > consistency?
> > > > > > >
> > > > > >
> > > > > > This definitely makese sense. I was thinking about this and
> > > > > > wasn't sure
> > > > > whether I should put it into this commit or a separate one. The
> > > > > commit evolved a bit and now that it's just an implementation of
> > > > > an allow/blocklist it makes sense to include a meson option in
> > > > > it I think
> > > > > - I'll put it into the next version.
> > > > > >
> > > > > > > /Bruce
> > > > > >
Juraj Linkeš March 10, 2021, 7:42 a.m. UTC | #12
> -----Original Message-----
> From: Honnappa Nagarahalli <Honnappa.Nagarahalli@arm.com>
> Sent: Tuesday, March 9, 2021 8:55 PM
> To: Juraj Linkeš <juraj.linkes@pantheon.tech>; Jerin Jacob
> <jerinjacobk@gmail.com>
> Cc: Bruce Richardson <bruce.richardson@intel.com>; Ruifeng Wang
> <Ruifeng.Wang@arm.com>; vcchunga@amazon.com; Dharmik Thakkar
> <Dharmik.Thakkar@arm.com>; hemant.agrawal@nxp.com; Ajit Khaparde
> (ajit.khaparde@broadcom.com) <ajit.khaparde@broadcom.com>;
> ferruh.yigit@intel.com; aboyer@pensando.io; lironh@marvell.com;
> dev@dpdk.org; nd <nd@arm.com>; nd <nd@arm.com>
> Subject: RE: [PATCH v16 1/3] build: disable/enable drivers in Arm builds
> 
> <snip>
> > >
> > > > > > > >
> > > > > > > > On Tue, Mar 09, 2021 at 08:58:39AM +0000, Juraj Linkeš wrote:
> > > > > > > > > Honnappa, Thomas, Bruce, Jerin, you've comments in the past.
> > > > > > > > > Do you have
> > > > > > > > any further input?
> > > > > > > > >
> > > > > > > > > I think we just need to agree on the allowlist/blocklist
> > > > > > > > > mechanism. The current
> > > > > > > > commit allows specifying either an allowlist or a
> > > > > > > > blocklist, but not
> > > > both.
> > > > > > > > However, it would possible to implement specifying both -
> > > > > > > > first we'll allow what's in allowlist and then we'll
> > > > > > > > remove from that set what's
> > > > > > in blocklist.
> > > > > > > > Thoughts?
> > > > > > > > >
> > > > > > > >
> > > > > > > > If we have both, I think limiting to only one is by far
> > > > > > > > the sanest
> > option.
> > > > > +1
> > > > >
> > > > > > > > I'm not fully convinced by the need to have both, since
> > > > > > > > the blocklist allows wildcarding and exception cases. For
> > > > > > > > example "net/[!i]*" will blocklist all net drivers except
> > > > > > > > those starting with an "i". Admittedly, for usability
> > > > > > > > purposes having an allowlist might
> > > > > > work better.
> > > > > > > >
> > > > > > >
> > > > > > > If we only want to build a handful of drivers then the list
> > > > > > > could be very long
> > > > > > (which was the original reason behind having an allowlist),
> > > > > > such as
> > here:
> > > > > > > https://gerrit.fd.io/r/gitweb?p=vpp.git;a=blob;f=build/exter
> > > > > > > na
> > > > > > > l/
> > > > > > > pa
> > > > > > > ck ag
> > > > > > >
> > es/dpdk.mk;h=c35ac84c27b19411a0cfdf9a3524fdf36024762c;hb=HEAD
> > > > > > >
> > > > > > > An allowlist could also help with maintenance - users won't
> > > > > > > need to add
> > > > > > new drivers to their blocklists (if that's what users need,
> > > > > > like in the case of VPP).
> > > > > >
> > > > > > +1 for allowlist.
> > > > > May be I am missing something here. By creating an allowlist,
> > > > > does it mean drivers are disabled (from compilation) by default?
> > > > > For a server platform, where almost all the drivers can be
> > > > > compiled, does the allowlist
> > > > contain all the drivers?
> > > > >
> > > >
> > > > If no allowlist is specified, then everythin will be built -
> > > > nothing will be filtered.
> > > That's confusing.
> > > If a platform like bluefield has an allow list, a new PMD that gets
> > > added will not be compiled for that platform unless someone
> > > explicitly adds
> > it to the allow list.
> > > Is my understanding correct?
> >
> > Yes.
> With this it becomes very easy to skip compiling on a platform.
> 

It wouldn't be mandatory. Maybe I should've said we would be able to choose between three behaviors - the current (where everything is built), with allowlist or with blocklist.

But maybe the worry is that someone will use the allowlist without fully understanding the consequences?

> >
> > > Whereas if the bluefield platform had a block list, then the new PMD
> > > would be compiled for bluefield platform.
> > >
> >
> > Again, yes.
> >
> > Supporting both would give us the option to choose between the two
> > behaviors.
> >
> > > >
> > > > > If we assume by default everything should compile on Arm
> > > > > platform, but allow for few exceptions (where things are really
> > > > > painful to fix, for
> > > > > ex: compiler needs to be fixed), having a blocklist should be
> > shorter/better?
> > > > >
> > > >
> > > > The blocklist is, I think, agreed upon by everyone. The question
> > > > is whether we want to support the allowlist alongside it and there
> > > > seem to be enough reasons to do that.
> > > Sorry, may be this is answered already, but, what additional benefit
> > > does allowlist provide over the blocklist?
> > >
> >
> > VPP could use it:
> > https://gerrit.fd.io/r/gitweb?p=vpp.git;a=blob;f=build/external/packag
> > es/dpdk .mk;h=c35ac84c27b19411a0cfdf9a3524fdf36024762c;hb=HEAD
> >
> > They're disabling almost everything so an allowlist would fit there.
> > And they won't need to update the list when a new driver is added
> > (which they won't need).
> This is different from how we started this discussion. The current discussion was
> for DPDK internal use. But the one you are referencing above is for users of
> DPDK. I am fine for providing the allow list for the users of DPDK. But for DPDK
> internal, I think block list is enough.
> 

That's an interesting suggestion. Jerin, what do you think? Why did you want to have an allowlist? Would this work?

> >
> > I think it was Jerin who suggested the allowlist. I don't know of an
> > Arm usecase for it, but maybe he has an example.
> >
> > > >
> > > > > By having an allowlist, we will end up with a large part of the
> > > > > code that might not compile on Arm platforms.
> > > > >
> > > > > >
> > > > > > >
> > > > > > > > One final thought, if we add a driver allowlist for cross
> > > > > > > > files, should we also add one as a top-level meson option
> > > > > > > > also for
> > > > consistency?
> > > > > > > >
> > > > > > >
> > > > > > > This definitely makese sense. I was thinking about this and
> > > > > > > wasn't sure
> > > > > > whether I should put it into this commit or a separate one.
> > > > > > The commit evolved a bit and now that it's just an
> > > > > > implementation of an allow/blocklist it makes sense to include
> > > > > > a meson option in it I think
> > > > > > - I'll put it into the next version.
> > > > > > >
> > > > > > > > /Bruce
> > > > > > >
Jerin Jacob March 10, 2021, 9:12 a.m. UTC | #13
On Wed, Mar 10, 2021 at 1:12 PM Juraj Linkeš <juraj.linkes@pantheon.tech> wrote:
>
>
>
> > -----Original Message-----
> > From: Honnappa Nagarahalli <Honnappa.Nagarahalli@arm.com>
> > Sent: Tuesday, March 9, 2021 8:55 PM
> > To: Juraj Linkeš <juraj.linkes@pantheon.tech>; Jerin Jacob
> > <jerinjacobk@gmail.com>
> > Cc: Bruce Richardson <bruce.richardson@intel.com>; Ruifeng Wang
> > <Ruifeng.Wang@arm.com>; vcchunga@amazon.com; Dharmik Thakkar
> > <Dharmik.Thakkar@arm.com>; hemant.agrawal@nxp.com; Ajit Khaparde
> > (ajit.khaparde@broadcom.com) <ajit.khaparde@broadcom.com>;
> > ferruh.yigit@intel.com; aboyer@pensando.io; lironh@marvell.com;
> > dev@dpdk.org; nd <nd@arm.com>; nd <nd@arm.com>
> > Subject: RE: [PATCH v16 1/3] build: disable/enable drivers in Arm builds
> >
> > <snip>
> > > >
> > > > > > > > >
> > > > > > > > > On Tue, Mar 09, 2021 at 08:58:39AM +0000, Juraj Linkeš wrote:
> > > > > > > > > > Honnappa, Thomas, Bruce, Jerin, you've comments in the past.
> > > > > > > > > > Do you have
> > > > > > > > > any further input?
> > > > > > > > > >
> > > > > > > > > > I think we just need to agree on the allowlist/blocklist
> > > > > > > > > > mechanism. The current
> > > > > > > > > commit allows specifying either an allowlist or a
> > > > > > > > > blocklist, but not
> > > > > both.
> > > > > > > > > However, it would possible to implement specifying both -
> > > > > > > > > first we'll allow what's in allowlist and then we'll
> > > > > > > > > remove from that set what's
> > > > > > > in blocklist.
> > > > > > > > > Thoughts?
> > > > > > > > > >
> > > > > > > > >
> > > > > > > > > If we have both, I think limiting to only one is by far
> > > > > > > > > the sanest
> > > option.
> > > > > > +1
> > > > > >
> > > > > > > > > I'm not fully convinced by the need to have both, since
> > > > > > > > > the blocklist allows wildcarding and exception cases. For
> > > > > > > > > example "net/[!i]*" will blocklist all net drivers except
> > > > > > > > > those starting with an "i". Admittedly, for usability
> > > > > > > > > purposes having an allowlist might
> > > > > > > work better.
> > > > > > > > >
> > > > > > > >
> > > > > > > > If we only want to build a handful of drivers then the list
> > > > > > > > could be very long
> > > > > > > (which was the original reason behind having an allowlist),
> > > > > > > such as
> > > here:
> > > > > > > > https://gerrit.fd.io/r/gitweb?p=vpp.git;a=blob;f=build/exter
> > > > > > > > na
> > > > > > > > l/
> > > > > > > > pa
> > > > > > > > ck ag
> > > > > > > >
> > > es/dpdk.mk;h=c35ac84c27b19411a0cfdf9a3524fdf36024762c;hb=HEAD
> > > > > > > >
> > > > > > > > An allowlist could also help with maintenance - users won't
> > > > > > > > need to add
> > > > > > > new drivers to their blocklists (if that's what users need,
> > > > > > > like in the case of VPP).
> > > > > > >
> > > > > > > +1 for allowlist.
> > > > > > May be I am missing something here. By creating an allowlist,
> > > > > > does it mean drivers are disabled (from compilation) by default?
> > > > > > For a server platform, where almost all the drivers can be
> > > > > > compiled, does the allowlist
> > > > > contain all the drivers?
> > > > > >
> > > > >
> > > > > If no allowlist is specified, then everythin will be built -
> > > > > nothing will be filtered.
> > > > That's confusing.
> > > > If a platform like bluefield has an allow list, a new PMD that gets
> > > > added will not be compiled for that platform unless someone
> > > > explicitly adds
> > > it to the allow list.
> > > > Is my understanding correct?
> > >
> > > Yes.
> > With this it becomes very easy to skip compiling on a platform.
> >
>
> It wouldn't be mandatory. Maybe I should've said we would be able to choose between three behaviors - the current (where everything is built), with allowlist or with blocklist.
>
> But maybe the worry is that someone will use the allowlist without fully understanding the consequences?
>
> > >
> > > > Whereas if the bluefield platform had a block list, then the new PMD
> > > > would be compiled for bluefield platform.
> > > >
> > >
> > > Again, yes.
> > >
> > > Supporting both would give us the option to choose between the two
> > > behaviors.
> > >
> > > > >
> > > > > > If we assume by default everything should compile on Arm
> > > > > > platform, but allow for few exceptions (where things are really
> > > > > > painful to fix, for
> > > > > > ex: compiler needs to be fixed), having a blocklist should be
> > > shorter/better?
> > > > > >
> > > > >
> > > > > The blocklist is, I think, agreed upon by everyone. The question
> > > > > is whether we want to support the allowlist alongside it and there
> > > > > seem to be enough reasons to do that.
> > > > Sorry, may be this is answered already, but, what additional benefit
> > > > does allowlist provide over the blocklist?
> > > >
> > >
> > > VPP could use it:
> > > https://gerrit.fd.io/r/gitweb?p=vpp.git;a=blob;f=build/external/packag
> > > es/dpdk .mk;h=c35ac84c27b19411a0cfdf9a3524fdf36024762c;hb=HEAD
> > >
> > > They're disabling almost everything so an allowlist would fit there.
> > > And they won't need to update the list when a new driver is added
> > > (which they won't need).
> > This is different from how we started this discussion. The current discussion was
> > for DPDK internal use. But the one you are referencing above is for users of
> > DPDK. I am fine for providing the allow list for the users of DPDK. But for DPDK
> > internal, I think block list is enough.
> >
>
> That's an interesting suggestion. Jerin, what do you think? Why did you want to have an allowlist? Would this work?

# The very reason why VPP chooses to have allow list so that they can
control what needs to include.
# Another use case is like, in SoCs have fixed internal devices, we
can have optimized build for that can have only allow list
of the drivers that care about
# For server market, block list makes sense
# For embedded SoC, allow list makes sense.
# Ideal situation is if we support both.
# I dont quite understand the above comments for internal use vs
external use. If it is exposed
as a meson option then I think, it does not matter. Right?

>
> > >
> > > I think it was Jerin who suggested the allowlist. I don't know of an
> > > Arm usecase for it, but maybe he has an example.
> > >
> > > > >
> > > > > > By having an allowlist, we will end up with a large part of the
> > > > > > code that might not compile on Arm platforms.
> > > > > >
> > > > > > >
> > > > > > > >
> > > > > > > > > One final thought, if we add a driver allowlist for cross
> > > > > > > > > files, should we also add one as a top-level meson option
> > > > > > > > > also for
> > > > > consistency?
> > > > > > > > >
> > > > > > > >
> > > > > > > > This definitely makese sense. I was thinking about this and
> > > > > > > > wasn't sure
> > > > > > > whether I should put it into this commit or a separate one.
> > > > > > > The commit evolved a bit and now that it's just an
> > > > > > > implementation of an allow/blocklist it makes sense to include
> > > > > > > a meson option in it I think
> > > > > > > - I'll put it into the next version.
> > > > > > > >
> > > > > > > > > /Bruce
> > > > > > > >
Honnappa Nagarahalli March 10, 2021, 7:36 p.m. UTC | #14
<snip>

> > > > >
> > > > > > > > > >
> > > > > > > > > > On Tue, Mar 09, 2021 at 08:58:39AM +0000, Juraj Linkeš
> wrote:
> > > > > > > > > > > Honnappa, Thomas, Bruce, Jerin, you've comments in the
> past.
> > > > > > > > > > > Do you have
> > > > > > > > > > any further input?
> > > > > > > > > > >
> > > > > > > > > > > I think we just need to agree on the
> > > > > > > > > > > allowlist/blocklist mechanism. The current
> > > > > > > > > > commit allows specifying either an allowlist or a
> > > > > > > > > > blocklist, but not
> > > > > > both.
> > > > > > > > > > However, it would possible to implement specifying
> > > > > > > > > > both - first we'll allow what's in allowlist and then
> > > > > > > > > > we'll remove from that set what's
> > > > > > > > in blocklist.
> > > > > > > > > > Thoughts?
> > > > > > > > > > >
> > > > > > > > > >
> > > > > > > > > > If we have both, I think limiting to only one is by
> > > > > > > > > > far the sanest
> > > > option.
> > > > > > > +1
> > > > > > >
> > > > > > > > > > I'm not fully convinced by the need to have both,
> > > > > > > > > > since the blocklist allows wildcarding and exception
> > > > > > > > > > cases. For example "net/[!i]*" will blocklist all net
> > > > > > > > > > drivers except those starting with an "i". Admittedly,
> > > > > > > > > > for usability purposes having an allowlist might
> > > > > > > > work better.
> > > > > > > > > >
> > > > > > > > >
> > > > > > > > > If we only want to build a handful of drivers then the
> > > > > > > > > list could be very long
> > > > > > > > (which was the original reason behind having an
> > > > > > > > allowlist), such as
> > > > here:
> > > > > > > > > https://gerrit.fd.io/r/gitweb?p=vpp.git;a=blob;f=build/e
> > > > > > > > > xter
> > > > > > > > > na
> > > > > > > > > l/
> > > > > > > > > pa
> > > > > > > > > ck ag
> > > > > > > > >
> > > > es/dpdk.mk;h=c35ac84c27b19411a0cfdf9a3524fdf36024762c;hb=HEAD
> > > > > > > > >
> > > > > > > > > An allowlist could also help with maintenance - users
> > > > > > > > > won't need to add
> > > > > > > > new drivers to their blocklists (if that's what users
> > > > > > > > need, like in the case of VPP).
> > > > > > > >
> > > > > > > > +1 for allowlist.
> > > > > > > May be I am missing something here. By creating an
> > > > > > > allowlist, does it mean drivers are disabled (from compilation) by
> default?
> > > > > > > For a server platform, where almost all the drivers can be
> > > > > > > compiled, does the allowlist
> > > > > > contain all the drivers?
> > > > > > >
> > > > > >
> > > > > > If no allowlist is specified, then everythin will be built -
> > > > > > nothing will be filtered.
> > > > > That's confusing.
> > > > > If a platform like bluefield has an allow list, a new PMD that
> > > > > gets added will not be compiled for that platform unless someone
> > > > > explicitly adds
> > > > it to the allow list.
> > > > > Is my understanding correct?
> > > >
> > > > Yes.
> > > With this it becomes very easy to skip compiling on a platform.
> > >
> >
> > It wouldn't be mandatory. Maybe I should've said we would be able to
> choose between three behaviors - the current (where everything is built),
> with allowlist or with blocklist.
> >
> > But maybe the worry is that someone will use the allowlist without fully
> understanding the consequences?
Yes.

> >
> > > >
> > > > > Whereas if the bluefield platform had a block list, then the new
> > > > > PMD would be compiled for bluefield platform.
> > > > >
> > > >
> > > > Again, yes.
> > > >
> > > > Supporting both would give us the option to choose between the two
> > > > behaviors.
> > > >
> > > > > >
> > > > > > > If we assume by default everything should compile on Arm
> > > > > > > platform, but allow for few exceptions (where things are
> > > > > > > really painful to fix, for
> > > > > > > ex: compiler needs to be fixed), having a blocklist should
> > > > > > > be
> > > > shorter/better?
> > > > > > >
> > > > > >
> > > > > > The blocklist is, I think, agreed upon by everyone. The
> > > > > > question is whether we want to support the allowlist alongside
> > > > > > it and there seem to be enough reasons to do that.
> > > > > Sorry, may be this is answered already, but, what additional
> > > > > benefit does allowlist provide over the blocklist?
> > > > >
> > > >
> > > > VPP could use it:
> > > > https://gerrit.fd.io/r/gitweb?p=vpp.git;a=blob;f=build/external/pa
> > > > ckag es/dpdk
> > > > .mk;h=c35ac84c27b19411a0cfdf9a3524fdf36024762c;hb=HEAD
> > > >
> > > > They're disabling almost everything so an allowlist would fit there.
> > > > And they won't need to update the list when a new driver is added
> > > > (which they won't need).
> > > This is different from how we started this discussion. The current
> > > discussion was for DPDK internal use. But the one you are
> > > referencing above is for users of DPDK. I am fine for providing the
> > > allow list for the users of DPDK. But for DPDK internal, I think block list is
> enough.
> > >
> >
> > That's an interesting suggestion. Jerin, what do you think? Why did you
> want to have an allowlist? Would this work?
> 
> # The very reason why VPP chooses to have allow list so that they can
> control what needs to include.
> # Another use case is like, in SoCs have fixed internal devices, we can have
> optimized build for that can have only allow list of the drivers that care
> about # For server market, block list makes sense # For embedded SoC, allow
> list makes sense.
For the embedded SoC, IMO, the upstream project could allow the compilation for wider set of PMDs/libs. May be the end customer can use the allow list to compile/use what is required?
For ex: we use PCIe interfaces with external NICs for the embedded SoCs (where there is support).
I think the list of PMDs/libs enabled/disabled on a given platform is another discussion. This should not prevent us from supporting the allowlist.

> # Ideal situation is if we support both.
> # I dont quite understand the above comments for internal use vs external
> use. If it is exposed as a meson option then I think, it does not matter. Right?
> 
> >
> > > >
> > > > I think it was Jerin who suggested the allowlist. I don't know of
> > > > an Arm usecase for it, but maybe he has an example.
> > > >
> > > > > >
> > > > > > > By having an allowlist, we will end up with a large part of
> > > > > > > the code that might not compile on Arm platforms.
> > > > > > >
> > > > > > > >
> > > > > > > > >
> > > > > > > > > > One final thought, if we add a driver allowlist for
> > > > > > > > > > cross files, should we also add one as a top-level
> > > > > > > > > > meson option also for
> > > > > > consistency?
> > > > > > > > > >
> > > > > > > > >
> > > > > > > > > This definitely makese sense. I was thinking about this
> > > > > > > > > and wasn't sure
> > > > > > > > whether I should put it into this commit or a separate one.
> > > > > > > > The commit evolved a bit and now that it's just an
> > > > > > > > implementation of an allow/blocklist it makes sense to
> > > > > > > > include a meson option in it I think
> > > > > > > > - I'll put it into the next version.
> > > > > > > > >
> > > > > > > > > > /Bruce
> > > > > > > > >
Jerin Jacob March 11, 2021, 5:14 p.m. UTC | #15
On Thu, Mar 11, 2021 at 1:06 AM Honnappa Nagarahalli
<Honnappa.Nagarahalli@arm.com> wrote:
>
> <snip>
>
> > > > > >
> > > > > > > > > > >
> > > > > > > > > > > On Tue, Mar 09, 2021 at 08:58:39AM +0000, Juraj Linkeš
> > wrote:
> > > > > > > > > > > > Honnappa, Thomas, Bruce, Jerin, you've comments in the
> > past.
> > > > > > > > > > > > Do you have
> > > > > > > > > > > any further input?
> > > > > > > > > > > >
> > > > > > > > > > > > I think we just need to agree on the
> > > > > > > > > > > > allowlist/blocklist mechanism. The current
> > > > > > > > > > > commit allows specifying either an allowlist or a
> > > > > > > > > > > blocklist, but not
> > > > > > > both.
> > > > > > > > > > > However, it would possible to implement specifying
> > > > > > > > > > > both - first we'll allow what's in allowlist and then
> > > > > > > > > > > we'll remove from that set what's
> > > > > > > > > in blocklist.
> > > > > > > > > > > Thoughts?
> > > > > > > > > > > >
> > > > > > > > > > >
> > > > > > > > > > > If we have both, I think limiting to only one is by
> > > > > > > > > > > far the sanest
> > > > > option.
> > > > > > > > +1
> > > > > > > >
> > > > > > > > > > > I'm not fully convinced by the need to have both,
> > > > > > > > > > > since the blocklist allows wildcarding and exception
> > > > > > > > > > > cases. For example "net/[!i]*" will blocklist all net
> > > > > > > > > > > drivers except those starting with an "i". Admittedly,
> > > > > > > > > > > for usability purposes having an allowlist might
> > > > > > > > > work better.
> > > > > > > > > > >
> > > > > > > > > >
> > > > > > > > > > If we only want to build a handful of drivers then the
> > > > > > > > > > list could be very long
> > > > > > > > > (which was the original reason behind having an
> > > > > > > > > allowlist), such as
> > > > > here:
> > > > > > > > > > https://gerrit.fd.io/r/gitweb?p=vpp.git;a=blob;f=build/e
> > > > > > > > > > xter
> > > > > > > > > > na
> > > > > > > > > > l/
> > > > > > > > > > pa
> > > > > > > > > > ck ag
> > > > > > > > > >
> > > > > es/dpdk.mk;h=c35ac84c27b19411a0cfdf9a3524fdf36024762c;hb=HEAD
> > > > > > > > > >
> > > > > > > > > > An allowlist could also help with maintenance - users
> > > > > > > > > > won't need to add
> > > > > > > > > new drivers to their blocklists (if that's what users
> > > > > > > > > need, like in the case of VPP).
> > > > > > > > >
> > > > > > > > > +1 for allowlist.
> > > > > > > > May be I am missing something here. By creating an
> > > > > > > > allowlist, does it mean drivers are disabled (from compilation) by
> > default?
> > > > > > > > For a server platform, where almost all the drivers can be
> > > > > > > > compiled, does the allowlist
> > > > > > > contain all the drivers?
> > > > > > > >
> > > > > > >
> > > > > > > If no allowlist is specified, then everythin will be built -
> > > > > > > nothing will be filtered.
> > > > > > That's confusing.
> > > > > > If a platform like bluefield has an allow list, a new PMD that
> > > > > > gets added will not be compiled for that platform unless someone
> > > > > > explicitly adds
> > > > > it to the allow list.
> > > > > > Is my understanding correct?
> > > > >
> > > > > Yes.
> > > > With this it becomes very easy to skip compiling on a platform.
> > > >
> > >
> > > It wouldn't be mandatory. Maybe I should've said we would be able to
> > choose between three behaviors - the current (where everything is built),
> > with allowlist or with blocklist.
> > >
> > > But maybe the worry is that someone will use the allowlist without fully
> > understanding the consequences?
> Yes.
>
> > >
> > > > >
> > > > > > Whereas if the bluefield platform had a block list, then the new
> > > > > > PMD would be compiled for bluefield platform.
> > > > > >
> > > > >
> > > > > Again, yes.
> > > > >
> > > > > Supporting both would give us the option to choose between the two
> > > > > behaviors.
> > > > >
> > > > > > >
> > > > > > > > If we assume by default everything should compile on Arm
> > > > > > > > platform, but allow for few exceptions (where things are
> > > > > > > > really painful to fix, for
> > > > > > > > ex: compiler needs to be fixed), having a blocklist should
> > > > > > > > be
> > > > > shorter/better?
> > > > > > > >
> > > > > > >
> > > > > > > The blocklist is, I think, agreed upon by everyone. The
> > > > > > > question is whether we want to support the allowlist alongside
> > > > > > > it and there seem to be enough reasons to do that.
> > > > > > Sorry, may be this is answered already, but, what additional
> > > > > > benefit does allowlist provide over the blocklist?
> > > > > >
> > > > >
> > > > > VPP could use it:
> > > > > https://gerrit.fd.io/r/gitweb?p=vpp.git;a=blob;f=build/external/pa
> > > > > ckag es/dpdk
> > > > > .mk;h=c35ac84c27b19411a0cfdf9a3524fdf36024762c;hb=HEAD
> > > > >
> > > > > They're disabling almost everything so an allowlist would fit there.
> > > > > And they won't need to update the list when a new driver is added
> > > > > (which they won't need).
> > > > This is different from how we started this discussion. The current
> > > > discussion was for DPDK internal use. But the one you are
> > > > referencing above is for users of DPDK. I am fine for providing the
> > > > allow list for the users of DPDK. But for DPDK internal, I think block list is
> > enough.
> > > >
> > >
> > > That's an interesting suggestion. Jerin, what do you think? Why did you
> > want to have an allowlist? Would this work?
> >
> > # The very reason why VPP chooses to have allow list so that they can
> > control what needs to include.
> > # Another use case is like, in SoCs have fixed internal devices, we can have
> > optimized build for that can have only allow list of the drivers that care
> > about # For server market, block list makes sense # For embedded SoC, allow
> > list makes sense.
> For the embedded SoC, IMO, the upstream project could allow the compilation for wider set of PMDs/libs. May be the end customer can use the allow list to compile/use what is required?

Just to understand, how end customer can enable allow list, if DPDK
build system does not support it?
Also to understand, If we are supporting blocklist, why not have
allowlist (I mean, both of them) as both are required as it caters
different use case
as mention above. We can not emulate allowlist with blocklist as each
version of DPDK will have new libraries and PMD's which
end user has no clue. Right?
I think, that is the reason why VPP is doing the allow list.


> For ex: we use PCIe interfaces with external NICs for the embedded SoCs (where there is support).
> I think the list of PMDs/libs enabled/disabled on a given platform is another discussion. This should not prevent us from supporting the allowlist.
>
> > # Ideal situation is if we support both.
> > # I dont quite understand the above comments for internal use vs external
> > use. If it is exposed as a meson option then I think, it does not matter. Right?
> >
> > >
> > > > >
> > > > > I think it was Jerin who suggested the allowlist. I don't know of
> > > > > an Arm usecase for it, but maybe he has an example.
> > > > >
> > > > > > >
> > > > > > > > By having an allowlist, we will end up with a large part of
> > > > > > > > the code that might not compile on Arm platforms.
> > > > > > > >
> > > > > > > > >
> > > > > > > > > >
> > > > > > > > > > > One final thought, if we add a driver allowlist for
> > > > > > > > > > > cross files, should we also add one as a top-level
> > > > > > > > > > > meson option also for
> > > > > > > consistency?
> > > > > > > > > > >
> > > > > > > > > >
> > > > > > > > > > This definitely makese sense. I was thinking about this
> > > > > > > > > > and wasn't sure
> > > > > > > > > whether I should put it into this commit or a separate one.
> > > > > > > > > The commit evolved a bit and now that it's just an
> > > > > > > > > implementation of an allow/blocklist it makes sense to
> > > > > > > > > include a meson option in it I think
> > > > > > > > > - I'll put it into the next version.
> > > > > > > > > >
> > > > > > > > > > > /Bruce
> > > > > > > > > >
Juraj Linkeš March 19, 2021, 1:21 p.m. UTC | #16
<removed parts which I think are not that relevant>

> > > > > > > > The blocklist is, I think, agreed upon by everyone. The
> > > > > > > > question is whether we want to support the allowlist
> > > > > > > > alongside it and there seem to be enough reasons to do that.
> > > > > > > Sorry, may be this is answered already, but, what additional
> > > > > > > benefit does allowlist provide over the blocklist?
> > > > > > >
> > > > > >
> > > > > > VPP could use it:
> > > > > > https://gerrit.fd.io/r/gitweb?p=vpp.git;a=blob;f=build/externa
> > > > > > l/pa
> > > > > > ckag es/dpdk
> > > > > > .mk;h=c35ac84c27b19411a0cfdf9a3524fdf36024762c;hb=HEAD
> > > > > >
> > > > > > They're disabling almost everything so an allowlist would fit there.
> > > > > > And they won't need to update the list when a new driver is
> > > > > > added (which they won't need).
> > > > > This is different from how we started this discussion. The
> > > > > current discussion was for DPDK internal use. But the one you
> > > > > are referencing above is for users of DPDK. I am fine for
> > > > > providing the allow list for the users of DPDK. But for DPDK
> > > > > internal, I think block list is
> > > enough.
> > > > >
> > > >
> > > > That's an interesting suggestion. Jerin, what do you think? Why
> > > > did you
> > > want to have an allowlist? Would this work?
> > >
> > > # The very reason why VPP chooses to have allow list so that they
> > > can control what needs to include.
> > > # Another use case is like, in SoCs have fixed internal devices, we
> > > can have optimized build for that can have only allow list of the
> > > drivers that care about # For server market, block list makes sense
> > > # For embedded SoC, allow list makes sense.
> > For the embedded SoC, IMO, the upstream project could allow the compilation
> for wider set of PMDs/libs. May be the end customer can use the allow list to
> compile/use what is required?
> 
> Just to understand, how end customer can enable allow list, if DPDK build system
> does not support it?
> Also to understand, If we are supporting blocklist, why not have allowlist (I
> mean, both of them) as both are required as it caters different use case as
> mention above. We can not emulate allowlist with blocklist as each version of
> DPDK will have new libraries and PMD's which end user has no clue. Right?

> I think, that is the reason why VPP is doing the allow list.

I'm not sure what you mean by this, but to clarify, VPP likely would be using the allowlist in this fashion, but that is not an arm specific usecase. I think what Honnappa wanted to see was how the allowlist could be used in an arm usecase (such as using it in an SoC configuration).
Jerin Jacob March 19, 2021, 1:32 p.m. UTC | #17
On Fri, Mar 19, 2021 at 6:51 PM Juraj Linkeš <juraj.linkes@pantheon.tech> wrote:
>
> <removed parts which I think are not that relevant>
>
> > > > > > > > > The blocklist is, I think, agreed upon by everyone. The
> > > > > > > > > question is whether we want to support the allowlist
> > > > > > > > > alongside it and there seem to be enough reasons to do that.
> > > > > > > > Sorry, may be this is answered already, but, what additional
> > > > > > > > benefit does allowlist provide over the blocklist?
> > > > > > > >
> > > > > > >
> > > > > > > VPP could use it:
> > > > > > > https://gerrit.fd.io/r/gitweb?p=vpp.git;a=blob;f=build/externa
> > > > > > > l/pa
> > > > > > > ckag es/dpdk
> > > > > > > .mk;h=c35ac84c27b19411a0cfdf9a3524fdf36024762c;hb=HEAD
> > > > > > >
> > > > > > > They're disabling almost everything so an allowlist would fit there.
> > > > > > > And they won't need to update the list when a new driver is
> > > > > > > added (which they won't need).
> > > > > > This is different from how we started this discussion. The
> > > > > > current discussion was for DPDK internal use. But the one you
> > > > > > are referencing above is for users of DPDK. I am fine for
> > > > > > providing the allow list for the users of DPDK. But for DPDK
> > > > > > internal, I think block list is
> > > > enough.
> > > > > >
> > > > >
> > > > > That's an interesting suggestion. Jerin, what do you think? Why
> > > > > did you
> > > > want to have an allowlist? Would this work?
> > > >
> > > > # The very reason why VPP chooses to have allow list so that they
> > > > can control what needs to include.
> > > > # Another use case is like, in SoCs have fixed internal devices, we
> > > > can have optimized build for that can have only allow list of the
> > > > drivers that care about # For server market, block list makes sense
> > > > # For embedded SoC, allow list makes sense.
> > > For the embedded SoC, IMO, the upstream project could allow the compilation
> > for wider set of PMDs/libs. May be the end customer can use the allow list to
> > compile/use what is required?
> >
> > Just to understand, how end customer can enable allow list, if DPDK build system
> > does not support it?
> > Also to understand, If we are supporting blocklist, why not have allowlist (I
> > mean, both of them) as both are required as it caters different use case as
> > mention above. We can not emulate allowlist with blocklist as each version of
> > DPDK will have new libraries and PMD's which end user has no clue. Right?
>
> > I think, that is the reason why VPP is doing the allow list.
>
> I'm not sure what you mean by this, but to clarify, VPP likely would be using the allowlist in this fashion, but that is not an arm specific usecase. I think what Honnappa wanted to see was how the allowlist could be used in an arm usecase (such as using it in an SoC configuration).

There is nothing arm-specific here. Right? allowlist will be common
and will be used by all architecture. Right?


>
>
Honnappa Nagarahalli March 30, 2021, 12:39 a.m. UTC | #18
<snip>

> > <removed parts which I think are not that relevant>
> >
> > > > > > > > > > The blocklist is, I think, agreed upon by everyone.
> > > > > > > > > > The question is whether we want to support the
> > > > > > > > > > allowlist alongside it and there seem to be enough reasons to do
> that.
> > > > > > > > > Sorry, may be this is answered already, but, what
> > > > > > > > > additional benefit does allowlist provide over the blocklist?
> > > > > > > > >
> > > > > > > >
> > > > > > > > VPP could use it:
> > > > > > > > https://gerrit.fd.io/r/gitweb?p=vpp.git;a=blob;f=build/ext
> > > > > > > > erna
> > > > > > > > l/pa
> > > > > > > > ckag es/dpdk
> > > > > > > > .mk;h=c35ac84c27b19411a0cfdf9a3524fdf36024762c;hb=HEAD
> > > > > > > >
> > > > > > > > They're disabling almost everything so an allowlist would fit there.
> > > > > > > > And they won't need to update the list when a new driver
> > > > > > > > is added (which they won't need).
> > > > > > > This is different from how we started this discussion. The
> > > > > > > current discussion was for DPDK internal use. But the one
> > > > > > > you are referencing above is for users of DPDK. I am fine
> > > > > > > for providing the allow list for the users of DPDK. But for
> > > > > > > DPDK internal, I think block list is
> > > > > enough.
> > > > > > >
> > > > > >
> > > > > > That's an interesting suggestion. Jerin, what do you think?
> > > > > > Why did you
> > > > > want to have an allowlist? Would this work?
> > > > >
> > > > > # The very reason why VPP chooses to have allow list so that
> > > > > they can control what needs to include.
> > > > > # Another use case is like, in SoCs have fixed internal devices,
> > > > > we can have optimized build for that can have only allow list of
> > > > > the drivers that care about # For server market, block list
> > > > > makes sense # For embedded SoC, allow list makes sense.
> > > > For the embedded SoC, IMO, the upstream project could allow the
> > > > compilation
> > > for wider set of PMDs/libs. May be the end customer can use the
> > > allow list to compile/use what is required?
> > >
> > > Just to understand, how end customer can enable allow list, if DPDK
> > > build system does not support it?
> > > Also to understand, If we are supporting blocklist, why not have
> > > allowlist (I mean, both of them) as both are required as it caters
> > > different use case as mention above. We can not emulate allowlist
> > > with blocklist as each version of DPDK will have new libraries and PMD's
> which end user has no clue. Right?
> >
> > > I think, that is the reason why VPP is doing the allow list.
> >
> > I'm not sure what you mean by this, but to clarify, VPP likely would be using
> the allowlist in this fashion, but that is not an arm specific usecase. I think what
> Honnappa wanted to see was how the allowlist could be used in an arm usecase
> (such as using it in an SoC configuration).
> 
> There is nothing arm-specific here. Right? allowlist will be common and will be
> used by all architecture. Right?
Nothing Arm specific. I think for generic Arm servers platforms we can make sure that we allow for compilation of all the DPDK code. We can go ahead with implementing the allow list.

> 
> 
> >
> >
Juraj Linkeš March 31, 2021, 8:39 a.m. UTC | #19
> -----Original Message-----
> From: Honnappa Nagarahalli <Honnappa.Nagarahalli@arm.com>
> Sent: Tuesday, March 30, 2021 2:39 AM
> To: Jerin Jacob <jerinjacobk@gmail.com>; Juraj Linkeš
> <juraj.linkes@pantheon.tech>
> Cc: Bruce Richardson <bruce.richardson@intel.com>; Ruifeng Wang
> <Ruifeng.Wang@arm.com>; vcchunga@amazon.com; Dharmik Thakkar
> <Dharmik.Thakkar@arm.com>; hemant.agrawal@nxp.com; Ajit Khaparde
> (ajit.khaparde@broadcom.com) <ajit.khaparde@broadcom.com>;
> ferruh.yigit@intel.com; aboyer@pensando.io; lironh@marvell.com;
> dev@dpdk.org; nd <nd@arm.com>; Honnappa Nagarahalli
> <Honnappa.Nagarahalli@arm.com>; nd <nd@arm.com>
> Subject: RE: [PATCH v16 1/3] build: disable/enable drivers in Arm builds
> 
> <snip>
> 
> > > <removed parts which I think are not that relevant>
> > >
> > > > > > > > > > > The blocklist is, I think, agreed upon by everyone.
> > > > > > > > > > > The question is whether we want to support the
> > > > > > > > > > > allowlist alongside it and there seem to be enough
> > > > > > > > > > > reasons to do
> > that.
> > > > > > > > > > Sorry, may be this is answered already, but, what
> > > > > > > > > > additional benefit does allowlist provide over the blocklist?
> > > > > > > > > >
> > > > > > > > >
> > > > > > > > > VPP could use it:
> > > > > > > > > https://gerrit.fd.io/r/gitweb?p=vpp.git;a=blob;f=build/e
> > > > > > > > > xt
> > > > > > > > > erna
> > > > > > > > > l/pa
> > > > > > > > > ckag es/dpdk
> > > > > > > > > .mk;h=c35ac84c27b19411a0cfdf9a3524fdf36024762c;hb=HEAD
> > > > > > > > >
> > > > > > > > > They're disabling almost everything so an allowlist would fit there.
> > > > > > > > > And they won't need to update the list when a new driver
> > > > > > > > > is added (which they won't need).
> > > > > > > > This is different from how we started this discussion. The
> > > > > > > > current discussion was for DPDK internal use. But the one
> > > > > > > > you are referencing above is for users of DPDK. I am fine
> > > > > > > > for providing the allow list for the users of DPDK. But
> > > > > > > > for DPDK internal, I think block list is
> > > > > > enough.
> > > > > > > >
> > > > > > >
> > > > > > > That's an interesting suggestion. Jerin, what do you think?
> > > > > > > Why did you
> > > > > > want to have an allowlist? Would this work?
> > > > > >
> > > > > > # The very reason why VPP chooses to have allow list so that
> > > > > > they can control what needs to include.
> > > > > > # Another use case is like, in SoCs have fixed internal
> > > > > > devices, we can have optimized build for that can have only
> > > > > > allow list of the drivers that care about # For server market,
> > > > > > block list makes sense # For embedded SoC, allow list makes sense.
> > > > > For the embedded SoC, IMO, the upstream project could allow the
> > > > > compilation
> > > > for wider set of PMDs/libs. May be the end customer can use the
> > > > allow list to compile/use what is required?
> > > >
> > > > Just to understand, how end customer can enable allow list, if
> > > > DPDK build system does not support it?
> > > > Also to understand, If we are supporting blocklist, why not have
> > > > allowlist (I mean, both of them) as both are required as it caters
> > > > different use case as mention above. We can not emulate allowlist
> > > > with blocklist as each version of DPDK will have new libraries and
> > > > PMD's
> > which end user has no clue. Right?
> > >
> > > > I think, that is the reason why VPP is doing the allow list.
> > >
> > > I'm not sure what you mean by this, but to clarify, VPP likely would
> > > be using
> > the allowlist in this fashion, but that is not an arm specific
> > usecase. I think what Honnappa wanted to see was how the allowlist
> > could be used in an arm usecase (such as using it in an SoC configuration).
> >
> > There is nothing arm-specific here. Right? allowlist will be common
> > and will be used by all architecture. Right?
> Nothing Arm specific. I think for generic Arm servers platforms we can make
> sure that we allow for compilation of all the DPDK code. We can go ahead with
> implementing the allow list.
> 

I tried to actually use an allowlist and there are some problems with building apps and tests. When I tried to enable a random driver, such as common/sfc_efx, I've ran into dependency issues with apps:
app/meson.build:53:3: ERROR:  Tried to get unknown variable "static_rte_bus_vdev".

This is because bus/vdev is not enabled (the allowlist only enabled net/sfc_efx). When I implemented dependency discovery in apps similar to which exists for drivers/libs, I was then unable to build tests (which is a special case app):
app/test/meson.build:427:1: ERROR:  Tried to get unknown variable "static_rte_bus_pci".

This is becasue bus/pci is not enabled. If I understand the code correctly, the test dependencies are not matched to each test, meaning we can't disable the tests for which we don't have dependencies - we can only disable all tests.

In general, this problem also affects the blocklist, i.e. meson build -Ddisable_drivers=bus/pci produces
drivers/net/hinic/base/meson.build:34:0: ERROR:  Unknown variable "static_rte_bus_pci".
For blocklists, this is seldom going to be a problem, since most users won't disable "core" drivers.

I'm not sure what is the best course of action. We can implement the allowlist and then we'll leave it up to implementers to implement allowlists that produce a working build. We could then optionally address the dependency issues brought by disabled drivers in a separate patch.

Maybe we can just have a list of "core" drivers that can never be disabled (either using blocklists or allowlists)?

Bruce, what do you think?

> >
> >
> > >
> > >
Bruce Richardson March 31, 2021, 9:07 a.m. UTC | #20
On Wed, Mar 31, 2021 at 08:39:57AM +0000, Juraj Linkeš wrote:
> 
> 
> > -----Original Message-----
> > From: Honnappa Nagarahalli <Honnappa.Nagarahalli@arm.com>
> > Sent: Tuesday, March 30, 2021 2:39 AM
> > To: Jerin Jacob <jerinjacobk@gmail.com>; Juraj Linkeš
> > <juraj.linkes@pantheon.tech>
> > Cc: Bruce Richardson <bruce.richardson@intel.com>; Ruifeng Wang
> > <Ruifeng.Wang@arm.com>; vcchunga@amazon.com; Dharmik Thakkar
> > <Dharmik.Thakkar@arm.com>; hemant.agrawal@nxp.com; Ajit Khaparde
> > (ajit.khaparde@broadcom.com) <ajit.khaparde@broadcom.com>;
> > ferruh.yigit@intel.com; aboyer@pensando.io; lironh@marvell.com;
> > dev@dpdk.org; nd <nd@arm.com>; Honnappa Nagarahalli
> > <Honnappa.Nagarahalli@arm.com>; nd <nd@arm.com>
> > Subject: RE: [PATCH v16 1/3] build: disable/enable drivers in Arm builds
> > 
> > <snip>
> > 
> > > > <removed parts which I think are not that relevant>
> > > >
> > > > > > > > > > > > The blocklist is, I think, agreed upon by everyone.
> > > > > > > > > > > > The question is whether we want to support the
> > > > > > > > > > > > allowlist alongside it and there seem to be enough
> > > > > > > > > > > > reasons to do
> > > that.
> > > > > > > > > > > Sorry, may be this is answered already, but, what
> > > > > > > > > > > additional benefit does allowlist provide over the blocklist?
> > > > > > > > > > >
> > > > > > > > > >
> > > > > > > > > > VPP could use it:
> > > > > > > > > > https://gerrit.fd.io/r/gitweb?p=vpp.git;a=blob;f=build/e
> > > > > > > > > > xt
> > > > > > > > > > erna
> > > > > > > > > > l/pa
> > > > > > > > > > ckag es/dpdk
> > > > > > > > > > .mk;h=c35ac84c27b19411a0cfdf9a3524fdf36024762c;hb=HEAD
> > > > > > > > > >
> > > > > > > > > > They're disabling almost everything so an allowlist would fit there.
> > > > > > > > > > And they won't need to update the list when a new driver
> > > > > > > > > > is added (which they won't need).
> > > > > > > > > This is different from how we started this discussion. The
> > > > > > > > > current discussion was for DPDK internal use. But the one
> > > > > > > > > you are referencing above is for users of DPDK. I am fine
> > > > > > > > > for providing the allow list for the users of DPDK. But
> > > > > > > > > for DPDK internal, I think block list is
> > > > > > > enough.
> > > > > > > > >
> > > > > > > >
> > > > > > > > That's an interesting suggestion. Jerin, what do you think?
> > > > > > > > Why did you
> > > > > > > want to have an allowlist? Would this work?
> > > > > > >
> > > > > > > # The very reason why VPP chooses to have allow list so that
> > > > > > > they can control what needs to include.
> > > > > > > # Another use case is like, in SoCs have fixed internal
> > > > > > > devices, we can have optimized build for that can have only
> > > > > > > allow list of the drivers that care about # For server market,
> > > > > > > block list makes sense # For embedded SoC, allow list makes sense.
> > > > > > For the embedded SoC, IMO, the upstream project could allow the
> > > > > > compilation
> > > > > for wider set of PMDs/libs. May be the end customer can use the
> > > > > allow list to compile/use what is required?
> > > > >
> > > > > Just to understand, how end customer can enable allow list, if
> > > > > DPDK build system does not support it?
> > > > > Also to understand, If we are supporting blocklist, why not have
> > > > > allowlist (I mean, both of them) as both are required as it caters
> > > > > different use case as mention above. We can not emulate allowlist
> > > > > with blocklist as each version of DPDK will have new libraries and
> > > > > PMD's
> > > which end user has no clue. Right?
> > > >
> > > > > I think, that is the reason why VPP is doing the allow list.
> > > >
> > > > I'm not sure what you mean by this, but to clarify, VPP likely would
> > > > be using
> > > the allowlist in this fashion, but that is not an arm specific
> > > usecase. I think what Honnappa wanted to see was how the allowlist
> > > could be used in an arm usecase (such as using it in an SoC configuration).
> > >
> > > There is nothing arm-specific here. Right? allowlist will be common
> > > and will be used by all architecture. Right?
> > Nothing Arm specific. I think for generic Arm servers platforms we can make
> > sure that we allow for compilation of all the DPDK code. We can go ahead with
> > implementing the allow list.
> > 
> 
> I tried to actually use an allowlist and there are some problems with building apps and tests. When I tried to enable a random driver, such as common/sfc_efx, I've ran into dependency issues with apps:
> app/meson.build:53:3: ERROR:  Tried to get unknown variable "static_rte_bus_vdev".
> 
> This is because bus/vdev is not enabled (the allowlist only enabled net/sfc_efx). When I implemented dependency discovery in apps similar to which exists for drivers/libs, I was then unable to build tests (which is a special case app):
> app/test/meson.build:427:1: ERROR:  Tried to get unknown variable "static_rte_bus_pci".
> 
> This is becasue bus/pci is not enabled. If I understand the code correctly, the test dependencies are not matched to each test, meaning we can't disable the tests for which we don't have dependencies - we can only disable all tests.
> 
> In general, this problem also affects the blocklist, i.e. meson build -Ddisable_drivers=bus/pci produces
> drivers/net/hinic/base/meson.build:34:0: ERROR:  Unknown variable "static_rte_bus_pci".
> For blocklists, this is seldom going to be a problem, since most users won't disable "core" drivers.
> 
> I'm not sure what is the best course of action. We can implement the allowlist and then we'll leave it up to implementers to implement allowlists that produce a working build. We could then optionally address the dependency issues brought by disabled drivers in a separate patch.
> 
> Maybe we can just have a list of "core" drivers that can never be disabled (either using blocklists or allowlists)?
> 
> Bruce, what do you think?
> 
Hi,

from my experience this mainly seems to affect the bus drivers,
specifically pci and vdev buses, which seem to be assumed to be always
enabled. I agree it's probably not a real problem right now, though it
would be nice for some testing purposes to have a build possible with
"disable_drivers=*/*". For a solution I am happy for us to have whichever
is easiest to implement - either refusing disabling of bus/pci and
bus/vdev, or fixing the tests and apps to allow them to be built with
reduced functionality.

Apart from those two drivers, I would hope that disabling everything else
works. If not, we should definitely fix it.

/Bruce
Juraj Linkeš March 31, 2021, 9:14 a.m. UTC | #21
> -----Original Message-----
> From: Bruce Richardson <bruce.richardson@intel.com>
> Sent: Wednesday, March 31, 2021 11:08 AM
> To: Juraj Linkeš <juraj.linkes@pantheon.tech>
> Cc: Honnappa Nagarahalli <Honnappa.Nagarahalli@arm.com>; Jerin Jacob
> <jerinjacobk@gmail.com>; Ruifeng Wang <Ruifeng.Wang@arm.com>;
> vcchunga@amazon.com; Dharmik Thakkar <Dharmik.Thakkar@arm.com>;
> hemant.agrawal@nxp.com; Ajit Khaparde (ajit.khaparde@broadcom.com)
> <ajit.khaparde@broadcom.com>; ferruh.yigit@intel.com; aboyer@pensando.io;
> lironh@marvell.com; dev@dpdk.org; nd <nd@arm.com>
> Subject: Re: [PATCH v16 1/3] build: disable/enable drivers in Arm builds
> 
> On Wed, Mar 31, 2021 at 08:39:57AM +0000, Juraj Linkeš wrote:
> >
> >
> > > -----Original Message-----
> > > From: Honnappa Nagarahalli <Honnappa.Nagarahalli@arm.com>
> > > Sent: Tuesday, March 30, 2021 2:39 AM
> > > To: Jerin Jacob <jerinjacobk@gmail.com>; Juraj Linkeš
> > > <juraj.linkes@pantheon.tech>
> > > Cc: Bruce Richardson <bruce.richardson@intel.com>; Ruifeng Wang
> > > <Ruifeng.Wang@arm.com>; vcchunga@amazon.com; Dharmik Thakkar
> > > <Dharmik.Thakkar@arm.com>; hemant.agrawal@nxp.com; Ajit Khaparde
> > > (ajit.khaparde@broadcom.com) <ajit.khaparde@broadcom.com>;
> > > ferruh.yigit@intel.com; aboyer@pensando.io; lironh@marvell.com;
> > > dev@dpdk.org; nd <nd@arm.com>; Honnappa Nagarahalli
> > > <Honnappa.Nagarahalli@arm.com>; nd <nd@arm.com>
> > > Subject: RE: [PATCH v16 1/3] build: disable/enable drivers in Arm
> > > builds
> > >
> > > <snip>
> > >
> > > > > <removed parts which I think are not that relevant>
> > > > >
> > > > > > > > > > > > > The blocklist is, I think, agreed upon by everyone.
> > > > > > > > > > > > > The question is whether we want to support the
> > > > > > > > > > > > > allowlist alongside it and there seem to be
> > > > > > > > > > > > > enough reasons to do
> > > > that.
> > > > > > > > > > > > Sorry, may be this is answered already, but, what
> > > > > > > > > > > > additional benefit does allowlist provide over the blocklist?
> > > > > > > > > > > >
> > > > > > > > > > >
> > > > > > > > > > > VPP could use it:
> > > > > > > > > > > https://gerrit.fd.io/r/gitweb?p=vpp.git;a=blob;f=bui
> > > > > > > > > > > ld/e
> > > > > > > > > > > xt
> > > > > > > > > > > erna
> > > > > > > > > > > l/pa
> > > > > > > > > > > ckag es/dpdk
> > > > > > > > > > > .mk;h=c35ac84c27b19411a0cfdf9a3524fdf36024762c;hb=HE
> > > > > > > > > > > AD
> > > > > > > > > > >
> > > > > > > > > > > They're disabling almost everything so an allowlist would fit
> there.
> > > > > > > > > > > And they won't need to update the list when a new
> > > > > > > > > > > driver is added (which they won't need).
> > > > > > > > > > This is different from how we started this discussion.
> > > > > > > > > > The current discussion was for DPDK internal use. But
> > > > > > > > > > the one you are referencing above is for users of
> > > > > > > > > > DPDK. I am fine for providing the allow list for the
> > > > > > > > > > users of DPDK. But for DPDK internal, I think block
> > > > > > > > > > list is
> > > > > > > > enough.
> > > > > > > > > >
> > > > > > > > >
> > > > > > > > > That's an interesting suggestion. Jerin, what do you think?
> > > > > > > > > Why did you
> > > > > > > > want to have an allowlist? Would this work?
> > > > > > > >
> > > > > > > > # The very reason why VPP chooses to have allow list so
> > > > > > > > that they can control what needs to include.
> > > > > > > > # Another use case is like, in SoCs have fixed internal
> > > > > > > > devices, we can have optimized build for that can have
> > > > > > > > only allow list of the drivers that care about # For
> > > > > > > > server market, block list makes sense # For embedded SoC, allow
> list makes sense.
> > > > > > > For the embedded SoC, IMO, the upstream project could allow
> > > > > > > the compilation
> > > > > > for wider set of PMDs/libs. May be the end customer can use
> > > > > > the allow list to compile/use what is required?
> > > > > >
> > > > > > Just to understand, how end customer can enable allow list, if
> > > > > > DPDK build system does not support it?
> > > > > > Also to understand, If we are supporting blocklist, why not
> > > > > > have allowlist (I mean, both of them) as both are required as
> > > > > > it caters different use case as mention above. We can not
> > > > > > emulate allowlist with blocklist as each version of DPDK will
> > > > > > have new libraries and PMD's
> > > > which end user has no clue. Right?
> > > > >
> > > > > > I think, that is the reason why VPP is doing the allow list.
> > > > >
> > > > > I'm not sure what you mean by this, but to clarify, VPP likely
> > > > > would be using
> > > > the allowlist in this fashion, but that is not an arm specific
> > > > usecase. I think what Honnappa wanted to see was how the allowlist
> > > > could be used in an arm usecase (such as using it in an SoC configuration).
> > > >
> > > > There is nothing arm-specific here. Right? allowlist will be
> > > > common and will be used by all architecture. Right?
> > > Nothing Arm specific. I think for generic Arm servers platforms we
> > > can make sure that we allow for compilation of all the DPDK code. We
> > > can go ahead with implementing the allow list.
> > >
> >
> > I tried to actually use an allowlist and there are some problems with building
> apps and tests. When I tried to enable a random driver, such as
> common/sfc_efx, I've ran into dependency issues with apps:
> > app/meson.build:53:3: ERROR:  Tried to get unknown variable
> "static_rte_bus_vdev".
> >
> > This is because bus/vdev is not enabled (the allowlist only enabled
> net/sfc_efx). When I implemented dependency discovery in apps similar to
> which exists for drivers/libs, I was then unable to build tests (which is a special
> case app):
> > app/test/meson.build:427:1: ERROR:  Tried to get unknown variable
> "static_rte_bus_pci".
> >
> > This is becasue bus/pci is not enabled. If I understand the code correctly, the
> test dependencies are not matched to each test, meaning we can't disable the
> tests for which we don't have dependencies - we can only disable all tests.
> >
> > In general, this problem also affects the blocklist, i.e. meson build
> > -Ddisable_drivers=bus/pci produces
> > drivers/net/hinic/base/meson.build:34:0: ERROR:  Unknown variable
> "static_rte_bus_pci".
> > For blocklists, this is seldom going to be a problem, since most users won't
> disable "core" drivers.
> >
> > I'm not sure what is the best course of action. We can implement the allowlist
> and then we'll leave it up to implementers to implement allowlists that produce
> a working build. We could then optionally address the dependency issues
> brought by disabled drivers in a separate patch.
> >
> > Maybe we can just have a list of "core" drivers that can never be disabled
> (either using blocklists or allowlists)?
> >
> > Bruce, what do you think?
> >
> Hi,
> 
> from my experience this mainly seems to affect the bus drivers, specifically pci
> and vdev buses, which seem to be assumed to be always enabled. I agree it's
> probably not a real problem right now, though it would be nice for some testing
> purposes to have a build possible with "disable_drivers=*/*". For a solution I am
> happy for us to have whichever is easiest to implement - either refusing disabling
> of bus/pci and bus/vdev, or fixing the tests and apps to allow them to be built
> with reduced functionality.
> 
> Apart from those two drivers, I would hope that disabling everything else works.
> If not, we should definitely fix it.

I tried with just those two enabled and it works. I'll introduce a list of drivers that won't ever be disabled (ther other solution is more complicated and I think out of scope of this patch) and submit a new version.

> 
> /Bruce
diff mbox series

Patch

diff --git a/config/arm/meson.build b/config/arm/meson.build
index f948768578..d279724dec 100644
--- a/config/arm/meson.build
+++ b/config/arm/meson.build
@@ -16,9 +16,6 @@  flags_common = [
 	#	['RTE_ARM64_MEMCPY_ALIGN_MASK', 0xF],
 	#	['RTE_ARM64_MEMCPY_STRICT_ALIGN', false],
 
-	['RTE_NET_FM10K', false],
-	['RTE_NET_AVP', false],
-
 	['RTE_SCHED_VECTOR', false],
 	['RTE_ARM_USE_WFE', false],
 	['RTE_ARCH_ARM64', true],
@@ -125,7 +122,6 @@  implementer_cavium = {
 				['RTE_MACHINE', '"octeontx2"'],
 				['RTE_ARM_FEATURE_ATOMICS', true],
 				['RTE_USE_C11_MEM_MODEL', true],
-				['RTE_EAL_IGB_UIO', false],
 				['RTE_MAX_LCORE', 36],
 				['RTE_MAX_NUMA_NODES', 1]
 			]
diff --git a/doc/guides/linux_gsg/cross_build_dpdk_for_arm64.rst b/doc/guides/linux_gsg/cross_build_dpdk_for_arm64.rst
index faaf24b95b..1504dbfef0 100644
--- a/doc/guides/linux_gsg/cross_build_dpdk_for_arm64.rst
+++ b/doc/guides/linux_gsg/cross_build_dpdk_for_arm64.rst
@@ -234,3 +234,11 @@  There are other options you may specify in a cross file to tailor the build::
       numa = false        # set to false to force building for a non-NUMA system
          # if not set or set to true, the build system will build for a NUMA
          # system only if libnuma is installed
+
+      disabled_drivers = ['bus/dpaa', 'crypto/*']  # add disabled drivers
+         # valid values are dir/subdirs in the drivers directory
+         # wildcards are allowed
+
+      enabled_drivers = ['common/*', 'bus/*']  # build only these drivers
+         # valid values are dir/subdirs in the drivers directory
+         # wildcards are allowed
diff --git a/drivers/meson.build b/drivers/meson.build
index fdf76120ac..70c1aa4e6c 100644
--- a/drivers/meson.build
+++ b/drivers/meson.build
@@ -18,8 +18,36 @@  subdirs = [
 	'baseband', # depends on common and bus.
 ]
 
-disabled_drivers = run_command(list_dir_globs, get_option('disable_drivers'),
-		).stdout().split()
+if meson.is_cross_build()
+	disabled_drivers += meson.get_cross_property('disabled_drivers', [])
+	enabled_drivers += meson.get_cross_property('enabled_drivers', [])
+endif
+
+# add cmdline disabled drivers (comma separated string)
+# and meson disabled drivers (list)
+# together into a comma separated string
+disabled_drivers = ','.join([get_option('disable_drivers'), ','.join(disabled_drivers)]).strip(',')
+if disabled_drivers != ''
+	disabled_drivers = run_command(list_dir_globs,
+		disabled_drivers).stdout().split()
+else
+	disabled_drivers = []
+endif
+
+if enabled_drivers != []
+	enabled_drivers = run_command(list_dir_globs,
+		','.join(enabled_drivers)).stdout().split()
+endif
+
+if disabled_drivers != [] and enabled_drivers != []
+	# TODO/QUERY we could support both:
+	# first 'select' only drivers by enabled_drivers
+	# then 'deselect' those in disabled_drivers
+	# this would be useful if a directory is in enabled_drivers
+	# and a driver from that directory is in disabled_drivers
+	error('Simultaneous disabled drivers and enabled drivers ' +
+	      'configuration is not supported.')
+endif
 
 default_cflags = machine_args
 default_cflags += ['-DALLOW_EXPERIMENTAL_API']
@@ -48,7 +76,7 @@  foreach subpath:subdirs
 		dpdk_driver_classes += class
 	endif
 	# get already enabled drivers of the same class
-	enabled_drivers = get_variable(class + '_drivers', [])
+	enabled_class_drivers = get_variable(class + '_drivers', [])
 
 	foreach drv:drivers
 		drv_path = join_paths(class, drv)
@@ -76,6 +104,9 @@  foreach subpath:subdirs
 		if disabled_drivers.contains(drv_path)
 			build = false
 			reason = 'explicitly disabled via build config'
+		elif enabled_drivers.length() > 0 and not enabled_drivers.contains(drv_path)
+			build = false
+			reason = 'not in enabled drivers build config'
 		else
 			# pull in driver directory which should update all the local variables
 			subdir(drv_path)
@@ -108,7 +139,7 @@  foreach subpath:subdirs
 						'_disable_reason', reason)
 			endif
 		else
-			enabled_drivers += name
+			enabled_class_drivers += name
 			lib_name = '_'.join(['rte', class, name])
 			dpdk_conf.set(lib_name.to_upper(), 1)
 
@@ -213,5 +244,5 @@  foreach subpath:subdirs
 		endif # build
 	endforeach
 
-	set_variable(class + '_drivers', enabled_drivers)
+	set_variable(class + '_drivers', enabled_class_drivers)
 endforeach
diff --git a/meson.build b/meson.build
index fcc4d4c900..ea7ccfdae3 100644
--- a/meson.build
+++ b/meson.build
@@ -22,6 +22,8 @@  dpdk_drivers = []
 dpdk_extra_ldflags = []
 dpdk_libs_disabled = []
 dpdk_drvs_disabled = []
+disabled_drivers = []
+enabled_drivers = []
 abi_version_file = files('ABI_VERSION')
 
 if host_machine.cpu_family().startswith('x86')