On Wed, Oct 15, 2025 at 06:10:59PM +0200, David Marchand wrote: > Hello Bruce, > > On Thu, 9 Oct 2025 at 15:01, 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. > > > > > > V11: > > * fix issues flagged by unit tests in CI and subsequent testing: > > - when passing in an lcore >= MAX_LCORES, return error rather than > > ignoring it. (compatibility issue) > > - return error when an invalid lcore set of "1-3-5" is passed in, > > rather than just treating it as "3-5". > > I did some tweaking on the series (and put my sob for taking the > bullet if I broke something ;-)), namely: > - squashed the init arg list patch into the initial patch that > introduces eal_option_list.h, > - inverted order of the patch on coremask rework with the one > introducing lcore remapping, > - I updated patch 6 as Chengwen requested, and I fixed return codes > for parse_arg_corelist(), > - I updated the doc for patch 8 as Chengwen requested, >
Thanks for the rework. > I fixed a few reintroductions of socket-mem (should be numa-mem) in > intermediate patches. > Are both not needing to be supported? With current implementation I think they are just aliases for another. [Though maybe I'm missunderstanding the rework here] > I noticed that the leak reported earlier on patch "eal: gather EAL > args before processing" is still present when stopping at this commit, > and it is fixed in the next commit. > We could have avoid this transient issue, but I did not spend time to > fix as it is just a leak in the event wrong EAL options are passed. > > There were some little checkpatch issues I fixed (plus some spurious > empty lines/spaces). > But I left the options definitions as is (wrt line length warning). > Thanks. I think the options are better kept one-per-line irrespective of checkpatch warnings. > Wrt storing the cores as a fixed size cpuset, this storage is internal > and we can change in the future (no ABI concern afaics). > > Series applied. > > Thanks Chengwen for the reviews on argparse. > > Thanks Bruce, this was kind of an unexpected long road. > This is a nice cleanup and I like this auto magic option and the debug logs. > Thanks. It was indeed a long road, and far more churn than expected, since I started out with only 5 or 7 patches in initial versions and ended up with over 20! > > I have two questions which could be addressed in followup patches but > seem more risky than what I touched, and require a new round of CI: > - are we missing a build check on RTE_MAX_LCORE < CPU_SETSIZE? Maybe. However, I think be best way to fix that is as part of any cleanup work to eliminate any dependency on CPU_SETSIZE. That will probably be its own epic patchset! > - should eal_clean_saved_args() be called in rte_eal_cleanup()? > > > And finally my dumb idea: > > Do you think it would be feasible to extend this remapping mechanism > for multi process? > I would like to start all processes with only the -R option (and each > application has a dedicated cpu affinity, set by an external mechanism > out of DPDK). > Then some exchanges between primary and secondary processes are done > at init, with secondary announcing a number of lcores it needs, and > the primary replying with a lcoreid base for remapping. > One problem is that it would require tracking life and death of the > secondary processes to that primary can reallocate unused lcores > ranges. > I think that is a tough ask. I don't think I'll attempt to implement that. Having users of secondary processes manage each process own starting lcore id is probably not a massive ask. Thanks again for all the reviews. /Bruce
