Re: [PATCH v5 2/3] vsock/test: fix parameter types in SO_VM_SOCKETS_* calls

[Stefano Garzarella <sgarzare@xxxxxxxxxx>]

On Thu, Nov 07, 2024 at 07:17:25PM -0600, Konstantin Shkolnyy wrote:
> Change parameters of SO_VM_SOCKETS_* to unsigned long long as documented
> in the vm_sockets.h, because the corresponding kernel code requires them
> to be at least 64-bit, no matter what architecture. Otherwise they are
> too small on 32-bit machines.
>
> Fixes: 5c338112e48a ("test/vsock: rework message bounds test")
> Fixes: 685a21c314a8 ("test/vsock: add big message test")
> Fixes: 542e893fbadc ("vsock/test: two tests to check credit update logic")
> Fixes: 8abbffd27ced ("test/vsock: vsock_perf utility")
> Signed-off-by: Konstantin Shkolnyy <kshk@xxxxxxxxxxxxx>
> ---
> tools/testing/vsock/vsock_perf.c |  4 ++--
> tools/testing/vsock/vsock_test.c | 22 +++++++++++++++++-----
> 2 files changed, 19 insertions(+), 7 deletions(-)

Reviewed-by: Stefano Garzarella <sgarzare@xxxxxxxxxx>

> diff --git a/tools/testing/vsock/vsock_perf.c b/tools/testing/vsock/vsock_perf.c
> index 22633c2848cc..8e0a6c0770d3 100644
> --- a/tools/testing/vsock/vsock_perf.c
> +++ b/tools/testing/vsock/vsock_perf.c
> @@ -33,7 +33,7 @@
>
> static unsigned int port = DEFAULT_PORT;
> static unsigned long buf_size_bytes = DEFAULT_BUF_SIZE_BYTES;
> -static unsigned long vsock_buf_bytes = DEFAULT_VSOCK_BUF_BYTES;
> +static unsigned long long vsock_buf_bytes = DEFAULT_VSOCK_BUF_BYTES;
> static bool zerocopy;
>
> static void error(const char *s)
> @@ -162,7 +162,7 @@ static void run_receiver(int rcvlowat_bytes)
> 	printf("Run as receiver\n");
> 	printf("Listen port %u\n", port);
> 	printf("RX buffer %lu bytes\n", buf_size_bytes);
> -	printf("vsock buffer %lu bytes\n", vsock_buf_bytes);
> +	printf("vsock buffer %llu bytes\n", vsock_buf_bytes);
> 	printf("SO_RCVLOWAT %d bytes\n", rcvlowat_bytes);
>
> 	fd = socket(AF_VSOCK, SOCK_STREAM, 0);
> diff --git a/tools/testing/vsock/vsock_test.c b/tools/testing/vsock/vsock_test.c
> index 7fd25b814b4b..c7af23332e57 100644
> --- a/tools/testing/vsock/vsock_test.c
> +++ b/tools/testing/vsock/vsock_test.c
> @@ -429,7 +429,7 @@ static void test_seqpacket_msg_bounds_client(const struct test_opts *opts)
>
> static void test_seqpacket_msg_bounds_server(const struct test_opts *opts)
> {
> -	unsigned long sock_buf_size;
> +	unsigned long long sock_buf_size;
> 	unsigned long remote_hash;
> 	unsigned long curr_hash;
> 	int fd;
> @@ -634,7 +634,8 @@ static void test_seqpacket_timeout_server(const struct test_opts *opts)
>
> static void test_seqpacket_bigmsg_client(const struct test_opts *opts)
> {
> -	unsigned long sock_buf_size;
> +	unsigned long long sock_buf_size;
> +	size_t buf_size;
> 	socklen_t len;
> 	void *data;
> 	int fd;
> @@ -655,13 +656,20 @@ static void test_seqpacket_bigmsg_client(const struct test_opts *opts)
>
> 	sock_buf_size++;
>
> -	data = malloc(sock_buf_size);
> +	/* size_t can be < unsigned long long */
> +	buf_size = (size_t) sock_buf_size;
> +	if (buf_size != sock_buf_size) {
> +		fprintf(stderr, "Returned BUFFER_SIZE too large\n");
> +		exit(EXIT_FAILURE);
> +	}
> +
> +	data = malloc(buf_size);
> 	if (!data) {
> 		perror("malloc");
> 		exit(EXIT_FAILURE);
> 	}
>
> -	send_buf(fd, data, sock_buf_size, 0, -EMSGSIZE);
> +	send_buf(fd, data, buf_size, 0, -EMSGSIZE);
>
> 	control_writeln("CLISENT");
>
> @@ -1360,6 +1368,7 @@ static void test_stream_credit_update_test(const struct test_opts *opts,
> 	int recv_buf_size;
> 	struct pollfd fds;
> 	size_t buf_size;
> +	unsigned long long sock_buf_size;
> 	void *buf;
> 	int fd;
>
> @@ -1371,8 +1380,11 @@ static void test_stream_credit_update_test(const struct test_opts *opts,
>
> 	buf_size = RCVLOWAT_CREDIT_UPD_BUF_SIZE;
>
> +	/* size_t can be < unsigned long long */
> +	sock_buf_size = buf_size;
> +
> 	if (setsockopt(fd, AF_VSOCK, SO_VM_SOCKETS_BUFFER_SIZE,
> -		       &buf_size, sizeof(buf_size))) {
> +		       &sock_buf_size, sizeof(sock_buf_size))) {
> 		perror("setsockopt(SO_VM_SOCKETS_BUFFER_SIZE)");
> 		exit(EXIT_FAILURE);
> 	}
> --
> 2.34.1