From: Stephen Hemminger <stephen@networkplumber.org>
To: Kai Ji <kai.ji@intel.com>
Cc: dev@dpdk.org, stable@dpdk.org,
Bruce Richardson <bruce.richardson@intel.com>,
Konstantin Ananyev <konstantin.ananyev@huawei.com>,
Jie Liu <liujie5@linkdatatechnology.com>
Subject: Re: [PATCH v3] net/sxe2: replace private mempool cache bypass with rte_mbuf_raw_free_bulk
Date: Tue, 22 Sep 2026 09:24:31 -0700 [thread overview]
Message-ID: <20260922092431.0c78a054@phoenix.local> (raw)
In-Reply-To: <20260921155214.2690954-1-kai.ji@intel.com>
On Mon, 21 Sep 2026 15:52:13 +0000
Kai Ji <kai.ji@intel.com> wrote:
> The AVX-512 TX completion path directly manipulated the mempool cache
> internals (cache->objs, cache->len, cache->flushthresh) instead of using
> the mempool API. This pattern is the same private bypass that existed in
> the Intel common TX library before it was removed by commit 062d6fe5d00d
> ("net/intel: do not bypass mbuf lib for buffer fast-free") for the same
> reason: it omits mbuf instrumentation (history marking) and reaches
> directly into mempool cache internals, including the flushthresh field
> that is now obsolete (kept only for API/ABI compatibility), making the
> private fast path fragile against mempool cache layout changes.
>
> Replace with a single rte_mbuf_raw_free_bulk() call, matching the Intel
> common library. The RTE_ETH_TX_OFFLOAD_MBUF_FAST_FREE contract in
> rte_ethdev.h requires the application to guarantee that per-queue all
> mbufs come from the same mempool, have refcnt == 1, and are direct;
> that documented guarantee, whose @see already points to
> rte_mbuf_raw_free_bulk(), is exactly what makes this call correct. The
> compiler inlines the bulk-free call to eliminate the overhead
> difference.
>
> Fixes: 0af0bdcdcf83 ("net/sxe2: add AVX512 Rx and Tx")
> Cc: stable@dpdk.org
>
> Signed-off-by: Kai Ji <kai.ji@intel.com>
> ---
AI review had some suggestions here. They seem good:
Review: [PATCH v3] net/sxe2: replace private mempool cache bypass with
rte_mbuf_raw_free_bulk
Applied cleanly to main (6bbb7b3). common/sxe2 + net/sxe2 build clean
with -Dwerror=true, AVX512 object included.
The code change is correct. rte_mbuf_raw_free_bulk() takes the mbuf
array directly and the static_assert covers the cast. Remaining
comments are on the tags and the commit message.
Warning
Drop "Cc: stable@dpdk.org". net/sxe2 first shipped in v26.07
(0af0bdcdcf83 is contained in v26.07-rc2 onward); 25.11 LTS does
not have this driver, so there is no stable branch to backport to.
The old code was not functionally broken on current mempool:
flushthresh is still initialized to cache->size, objs[] is still
2 * RTE_MEMPOOL_CACHE_MAX_SIZE, and rs_thresh is capped at 64, so
the cache invariant held. What is lost is mbuf history marking
and debug sanity checks. That is a cleanup, not a stable fix;
the Fixes: tag is optional.
Commit message is too long for a 35 line deletion. Also "The
compiler inlines the bulk-free call to eliminate the overhead
difference" is an unsupported claim; either give throughput
numbers or drop the sentence. Suggest:
The AVX512 Tx free path writes directly into the mempool
cache (objs, len, flushthresh). This skips mbuf history
marking and depends on mempool cache internals; flushthresh
is now obsolete.
Use rte_mbuf_raw_free_bulk(), as done for net/intel in
commit 062d6fe5d00d ("net/intel: do not bypass mbuf lib for
buffer fast-free"). MBUF_FAST_FREE guarantees single pool,
refcnt 1 and direct mbufs per queue.
Info
The "(rs_thresh & 31) == 0" condition only existed to feed the
32-wide unrolled AVX512 copy loop. rs_thresh is validated to
32..64 in sxe2_txrx_vec.c, so e.g. rs_thresh=48 currently falls
back to the per-mbuf prefree path even with MBUF_FAST_FREE set.
With rte_mbuf_raw_free_bulk() the guard serves no purpose; drop
it.
No v2 -> v3 changelog below the "---".
prev parent reply other threads:[~2026-09-22 16:24 UTC|newest]
Thread overview: 5+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-08-20 15:50 [dpdk-dev v1] net/sxe2: replace private mempool cache bypass with rte_mbuf_raw_free_bulk Kai Ji
2026-08-21 18:03 ` Stephen Hemminger
2026-08-27 15:10 ` [dpdk-dev v2] " Kai Ji
2026-09-21 15:52 ` [PATCH v3] " Kai Ji
2026-09-22 16:24 ` Stephen Hemminger [this message]
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=20260922092431.0c78a054@phoenix.local \
--to=stephen@networkplumber.org \
--cc=bruce.richardson@intel.com \
--cc=dev@dpdk.org \
--cc=kai.ji@intel.com \
--cc=konstantin.ananyev@huawei.com \
--cc=liujie5@linkdatatechnology.com \
--cc=stable@dpdk.org \
/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