Re: [PATCH v9 00/18] Simplify running with high-numbered CPUs

[Thomas Monjalon <thomas@monjalon.net>]

07/10/2025 18:15, Bruce Richardson:
> On Mon, Oct 06, 2025 at 04:10:52PM +0200, David Marchand wrote:
> > On Fri, 3 Oct 2025 at 10:15, Bruce Richardson
> > <bruce.richardson@intel.com> wrote:
> > >
> > > The ultimate of this patchset is to make it easier to run on systems
> > > with large numbers of cores, by simplifying the process of using core
> > > numbers >RTE_MAX_LCORE. The new EAL arg ``-remap-lcore-ids``, also
> > > shortened to ``-R``, is added to DPDK to support this.
> > >
> > > However, in order to add this new flag easily, the first dozen or more
> > > patches rework the argument handling in EAL to simplify things, using
> > > the argparse library for argument handling.
> > >
> > > When processing cmdline arguments in DPDK, we always do so with very
> > > little context. So, for example, when processing the "-l" flag, we have
> > > no idea whether there will be later a --proc-type=secondary flag. We
> > > have all sorts of post-arg-processing checks in place to try and catch
> > > these scenarios.
> > >
> > > To improve this situation, this patchset tries to simplify the handling
> > > of argument processing, by explicitly doing an initial pass to collate
> > > all arguments into a structure. Thereafter, the actual arg parsing is
> > > done in a fixed order, meaning that e.g. when processing the
> > > --main-lcore flag, we have already processed the service core flags. We
> > > also can far quicker and easier check for conflicting options, since
> > > they can all be checked for NULL/non-NULL in the arg structure
> > > immediately after the struct has been populated.
> > >
> > > An additional benefit of this work is that the argument parsing for EAL
> > > is much more centralised into common options and the options list file.
> > > This single list with ifdefs makes it clear to the viewer what options
> > > are common across OS's, vs what are unix-only or linux-only.
> > >
> > > Once the cleanup and rework is done, adding the new options for
> > > remapping cores becomes a lot simpler, since we can very easily check
> > > for scenarios like multi-process and handle those appropriately.
> > >
> > > V9: rebase to latest main. CI complains cannot apply v8 patches.
> > >
> > > V8:
> > > * dropped the final two patches from the series, dropping the new -L
> > >   option in favour of the -R modifier.
> > > * reordered patch 11 to be with the other argparse patches (now patch 5)
> > > * added patch 12, which uses macros to initialize the args structure
> > >   from the arguments header file, avoiding potential issues when we add
> > >   new args.
> > > * simplified and consolidated lcore mask and core list parsing to always
> > >   work off cpusets rather than arrays of uint8
> > > * enhanced debug printouts to also work better with cpusets and handle
> > >   core values in those sets >= RTE_MAX_LCORE
> > > * for completeness, ensure the new -R option works for coremasks, and
> > >   for cases where no explicit core-list or coremask is specified.
> > 
> > I was planning to merge the first part of the series (before reaching
> > the cpuset rework and addition of remap option).
> > 
> > I am facing two issues for which I prefer other's opinions.
> > 
> > 
> > - First, I see a change in how non-option arguments are handled with
> > the switch to argparse.
> > $ ./build-mini/app/dpdk-test --no-huge -m 2048 -l 0,1 func_reentrancy_autotest
> > ARGPARSE: too many positional arguments func_reentrancy_autotest!
> > 
> > Passing the test name after -- does work, but it was working without
> > -- before the patch, so we are introducing a regression here.
> > 
> 
> Looking for input on how much we need to handle here.
> 
> The quick patch below adds support for ignoring trailing args in argparse,
> which means that the command above would work. However, gnu getopt also
> does argument reordering which means that this command works right now (on
> linux anyway):
> 
> 	./build/app/dpdk-test -l 0,1 lcores_autotest --no-huge
> 
> since getopt moves the lcores_autotest to the end.
> 
> Is this behaviour what we need to emulate too, or is adding a flag to
> ignore trailing args sufficient? [I'd tend toward it being sufficient - mixing
> app args and EAL args together is not a great idea IMHO]

I am in favor of being tolerant.
I agree mixing args is not a great idea,
but rejecting them is not really kind, and is a regression.