Re: [RFC PATCH v1 1/3] test/vsock: rework message bound test
From: Stefano Garzarella
Date: Wed Nov 23 2022 - 10:25:58 EST
On Mon, Nov 21, 2022 at 04:49:23PM +0000, Arseniy Krasnov wrote:
On 21.11.2022 17:46, Stefano Garzarella wrote:
On Tue, Nov 15, 2022 at 08:50:36PM +0000, Arseniy Krasnov wrote:
This updates message bound test making it more complex. Instead of
sending 1 bytes messages with one MSG_EOR bit, it sends messages of
random length(one half of messages are smaller than page size, second
half are bigger) with random number of MSG_EOR bits set. Receiver
also don't know total number of messages.
Signed-off-by: Arseniy Krasnov <AVKrasnov@xxxxxxxxxxxxxx>
---
tools/testing/vsock/control.c | 34 +++++++++
tools/testing/vsock/control.h | 2 +
tools/testing/vsock/util.c | 13 ++++
tools/testing/vsock/util.h | 1 +
tools/testing/vsock/vsock_test.c | 115 +++++++++++++++++++++++++++----
5 files changed, 152 insertions(+), 13 deletions(-)
diff --git a/tools/testing/vsock/control.c b/tools/testing/vsock/control.c
index 4874872fc5a3..bed1649bdf3d 100644
--- a/tools/testing/vsock/control.c
+++ b/tools/testing/vsock/control.c
@@ -141,6 +141,40 @@ void control_writeln(const char *str)
timeout_end();
}
+void control_writeulong(unsigned long value)
+{
+ char str[32];
+
+ if (snprintf(str, sizeof(str), "%lu", value) >= sizeof(str)) {
+ perror("snprintf");
+ exit(EXIT_FAILURE);
+ }
+
+ control_writeln(str);
+}
+
+unsigned long control_readulong(bool *ok)
+{
+ unsigned long value;
+ char *str;
+
+ if (ok)
+ *ok = false;
+
+ str = control_readln();
+
+ if (str == NULL)
checkpatch suggests to use !str
+ return 0;
Maybe we can just call exit(EXIT_FAILURE) here and remove the `ok`
parameter, since I'm not sure we can recover from this error.
+
+ value = strtoul(str, NULL, 10);
+ free(str);
+
+ if (ok)
+ *ok = true;
+
+ return value;
+}
+
/* Return the next line from the control socket (without the trailing newline).
*
* The program terminates if a timeout occurs.
diff --git a/tools/testing/vsock/control.h b/tools/testing/vsock/control.h
index 51814b4f9ac1..cdd922dfea68 100644
--- a/tools/testing/vsock/control.h
+++ b/tools/testing/vsock/control.h
@@ -9,7 +9,9 @@ void control_init(const char *control_host, const char *control_port,
void control_cleanup(void);
void control_writeln(const char *str);
char *control_readln(void);
+unsigned long control_readulong(bool *ok);
void control_expectln(const char *str);
bool control_cmpln(char *line, const char *str, bool fail);
+void control_writeulong(unsigned long value);
#endif /* CONTROL_H */
diff --git a/tools/testing/vsock/util.c b/tools/testing/vsock/util.c
index 2acbb7703c6a..351903836774 100644
--- a/tools/testing/vsock/util.c
+++ b/tools/testing/vsock/util.c
@@ -395,3 +395,16 @@ void skip_test(struct test_case *test_cases, size_t test_cases_len,
test_cases[test_id].skip = true;
}
+
+unsigned long djb2(const void *data, size_t len)
I would add hash_ as a prefix (or suffix).
+{
+ unsigned long hash = 5381;
+ int i = 0;
+
+ while (i < len) {
+ hash = ((hash << 5) + hash) + ((unsigned char *)data)[i];
+ i++;
+ }
+
+ return hash;
+}
diff --git a/tools/testing/vsock/util.h b/tools/testing/vsock/util.h
index a3375ad2fb7f..988cc69a4642 100644
--- a/tools/testing/vsock/util.h
+++ b/tools/testing/vsock/util.h
@@ -49,4 +49,5 @@ void run_tests(const struct test_case *test_cases,
void list_tests(const struct test_case *test_cases);
void skip_test(struct test_case *test_cases, size_t test_cases_len,
const char *test_id_str);
+unsigned long djb2(const void *data, size_t len);
#endif /* UTIL_H */
diff --git a/tools/testing/vsock/vsock_test.c b/tools/testing/vsock/vsock_test.c
index bb6d691cb30d..107c11165887 100644
--- a/tools/testing/vsock/vsock_test.c
+++ b/tools/testing/vsock/vsock_test.c
@@ -284,10 +284,14 @@ static void test_stream_msg_peek_server(const struct test_opts *opts)
close(fd);
}
-#define MESSAGES_CNT 7
-#define MSG_EOR_IDX (MESSAGES_CNT / 2)
+#define SOCK_BUF_SIZE (2 * 1024 * 1024)
+#define MAX_MSG_SIZE (32 * 1024)
+
static void test_seqpacket_msg_bounds_client(const struct test_opts *opts)
{
+ unsigned long curr_hash;
+ int page_size;
+ int msg_count;
int fd;
fd = vsock_seqpacket_connect(opts->peer_cid, 1234);
@@ -296,18 +300,69 @@ static void test_seqpacket_msg_bounds_client(const struct test_opts *opts)
exit(EXIT_FAILURE);
}
- /* Send several messages, one with MSG_EOR flag */
- for (int i = 0; i < MESSAGES_CNT; i++)
- send_byte(fd, 1, (i == MSG_EOR_IDX) ? MSG_EOR : 0);
+ /* Wait, until receiver sets buffer size. */
+ control_expectln("SRVREADY");
+
+ curr_hash = 0;
+ page_size = getpagesize();
+ msg_count = SOCK_BUF_SIZE / MAX_MSG_SIZE;
+
+ for (int i = 0; i < msg_count; i++) {
+ ssize_t send_size;
+ size_t buf_size;
+ int flags;
+ void *buf;
+
+ /* Use "small" buffers and "big" buffers. */
+ if (i & 1)
+ buf_size = page_size +
+ (rand() % (MAX_MSG_SIZE - page_size));
+ else
+ buf_size = 1 + (rand() % page_size);
+
+ buf = malloc(buf_size);
+
+ if (!buf) {
+ perror("malloc");
+ exit(EXIT_FAILURE);
+ }
+
+ /* Set at least one MSG_EOR + some random. */
+ if (i == (msg_count / 2) || (rand() & 1)) {
+ flags = MSG_EOR;
+ curr_hash++;
+ } else {
+ flags = 0;
+ }
+
+ send_size = send(fd, buf, buf_size, flags);
+
+ if (send_size < 0) {
+ perror("send");
+ exit(EXIT_FAILURE);
+ }
+
+ if (send_size != buf_size) {
+ fprintf(stderr, "Invalid send size\n");
+ exit(EXIT_FAILURE);
+ }
+
+ curr_hash += send_size;
+ curr_hash = djb2(&curr_hash, sizeof(curr_hash));
+ }
control_writeln("SENDDONE");
+ control_writeulong(curr_hash);
Why do we need to hash the size?
Maybe we can send it without making the hash, anyway even if it wraps,
it should wrap the same way in both the server and the client.
(Or maybe we can use uin32_t or uint64_t to make sure both were
using 4 or 8 bytes)
Hello, thanks for review. I think if we will use sum of message size(IIUC), in most
paranoic case it won't guarantee message bounds control: single 4 bytes message
could be read as 4 x 1 byte message(IIUC of course). Idea of hashing is simple:
every iteration we do current_hash = hash(previous_hash + size of current message);
I think this is more reliable and protects from case described above.
Okay, now I understand what it is for and agree that using hash is
better.
Please add a comment to explain it.
All other comments - ack.
Thanks,
Stefano