From: Vitaliy Makkoveev <mvs@openbsd.org>
Subject: Re: Please test: shared solock for all intet sockets within knote(9) routines
To: Alexander Bluhm <alexander.bluhm@gmx.net>
Cc: tech@openbsd.org
Date: Sun, 28 Jan 2024 22:25:14 +0300

On Sun, Jan 28, 2024 at 12:54:24PM +0100, Alexander Bluhm wrote:
> On Fri, Jan 26, 2024 at 07:59:02PM +0300, Vitaliy Makkoveev wrote:
> > Not for commit yet, but could be interesting. The idea is the same as
> > for PCB layer: take shared netlock and exclusive `so_lock' to serialize
> > access to the socket. The tcp(4) sockets permit this because knote(9)
> > routines do read-only access to sockets.
> 
> Passes regress with witness.
> 
> OK bluhm@
> 
> > Should provide some additional performance.
> 
> My setup is currently busy with some other test run.
> Can do performance comparison in a few days.

Thanks for attention. Unfortunately current soassertlocked() is very
dumb for inet case:

soassertlocked(struct socket *so)
{       
        switch (so->so_proto->pr_domain->dom_family) {
        case PF_INET:
        case PF_INET6:
                NET_ASSERT_LOCKED();
                break;
        default:
                rw_assert_wrlock(&so->so_lock);
                break;
        }
}

Could we update it first and check `so_lock' state for shared netlock
case? Since we already use solock_shared(), I want to raise assertion
instead of panic.

Index: sys/kern/uipc_socket2.c
===================================================================
RCS file: /cvs/src/sys/kern/uipc_socket2.c,v
retrieving revision 1.140
diff -u -p -r1.140 uipc_socket2.c
--- sys/kern/uipc_socket2.c	11 Jan 2024 14:15:11 -0000	1.140
+++ sys/kern/uipc_socket2.c	28 Jan 2024 19:18:48 -0000
@@ -444,7 +444,14 @@ soassertlocked(struct socket *so)
 	switch (so->so_proto->pr_domain->dom_family) {
 	case PF_INET:
 	case PF_INET6:
-		NET_ASSERT_LOCKED();
+		if (rw_status(&netlock) == RW_READ) {
+			NET_ASSERT_LOCKED();
+
+			if (splassert_ctl > 0 &&
+			    rw_status(&so->so_lock) != RW_WRITE)
+				splassert_fail(0, RW_WRITE, __func__);
+		} else
+			NET_ASSERT_LOCKED_EXCLUSIVE();
 		break;
 	default:
 		rw_assert_wrlock(&so->so_lock);