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. > > > - Second, stopping at patch "eal: gather EAL args before processing", > I see a crash in this same func_reentrancy_autotest unit test. > > Here, I would fix the unit test itself, as it passes a NULL argv[0] > which is invalid according to the C standard. > I fixed this unit test not so long ago (978ead0144c1 > ("test/func_reentrancy: fix EAL init call")), but the fix is > incomplete. > And if you wonder why the CI did not catch this, the next patch hides > the issue for the unit test, as the run_once flag gets set earlier, > which makes rte_eal_init fail before evaluating argv[0]. > > On the other hand, even if DPDK does not really care about argv[0] > content (well, until eal_save_args is called and crashes), we are > introducing a change in behavior for (arguably) non standard argv[0] > == NULL, and some programs, which are not passing C library argv[] and > instead building a argv[] array, may rely on this... ? >
The second should be relatively easy to fix, I hope, so I think it should be fixed both in a new version of this set, and also in the unit test that has the bad behaviour /Bruce
