Index | Thread | Search

From:
Vitaliy Makkoveev <mvs@openbsd.org>
Subject:
Re: Relax sockets splicing locking
To:
Alexander Bluhm <alexander.bluhm@gmx.net>
Cc:
OpenBSD tech <tech@openbsd.org>
Date:
Tue, 7 Jan 2025 10:56:57 +0300

Download raw body.

Thread
> On 7 Jan 2025, at 02:31, Alexander Bluhm <alexander.bluhm@gmx.net> wrote:
> 
> On Mon, Jan 06, 2025 at 04:26:30PM +0300, Vitaliy Makkoveev wrote:
>> 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().
> 
> I have nearly the same diff.  The ENOTCONN is not unique.  sosplice()
> may return that for other conditions.  I find it useful that different
> error numbers point to the failure.  Reading errno(2) up and down,
> I came to the conclusion that EDESTADDRREQ may be suitable.  The
> socket is not spliced, you are supposed to specify a drain file
> descriptor.
> 

Not sure. POSIX describes it as

[EDESTADDRREQ]
	Destination address required. No bind address was established.

Unsplicing is the some kind of disconnection, we don't drain
sockets dunring unsplice. ENOTCONN is uniq for unsplicing. We need
to explicitly pass NULL to setsockopt() of set sp_df to -1 for
unsplicing:

setsockopt(s, SOL_SOCKET, SO_SPLICE, NULL, 0);

sp_fd = -1;
setsockopt(s, SOL_SOCKET, SO_SPLICE, &sp_fd, sizeof(sp_fd));

sp.sp_fd = -1;
setsockopt(s, SOL_SOCKET, SO_SPLICE, &sp, sizeof(sp));

Hard to mismatch with splicing.

> The new error should be documented in sosplice(9) man page.
> 
> I have fixed the failing test in regress/sys/kern/sosplice .
> 
> With EDESTADDRREQ and man page changes
> OK bluhm@
> 
>> 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) {