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 67e8c1a..9d3fbc9 100644 (file)
--- 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 acacbe7..37641bd 100644 (file)
--- 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 4396e10..69910ec 100644 (file)
--- 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 71bdbb3..15103b8 100644 (file)
--- 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 *);