Re: [PATCH net v3 2/3] vsock/test: fix MSG_PEEK handling in recv_buf()

Stefano Garzarella
2026-04-15T11:31:38Z

On Tue, Apr 14, 2026 at 06:10:22PM +0200, Luigi Leonardi wrote:
>`recv_buf` does not handle the MSG_PEEK flag correctly: it keeps calling
>`recv` until all requested bytes are available or an error occurs.
>
>The problem is how it calculates the amount of bytes read: MSG_PEEK
>doesn't consume any bytes, will re-read the same bytes from the buffer
>head, so, summing the return value every time is wrong.
>
>Moreover, MSG_PEEK doesn't consume the bytes in the buffer, so if the
>requested amount is more than the bytes available, the loop will never
>terminate, because `recv` will never return EOF. For this reason we need
>to compare the amount of read bytes with the number of bytes expected.
>
>Add a check, and if the MSG_PEEK flag is present, update the counter of
>read bytes differently, and break if we read the expected amount.

nit: "..., update the counter for bytes read only after all expected
bytes have been read and break out of the loop; otherwise, try again
after a short delay to avoid consuming too many CPU cycles."

>
>This allows us to simplify the `test_stream_credit_update_test`, by
>reusing `recv_buf`, like some other tests already do.
>
>This also fixes callers that pass MSG_PEEK to recv_buf().

nit: this is implicit from the first part of the description.

>
>Suggested-by: Stefano Garzarella <sgarzare@redhat.com>
>Signed-off-by: Luigi Leonardi <leonardi@redhat.com>
>---
> tools/testing/vsock/util.c | 15 +++++++++++++++
> tools/testing/vsock/vsock_test.c | 13 +------------
> 2 files changed, 16 insertions(+), 12 deletions(-)
>
>diff --git a/tools/testing/vsock/util.c b/tools/testing/vsock/util.c
>index 1fe1338c79cd..2c9ee3210090 100644
>--- a/tools/testing/vsock/util.c
>+++ b/tools/testing/vsock/util.c
>@@ -381,7 +381,13 @@ void send_buf(int fd, const void *buf, size_t len, int flags,
> }
> }
>
>+#define RECV_PEEK_RETRY_USEC 10

10 usec IMO are a bit low, it could be the same order of the syscalls
involved in the loop, I'd go to some milliseconds like we do for
SEND_SLEEP_USEC.

>+
> /* Receive bytes in a buffer and check the return value.
>+ *
>+ * MSG_PEEK note: MSG_PEEK doesn't consume bytes from the buffer, so partial
>+ * reads cannot be summed. Instead, the function retries until recv() returns
>+ * exactly expected_ret bytes in a single call.

I'd replace with something like this:

* When MSG_PEEK is set, recv() is retried until it returns exactly
* expected_ret bytes. The function returns on error, EOF, or timeout
* as usual.

Thanks,
Stefano

> *
> * expected_ret:
> * <0 Negative errno (for testing errors)
>@@ -403,6 +409,15 @@ void recv_buf(int fd, void *buf, size_t len, int flags, ssize_t expected_ret)
> if (ret <= 0)
> break;
>
>+ if (flags & MSG_PEEK) {
>+ if (ret == expected_ret) {
>+ nread = ret;
>+ break;
>+ }
>+ timeout_usleep(RECV_PEEK_RETRY_USEC);
>+ continue;
>+ }
>+
> nread += ret;
> } while (nread < len);
> timeout_end();
>diff --git a/tools/testing/vsock/vsock_test.c b/tools/testing/vsock/vsock_test.c
>index 5bd20ccd9335..bdb0754965df 100644
>--- a/tools/testing/vsock/vsock_test.c
>+++ b/tools/testing/vsock/vsock_test.c
>@@ -1500,18 +1500,7 @@ static void test_stream_credit_update_test(const struct test_opts *opts,
> }
>
> /* Wait until there will be 128KB of data in rx queue. */
>- while (1) {
>- ssize_t res;
>-
>- res = recv(fd, buf, buf_size, MSG_PEEK);
>- if (res == buf_size)
>- break;
>-
>- if (res <= 0) {
>- fprintf(stderr, "unexpected 'recv()' return: %zi\n", res);
>- exit(EXIT_FAILURE);
>- }
>- }
>+ recv_buf(fd, buf, buf_size, MSG_PEEK, buf_size);
>
> /* There is 128KB of data in the socket's rx queue, dequeue first
> * 64KB, credit update is sent if 'low_rx_bytes_test' == true.
>
>--
>2.53.0
>