Re: [RFC] lib: set/get max memzone segments

[Thomas Monjalon <thomas@monjalon.net>]

21/04/2023 13:08, Morten Brørup:
> > From: Thomas Monjalon [mailto:thomas@monjalon.net]
> > Sent: Friday, 21 April 2023 10.35
> > 20/04/2023 20:20, Tyler Retzlaff:
> > > On Thu, Apr 20, 2023 at 09:43:28AM +0200, Thomas Monjalon wrote:
> > > > 19/04/2023 16:51, Tyler Retzlaff:
> > > > > On Wed, Apr 19, 2023 at 11:36:34AM +0300, 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 the max value via an rte API
> > -
> > > > > > rather than changing the dpdk source code per application.  In many
> > > > > > organizations, the possibility to compile a private DPDK library for a
> > > > > > particular application does not exist at all.  With this option there
> > is
> > > > > > no need to recompile DPDK and it allows using an in-box packaged DPDK.
> > > > > > An example usage for updating the RTE_MAX_MEMZONE 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 is not about how to optimize the application usage of
> > > > > > mempool nor about how to improve the mempool implementation based on
> > > > > > memzone.  It is about how to make the max memzone definition - run-
> > time
> > > > > > customized.
> > > > > > This commit adds an API which must be called before rte_eal_init():
> > > > > > rte_memzone_max_set(int max).  If not called, the default memzone
> > > > > > (RTE_MAX_MEMZONE) is used.  There is also an API to query the
> > effective
> > > > > > max memzone: rte_memzone_max_get().
> > > > > >
> > > > > > Signed-off-by: Ophir Munk <ophirmu@nvidia.com>
> > > > > > ---
> > > > >
> > > > > the use case of each application may want a different non-hard coded
> > > > > value makes sense.
> > > > >
> > > > > it's less clear to me that requiring it be called before eal init makes
> > > > > sense over just providing it as configuration to eal init so that it is
> > > > > composed.
> > > >
> > > > Why do you think it would be better as EAL init option?
> > > > From an API perspective, I think it is simpler to call a dedicated
> > function.
> > > > And I don't think a user wants to deal with it when starting the
> > application.
> > >
> > > because a dedicated function that can be called detached from the eal
> > > state enables an opportunity for accidental and confusing use outside
> > > the correct context.
> > >
> > > i know the above prescribes not to do this but.
> > >
> > > now you can call set after eal init, but we protect about calling it
> > > after init by failing. what do we do sensibly with the failure?
> > 
> > It would be a developer mistake which could be fix during development stage
> > very easily. I don't see a problem here.
> 
> Why is this not just a command line parameter, like other EAL configuration options?
> 
> Do any other pre-init APIs exist, or are you introducing a new design pattern for configuring EAL?

Let's say it is a "new" design pattern, as discussed multiple times in previous years.
But this one is only for the application,
it is not a user configuration as in rte_eal_init(int argc, char **argv).

> Any application can simply modify the command line parameters before calling EAL init. It doesn't need to pass the command line parameters as-is to EAL init.

It is not very easy to use.

> In other words: There is an existing design pattern for configuring EAL, why introduce a new design pattern?

Because argc/argv is a bad pattern.
We had multiple requests to avoid it.
So when introducing a new option, it is better to avoid it.

> If we want to expose APIs for configuring EAL instead of passing command line parameters, such APIs should be added for all EAL configuration parameters.

The memzone parameter is not supposed to be configured by the user,
so it does not make sense to expose it via argc/argv.

> That would be nice, but I dislike that some EAL configuration parameters must be passed using one method and some other passed using another method.

We asked multiple times for such rework.
And the patches from Bruce to split some EAL parts are in this direction.
If you want to propose some new functions to configure EAL, you are welcome.