From: Alexander Bluhm <bluhm@openbsd.org>
Subject: Re: fix tcp keepalive maxidle calculation
To: tech@openbsd.org
Date: Wed, 17 Sep 2025 00:41:53 +0200

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?

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)