Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::3:3 as permitted sender) client-ip=2620:137:e000::3:3;
Date:   Wed, 22 Nov 2023 10:16:14 +0100
From:   Stefano Garzarella <sgarzare@redhat.com>
To:     Arseniy Krasnov <avkrasnov@salutedevices.com>
Cc:     Stefan Hajnoczi <stefanha@redhat.com>,
        "David S. Miller" <davem@davemloft.net>,
        Eric Dumazet <edumazet@google.com>,
        Jakub Kicinski <kuba@kernel.org>,
        Paolo Abeni <pabeni@redhat.com>,
        "Michael S. Tsirkin" <mst@redhat.com>,
        Jason Wang <jasowang@redhat.com>,
        Bobby Eshleman <bobby.eshleman@bytedance.com>,
        kvm@vger.kernel.org, virtualization@lists.linux-foundation.org,
        netdev@vger.kernel.org, linux-kernel@vger.kernel.org,
        kernel@sberdevices.ru, oxffffaa@gmail.com,
        Bogdan Marcynkov <bmarcynk@redhat.com>
Subject: Re: [PATCH net v1] vsock/test: fix SEQPACKET message bounds test
Message-ID: <zoq32fkokk2ygtiabxgf74xu6vkfdynrlfzdqguh67qlogzd7j@qfd57sgudzpw>
References: <20231121211642.163474-1-avkrasnov@salutedevices.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii; format=flowed
Content-Disposition: inline
In-Reply-To: <20231121211642.163474-1-avkrasnov@salutedevices.com>
Precedence: bulk

On Wed, Nov 22, 2023 at 12:16:42AM +0300, Arseniy Krasnov wrote:
>Tune message length calculation to make this test work on machines
>where 'getpagesize()' returns >32KB. Now maximum message length is not
>hardcoded (on machines above it was smaller than 'getpagesize()' return
>value, thus we get negative value and test fails), but calculated at
>runtime and always bigger than 'getpagesize()' result. Reproduced on
>aarch64 with 64KB page size.

It was reported to me by Bogdan, so we can add:

Reported-by: Bogdan Marcynkov <bmarcynk@redhat.com>

>
>Fixes: 5c338112e48a ("test/vsock: rework message bounds test")
>Signed-off-by: Arseniy Krasnov <avkrasnov@salutedevices.com>
>---
> tools/testing/vsock/vsock_test.c | 19 +++++++++++++------
> 1 file changed, 13 insertions(+), 6 deletions(-)

The fix LGTM and it worked on aarch64 machine.

Reviewed-by: Stefano Garzarella <sgarzare@redhat.com>

Thanks for the fast fix!
Stefano

>
>diff --git a/tools/testing/vsock/vsock_test.c b/tools/testing/vsock/vsock_test.c
>index f5623b8d76b7..691e44c746bf 100644
>--- a/tools/testing/vsock/vsock_test.c
>+++ b/tools/testing/vsock/vsock_test.c
>@@ -353,11 +353,12 @@ static void test_stream_msg_peek_server(const struct test_opts *opts)
> }
>
> #define SOCK_BUF_SIZE (2 * 1024 * 1024)
>-#define MAX_MSG_SIZE (32 * 1024)
>+#define MAX_MSG_PAGES 4
>
> static void test_seqpacket_msg_bounds_client(const struct test_opts *opts)
> {
> 	unsigned long curr_hash;
>+	size_t max_msg_size;
> 	int page_size;
> 	int msg_count;
> 	int fd;
>@@ -373,7 +374,8 @@ static void test_seqpacket_msg_bounds_client(const struct test_opts *opts)
>
> 	curr_hash = 0;
> 	page_size = getpagesize();
>-	msg_count = SOCK_BUF_SIZE / MAX_MSG_SIZE;
>+	max_msg_size = MAX_MSG_PAGES * page_size;
>+	msg_count = SOCK_BUF_SIZE / max_msg_size;
>
> 	for (int i = 0; i < msg_count; i++) {
> 		size_t buf_size;
>@@ -383,7 +385,7 @@ static void test_seqpacket_msg_bounds_client(const struct test_opts *opts)
> 		/* Use "small" buffers and "big" buffers. */
> 		if (i & 1)
> 			buf_size = page_size +
>-					(rand() % (MAX_MSG_SIZE - page_size));
>+					(rand() % (max_msg_size - page_size));
> 		else
> 			buf_size = 1 + (rand() % page_size);
>
>@@ -429,7 +431,6 @@ static void test_seqpacket_msg_bounds_server(const struct test_opts *opts)
> 	unsigned long remote_hash;
> 	unsigned long curr_hash;
> 	int fd;
>-	char buf[MAX_MSG_SIZE];
> 	struct msghdr msg = {0};
> 	struct iovec iov = {0};
>
>@@ -457,8 +458,13 @@ static void test_seqpacket_msg_bounds_server(const struct test_opts *opts)
> 	control_writeln("SRVREADY");
> 	/* Wait, until peer sends whole data. */
> 	control_expectln("SENDDONE");
>-	iov.iov_base = buf;
>-	iov.iov_len = sizeof(buf);
>+	iov.iov_len = MAX_MSG_PAGES * getpagesize();
>+	iov.iov_base = malloc(iov.iov_len);
>+	if (!iov.iov_base) {
>+		perror("malloc");
>+		exit(EXIT_FAILURE);
>+	}
>+
> 	msg.msg_iov = &iov;
> 	msg.msg_iovlen = 1;
>
>@@ -483,6 +489,7 @@ static void test_seqpacket_msg_bounds_server(const struct test_opts *opts)
> 		curr_hash += hash_djb2(msg.msg_iov[0].iov_base, recv_size);
> 	}
>
>+	free(iov.iov_base);
> 	close(fd);
> 	remote_hash = control_readulong();
>
>-- 
>2.25.1
>