From: Stephen Hemminger <stephen@networkplumber.org>
To: "Randy Tice (rtice)" <rtice@cisco.com>
Cc: "dev@dpdk.org" <dev@dpdk.org>,
"Morten Brørup" <mb@smartsharesystems.com>,
"Nithin Dabilpuram" <ndabilpuram@marvell.com>,
"Harman Kalra" <hkalra@marvell.com>
Subject: Re: [PATCH v4 0/1] mbuf: add runtime metadata dynamic-field storage
Date: Fri, 9 Oct 2026 09:14:05 -0700 [thread overview]
Message-ID: <20261009091405.46b9f3bf@phoenix.local> (raw)
In-Reply-To: <PH0PR11MB7616A68AC3EA1349948CAC75A9932@PH0PR11MB7616.namprd11.prod.outlook.com>
On Thu, 8 Oct 2026 16:15:26 +0000
"Randy Tice (rtice)" <rtice@cisco.com> wrote:
> 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.
>
> #RT:
> Our implementation currently requires an additional 256 bytes of metadata per
> mbuf. That does not fit in the existing dynamic-field storage, so today we
> carry a private patch to extend struct rte_mbuf. The goal of this work is to
> replace that private struct change with a supported upstream mechanism.
>
> We did look at using mbuf private data first. It can work when the application
> owns all mbuf pool creation, but it is not sufficient for our case without
> another global/base reservation mechanism. Some mbuf pools are created by
> drivers or libraries rather than directly by the application; the CNXK inline
> IPsec/OOP meta pool is one example (NIX_INL_META_POOL, created through
> cnxk_nix_inl_meta_pool_cb()). To make private data work there, we had to add
> an EAL argument that reserved a base private size for all pktmbufs, including
> PMD-created pools.
>
> Private data also lacks a central layout registry. If multiple modules use
> private data, they must coordinate offsets out of band to avoid overlaying
> each other. The dynamic-field registry solves that coordination problem, but
> the existing copied dynamic-field area is too small and has copy/clone
> semantics that are wrong for this metadata.
>
> That is why this version uses a globally configured per-mbuf metadata area
> managed by the dynamic-field registry, with explicit metadata fields that are
> not copied by generic copy/clone/attach paths. This direction came out of the
> prior discussion with you, Morten, and me: avoid a Cisco-private mbuf struct
> patch, avoid per-pool private-data layout coordination, and keep sizeof(struct
> rte_mbuf) fixed.
You uncovered a design flaw in the CNXK driver and the
proposed solution is wider than it needs to be.
All mbufs visible to application must come from mempools controlled by the application.
The design of CNXK driver is wrong, it shouldn't be using a private hidden pool.
It is ok for drivers to have hidden mempools that are used for non-visible things,
an example is the packet capture mempool where the mbufs only go into capture stream.
More long winded AI description:
On Thu, 8 Oct 2026 16:15:26 +0000
"Randy Tice (rtice)" <rtice@cisco.com> wrote:
> We did look at using mbuf private data first. It can work when the
> application owns all mbuf pool creation, but it is not sufficient for
> our case without another global/base reservation mechanism. Some mbuf
> pools are created by drivers or libraries rather than directly by the
> application; the CNXK inline IPsec/OOP meta pool is one example
That is the real bug. A driver should not hand the application mbufs
from a pool the application did not create. The Rx queue API already
takes the pool from the application, and rte_eth_rxconf can carry
more than one (rx_mempools). If cnxk needs a meta pool whose mbufs
reach the application, it should get it from the application, or at
minimum create it with the same priv_size as the Rx queue pool.
Internal pools are fine when the mbufs never cross the API, e.g.
dumpcap or the bonding LACP pool.
With that fixed, the existing private area does what you need. It is
per pool, sized by the application, and is not copied by copy, clone
or attach. Offset coordination inside it is the application's job,
and if a registry is wanted it can be layered on top of priv without
changing the mbuf layout.
So my answer to option 1 vs 2 is neither. I don't want a global
layout knob that puts a load in every inline helper and needs
per-driver range checks, to work around one driver.
What I would take:
- cnxk: validate wqe_skip/later_skip/first_skip against priv_size
and headroom. This is a bug today, independent of this series.
- cnxk: meta pool comes from, or matches, the application pool.
- ethdev: under RTE_ETHDEV_DEBUG_RX, check that received mbufs
belong to a pool configured on that queue.
Nithin, Harman: do mbufs from NIX_INL_META_POOL get returned to the
application by rx_burst, or are they only consumed internally?
next prev parent reply other threads:[~2026-10-09 16:14 UTC|newest]
Thread overview: 27+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-09-24 19:37 [PATCH 0/1] mbuf: add optional dynfield3 storage Randy L Tice
2026-09-24 19:37 ` [PATCH 1/1] " Randy L Tice
2026-09-25 9:48 ` Morten Brørup
2026-09-25 19:31 ` [PATCH v2 0/1] " Randy L Tice
2026-09-25 19:31 ` [PATCH v2 1/1] " Randy L Tice
2026-09-26 16:51 ` Stephen Hemminger
2026-09-28 14:45 ` Randy Tice (rtice)
2026-09-28 18:16 ` [PATCH v3 0/1] mbuf: add optional no-copy dynamic field storage Randy L Tice
2026-09-28 18:16 ` [PATCH v3 1/1] " Randy L Tice
2026-09-29 7:15 ` Morten Brørup
2026-09-29 11:59 ` Konstantin Ananyev
2026-09-29 12:44 ` Morten Brørup
2026-09-29 13:12 ` Konstantin Ananyev
2026-09-29 13:34 ` Morten Brørup
2026-09-29 14:14 ` Konstantin Ananyev
2026-09-29 14:44 ` Randy Tice (rtice)
2026-09-29 15:20 ` Morten Brørup
2026-09-29 19:03 ` Randy Tice (rtice)
2026-09-29 19:28 ` Konstantin Ananyev
2026-09-29 19:26 ` Konstantin Ananyev
2026-10-06 15:54 ` [PATCH v4 0/1] mbuf: add runtime metadata dynamic-field storage Randy L Tice
2026-10-06 15:54 ` [PATCH v4 1/1] " Randy L Tice
2026-10-06 16:51 ` [PATCH v4 0/1] " Stephen Hemminger
2026-10-08 16:15 ` Randy Tice (rtice)
2026-10-09 16:14 ` Stephen Hemminger [this message]
2026-10-09 18:12 ` Randy Tice (rtice)
2026-10-09 19:16 ` Stephen Hemminger
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20261009091405.46b9f3bf@phoenix.local \
--to=stephen@networkplumber.org \
--cc=dev@dpdk.org \
--cc=hkalra@marvell.com \
--cc=mb@smartsharesystems.com \
--cc=ndabilpuram@marvell.com \
--cc=rtice@cisco.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox