[PATCH v1] config: make max memzones definition configurable

[PATCH v1] config: make max memzones definition configurable

[Ophir Munk]

In current DPDK the RTE_MAX_MEMZONE definition is unconditionally hard
coded as 2560.  For applications requiring different values of this
parameter – it is more convenient to set its value as part of the meson
command line rather than changing the dpdk source code per application.
An example would be of an application that uses the DPDK mempool library
which is based on DPDK memzone library.  The application may need to
create a number of steering tables, each of which will require its own
mempool allocation.
This commit adds a meson optional parameter named max_memzones. If not
specified - it is set by default to 2560. The hard coded definition of
RTE_MAX_MEMZONE is removed. During meson build time the RTE_MAX_MEMZONE
can be optionally defined as the value of max_memzones parameter.

Signed-off-by: Ophir Munk <ophirmu@nvidia.com>
---
RFC:
https://patchwork.dpdk.org/project/dpdk/patch/20230130092302.376145-1-ophirmu@nvidia.com/

 config/meson.build  | 1 +
 config/rte_config.h | 1 -
 meson_options.txt   | 2 ++
 3 files changed, 3 insertions(+), 1 deletion(-)

diff --git a/config/meson.build b/config/meson.build
index 26f3168..b55390f 100644
--- a/config/meson.build
+++ b/config/meson.build
@@ -304,6 +304,7 @@ endforeach
 dpdk_conf.set('RTE_MAX_ETHPORTS', get_option('max_ethports'))
 dpdk_conf.set('RTE_LIBEAL_USE_HPET', get_option('use_hpet'))
 dpdk_conf.set('RTE_ENABLE_TRACE_FP', get_option('enable_trace_fp'))
+dpdk_conf.set('RTE_MAX_MEMZONE', get_option('max_memzones'))
 # values which have defaults which may be overridden
 dpdk_conf.set('RTE_MAX_VFIO_GROUPS', 64)
 dpdk_conf.set('RTE_DRIVER_MEMPOOL_BUCKET_SIZE_KB', 64)
diff --git a/config/rte_config.h b/config/rte_config.h
index 7b8c85e..400e44e 100644
--- a/config/rte_config.h
+++ b/config/rte_config.h
@@ -34,7 +34,6 @@
 #define RTE_MAX_MEM_MB_PER_LIST 32768
 #define RTE_MAX_MEMSEG_PER_TYPE 32768
 #define RTE_MAX_MEM_MB_PER_TYPE 65536
-#define RTE_MAX_MEMZONE 2560
 #define RTE_MAX_TAILQ 32
 #define RTE_LOG_DP_LEVEL RTE_LOG_INFO
 #define RTE_MAX_VFIO_CONTAINERS 64
diff --git a/meson_options.txt b/meson_options.txt
index 0852849..62888fe 100644
--- a/meson_options.txt
+++ b/meson_options.txt
@@ -36,6 +36,8 @@ option('machine', type: 'string', value: 'auto', description:
        'Alias of cpu_instruction_set.')
 option('max_ethports', type: 'integer', value: 32, description:
        'maximum number of Ethernet devices')
+option('max_memzones', type: 'integer', value: 2560, description:
+       'maximum number of memory zones supported by EAL')
 option('max_lcores', type: 'string', value: 'default', description:
        'Set maximum number of cores/threads supported by EAL; "default" is different per-arch, "detect" detects the number of cores on the build machine.')
 option('max_numa_nodes', type: 'string', value: 'default', description:
-- 
2.8.4

Re: [PATCH v1] config: make max memzones definition configurable

[Bruce Richardson]

On Sun, Feb 12, 2023 at 10:53:19AM +0200, Ophir Munk wrote:
> In current DPDK the RTE_MAX_MEMZONE definition is unconditionally hard
> coded as 2560.  For applications requiring different values of this
> parameter – it is more convenient to set its value as part of the meson
> command line rather than changing the dpdk source code per application.
> An example would be of an application that uses the DPDK mempool library
> which is based on DPDK memzone library.  The application may need to
> create a number of steering tables, each of which will require its own
> mempool allocation.
> This commit adds a meson optional parameter named max_memzones. If not
> specified - it is set by default to 2560. The hard coded definition of
> RTE_MAX_MEMZONE is removed. During meson build time the RTE_MAX_MEMZONE
> can be optionally defined as the value of max_memzones parameter.
> 
> Signed-off-by: Ophir Munk <ophirmu@nvidia.com>
> ---
> RFC:
> https://patchwork.dpdk.org/project/dpdk/patch/20230130092302.376145-1-ophirmu@nvidia.com/
> 
>  config/meson.build  | 1 +
>  config/rte_config.h | 1 -
>  meson_options.txt   | 2 ++
>  3 files changed, 3 insertions(+), 1 deletion(-)
> 
Acked-by: Bruce Richardson <bruce.richardson@intel.com>

Re: [PATCH v1] config: make max memzones definition configurable

[Thomas Monjalon]

13/02/2023 12:05, Bruce Richardson:
> On Sun, Feb 12, 2023 at 10:53:19AM +0200, Ophir Munk wrote:
> > In current DPDK the RTE_MAX_MEMZONE definition is unconditionally hard
> > coded as 2560.  For applications requiring different values of this
> > parameter – it is more convenient to set its value as part of the meson
> > command line rather than changing the dpdk source code per application.
> > An example would be of an application that uses the DPDK mempool library
> > which is based on DPDK memzone library.  The application may need to
> > create a number of steering tables, each of which will require its own
> > mempool allocation.
> > This commit adds a meson optional parameter named max_memzones. If not
> > specified - it is set by default to 2560. The hard coded definition of
> > RTE_MAX_MEMZONE is removed. During meson build time the RTE_MAX_MEMZONE
> > can be optionally defined as the value of max_memzones parameter.
> > 
> > Signed-off-by: Ophir Munk <ophirmu@nvidia.com>
> > ---
> > RFC:
> > https://patchwork.dpdk.org/project/dpdk/patch/20230130092302.376145-1-ophirmu@nvidia.com/
> > 
> >  config/meson.build  | 1 +
> >  config/rte_config.h | 1 -
> >  meson_options.txt   | 2 ++
> >  3 files changed, 3 insertions(+), 1 deletion(-)
> > 
> Acked-by: Bruce Richardson <bruce.richardson@intel.com>

Are we going to move all compilation-defined settings to meson_options.txt?
The direction discussed in recent years was to configure things at runtime,
and stop adding compilation-time settings.

In this case, it is quite easy to add a new function
	void rte_memzone_set_max(int max)
to be called before rte_eal_init().
If not called, the historical default is used.

Re: [PATCH v1] config: make max memzones definition configurable

[Bruce Richardson]

On Mon, Feb 13, 2023 at 02:55:41PM +0100, Thomas Monjalon wrote:
> 13/02/2023 12:05, Bruce Richardson:
> > On Sun, Feb 12, 2023 at 10:53:19AM +0200, Ophir Munk wrote:
> > > In current DPDK the RTE_MAX_MEMZONE definition is unconditionally
> > > hard coded as 2560.  For applications requiring different values of
> > > this parameter – it is more convenient to set its value as part of
> > > the meson command line rather than changing the dpdk source code per
> > > application.  An example would be of an application that uses the
> > > DPDK mempool library which is based on DPDK memzone library.  The
> > > application may need to create a number of steering tables, each of
> > > which will require its own mempool allocation.  This commit adds a
> > > meson optional parameter named max_memzones. If not specified - it is
> > > set by default to 2560. The hard coded definition of RTE_MAX_MEMZONE
> > > is removed. During meson build time the RTE_MAX_MEMZONE can be
> > > optionally defined as the value of max_memzones parameter.
> > > 
> > > Signed-off-by: Ophir Munk <ophirmu@nvidia.com> --- RFC:
> > > https://patchwork.dpdk.org/project/dpdk/patch/20230130092302.376145-1-ophirmu@nvidia.com/
> > > 
> > >  config/meson.build  | 1 + config/rte_config.h | 1 -
> > >  meson_options.txt   | 2 ++ 3 files changed, 3 insertions(+), 1
> > >  deletion(-)
> > > 
> > Acked-by: Bruce Richardson <bruce.richardson@intel.com>
> 
> Are we going to move all compilation-defined settings to
> meson_options.txt?  The direction discussed in recent years was to
> configure things at runtime, and stop adding compilation-time settings.
> 
> In this case, it is quite easy to add a new function void
> rte_memzone_set_max(int max) to be called before rte_eal_init().  If not
> called, the historical default is used.
> 
Good point, I admit I had forgotten that.

Looking at the use of RTE_MAX_MEMZONE, it is used as an array dimension in
a number of places, but, from what I see on cursory examination, it should
be replacable with a runtime value without significant pain in most cases.
The one that probably needs more attention is the fact that the "net/qede"
driver maintains an array of memzones in it's base-code layer. Therefore,
we probably need input from that driver maintainer to know the impact there
and why that array is needed in a net driver. [Adding the two maintainers
on CC]

/Bruce

RE: [PATCH v1] config: make max memzones definition configurable

[Ophir Munk]

Since the new rte API was "discussed in recent years" and it is also dependent on different driver vendors acceptance - I suggest that the compilation option will be applied now.
The new rte API effort will start in parallel. Once accepted - it will replace the compilation option.

> -----Original Message-----
> From: Bruce Richardson <bruce.richardson@intel.com>
> Sent: Monday, 13 February 2023 16:53
> To: NBU-Contact-Thomas Monjalon (EXTERNAL) <thomas@monjalon.net>
> Cc: Ophir Munk <ophirmu@nvidia.com>; dev@dpdk.org; Matan Azrad
> <matan@nvidia.com>; Lior Margalit <lmargalit@nvidia.com>; Asaf Penso
> <asafp@nvidia.com>; david.marchand@redhat.com;
> honnappa.nagarahalli@arm.com; jerinj@marvell.com; rmody@marvell.com;
> dsinghrawat@marvell.com
> Subject: Re: [PATCH v1] config: make max memzones definition configurable
> 
> On Mon, Feb 13, 2023 at 02:55:41PM +0100, Thomas Monjalon wrote:
> > 13/02/2023 12:05, Bruce Richardson:
> > > On Sun, Feb 12, 2023 at 10:53:19AM +0200, Ophir Munk wrote:
> > > > In current DPDK the RTE_MAX_MEMZONE definition is unconditionally
> > > > hard coded as 2560.  For applications requiring different values
> > > > of this parameter – it is more convenient to set its value as part
> > > > of the meson command line rather than changing the dpdk source
> > > > code per application.  An example would be of an application that
> > > > uses the DPDK mempool library which is based on DPDK memzone
> > > > library.  The application may need to create a number of steering
> > > > tables, each of which will require its own mempool allocation.
> > > > This commit adds a meson optional parameter named max_memzones.
> If
> > > > not specified - it is set by default to 2560. The hard coded
> > > > definition of RTE_MAX_MEMZONE is removed. During meson build time
> > > > the RTE_MAX_MEMZONE can be optionally defined as the value of
> max_memzones parameter.
> > > >
> > > > Signed-off-by: Ophir Munk <ophirmu@nvidia.com> --- RFC:
> > > >
> https://patchwork.dpdk.org/project/dpdk/patch/20230130092302.37614
> > > > 5-1-ophirmu@nvidia.com/
> > > >
> > > >  config/meson.build  | 1 + config/rte_config.h | 1 -
> > > >  meson_options.txt   | 2 ++ 3 files changed, 3 insertions(+), 1
> > > >  deletion(-)
> > > >
> > > Acked-by: Bruce Richardson <bruce.richardson@intel.com>
> >
> > Are we going to move all compilation-defined settings to
> > meson_options.txt?  The direction discussed in recent years was to
> > configure things at runtime, and stop adding compilation-time settings.
> >
> > In this case, it is quite easy to add a new function void
> > rte_memzone_set_max(int max) to be called before rte_eal_init().  If
> > not called, the historical default is used.
> >
> Good point, I admit I had forgotten that.
> 
> Looking at the use of RTE_MAX_MEMZONE, it is used as an array dimension
> in a number of places, but, from what I see on cursory examination, it should
> be replacable with a runtime value without significant pain in most cases.
> The one that probably needs more attention is the fact that the "net/qede"
> driver maintains an array of memzones in it's base-code layer. Therefore, we
> probably need input from that driver maintainer to know the impact there
> and why that array is needed in a net driver. [Adding the two maintainers on
> CC]
> 
> /Bruce

Re: [PATCH v1] config: make max memzones definition configurable

[Thomas Monjalon]

13/02/2023 18:04, Ophir Munk:
> Since the new rte API was "discussed in recent years" and it is also dependent on different driver vendors acceptance - I suggest that the compilation option will be applied now.

I think there is a misunderstanding here.

> The new rte API effort will start in parallel. Once accepted - it will replace the compilation option.

The effort is just adding a single simple function.
I can work on it.


> From: Bruce Richardson <bruce.richardson@intel.com>
> > On Mon, Feb 13, 2023 at 02:55:41PM +0100, Thomas Monjalon wrote:
> > > 13/02/2023 12:05, Bruce Richardson:
> > > > On Sun, Feb 12, 2023 at 10:53:19AM +0200, Ophir Munk wrote:
> > > > > In current DPDK the RTE_MAX_MEMZONE definition is unconditionally
> > > > > hard coded as 2560.  For applications requiring different values
> > > > > of this parameter – it is more convenient to set its value as part
> > > > > of the meson command line rather than changing the dpdk source
> > > > > code per application.  An example would be of an application that
> > > > > uses the DPDK mempool library which is based on DPDK memzone
> > > > > library.  The application may need to create a number of steering
> > > > > tables, each of which will require its own mempool allocation.
> > > > > This commit adds a meson optional parameter named max_memzones.
> > If
> > > > > not specified - it is set by default to 2560. The hard coded
> > > > > definition of RTE_MAX_MEMZONE is removed. During meson build time
> > > > > the RTE_MAX_MEMZONE can be optionally defined as the value of
> > max_memzones parameter.
> > > > >
> > > > > Signed-off-by: Ophir Munk <ophirmu@nvidia.com> --- RFC:
> > > > >
> > https://patchwork.dpdk.org/project/dpdk/patch/20230130092302.37614
> > > > > 5-1-ophirmu@nvidia.com/
> > > > >
> > > > >  config/meson.build  | 1 + config/rte_config.h | 1 -
> > > > >  meson_options.txt   | 2 ++ 3 files changed, 3 insertions(+), 1
> > > > >  deletion(-)
> > > > >
> > > > Acked-by: Bruce Richardson <bruce.richardson@intel.com>
> > >
> > > Are we going to move all compilation-defined settings to
> > > meson_options.txt?  The direction discussed in recent years was to
> > > configure things at runtime, and stop adding compilation-time settings.
> > >
> > > In this case, it is quite easy to add a new function void
> > > rte_memzone_set_max(int max) to be called before rte_eal_init().  If
> > > not called, the historical default is used.
> > >
> > Good point, I admit I had forgotten that.
> > 
> > Looking at the use of RTE_MAX_MEMZONE, it is used as an array dimension
> > in a number of places, but, from what I see on cursory examination, it should
> > be replacable with a runtime value without significant pain in most cases.
> > The one that probably needs more attention is the fact that the "net/qede"
> > driver maintains an array of memzones in it's base-code layer. Therefore, we
> > probably need input from that driver maintainer to know the impact there
> > and why that array is needed in a net driver. [Adding the two maintainers on
> > CC]
> > 
> > /Bruce
>