Re: [PATCH net 2/2] vsock/test: Test setting SO_ZEROCOPY on accept()ed socket

Michal Luczaj
2025-12-23T20:38:27Z

On 12/23/25 17:50, Stefano Garzarella wrote:
> On Tue, Dec 23, 2025 at 02:20:33PM +0100, Stefano Garzarella wrote:
>> On Tue, Dec 23, 2025 at 12:10:25PM +0100, Michal Luczaj wrote:
>>> On 12/23/25 11:27, Stefano Garzarella wrote:
>>>> On Tue, Dec 23, 2025 at 10:15:29AM +0100, Michal Luczaj wrote:
>>>>> Make sure setsockopt(SOL_SOCKET, SO_ZEROCOPY) on an accept()ed socket is
>>>>> handled by vsock's implementation.
>>>>>
>>>>> Signed-off-by: Michal Luczaj <mhal@rbox.co>
>>>>> ---
>>>>> tools/testing/vsock/vsock_test.c | 33 +++++++++++++++++++++++++++++++++
>>>>> 1 file changed, 33 insertions(+)
>>>>>
>>>>> diff --git a/tools/testing/vsock/vsock_test.c b/tools/testing/vsock/vsock_test.c
>>>>> index 9e1250790f33..8ec8f0844e22 100644
>>>>> --- a/tools/testing/vsock/vsock_test.c
>>>>> +++ b/tools/testing/vsock/vsock_test.c
>>>>> @@ -2192,6 +2192,34 @@ static void test_stream_nolinger_server(const struct test_opts *opts)
>>>>> close(fd);
>>>>> }
>>>>>
>>>>> +static void test_stream_accepted_setsockopt_client(const struct test_opts *opts)
>>>>> +{
>>>>> + int fd;
>>>>> +
>>>>> + fd = vsock_stream_connect(opts->peer_cid, opts->peer_port);
>>>>> + if (fd < 0) {
>>>>> + perror("connect");
>>>>> + exit(EXIT_FAILURE);
>>>>> + }
>>>>> +
>>>>> + vsock_wait_remote_close(fd);
>
> On a second look, why we need to wait the remote close?
> can we just have a control message?

I think we can. I've used vsock_wait_remote_close() simply as a sync
primitive. It's one line of code less.

> I'm not sure even on that, I mean why this peer can't close the
> connection while the other is checking if it's able to set zerocopy?

I was worried that without any sync, client-side close() may race
server-side accept(), but I've just checked and it doesn't seem to cause
any issues, at least for the virtio transports.

>>>>> + close(fd);
>>>>> +}
>>>>> +
>>>>> +static void test_stream_accepted_setsockopt_server(const struct test_opts *opts)
>>>>> +{
>>>>> + int fd;
>>>>> +
>>>>> + fd = vsock_stream_accept(VMADDR_CID_ANY, opts->peer_port, NULL);
>>>>> + if (fd < 0) {
>>>>> + perror("accept");
>>>>> + exit(EXIT_FAILURE);
>>>>> + }
>>>>> +
>>>>> + enable_so_zerocopy_check(fd);
>>>>
>>>> This test is passing on my env also without the patch applied.
>>>>
>>>> Is that expected?
>>>
>>> Oh, no, definitely not. It fails for me:
>>> 36 - SOCK_STREAM accept()ed socket custom setsockopt()...36 - SOCK_STREAM
>>> accept()ed socket custom setsockopt()...setsockopt err: Operation not
>>> supported (95)
>>> setsockopt SO_ZEROCOPY val 1
>>
>> aaa, right, the server is failing, sorry ;-)
>>
>> Reviewed-by: Stefano Garzarella <sgarzare@redhat.com>
>>>
>>> I have no idea what's going on :)
>>>
>>
>> In my suite, I'm checking the client, and if the last test fails only
>> on the server, I'm missing it. I'd fix my suite, and maybe also
>> vsock_test adding another sync point.
>
> Added a full barrier here:
> https://lore.kernel.org/netdev/20251223162210.43976-1-sgarzare@redhat.com

Which reminds me of discussion in
https://lore.kernel.org/netdev/151bf5fe-c9ca-4244-aa21-8d7b8ff2470f@rbox.co/
. Sorry for postponing, I've put it on my vsock-cleanups branch and kept
adding more little fixes, and it was never the right time to post the series.