[RFC] build: allow passing extra config header to build

  To allow per-build override of some settings, without having to change
DPDK source-code files, i.e. rte_config.h, we can add an option to allow
the user to pass in a file containing their own defines for the build.

Signed-off-by: Bruce Richardson <bruce.richardson@intel.com>
---
This is just a quick RFC to show what might be possible and to help keep the
discussion going on how to improve build config in DPDK!
---
 config/meson.build  | 10 ++++++++++
 config/rte_config.h |  4 ++++
 meson_options.txt   |  2 ++
 3 files changed, 16 insertions(+)

--
2.25.1
  

<snip>

> 
> To allow per-build override of some settings, without having to change DPDK
> source-code files, i.e. rte_config.h, we can add an option to allow the user to
> pass in a file containing their own defines for the build.
I guess, the file format should be same as any header file.
One could derive a file from rte_config.h. I think this will provide good flexibility.
If this is provided, we could deprecate options such as max_lcores, max_numa_nodes. But it would mean, one needs to know the exact #defines.

> 
> Signed-off-by: Bruce Richardson <bruce.richardson@intel.com>
> ---
> This is just a quick RFC to show what might be possible and to help keep the
> discussion going on how to improve build config in DPDK!
> ---
>  config/meson.build  | 10 ++++++++++
>  config/rte_config.h |  4 ++++
>  meson_options.txt   |  2 ++
>  3 files changed, 16 insertions(+)
> 
> diff --git a/config/meson.build b/config/meson.build index
> 258b01d06..5c137e4f5 100644
> --- a/config/meson.build
> +++ b/config/meson.build
> @@ -14,6 +14,16 @@ foreach env:supported_exec_envs
>  	set_variable('is_' + env, exec_env == env)  endforeach
> 
> +if get_option('extra_config') != ''
> +	extra_config = files(get_option('extra_config'))
> +	configure_file(copy: true,
> +			input: extra_config,
> +			output: 'rte_build_config_extra.h',
> +			install_dir: join_paths(get_option('includedir'),
> +				get_option('include_subdir_arch')))
> +	dpdk_conf.set('RTE_BUILD_CONFIG_EXTRA', 1) endif
> +
>  # MS linker requires special treatment.
>  # TODO: use cc.get_linker_id() with Meson >= 0.54  is_ms_linker =
> is_windows and (cc.get_id() == 'clang') diff --git a/config/rte_config.h
> b/config/rte_config.h index 25219f04a..2f5ecf999 100644
> --- a/config/rte_config.h
> +++ b/config/rte_config.h
> @@ -148,4 +148,8 @@
>  #define RTE_LIBRTE_PMD_DLB2_SW_CREDIT_QUANTA 32  #define
> RTE_PMD_DLB2_DEFAULT_DEPTH_THRESH 256
> 
> +#ifdef RTE_BUILD_CONFIG_EXTRA
> +#include <rte_build_config_extra.h>
> +#endif
> +
>  #endif /* _RTE_CONFIG_H_ */
> diff --git a/meson_options.txt b/meson_options.txt index
> 9bf18ab6b..91319b0e4 100644
> --- a/meson_options.txt
> +++ b/meson_options.txt
> @@ -2,6 +2,8 @@
> 
>  option('armv8_crypto_dir', type: 'string', value: '',
>  	description: 'path to the armv8_crypto library installation directory')
> +option('extra_config', type: 'string', value: '',
> +	description: 'path to a header file with extra build defines. Will be
> +installed as rte_build_config_extra.h')
>  option('disable_drivers', type: 'string', value: '',
>  	description: 'Comma-separated list of drivers to explicitly disable.')
> option('drivers_install_subdir', type: 'string', value: 'dpdk/pmds-<VERSION>',
> --
> 2.25.1
  

On Fri, Nov 13, 2020 at 04:55:57AM +0000, Honnappa Nagarahalli wrote:
> <snip>
> 
> > 
> > To allow per-build override of some settings, without having to change
> > DPDK source-code files, i.e. rte_config.h, we can add an option to
> > allow the user to pass in a file containing their own defines for the
> > build.
> I guess, the file format should be same as any header file.  One could
> derive a file from rte_config.h. I think this will provide good
> flexibility.  If this is provided, we could deprecate options such as
> max_lcores, max_numa_nodes. But it would mean, one needs to know the
> exact #defines.
>
Yep, fully agree and I definitely think we should look to remove some
options like those.

 However, this is just one possibility of the way we can do things, so I'd
like to see other prototype suggestions also proposed here on-list (or
suggested approaches just outlined in email such as proposed by David
Harton).  I also will add this to the techboard agenda for future
discussion, so that we can do some brain-storming as to how best to improve
this area, and discuss any prototypes covered on-list.

/Bruce
  

> -----Original Message-----
> From: dev <dev-bounces@dpdk.org> On Behalf Of Bruce Richardson
> Sent: Thursday, November 12, 2020 11:32 AM
> To: dev@dpdk.org
> Cc: Bruce Richardson <bruce.richardson@intel.com>
> Subject: [dpdk-dev] [RFC PATCH] build: allow passing extra config header
> to build
> 
> To allow per-build override of some settings, without having to change
> DPDK source-code files, i.e. rte_config.h, we can add an option to allow
> the user to pass in a file containing their own defines for the build.
> 

This is definitely better than what we have now.

Even going forward with this as a temp solution would be good while we work out longer-term strategy would be good.

What I believe is missing in the existing code base:
- knowing what all the knobs are
- ability to tune knobs in a build specific manner (providing per build config)

Regards,
Dave

> Signed-off-by: Bruce Richardson <bruce.richardson@intel.com>
> ---
> This is just a quick RFC to show what might be possible and to help keep
> the discussion going on how to improve build config in DPDK!
> ---
>  config/meson.build  | 10 ++++++++++
>  config/rte_config.h |  4 ++++
>  meson_options.txt   |  2 ++
>  3 files changed, 16 insertions(+)
> 
> diff --git a/config/meson.build b/config/meson.build index
> 258b01d06..5c137e4f5 100644
> --- a/config/meson.build
> +++ b/config/meson.build
> @@ -14,6 +14,16 @@ foreach env:supported_exec_envs
>  	set_variable('is_' + env, exec_env == env)  endforeach
> 
> +if get_option('extra_config') != ''
> +	extra_config = files(get_option('extra_config'))
> +	configure_file(copy: true,
> +			input: extra_config,
> +			output: 'rte_build_config_extra.h',
> +			install_dir: join_paths(get_option('includedir'),
> +				get_option('include_subdir_arch')))
> +	dpdk_conf.set('RTE_BUILD_CONFIG_EXTRA', 1) endif
> +
>  # MS linker requires special treatment.
>  # TODO: use cc.get_linker_id() with Meson >= 0.54  is_ms_linker =
> is_windows and (cc.get_id() == 'clang') diff --git a/config/rte_config.h
> b/config/rte_config.h index 25219f04a..2f5ecf999 100644
> --- a/config/rte_config.h
> +++ b/config/rte_config.h
> @@ -148,4 +148,8 @@
>  #define RTE_LIBRTE_PMD_DLB2_SW_CREDIT_QUANTA 32  #define
> RTE_PMD_DLB2_DEFAULT_DEPTH_THRESH 256
> 
> +#ifdef RTE_BUILD_CONFIG_EXTRA
> +#include <rte_build_config_extra.h>
> +#endif
> +
>  #endif /* _RTE_CONFIG_H_ */
> diff --git a/meson_options.txt b/meson_options.txt index
> 9bf18ab6b..91319b0e4 100644
> --- a/meson_options.txt
> +++ b/meson_options.txt
> @@ -2,6 +2,8 @@
> 
>  option('armv8_crypto_dir', type: 'string', value: '',
>  	description: 'path to the armv8_crypto library installation
> directory')
> +option('extra_config', type: 'string', value: '',
> +	description: 'path to a header file with extra build defines. Will
> be
> +installed as rte_build_config_extra.h')
>  option('disable_drivers', type: 'string', value: '',
>  	description: 'Comma-separated list of drivers to explicitly
> disable.')  option('drivers_install_subdir', type: 'string', value:
> 'dpdk/pmds-<VERSION>',
> --
> 2.25.1