[PATCH v11 00/21]

[Bruce Richardson]

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
> <bruce.richardson at 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.
> >
> >
> > 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