From: Vitaliy Makkoveev <mvs@openbsd.org>
Subject: Re: fix tcp keepalive maxidle calculation
To: Alexander Bluhm <bluhm@openbsd.org>
Cc: tech@openbsd.org
Date: Wed, 17 Sep 2025 04:18:36 +0300

On Wed, Sep 17, 2025 at 12:41:53AM +0200, Alexander Bluhm wrote:
> On Fri, Sep 12, 2025 at 12:25:46AM +0200, Alexander Bluhm wrote:
> > Hi,
> > 
> > Here I messed up TCP keepalive variables keepidle and keepintvl.
> > 
> > ----------------------------
> > revision 1.82
> > date: 2025/01/16 11:59:20;  author: bluhm;  state: Exp;  lines: +17 -39;  commit
> > id: FhSoryD8VCfunQNr;
> > Remove net lock from TCP sysctl for keep alive.
> > 
> > Keep copies in seconds for the sysctl and update timer variables
> > atomically when they change.  tcp_maxidle was historically calculated
> > in tcp_slowtimo() as the timers were called from there.  Better
> > calculate maxidle when needed.  tcp_timer_init() is useless, just
> > initialize data.  While there make the names consistent.
> > 
> > input sthen@; OK mvs@
> > ----------------------------
> > 
> > Restore original behavior.
> > 
> > ok?
> 
> Anyone?
> 

ok mvs

> bluhm
> 
> Index: netinet/tcp_timer.c
> ===================================================================
> RCS file: /data/mirror/openbsd/cvs/src/sys/netinet/tcp_timer.c,v
> diff -u -p -r1.87 tcp_timer.c
> --- netinet/tcp_timer.c	8 Jul 2025 00:47:41 -0000	1.87
> +++ netinet/tcp_timer.c	16 Sep 2025 22:27:49 -0000
> @@ -472,11 +472,12 @@ tcp_timer_keep(void *arg)
>  	if ((atomic_load_int(&tcp_always_keepalive) ||
>  	    so->so_options & SO_KEEPALIVE) &&
>  	    tp->t_state <= TCPS_CLOSING) {
> -		int keepidle, maxidle;
> +		int keepidle, keepintvl, maxidle;
>  		uint64_t now;
>  
>  		keepidle = atomic_load_int(&tcp_keepidle);
> -		maxidle = TCPTV_KEEPCNT * keepidle;
> +		keepintvl = atomic_load_int(&tcp_keepintvl);
> +		maxidle = TCPTV_KEEPCNT * keepintvl;
>  		now = tcp_now();
>  		if ((maxidle > 0) &&
>  		    ((now - tp->t_rcvtime) >= keepidle + maxidle)) {
> @@ -499,7 +500,7 @@ tcp_timer_keep(void *arg)
>  		tcpstat_inc(tcps_keepprobe);
>  		tcp_respond(tp, mtod(tp->t_template, caddr_t),
>  		    NULL, tp->rcv_nxt, tp->snd_una - 1, 0, 0, now);
> -		TCP_TIMER_ARM(tp, TCPT_KEEP, atomic_load_int(&tcp_keepintvl));
> +		TCP_TIMER_ARM(tp, TCPT_KEEP, keepintvl);
>  	} else
>  		TCP_TIMER_ARM(tp, TCPT_KEEP, atomic_load_int(&tcp_keepidle));
>  	if (otp)
> @@ -516,7 +517,7 @@ tcp_timer_2msl(void *arg)
>  	struct tcpcb *otp = NULL, *tp;
>  	short ostate;
>  	uint64_t now;
> -	int maxidle;
> +	int keepintvl, maxidle;
>  
>  	if (tcp_timer_enter(inp, &so, &tp, TCPT_2MSL))
>  		goto out;
> @@ -527,11 +528,12 @@ tcp_timer_2msl(void *arg)
>  	}
>  	tcp_timer_freesack(tp);
>  
> -	maxidle = TCPTV_KEEPCNT * atomic_load_int(&tcp_keepidle);
> +	keepintvl = atomic_load_int(&tcp_keepintvl);
> +	maxidle = TCPTV_KEEPCNT * keepintvl;
>  	now = tcp_now();
>  	if (tp->t_state != TCPS_TIME_WAIT &&
>  	    ((maxidle == 0) || ((now - tp->t_rcvtime) <= maxidle)))
> -		TCP_TIMER_ARM(tp, TCPT_2MSL, atomic_load_int(&tcp_keepintvl));
> +		TCP_TIMER_ARM(tp, TCPT_2MSL, keepintvl);
>  	else
>  		tp = tcp_close(tp);
>  	if (otp)
>