Index | Thread | Search

From:
Vitaliy Makkoveev <mvs@openbsd.org>
Subject:
Re: bse: drop `if_timer' logic
To:
Mark Kettenis <mark.kettenis@xs4all.nl>
Cc:
tech@openbsd.org, kettenis@openbsd.org
Date:
Fri, 2 Jan 2026 17:22:01 +0000

Download raw body.

Thread
On Fri, Jan 02, 2026 at 06:06:53PM +0100, Mark Kettenis wrote:
> > Date: Fri, 2 Jan 2026 16:49:19 +0000
> > From: Vitaliy Makkoveev <mvs@openbsd.org>
> > 
> > We don't set `if_watchdog' so any logic around `if_timer' is useless.
> 
> Maybe we should set if_watchdog?
> 

I want to make genet_start mp-safe, but the existing `if_timer' logic
makes it useless because the if_timer should be protected with kernel
lock. If you want to implement watchdog it should be done at driver
level.

Also, I see no "bse stuck" reports and other BSDs also don't set it.
What is the problem you want to solve with this watchdog handler?

> > Index: sys/dev/ic/bcmgenet.c
> > ===================================================================
> > RCS file: /cvs/src/sys/dev/ic/bcmgenet.c,v
> > retrieving revision 1.8
> > diff -u -p -r1.8 bcmgenet.c
> > --- sys/dev/ic/bcmgenet.c	5 Nov 2024 18:58:59 -0000	1.8
> > +++ sys/dev/ic/bcmgenet.c	2 Jan 2026 16:35:45 -0000
> > @@ -656,7 +656,6 @@ genet_stop(struct genet_softc *sc)
> >  
> >  	ifp->if_flags &= ~IFF_RUNNING;
> >  	ifq_clr_oactive(&ifp->if_snd);
> > -	ifp->if_timer = 0;
> >  
> >  	intr_barrier(sc->sc_ih);
> >  
> > @@ -773,9 +772,6 @@ genet_txintr(struct genet_softc *sc, int
> >  		--sc->sc_tx.queued;
> >  	}
> >  
> > -	if (sc->sc_tx.queued == 0)
> > -		ifp->if_timer = 0;
> > -
> >  	if (sc->sc_tx.cidx != cidx) {
> >  		sc->sc_tx.next = i;
> >  		sc->sc_tx.cidx = cidx;
> > @@ -828,10 +824,8 @@ genet_start(struct ifnet *ifp)
> >  		cnt++;
> >  	}
> >  
> > -	if (cnt != 0) {
> > +	if (cnt != 0)
> >  		WR4(sc, GENET_TX_DMA_PROD_INDEX(qid), sc->sc_tx.pidx);
> > -		ifp->if_timer = 5;
> > -	}
> >  }
> >  
> >  int
> > 
> > 
>