On Tue, 25 Aug 2026 20:00:02 +0800
Junlong Wang <[email protected]> wrote:
> v12:
> - Patch 3/4 no longer introduces the unused `uint64_t offloads` field
> in `struct zxdh_virtnet_rx`, and patch 4/4's commit log drops the
> stale "drop offloads" claim so the message matches the actual diff.
>
> v11:
> - Restructure queue types for cache locality and dead-code removal (patch
> 2/4),
> optimize the packed-ring Rx path with a single-segment fast path,
> MTU/scatter refactor, and xstats cleanup (patch 3/4), and rework the
> packed-ring Tx flush with per-descriptor mbuf free, walk budget, and
> masked prefetch index (patch 4/4).
>
> v10:
> - Based on the issues raised in the AI review, Patch 2/4 Patch 3/4 Patch
> 4/4 have been modified.
>
> v9:
> - Remove add simple Tx xmit functions (zxdh_xmit_pkts_simple) in the last
> patch.
>
> v8:
> - Add checked the size of ZXDH_DL_NET_HDR_SIZE and RTE_PKTMBUF_HEADROOM in
> zxdh_xmit_pkts_simple() before submitting. Add static_assert to reject
> builds with insufficient
> default headroom at compile time.
>
> v7:
> - Add a new xmit prepare func for xmit_pkts_simple, which will checked the
> size of
> ZXDH_DL_NET_HDR_SIZE and RTE_PKTMBUF_HEADROOM.
>
> v6:
> - Remove unnecessary error checking code in submit_to_backend_simple() and
> pkt_padding(). Since as the max dl_net_hdr_len is always less than
> RTE_PKTMBUF_HEADROOM, rte_pktmbuf_prepend() cannot fail in the
> simple path (single-segment mbufs).
>
> v5:
> - Reorganize patch series, placing interrupt fix as the first patch
> and fix condition check to properly enable interrupts.
> - Fix zxdh_recv_single_pkts() not compacting rcv_pkts[] on failure,
> which could cause use-after-free and mbuf leak.
> - Fix tx_bunch() and tx1() missing store barrier before setting AVAIL flag,
> preventing data race on weakly-ordered architectures.
> - Fix submit_to_backend_simple() writing descriptors for packets that
> failed pkt_padding(), causing mbuf leak.
>
> v4:
> - fix some AI review issues.
> - fix queue enable intr bug.
>
> v3:
> - remove unnecessary NULL check in zxdh_init_queue.
> - Split Ring: Bit[31] is unused and reserved, zxdh_queue_notify(): removing
> the
> zxdh_pci_with_feature(hw, ZXDH_F_RING_PACKED) check;
> - remove unnecessary double-free in in zxdh_recv_single_pkts();
> - used rte_pktmbuf_mtod();
> - remove rxq_get_vq(q) macro, use q->vq and apply it consistently;
> - Refactoring scatter and mtu check logic in zxdh_dev_mtu_set();
> - set txdp->id = avail_idx + i in tx_bunch/tx1.
> - add comment documenting zxdh_xmit_enqueue_append() now sets dxp->cookie =
> NULL for
> the head slot and stores cookies per descriptor via dep[idx].cookie.
> - add one-line comment noting tx_bunch() is the simple path handles
> single-segment.
> - remove unnecessary Extra initialization and the uint32_t cast.
>
> v2:
> - zxdh_rxtx.c, pkt_padding(): modifyed the return value of pkt_padding();
> - zxdh_rxtx.c, zxdh_recv_single_pkts(): modifyed When zxdh_init_mbuf() fails
> the loop does "continue" and free mbufs;
> - zxdh_rxtx.c, refill_desc_unwrap(): Add rte_io_wmb() before writing flags
> in the refill_que_descs();
> - zxdh_queue.h, zxdh_queue_enable_intr(): Remove unnecessary function of
> zxdh_queue_enable_intr;
> - zxdh_ethdev.c, zxdh_init_queue(): changed the hdr_mz NULL check logic;
> - zxdh_rxtx.c, zxdh_xmit_pkts_simple()、zxdh_recv_single_pkts(): add
> stats.bytes count;
> - zxdh_rxtx.c, zxdh_init_mbuf():remove rte_pktmbuf_dump(stdout, rxm, 40);
> - zxdh_ethdev.c, zxdh_dev_free_mbufs(): using rte_pktmbuf_free() to free
> mbufs;
> - Splitting into separate patches, structure reorganization and sw_ring
> removal、
> RX recv optimize、Tx xmit optimize、Tx;
>
> v1:
> This patch optimizes the ZXDH PMD's receive and transmit path for better
> performance through several improvements:
> - Add simple TX/RX burst functions (zxdh_xmit_pkts_simple and
> zxdh_recv_single_pkts) for single-segment packet scenarios.
> - Remove RX software ring (sw_ring) to reduce memory allocation and
> copy.
> - Optimize descriptor management with prefetching and simplified
> cleanup.
> - Reorganize structure fields for better cache locality.
>
> These changes reduce CPU cycles and memory bandwidth consumption,
> resulting in improved packet processing throughput.
>
> Junlong Wang (4):
> net/zxdh: fix queue enable intr issues
> net/zxdh: optimize queue structure to improve performance
> net/zxdh: optimize Rx recv pkts performance
> net/zxdh: optimize Tx xmit pkts performance
>
> doc/guides/rel_notes/release_26_11.rst | 10 +
> drivers/net/zxdh/zxdh_ethdev.c | 85 ++++---
> drivers/net/zxdh/zxdh_ethdev_ops.c | 22 +-
> drivers/net/zxdh/zxdh_ethdev_ops.h | 7 +
> drivers/net/zxdh/zxdh_pci.c | 21 --
> drivers/net/zxdh/zxdh_pci.h | 1 -
> drivers/net/zxdh/zxdh_queue.c | 11 +-
> drivers/net/zxdh/zxdh_queue.h | 134 ++++------
> drivers/net/zxdh/zxdh_rxtx.c | 323 +++++++++++++++----------
> drivers/net/zxdh/zxdh_rxtx.h | 14 +-
> 10 files changed, 326 insertions(+), 302 deletions(-)
>
Still get some detailed AI issues..
You don't need to fix every detail and AI does tend to "bikeshed"
Review of [PATCH v12 0/4] net/zxdh: optimize queue/Rx/Tx paths
Resolved since v11
------------------
- Patch 4: the unused "uint64_t offloads" field left behind in
struct zxdh_virtnet_rx is gone; zxdh_rxtx.h is now updated in the
same patch that stops using the removed members.
- Patch 4: the descriptor walk in the Tx flush is bounded (budget
initialised to vq_nentries, break on id >= size) and the prefetch
index is masked with (size - 1).
- Patch 3: zxdh_dev_mtu_set() and zxdh_scattered_rx() now use the
same predicate (ZXDH_MTU_TO_PKTLEN vs min_rx_buf_size minus
headroom), so the two no longer diverge.
- Patch 1: Fixes: 7677f3871ef3 resolves to a real commit whose
subject matches, and the tag uses a 12-character hash.
Warnings
--------
Patch 3/4 (net/zxdh: optimize Rx recv pkts performance)
1. drivers/net/zxdh/zxdh_ethdev.c, zxdh_set_rxtx_funcs()
The ZXDH_NET_F_MRG_RXBUF runtime check is removed and replaced
with a comment stating the feature "is always negotiated (set in
both ZXDH_PMD_DEFAULT_GUEST_FEATURES and
ZXDH_PMD_DEFAULT_HOST_FEATURES)". That premise does not hold on
every path. In zxdh_get_pci_dev_config():
hw->host_features = ZXDH_PMD_DEFAULT_HOST_FEATURES;
if (hw->switchoffload)
hw->host_features = zxdh_pci_get_features(hw);
nego_features = guest_features & hw->host_features;
When switchoffload is set, host_features comes from the device,
so ZXDH_NET_F_MRG_RXBUF can be absent from the negotiated set.
Both zxdh_recv_pkts_packed() and zxdh_init_mbuf() then read
header->type_hdr.num_buffers for a device that never agreed to
populate it.
Note the old check was already ineffective in a different way:
the caller in dev_start ignores the int32_t return, so the -1
only left rx_pkt_burst/tx_pkt_burst unassigned. Either keep the
check and make dev_start actually fail on it, or narrow the
comment to the non-switchoffload case.
2. doc/guides/rel_notes/release_26_11.rst
The release note enumerates the individual counters removed
("full", "norefill", "multicast_packets", "broadcast_packets").
That level of detail does not belong in the release notes and
dates badly as the counter set keeps changing. A single bullet
along the lines of
* Changed the set of per-queue xstats counters.
covers it. The "New Features" bullets for the fast Rx path and
the packed-ring optimisation are fine as they are.
Patch 4/4 (net/zxdh: optimize Tx xmit pkts performance)
3. drivers/net/zxdh/zxdh_rxtx.c, zxdh_xmit_fast_flush()
The chain-walk scaffolding is now dead code. Both enqueue paths
write the descriptor's own index into the id field
(zxdh_xmit_enqueue_push: dp->id = id where id == vq_avail_idx;
zxdh_xmit_enqueue_append: start_dp[idx].id = idx), and the commit
message states the flush relies on desc[k].id == k being
preserved. Given that invariant, "id" always equals "used_idx",
so "curr_id != id" is false on the first pass and the do/while
body executes exactly once per outer iteration.
The result is a loop that simultaneously assumes id == index and
retains the machinery for the case where it is not, which makes
the bound reasoning harder to follow than it needs to be.
Suggest dropping id, curr_id and the inner do/while and walking a
single descriptor per outer iteration, keeping the budget counter
and the id >= size guard as the corruption backstop:
while (budget-- > 0 && desc_is_used(&desc[used_idx], vq)) {
rte_prefetch0(&desc[(used_idx +
NEXT_CACHELINE_OFF_16B) & (size - 1)]);
if (unlikely(desc[used_idx].id >= size))
break;
dxp = &vq->vq_descx[used_idx];
...
}
Info
----
Patch 3/4
4. drivers/net/zxdh/zxdh_rxtx.c, zxdh_update_packet_stats()
The patch removes stats.multicast and stats.broadcast from
struct zxdh_virtnet_stats, but the QUEUE_XSTAT block still
references them (and an undeclared "ea"). The block was already
uncompilable before this series, so nothing regresses, but since
the series is cleaning out unused counters this is a good moment
to delete the whole #ifdef QUEUE_XSTAT body. The size_bins[]
entries left in zxdh_rxq_stat_strings[]/zxdh_txq_stat_strings[]
are always zero for the same reason and are candidates for the
same cleanup.
5. drivers/net/zxdh/zxdh_rxtx.c, refill path
zxdh_queue_notify() is called whenever vq_free_cnt > 0, including
when rte_pktmbuf_alloc_bulk() inside zxdh_refill_que_descs()
failed and no descriptor was made available. Harmless, but it
costs an MMIO write on the mempool-exhausted path. Having
zxdh_refill_que_descs() return the number refilled and notifying
only on a non-zero result would avoid it.
Patch 4/4
6. drivers/net/zxdh/zxdh_rxtx.c
NEXT_CACHELINE_OFF_16B is the only macro in the file without the
ZXDH_ prefix used everywhere else in the driver. There are also
two consecutive blank lines before it.
7. drivers/net/zxdh/zxdh_rxtx.c, zxdh_xmit_enqueue_append()
The comment naming the expected readers of head cookies lists
zxdh_queue_rxvq_flush(), which only ever walks Rx virtqueues and
never sees a Tx head descriptor. zxdh_queue_detach_unused() is
the relevant one.
8. drivers/net/zxdh/zxdh_queue.h
struct zxdh_vq_desc_extra's "ndescs" is renamed to "rsv" rather
than removed. Nothing reads it after this patch, and "next" is
only written by zxdh_vring_desc_init_packed() and never read, so
the struct could be reduced to just the cookie pointer if you
want the whole array to shrink.
