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

[Stefano Garzarella <sgarzare@xxxxxxxxxx>]

On Tue, Oct 29, 2024 at 09:49:54AM -0500, Konstantin Shkolnyy wrote:
> Change parameters of SO_VM_SOCKETS_* to uint64_t so that they are always

In include/uapi/linux/vm_sockets.h we talk about "unsigned long long",
but in the kernel code we use u64. IIUC "unsigned long long" should be 
u64 on every architecture, at least till we will have some 128-bit cpu, 
right?

> 64-bit, because the corresponding kernel code requires them to be at 
> least
> that large, no matter what architecture.
>
> 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 |  2 +-
> tools/testing/vsock/vsock_test.c | 19 ++++++++++++++-----
> 2 files changed, 15 insertions(+), 6 deletions(-)
>
> diff --git a/tools/testing/vsock/vsock_perf.c b/tools/testing/vsock/vsock_perf.c
> index 22633c2848cc..88f6be4162a6 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 uint64_t vsock_buf_bytes = DEFAULT_VSOCK_BUF_BYTES;

What about using `unsigned long long` as documented in the vm_sockets.h?

Thanks,
Stefano

> static bool zerocopy;
>
> static void error(const char *s)
> diff --git a/tools/testing/vsock/vsock_test.c b/tools/testing/vsock/vsock_test.c
> index 7fd25b814b4b..49a32515886f 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;
> +	uint64_t 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;
> +	uint64_t sock_buf_size;
> +	size_t buf_size;
> 	socklen_t len;
> 	void *data;
> 	int fd;
> @@ -655,13 +656,19 @@ static void test_seqpacket_bigmsg_client(const struct test_opts *opts)
>
> 	sock_buf_size++;
>
> -	data = malloc(sock_buf_size);
> +	buf_size = (size_t) sock_buf_size; /* size_t can be < uint64_t */
> +	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 +1367,7 @@ static void test_stream_credit_update_test(const struct test_opts *opts,
> 	int recv_buf_size;
> 	struct pollfd fds;
> 	size_t buf_size;
> +	uint64_t sock_buf_size;
> 	void *buf;
> 	int fd;
>
> @@ -1370,9 +1378,10 @@ static void test_stream_credit_update_test(const struct test_opts *opts,
> 	}
>
> 	buf_size = RCVLOWAT_CREDIT_UPD_BUF_SIZE;
> +	sock_buf_size = buf_size; /* size_t can be < uint64_t */
>
> 	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