[PATCH v2 1/1] mbuf: add optional dynfield3 storage

[Randy Tice (rtice)]

  Hi Stephen,

  Thanks for the detailed review. I agree v2 needs a design rework rather than
  just patching the reported failures.

  For the octeontx issue, I’ll handle this in v3 by disabling mempool/octeontx
  at Meson configure time when mbuf_dynfield3_size != 0, with a clear reason
  that it requires sizeof(struct rte_mbuf) <= 128. I don’t plan to change
  OCTEONTX_FPAVF_BUF_OFFSET in this patch since that looks like hardware-
  programmed layout behavior and should be owned/validated by the Marvell
  maintainers.

  For CN20K, that specific size-truncation problem has already been fixed upstream
  by 570c0b273cde (“drivers: fix CN20K mbuf size truncation"), so I’m using that as
  confirmation that the current tree no longer has the same CN20K blocker.

  For the copy semantics, agreed. The build-wide mbuf_dynfield3_copy option is
  the wrong abstraction. In v3 I’m dropping it and preserving the existing
  default dynamic-field behavior: fields registered with flags = 0 are copied by
  generic mbuf copy/clone paths. For deployments that need non-copy metadata,
  I’m adding an explicit per-field flag, RTE_MBUF_DYNFIELD_F_NO_COPY. Fields
  using that flag are restricted to the optional dynfield3 area, so the no-copy
  behavior is explicit and cannot accidentally affect existing dynamic-field
  users.

  That also fixes the dynfield1/dynfield3 straddling problem. Normal fields keep
  copy semantics, and no-copy fields are constrained to dynfield3. The copy path
  tracks the registered copied portions of dynfield3 and only copies those
  bytes, instead of relying on a global copy switch.

  For the allocator score overflow, I’m widening free_space[] from uint8_t to
  uint16_t.

  For the mbuf autotest issue, I’ll change the “too big” negative case to use
  sizeof(struct rte_mbuf) rather than a fixed 256-byte field, and add dynfield3-
  specific coverage for placement and copy/no-copy behavior.

  I’ll also add a devtools/test-meson-builds.sh build with
  -Dmbuf_dynfield3_size=256, update the programmer guide and release notes, drop
  the unused dynfield3 count/offset macros, and remove the redundant
  sizeof(uint64_t) Meson check.

  Thanks again — the v3 will be a more general-purpose API with default copy
  semantics preserved, and Cisco’s no-copy use case handled explicitly through
  the new field flag.
-rt

From: Stephen Hemminger <stephen at networkplumber.org>
Date: Saturday, September 26, 2026 at 12:51 PM
To: Randy Tice (rtice) <rtice at cisco.com>
Cc: dev at dpdk.org <dev at dpdk.org>; Morten Brørup <mb at smartsharesystems.com>; Bruce Richardson <bruce.richardson at intel.com>
Subject: Re: [PATCH v2 1/1] mbuf: add optional dynfield3 storage

On Fri, 25 Sep 2026 15:31:08 -0400
Randy L Tice <rtice at cisco.com> wrote:

> From: Randy L Tice <rtice at cisco.com>
> Date: Thu, 03 Sep 2026 09:13:28 -0400
>
> Add build-time support for optional cache-line-aligned dynamic-field
> storage at the end of struct rte_mbuf.
>
> The mbuf_dynfield3_size Meson option sets RTE_MBUF_DYNFIELD3_SIZE
> in rte_build_config.h. A non-zero value enables the extra area. The
> storage is represented as uint64_t elements for 32-bit and 64-bit
> build consistency.
>
> When enabled, dynfield3 is made available to the mbuf dynamic field
> allocator. The mbuf_dynfield3_copy option controls whether the area is
> copied by the generic mbuf dynamic-field copy helper, and defaults to
> false.
>
> Validate that the configured size is non-negative, is a multiple of
> sizeof(uint64_t), and reserves a multiple of the cache line size.
>
> Signed-off-by: Randy L Tice <rtice at cisco.com>
> ---

Ran this through AI review with full model

Subject: Re: [PATCH v2 1/1] mbuf: add optional dynfield3 storage

Applied to main (4f795dd), built and tested on x86_64 with
-Dmbuf_dynfield3_size=256 and 512.

Errors
------

1. Enabling the option breaks the build.

drivers/mempool/octeontx is built on all 64-bit Linux targets,
including x86_64, and has:

  RTE_BUILD_BUG_ON(sizeof(struct rte_mbuf) > OCTEONTX_FPAVF_BUF_OFFSET);

with OCTEONTX_FPAVF_BUF_OFFSET fixed at 128. Any non-zero
mbuf_dynfield3_size fails to compile with the default driver set.

The Known Issues entry is not a substitute. Drivers that depend on a
128 byte mbuf must be disabled at configure time when the option is
set, the same way require_iova_in_mbuf handles
enable_iova_as_pa=false. Please audit drivers that program
sizeof(struct rte_mbuf) or a fixed offset into hardware (octeontx
FPA buf_offset, cnxk first_skip) and state the result in the commit
message.

2. Fields can straddle dynfield1 and dynfield3; clone copies half.

On 64-bit targets dynfield1 ends at offset 128 and dynfield3 starts
at 128 (for both 64 and 128 byte cache lines), so init_shared_mem()
creates one contiguous free run. The best-fit allocator places a
field across the boundary, and with mbuf_dynfield3_copy=false (the
default) rte_mbuf_dynfield_copy() copies only the dynfield1 part.

Reproduced with size=512: register three 8 byte fields (96, 104,
112), then a 16 byte align 8 field. It lands at 120..135. Set it to
0xab and rte_pktmbuf_clone():

  abababababababab0000000000000000

The second half is whatever the clone mbuf held before, not zero.

Even without straddling, whether a field is copied now depends on
registration order and on what other libraries and PMDs registered
first. The field owner cannot know or control this, and every
existing user of rte_mbuf_dynfield_register() assumes copy on clone.

3. free_space[] score overflows for sizes >= 384.

struct mbuf_dyn_shm keeps the score in uint8_t free_space[].
process_score() computes align = 256 for a free run of 256 bytes or
more at a 256 byte aligned offset; the store truncates it to 0,
which means occupied, permanently.

With size=512 (dynfield3 at 128..639), bytes 256..511 are never
allocatable: a 256 byte field fails with ENOENT, and only 288 bytes
of 8 byte fields can be registered in total.

Widen free_space[] or cap the option. Meson integer options take
min/max, which also replaces the explicit < 0 check:

  option('mbuf_dynfield3_size', type: 'integer',
         min: 0, max: 256, value: 0, description: ...)

Warnings
--------

4. mbuf_autotest fails with size >= 256.

test_mbuf_dyn() expects dynfield_fail_big (size 256, align 1) to be
rejected. With size=256 it registers at offset 92, spanning 92..347
across both areas (see 2). The "too big" case should use
sizeof(struct rte_mbuf).

5. Copy policy is at the wrong level.

One build-time switch for the whole area cannot be right for every
field placed there. struct rte_mbuf_dynfield has a flags member,
reserved and required to be 0 today. Keep dynfield3 out of the
default allocator and hand it out only to callers that ask for it
with a new flag. That makes the no-copy semantics explicit and per
field, fixes 2, and removes mbuf_dynfield3_copy.

6. No test or CI coverage.

The option defaults to 0, so CI compiles none of the new code. Add a
devtools/test-meson-builds.sh build with the option set (it would
have caught 1), and a test_mbuf.c case that checks placement and
clone behaviour of a field in dynfield3.

7. Missing documentation and rationale.

doc/guides/prog_guide/mbuf_lib.rst describes dynamic fields and
needs to cover the new area, its copy semantics, and that the mbuf
layout now depends on a build option: applications and secondary
processes must be built with the same value.

The commit message does not say why the existing per-mbuf private
area (priv_size in rte_pktmbuf_pool_create()) is not sufficient.
The cover letter does not go into git; the rationale belongs in the
commit message, along with the cost: every mbuf grows by at least
one cache line.

Info
----

8. RTE_MBUF_DYNFIELD3_CNT and RTE_MBUF_DYNFIELD3_OFFSET are not
needed, and defining the offset as 0 when disabled is misleading
(offset 0 is buf_addr). Use the member directly, like dynfield1:

        memcpy(mdst->dynfield3, msrc->dynfield3,
               sizeof(mdst->dynfield3));

and drop the <stddef.h> include.

9. The sizeof(uint64_t) check in config/meson.build is redundant; a
multiple of RTE_CACHE_LINE_SIZE is always a multiple of 8.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mails.dpdk.org/archives/dev/attachments/20260928/626a0f01/attachment-0001.htm>