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 <[email protected]>
Signed-off-by: Luigi Leonardi <[email protected]>
---
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
