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 > > <[email protected]> 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.
