On Tue, Dec 23, 2025 at 09:38:07PM +0100, Michal Luczaj wrote:

On 12/23/25 17:50, Stefano Garzarella wrote:

quoted

On Tue, Dec 23, 2025 at 02:20:33PM +0100, Stefano Garzarella wrote:

quoted

On Tue, Dec 23, 2025 at 12:10:25PM +0100, Michal Luczaj wrote:

quoted

On 12/23/25 11:27, Stefano Garzarella wrote:

quoted

On Tue, Dec 23, 2025 at 10:15:29AM +0100, Michal Luczaj wrote:

quoted

Make sure setsockopt(SOL_SOCKET, SO_ZEROCOPY) on an accept()ed socket is
handled by vsock's implementation.

Signed-off-by: Michal Luczaj <redacted>
---
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.

quoted

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.

Okay, I see. Feel free to leave it, but if it's not really needed, I'd 
prefer to keep the tests as simple as possible.

quoted

quoted

quoted

quoted

quoted

+	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>

quoted

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 (local)

Which reminds me of discussion in
https://lore.kernel.org/netdev/151bf5fe-c9ca-4244-aa21-8d7b8ff2470f@rbox.co/ (local)

Oh, I forgot that we already discussed that.

My first attempt was exactly that, but then discovered that it didn't 
add too much except for the last one since for the others we have 2 full 
barriers back to back, so I preferred to move outside the loop. In that 
way we can also be sure the 2 `vsock_tests` are in sync with the amount 
of tests to run.

But yeah, also that one fix the issue.

. 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.

Nah, don't apologize, you're doing an amazing job!

Thanks,
Stefano