From 1c39f5dbbfd2b44602d46936fe55361205196257 Mon Sep 17 00:00:00 2001 From: Michal Luczaj Date: Thu, 22 May 2025 01:18:21 +0200 Subject: [PATCH 1/5] vsock/virtio: Linger on unsent data Currently vsock's lingering effectively boils down to waiting (or timing out) until packets are consumed or dropped by the peer; be it by receiving the data, closing or shutting down the connection. To align with the semantics described in the SO_LINGER section of man socket(7) and to mimic AF_INET's behaviour more closely, change the logic of a lingering close(): instead of waiting for all data to be handled, block until data is considered sent from the vsock's transport point of view. That is until worker picks the packets for processing and decrements virtio_vsock_sock::bytes_unsent down to 0. Note that (some interpretation of) lingering was always limited to transports that called virtio_transport_wait_close() on transport release. This does not change, i.e. under Hyper-V and VMCI no lingering would be observed. The implementation does not adhere strictly to man page's interpretation of SO_LINGER: shutdown() will not trigger the lingering. This follows AF_INET. Reviewed-by: Stefano Garzarella Signed-off-by: Michal Luczaj Link: https://patch.msgid.link/20250522-vsock-linger-v6-1-2ad00b0e447e@rbox.co Signed-off-by: Paolo Abeni --- net/vmw_vsock/virtio_transport_common.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/net/vmw_vsock/virtio_transport_common.c b/net/vmw_vsock/virtio_transport_common.c index 6e7b727c781c..f2f1b166731b 100644 --- a/net/vmw_vsock/virtio_transport_common.c +++ b/net/vmw_vsock/virtio_transport_common.c @@ -1195,12 +1195,14 @@ static void virtio_transport_wait_close(struct sock *sk, long timeout) { if (timeout) { DEFINE_WAIT_FUNC(wait, woken_wake_function); + struct vsock_sock *vsk = vsock_sk(sk); add_wait_queue(sk_sleep(sk), &wait); do { if (sk_wait_event(sk, &timeout, - sock_flag(sk, SOCK_DONE), &wait)) + virtio_transport_unsent_bytes(vsk) == 0, + &wait)) break; } while (!signal_pending(current) && timeout); From 5ec40864aaecc4bd66fe67541d4a41091ed664a5 Mon Sep 17 00:00:00 2001 From: Michal Luczaj Date: Thu, 22 May 2025 01:18:22 +0200 Subject: [PATCH 2/5] vsock: Move lingering logic to af_vsock core Lingering should be transport-independent in the long run. In preparation for supporting other transports, as well as the linger on shutdown(), move code to core. Generalize by querying vsock_transport::unsent_bytes(), guard against the callback being unimplemented. Do not pass sk_lingertime explicitly. Pull SOCK_LINGER check into vsock_linger(). Flatten the function. Remove the nested block by inverting the condition: return early on !timeout. Suggested-by: Stefano Garzarella Reviewed-by: Stefano Garzarella Signed-off-by: Michal Luczaj Link: https://patch.msgid.link/20250522-vsock-linger-v6-2-2ad00b0e447e@rbox.co Signed-off-by: Paolo Abeni --- include/net/af_vsock.h | 1 + net/vmw_vsock/af_vsock.c | 33 +++++++++++++++++++++++++ net/vmw_vsock/virtio_transport_common.c | 23 ++--------------- 3 files changed, 36 insertions(+), 21 deletions(-) diff --git a/include/net/af_vsock.h b/include/net/af_vsock.h index 9e85424c8343..d56e6e135158 100644 --- a/include/net/af_vsock.h +++ b/include/net/af_vsock.h @@ -221,6 +221,7 @@ void vsock_for_each_connected_socket(struct vsock_transport *transport, void (*fn)(struct sock *sk)); int vsock_assign_transport(struct vsock_sock *vsk, struct vsock_sock *psk); bool vsock_find_cid(unsigned int cid); +void vsock_linger(struct sock *sk); /**** TAP ****/ diff --git a/net/vmw_vsock/af_vsock.c b/net/vmw_vsock/af_vsock.c index fc6afbc8d680..2e7a3034e965 100644 --- a/net/vmw_vsock/af_vsock.c +++ b/net/vmw_vsock/af_vsock.c @@ -1013,6 +1013,39 @@ static int vsock_getname(struct socket *sock, return err; } +void vsock_linger(struct sock *sk) +{ + DEFINE_WAIT_FUNC(wait, woken_wake_function); + ssize_t (*unsent)(struct vsock_sock *vsk); + struct vsock_sock *vsk = vsock_sk(sk); + long timeout; + + if (!sock_flag(sk, SOCK_LINGER)) + return; + + timeout = sk->sk_lingertime; + if (!timeout) + return; + + /* Transports must implement `unsent_bytes` if they want to support + * SOCK_LINGER through `vsock_linger()` since we use it to check when + * the socket can be closed. + */ + unsent = vsk->transport->unsent_bytes; + if (!unsent) + return; + + add_wait_queue(sk_sleep(sk), &wait); + + do { + if (sk_wait_event(sk, &timeout, unsent(vsk) == 0, &wait)) + break; + } while (!signal_pending(current) && timeout); + + remove_wait_queue(sk_sleep(sk), &wait); +} +EXPORT_SYMBOL_GPL(vsock_linger); + static int vsock_shutdown(struct socket *sock, int mode) { int err; diff --git a/net/vmw_vsock/virtio_transport_common.c b/net/vmw_vsock/virtio_transport_common.c index f2f1b166731b..7897fd970dd8 100644 --- a/net/vmw_vsock/virtio_transport_common.c +++ b/net/vmw_vsock/virtio_transport_common.c @@ -1191,25 +1191,6 @@ static void virtio_transport_remove_sock(struct vsock_sock *vsk) vsock_remove_sock(vsk); } -static void virtio_transport_wait_close(struct sock *sk, long timeout) -{ - if (timeout) { - DEFINE_WAIT_FUNC(wait, woken_wake_function); - struct vsock_sock *vsk = vsock_sk(sk); - - add_wait_queue(sk_sleep(sk), &wait); - - do { - if (sk_wait_event(sk, &timeout, - virtio_transport_unsent_bytes(vsk) == 0, - &wait)) - break; - } while (!signal_pending(current) && timeout); - - remove_wait_queue(sk_sleep(sk), &wait); - } -} - static void virtio_transport_cancel_close_work(struct vsock_sock *vsk, bool cancel_timeout) { @@ -1279,8 +1260,8 @@ static bool virtio_transport_close(struct vsock_sock *vsk) if ((sk->sk_shutdown & SHUTDOWN_MASK) != SHUTDOWN_MASK) (void)virtio_transport_shutdown(vsk, SHUTDOWN_MASK); - if (sock_flag(sk, SOCK_LINGER) && !(current->flags & PF_EXITING)) - virtio_transport_wait_close(sk, sk->sk_lingertime); + if (!(current->flags & PF_EXITING)) + vsock_linger(sk); if (sock_flag(sk, SOCK_DONE)) { return true; From e78e0596c762609ee5a92bd9d38351694b52f249 Mon Sep 17 00:00:00 2001 From: Michal Luczaj Date: Thu, 22 May 2025 01:18:23 +0200 Subject: [PATCH 3/5] vsock/test: Introduce vsock_wait_sent() helper Distill the virtio_vsock_sock::bytes_unsent checking loop (ioctl SIOCOUTQ) and move it to utils. Tweak the comment. Signed-off-by: Michal Luczaj Reviewed-by: Stefano Garzarella Link: https://patch.msgid.link/20250522-vsock-linger-v6-3-2ad00b0e447e@rbox.co Signed-off-by: Paolo Abeni --- tools/testing/vsock/util.c | 25 +++++++++++++++++++++++++ tools/testing/vsock/util.h | 1 + tools/testing/vsock/vsock_test.c | 23 ++++++----------------- 3 files changed, 32 insertions(+), 17 deletions(-) diff --git a/tools/testing/vsock/util.c b/tools/testing/vsock/util.c index de25892f865f..4427d459e199 100644 --- a/tools/testing/vsock/util.c +++ b/tools/testing/vsock/util.c @@ -17,6 +17,7 @@ #include #include #include +#include #include "timeout.h" #include "control.h" @@ -96,6 +97,30 @@ void vsock_wait_remote_close(int fd) close(epollfd); } +/* Wait until transport reports no data left to be sent. + * Return false if transport does not implement the unsent_bytes() callback. + */ +bool vsock_wait_sent(int fd) +{ + int ret, sock_bytes_unsent; + + timeout_begin(TIMEOUT); + do { + ret = ioctl(fd, SIOCOUTQ, &sock_bytes_unsent); + if (ret < 0) { + if (errno == EOPNOTSUPP) + break; + + perror("ioctl(SIOCOUTQ)"); + exit(EXIT_FAILURE); + } + timeout_check("SIOCOUTQ"); + } while (sock_bytes_unsent != 0); + timeout_end(); + + return !ret; +} + /* Create socket , bind to and return the file descriptor. */ int vsock_bind(unsigned int cid, unsigned int port, int type) { diff --git a/tools/testing/vsock/util.h b/tools/testing/vsock/util.h index d1f765ce3eee..91f9df12f26a 100644 --- a/tools/testing/vsock/util.h +++ b/tools/testing/vsock/util.h @@ -54,6 +54,7 @@ int vsock_stream_listen(unsigned int cid, unsigned int port); int vsock_seqpacket_accept(unsigned int cid, unsigned int port, struct sockaddr_vm *clientaddrp); void vsock_wait_remote_close(int fd); +bool vsock_wait_sent(int fd); void send_buf(int fd, const void *buf, size_t len, int flags, ssize_t expected_ret); void recv_buf(int fd, void *buf, size_t len, int flags, ssize_t expected_ret); diff --git a/tools/testing/vsock/vsock_test.c b/tools/testing/vsock/vsock_test.c index 9ea33b78b9fc..9d3a77be26f4 100644 --- a/tools/testing/vsock/vsock_test.c +++ b/tools/testing/vsock/vsock_test.c @@ -21,7 +21,6 @@ #include #include #include -#include #include #include "vsock_test_zerocopy.h" @@ -1280,7 +1279,7 @@ static void test_unsent_bytes_server(const struct test_opts *opts, int type) static void test_unsent_bytes_client(const struct test_opts *opts, int type) { unsigned char buf[MSG_BUF_IOCTL_LEN]; - int ret, fd, sock_bytes_unsent; + int fd; fd = vsock_connect(opts->peer_cid, opts->peer_port, type); if (fd < 0) { @@ -1297,22 +1296,12 @@ static void test_unsent_bytes_client(const struct test_opts *opts, int type) /* SIOCOUTQ isn't guaranteed to instantly track sent data. Even though * the "RECEIVED" message means that the other side has received the * data, there can be a delay in our kernel before updating the "unsent - * bytes" counter. Repeat SIOCOUTQ until it returns 0. + * bytes" counter. vsock_wait_sent() will repeat SIOCOUTQ until it + * returns 0. */ - timeout_begin(TIMEOUT); - do { - ret = ioctl(fd, SIOCOUTQ, &sock_bytes_unsent); - if (ret < 0) { - if (errno == EOPNOTSUPP) { - fprintf(stderr, "Test skipped, SIOCOUTQ not supported.\n"); - break; - } - perror("ioctl"); - exit(EXIT_FAILURE); - } - timeout_check("SIOCOUTQ"); - } while (sock_bytes_unsent != 0); - timeout_end(); + if (!vsock_wait_sent(fd)) + fprintf(stderr, "Test skipped, SIOCOUTQ not supported.\n"); + close(fd); } From 8b07b7e5c253981ccbab2f2506e07f5ef1082181 Mon Sep 17 00:00:00 2001 From: Michal Luczaj Date: Thu, 22 May 2025 01:18:24 +0200 Subject: [PATCH 4/5] vsock/test: Introduce enable_so_linger() helper Add a helper function that sets SO_LINGER. Adapt the caller. Signed-off-by: Michal Luczaj Reviewed-by: Stefano Garzarella Link: https://patch.msgid.link/20250522-vsock-linger-v6-4-2ad00b0e447e@rbox.co Signed-off-by: Paolo Abeni --- tools/testing/vsock/util.c | 13 +++++++++++++ tools/testing/vsock/util.h | 1 + tools/testing/vsock/vsock_test.c | 10 +--------- 3 files changed, 15 insertions(+), 9 deletions(-) diff --git a/tools/testing/vsock/util.c b/tools/testing/vsock/util.c index 4427d459e199..0c7e9cbcbc85 100644 --- a/tools/testing/vsock/util.c +++ b/tools/testing/vsock/util.c @@ -823,3 +823,16 @@ void enable_so_zerocopy_check(int fd) setsockopt_int_check(fd, SOL_SOCKET, SO_ZEROCOPY, 1, "setsockopt SO_ZEROCOPY"); } + +void enable_so_linger(int fd, int timeout) +{ + struct linger optval = { + .l_onoff = 1, + .l_linger = timeout + }; + + if (setsockopt(fd, SOL_SOCKET, SO_LINGER, &optval, sizeof(optval))) { + perror("setsockopt(SO_LINGER)"); + exit(EXIT_FAILURE); + } +} diff --git a/tools/testing/vsock/util.h b/tools/testing/vsock/util.h index 91f9df12f26a..5e2db67072d5 100644 --- a/tools/testing/vsock/util.h +++ b/tools/testing/vsock/util.h @@ -80,4 +80,5 @@ void setsockopt_int_check(int fd, int level, int optname, int val, void setsockopt_timeval_check(int fd, int level, int optname, struct timeval val, char const *errmsg); void enable_so_zerocopy_check(int fd); +void enable_so_linger(int fd, int timeout); #endif /* UTIL_H */ diff --git a/tools/testing/vsock/vsock_test.c b/tools/testing/vsock/vsock_test.c index 9d3a77be26f4..b3258d6ba21a 100644 --- a/tools/testing/vsock/vsock_test.c +++ b/tools/testing/vsock/vsock_test.c @@ -1813,10 +1813,6 @@ static void test_stream_connect_retry_server(const struct test_opts *opts) static void test_stream_linger_client(const struct test_opts *opts) { - struct linger optval = { - .l_onoff = 1, - .l_linger = 1 - }; int fd; fd = vsock_stream_connect(opts->peer_cid, opts->peer_port); @@ -1825,11 +1821,7 @@ static void test_stream_linger_client(const struct test_opts *opts) exit(EXIT_FAILURE); } - if (setsockopt(fd, SOL_SOCKET, SO_LINGER, &optval, sizeof(optval))) { - perror("setsockopt(SO_LINGER)"); - exit(EXIT_FAILURE); - } - + enable_so_linger(fd, 1); close(fd); } From 393d070135ad01f954ca8289c3ee134950e2d0c8 Mon Sep 17 00:00:00 2001 From: Michal Luczaj Date: Thu, 22 May 2025 01:18:25 +0200 Subject: [PATCH 5/5] vsock/test: Add test for an unexpectedly lingering close() There was an issue with SO_LINGER: instead of blocking until all queued messages for the socket have been successfully sent (or the linger timeout has been reached), close() would block until packets were handled by the peer. Add a test to alert on close() lingering when it should not. Signed-off-by: Michal Luczaj Reviewed-by: Stefano Garzarella Link: https://patch.msgid.link/20250522-vsock-linger-v6-5-2ad00b0e447e@rbox.co Signed-off-by: Paolo Abeni --- tools/testing/vsock/vsock_test.c | 52 ++++++++++++++++++++++++++++++++ 1 file changed, 52 insertions(+) diff --git a/tools/testing/vsock/vsock_test.c b/tools/testing/vsock/vsock_test.c index b3258d6ba21a..f669baaa0dca 100644 --- a/tools/testing/vsock/vsock_test.c +++ b/tools/testing/vsock/vsock_test.c @@ -1839,6 +1839,53 @@ static void test_stream_linger_server(const struct test_opts *opts) close(fd); } +/* Half of the default to not risk timing out the control channel */ +#define LINGER_TIMEOUT (TIMEOUT / 2) + +static void test_stream_nolinger_client(const struct test_opts *opts) +{ + bool waited; + time_t ns; + int fd; + + fd = vsock_stream_connect(opts->peer_cid, opts->peer_port); + if (fd < 0) { + perror("connect"); + exit(EXIT_FAILURE); + } + + enable_so_linger(fd, LINGER_TIMEOUT); + send_byte(fd, 1, 0); /* Left unread to expose incorrect behaviour. */ + waited = vsock_wait_sent(fd); + + ns = current_nsec(); + close(fd); + ns = current_nsec() - ns; + + if (!waited) { + fprintf(stderr, "Test skipped, SIOCOUTQ not supported.\n"); + } else if (DIV_ROUND_UP(ns, NSEC_PER_SEC) >= LINGER_TIMEOUT) { + fprintf(stderr, "Unexpected lingering\n"); + exit(EXIT_FAILURE); + } + + control_writeln("DONE"); +} + +static void test_stream_nolinger_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); + } + + control_expectln("DONE"); + close(fd); +} + static struct test_case test_cases[] = { { .name = "SOCK_STREAM connection reset", @@ -1999,6 +2046,11 @@ static struct test_case test_cases[] = { .run_client = test_stream_linger_client, .run_server = test_stream_linger_server, }, + { + .name = "SOCK_STREAM SO_LINGER close() on unread", + .run_client = test_stream_nolinger_client, + .run_server = test_stream_nolinger_server, + }, {}, };