diff mbox series

[net,v1] vsock/test: fix SEQPACKET message bounds test

Message ID 20231121211642.163474-1-avkrasnov@salutedevices.com (mailing list archive)
State Accepted
Commit f0863888f6cfef33e3117dccfe94fa78edf76be4
Delegated to: Netdev Maintainers
Headers show
Series [net,v1] vsock/test: fix SEQPACKET message bounds test | expand

Checks

Context Check Description
netdev/series_format success Single patches do not need cover letters
netdev/codegen success Generated files up to date
netdev/tree_selection success Clearly marked for net
netdev/fixes_present success Fixes tag present in non-next series
netdev/header_inline success No static functions without inline keyword in header files
netdev/build_32bit success Errors and warnings before: 8 this patch: 8
netdev/cc_maintainers fail 1 blamed authors not CCed: AVKrasnov@sberdevices.ru; 2 maintainers not CCed: AVKrasnov@sberdevices.ru virtualization@lists.linux.dev
netdev/build_clang success Errors and warnings before: 8 this patch: 8
netdev/verify_signedoff success Signed-off-by tag matches author and committer
netdev/deprecated_api success None detected
netdev/check_selftest success No net selftest shell script
netdev/verify_fixes success Fixes tag looks correct
netdev/build_allmodconfig_warn success Errors and warnings before: 8 this patch: 8
netdev/checkpatch success total: 0 errors, 0 warnings, 0 checks, 59 lines checked
netdev/build_clang_rust success No Rust files in patch. Skipping build
netdev/kdoc success Errors and warnings before: 0 this patch: 0
netdev/source_inline success Was 0 now: 0

Commit Message

Arseniy Krasnov Nov. 21, 2023, 9:16 p.m. UTC
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.

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(-)

Comments

Stefano Garzarella Nov. 22, 2023, 9:16 a.m. UTC | #1
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
>
patchwork-bot+netdevbpf@kernel.org Nov. 23, 2023, 5 p.m. UTC | #2
Hello:

This patch was applied to netdev/net.git (main)
by Jakub Kicinski <kuba@kernel.org>:

On Wed, 22 Nov 2023 00:16:42 +0300 you 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.
> 
> [...]

Here is the summary with links:
  - [net,v1] vsock/test: fix SEQPACKET message bounds test
    https://git.kernel.org/netdev/net/c/f0863888f6cf

You are awesome, thank you!
diff mbox series

Patch

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