Re: [PATCH RFC v3 8/8] vsock: add datagram tests to vsock_test

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential
issues to consider:
- [Medium] selftests/vsock: unguarded loopback SOCK_SEQPACKET connect in
  local netns
- [Low] vsock/test: incomplete DGRAM_VMCI probe in dgram_capabilities()
--

commit 75f8685914783d3ed99f76000f4d13a1f1c04ea3
Author: Sergio Lopez <slp@redhat.com>

vsock: add datagram tests to vsock_test

This commit adds AF_VSOCK datagram tests to vsock_test covering local
semantics, peer transfers, routing, queue accounting, and capability-based
skips, along with a --local mode for running local test cases standalone.

> diff --git a/tools/testing/vsock/vsock_test_dgram.c b/tools/testing/vsock/vsock_test_dgram.c
> new file mode 100644
> index 0000000000000..4b45dcf2a15cc
> --- /dev/null
> +++ b/tools/testing/vsock/vsock_test_dgram.c
> @@ -0,0 +1,1061 @@

[ ... ]

> +static unsigned int dgram_capabilities(const struct test_opts *opts,
> +				       unsigned int needs)
> +{
> +	unsigned int caps = DGRAM_SOCKET;
> +	struct sockaddr_vm peer;
> +	int fd, local_fd;
> +
> +	fd = socket(AF_VSOCK, SOCK_DGRAM | SOCK_NONBLOCK | SOCK_CLOEXEC, 0);
> +	if (fd < 0) {
> +		if (dgram_unavailable(errno))
> +			return 0;
> +		check(fd, "probe datagram socket");
> +	}
> +	/* VMCI may own the socket while a directional transport serves peers.
> +	 * A nested guest can have both virtio and vhost registered. Probe the
> +	 * requested direction so vhost registration cannot mask a virtio device
> +	 * that did not negotiate datagrams.
> +	 */
> +	if (!access("/sys/module/vmw_vsock_vmci_transport", F_OK))
> +		caps |= DGRAM_VMCI;

[Severity: Low]
Does checking only for /sys/module/vmw_vsock_vmci_transport leave
test_dgram_vmci_inq() unskipped when VMCI cannot route datagrams to
itself?

When vmw_vsock_vmci_transport is loaded without vsock_loopback, running in
a local-mode vsock network namespace causes vmci_transport_dgram_allow()
to return false and sendto() in test_dgram_vmci_inq() to fail with EINVAL
(or EADDRNOTAVAIL when VMCI has no active context ID), aborting the test
run instead of skipping.

[ ... ]

> +	if (needs & DGRAM_LOOPBACK) {
> +		/* Binding a stream socket to CID_LOCAL probes registration of
> +		 * the loopback transport without sending a packet to the host.
> +		 */
> +		local_fd = vsock_bind_try(VMADDR_CID_LOCAL, VMADDR_PORT_ANY, SOCK_STREAM);
> +		if (local_fd >= 0) {
> +			caps |= DGRAM_LOOPBACK;
> +			close(local_fd);
> +		} else if (errno != EADDRNOTAVAIL) {
> +			check(local_fd, "probe loopback transport");
> +		}
> +	}
> +	return caps;
> +}

[ ... ]

> +void test_dgram_bound_lookup(const struct test_opts *opts)
> +{
> +	const int types[] = { SOCK_STREAM, SOCK_SEQPACKET };
> +	unsigned int cid = local_cid();
> +	size_t i;
> +
> +	for (i = 0; i < ARRAY_SIZE(types); i++) {
> +		int listener = vsock_bind(VMADDR_CID_ANY, VMADDR_PORT_ANY, types[i]);
> +		int stream = vsock_bind(VMADDR_CID_LOCAL, VMADDR_PORT_ANY, types[i]);
> +		struct sockaddr_vm listen_addr = local_address(listener);
> +		struct sockaddr_vm src = sock_name(stream, false);
> +		struct sockaddr_vm dest;
> +		int accepted, tx, rx;
> +		char byte;
> +
> +		check(listen(listener, 1), "listen");
> +		if (vsock_connect_fd(stream, listen_addr.svm_cid, listen_addr.svm_port))
> +			fail("%s loopback connect: %s",
> +			     types[i] == SOCK_STREAM ? "stream" : "seqpacket", strerror(errno));

[Severity: Medium]
Will the SOCK_SEQPACKET iteration in test_dgram_bound_lookup() and
test_dgram_port_space() fail with ESOCKTNOSUPPORT when vsock_test runs in
a local-mode vsock network namespace?

dgram_capabilities() probes DGRAM_LOOPBACK with SOCK_STREAM, which is
allowed in both global and local namespace modes, whereas
vsock_loopback_seqpacket_allow() rejects SOCK_SEQPACKET when the socket is
not in global mode:

net/vmw_vsock/vsock_loopback.c:vsock_loopback_seqpacket_allow() {
	return vsock_net_mode_global(vsk);
}

As a result, running vsock_test in a local-mode namespace (such as the
local-namespace cases in tools/testing/selftests/vsock/vmtest.sh) will
pass the capability check and then abort in test_dgram_bound_lookup() and
test_dgram_port_space() when connecting the SOCK_SEQPACKET socket to
VMADDR_CID_LOCAL.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261113-vsock-dgram-support-v3-0-1523c8e9b27e@redhat.com?part=8