<html>
<head>
<meta http-equiv="Content-Type" content="text/html; charset=utf-8">
</head>
<body>
<div style="direction: ltr; font-family: Aptos, Arial, Helvetica, sans-serif; font-size: 12pt; color: rgb(0, 0, 0);">
Stephen,</div>
<div style="direction: ltr; font-family: Aptos, Arial, Helvetica, sans-serif; font-size: 12pt; color: rgb(0, 0, 0);">
Please see inline, look for #RT:</div>
<div style="direction: ltr; font-family: Aptos, Arial, Helvetica, sans-serif; font-size: 12pt; color: rgb(0, 0, 0);">
Thanks,</div>
<div style="direction: ltr; font-family: Aptos, Arial, Helvetica, sans-serif; font-size: 12pt; color: rgb(0, 0, 0);">
-rt</div>
<div style="direction: ltr; font-family: Aptos, Arial, Helvetica, sans-serif; font-size: 12pt; color: rgb(0, 0, 0);">
<br>
</div>
<div id="mail-editor-reference-message-container">
<div style="padding: 3pt 0in 0in; border-width: 1pt medium medium; border-style: solid none none; border-color: rgb(181, 196, 223) currentcolor currentcolor;">
<div style="text-align: left; font-family: Aptos; font-size: 12pt; color: black;">
<b>From: </b>Stephen Hemminger <stephen@networkplumber.org><br>
<b>Date: </b>Tuesday, October 6, 2026 at 12:51 PM<br>
<b>To: </b>Randy Tice (rtice) <rtice@cisco.com><br>
<b>Cc: </b>dev@dpdk.org <dev@dpdk.org>; Morten Brørup <mb@smartsharesystems.com>; Nithin Dabilpuram <ndabilpuram@marvell.com>; Harman Kalra <hkalra@marvell.com><br>
<b>Subject: </b>Re: [PATCH v4 0/1] mbuf: add runtime metadata dynamic-field storage<br>
<br>
</div>
</div>
<div id="mail-editor-reference-message-body">
<div class="ms-outlook-mobile-reference-message skipProofing">
<meta name="Generator" content="Microsoft Exchange Server">
</div>
<div class="PlainText" style="font-size: 11pt;">On Tue, 06 Oct 2026 11:54:37 -0400<br>
Randy L Tice <rtice@cisco.com> wrote:<br>
<br>
> This revision changes direction from the v3 build-time dynfield3<br>
> layout. It adds a runtime EAL-configured per-mbuf metadata area that is<br>
> placed after the fixed struct rte_mbuf header and before per-pool private<br>
> data. This keeps sizeof(struct rte_mbuf) fixed while allowing deployments<br>
> that need globally consistent per-mbuf metadata to reserve that storage.<br>
><br>
> The metadata area is still managed by the mbuf dynamic-field registry.<br>
> Fields registered with RTE_MBUF_DYNFIELD_F_METADATA are allocated from the<br>
> metadata area and are not copied by generic mbuf copy, clone, or attach<br>
> operations. Fields registered without the flag continue to use the existing<br>
> copied mbuf dynamic-field storage and cannot overlap the metadata area.<br>
><br>
> The existing per-pool private data area does not provide a central layout<br>
> registry and is configured independently for each mbuf pool. That makes it<br>
> hard for multiple libraries, drivers, or application modules to safely share<br>
> metadata without out-of-band coordination, especially when pools are created<br>
> by different components.<br>
><br>
> Mbuf object layout calculations that need to include the optional metadata<br>
> area now use rte_mbuf_size(). The primary process validates and publishes<br>
> the metadata size through the shared mem config; secondary processes may<br>
> omit the option, but if provided it must match the primary value.<br>
><br>
> The octeontx mempool driver rejects allocation when mbuf metadata is<br>
> configured because the hardware mbuf header offset must remain 128 bytes.<br>
<br>
I did a brief look at this and still not convinced about the exact use case.<br>
What is the problem this is trying to solve and why can't it be done<br>
by using existing API’s.</div>
<div class="PlainText" style="direction: ltr; font-size: 11pt;"><br>
</div>
<div class="PlainText" style="font-size: 11pt;">#RT:</div>
<div class="PlainText" style="font-size: 11pt; color: rgb(0, 0, 0);"> Our implementation currently requires an additional 256 bytes of metadata per</div>
<div style="font-size: 11pt; color: rgb(0, 0, 0);"> mbuf. That does not fit in the existing dynamic-field storage, so today we</div>
<div style="font-size: 11pt; color: rgb(0, 0, 0);"> carry a private patch to extend struct rte_mbuf. The goal of this work is to</div>
<div style="font-size: 11pt; color: rgb(0, 0, 0);"> replace that private struct change with a supported upstream mechanism.</div>
<div style="direction: ltr; font-size: 11pt; color: rgb(0, 0, 0);"><br>
</div>
<div style="font-size: 11pt; color: rgb(0, 0, 0);"> We did look at using mbuf private data first. It can work when the application</div>
<div style="font-size: 11pt; color: rgb(0, 0, 0);"> owns all mbuf pool creation, but it is not sufficient for our case without</div>
<div style="font-size: 11pt; color: rgb(0, 0, 0);"> another global/base reservation mechanism. Some mbuf pools are created by</div>
<div style="font-size: 11pt; color: rgb(0, 0, 0);"> drivers or libraries rather than directly by the application; the CNXK inline</div>
<div style="font-size: 11pt; color: rgb(0, 0, 0);"> IPsec/OOP meta pool is one example (NIX_INL_META_POOL, created through</div>
<div style="font-size: 11pt; color: rgb(0, 0, 0);"> cnxk_nix_inl_meta_pool_cb()). To make private data work there, we had to add</div>
<div style="font-size: 11pt; color: rgb(0, 0, 0);"> an EAL argument that reserved a base private size for all pktmbufs, including</div>
<div style="font-size: 11pt; color: rgb(0, 0, 0);"> PMD-created pools.</div>
<div style="direction: ltr; font-size: 11pt; color: rgb(0, 0, 0);"><br>
</div>
<div style="font-size: 11pt; color: rgb(0, 0, 0);"> Private data also lacks a central layout registry. If multiple modules use</div>
<div style="font-size: 11pt; color: rgb(0, 0, 0);"> private data, they must coordinate offsets out of band to avoid overlaying</div>
<div style="font-size: 11pt; color: rgb(0, 0, 0);"> each other. The dynamic-field registry solves that coordination problem, but</div>
<div style="font-size: 11pt; color: rgb(0, 0, 0);"> the existing copied dynamic-field area is too small and has copy/clone</div>
<div style="font-size: 11pt; color: rgb(0, 0, 0);"> semantics that are wrong for this metadata.</div>
<div style="direction: ltr; font-size: 11pt; color: rgb(0, 0, 0);"><br>
</div>
<div style="font-size: 11pt; color: rgb(0, 0, 0);"> That is why this version uses a globally configured per-mbuf metadata area</div>
<div style="font-size: 11pt; color: rgb(0, 0, 0);"> managed by the dynamic-field registry, with explicit metadata fields that are</div>
<div style="font-size: 11pt; color: rgb(0, 0, 0);"> not copied by generic copy/clone/attach paths. This direction came out of the</div>
<div style="font-size: 11pt; color: rgb(0, 0, 0);"> prior discussion with you, Morten, and me: avoid a Cisco-private mbuf struct</div>
<div style="font-size: 11pt; color: rgb(0, 0, 0);"> patch, avoid per-pool private-data layout coordination, and keep sizeof(struct</div>
<div style="font-size: 11pt; color: rgb(0, 0, 0);"> rte_mbuf) fixed.</div>
<div style="font-size: 11pt;"><span style="color: rgb(0, 0, 0);"><br>
</span><br>
On a reviewer level, you need to break this up into:<br>
- core mbuf changes<br>
- per-driver patch to use those core mbuf changes<br>
- example of usage<br>
- documentation<br>
- any review safe guards that future changes don't break this.</div>
<div class="PlainText" style="direction: ltr; font-size: 11pt;"><br>
</div>
<div class="PlainText" style="font-size: 11pt;">#RT:</div>
<div class="PlainText" style="font-size: 11pt; color: rgb(0, 0, 0);"> I will split the next revision into a series instead of carrying</div>
<div style="font-size: 11pt; color: rgb(0, 0, 0);"> this as one patch. The exact split may depend on the helper model we settle</div>
<div style="font-size: 11pt; color: rgb(0, 0, 0);"> on, but the intent will be the same: introduce any mbuf layout helper first</div>
<div style="font-size: 11pt; color: rgb(0, 0, 0);"> with no behavior change, then convert core/libs/apps/examples, then convert</div>
<div style="font-size: 11pt; color: rgb(0, 0, 0);"> drivers by family so maintainers can review/ack their areas, and finally add</div>
<div style="font-size: 11pt; color: rgb(0, 0, 0);"> the EAL metadata option, registry flag, validation/rejection logic, tests,</div>
<div style="font-size: 11pt; color: rgb(0, 0, 0);"> and documentation. CNXK will be its own driver patch because it needs range</div>
<div style="font-size: 11pt;"><span style="color: rgb(0, 0, 0);"> checks and fast-path caching rather than a purely mechanical conversion.</span>
<br>
<br>
</div>
<div style="direction: ltr; font-size: 11pt;"><br>
</div>
<div style="font-size: 11pt;">The more detailed AI review focused on problems as well...<br>
<br>
<br>
[PATCH v4 1/1] mbuf: add runtime metadata dynamic-field storage<br>
<br>
Does not apply to current main: rte_mbuf_from_indirect() and<br>
rte_mbuf_to_priv() now have RTE_ASSERT lines. Needs rebase.<br>
Fixed up by hand for testing; builds with -Dwerror=true for the cnxk,<br>
bnxt, nfp, softnic and mempool drivers. mbuf_autotest passes with<br>
metadata size 0, 64, 256 and 65472.<br>
<br>
#RT:</div>
<div class="PlainText" style="font-size: 11pt; color: rgb(0, 0, 0);"> Agreed. I will rebase v5 on current main and preserve the new RTE_ASSERT</div>
<div style="font-size: 11pt; color: rgb(0, 0, 0);"> checks in rte_mbuf_from_indirect() and rte_mbuf_to_priv(). Thanks for fixing</div>
<div style="font-size: 11pt; color: rgb(0, 0, 0);"> it up locally and for the additional build/test coverage. I will also</div>
<div style="font-size: 11pt; color: rgb(0, 0, 0);"> include comparable validation results in the next revision, including normal</div>
<div style="font-size: 11pt; color: rgb(0, 0, 0);"> metadata sizes and rejected/limit cases rather than relying on very large</div>
<div style="font-size: 11pt; color: rgb(0, 0, 0);"> values such as 65472 if we cap the option more tightly.</div>
<div style="direction: ltr; font-size: 11pt;"><br>
</div>
<div style="font-size: 11pt;"><br>
Error<br>
<br>
lib/mbuf/rte_mbuf_core.h: RTE_MARKER8 is not defined when building<br>
with MSVC (rte_common.h wraps the marker typedefs in<br>
#ifndef RTE_TOOLCHAIN_MSVC). The markers were removed from struct<br>
rte_mbuf for exactly this reason; this re-adds one and breaks the<br>
Windows MSVC build. The marker is also unnecessary: the metadata<br>
area starts at sizeof(struct rte_mbuf), so use that instead of<br>
offsetof(struct rte_mbuf, metadata) and drop the member.</div>
<div class="PlainText" style="direction: ltr; font-size: 11pt;"><br>
</div>
<div class="PlainText" style="font-size: 11pt;">#RT:</div>
<div class="PlainText" style="font-size: 11pt;">I will fix this!<br>
<br>
<br>
lib/eal/common/eal_common_config.c: rte_mbuf_metadata_size is<br>
exported with RTE_EXPORT_SYMBOL and lands in DPDK_27 (stable).<br>
New API must be experimental. Same for rte_mbuf_metadata_size_get(),<br>
rte_mbuf_size() and RTE_MBUF_DYNFIELD_F_METADATA. Exporting a bare<br>
variable as stable ABI is worse than a function; it freezes the type<br>
and the symbol location.<br>
<br>
#RT:</div>
<div class="PlainText" style="direction: ltr; font-size: 11pt; color: rgb(0, 0, 0);">
I agree with the concern about exporting</div>
<div style="font-size: 11pt; color: rgb(0, 0, 0);"> rte_mbuf_metadata_size as a writable global. That is the wrong ABI shape and I</div>
<div style="font-size: 11pt; color: rgb(0, 0, 0);"> will remove it.</div>
<div style="direction: ltr; font-size: 11pt; color: rgb(0, 0, 0);"><br>
</div>
<div style="font-size: 11pt; color: rgb(0, 0, 0);"> I want to pause before spinning v5, though, because this exposes a design</div>
<div style="font-size: 11pt; color: rgb(0, 0, 0);"> choice around the existing mbuf helpers. The current patch makes helpers such</div>
<div style="font-size: 11pt; color: rgb(0, 0, 0);"> as rte_mbuf_to_priv(), rte_mbuf_buf_addr(), rte_mbuf_from_indirect(), and</div>
<div style="font-size: 11pt; color: rgb(0, 0, 0);"> rte_pktmbuf_detach() metadata-aware. That gives the least ambiguous semantics</div>
<div style="font-size: 11pt; color: rgb(0, 0, 0);"> — private data and buffer data remain after the full mbuf object — but those</div>
<div style="font-size: 11pt; color: rgb(0, 0, 0);"> helpers are public inline functions, so they need some runtime-visible mbuf</div>
<div style="font-size: 11pt; color: rgb(0, 0, 0);"> object size. If that value is hidden entirely inside EAL/mbuf C files, the</div>
<div style="font-size: 11pt; color: rgb(0, 0, 0);"> inline helpers cannot access it without becoming out-of-line calls.</div>
<div style="direction: ltr; font-size: 11pt; color: rgb(0, 0, 0);"><br>
</div>
<div style="font-size: 11pt; color: rgb(0, 0, 0);"> The alternative is an explicit opt-in model: leave existing stable helpers</div>
<div style="font-size: 11pt; color: rgb(0, 0, 0);"> with their current fixed sizeof(struct rte_mbuf) behavior, use internal cached</div>
<div style="font-size: 11pt; color: rgb(0, 0, 0);"> layout values in PMDs/libraries that support metadata, and expose separate</div>
<div style="font-size: 11pt; color: rgb(0, 0, 0);"> metadata-aware helpers for applications that do raw mbuf layout math. For</div>
<div style="font-size: 11pt; color: rgb(0, 0, 0);"> example, rte_mbuf_to_priv() would continue to return (char *)m + sizeof(struct</div>
<div style="font-size: 11pt; color: rgb(0, 0, 0);"> rte_mbuf), while a new metadata-aware helper would return (char *)m +</div>
<div style="font-size: 11pt; color: rgb(0, 0, 0);"> sizeof(struct rte_mbuf) + metadata_size. PMDs that support metadata would use</div>
<div style="font-size: 11pt; color: rgb(0, 0, 0);"> cached internal equivalents of the latter during setup/fast path preparation,</div>
<div style="font-size: 11pt; color: rgb(0, 0, 0);"> without requiring the application to compile with experimental APIs. That</div>
<div style="font-size: 11pt; color: rgb(0, 0, 0);"> avoids silently changing existing helper performance/behavior, and the new</div>
<div style="font-size: 11pt; color: rgb(0, 0, 0);"> application-facing helpers could be experimental. The downside is parallel</div>
<div style="font-size: 11pt; color: rgb(0, 0, 0);"> helper semantics and a requirement that metadata-enabled code use the new</div>
<div style="font-size: 11pt; color: rgb(0, 0, 0);"> helpers correctly.</div>
<div style="direction: ltr; font-size: 11pt; color: rgb(0, 0, 0);"><br>
</div>
<div style="font-size: 11pt; color: rgb(0, 0, 0);"> For PMDs, I think the right implementation either way is to avoid repeated</div>
<div style="font-size: 11pt; color: rgb(0, 0, 0);"> runtime helper loads in fast paths. The metadata size is system-wide after EAL</div>
<div style="font-size: 11pt; color: rgb(0, 0, 0);"> init, so DPDK can compute an internal mbuf object size once and PMDs can cache</div>
<div style="font-size: 11pt; color: rgb(0, 0, 0);"> any derived queue/device values during setup. CNXK also needs explicit guards:</div>
<div style="font-size: 11pt; color: rgb(0, 0, 0);"> the hardware skip fields cap the supported metadata size and related private-</div>
<div style="font-size: 11pt; color: rgb(0, 0, 0);"> data/headroom combinations. In particular, with the current 128-byte mbuf</div>
<div style="font-size: 11pt; color: rgb(0, 0, 0);"> header, 256 bytes of metadata already reaches the wqe_skip limit, so CNXK</div>
<div style="font-size: 11pt; color: rgb(0, 0, 0);"> support must reject configurations that exceed the hardware-encodable offsets.</div>
<div style="direction: ltr; font-size: 11pt; color: rgb(0, 0, 0);"><br>
</div>
<div style="font-size: 11pt; color: rgb(0, 0, 0);"> Before I rework the series, I’d like your guidance on which ABI/API model you</div>
<div style="font-size: 11pt; color: rgb(0, 0, 0);"> think is acceptable upstream:</div>
<div style="direction: ltr; font-size: 11pt; color: rgb(0, 0, 0);"><br>
</div>
<div style="font-size: 11pt; color: rgb(0, 0, 0);"> 1. make the existing helpers metadata-aware, accepting a small stable read</div>
<div style="font-size: 11pt; color: rgb(0, 0, 0);"> path for the configured mbuf object size; or</div>
<div style="direction: ltr; font-size: 11pt; color: rgb(0, 0, 0);"><br>
</div>
<div style="font-size: 11pt; color: rgb(0, 0, 0);"> 2. keep existing helpers fixed and add explicit metadata-aware APIs for opt-in</div>
<div style="font-size: 11pt; color: rgb(0, 0, 0);"> users, while PMDs/libraries use internal cached layout values.</div>
<div style="direction: ltr; font-size: 11pt; color: rgb(0, 0, 0);"><br>
</div>
<div style="font-size: 11pt; color: rgb(0, 0, 0);"> My preference is to remove the writable global, keep PMD fast paths cached,</div>
<div style="font-size: 11pt; color: rgb(0, 0, 0);"> add the necessary driver guards, and avoid requiring applications to enable</div>
<div style="font-size: 11pt; color: rgb(0, 0, 0);"> experimental API merely because a PMD supports metadata. I’m open to either</div>
<div style="font-size: 11pt; color: rgb(0, 0, 0);"> helper model, but I’d rather align on that before producing another revision.</div>
<div style="direction: ltr; font-size: 11pt;"><br>
</div>
<div style="direction: ltr; font-size: 11pt;"><br>
</div>
<div style="direction: ltr; font-size: 11pt;">drivers/net/cnxk, drivers/event/cnxk: the hardware skip fields are<br>
narrow and roc_nix_rq_init() only checks 8-byte alignment, not range:<br>
wqe_skip 2 bits, in 128B lines -> rte_mbuf_size() <= 384<br>
later_skip 6 bits, in 8B units -> mbuf + meta + priv <= 504<br>
first_skip 7 bits, in 8B units -> mbuf + meta + priv<br>
+ headroom <= 1016<br>
With --mbuf-metadata-size=384, wqe_skip becomes 4 and is truncated to<br>
0. With metadata 256 and priv_size 128, later_skip is 512 and is<br>
truncated to 0. Hardware then writes the WQE/packet over the mbuf.<br>
The driver must reject configurations that do not fit, as was done<br>
for octeontx.<br>
<br>
#RT:</div>
<div style="direction: ltr; font-size: 11pt; color: rgb(0, 0, 0);"> Agreed. This is a real correctness issue, not just a local type-width issue.</div>
<div style="font-size: 11pt; color: rgb(0, 0, 0);"> The local roc_nix_rq fields are wider, but the programmed CNXK hardware</div>
<div style="font-size: 11pt; color: rgb(0, 0, 0);"> context fields are limited to wqe_skip:2, later_skip:6, and first_skip:7, so</div>
<div style="font-size: 11pt; color: rgb(0, 0, 0);"> values beyond those limits can truncate. I will add explicit CNXK validation</div>
<div style="font-size: 11pt; color: rgb(0, 0, 0);"> before programming the RQ context and reject unsupported combinations. The</div>
<div style="font-size: 11pt; color: rgb(0, 0, 0);"> checks need to cover both the global metadata size and the per-pool layout:</div>
<div style="font-size: 11pt; color: rgb(0, 0, 0);"> wqe_skip limits sizeof(struct rte_mbuf) + metadata_size to 384 bytes,</div>
<div style="font-size: 11pt; color: rgb(0, 0, 0);"> later_skip limits sizeof(struct rte_mbuf) + metadata_size + priv_size to 504</div>
<div style="font-size: 11pt; color: rgb(0, 0, 0);"> bytes, and first_skip limits sizeof(struct rte_mbuf) + metadata_size +</div>
<div style="font-size: 11pt; color: rgb(0, 0, 0);"> priv_size + headroom to 1016 bytes. With the current 128-byte mbuf header,</div>
<div style="font-size: 11pt; color: rgb(0, 0, 0);"> --mbuf-metadata-size=256 is already the CNXK metadata ceiling from wqe_skip,</div>
<div style="font-size: 11pt; color: rgb(0, 0, 0);"> while remaining private-data headroom is checked per pool.</div>
<div style="direction: ltr; font-size: 11pt; color: rgb(0, 0, 0);"><br>
</div>
<div style="direction: ltr; font-size: 11pt;"><br>
Warning<br>
<br>
EAL defines and exports a symbol named rte_mbuf_*, and it is declared<br>
extern in both eal_private.h and rte_mbuf_core.h. Follow the existing<br>
--mbuf-pool-ops-name pattern: EAL stores the value and provides a<br>
getter (rte_eal_mbuf_user_pool_ops() equivalent); mbuf owns any<br>
mbuf-named symbols.<br>
<br>
#RT:</div>
<div style="direction: ltr; font-size: 11pt; color: rgb(0, 0, 0);"> Agreed. I will remove the rte_mbuf_* storage/export from EAL. The configured</div>
<div style="font-size: 11pt; color: rgb(0, 0, 0);"> value should be stored and shared by EAL, following the existing --mbuf-</div>
<div style="font-size: 11pt; color: rgb(0, 0, 0);"> pool-ops-name style, while mbuf-owned APIs/helpers remain in the mbuf</div>
<div style="font-size: 11pt; color: rgb(0, 0, 0);"> library. The next revision will separate EAL configuration/storage from mbuf</div>
<div style="font-size: 11pt; color: rgb(0, 0, 0);"> naming/ownership instead of declaring the same external value through both</div>
<div style="font-size: 11pt; color: rgb(0, 0, 0);"> EAL-private and mbuf public headers.</div>
<div style="direction: ltr; font-size: 11pt;"><br>
</div>
<div style="font-size: 11pt;"><br>
Fast path cost. rte_mbuf_size() turns a compile-time constant into a<br>
load of a global (through the GOT in shared builds) in<br>
rte_mbuf_to_priv(), rte_mbuf_from_indirect(), rte_mbuf_buf_addr() and<br>
rte_pktmbuf_detach(), all inline and used per packet. cnxk Rx vector<br>
paths now do vdupq_n_u64(rte_mbuf_size()) inside the burst loop, and<br>
the cn9k get_work asm needs an extra register load per event. The<br>
cnxk paths should cache the value in rxq/ws at setup like data_off.<br>
Need l3fwd or testpmd numbers with the option unset, on cnxk at<br>
least, before this goes in.<br>
<br>
lib/eal/common/eal_common_options.c: upper bound of UINT16_MAX is not<br>
justified. Beyond the cnxk limits above, examples/fips_validation<br>
DEF_MBUF_SEG_SIZE is UINT16_MAX - rte_mbuf_size() - headroom and<br>
wraps for large values. Cap the option at a small number of cache<br>
lines and document the limit.</div>
<div style="direction: ltr; font-size: 11pt;"><br>
</div>
<div style="font-size: 11pt;">#RT:</div>
<div style="font-size: 11pt; color: rgb(0, 0, 0);"> Agreed. UINT16_MAX is not a justified upper bound. I will cap the option to</div>
<div style="font-size: 11pt; color: rgb(0, 0, 0);"> A small documented number of cache lines and add boundary tests. CNXK also</div>
<div style="font-size: 11pt; color: rgb(0, 0, 0);"> needs separate queue/pool validation because its later_skip and first_skip limits</div>
<div style="font-size: 11pt; color: rgb(0, 0, 0);"> include priv_size and headroom; with the current 128-byte mbuf header, its</div>
<div style="font-size: 11pt;"><span style="color: rgb(0, 0, 0);"> global metadata ceiling is 256 bytes due to wqe_skip.<br>
<br>
drivers/net/cnxk/cn10k_rx.h nix_cqe_xtract_mseg(): the rx_inj block<br>
was reindented one tab too deep; it is still inside the same if.<br>
Only the two wqe assignment lines need to change.<br>
<br>
Too much in one patch. Put the mbuf helpers first, then the<br>
conversions, then the feature:<br>
1. mbuf: add rte_mbuf_size() returning sizeof(struct rte_mbuf),<br>
no behaviour change<br>
2. convert libs, apps and examples<br>
3. convert drivers (one per driver family, so maintainers can ack)<br>
4. add the EAL option, metadata dynfield flag, driver rejections<br>
and docs<br>
Patches 1-3 are a no-op and can be reviewed and merged on their own.<br>
Patch 4 is then small enough to review for what it actually changes.<br>
It also makes it bisectable when a driver conversion is wrong.<br>
<br>
Many of the conversions open-code rte_mbuf_size() where an existing<br>
helper fits: RTE_PTR_ADD(m, rte_mbuf_size()) is rte_mbuf_to_priv(m),<br>
and mbuf + rte_mbuf_size() + priv_size is rte_mbuf_buf_addr(). Use<br>
the helpers so drivers stop depending on the layout directly.<br>
<br>
Info<br>
<br>
Unrelated whitespace churn: blank line removed in<br>
drivers/mempool/octeontx/meson.build, in the release notes Known<br>
Issues section, and in test_mbuf(). Drop these.<br>
<br>
lib/mbuf/rte_mbuf_dyn.c: the "check if this offset can be used"<br>
comment now sits above dynfield_in_metadata() instead of<br>
check_offset().<br>
<br>
drivers/mempool/octeontx: the check rejects every fpavf pool, not<br>
just pktmbuf pools. mp->private_data_size is known at alloc time, so<br>
the check could be limited to pools carrying<br>
rte_pktmbuf_pool_private. Not required since net/octeontx is the only<br>
user that cares.<br>
<br>
app/test/test_mbuf.c: with metadata configured, dynfield_fail_big<br>
(size 128) passes the size check and fails only because no free<br>
space exists, so the size limit path is no longer tested in the<br>
metadata run.<br>
<br>
Any out-of-tree code using (m + 1) or sizeof(struct rte_mbuf) to find<br>
private data silently breaks when a user adds this EAL option. That<br>
belongs in the API changes section of the release notes, not only<br>
under New Features.<br>
</span></div>
<div style="direction: ltr; font-size: 11pt;"><br>
</div>
<div style="direction: ltr; font-size: 11pt; color: rgb(0, 0, 0);">#RT:</div>
<div style="direction: ltr; font-size: 11pt; color: rgb(0, 0, 0);"> I will also clean up the lower-level review items in the next revision:</div>
<div style="font-size: 11pt; color: rgb(0, 0, 0);"> remove unrelated whitespace churn, restore the misplaced check_offset()</div>
<div style="font-size: 11pt; color: rgb(0, 0, 0);"> comment, tighten or document the octeontx rejection as appropriate, keep the</div>
<div style="font-size: 11pt; color: rgb(0, 0, 0);"> mbuf tests covering the intended size-limit path, and move the out-of-tree</div>
<div style="font-size: 11pt; color: rgb(0, 0, 0);"> layout impact note into the API changes release-note section.</div>
</div>
</div>
</body>
</html>