From: "Morten Brørup" <mb@smartsharesystems.com>
To: "Thomas Monjalon" <thomas@monjalon.net>, <dev@dpdk.org>,
"Shani Peretz" <shperetz@nvidia.com>
Cc: <viacheslavo@nvidia.com>, <bruce.richardson@intel.com>,
<stephen@networkplumber.org>
Subject: RE: [PATCH v3 2/5] mbuf: record mbuf operations history
Date: Thu, 2 Oct 2025 09:37:38 +0200 [thread overview]
Message-ID: <98CBD80474FA8B44BF855DF32C47DC35F6548D@smartserver.smartshare.dk> (raw)
In-Reply-To: <20250930233828.3999565-3-thomas@monjalon.net>
> From: Thomas Monjalon [mailto:thomas@monjalon.net]
> Sent: Wednesday, 1 October 2025 01.25
>
> From: Shani Peretz <shperetz@nvidia.com>
>
> This feature is designed to monitor the lifecycle of mbufs
> as they move between the application and the PMD.
> It will allow to track the operations and transitions
> of each mbuf throughout the system, helping in debugging
> and understanding objects flow.
>
> As a debug feature impacting the data path, it is disabled by default.
>
> The implementation uses a dynamic field to store a 64-bit
> atomic history value in each mbuf. Each operation is represented
> by a 4-bit value, allowing up to 16 operations to be tracked.
> Atomic operations ensure thread safety for cloned mbufs accessed
> by multiple lcores. The dynamic field is automatically initialized
> when the first mbuf pool is created.
>
> Some operations done in the mbuf library are marked.
> More operations from other libraries or the application can be marked.
>
> Signed-off-by: Shani Peretz <shperetz@nvidia.com>
> Signed-off-by: Thomas Monjalon <thomas@monjalon.net>
> ---
[...]
> diff --git a/config/meson.build b/config/meson.build
> index 55497f0bf5..d1f21f3115 100644
> --- a/config/meson.build
> +++ b/config/meson.build
> @@ -379,6 +379,7 @@ if get_option('mbuf_refcnt_atomic')
> dpdk_conf.set('RTE_MBUF_REFCNT_ATOMIC', true)
> endif
> dpdk_conf.set10('RTE_IOVA_IN_MBUF', get_option('enable_iova_as_pa'))
> +dpdk_conf.set10('RTE_MBUF_HISTORY_DEBUG',
> get_option('enable_mbuf_history'))
Not really important, just a suggestion:
The mempool library has its debug options defined in /config/rte_config.h.
For consistency, the mbuf history debug option also belongs in /config/rte_config.h, instead of being a meson option.
It also means using "#ifdef RTE_MBUF_HISTORY_DEBUG" instead of "#if RTE_MBUF_HISTORY_DEBUG".
[...]
> +RTE_EXPORT_EXPERIMENTAL_SYMBOL(rte_mbuf_history_dump_mempool, 25.11)
> +void rte_mbuf_history_dump_mempool(FILE *f, struct rte_mempool *mp)
> +{
> +#if !RTE_MBUF_HISTORY_DEBUG
> + RTE_SET_USED(f);
> + RTE_SET_USED(mp);
> + MBUF_LOG(INFO, "mbuf history recorder is not enabled");
> +#else
> + if (f == NULL) {
> + MBUF_LOG(ERR, "Invalid mbuf dump file.");
> + return;
> + }
> + if (mp == NULL) {
> + fprintf(f, "ERROR: Invalid mempool pointer\n");
Should be MBUF_LOG(ERR, ...), not fprintf().
> + return;
> + }
> + if (rte_mbuf_history_field_offset < 0) {
> + fprintf(f, "WARNING: mbuf history not initialized. Call
> rte_mbuf_history_init() first.\n");
Should be MBUF_LOG(ERR, ...), not fprintf().
> + return;
> + }
Since the type of objects held in a mempool is not identifiable as mbufs, you should check that (mp->elt_size >= sizeof(struct rte_mbuf)). Imagine some non-mbuf mempool holding 64 byte sized objects, and rte_mbuf_history_field_offset being in the second cache line.
You might want to log an error if called directly, and silently skip of called from rte_mbuf_history_dump_all(), so suggest adding a wrapper when calling this function through rte_mempool_walk().
> + mbuf_history_get_stats(mp, f);
> +#endif
> +}
> +
> +RTE_EXPORT_EXPERIMENTAL_SYMBOL(rte_mbuf_history_dump_all, 25.11)
> +void rte_mbuf_history_dump_all(FILE *f)
> +{
> +#if !RTE_MBUF_HISTORY_DEBUG
> + RTE_SET_USED(f);
> + MBUF_LOG(INFO, "mbuf history recorder is not enabled");
> +#else
> + if (f == NULL) {
> + MBUF_LOG(ERR, "Invalid mbuf dump file.");
> + return;
> + }
> + fprintf(f, "mbuf history statistics:\n");
> + if (rte_mbuf_history_field_offset < 0) {
> + fprintf(f, "WARNING: mbuf history not initialized. Call
> rte_mbuf_history_init() first.\n\n");
> + return;
> + }
> + rte_mempool_walk(mbuf_history_get_stats, f);
> +#endif
> +}
> +
[...]
> +/**
> + * Mark an mbuf with a history event.
> + *
> + * @param m
> + * Pointer to the mbuf.
> + * @param op
> + * The operation to record.
> + */
> +static inline void rte_mbuf_history_mark(struct rte_mbuf *m, uint32_t
> op)
> +{
> +#if !RTE_MBUF_HISTORY_DEBUG
> + RTE_SET_USED(m);
> + RTE_SET_USED(op);
> +#else
> + RTE_ASSERT(rte_mbuf_history_field_offset >= 0);
> + RTE_ASSERT(op < RTE_MBUF_HISTORY_OP_MAX);
> + if (unlikely (m == NULL))
> + return;
> +
> + rte_mbuf_history_t *history_field = RTE_MBUF_DYNFIELD(m,
> + rte_mbuf_history_field_offset, rte_mbuf_history_t *);
> + uint64_t history = rte_atomic_load_explicit(history_field,
> rte_memory_order_acquire);
> + history = (history << RTE_MBUF_HISTORY_BITS) | op;
> + rte_atomic_store_explicit(history_field, history,
> rte_memory_order_release);
This is not thread safe.
Some other thread can race to update history_field after this thread loads it, so when this tread stores the updated history, the other thread's history update is overwritten and lost.
To make it thread safe, you must use a CAS operation and start over if it failed.
> +#endif
> +}
next prev parent reply other threads:[~2025-10-02 7:37 UTC|newest]
Thread overview: 105+ messages / expand[flat|nested] mbox.gz Atom feed top
2025-06-16 7:29 [RFC PATCH 0/5] Introduce mempool object new debug capabilities Shani Peretz
2025-06-16 7:29 ` [RFC PATCH 1/5] mempool: record mempool objects operations history Shani Peretz
2025-06-16 7:29 ` [RFC PATCH 2/5] drivers: add mempool history compilation flag Shani Peretz
2025-06-16 7:29 ` [RFC PATCH 3/5] net/mlx5: mark an operation in mempool object's history Shani Peretz
2025-06-16 7:29 ` [RFC PATCH 4/5] app/testpmd: add testpmd command to dump mempool history Shani Peretz
2025-06-16 7:29 ` [RFC PATCH 5/5] usertool: add a script to parse mempool history dump Shani Peretz
2025-06-16 15:30 ` [RFC PATCH 0/5] Introduce mempool object new debug capabilities Stephen Hemminger
2025-06-19 12:57 ` Morten Brørup
2025-07-07 5:46 ` Shani Peretz
2025-07-07 5:45 ` Shani Peretz
2025-07-07 12:10 ` Morten Brørup
2025-07-19 14:39 ` Morten Brørup
2025-08-25 11:27 ` Slava Ovsiienko
2025-09-01 15:34 ` Morten Brørup
2025-09-16 15:12 ` [PATCH v2 0/4] add mbuf " Shani Peretz
2025-09-16 15:12 ` [PATCH v2 1/4] mbuf: record mbuf operations history Shani Peretz
2025-09-16 21:17 ` Stephen Hemminger
2025-09-16 21:33 ` Thomas Monjalon
2025-09-17 1:22 ` Morten Brørup
2025-09-17 14:46 ` Morten Brørup
2025-09-19 9:14 ` Shani Peretz
2025-09-16 15:12 ` [PATCH v2 2/4] net/mlx5: mark an operation in mbuf's history Shani Peretz
2025-09-16 21:14 ` Stephen Hemminger
2025-09-16 21:31 ` Thomas Monjalon
2025-09-17 15:04 ` Stephen Hemminger
2025-09-16 15:12 ` [PATCH v2 3/4] app/testpmd: add testpmd command to dump mbuf history Shani Peretz
2025-09-16 15:12 ` [PATCH v2 4/4] usertool: add a script to parse mbuf history dump Shani Peretz
2025-09-30 23:25 ` [PATCH v3 0/5] add mbuf debug capabilities Thomas Monjalon
2025-09-30 23:25 ` [PATCH v3 1/5] mbuf: move header include for logs Thomas Monjalon
2025-09-30 23:25 ` [PATCH v3 2/5] mbuf: record mbuf operations history Thomas Monjalon
2025-10-01 0:12 ` Thomas Monjalon
2025-10-02 7:37 ` Morten Brørup [this message]
2025-10-02 21:23 ` Thomas Monjalon
2025-10-13 18:39 ` Thomas Monjalon
2025-10-13 20:08 ` Morten Brørup
2025-10-13 21:07 ` Thomas Monjalon
2025-10-14 10:04 ` Morten Brørup
2025-09-30 23:25 ` [PATCH v3 3/5] ethdev: mark mbufs in burst functions Thomas Monjalon
2025-10-02 7:44 ` Morten Brørup
2025-10-13 15:32 ` Thomas Monjalon
2025-09-30 23:25 ` [PATCH v3 4/5] app/testpmd: add commands to dump mbuf history Thomas Monjalon
2025-10-01 8:20 ` Stephen Hemminger
2025-10-13 15:31 ` Thomas Monjalon
2025-09-30 23:25 ` [PATCH v3 5/5] usertools/mbuf: parse mbuf history dump Thomas Monjalon
2025-10-02 8:07 ` Robin Jarry
2025-10-13 21:16 ` [PATCH v4 0/7] add mbuf debug capabilities Thomas Monjalon
2025-10-13 21:16 ` [PATCH v4 1/7] doc: explain debug options in mbuf guide Thomas Monjalon
2025-10-13 21:16 ` [PATCH v4 2/7] mbuf: move header include for logs Thomas Monjalon
2025-10-13 21:16 ` [PATCH v4 3/7] mbuf: record mbuf operations history Thomas Monjalon
2025-10-13 21:16 ` [PATCH v4 4/7] ethdev: mark mbufs in burst functions Thomas Monjalon
2025-10-13 21:16 ` [PATCH v4 5/7] app/testpmd: use space separator in dump commands Thomas Monjalon
2025-10-13 21:16 ` [PATCH v4 6/7] app/testpmd: add commands to dump mbuf history Thomas Monjalon
2025-10-13 21:16 ` [PATCH v4 7/7] usertools/mbuf: parse mbuf history dump Thomas Monjalon
2025-10-14 6:58 ` [PATCH v5 0/7] add mbuf debug capabilities Thomas Monjalon
2025-10-14 6:58 ` [PATCH v5 1/7] doc: explain debug options in mbuf guide Thomas Monjalon
2025-10-14 6:58 ` [PATCH v5 2/7] mbuf: move header include for logs Thomas Monjalon
2025-10-14 8:47 ` Morten Brørup
2025-10-14 6:58 ` [PATCH v5 3/7] mbuf: record mbuf operations history Thomas Monjalon
2025-10-14 9:59 ` Morten Brørup
2025-10-14 12:03 ` Thomas Monjalon
2025-10-14 12:31 ` Thomas Monjalon
2025-10-14 6:58 ` [PATCH v5 4/7] ethdev: mark mbufs in burst functions Thomas Monjalon
2025-10-14 6:58 ` [PATCH v5 5/7] app/testpmd: use space separator in dump commands Thomas Monjalon
2025-10-14 6:58 ` [PATCH v5 6/7] app/testpmd: add commands to dump mbuf history Thomas Monjalon
2025-10-14 8:45 ` Morten Brørup
2025-10-14 9:43 ` Thomas Monjalon
2025-10-14 9:48 ` Bruce Richardson
2025-10-14 9:55 ` Thomas Monjalon
2025-10-14 6:58 ` [PATCH v5 7/7] usertools/mbuf: parse mbuf history dump Thomas Monjalon
2025-10-14 12:33 ` [PATCH v6 0/7] add mbuf debug capabilities Thomas Monjalon
2025-10-14 12:33 ` [PATCH v6 1/7] doc: explain debug options in mbuf guide Thomas Monjalon
2025-10-14 12:33 ` [PATCH v6 2/7] mbuf: move header include for logs Thomas Monjalon
2025-10-14 12:33 ` [PATCH v6 3/7] mbuf: record mbuf operations history Thomas Monjalon
2025-10-16 9:04 ` Morten Brørup
2025-10-16 9:53 ` Thomas Monjalon
2025-10-14 12:33 ` [PATCH v6 4/7] ethdev: mark mbufs in burst functions Thomas Monjalon
2025-10-16 9:26 ` Morten Brørup
2025-10-14 12:33 ` [PATCH v6 5/7] app/testpmd: use space separator in dump commands Thomas Monjalon
2025-10-15 17:52 ` Stephen Hemminger
2025-10-15 19:10 ` Thomas Monjalon
2025-10-16 9:28 ` Morten Brørup
2025-10-14 12:33 ` [PATCH v6 6/7] app/testpmd: add commands to dump mbuf history Thomas Monjalon
2025-10-14 12:33 ` [PATCH v6 7/7] usertools/mbuf: parse mbuf history dump Thomas Monjalon
2025-10-14 14:03 ` Robin Jarry
2025-10-15 17:50 ` Stephen Hemminger
2025-10-15 19:11 ` Thomas Monjalon
2025-10-15 19:41 ` Robin Jarry
2025-10-16 8:10 ` Bruce Richardson
2025-10-16 8:42 ` Thomas Monjalon
2025-10-16 12:07 ` Robin Jarry
2025-10-16 9:46 ` Morten Brørup
2025-10-16 20:34 ` [PATCH v7 0/7] add mbuf debug capabilities Thomas Monjalon
2025-10-16 20:34 ` [PATCH v7 1/7] doc: explain debug options in mbuf guide Thomas Monjalon
2025-10-16 22:30 ` Morten Brørup
2025-10-16 20:34 ` [PATCH v7 2/7] mbuf: move header include for logs Thomas Monjalon
2025-10-16 20:34 ` [PATCH v7 3/7] mbuf: record mbuf operations history Thomas Monjalon
2025-10-16 22:29 ` Morten Brørup
2025-10-17 7:32 ` Thomas Monjalon
2025-10-17 7:55 ` Morten Brørup
2025-10-17 9:09 ` Thomas Monjalon
2025-10-16 20:34 ` [PATCH v7 4/7] ethdev: mark mbufs in burst functions Thomas Monjalon
2025-10-16 20:34 ` [PATCH v7 5/7] app/testpmd: use space separator in dump commands Thomas Monjalon
2025-10-16 20:34 ` [PATCH v7 6/7] app/testpmd: add commands to dump mbuf history Thomas Monjalon
2025-10-16 20:34 ` [PATCH v7 7/7] usertools/mbuf: parse mbuf history dump Thomas Monjalon
2025-10-17 16:10 ` [PATCH v7 0/7] add mbuf debug capabilities Thomas Monjalon
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=98CBD80474FA8B44BF855DF32C47DC35F6548D@smartserver.smartshare.dk \
--to=mb@smartsharesystems.com \
--cc=bruce.richardson@intel.com \
--cc=dev@dpdk.org \
--cc=shperetz@nvidia.com \
--cc=stephen@networkplumber.org \
--cc=thomas@monjalon.net \
--cc=viacheslavo@nvidia.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;
as well as URLs for NNTP newsgroup(s).