Rework socket upcalls to close some races with setup/teardown of upcalls.

- Each socket upcall is now invoked with the appropriate socket buffer
  locked.  It is not permissible to call soisconnected() with this lock
  held; however, so socket upcalls now return an integer value.  The two
  possible values are SU_OK and SU_ISCONNECTED.  If an upcall returns
  SU_ISCONNECTED, then the soisconnected() will be invoked on the
  socket after the socket buffer lock is dropped.
- A new API is provided for setting and clearing socket upcalls.  The
  API consists of soupcall_set() and soupcall_clear().
- To simplify locking, each socket buffer now has a separate upcall.
- When a socket upcall returns SU_ISCONNECTED, the upcall is cleared from
  the receive socket buffer automatically.  Note that a SO_SND upcall
  should never return SU_ISCONNECTED.
- All this means that accept filters should now return SU_ISCONNECTED
  instead of calling soisconnected() directly.  They also no longer need
  to explicitly clear the upcall on the new socket.
- The HTTP accept filter still uses soupcall_set() to manage its internal
  state machine, but other accept filters no longer have any explicit
  knowlege of socket upcall internals aside from their return value.
- The various RPC client upcalls currently drop the socket buffer lock
  while invoking soreceive() as a temporary band-aid.  The plan for
  the future is to add a new flag to allow soreceive() to be called with
  the socket buffer locked.
- The AIO callback for socket I/O is now also invoked with the socket
  buffer locked.  Previously sowakeup() would drop the socket buffer
  lock only to call aio_swake() which immediately re-acquired the socket
  buffer lock for the duration of the function call.

Discussed with:	rwatson, rmacklem

3 files changed, 28 insertions, 47 deletions

diff --git a/sys/netinet/accf_data.c b/sys/netinet/accf_data.c
index 1bbd3fe..599392c 100644
--- a/sys/netinet/accf_data.c
+++ b/sys/netinet/accf_data.c
@@ -38,7 +38,7 @@ __FBSDID("$FreeBSD$");
 
 /* accept filter that holds a socket until data arrives */
 
-static void	sohasdata(struct socket *so, void *arg, int waitflag);
+static int	sohasdata(struct socket *so, void *arg, int waitflag);
 
 static struct accept_filter accf_data_filter = {
 	"dataready",
@@ -55,15 +55,12 @@ static moduledata_t accf_data_mod = {
 
 DECLARE_MODULE(accf_data, accf_data_mod, SI_SUB_DRIVERS, SI_ORDER_MIDDLE);
 
-static void
+static int
 sohasdata(struct socket *so, void *arg, int waitflag)
 {
 
 	if (!soreadable(so))
-		return;
+		return (SU_OK);
 
-	so->so_upcall = NULL;
-	so->so_rcv.sb_flags &= ~SB_UPCALL;
-	soisconnected(so);
-	return;
+	return (SU_ISCONNECTED);
 }
diff --git a/sys/netinet/accf_dns.c b/sys/netinet/accf_dns.c
index 8789906..ec2b4cf 100644
--- a/sys/netinet/accf_dns.c
+++ b/sys/netinet/accf_dns.c
@@ -37,7 +37,7 @@
 #include <sys/socketvar.h>
 
 /* check for full DNS request */
-static void sohasdns(struct socket *so, void *arg, int waitflag);
+static int sohasdns(struct socket *so, void *arg, int waitflag);
 
 struct packet {
 	struct mbuf *m;		/* Current mbuf. */
@@ -69,7 +69,7 @@ static moduledata_t accf_dns_mod = {
 
 DECLARE_MODULE(accf_dns, accf_dns_mod, SI_SUB_DRIVERS, SI_ORDER_MIDDLE);
 
-static void
+static int
 sohasdns(struct socket *so, void *arg, int waitflag)
 {
 	struct sockbuf *sb = &so->so_rcv;
@@ -80,13 +80,10 @@ sohasdns(struct socket *so, void *arg, int waitflag)
 
 	/* Check to see if we have a request. */
 	if (skippacket(sb) == DNS_WAIT)
-		return;
+		return (SU_OK);
 
 ready:
-	so->so_upcall = NULL;
-	so->so_rcv.sb_flags &= ~SB_UPCALL;
-	soisconnected(so);
-	return;
+	return (SU_ISCONNECTED);
 }
 
 #define GET8(p, val) do { \
diff --git a/sys/netinet/accf_http.c b/sys/netinet/accf_http.c
index 19ad380..b3761ef 100644
--- a/sys/netinet/accf_http.c
+++ b/sys/netinet/accf_http.c
@@ -39,11 +39,11 @@ __FBSDID("$FreeBSD$");
 #include <sys/socketvar.h>
 
 /* check for GET/HEAD */
-static void sohashttpget(struct socket *so, void *arg, int waitflag);
+static int sohashttpget(struct socket *so, void *arg, int waitflag);
 /* check for HTTP/1.0 or HTTP/1.1 */
-static void soparsehttpvers(struct socket *so, void *arg, int waitflag);
+static int soparsehttpvers(struct socket *so, void *arg, int waitflag);
 /* check for end of HTTP/1.x request */
-static void soishttpconnected(struct socket *so, void *arg, int waitflag);
+static int soishttpconnected(struct socket *so, void *arg, int waitflag);
 /* strcmp on an mbuf chain */
 static int mbufstrcmp(struct mbuf *m, struct mbuf *npkt, int offset, char *cmp);
 /* strncmp on an mbuf chain */
@@ -158,7 +158,7 @@ mbufstrncmp(struct mbuf *m, struct mbuf *npkt, int offset, int max, char *cmp)
 		slen = sizeof(str) - 1;					\
 	} while(0)
 
-static void
+static int
 sohashttpget(struct socket *so, void *arg, int waitflag)
 {
 
@@ -170,7 +170,7 @@ sohashttpget(struct socket *so, void *arg, int waitflag)
 		m = so->so_rcv.sb_mb;
 		cc = so->so_rcv.sb_cc - 1;
 		if (cc < 1)
-			return;
+			return (SU_OK);
 		switch (*mtod(m, char *)) {
 		case 'G':
 			STRSETUP(cmp, cmplen, "ET ");
@@ -184,7 +184,7 @@ sohashttpget(struct socket *so, void *arg, int waitflag)
 		if (cc < cmplen) {
 			if (mbufstrncmp(m, m->m_nextpkt, 1, cc, cmp) == 1) {
 				DPRINT("short cc (%d) but mbufstrncmp ok", cc);
-				return;
+				return (SU_OK);
 			} else {
 				DPRINT("short cc (%d) mbufstrncmp failed", cc);
 				goto fallout;
@@ -193,23 +193,19 @@ sohashttpget(struct socket *so, void *arg, int waitflag)
 		if (mbufstrcmp(m, m->m_nextpkt, 1, cmp) == 1) {
 			DPRINT("mbufstrcmp ok");
 			if (parse_http_version == 0)
-				soishttpconnected(so, arg, waitflag);
+				return (soishttpconnected(so, arg, waitflag));
 			else
-				soparsehttpvers(so, arg, waitflag);
-			return;
+				return (soparsehttpvers(so, arg, waitflag));
 		}
 		DPRINT("mbufstrcmp bad");
 	}
 
 fallout:
 	DPRINT("fallout");
-	so->so_upcall = NULL;
-	so->so_rcv.sb_flags &= ~SB_UPCALL;
-	soisconnected(so);
-	return;
+	return (SU_ISCONNECTED);
 }
 
-static void
+static int
 soparsehttpvers(struct socket *so, void *arg, int waitflag)
 {
 	struct mbuf *m, *n;
@@ -261,10 +257,9 @@ soparsehttpvers(struct socket *so, void *arg, int waitflag)
 					} else if (
 					    mbufstrcmp(m, n, i, "HTTP/1.0") ||
 					    mbufstrcmp(m, n, i, "HTTP/1.1")) {
-							DPRINT("ok");
-							soishttpconnected(so,
-							    arg, waitflag);
-							return;
+						DPRINT("ok");
+						return (soishttpconnected(so,
+						    arg, waitflag));
 					} else {
 						DPRINT("bad");
 						goto fallout;
@@ -279,22 +274,18 @@ readmore:
 	 * if we hit here we haven't hit something
 	 * we don't understand or a newline, so try again
 	 */
-	so->so_upcall = soparsehttpvers;
-	so->so_rcv.sb_flags |= SB_UPCALL;
-	return;
+	soupcall_set(so, SO_RCV, soparsehttpvers, arg);
+	return (SU_OK);
 
 fallout:
 	DPRINT("fallout");
-	so->so_upcall = NULL;
-	so->so_rcv.sb_flags &= ~SB_UPCALL;
-	soisconnected(so);
-	return;
+	return (SU_ISCONNECTED);
 }
 
 
 #define NCHRS 3
 
-static void
+static int
 soishttpconnected(struct socket *so, void *arg, int waitflag)
 {
 	char a, b, c;
@@ -350,13 +341,9 @@ soishttpconnected(struct socket *so, void *arg, int waitflag)
 	}
 
 readmore:
-	so->so_upcall = soishttpconnected;
-	so->so_rcv.sb_flags |= SB_UPCALL;
-	return;
+	soupcall_set(so, SO_RCV, soishttpconnected, arg);
+	return (SU_OK);
 
 gotit:
-	so->so_upcall = NULL;
-	so->so_rcv.sb_flags &= ~SB_UPCALL;
-	soisconnected(so);
-	return;
+	return (SU_ISCONNECTED);
 }