Skip to contentSkip to conversation

net/sxe2: replace private mempool cache bypass with rte_mbuf_raw_free_bulk

PATCH Superseded Kai Ji 2 participants 5 messages Aug 20 – Sep 22 Network drivers

lore
Share & tools
lore

Kai Ji

author
#0 lore raw

PATCH 1/1 net/sxe2: replace private mempool cache bypass with rte_mbuf_raw_free_bulk

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 062d6fe5d0e4 ("net/intel: do not bypass mbuf lib for buffer fast-free") for the same
reason: it omits mbuf instrumentation (history marking) and contains dead flush code that accesses cache->objs[cache->size], which is one past the end of the array when cache_size == RTE_MEMPOOL_CACHE_MAX_SIZE.

Replace with a single rte_mbuf_raw_free_bulk() call, matching the Intel common library. The MBUF_FAST_FREE offload guarantee (single pool, refcnt == 1) makes this correct and the compiler inlines the bulk-free call to eliminate the overhead difference.

---
 drivers/net/sxe2/sxe2_txrx_vec_avx512.c | 37 ++-----------------------
 1 file changed, 3 insertions(+), 34 deletions(-)
drivers/net/sxe2/sxe2_txrx_vec_avx512.c +3 -34
index a830c7a33b..9f992a9ddf 100644
@@ -18,9 +18,6 @@ static __rte_always_inline int32_t sxe2_tx_bufs_free_vec_avx512(struct sxe2_tx_q
 	struct rte_mbuf *mbuf;
 	struct rte_mbuf *mbuf_free_arr[SXE2_TX_FREE_BUFFER_SIZE_MAX_VEC];
 	struct rte_mempool *mp;
-	struct rte_mempool_cache *cache;
-	void **cache_objs;
-	uint32_t copied;
 	uint32_t i;
 	int32_t ret;
 	uint16_t rs_thresh;
@@ -41,37 +38,9 @@ static __rte_always_inline int32_t sxe2_tx_bufs_free_vec_avx512(struct sxe2_tx_q
 	if ((txq->offloads & RTE_ETH_TX_OFFLOAD_MBUF_FAST_FREE) &&
 			(rs_thresh & 31) == 0) {
 		mp = buffer[0].mbuf->pool;
-		cache = rte_mempool_default_cache(mp, rte_lcore_id());
-
-		if (cache == NULL || cache->len)
-			goto normal;
-
-		if (rs_thresh > RTE_MEMPOOL_CACHE_MAX_SIZE) {
-			(void)rte_mempool_ops_enqueue_bulk(mp, (void *)buffer, rs_thresh);
-			goto done;
-		}
-		cache_objs = &cache->objs[cache->len];
-
-		copied = 0;
-		while (copied < rs_thresh) {
-			const __m512i objs0 = _mm512_loadu_si512(&buffer[copied]);
-			const __m512i objs1 = _mm512_loadu_si512(&buffer[copied + 8]);
-			const __m512i objs2 = _mm512_loadu_si512(&buffer[copied + 16]);
-			const __m512i objs3 = _mm512_loadu_si512(&buffer[copied + 24]);
-
-			_mm512_storeu_si512(&cache_objs[copied], objs0);
-			_mm512_storeu_si512(&cache_objs[copied + 8], objs1);
-			_mm512_storeu_si512(&cache_objs[copied + 16], objs2);
-			_mm512_storeu_si512(&cache_objs[copied + 24], objs3);
-			copied += 32;
-		}
-		cache->len += rs_thresh;
-
-		if (cache->len >= cache->flushthresh) {
-			(void)rte_mempool_ops_enqueue_bulk(mp,
-					&cache->objs[cache->size], cache->len - cache->size);
-			cache->len = cache->size;
-		}
+		static_assert(sizeof(buffer[0]) == sizeof(struct rte_mbuf *),
+			"sxe2_tx_buffer_vec must be pointer-sized for bulk free cast");
+		rte_mbuf_raw_free_bulk(mp, (void *)buffer, rs_thresh);
 		goto done;
 	}
 
Signature · 1 line
2.43.0
#1 lore raw
Quoted 9 lines · On Thu, 20 Aug 2026 15:50:08 +0000 Kai Ji <[email protected]> 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 062d6fe5d0e4 ("net/intel: do not bypass mbuf lib for buffer fast-free") for the same
reason: it omits mbuf instrumentation (history marking) and contains dead flush code that accesses cache->objs[cache->size], which is one past the end of the array when cache_size == RTE_MEMPOOL_CACHE_MAX_SIZE.

Quoted 7 lines

Replace with a single rte_mbuf_raw_free_bulk() call, matching the Intel common library. The MBUF_FAST_FREE offload guarantee (single pool, refcnt == 1) makes this correct and the compiler inlines the bulk-free call to eliminate the overhead difference.

---

Looks good but still some leftovers to remove.

FAILED: drivers/net/sxe2/libsxe2_avx512_lib.a.p/sxe2_txrx_vec_avx512.c.o
gcc -Idrivers/net/sxe2/libsxe2_avx512_lib.a.p -Idrivers/net/sxe2 -I../drivers/net/sxe2 -Idrivers/common/sxe2 -I../drivers/common/sxe2 -Ilib/ethdev -I../lib/ethdev -Ilib/eal/common -I../lib/eal/common -I. -I.. -Iconfig -I../config -Ilib/eal/include -I../lib/eal/include -Ilib/eal/linux/include -I../lib/eal/linux/include -Ilib/eal/x86/include -I../lib/eal/x86/include -I../kernel/linux -Ilib/eal -I../lib/eal -Ilib/kvargs -I../lib/kvargs -Ilib/log -I../lib/log -Ilib/metrics -I../lib/metrics -Ilib/telemetry -I../lib/telemetry -Ilib/argparse -I../lib/argparse -Ilib/net -I../lib/net -Ilib/mbuf -I../lib/mbuf -Ilib/mempool -I../lib/mempool -Ilib/ring -I../lib/ring -Ilib/meter -I../lib/meter -Ilib/hash -I../lib/hash -Ilib/rcu -I../lib/rcu -Ilib/security -I../lib/security -Ilib/cryptodev -I../lib/cryptodev -Idrivers/bus/pci -I../drivers/bus/pci -I../drivers/bus/pci/linux -Ilib/pci -I../lib/pci -fdiagnostics-color=always -D_FILE_OFFSET_BITS=64 -Wall -Winvalid-pch -Wextra -Werror -std=c11 -O3 -include rte_config.h -Wvla -Wcast-qual -Wdeprecated -Wformat -Wformat-nonliteral -Wformat-security -Wmissing-declarations -Wmissing-prototypes -Wnested-externs -Wold-style-definition -Wpointer-arith -Wshadow -Wsign-compare -Wstrict-prototypes -Wundef -Wwrite-strings -Wno-packed-not-aligned -Wno-missing-field-initializers -D_GNU_SOURCE -fPIC -march=native -mrtm -DALLOW_EXPERIMENTAL_API -DALLOW_INTERNAL_API -Wno-format-truncation -g -DCC_AVX512_SUPPORT -mavx512f -mavx512bw -march=skylake-avx512 -MD -MQ drivers/net/sxe2/libsxe2_avx512_lib.a.p/sxe2_txrx_vec_avx512.c.o -MF drivers/net/sxe2/libsxe2_avx512_lib.a.p/sxe2_txrx_vec_avx512.c.o.d -o drivers/net/sxe2/libsxe2_avx512_lib.a.p/sxe2_txrx_vec_avx512.c.o -c ../drivers/net/sxe2/sxe2_txrx_vec_avx512.c ../drivers/net/sxe2/sxe2_txrx_vec_avx512.c: In function ‘sxe2_tx_bufs_free_vec_avx512’:
../drivers/net/sxe2/sxe2_txrx_vec_avx512.c:47:1: error: label ‘normal’ defined but not used [-Werror=unused-label]
normal:
^~~~~~
cc1: all warnings being treated as errors
[2434/3759] Generating drivers/rte_net_sxe2_ma

author
#2 lore raw

PATCH v2 1/1 net/sxe2: replace private mempool cache bypass with rte_mbuf_raw_free_bulk

In reply to Kai Ji

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 062d6fe5d0e4 ("net/intel: do not bypass mbuf lib for buffer fast-free") for the same
reason: it omits mbuf instrumentation (history marking) and contains dead flush code that accesses cache->objs[cache->size], which is one past the end of the array when cache_size == RTE_MEMPOOL_CACHE_MAX_SIZE.

Replace with a single rte_mbuf_raw_free_bulk() call, matching the Intel common library. The MBUF_FAST_FREE offload guarantee (single pool, refcnt == 1) makes this correct and the compiler inlines the bulk-free call to eliminate the overhead difference.

---
 drivers/net/sxe2/sxe2_txrx_vec_avx512.c | 38 ++-----------------------
 1 file changed, 3 insertions(+), 35 deletions(-)
drivers/net/sxe2/sxe2_txrx_vec_avx512.c +3 -35
index a830c7a33b..7311c9d35a 100644
@@ -18,9 +18,6 @@ static __rte_always_inline int32_t sxe2_tx_bufs_free_vec_avx512(struct sxe2_tx_q
 	struct rte_mbuf *mbuf;
 	struct rte_mbuf *mbuf_free_arr[SXE2_TX_FREE_BUFFER_SIZE_MAX_VEC];
 	struct rte_mempool *mp;
-	struct rte_mempool_cache *cache;
-	void **cache_objs;
-	uint32_t copied;
 	uint32_t i;
 	int32_t ret;
 	uint16_t rs_thresh;
@@ -41,41 +38,12 @@ static __rte_always_inline int32_t sxe2_tx_bufs_free_vec_avx512(struct sxe2_tx_q
 	if ((txq->offloads & RTE_ETH_TX_OFFLOAD_MBUF_FAST_FREE) &&
 			(rs_thresh & 31) == 0) {
 		mp = buffer[0].mbuf->pool;
-		cache = rte_mempool_default_cache(mp, rte_lcore_id());
-
-		if (cache == NULL || cache->len)
-			goto normal;
-
-		if (rs_thresh > RTE_MEMPOOL_CACHE_MAX_SIZE) {
-			(void)rte_mempool_ops_enqueue_bulk(mp, (void *)buffer, rs_thresh);
-			goto done;
-		}
-		cache_objs = &cache->objs[cache->len];
-
-		copied = 0;
-		while (copied < rs_thresh) {
-			const __m512i objs0 = _mm512_loadu_si512(&buffer[copied]);
-			const __m512i objs1 = _mm512_loadu_si512(&buffer[copied + 8]);
-			const __m512i objs2 = _mm512_loadu_si512(&buffer[copied + 16]);
-			const __m512i objs3 = _mm512_loadu_si512(&buffer[copied + 24]);
-
-			_mm512_storeu_si512(&cache_objs[copied], objs0);
-			_mm512_storeu_si512(&cache_objs[copied + 8], objs1);
-			_mm512_storeu_si512(&cache_objs[copied + 16], objs2);
-			_mm512_storeu_si512(&cache_objs[copied + 24], objs3);
-			copied += 32;
-		}
-		cache->len += rs_thresh;
-
-		if (cache->len >= cache->flushthresh) {
-			(void)rte_mempool_ops_enqueue_bulk(mp,
-					&cache->objs[cache->size], cache->len - cache->size);
-			cache->len = cache->size;
-		}
+		static_assert(sizeof(buffer[0]) == sizeof(struct rte_mbuf *),
+			"sxe2_tx_buffer_vec must be pointer-sized for bulk free cast");
+		rte_mbuf_raw_free_bulk(mp, (void *)buffer, rs_thresh);
 		goto done;
 	}
 
-normal:
 	mbuf = rte_pktmbuf_prefree_seg(buffer[0].mbuf);
 
 	if (likely(mbuf)) {
Signature · 1 line
2.43.0
author
#3 lore raw

PATCH v3 1/1 net/sxe2: replace private mempool cache bypass with rte_mbuf_raw_free_bulk

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")
---
 drivers/net/sxe2/sxe2_txrx_vec_avx512.c | 39 +++----------------------
 1 file changed, 4 insertions(+), 35 deletions(-)
drivers/net/sxe2/sxe2_txrx_vec_avx512.c +4 -35
index a830c7a33b..1a4ebd93c3 100644
@@ -12,15 +12,15 @@
 #include "sxe2_txrx_vec_common.h"
 #include "sxe2_vsi.h"
 
+static_assert(sizeof(struct sxe2_tx_buffer_vec) == sizeof(struct rte_mbuf *),
+	"sxe2_tx_buffer_vec must be pointer-sized for bulk free cast");
+
 static __rte_always_inline int32_t sxe2_tx_bufs_free_vec_avx512(struct sxe2_tx_queue *txq)
 {
 	struct sxe2_tx_buffer_vec *buffer;
 	struct rte_mbuf *mbuf;
 	struct rte_mbuf *mbuf_free_arr[SXE2_TX_FREE_BUFFER_SIZE_MAX_VEC];
 	struct rte_mempool *mp;
-	struct rte_mempool_cache *cache;
-	void **cache_objs;
-	uint32_t copied;
 	uint32_t i;
 	int32_t ret;
 	uint16_t rs_thresh;
@@ -41,41 +41,10 @@ static __rte_always_inline int32_t sxe2_tx_bufs_free_vec_avx512(struct sxe2_tx_q
 	if ((txq->offloads & RTE_ETH_TX_OFFLOAD_MBUF_FAST_FREE) &&
 			(rs_thresh & 31) == 0) {
 		mp = buffer[0].mbuf->pool;
-		cache = rte_mempool_default_cache(mp, rte_lcore_id());
-
-		if (cache == NULL || cache->len)
-			goto normal;
-
-		if (rs_thresh > RTE_MEMPOOL_CACHE_MAX_SIZE) {
-			(void)rte_mempool_ops_enqueue_bulk(mp, (void *)buffer, rs_thresh);
-			goto done;
-		}
-		cache_objs = &cache->objs[cache->len];
-
-		copied = 0;
-		while (copied < rs_thresh) {
-			const __m512i objs0 = _mm512_loadu_si512(&buffer[copied]);
-			const __m512i objs1 = _mm512_loadu_si512(&buffer[copied + 8]);
-			const __m512i objs2 = _mm512_loadu_si512(&buffer[copied + 16]);
-			const __m512i objs3 = _mm512_loadu_si512(&buffer[copied + 24]);
-
-			_mm512_storeu_si512(&cache_objs[copied], objs0);
-			_mm512_storeu_si512(&cache_objs[copied + 8], objs1);
-			_mm512_storeu_si512(&cache_objs[copied + 16], objs2);
-			_mm512_storeu_si512(&cache_objs[copied + 24], objs3);
-			copied += 32;
-		}
-		cache->len += rs_thresh;
-
-		if (cache->len >= cache->flushthresh) {
-			(void)rte_mempool_ops_enqueue_bulk(mp,
-					&cache->objs[cache->size], cache->len - cache->size);
-			cache->len = cache->size;
-		}
+		rte_mbuf_raw_free_bulk(mp, (void *)buffer, rs_thresh);
 		goto done;
 	}
 
-normal:
 	mbuf = rte_pktmbuf_prefree_seg(buffer[0].mbuf);
 
 	if (likely(mbuf)) {
Signature · 1 line
2.34.1
#4 lore raw
Quoted 19 lines · On Mon, 21 Sep 2026 15:52:13 +0000 Kai Ji <[email protected]> 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.

Quoted 5 lines
Fixes 0af0bdcdcf83 ("net/sxe2: add AVX512 Rx and Tx")

---

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: [email protected]". 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 "---".