On Mon, Jul 27, 2026 at 04:30:07PM -0700, Stephen Hemminger wrote:
> On Tue, 21 Jul 2026 10:45:08 +0100
> Bruce Richardson <[email protected]> wrote:
>
> > This patchset reworks how configuration is stored and managed in EAL.
> > The existing "internal_config", "rte_config", "lcore_config" structures,
> > which sometimes have arbitrary separation between them (especially the
> > first two) are replaced by three new structures with clearly defined
> > roles:
> >
> > - eal_platform_info - contains the raw HW info for the system, details
> > of CPUs and hugepage mounts. This is initialized on first use - even
> > before EAL init is called - and is then immutable, since our HW should
> > not change much underneath us. Its early availability means that it
> > can be used to sanity check the contents of the other structs as they
> > are being built up.
> >
> > - eal_user_cfg - contains the config settings passed in by the user. For
> > existing rte_eal_init, this is built up in the arg parse stage, and
> > it's contents verified against the platform info, e.g. to check core
> > masks are valid etc. Once argument parsing is completed, is also
> > immutable.
> >
> > - runtime_cfg - basically all the runtime settings that need to be there
> > for DPDK to run, or which change over time. Largely combined content
> > of the old rte_config, internal_config and lcore_config structs. This
> > is initialized from the other two structs by eal initialization and
> > can be modified by EAL at any time.
> >
> > Once that is done, we have a clean separation between user provided
> > configuration and the rest of EAL, we can split EAL init into two parts,
> > the first of which parses cmdline arguments and then calls the second
> > which takes the eal_user_cfg struct result of that parse and does the
> > actual initialization. The longer-term objective is to have other
> > first-stage functions that prepare the user_cfg struct for
> > initialization, so that we can move away from argc/argv as the only
> > method of configuring DPDK initialization.
<snip>
> > --
> > 2.53.0
> >
>
> Ran deeper AI review on this and it found a couple small things:
>
> Review of "eal: rework EAL initialization" (39 patches)
>
> Series applies cleanly to main (9231dc7). All 39 commits build individually
> with -Dwerror=true, so bisect is safe. Findings verified against the merged
> tree rather than the diffs alone.
>
>
> Patch 02/39 - argparse: check for range overflow in CPU lists
>
> Warning: missing Fixes: and Cc: stable.
>
> This is an out-of-bounds write, not a cleanup - CPU_SET(min, cpuset) with
> min >= CPU_SETSIZE writes past the end of rte_cpuset_t. The introducing
> commit is in v25.11, so it needs backporting:
>
> Fixes: d78103fb9488 ("argparse: support core lists")
> Cc: [email protected]
>
> The fix itself is correct and complete: all three branches assign max before
> the new check, min >= 0 is guaranteed by the isdigit() gate, and min <= max
> holds in every branch, so the CPU_SET loop is fully bounded.
>
Added in v2
>
> Patch 33/39 - eal: remove internal config reset function
>
> Error: removing eal_reset_internal_config() drops the lock_descriptor = -1
> preset, which can lead to close(0) on stdin. Linux only.
>
> The deleted function did:
>
> for (i = 0; i < MAX_HUGEPAGE_SIZES; i++) {
> memset(&internal_cfg->hugepage_info[i], 0, ...);
> internal_cfg->hugepage_info[i].lock_descriptor = -1;
> }
>
> That -1 was the invariant eal_hugedirs_unlock() relied on. hugepage_info[]
> now lives in eal_runtime_state, which is a static initialised only with
> .mem_config, so every lock_descriptor starts at 0 - a valid fd.
>
> This patch compensates by widening the guard:
>
> if (hugepage_info[i].hugepage_sz == 0 ||
> hugepage_info[i].lock_descriptor < 0)
> continue;
>
> but hugepage_sz == 0 is not a sufficient proxy. In hugepage_info_init()
> (lib/eal/linux/eal_hugepage_info.c) hpi->hugepage_sz is assigned *before*
> the mountpoint check, and the no-mountpoint path continues without ever
> assigning lock_descriptor:
>
> hpi = &rs->hugepage_info[num_sizes];
> hpi->hugepage_sz = hps->size; /* set first */
>
> if (get_hugepage_dir(...) < 0) {
> if (user_cfg->in_memory) {
> calc_num_pages(hpi, hps, 0);
> num_sizes++; /* entry accepted */
> }
> continue; /* lock_descriptor never set */
> }
>
> hpi->lock_descriptor = open(hpi->hugedir, O_RDONLY);
>
> Two reachable cases:
>
> (a) --in-memory with a reserved-but-unmounted size (e.g. 1G reserved,
> only 2M mounted). The entry is accepted with hugepage_sz != 0 and
> lock_descriptor == 0.
>
> (b) Default mode where the *last* size has no mountpoint. num_sizes is
> not incremented, so slot [num_sizes] keeps a nonzero hugepage_sz with
> lock_descriptor == 0. eal_hugedirs_unlock() iterates to
> MAX_HUGEPAGE_SIZES, not num_hugepage_sizes, so it still visits it.
>
> In both, the guard passes and the code runs flock(0, LOCK_UN) followed by
> close(0) on the normal init path (lib/eal/linux/eal.c:831, unconditional
> after rte_eal_memory_init()). Confirmed with a standalone harness
> reproducing the two functions' control flow.
>
> FreeBSD is unaffected (single entry, fd assigned unconditionally, no
> unlock loop); Windows sets -1 explicitly in eal_hugepages.c.
>
> Simplest fix is to restore the invariant rather than widen the guard - set
> lock_descriptor = -1 for all MAX_HUGEPAGE_SIZES entries when runtime state
> is set up, or initialise the entry immediately after hugepage_sz is
> assigned in hugepage_info_init(). Bounding the unlock loop by
> num_hugepage_sizes would fix (b) but not (a).
>
> The other non-zero defaults from the deleted function are all preserved
> correctly: hugepage_file.unlink_existing, no_hpet, and
> max_simd_bitwidth.bitwidth are in EAL_USER_CFG_INITIALIZER, and
> RTE_IOVA_DC / RTE_INTR_MODE_NONE / RTE_PROC_PRIMARY are all genuinely 0.
> lock_descriptor is the only one lost.
>
Yes, this is a valid issue. Reworked a couple of patches to fix it for v2.
>
> Patch 29/39 - eal: move trace config into user config struct
>
> Warning: --trace-dir accumulate semantics changed, plus a leak on repeat.
>
> The old path went through trace_dir_update(), which concatenated onto any
> existing value:
>
> asprintf(&dir, "%s%s", trace->dir != NULL ? trace->dir : "", str);
>
> The new code does a plain asprintf into user_cfg->trace_dir. Passing
> --trace-dir more than once now replaces rather than appends, and the
> earlier allocation leaks since trace_dir is overwritten without a free.
> If the replace behaviour is intended, worth saying so in the commit
> message; otherwise free the previous value first.
>
This I believe to be a false positive. The trace-dir EAL flag can only be
specified once on the command line, enforced by the argparse library, so
the fact that the later functions don't handle multiple values is not a
problem. The concatenation here is actually for appending a filename to an
existing trace dir.
>
> Patch 39/39 - eal: provide hooks for init with externally supplied config
>
> Error: rte_eal_runtime_init() returns -1 without setting rte_errno on the
> platform-info path. Identical in all three platform copies:
>
> if (rte_eal_get_platform_info() == NULL) {
> rte_eal_init_alert("Platform information is not available.");
> return -1; /* rte_errno not set */
> }
>
> The other two error paths in the same function set EINVAL and EALREADY,
> and the equivalent path in rte_eal_init() sets ENOTSUP. A caller checking
> rte_errno gets a stale value. Suggest rte_errno = ENOTSUP to match.
>
Fixed in v2. Explicitly set rte_errno = 0 at the start of function and set
it explicitly only when it's not already set by a subfunction of
get_platform_info.
> Warning: the stated purpose is not reachable as posted.
>
> The commit message says the hooks let "other libraries init EAL by passing
> in that structure pre-configured", but struct eal_user_cfg and both new
> prototypes live in lib/eal/common/eal_internal_cfg.h. lib/meson.build:143
> only adds eal/common to the include path when RTE_LIB_EAL is not yet set,
> i.e. for EAL's own sub-build; afterwards dependent libraries get
> deps += ['eal'], which exposes only EAL's public include dirs. No in-tree
> library can declare the type or call the function without the explicit
> include_directories() hack used by drivers/common/mlx5/linux/meson.build.
> Either make the header reachable or note that a follow-up is required.
>
Expected. In RFC I included an example of use, but dropped from this v1
series as it's already long enough.
> Warning: both new __rte_internal symbols have no in-tree consumer and no
> test, so the deep-copy path in eal_user_cfg_copy() is never exercised by
> anything. A test driving rte_eal_runtime_init() with a hand-populated
> config would be worth adding alongside.
>
As above, will hopefully be added later if this makes it in.
> Info: eal_internal_cfg.h uses #include "rte_compat.h" while every other
> public RTE header in the same file uses angle brackets.
>
>
> Checked and found correct
>
<snip>
