> On 3 Sep 2023, at 21:08, Alexander Bluhm <alexander.bl...@gmx.net> wrote: > > Hi, > > Avoid a useless increment and decrement of of the tcp syn cache > refcount by unexpanding the SYN_CACHE_TIMER_ARM() macro in the timer > callback. > > ok? >
ok mvs > bluhm > > Index: netinet/tcp_input.c > =================================================================== > RCS file: /data/mirror/openbsd/cvs/src/sys/netinet/tcp_input.c,v > retrieving revision 1.390 > diff -u -p -r1.390 tcp_input.c > --- netinet/tcp_input.c 28 Aug 2023 14:50:01 -0000 1.390 > +++ netinet/tcp_input.c 3 Sep 2023 18:04:13 -0000 > @@ -3159,19 +3159,6 @@ syn_cache_put(struct syn_cache *sc) > pool_put(&syn_cache_pool, sc); > } > > -/* > - * We don't estimate RTT with SYNs, so each packet starts with the default > - * RTT and each timer step has a fixed timeout value. > - */ > -#define SYN_CACHE_TIMER_ARM(sc) > \ > -do { \ > - TCPT_RANGESET((sc)->sc_rxtcur, \ > - TCPTV_SRTTDFLT * tcp_backoff[(sc)->sc_rxtshift], TCPTV_MIN, \ > - TCPTV_REXMTMAX); \ > - if (timeout_add_msec(&(sc)->sc_timer, (sc)->sc_rxtcur)) \ > - refcnt_take(&(sc)->sc_refcnt); \ > -} while (/*CONSTCOND*/0) > - > void > syn_cache_init(void) > { > @@ -3300,11 +3287,17 @@ syn_cache_insert(struct syn_cache *sc, s > } > > /* > - * Initialize the entry's timer. > + * Initialize the entry's timer. We don't estimate RTT > + * with SYNs, so each packet starts with the default RTT > + * and each timer step has a fixed timeout value. > */ > sc->sc_rxttot = 0; > sc->sc_rxtshift = 0; > - SYN_CACHE_TIMER_ARM(sc); > + TCPT_RANGESET(sc->sc_rxtcur, > + TCPTV_SRTTDFLT * tcp_backoff[sc->sc_rxtshift], TCPTV_MIN, > + TCPTV_REXMTMAX); > + if (timeout_add_msec(&sc->sc_timer, sc->sc_rxtcur)) > + refcnt_take(&sc->sc_refcnt); > > /* Link it from tcpcb entry */ > refcnt_take(&sc->sc_refcnt); > @@ -3365,15 +3358,12 @@ syn_cache_timer(void *arg) > > /* Advance the timer back-off. */ > sc->sc_rxtshift++; > - SYN_CACHE_TIMER_ARM(sc); > + TCPT_RANGESET(sc->sc_rxtcur, > + TCPTV_SRTTDFLT * tcp_backoff[sc->sc_rxtshift], TCPTV_MIN, > + TCPTV_REXMTMAX); > + if (!timeout_add_msec(&sc->sc_timer, sc->sc_rxtcur)) > + syn_cache_put(sc); > > - /* > - * Decrement reference of this timer. We know there is another timer > - * as we just added it. So just deref, free is not necessary. > - */ > - lastref = refcnt_rele(&sc->sc_refcnt); > - KASSERT(lastref == 0); > - (void)lastref; > NET_UNLOCK(); > return; > >