Re: [PATCH v5 4/4] net/zxdh: optimize Tx xmit pkts performance

[Stephen Hemminger <stephen@networkplumber.org>]

On Mon, 15 Jun 2026 09:19:30 +0800
Junlong Wang <wang.junlong1@zte.com.cn> wrote:

> Add simple Tx xmit functions (zxdh_xmit_pkts_simple)
> for single-segment packet xmit.
> 
> Signed-off-by: Junlong Wang <wang.junlong1@zte.com.cn>
> ---

Ignore the CI AI review (9 false positives) but more detailed review found:

Series review: net/zxdh Rx/Tx optimization (v5)

The v4 issues are resolved: zxdh_queue_enable_intr() now tests
!= ENABLE and assigns ENABLE (1/4), and the fix is first in the
series against the original field name so it backports cleanly; the
simple Tx path publishes the AVAIL flag through
zxdh_queue_store_flags_packed() (rte_io_wmb); zxdh_recv_single_pkts()
now compacts surviving mbufs with rcv_pkts[nb_rx] = rxm; and a packet
that fails padding no longer has a descriptor written for it. One issue
remains.


[PATCH v5 4/4] net/zxdh: optimize Tx xmit pkts performance

Error: in submit_to_backend_simple(), when pkt_padding() fails the mbuf
is leaked and a hole is left in the packed ring.

	for (j = 0; j < N_PER_LOOP; ++j) {
		m  = *(tx_pkts + i + j);
		if (unlikely(pkt_padding(m, hw) < 0)) {
			vq->txq.stats.errors++;
			continue;
		}
		(dxp + i + j)->cookie = (void *)m;
		tx1(vq, txdp + i + j, m, id + i + j);
		zxdh_update_packet_stats(&vq->txq.stats, m);
	}

pkt_padding() returns -1 when rte_pktmbuf_prepend() finds less headroom
than dl_net_hdr_len. On that path the loop does continue, so m is never
freed and never enqueued, yet zxdh_xmit_pkts_simple() advances
vq_avail_idx / decrements vq_free_cnt by the full count and returns
nb_pkts. The application sees the packet as accepted and will not free
it -> mbuf leak.

The descriptor at txdp[id+i+j] is also left untouched, so its flags
keep the AVAIL bit from the previous ring pass while the surrounding
slots carry the current generation. The device consumes packed
descriptors in order and stops at the first slot whose AVAIL bit does
not match the current wrap, so it stalls at the hole; the
already-published packets behind it are never processed and their
cookies never reclaimed. A single padding failure can wedge the Tx
queue.

If pkt_padding() cannot fail in the simple path (single-segment, full
headroom guaranteed), the error handling is dead code and the branch
should be dropped. If it can fail, it must not desync the ring: free m
and stop the burst at the first failure, submitting only the
contiguous successfully-padded prefix and returning that count, rather
than skipping a slot mid-run.