Re: [PATCH net v2 2/3] vsock/test: handle MSG_PEEK in `recv_buf`

[Stefano Garzarella]

On Tue, Apr 07, 2026 at 11:13:56AM +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.
>
> This allows us to simplify the `test_stream_credit_update_test`, by
> reusing `recv_buf`.
>
> Suggested-by: Stefano Garzarella <sgarzare@xxxxxxxxxx>
> Signed-off-by: Luigi Leonardi <leonardi@xxxxxxxxxx>
> ---
> tools/testing/vsock/util.c       |  8 ++++++++
> tools/testing/vsock/vsock_test.c | 13 +------------
> 2 files changed, 9 insertions(+), 12 deletions(-)
>
> diff --git a/tools/testing/vsock/util.c b/tools/testing/vsock/util.c
> index 9430ef5b8bc3..f12425ca99ed 100644
> --- a/tools/testing/vsock/util.c
> +++ b/tools/testing/vsock/util.c
> @@ -399,6 +399,14 @@ void recv_buf(int fd, void *buf, size_t len, int flags, ssize_t expected_ret)
> 		if (ret == 0 || (ret < 0 && errno != EINTR))
> 			break;

We should add a comment here or even better in the documentation on top 
of the function to explain better this behaviour for future reference.

> +		if (flags & MSG_PEEK) {
> +			if (ret == expected_ret) {
> +				nread = ret;
> +				break;
> +			}

Not that I expect this to happen often, but I wonder if it would be 
better to add a `timeout_usleep()` here to avoid using up too many CPU 
cycles.

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

Not sure about this change in this patch, but since they are just tests 
it could be fine.

mmm, on a second thought, we already used recv_buf() with MSG_PEEK in 
several other tests, so yep, better to add this change, but I'd make 
more clear in the commit title/description that this is at the end a fix 
for other tests using MSG_PEEK with recv_buf(), I mean something like 
this:

    vsock/test: fix MSG_PEEK handling in recv_buf()

And also pointing out that other tests already used MSG_PEEK with 
recv_buf(), but it can be broken in some cases.

Thanks,
Stefano