Index | Thread | Search

From:
Vitaliy Makkoveev <mvs@openbsd.org>
Subject:
bse: do the TX ring space check before packet dequeue
To:
Jonathan Matthew <jonathan@d14n.org>, tech@openbsd.org
Date:
Sat, 17 Jan 2026 19:10:41 +0000

Download raw body.

Thread
Subj. jmatthew@ pointed this while doing review of my previous bse(4)
diff. This early check removes possible bus_dmamap_unload() and dequeue
rollback in the output path.

Also jmatthew@  pointed that the maximum TX segments count of 128 is too
big and proposed to limit limit it with 8. I checked others and found,
that NetBSD uses 128 as we do and FreeBSD uses 20, so I used 20. May be
this value is also too big, but it could be decreased at any moment.

I'm running this diff without any issue more than 24 hours.

Index: sys/dev/ic/bcmgenet.c
===================================================================
RCS file: /cvs/src/sys/dev/ic/bcmgenet.c,v
retrieving revision 1.11
diff -u -p -r1.11 bcmgenet.c
--- sys/dev/ic/bcmgenet.c	15 Jan 2026 03:12:49 -0000	1.11
+++ sys/dev/ic/bcmgenet.c	17 Jan 2026 13:18:42 -0000
@@ -70,7 +70,7 @@ CTASSERT(MCLBYTES == 2048);
 #define	TX_NEXT(n)		TX_SKIP(n, 1)
 #define	RX_NEXT(n)		(((n) + 1) & (GENET_DMA_DESC_COUNT - 1))
 
-#define	TX_MAX_SEGS		128
+#define	TX_MAX_SEGS		20
 #define	TX_DESC_COUNT		GENET_DMA_DESC_COUNT
 #define	RX_DESC_COUNT		GENET_DMA_DESC_COUNT
 #define	MII_BUSY_RETRY		1000
@@ -205,7 +205,7 @@ int
 genet_setup_txbuf(struct genet_softc *sc, int index, struct mbuf *m)
 {
 	bus_dma_segment_t *segs;
-	int error, queued, nsegs, cur, i;
+	int error, nsegs, cur, i;
 	uint32_t flags;
 
 	/*
@@ -232,13 +232,6 @@ genet_setup_txbuf(struct genet_softc *sc
 
 	segs = sc->sc_tx.buf_map[index].map->dm_segs;
 	nsegs = sc->sc_tx.buf_map[index].map->dm_nsegs;
-	queued = (sc->sc_tx.pidx - sc->sc_tx.cidx) & 0xffff;
-
-	if (queued >= GENET_DMA_DESC_COUNT - nsegs) {
-		bus_dmamap_unload(sc->sc_tx.buf_tag,
-		    sc->sc_tx.buf_map[index].map);
-		return -1;
-	}
 
 	flags = GENET_TX_DESC_STATUS_SOP |
 		GENET_TX_DESC_STATUS_CRC |
@@ -784,29 +777,27 @@ genet_qstart(struct ifqueue *ifq)
 	struct genet_softc *sc = ifp->if_softc;
 	struct mbuf *m;
 	const int qid = GENET_DMA_DEFAULT_QUEUE;
-	int nsegs, index, cnt;
+	int queued, nsegs, index, cnt;
 
 	index = sc->sc_tx.pidx & (TX_DESC_COUNT - 1);
 	cnt = 0;
 
 	for (;;) {
-		m = ifq_deq_begin(&ifp->if_snd);
+		queued = (sc->sc_tx.pidx - sc->sc_tx.cidx) & 0xffff;
+		if (queued >= TX_DESC_COUNT - TX_MAX_SEGS)
+			break;
+
+		m = ifq_dequeue(&ifp->if_snd);
 		if (m == NULL)
 			break;
 
 		nsegs = genet_setup_txbuf(sc, index, m);
-		if (nsegs == -1) {
-			ifq_deq_rollback(&ifp->if_snd, m);
-			ifq_set_oactive(&ifp->if_snd);
-			break;
-		}
 		if (nsegs == 0) {
-			ifq_deq_commit(&ifp->if_snd, m);
 			m_freem(m);
 			ifp->if_oerrors++;
 			continue;
 		}
-		ifq_deq_commit(&ifp->if_snd, m);
+
 		if (ifp->if_bpf)
 			bpf_mtap(ifp->if_bpf, m, BPF_DIRECTION_OUT);