From: Vitaliy Makkoveev <mvs@openbsd.org>
Subject: Re: bse: do the TX ring space check before packet dequeue
To: Jonathan Matthew <jonathan@d14n.org>, tech@openbsd.org
Date: Wed, 21 Jan 2026 12:30:00 +0000

On Sat, Jan 17, 2026 at 07:10:41PM +0000, Vitaliy Makkoveev wrote:
> 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.
> 

No issues after 5 days.

> 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);
>