Re: [PATCH v4 0/1] mbuf: add runtime metadata dynamic-field storage

[Stephen Hemminger <stephen@networkplumber.org>]

On Tue, 06 Oct 2026 11:54:37 -0400
Randy L Tice <rtice@cisco.com> wrote:

> This revision changes direction from the v3 build-time dynfield3
> layout.  It adds a runtime EAL-configured per-mbuf metadata area that is
> placed after the fixed struct rte_mbuf header and before per-pool private
> data.  This keeps sizeof(struct rte_mbuf) fixed while allowing deployments
> that need globally consistent per-mbuf metadata to reserve that storage.
> 
> The metadata area is still managed by the mbuf dynamic-field registry.
> Fields registered with RTE_MBUF_DYNFIELD_F_METADATA are allocated from the
> metadata area and are not copied by generic mbuf copy, clone, or attach
> operations.  Fields registered without the flag continue to use the existing
> copied mbuf dynamic-field storage and cannot overlap the metadata area.
> 
> The existing per-pool private data area does not provide a central layout
> registry and is configured independently for each mbuf pool.  That makes it
> hard for multiple libraries, drivers, or application modules to safely share
> metadata without out-of-band coordination, especially when pools are created
> by different components.
> 
> Mbuf object layout calculations that need to include the optional metadata
> area now use rte_mbuf_size().  The primary process validates and publishes
> the metadata size through the shared mem config; secondary processes may
> omit the option, but if provided it must match the primary value.
> 
> The octeontx mempool driver rejects allocation when mbuf metadata is
> configured because the hardware mbuf header offset must remain 128 bytes.

I did a brief look at this and still not convinced about the exact use case.
What is the problem this is trying to solve and why can't it be done
by using existing API's.

On a reviewer level, you need to break this up into:
  - core mbuf changes
  - per-driver patch to use those core mbuf changes
  - example of usage
  - documentation
  - any review safe guards that future changes don't break this.

The more detailed AI review focused on problems as well...


[PATCH v4 1/1] mbuf: add runtime metadata dynamic-field storage

Does not apply to current main: rte_mbuf_from_indirect() and
rte_mbuf_to_priv() now have RTE_ASSERT lines. Needs rebase.
Fixed up by hand for testing; builds with -Dwerror=true for the cnxk,
bnxt, nfp, softnic and mempool drivers. mbuf_autotest passes with
metadata size 0, 64, 256 and 65472.

Error

lib/mbuf/rte_mbuf_core.h: RTE_MARKER8 is not defined when building
with MSVC (rte_common.h wraps the marker typedefs in
#ifndef RTE_TOOLCHAIN_MSVC). The markers were removed from struct
rte_mbuf for exactly this reason; this re-adds one and breaks the
Windows MSVC build. The marker is also unnecessary: the metadata
area starts at sizeof(struct rte_mbuf), so use that instead of
offsetof(struct rte_mbuf, metadata) and drop the member.

lib/eal/common/eal_common_config.c: rte_mbuf_metadata_size is
exported with RTE_EXPORT_SYMBOL and lands in DPDK_27 (stable).
New API must be experimental. Same for rte_mbuf_metadata_size_get(),
rte_mbuf_size() and RTE_MBUF_DYNFIELD_F_METADATA. Exporting a bare
variable as stable ABI is worse than a function; it freezes the type
and the symbol location.

drivers/net/cnxk, drivers/event/cnxk: the hardware skip fields are
narrow and roc_nix_rq_init() only checks 8-byte alignment, not range:
  wqe_skip   2 bits, in 128B lines  -> rte_mbuf_size() <= 384
  later_skip 6 bits, in 8B units    -> mbuf + meta + priv <= 504
  first_skip 7 bits, in 8B units    -> mbuf + meta + priv
                                       + headroom <= 1016
With --mbuf-metadata-size=384, wqe_skip becomes 4 and is truncated to
0. With metadata 256 and priv_size 128, later_skip is 512 and is
truncated to 0. Hardware then writes the WQE/packet over the mbuf.
The driver must reject configurations that do not fit, as was done
for octeontx.

Warning

EAL defines and exports a symbol named rte_mbuf_*, and it is declared
extern in both eal_private.h and rte_mbuf_core.h. Follow the existing
--mbuf-pool-ops-name pattern: EAL stores the value and provides a
getter (rte_eal_mbuf_user_pool_ops() equivalent); mbuf owns any
mbuf-named symbols.

Fast path cost. rte_mbuf_size() turns a compile-time constant into a
load of a global (through the GOT in shared builds) in
rte_mbuf_to_priv(), rte_mbuf_from_indirect(), rte_mbuf_buf_addr() and
rte_pktmbuf_detach(), all inline and used per packet. cnxk Rx vector
paths now do vdupq_n_u64(rte_mbuf_size()) inside the burst loop, and
the cn9k get_work asm needs an extra register load per event. The
cnxk paths should cache the value in rxq/ws at setup like data_off.
Need l3fwd or testpmd numbers with the option unset, on cnxk at
least, before this goes in.

lib/eal/common/eal_common_options.c: upper bound of UINT16_MAX is not
justified. Beyond the cnxk limits above, examples/fips_validation
DEF_MBUF_SEG_SIZE is UINT16_MAX - rte_mbuf_size() - headroom and
wraps for large values. Cap the option at a small number of cache
lines and document the limit.

drivers/net/cnxk/cn10k_rx.h nix_cqe_xtract_mseg(): the rx_inj block
was reindented one tab too deep; it is still inside the same if.
Only the two wqe assignment lines need to change.

Too much in one patch. Put the mbuf helpers first, then the
conversions, then the feature:
  1. mbuf: add rte_mbuf_size() returning sizeof(struct rte_mbuf),
     no behaviour change
  2. convert libs, apps and examples
  3. convert drivers (one per driver family, so maintainers can ack)
  4. add the EAL option, metadata dynfield flag, driver rejections
     and docs
Patches 1-3 are a no-op and can be reviewed and merged on their own.
Patch 4 is then small enough to review for what it actually changes.
It also makes it bisectable when a driver conversion is wrong.

Many of the conversions open-code rte_mbuf_size() where an existing
helper fits: RTE_PTR_ADD(m, rte_mbuf_size()) is rte_mbuf_to_priv(m),
and mbuf + rte_mbuf_size() + priv_size is rte_mbuf_buf_addr(). Use
the helpers so drivers stop depending on the layout directly.

Info

Unrelated whitespace churn: blank line removed in
drivers/mempool/octeontx/meson.build, in the release notes Known
Issues section, and in test_mbuf(). Drop these.

lib/mbuf/rte_mbuf_dyn.c: the "check if this offset can be used"
comment now sits above dynfield_in_metadata() instead of
check_offset().

drivers/mempool/octeontx: the check rejects every fpavf pool, not
just pktmbuf pools. mp->private_data_size is known at alloc time, so
the check could be limited to pools carrying
rte_pktmbuf_pool_private. Not required since net/octeontx is the only
user that cares.

app/test/test_mbuf.c: with metadata configured, dynfield_fail_big
(size 128) passes the size check and fails only because no free
space exists, so the size limit path is no longer tested in the
metadata run.

Any out-of-tree code using (m + 1) or sizeof(struct rte_mbuf) to find
private data silently breaks when a user adds this EAL option. That
belongs in the API changes section of the release notes, not only
under New Features.