From: Alexander Bluhm <bluhm@openbsd.org>
Subject: Re: tcp output MP fixes
To: tech@openbsd.org
Date: Fri, 8 Nov 2024 16:20:41 +0100

On Wed, Nov 06, 2024 at 11:50:03AM +0100, Alexander Bluhm wrote:
> Next goal is running TCP socket layer with shared netlock.  I went
> through tcp_output() and looked for MP races.  Mostly documentation
> and small fixes.

tcp_update_sndspace() and tcp_update_rcvspace() only read global
variables that don't change.  Mark them as such.

And can we get braces consistently around the multi-line if blocks?

ok?

bluhm

Index: netinet/tcp_usrreq.c
===================================================================
RCS file: /data/mirror/openbsd/cvs/src/sys/netinet/tcp_usrreq.c,v
diff -u -p -r1.231 tcp_usrreq.c
--- netinet/tcp_usrreq.c	12 Apr 2024 16:07:09 -0000	1.231
+++ netinet/tcp_usrreq.c	8 Nov 2024 15:15:57 -0000
@@ -102,15 +102,20 @@
 #include <netinet6/in6_var.h>
 #endif
 
+/*
+ * Locks used to protect global variables in this file:
+ *	I	immutable after creation
+ */
+
 #ifndef TCP_SENDSPACE
 #define	TCP_SENDSPACE	1024*16
 #endif
-u_int	tcp_sendspace = TCP_SENDSPACE;
+u_int	tcp_sendspace = TCP_SENDSPACE;		/* [I] */
 #ifndef TCP_RECVSPACE
 #define	TCP_RECVSPACE	1024*16
 #endif
-u_int	tcp_recvspace = TCP_RECVSPACE;
-u_int	tcp_autorcvbuf_inc = 16 * 1024;
+u_int	tcp_recvspace = TCP_RECVSPACE;		/* [I] */
+u_int	tcp_autorcvbuf_inc = 16 * 1024;		/* [I] */
 
 const struct pr_usrreqs tcp_usrreqs = {
 	.pru_attach	= tcp_attach,
@@ -1516,13 +1521,14 @@ tcp_update_sndspace(struct tcpcb *tp)
 		/* low on memory try to get rid of some */
 		if (tcp_sendspace < nmax)
 			nmax = tcp_sendspace;
-	} else if (so->so_snd.sb_wat != tcp_sendspace)
+	} else if (so->so_snd.sb_wat != tcp_sendspace) {
 		/* user requested buffer size, auto-scaling disabled */
 		nmax = so->so_snd.sb_wat;
-	else
+	} else {
 		/* automatic buffer scaling */
 		nmax = MIN(sb_max, so->so_snd.sb_wat + tp->snd_max -
 		    tp->snd_una);
+	}
 
 	/* a writable socket must be preserved because of poll(2) semantics */
 	if (sbspace(so, &so->so_snd) >= so->so_snd.sb_lowat) {
@@ -1556,10 +1562,10 @@ tcp_update_rcvspace(struct tcpcb *tp)
 		/* low on memory try to get rid of some */
 		if (tcp_recvspace < nmax)
 			nmax = tcp_recvspace;
-	} else if (so->so_rcv.sb_wat != tcp_recvspace)
+	} else if (so->so_rcv.sb_wat != tcp_recvspace) {
 		/* user requested buffer size, auto-scaling disabled */
 		nmax = so->so_rcv.sb_wat;
-	else {
+	} else {
 		/* automatic buffer scaling */
 		if (tp->rfbuf_cnt > so->so_rcv.sb_hiwat / 8 * 7)
 			nmax = MIN(sb_max, so->so_rcv.sb_hiwat +