Re: [PATCH] mbuf: avoid temporary array for bulk free

[Stephen Hemminger <stephen@networkplumber.org>]

On Mon, 21 Sep 2026 21:28:56 -0400
Nam Tran <hoangnamtran18122005@gmail.com> wrote:

> rte_pktmbuf_free_bulk() currently stages freeable mbufs in a
> temporary array before returning them to their mempool. For flat
> packet arrays, this requires copying pointers even though the
> original array already contains contiguous freeable mbufs.
> 
> Track contiguous same-pool runs in the input array and pass them
> directly to rte_mbuf_raw_free_bulk(). Flush a run when encountering
> a NULL mbuf, an mbuf retained by reference counting, or a pool
> change. Preserve the existing array-based implementation as the
> fallback for chained packets.
> 
> On an ARM64 Linux test environment, same-binary A/B measurements
> using rte_rdtsc showed lower median timer ticks per call for flat
> bulk frees:
> 
>   burst  32:  2.05 ->  1.50
>   burst  64:  5.16 ->  4.52
>   burst 128: 11.16 ->  6.90
>   burst 256: 26.46 -> 19.73
> 
> This corresponds to reductions of approximately 12% to 38% across
> the tested burst sizes.
> 
> Add coverage for NULL entries, mixed mempools, and shared mbufs.
> 
> Signed-off-by: Nam Tran <hoangnamtran18122005@gmail.com>
> ---

More detailed AI review (Claude Opus 5)

Subject: Re: [PATCH] mbuf: avoid temporary array for bulk free

Warning:

lib/mbuf/rte_mbuf.c: runs are unbounded. The old code flushed at
RTE_PKTMBUF_FREE_PENDING_SZ (64). Now a same-pool run can be the whole
burst, and rte_mempool_do_generic_put() sends any n > cache->size / 2
straight to rte_mempool_ops_enqueue_bulk(), bypassing the per-lcore
cache. With a 256 entry cache, a 256 burst that previously went into
the cache in 64 entry chunks now hits the ring every time. That is
cheap on a single lcore, which is what the benchmark measured, but is
shared ring traffic with multiple lcores, and hands back cold objects
instead of keeping hot ones in cache. Cap run_count at
RTE_PKTMBUF_FREE_PENDING_SZ and flush when reached.

Benchmark: rte_rdtsc() on arm64 reads the generic timer (cntvct_el0)
unless built with PMU support; a delta of ~0.5 ticks per call is at
the resolution limit. Please state the timer frequency, the mempool
cache size used, the number of lcores, and include x86 results. The
test pools in test_mbuf.c have no cache, so they do not exercise the
cache path at all.

Info:

The flush sequence is open coded four times, and
rte_mbuf_raw_free_bulk() is __rte_always_inline, so the function body
grows accordingly. A small static helper or restructuring the loop so
NULL, not-freed, and pool-change share one flush point would be
cleaner.

Tests: add a case that mixes flat and chained packets in one array
(flat run pending when the chain is hit, then flat after it), and one
with an indirect (cloned) mbuf in the flat path. Current tests do not
cover the transition into __rte_pktmbuf_free_bulk_fallback() with a
non-empty run, which is the new logic most likely to break.

Nit: the blank line added after "m = mbufs[idx];" in the fallback is
unrelated churn.