From: Alexander Bluhm <bluhm@openbsd.org>
Subject: remove TCP timeout reaper
To: tech@openbsd.org
Date: Fri, 6 Jun 2025 21:20:26 +0200

Hi,

I think the TCP timeout reaper is no longer necessary.  Idea was
to prevent that timeouts are using TCP sockets that were already
closed.

But now tcp_close() runs with socket lock and tcp_timer_enter()
checks that intotcpcb(inp) is not NULL while holding the socket
lock.  So timeout cannot run if TCP has been closed.

ok?

bluhm

Index: netinet/in_pcb.h
===================================================================
RCS file: /data/mirror/openbsd/cvs/src/sys/netinet/in_pcb.h,v
diff -u -p -r1.169 in_pcb.h
--- netinet/in_pcb.h	3 Jun 2025 16:51:26 -0000	1.169
+++ netinet/in_pcb.h	6 Jun 2025 19:05:08 -0000
@@ -138,7 +138,7 @@ struct inpcb {
 	u_int16_t inp_fport;		/* [t] foreign port */
 	u_int16_t inp_lport;		/* [t] local port */
 	struct	  socket *inp_socket;	/* [I] back pointer to socket */
-	caddr_t	  inp_ppcb;		/* pointer to per-protocol pcb */
+	caddr_t	  inp_ppcb;		/* [s] pointer to per-protocol pcb */
 	struct    route inp_route;	/* [s] cached route */
 	struct    refcnt inp_refcnt;	/* refcount PCB, delay memory free */
 	int	  inp_flags;		/* generic IP/datagram flags */
Index: netinet/tcp_subr.c
===================================================================
RCS file: /data/mirror/openbsd/cvs/src/sys/netinet/tcp_subr.c,v
diff -u -p -r1.211 tcp_subr.c
--- netinet/tcp_subr.c	3 Jun 2025 16:51:26 -0000	1.211
+++ netinet/tcp_subr.c	6 Jun 2025 19:05:08 -0000
@@ -440,8 +440,6 @@ tcp_newtcpcb(struct inpcb *inp, int wait
 	tp->t_inpcb = inp;
 	for (i = 0; i < TCPT_NTIMERS; i++)
 		TCP_TIMER_INIT(tp, i);
-	timeout_set_flags(&tp->t_timer_reaper, tcp_timer_reaper, tp,
-	    KCLOCK_NONE, TIMEOUT_PROC | TIMEOUT_MPSAFE);
 
 	tp->sack_enable = atomic_load_int(&tcp_do_sack);
 	tp->t_flags = atomic_load_int(&tcp_do_rfc1323) ?
@@ -528,9 +526,8 @@ tcp_close(struct tcpcb *tp)
 	}
 
 	m_free(tp->t_template);
-	/* Free tcpcb after all pending timers have been run. */
-	timeout_add(&tp->t_timer_reaper, 0);
 	inp->inp_ppcb = NULL;
+	pool_put(&tcpcb_pool, tp);
 	soisdisconnected(so);
 	in_pcbdetach(inp);
 	tcpstat_inc(tcps_closed);
Index: netinet/tcp_timer.c
===================================================================
RCS file: /data/mirror/openbsd/cvs/src/sys/netinet/tcp_timer.c,v
diff -u -p -r1.84 tcp_timer.c
--- netinet/tcp_timer.c	3 Jun 2025 16:51:26 -0000	1.84
+++ netinet/tcp_timer.c	6 Jun 2025 19:05:08 -0000
@@ -541,19 +541,3 @@ tcp_timer_2msl(void *arg)
  out:
 	tcp_timer_leave(inp, so);
 }
-
-void
-tcp_timer_reaper(void *arg)
-{
-	struct tcpcb *tp = arg;
-
-	/*
-	 * This timer is necessary to delay the pool_put() after all timers
-	 * have finished, even if they were sleeping to grab the net lock.
-	 * Putting the pool_put() in a timer is sufficient as all timers run
-	 * from the same timeout thread.  Note that neither softnet thread nor
-	 * user process may access the tcpcb after arming the reaper timer.
-	 * Freeing may run in parallel as it does not grab the net lock.
-	 */
-	pool_put(&tcpcb_pool, tp);
-}
Index: netinet/tcp_timer.h
===================================================================
RCS file: /data/mirror/openbsd/cvs/src/sys/netinet/tcp_timer.h,v
diff -u -p -r1.26 tcp_timer.h
--- netinet/tcp_timer.h	16 Jan 2025 11:59:20 -0000	1.26
+++ netinet/tcp_timer.h	6 Jun 2025 19:05:08 -0000
@@ -164,8 +164,5 @@ extern int tcp_keepidle_sec;	/* [a] copy
 extern int tcp_keepintvl_sec;	/* [a] copy of above in seconds for sysctl */
 extern int tcp_ttl;		/* time to live for TCP segs */
 extern const int tcp_backoff[];
-
-void	tcp_timer_reaper(void *);
-
 #endif /* _KERNEL */
 #endif /* _NETINET_TCP_TIMER_H_ */
Index: netinet/tcp_var.h
===================================================================
RCS file: /data/mirror/openbsd/cvs/src/sys/netinet/tcp_var.h,v
diff -u -p -r1.191 tcp_var.h
--- netinet/tcp_var.h	7 May 2025 14:10:19 -0000	1.191
+++ netinet/tcp_var.h	6 Jun 2025 19:05:08 -0000
@@ -70,7 +70,6 @@ struct tcpqent {
 struct tcpcb {
 	struct tcpqehead t_segq;		/* sequencing queue */
 	struct timeout t_timer[TCPT_NTIMERS];	/* tcp timers */
-	struct timeout t_timer_reaper;	/* reaper is special, no refcnt */
 	short	t_state;		/* state of this connection */
 	short	t_rxtshift;		/* log(2) of rexmt exp. backoff */
 	int	t_rxtcur;		/* current retransmit value */