From: Vitaliy Makkoveev <mvs@openbsd.org>
Subject: Re: Relax sockets splicing locking
To: Alexander Bluhm <bluhm@openbsd.org>
Cc: tech@openbsd.org
Date: Mon, 6 Jan 2025 16:26:30 +0300

On Mon, Jan 06, 2025 at 01:17:44AM +0100, Alexander Bluhm wrote:
> On Sat, Jan 04, 2025 at 08:07:14PM +0300, Vitaliy Makkoveev wrote:
> > On Sat, Jan 04, 2025 at 06:39:26PM +0300, Vitaliy Makkoveev wrote:
> > > > On 4 Jan 2025, at 16:34, Alexander Bluhm <bluhm@openbsd.org> wrote:
> > > > 
> > 
> > > > [skip]
> > 
> > > > I have tested it with my regress and netlink setup.  My perform
> > > > testing machines are currently busy with other tests.
> > > > 
> > > > We have no propper regression tests for splicing and unsplicing
> > > > under heavy load.  I am currently trying to write a test.
> > > > 
> > > 
> > > We need something like multithreaded attempts of simultaneous
> > > splicing and unsplicing one socket to itself. Or the same but
> > > for two sockets.
> > > 
> > 
> > The simplest test to concurrently splice/unsplice socket.
> 
> I have commited a more exhaustive test in regress/sys/netinet/tcpthread.
> It does all kind of TCP socket operations in parallel.
> 
> But your test also has value as it does splice/unsplice very fast.
> If you want, I can integrate it somewhere in regress/sys/kern/sosplice
> with a proper regress Makefile.
> 
> Currently the kernel does not complain about unsplicing sockets
> that are not spliced.  I think it should report an error.  That
> would help counting success in our tests.
> 

Does ENOTCONN look reasonable? We are not interesting in this error in
the soclose(), soidle() and sotask() paths, only in the unsplicing path
of sosplice().

Index: sys/kern/uipc_socket.c
===================================================================
RCS file: /cvs/src/sys/kern/uipc_socket.c,v
diff -u -p -r1.356 uipc_socket.c
--- sys/kern/uipc_socket.c	4 Jan 2025 15:57:02 -0000	1.356
+++ sys/kern/uipc_socket.c	6 Jan 2025 13:22:20 -0000
@@ -1388,9 +1388,10 @@ sosplice(struct socket *so, int fd, off_
 			sosp = soref(so->so_sp->ssp_socket);
 			sounsplice(so, so->so_sp->ssp_socket, 0);
 			sorele(sosp);
-		}
+		} else
+			error = ENOTCONN;
 		sbunlock(&so->so_rcv);
-		return (0);
+		return (error);
 	}
 
 	if (sosplice_taskq == NULL) {