[PATCH v3 2/5] mbuf: record mbuf operations history

[Morten Brørup]

> From: Thomas Monjalon [mailto:thomas at monjalon.net]
> Sent: Wednesday, 1 October 2025 01.25
> 
> From: Shani Peretz <shperetz at 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 at nvidia.com>
> Signed-off-by: Thomas Monjalon <thomas at 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
> +}