Add a state to sanity check tcp_close() to make sure it is not called
authorMatthew Dillon <dillon@dragonflybsd.org>
Wed, 11 Aug 2004 02:36:22 +0000 (02:36 +0000)
committerMatthew Dillon <dillon@dragonflybsd.org>
Wed, 11 Aug 2004 02:36:22 +0000 (02:36 +0000)
twice.

Add a 'cpu' field to the inpcb so the cpu owning a pcb can be made
well-known, for use in later assertions as we move closer to removing
the BGL.

Fix a bug in the closing of listen sockets.  The inp wildcard hash table
removal was being done asynchronously with the freeing of the inp, which
could lead to problems.  Instead of sending messages in parallel to all tcp
protocol threads to remove the wildcard hash we instead chain a single
message through all tcp protocol threads to remove the hash, then detach the
inp at the end of the chain.

There is still an issue with the socket being ripped out from under other
protocol threads which might be trying to accept connections on behalf of
the listen socket which must be resolved before the BGL can be removed (amoung
other things).

sys/netinet/in_pcb.c
sys/netinet/in_pcb.h
sys/netinet/tcp_fsm.h
sys/netinet/tcp_input.c
sys/netinet/tcp_subr.c
sys/netinet/tcp_usrreq.c

index 3a2ef4b..3826b34 100644 (file)
@@ -82,7 +82,7 @@
  *
  *     @(#)in_pcb.c    8.4 (Berkeley) 5/24/95
  * $FreeBSD: src/sys/netinet/in_pcb.c,v 1.59.2.27 2004/01/02 04:06:42 ambrisko Exp $
- * $DragonFly: src/sys/netinet/in_pcb.c,v 1.24 2004/07/08 22:07:35 hsu Exp $
+ * $DragonFly: src/sys/netinet/in_pcb.c,v 1.25 2004/08/11 02:36:22 dillon Exp $
  */
 
 #include "opt_ipsec.h"
@@ -202,12 +202,16 @@ SYSCTL_INT(_net_inet_ip_portrange, OID_AUTO, randomized, CTLFLAG_RW,
  * NOTE: It is assumed that most of these functions will be called at
  * splnet(). XXX - There are, unfortunately, a few exceptions to this
  * rule that should be fixed.
+ *
+ * NOTE: The caller should initialize the cpu field to the cpu running the
+ * protocol stack associated with this inpcbinfo.
  */
 
 void
 in_pcbinfo_init(struct inpcbinfo *pcbinfo)
 {
        LIST_INIT(&pcbinfo->pcblisthead);
+       pcbinfo->cpu = -1;
 }
 
 /*
index 447b3aa..6c8abf4 100644 (file)
@@ -82,7 +82,7 @@
  *
  *     @(#)in_pcb.h    8.1 (Berkeley) 6/10/93
  * $FreeBSD: src/sys/netinet/in_pcb.h,v 1.32.2.7 2003/01/24 05:11:34 sam Exp $
- * $DragonFly: src/sys/netinet/in_pcb.h,v 1.15 2004/07/08 22:07:35 hsu Exp $
+ * $DragonFly: src/sys/netinet/in_pcb.h,v 1.16 2004/08/11 02:36:22 dillon Exp $
  */
 
 #ifndef _NETINET_IN_PCB_H_
@@ -305,6 +305,7 @@ struct inpcbinfo {          /* XXX documentation, prefixes */
        struct  vm_zone *ipi_zone; /* zone to allocate pcbs from */
        u_int   ipi_count;      /* number of pcbs in this list */
        u_quad_t ipi_gencnt;    /* current generation count */
+       int     cpu;            /* related protocol thread cpu or -1 */
 };
 
 
index d559784..6664c90 100644 (file)
@@ -32,7 +32,7 @@
  *
  *     @(#)tcp_fsm.h   8.1 (Berkeley) 6/10/93
  * $FreeBSD: src/sys/netinet/tcp_fsm.h,v 1.14 1999/11/07 04:18:30 jlemon Exp $
- * $DragonFly: src/sys/netinet/tcp_fsm.h,v 1.2 2003/06/17 04:28:51 dillon Exp $
+ * $DragonFly: src/sys/netinet/tcp_fsm.h,v 1.3 2004/08/11 02:36:22 dillon Exp $
  */
 
 #ifndef _NETINET_TCP_FSM_H_
  * Per RFC793, September, 1981.
  */
 
-#define        TCP_NSTATES     11
+#define        TCP_NSTATES     12
 
-#define        TCPS_CLOSED             0       /* closed */
-#define        TCPS_LISTEN             1       /* listening for connection */
-#define        TCPS_SYN_SENT           2       /* active, have sent syn */
-#define        TCPS_SYN_RECEIVED       3       /* have send and received syn */
+#define TCPS_TERMINATING       0
+#define        TCPS_CLOSED             1       /* closed */
+#define        TCPS_LISTEN             2       /* listening for connection */
+#define        TCPS_SYN_SENT           3       /* active, have sent syn */
+#define        TCPS_SYN_RECEIVED       4       /* have send and received syn */
 /* states < TCPS_ESTABLISHED are those where connections not established */
-#define        TCPS_ESTABLISHED        4       /* established */
-#define        TCPS_CLOSE_WAIT         5       /* rcvd fin, waiting for close */
+#define        TCPS_ESTABLISHED        5       /* established */
+#define        TCPS_CLOSE_WAIT         6       /* rcvd fin, waiting for close */
 /* states > TCPS_CLOSE_WAIT are those where user has closed */
-#define        TCPS_FIN_WAIT_1         6       /* have closed, sent fin */
-#define        TCPS_CLOSING            7       /* closed xchd FIN; await FIN ACK */
-#define        TCPS_LAST_ACK           8       /* had fin and close; await FIN ACK */
+#define        TCPS_FIN_WAIT_1         7       /* have closed, sent fin */
+#define        TCPS_CLOSING            8       /* closed xchd FIN; await FIN ACK */
+#define        TCPS_LAST_ACK           9       /* had fin and close; await FIN ACK */
 /* states > TCPS_CLOSE_WAIT && < TCPS_FIN_WAIT_2 await ACK of FIN */
-#define        TCPS_FIN_WAIT_2               /* have closed, fin is acked */
-#define        TCPS_TIME_WAIT          10      /* in 2*msl quiet wait after close */
+#define        TCPS_FIN_WAIT_2         10      /* have closed, fin is acked */
+#define        TCPS_TIME_WAIT          11      /* in 2*msl quiet wait after close */
 
 /* for KAME src sync over BSD*'s */
 #define        TCP6_NSTATES            TCP_NSTATES
  * if all data queued for output is included in the segment.
  */
 static u_char  tcp_outflags[TCP_NSTATES] = {
-       TH_RST|TH_ACK,          /* 0, CLOSED */
-       0,                      /* 1, LISTEN */
-       TH_SYN,                 /* 2, SYN_SENT */
-       TH_SYN|TH_ACK,          /* 3, SYN_RECEIVED */
-       TH_ACK,                 /* 4, ESTABLISHED */
-       TH_ACK,                 /* 5, CLOSE_WAIT */
-       TH_FIN|TH_ACK,          /* 6, FIN_WAIT_1 */
-       TH_FIN|TH_ACK,          /* 7, CLOSING */
-       TH_FIN|TH_ACK,          /* 8, LAST_ACK */
-       TH_ACK,                 /* 9, FIN_WAIT_2 */
-       TH_ACK,                 /* 10, TIME_WAIT */
+       TH_RST|TH_ACK,          /* 0, TERMINATING */
+       TH_RST|TH_ACK,          /* 1, CLOSED */
+       0,                      /* 2, LISTEN */
+       TH_SYN,                 /* 3, SYN_SENT */
+       TH_SYN|TH_ACK,          /* 4, SYN_RECEIVED */
+       TH_ACK,                 /* 5, ESTABLISHED */
+       TH_ACK,                 /* 6, CLOSE_WAIT */
+       TH_FIN|TH_ACK,          /* 7, FIN_WAIT_1 */
+       TH_FIN|TH_ACK,          /* 8, CLOSING */
+       TH_FIN|TH_ACK,          /* 9, LAST_ACK */
+       TH_ACK,                 /* 10, FIN_WAIT_2 */
+       TH_ACK,                 /* 11, TIME_WAIT */
 };     
 #endif
 
@@ -106,6 +108,7 @@ int tcp_acounts[TCP_NSTATES][PRU_NREQ];
 
 #ifdef TCPSTATES
 char *tcpstates[] = {
+       "TERMINATING",
        "CLOSED",       "LISTEN",       "SYN_SENT",     "SYN_RCVD",
        "ESTABLISHED",  "CLOSE_WAIT",   "FIN_WAIT_1",   "CLOSING",
        "LAST_ACK",     "FIN_WAIT_2",   "TIME_WAIT",
index 35bc11a..796abf9 100644 (file)
@@ -82,7 +82,7 @@
  *
  *     @(#)tcp_input.c 8.12 (Berkeley) 5/24/95
  * $FreeBSD: src/sys/netinet/tcp_input.c,v 1.107.2.38 2003/05/21 04:46:41 cjc Exp $
- * $DragonFly: src/sys/netinet/tcp_input.c,v 1.34 2004/08/08 06:33:24 hsu Exp $
+ * $DragonFly: src/sys/netinet/tcp_input.c,v 1.35 2004/08/11 02:36:22 dillon Exp $
  */
 
 #include "opt_ipfw.h"          /* for ipfw_fwd         */
@@ -730,7 +730,7 @@ findpcb:
                rstreason = BANDLIM_RST_CLOSEDPORT;
                goto dropwithreset;
        }
-       if (tp->t_state == TCPS_CLOSED)
+       if (tp->t_state <= TCPS_CLOSED)
                goto drop;
 
        /* Unscale the window into a 32-bit value. */
index e31b0db..cb7171f 100644 (file)
@@ -82,7 +82,7 @@
  *
  *     @(#)tcp_subr.c  8.2 (Berkeley) 5/24/95
  * $FreeBSD: src/sys/netinet/tcp_subr.c,v 1.73.2.31 2003/01/24 05:11:34 sam Exp $
- * $DragonFly: src/sys/netinet/tcp_subr.c,v 1.37 2004/08/03 00:04:13 dillon Exp $
+ * $DragonFly: src/sys/netinet/tcp_subr.c,v 1.38 2004/08/11 02:36:22 dillon Exp $
  */
 
 #include "opt_compat.h"
@@ -336,6 +336,7 @@ tcp_init()
 
        for (cpu = 0; cpu < ncpus2; cpu++) {
                in_pcbinfo_init(&tcbinfo[cpu]);
+               tcbinfo[cpu].cpu = cpu;
                tcbinfo[cpu].hashbase = hashinit(hashsize, M_PCB,
                    &tcbinfo[cpu].hashmask);
                tcbinfo[cpu].porthashbase = porthashbase;
@@ -675,6 +676,7 @@ tcp_newtcpcb(struct inpcb *inp)
        if (tcp_do_rfc1644)
                tp->t_flags |= TF_REQ_CC;
        tp->t_inpcb = inp;      /* XXX */
+       tp->t_state = TCPS_CLOSED;
        /*
         * Init srtt to TCPTV_SRTTBASE (0), so we can tell that we have no
         * rtt estimate.  Set rttvar so that srtt + 4 * rttvar gives
@@ -722,21 +724,49 @@ tcp_drop(struct tcpcb *tp, int errno)
 }
 
 #ifdef SMP
+
 struct netmsg_remwildcard {
        struct lwkt_msg         nm_lmsg;
        struct inpcb            *nm_inp;
        struct inpcbinfo        *nm_pcbinfo;
+#if defined(INET6)
+       int                     nm_isinet6;
+#else
+       int                     nm_unused01;
+#endif
 };
 
+/*
+ * Wildcard inpcb's on SMP boxes must be removed from all cpus before the
+ * inp can be detached.  We do this by cycling through the cpus, ending up
+ * on the cpu controlling the inp last and then doing the disconnect.
+ */
 static int
 in_pcbremwildcardhash_handler(struct lwkt_msg *msg0)
 {
        struct netmsg_remwildcard *msg = (struct netmsg_remwildcard *)msg0;
+       int cpu;
+
+       cpu = msg->nm_pcbinfo->cpu;
 
-       in_pcbremwildcardhash_oncpu(msg->nm_inp, msg->nm_pcbinfo);
-       lwkt_replymsg(&msg->nm_lmsg, 0);
+       if (cpu == msg->nm_inp->inp_pcbinfo->cpu) {
+               /* note: detach removes any wildcard hash entry */
+#ifdef INET6
+               if (msg->nm_isinet6)
+                       in6_pcbdetach(msg->nm_inp);
+               else
+#endif
+                       in_pcbdetach(msg->nm_inp);
+               lwkt_replymsg(&msg->nm_lmsg, 0);
+       } else {
+               in_pcbremwildcardhash_oncpu(msg->nm_inp, msg->nm_pcbinfo);
+               cpu = (cpu + 1) % ncpus2;
+               msg->nm_pcbinfo = &tcbinfo[cpu];
+               lwkt_forwardmsg(tcp_cport(cpu), &msg->nm_lmsg);
+       }
        return (EASYNC);
 }
+
 #endif
 
 /*
@@ -758,10 +788,25 @@ tcp_close(struct tcpcb *tp)
 #endif
 #ifdef INET6
        boolean_t isipv6 = ((inp->inp_vflag & INP_IPV6) != 0);
+       boolean_t isafinet6 = (INP_CHECK_SOCKAF(so, AF_INET6) != 0);
 #else
        const boolean_t isipv6 = FALSE;
 #endif
 
+       /*
+        * The tp is not instantly destroyed in the wildcard case.  Setting
+        * the state to TCPS_TERMINATING will prevent the TCP stack from
+        * messing with it, though it should be noted that this change may
+        * not take effect on other cpus until we have chained the wildcard
+        * hash removal.
+        *
+        * XXX we currently depend on the BGL to synchronize the tp->t_state
+        * update and prevent other tcp protocol threads from accepting new
+        * connections on the listen socket we might be trying to close down.
+        */
+       KKASSERT(tp->t_state != TCPS_TERMINATING);
+       tp->t_state = TCPS_TERMINATING;
+
        /*
         * Make sure that all of our timers are stopped before we
         * delete the PCB.
@@ -881,33 +926,46 @@ no_valid_rt:
                FREE(q, M_TSEGQ);
                tcp_reass_qsize--;
        }
+
        inp->inp_ppcb = NULL;
        soisdisconnected(so);
-
+       /*
+        * Discard the inp.  In the SMP case a wildcard inp's hash (created
+        * by a listen socket or an INADDR_ANY udp socket) is replicated
+        * for each protocol thread and must be removed in the context of
+        * that thread.  This is accomplished by chaining the message
+        * through the cpus.
+        *
+        * If the inp is not wildcarded we simply detach, which will remove
+        * the any hashes still present for this inp.
+        */
 #ifdef SMP
        if (inp->inp_flags & INP_WILDCARD_MP) {
-               for (cpu = 0; cpu < ncpus2; cpu ++) {
-                       struct netmsg_remwildcard *msg;
+               struct netmsg_remwildcard *msg;
 
-                       msg = malloc(sizeof(struct netmsg_remwildcard),
+               cpu = (inp->inp_pcbinfo->cpu + 1) % ncpus2;
+               msg = malloc(sizeof(struct netmsg_remwildcard),
                            M_LWKTMSG, M_INTWAIT);
-                       lwkt_initmsg(&msg->nm_lmsg, &netisr_afree_rport, 0,
-                           lwkt_cmd_func(in_pcbremwildcardhash_handler),
-                           lwkt_cmd_op_none);
-                       msg->nm_inp = inp;
-                       msg->nm_pcbinfo = &tcbinfo[cpu];
-                       lwkt_sendmsg(tcp_cport(cpu), &msg->nm_lmsg);
-               }
-               /* XXX wait? */
-       }
+               lwkt_initmsg(&msg->nm_lmsg, &netisr_afree_rport, 0,
+                   lwkt_cmd_func(in_pcbremwildcardhash_handler),
+                   lwkt_cmd_op_none);
+#ifdef INET6
+               msg->nm_isinet6 = isafinet6;
 #endif
-
+               msg->nm_inp = inp;
+               msg->nm_pcbinfo = &tcbinfo[cpu];
+               lwkt_sendmsg(tcp_cport(cpu), &msg->nm_lmsg);
+       } else 
+#endif
+       {
+               /* note: detach removes any wildcard hash entry */
 #ifdef INET6
-       if (INP_CHECK_SOCKAF(so, AF_INET6))
-               in6_pcbdetach(inp);
-       else
+               if (isafinet6)
+                       in6_pcbdetach(inp);
+               else
 #endif
-               in_pcbdetach(inp);
+                       in_pcbdetach(inp);
+       }
        tcpstat.tcps_closed++;
        return (NULL);
 }
index adea506..ca3de79 100644 (file)
@@ -82,7 +82,7 @@
  *
  *     From: @(#)tcp_usrreq.c  8.2 (Berkeley) 1/3/94
  * $FreeBSD: src/sys/netinet/tcp_usrreq.c,v 1.51.2.17 2002/10/11 11:46:44 ume Exp $
- * $DragonFly: src/sys/netinet/tcp_usrreq.c,v 1.25 2004/07/08 22:07:35 hsu Exp $
+ * $DragonFly: src/sys/netinet/tcp_usrreq.c,v 1.26 2004/08/11 02:36:22 dillon Exp $
  */
 
 #include "opt_ipsec.h"
@@ -357,6 +357,11 @@ tcp_usr_listen(struct socket *so, struct thread *td)
 
        tp->t_state = TCPS_LISTEN;
 #ifdef SMP
+       /*
+        * We have to set the flag because we can't have other cpus messing
+        * with our inp's flags.
+        */
+       inp->inp_flags |= INP_WILDCARD_MP;
        for (cpu = 0; cpu < ncpus2; cpu++) {
                struct netmsg_inswildcard *msg;
 
@@ -374,7 +379,6 @@ tcp_usr_listen(struct socket *so, struct thread *td)
                msg->nm_pcbinfo = &tcbinfo[cpu];
                lwkt_sendmsg(tcp_cport(cpu), &msg->nm_lmsg);
        }
-       inp->inp_flags |= INP_WILDCARD_MP;
 #else
        in_pcbinswildcardhash(inp);
 #endif