From: bluhm <bluhm@openbsd.org>
Date: Tue, 23 Jan 2018 21:41:17 +0000 (+0000)
Subject: The TCP reaper timeout was still imlemented as soft timeout.  So
X-Git-Url: http://artulab.com/gitweb/?a=commitdiff_plain;h=9acddc59a51489e71fb2f560a3a0f0570a1ac983;p=openbsd

The TCP reaper timeout was still imlemented as soft timeout.  So
it could run immediately and was not synchronized with the TCP
timeouts, although that was the intension when it was introduced
in revision 1.85.  Convert the reaper to an ordinary TCP timeout
so it is scheduled on the same timeout thread after all timeouts
have finished.  A net lock is not necessary as the process calling
tcp_close() will not access the tcpcb after arming the reaper
timeout.
OK mikeb@
---

diff --git a/sys/netinet/tcp_subr.c b/sys/netinet/tcp_subr.c
index 67e8c1a80a1..9d3fbc950ce 100644
--- a/sys/netinet/tcp_subr.c
+++ b/sys/netinet/tcp_subr.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: tcp_subr.c,v 1.167 2017/12/07 16:52:21 mikeb Exp $	*/
+/*	$OpenBSD: tcp_subr.c,v 1.168 2018/01/23 21:41:17 bluhm Exp $	*/
 /*	$NetBSD: tcp_subr.c,v 1.22 1996/02/13 23:44:00 christos Exp $	*/
 
 /*
@@ -436,7 +436,6 @@ tcp_newtcpcb(struct inpcb *inp)
 	TCP_INIT_DELACK(tp);
 	for (i = 0; i < TCPT_NTIMERS; i++)
 		TCP_TIMER_INIT(tp, i);
-	timeout_set(&tp->t_reap_to, tcp_reaper, tp);
 
 	tp->sack_enable = tcp_do_sack;
 	tp->t_flags = tcp_do_rfc1323 ? (TF_REQ_SCALE|TF_REQ_TSTMP) : 0;
@@ -532,23 +531,14 @@ tcp_close(struct tcpcb *tp)
 	m_free(tp->t_template);
 
 	tp->t_flags |= TF_DEAD;
-	timeout_add(&tp->t_reap_to, 0);
+	TCP_TIMER_ARM(tp, TCPT_REAPER, 0);
 
-	inp->inp_ppcb = 0;
+	inp->inp_ppcb = NULL;
 	soisdisconnected(so);
 	in_pcbdetach(inp);
 	return (NULL);
 }
 
-void
-tcp_reaper(void *arg)
-{
-	struct tcpcb *tp = arg;
-
-	pool_put(&tcpcb_pool, tp);
-	tcpstat_inc(tcps_closed);
-}
-
 int
 tcp_freeq(struct tcpcb *tp)
 {
diff --git a/sys/netinet/tcp_timer.c b/sys/netinet/tcp_timer.c
index acacbe7dcf8..37641bde19e 100644
--- a/sys/netinet/tcp_timer.c
+++ b/sys/netinet/tcp_timer.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: tcp_timer.c,v 1.61 2018/01/23 21:06:47 bluhm Exp $	*/
+/*	$OpenBSD: tcp_timer.c,v 1.62 2018/01/23 21:41:17 bluhm Exp $	*/
 /*	$NetBSD: tcp_timer.c,v 1.14 1996/02/13 23:44:09 christos Exp $	*/
 
 /*
@@ -70,12 +70,14 @@ void	tcp_timer_rexmt(void *);
 void	tcp_timer_persist(void *);
 void	tcp_timer_keep(void *);
 void	tcp_timer_2msl(void *);
+void	tcp_timer_reaper(void *);
 
 const tcp_timer_func_t tcp_timer_funcs[TCPT_NTIMERS] = {
 	tcp_timer_rexmt,
 	tcp_timer_persist,
 	tcp_timer_keep,
 	tcp_timer_2msl,
+	tcp_timer_reaper,
 };
 
 /*
@@ -464,3 +466,20 @@ tcp_timer_2msl(void *arg)
  out:
 	NET_UNLOCK();
 }
+
+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 sufficinet 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);
+	tcpstat_inc(tcps_closed);
+}
diff --git a/sys/netinet/tcp_timer.h b/sys/netinet/tcp_timer.h
index 4396e1082b8..69910ecb54e 100644
--- a/sys/netinet/tcp_timer.h
+++ b/sys/netinet/tcp_timer.h
@@ -1,4 +1,4 @@
-/*	$OpenBSD: tcp_timer.h,v 1.14 2016/10/04 13:54:32 mpi Exp $	*/
+/*	$OpenBSD: tcp_timer.h,v 1.15 2018/01/23 21:41:17 bluhm Exp $	*/
 /*	$NetBSD: tcp_timer.h,v 1.6 1995/03/26 20:32:37 jtc Exp $	*/
 
 /*
@@ -39,12 +39,13 @@
  * Definitions of the TCP timers.  These timers are counted
  * down PR_SLOWHZ times a second.
  */
-#define	TCPT_NTIMERS	4
+#define	TCPT_NTIMERS	5
 
 #define	TCPT_REXMT	0		/* retransmit */
 #define	TCPT_PERSIST	1		/* retransmit persistence */
 #define	TCPT_KEEP	2		/* keep alive */
 #define	TCPT_2MSL	3		/* 2*msl quiet time timer */
+#define	TCPT_REAPER	4		/* delayed cleanup timeout */
 
 /*
  * The TCPT_REXMT timer is used to force retransmissions.
@@ -108,8 +109,8 @@
 #define	TCP_DELACK_TICKS (hz / PR_FASTHZ)	/* time to delay ACK */
 
 #ifdef	TCPTIMERS
-const char *tcptimers[] =
-    { "REXMT", "PERSIST", "KEEP", "2MSL" };
+const char *tcptimers[TCPT_NTIMERS] =
+    { "REXMT", "PERSIST", "KEEP", "2MSL", "REAPER" };
 #endif /* TCPTIMERS */
 
 /*
diff --git a/sys/netinet/tcp_var.h b/sys/netinet/tcp_var.h
index 71bdbb33506..15103b88792 100644
--- a/sys/netinet/tcp_var.h
+++ b/sys/netinet/tcp_var.h
@@ -1,4 +1,4 @@
-/*	$OpenBSD: tcp_var.h,v 1.128 2017/11/02 14:01:18 florian Exp $	*/
+/*	$OpenBSD: tcp_var.h,v 1.129 2018/01/23 21:41:17 bluhm Exp $	*/
 /*	$NetBSD: tcp_var.h,v 1.17 1996/02/13 23:44:24 christos Exp $	*/
 
 /*
@@ -193,8 +193,6 @@ struct tcpcb {
 	u_short	t_pmtud_ip_hl;		/* IP header length from ICMP payload */
 
 	int pf;
-
-	struct	timeout t_reap_to;	/* delayed cleanup timeout */
 };
 
 #define	intotcpcb(ip)	((struct tcpcb *)(ip)->inp_ppcb)
@@ -679,6 +677,7 @@ tcpstat_pkt(enum tcpstat_counters pcounter, enum tcpstat_counters bcounter,
 	counters_pkt(tcpcounters, pcounter, bcounter, v);
 }
 
+extern	struct pool tcpcb_pool;
 extern	struct inpcbtable tcbtable;	/* head of queue of active tcpcb's */
 extern	u_int32_t tcp_now;		/* for RFC 1323 timestamps */
 extern	int tcp_do_rfc1323;	/* enabled/disabled? */
@@ -705,7 +704,6 @@ extern	int tcp_syn_cache_active; /* active syn cache, may be 0 or 1 */
 void	 tcp_canceltimers(struct tcpcb *);
 struct tcpcb *
 	 tcp_close(struct tcpcb *);
-void	 tcp_reaper(void *);
 int	 tcp_freeq(struct tcpcb *);
 #ifdef INET6
 void	 tcp6_ctlinput(int, struct sockaddr *, u_int, void *);