> On 3 Sep 2023, at 21:08, Alexander Bluhm <[email protected]> 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;
>
>
