[PATCH net-next 0/5] vsock/test: add recv_buf()/send_buf() utility functions and some improvements

[PATCH net-next 0/5] vsock/test: add recv_buf()/send_buf() utility functions and some improvements

[Stefano Garzarella]

We recently found that some tests were failing [1].

The problem was that we were not waiting for all the bytes correctly,
so we had a partial read. I had initially suggested using MSG_WAITALL,
but this could have timeout problems.

Since we already had send_byte() and recv_byte() that handled the timeout,
but also the expected return value, I moved that code to two new functions
that we can now use to send/receive generic buffers.

The last commit is just an improvement to a test I found difficult to
understand while using the new functions.

@Arseniy a review and some testing are really appreciated :-)

[1] https://lore.kernel.org/netdev/63xflnwiohdfo6m3vnrrxgv2ulplencpwug5qqacugqh7xxpu3@tsczkuqgwurb/

Stefano Garzarella (5):
  vsock/test: add recv_buf() utility function
  vsock/test: use recv_buf() in vsock_test.c
  vsock/test: add send_buf() utility function
  vsock/test: use send_buf() in vsock_test.c
  vsock/test: track bytes in MSG_PEEK test for SOCK_SEQPACKET

 tools/testing/vsock/util.h       |   3 +
 tools/testing/vsock/util.c       | 124 ++++++++++++--------
 tools/testing/vsock/vsock_test.c | 187 ++++++-------------------------
 3 files changed, 117 insertions(+), 197 deletions(-)

-- 
2.41.0

[PATCH net-next 1/5] vsock/test: add recv_buf() utility function

[Stefano Garzarella]

Move the code of recv_byte() out in a new utility function that
can be used to receive a generic buffer.

This new function can be used when we need to receive a custom
buffer and not just a single 'A' byte.

Signed-off-by: Stefano Garzarella <sgarzare@redhat.com>
---
 tools/testing/vsock/util.h |  1 +
 tools/testing/vsock/util.c | 88 +++++++++++++++++++++++---------------
 2 files changed, 54 insertions(+), 35 deletions(-)

diff --git a/tools/testing/vsock/util.h b/tools/testing/vsock/util.h
index fb99208a95ea..fe31f267e67e 100644
--- a/tools/testing/vsock/util.h
+++ b/tools/testing/vsock/util.h
@@ -42,6 +42,7 @@ int vsock_stream_accept(unsigned int cid, unsigned int port,
 int vsock_seqpacket_accept(unsigned int cid, unsigned int port,
 			   struct sockaddr_vm *clientaddrp);
 void vsock_wait_remote_close(int fd);
+void recv_buf(int fd, void *buf, size_t len, int flags, ssize_t expected_ret);
 void send_byte(int fd, int expected_ret, int flags);
 void recv_byte(int fd, int expected_ret, int flags);
 void run_tests(const struct test_case *test_cases,
diff --git a/tools/testing/vsock/util.c b/tools/testing/vsock/util.c
index 01b636d3039a..2826902706e8 100644
--- a/tools/testing/vsock/util.c
+++ b/tools/testing/vsock/util.c
@@ -211,6 +211,58 @@ int vsock_seqpacket_accept(unsigned int cid, unsigned int port,
 	return vsock_accept(cid, port, clientaddrp, SOCK_SEQPACKET);
 }
 
+/* Receive bytes in a buffer and check the return value.
+ *
+ * expected_ret:
+ *  <0 Negative errno (for testing errors)
+ *   0 End-of-file
+ *  >0 Success (bytes successfully read)
+ */
+void recv_buf(int fd, void *buf, size_t len, int flags, ssize_t expected_ret)
+{
+	ssize_t nread = 0;
+	ssize_t ret;
+
+	timeout_begin(TIMEOUT);
+	do {
+		ret = recv(fd, buf + nread, len - nread, flags);
+		timeout_check("recv");
+
+		if (ret == 0 || (ret < 0 && errno != EINTR))
+			break;
+
+		nread += ret;
+	} while (nread < len);
+	timeout_end();
+
+	if (expected_ret < 0) {
+		if (ret != -1) {
+			fprintf(stderr, "bogus recv(2) return value %zd (expected %zd)\n",
+				ret, expected_ret);
+			exit(EXIT_FAILURE);
+		}
+		if (errno != -expected_ret) {
+			perror("recv");
+			exit(EXIT_FAILURE);
+		}
+		return;
+	}
+
+	if (ret < 0) {
+		perror("recv");
+		exit(EXIT_FAILURE);
+	}
+
+	if (nread != expected_ret) {
+		if (ret == 0)
+			fprintf(stderr, "unexpected EOF while receiving bytes\n");
+
+		fprintf(stderr, "bogus recv(2) bytes read %zd (expected %zd)\n",
+			nread, expected_ret);
+		exit(EXIT_FAILURE);
+	}
+}
+
 /* Transmit one byte and check the return value.
  *
  * expected_ret:
@@ -270,43 +322,9 @@ void send_byte(int fd, int expected_ret, int flags)
 void recv_byte(int fd, int expected_ret, int flags)
 {
 	uint8_t byte;
-	ssize_t nread;
-
-	timeout_begin(TIMEOUT);
-	do {
-		nread = recv(fd, &byte, sizeof(byte), flags);
-		timeout_check("read");
-	} while (nread < 0 && errno == EINTR);
-	timeout_end();
-
-	if (expected_ret < 0) {
-		if (nread != -1) {
-			fprintf(stderr, "bogus recv(2) return value %zd\n",
-				nread);
-			exit(EXIT_FAILURE);
-		}
-		if (errno != -expected_ret) {
-			perror("read");
-			exit(EXIT_FAILURE);
-		}
-		return;
-	}
 
-	if (nread < 0) {
-		perror("read");
-		exit(EXIT_FAILURE);
-	}
-	if (nread == 0) {
-		if (expected_ret == 0)
-			return;
+	recv_buf(fd, &byte, sizeof(byte), flags, expected_ret);
 
-		fprintf(stderr, "unexpected EOF while receiving byte\n");
-		exit(EXIT_FAILURE);
-	}
-	if (nread != sizeof(byte)) {
-		fprintf(stderr, "bogus recv(2) return value %zd\n", nread);
-		exit(EXIT_FAILURE);
-	}
 	if (byte != 'A') {
 		fprintf(stderr, "unexpected byte read %c\n", byte);
 		exit(EXIT_FAILURE);
-- 
2.41.0

Re: [PATCH net-next 1/5] vsock/test: add recv_buf() utility function

[Arseniy Krasnov]

On 15.09.2023 15:14, Stefano Garzarella wrote:
> Move the code of recv_byte() out in a new utility function that
> can be used to receive a generic buffer.
> 
> This new function can be used when we need to receive a custom
> buffer and not just a single 'A' byte.
> 
> Signed-off-by: Stefano Garzarella <sgarzare@redhat.com>
> ---
>  tools/testing/vsock/util.h |  1 +
>  tools/testing/vsock/util.c | 88 +++++++++++++++++++++++---------------
>  2 files changed, 54 insertions(+), 35 deletions(-)

Reviewed-by: Arseniy Krasnov <avkrasnov@salutedevices.com>

> 
> diff --git a/tools/testing/vsock/util.h b/tools/testing/vsock/util.h
> index fb99208a95ea..fe31f267e67e 100644
> --- a/tools/testing/vsock/util.h
> +++ b/tools/testing/vsock/util.h
> @@ -42,6 +42,7 @@ int vsock_stream_accept(unsigned int cid, unsigned int port,
>  int vsock_seqpacket_accept(unsigned int cid, unsigned int port,
>  			   struct sockaddr_vm *clientaddrp);
>  void vsock_wait_remote_close(int fd);
> +void recv_buf(int fd, void *buf, size_t len, int flags, ssize_t expected_ret);
>  void send_byte(int fd, int expected_ret, int flags);
>  void recv_byte(int fd, int expected_ret, int flags);
>  void run_tests(const struct test_case *test_cases,
> diff --git a/tools/testing/vsock/util.c b/tools/testing/vsock/util.c
> index 01b636d3039a..2826902706e8 100644
> --- a/tools/testing/vsock/util.c
> +++ b/tools/testing/vsock/util.c
> @@ -211,6 +211,58 @@ int vsock_seqpacket_accept(unsigned int cid, unsigned int port,
>  	return vsock_accept(cid, port, clientaddrp, SOCK_SEQPACKET);
>  }
>  
> +/* Receive bytes in a buffer and check the return value.
> + *
> + * expected_ret:
> + *  <0 Negative errno (for testing errors)
> + *   0 End-of-file
> + *  >0 Success (bytes successfully read)
> + */
> +void recv_buf(int fd, void *buf, size_t len, int flags, ssize_t expected_ret)
> +{
> +	ssize_t nread = 0;
> +	ssize_t ret;
> +
> +	timeout_begin(TIMEOUT);
> +	do {
> +		ret = recv(fd, buf + nread, len - nread, flags);
> +		timeout_check("recv");
> +
> +		if (ret == 0 || (ret < 0 && errno != EINTR))
> +			break;
> +
> +		nread += ret;
> +	} while (nread < len);
> +	timeout_end();
> +
> +	if (expected_ret < 0) {
> +		if (ret != -1) {
> +			fprintf(stderr, "bogus recv(2) return value %zd (expected %zd)\n",
> +				ret, expected_ret);
> +			exit(EXIT_FAILURE);
> +		}
> +		if (errno != -expected_ret) {
> +			perror("recv");
> +			exit(EXIT_FAILURE);
> +		}
> +		return;
> +	}
> +
> +	if (ret < 0) {
> +		perror("recv");
> +		exit(EXIT_FAILURE);
> +	}
> +
> +	if (nread != expected_ret) {
> +		if (ret == 0)
> +			fprintf(stderr, "unexpected EOF while receiving bytes\n");
> +
> +		fprintf(stderr, "bogus recv(2) bytes read %zd (expected %zd)\n",
> +			nread, expected_ret);
> +		exit(EXIT_FAILURE);
> +	}
> +}
> +
>  /* Transmit one byte and check the return value.
>   *
>   * expected_ret:
> @@ -270,43 +322,9 @@ void send_byte(int fd, int expected_ret, int flags)
>  void recv_byte(int fd, int expected_ret, int flags)
>  {
>  	uint8_t byte;
> -	ssize_t nread;
> -
> -	timeout_begin(TIMEOUT);
> -	do {
> -		nread = recv(fd, &byte, sizeof(byte), flags);
> -		timeout_check("read");
> -	} while (nread < 0 && errno == EINTR);
> -	timeout_end();
> -
> -	if (expected_ret < 0) {
> -		if (nread != -1) {
> -			fprintf(stderr, "bogus recv(2) return value %zd\n",
> -				nread);
> -			exit(EXIT_FAILURE);
> -		}
> -		if (errno != -expected_ret) {
> -			perror("read");
> -			exit(EXIT_FAILURE);
> -		}
> -		return;
> -	}
>  
> -	if (nread < 0) {
> -		perror("read");
> -		exit(EXIT_FAILURE);
> -	}
> -	if (nread == 0) {
> -		if (expected_ret == 0)
> -			return;
> +	recv_buf(fd, &byte, sizeof(byte), flags, expected_ret);
>  
> -		fprintf(stderr, "unexpected EOF while receiving byte\n");
> -		exit(EXIT_FAILURE);
> -	}
> -	if (nread != sizeof(byte)) {
> -		fprintf(stderr, "bogus recv(2) return value %zd\n", nread);
> -		exit(EXIT_FAILURE);
> -	}
>  	if (byte != 'A') {
>  		fprintf(stderr, "unexpected byte read %c\n", byte);
>  		exit(EXIT_FAILURE);

[PATCH net-next 2/5] vsock/test: use recv_buf() in vsock_test.c

[Stefano Garzarella]

We have a very common pattern used in vsock_test that we can
now replace with the new recv_buf().

This allows us to reuse the code we already had to check the
actual return value and wait for all bytes to be received with
an appropriate timeout.

Signed-off-by: Stefano Garzarella <sgarzare@redhat.com>
---
 tools/testing/vsock/vsock_test.c | 104 +++++--------------------------
 1 file changed, 17 insertions(+), 87 deletions(-)

diff --git a/tools/testing/vsock/vsock_test.c b/tools/testing/vsock/vsock_test.c
index 90718c2fd4ea..d1dcbaeb477a 100644
--- a/tools/testing/vsock/vsock_test.c
+++ b/tools/testing/vsock/vsock_test.c
@@ -301,7 +301,6 @@ static void test_msg_peek_server(const struct test_opts *opts,
 	unsigned char buf_half[MSG_PEEK_BUF_LEN / 2];
 	unsigned char buf_normal[MSG_PEEK_BUF_LEN];
 	unsigned char buf_peek[MSG_PEEK_BUF_LEN];
-	ssize_t res;
 	int fd;
 
 	if (seqpacket)
@@ -315,34 +314,16 @@ static void test_msg_peek_server(const struct test_opts *opts,
 	}
 
 	/* Peek from empty socket. */
-	res = recv(fd, buf_peek, sizeof(buf_peek), MSG_PEEK | MSG_DONTWAIT);
-	if (res != -1) {
-		fprintf(stderr, "expected recv(2) failure, got %zi\n", res);
-		exit(EXIT_FAILURE);
-	}
-
-	if (errno != EAGAIN) {
-		perror("EAGAIN expected");
-		exit(EXIT_FAILURE);
-	}
+	recv_buf(fd, buf_peek, sizeof(buf_peek), MSG_PEEK | MSG_DONTWAIT,
+		 -EAGAIN);
 
 	control_writeln("SRVREADY");
 
 	/* Peek part of data. */
-	res = recv(fd, buf_half, sizeof(buf_half), MSG_PEEK);
-	if (res != sizeof(buf_half)) {
-		fprintf(stderr, "recv(2) + MSG_PEEK, expected %zu, got %zi\n",
-			sizeof(buf_half), res);
-		exit(EXIT_FAILURE);
-	}
+	recv_buf(fd, buf_half, sizeof(buf_half), MSG_PEEK, sizeof(buf_half));
 
 	/* Peek whole data. */
-	res = recv(fd, buf_peek, sizeof(buf_peek), MSG_PEEK);
-	if (res != sizeof(buf_peek)) {
-		fprintf(stderr, "recv(2) + MSG_PEEK, expected %zu, got %zi\n",
-			sizeof(buf_peek), res);
-		exit(EXIT_FAILURE);
-	}
+	recv_buf(fd, buf_peek, sizeof(buf_peek), MSG_PEEK, sizeof(buf_peek));
 
 	/* Compare partial and full peek. */
 	if (memcmp(buf_half, buf_peek, sizeof(buf_half))) {
@@ -355,22 +336,11 @@ static void test_msg_peek_server(const struct test_opts *opts,
 		 * so check it with MSG_PEEK. We must get length
 		 * of the message.
 		 */
-		res = recv(fd, buf_half, sizeof(buf_half), MSG_PEEK |
-			   MSG_TRUNC);
-		if (res != sizeof(buf_peek)) {
-			fprintf(stderr,
-				"recv(2) + MSG_PEEK | MSG_TRUNC, exp %zu, got %zi\n",
-				sizeof(buf_half), res);
-			exit(EXIT_FAILURE);
-		}
+		recv_buf(fd, buf_half, sizeof(buf_half), MSG_PEEK | MSG_TRUNC,
+			 sizeof(buf_peek));
 	}
 
-	res = recv(fd, buf_normal, sizeof(buf_normal), 0);
-	if (res != sizeof(buf_normal)) {
-		fprintf(stderr, "recv(2), expected %zu, got %zi\n",
-			sizeof(buf_normal), res);
-		exit(EXIT_FAILURE);
-	}
+	recv_buf(fd, buf_normal, sizeof(buf_normal), 0, sizeof(buf_normal));
 
 	/* Compare full peek and normal read. */
 	if (memcmp(buf_peek, buf_normal, sizeof(buf_peek))) {
@@ -900,7 +870,6 @@ static void test_stream_poll_rcvlowat_client(const struct test_opts *opts)
 	unsigned long lowat_val = RCVLOWAT_BUF_SIZE;
 	char buf[RCVLOWAT_BUF_SIZE];
 	struct pollfd fds;
-	ssize_t read_res;
 	short poll_flags;
 	int fd;
 
@@ -955,12 +924,7 @@ static void test_stream_poll_rcvlowat_client(const struct test_opts *opts)
 	/* Use MSG_DONTWAIT, if call is going to wait, EAGAIN
 	 * will be returned.
 	 */
-	read_res = recv(fd, buf, sizeof(buf), MSG_DONTWAIT);
-	if (read_res != RCVLOWAT_BUF_SIZE) {
-		fprintf(stderr, "Unexpected recv result %zi\n",
-			read_res);
-		exit(EXIT_FAILURE);
-	}
+	recv_buf(fd, buf, sizeof(buf), MSG_DONTWAIT, RCVLOWAT_BUF_SIZE);
 
 	control_writeln("POLLDONE");
 
@@ -972,7 +936,7 @@ static void test_stream_poll_rcvlowat_client(const struct test_opts *opts)
 static void test_inv_buf_client(const struct test_opts *opts, bool stream)
 {
 	unsigned char data[INV_BUF_TEST_DATA_LEN] = {0};
-	ssize_t ret;
+	ssize_t expected_ret;
 	int fd;
 
 	if (stream)
@@ -988,39 +952,18 @@ static void test_inv_buf_client(const struct test_opts *opts, bool stream)
 	control_expectln("SENDDONE");
 
 	/* Use invalid buffer here. */
-	ret = recv(fd, NULL, sizeof(data), 0);
-	if (ret != -1) {
-		fprintf(stderr, "expected recv(2) failure, got %zi\n", ret);
-		exit(EXIT_FAILURE);
-	}
-
-	if (errno != EFAULT) {
-		fprintf(stderr, "unexpected recv(2) errno %d\n", errno);
-		exit(EXIT_FAILURE);
-	}
-
-	ret = recv(fd, data, sizeof(data), MSG_DONTWAIT);
+	recv_buf(fd, NULL, sizeof(data), 0, -EFAULT);
 
 	if (stream) {
 		/* For SOCK_STREAM we must continue reading. */
-		if (ret != sizeof(data)) {
-			fprintf(stderr, "expected recv(2) success, got %zi\n", ret);
-			exit(EXIT_FAILURE);
-		}
-		/* Don't check errno in case of success. */
+		expected_ret = sizeof(data);
 	} else {
 		/* For SOCK_SEQPACKET socket's queue must be empty. */
-		if (ret != -1) {
-			fprintf(stderr, "expected recv(2) failure, got %zi\n", ret);
-			exit(EXIT_FAILURE);
-		}
-
-		if (errno != EAGAIN) {
-			fprintf(stderr, "unexpected recv(2) errno %d\n", errno);
-			exit(EXIT_FAILURE);
-		}
+		expected_ret = -EAGAIN;
 	}
 
+	recv_buf(fd, data, sizeof(data), MSG_DONTWAIT, expected_ret);
+
 	control_writeln("DONE");
 
 	close(fd);
@@ -1117,7 +1060,6 @@ static void test_stream_virtio_skb_merge_client(const struct test_opts *opts)
 static void test_stream_virtio_skb_merge_server(const struct test_opts *opts)
 {
 	unsigned char buf[64];
-	ssize_t res;
 	int fd;
 
 	fd = vsock_stream_accept(VMADDR_CID_ANY, 1234, NULL);
@@ -1129,26 +1071,14 @@ static void test_stream_virtio_skb_merge_server(const struct test_opts *opts)
 	control_expectln("SEND0");
 
 	/* Read skbuff partially. */
-	res = recv(fd, buf, 2, 0);
-	if (res != 2) {
-		fprintf(stderr, "expected recv(2) returns 2 bytes, got %zi\n", res);
-		exit(EXIT_FAILURE);
-	}
+	recv_buf(fd, buf, 2, 0, 2);
 
 	control_writeln("REPLY0");
 	control_expectln("SEND1");
 
-	res = recv(fd, buf + 2, sizeof(buf) - 2, 0);
-	if (res != 8) {
-		fprintf(stderr, "expected recv(2) returns 8 bytes, got %zi\n", res);
-		exit(EXIT_FAILURE);
-	}
+	recv_buf(fd, buf + 2, 8, 0, 8);
 
-	res = recv(fd, buf, sizeof(buf) - 8 - 2, MSG_DONTWAIT);
-	if (res != -1) {
-		fprintf(stderr, "expected recv(2) failure, got %zi\n", res);
-		exit(EXIT_FAILURE);
-	}
+	recv_buf(fd, buf, sizeof(buf) - 8 - 2, MSG_DONTWAIT, -EAGAIN);
 
 	if (memcmp(buf, HELLO_STR WORLD_STR, strlen(HELLO_STR WORLD_STR))) {
 		fprintf(stderr, "pattern mismatch\n");
-- 
2.41.0

Re: [PATCH net-next 2/5] vsock/test: use recv_buf() in vsock_test.c

[Arseniy Krasnov]

On 15.09.2023 15:14, Stefano Garzarella wrote:
> We have a very common pattern used in vsock_test that we can
> now replace with the new recv_buf().
> 
> This allows us to reuse the code we already had to check the
> actual return value and wait for all bytes to be received with
> an appropriate timeout.
> 
> Signed-off-by: Stefano Garzarella <sgarzare@redhat.com>
> ---
>  tools/testing/vsock/vsock_test.c | 104 +++++--------------------------
>  1 file changed, 17 insertions(+), 87 deletions(-)

Reviewed-by: Arseniy Krasnov <avkrasnov@salutedevices.com>

> 
> diff --git a/tools/testing/vsock/vsock_test.c b/tools/testing/vsock/vsock_test.c
> index 90718c2fd4ea..d1dcbaeb477a 100644
> --- a/tools/testing/vsock/vsock_test.c
> +++ b/tools/testing/vsock/vsock_test.c
> @@ -301,7 +301,6 @@ static void test_msg_peek_server(const struct test_opts *opts,
>  	unsigned char buf_half[MSG_PEEK_BUF_LEN / 2];
>  	unsigned char buf_normal[MSG_PEEK_BUF_LEN];
>  	unsigned char buf_peek[MSG_PEEK_BUF_LEN];
> -	ssize_t res;
>  	int fd;
>  
>  	if (seqpacket)
> @@ -315,34 +314,16 @@ static void test_msg_peek_server(const struct test_opts *opts,
>  	}
>  
>  	/* Peek from empty socket. */
> -	res = recv(fd, buf_peek, sizeof(buf_peek), MSG_PEEK | MSG_DONTWAIT);
> -	if (res != -1) {
> -		fprintf(stderr, "expected recv(2) failure, got %zi\n", res);
> -		exit(EXIT_FAILURE);
> -	}
> -
> -	if (errno != EAGAIN) {
> -		perror("EAGAIN expected");
> -		exit(EXIT_FAILURE);
> -	}
> +	recv_buf(fd, buf_peek, sizeof(buf_peek), MSG_PEEK | MSG_DONTWAIT,
> +		 -EAGAIN);
>  
>  	control_writeln("SRVREADY");
>  
>  	/* Peek part of data. */
> -	res = recv(fd, buf_half, sizeof(buf_half), MSG_PEEK);
> -	if (res != sizeof(buf_half)) {
> -		fprintf(stderr, "recv(2) + MSG_PEEK, expected %zu, got %zi\n",
> -			sizeof(buf_half), res);
> -		exit(EXIT_FAILURE);
> -	}
> +	recv_buf(fd, buf_half, sizeof(buf_half), MSG_PEEK, sizeof(buf_half));
>  
>  	/* Peek whole data. */
> -	res = recv(fd, buf_peek, sizeof(buf_peek), MSG_PEEK);
> -	if (res != sizeof(buf_peek)) {
> -		fprintf(stderr, "recv(2) + MSG_PEEK, expected %zu, got %zi\n",
> -			sizeof(buf_peek), res);
> -		exit(EXIT_FAILURE);
> -	}
> +	recv_buf(fd, buf_peek, sizeof(buf_peek), MSG_PEEK, sizeof(buf_peek));
>  
>  	/* Compare partial and full peek. */
>  	if (memcmp(buf_half, buf_peek, sizeof(buf_half))) {
> @@ -355,22 +336,11 @@ static void test_msg_peek_server(const struct test_opts *opts,
>  		 * so check it with MSG_PEEK. We must get length
>  		 * of the message.
>  		 */
> -		res = recv(fd, buf_half, sizeof(buf_half), MSG_PEEK |
> -			   MSG_TRUNC);
> -		if (res != sizeof(buf_peek)) {
> -			fprintf(stderr,
> -				"recv(2) + MSG_PEEK | MSG_TRUNC, exp %zu, got %zi\n",
> -				sizeof(buf_half), res);
> -			exit(EXIT_FAILURE);
> -		}
> +		recv_buf(fd, buf_half, sizeof(buf_half), MSG_PEEK | MSG_TRUNC,
> +			 sizeof(buf_peek));
>  	}
>  
> -	res = recv(fd, buf_normal, sizeof(buf_normal), 0);
> -	if (res != sizeof(buf_normal)) {
> -		fprintf(stderr, "recv(2), expected %zu, got %zi\n",
> -			sizeof(buf_normal), res);
> -		exit(EXIT_FAILURE);
> -	}
> +	recv_buf(fd, buf_normal, sizeof(buf_normal), 0, sizeof(buf_normal));
>  
>  	/* Compare full peek and normal read. */
>  	if (memcmp(buf_peek, buf_normal, sizeof(buf_peek))) {
> @@ -900,7 +870,6 @@ static void test_stream_poll_rcvlowat_client(const struct test_opts *opts)
>  	unsigned long lowat_val = RCVLOWAT_BUF_SIZE;
>  	char buf[RCVLOWAT_BUF_SIZE];
>  	struct pollfd fds;
> -	ssize_t read_res;
>  	short poll_flags;
>  	int fd;
>  
> @@ -955,12 +924,7 @@ static void test_stream_poll_rcvlowat_client(const struct test_opts *opts)
>  	/* Use MSG_DONTWAIT, if call is going to wait, EAGAIN
>  	 * will be returned.
>  	 */
> -	read_res = recv(fd, buf, sizeof(buf), MSG_DONTWAIT);
> -	if (read_res != RCVLOWAT_BUF_SIZE) {
> -		fprintf(stderr, "Unexpected recv result %zi\n",
> -			read_res);
> -		exit(EXIT_FAILURE);
> -	}
> +	recv_buf(fd, buf, sizeof(buf), MSG_DONTWAIT, RCVLOWAT_BUF_SIZE);
>  
>  	control_writeln("POLLDONE");
>  
> @@ -972,7 +936,7 @@ static void test_stream_poll_rcvlowat_client(const struct test_opts *opts)
>  static void test_inv_buf_client(const struct test_opts *opts, bool stream)
>  {
>  	unsigned char data[INV_BUF_TEST_DATA_LEN] = {0};
> -	ssize_t ret;
> +	ssize_t expected_ret;
>  	int fd;
>  
>  	if (stream)
> @@ -988,39 +952,18 @@ static void test_inv_buf_client(const struct test_opts *opts, bool stream)
>  	control_expectln("SENDDONE");
>  
>  	/* Use invalid buffer here. */
> -	ret = recv(fd, NULL, sizeof(data), 0);
> -	if (ret != -1) {
> -		fprintf(stderr, "expected recv(2) failure, got %zi\n", ret);
> -		exit(EXIT_FAILURE);
> -	}
> -
> -	if (errno != EFAULT) {
> -		fprintf(stderr, "unexpected recv(2) errno %d\n", errno);
> -		exit(EXIT_FAILURE);
> -	}
> -
> -	ret = recv(fd, data, sizeof(data), MSG_DONTWAIT);
> +	recv_buf(fd, NULL, sizeof(data), 0, -EFAULT);
>  
>  	if (stream) {
>  		/* For SOCK_STREAM we must continue reading. */
> -		if (ret != sizeof(data)) {
> -			fprintf(stderr, "expected recv(2) success, got %zi\n", ret);
> -			exit(EXIT_FAILURE);
> -		}
> -		/* Don't check errno in case of success. */
> +		expected_ret = sizeof(data);
>  	} else {
>  		/* For SOCK_SEQPACKET socket's queue must be empty. */
> -		if (ret != -1) {
> -			fprintf(stderr, "expected recv(2) failure, got %zi\n", ret);
> -			exit(EXIT_FAILURE);
> -		}
> -
> -		if (errno != EAGAIN) {
> -			fprintf(stderr, "unexpected recv(2) errno %d\n", errno);
> -			exit(EXIT_FAILURE);
> -		}
> +		expected_ret = -EAGAIN;
>  	}
>  
> +	recv_buf(fd, data, sizeof(data), MSG_DONTWAIT, expected_ret);
> +
>  	control_writeln("DONE");
>  
>  	close(fd);
> @@ -1117,7 +1060,6 @@ static void test_stream_virtio_skb_merge_client(const struct test_opts *opts)
>  static void test_stream_virtio_skb_merge_server(const struct test_opts *opts)
>  {
>  	unsigned char buf[64];
> -	ssize_t res;
>  	int fd;
>  
>  	fd = vsock_stream_accept(VMADDR_CID_ANY, 1234, NULL);
> @@ -1129,26 +1071,14 @@ static void test_stream_virtio_skb_merge_server(const struct test_opts *opts)
>  	control_expectln("SEND0");
>  
>  	/* Read skbuff partially. */
> -	res = recv(fd, buf, 2, 0);
> -	if (res != 2) {
> -		fprintf(stderr, "expected recv(2) returns 2 bytes, got %zi\n", res);
> -		exit(EXIT_FAILURE);
> -	}
> +	recv_buf(fd, buf, 2, 0, 2);
>  
>  	control_writeln("REPLY0");
>  	control_expectln("SEND1");
>  
> -	res = recv(fd, buf + 2, sizeof(buf) - 2, 0);
> -	if (res != 8) {
> -		fprintf(stderr, "expected recv(2) returns 8 bytes, got %zi\n", res);
> -		exit(EXIT_FAILURE);
> -	}
> +	recv_buf(fd, buf + 2, 8, 0, 8);
>  
> -	res = recv(fd, buf, sizeof(buf) - 8 - 2, MSG_DONTWAIT);
> -	if (res != -1) {
> -		fprintf(stderr, "expected recv(2) failure, got %zi\n", res);
> -		exit(EXIT_FAILURE);
> -	}
> +	recv_buf(fd, buf, sizeof(buf) - 8 - 2, MSG_DONTWAIT, -EAGAIN);
>  
>  	if (memcmp(buf, HELLO_STR WORLD_STR, strlen(HELLO_STR WORLD_STR))) {
>  		fprintf(stderr, "pattern mismatch\n");

[PATCH net-next 3/5] vsock/test: add send_buf() utility function

[Stefano Garzarella]

Move the code of send_byte() out in a new utility function that
can be used to send a generic buffer.

This new function can be used when we need to send a custom
buffer and not just a single 'A' byte.

Signed-off-by: Stefano Garzarella <sgarzare@redhat.com>
---
 tools/testing/vsock/util.h |  2 +
 tools/testing/vsock/util.c | 90 +++++++++++++++++++++++---------------
 2 files changed, 56 insertions(+), 36 deletions(-)

diff --git a/tools/testing/vsock/util.h b/tools/testing/vsock/util.h
index fe31f267e67e..e5407677ce05 100644
--- a/tools/testing/vsock/util.h
+++ b/tools/testing/vsock/util.h
@@ -42,6 +42,8 @@ int vsock_stream_accept(unsigned int cid, unsigned int port,
 int vsock_seqpacket_accept(unsigned int cid, unsigned int port,
 			   struct sockaddr_vm *clientaddrp);
 void vsock_wait_remote_close(int fd);
+void send_buf(int fd, const void *buf, size_t len, int flags,
+	      ssize_t expected_ret);
 void recv_buf(int fd, void *buf, size_t len, int flags, ssize_t expected_ret);
 void send_byte(int fd, int expected_ret, int flags);
 void recv_byte(int fd, int expected_ret, int flags);
diff --git a/tools/testing/vsock/util.c b/tools/testing/vsock/util.c
index 2826902706e8..6779d5008b27 100644
--- a/tools/testing/vsock/util.c
+++ b/tools/testing/vsock/util.c
@@ -211,6 +211,59 @@ int vsock_seqpacket_accept(unsigned int cid, unsigned int port,
 	return vsock_accept(cid, port, clientaddrp, SOCK_SEQPACKET);
 }
 
+/* Transmit bytes from a buffer and check the return value.
+ *
+ * expected_ret:
+ *  <0 Negative errno (for testing errors)
+ *   0 End-of-file
+ *  >0 Success (bytes successfully written)
+ */
+void send_buf(int fd, const void *buf, size_t len, int flags,
+	      ssize_t expected_ret)
+{
+	ssize_t nwritten = 0;
+	ssize_t ret;
+
+	timeout_begin(TIMEOUT);
+	do {
+		ret = send(fd, buf + nwritten, len - nwritten, flags);
+		timeout_check("send");
+
+		if (ret == 0 || (ret < 0 && errno != EINTR))
+			break;
+
+		nwritten += ret;
+	} while (nwritten < len);
+	timeout_end();
+
+	if (expected_ret < 0) {
+		if (ret != -1) {
+			fprintf(stderr, "bogus send(2) return value %zd (expected %zd)\n",
+				ret, expected_ret);
+			exit(EXIT_FAILURE);
+		}
+		if (errno != -expected_ret) {
+			perror("send");
+			exit(EXIT_FAILURE);
+		}
+		return;
+	}
+
+	if (ret < 0) {
+		perror("send");
+		exit(EXIT_FAILURE);
+	}
+
+	if (nwritten != expected_ret) {
+		if (ret == 0)
+			fprintf(stderr, "unexpected EOF while sending bytes\n");
+
+		fprintf(stderr, "bogus send(2) bytes written %zd (expected %zd)\n",
+			nwritten, expected_ret);
+		exit(EXIT_FAILURE);
+	}
+}
+
 /* Receive bytes in a buffer and check the return value.
  *
  * expected_ret:
@@ -273,43 +326,8 @@ void recv_buf(int fd, void *buf, size_t len, int flags, ssize_t expected_ret)
 void send_byte(int fd, int expected_ret, int flags)
 {
 	const uint8_t byte = 'A';
-	ssize_t nwritten;
-
-	timeout_begin(TIMEOUT);
-	do {
-		nwritten = send(fd, &byte, sizeof(byte), flags);
-		timeout_check("write");
-	} while (nwritten < 0 && errno == EINTR);
-	timeout_end();
-
-	if (expected_ret < 0) {
-		if (nwritten != -1) {
-			fprintf(stderr, "bogus send(2) return value %zd\n",
-				nwritten);
-			exit(EXIT_FAILURE);
-		}
-		if (errno != -expected_ret) {
-			perror("write");
-			exit(EXIT_FAILURE);
-		}
-		return;
-	}
 
-	if (nwritten < 0) {
-		perror("write");
-		exit(EXIT_FAILURE);
-	}
-	if (nwritten == 0) {
-		if (expected_ret == 0)
-			return;
-
-		fprintf(stderr, "unexpected EOF while sending byte\n");
-		exit(EXIT_FAILURE);
-	}
-	if (nwritten != sizeof(byte)) {
-		fprintf(stderr, "bogus send(2) return value %zd\n", nwritten);
-		exit(EXIT_FAILURE);
-	}
+	send_buf(fd, &byte, sizeof(byte), flags, expected_ret);
 }
 
 /* Receive one byte and check the return value.
-- 
2.41.0

Re: [PATCH net-next 3/5] vsock/test: add send_buf() utility function

[Arseniy Krasnov]

On 15.09.2023 15:14, Stefano Garzarella wrote:
> Move the code of send_byte() out in a new utility function that
> can be used to send a generic buffer.
> 
> This new function can be used when we need to send a custom
> buffer and not just a single 'A' byte.
> 
> Signed-off-by: Stefano Garzarella <sgarzare@redhat.com>
> ---
>  tools/testing/vsock/util.h |  2 +
>  tools/testing/vsock/util.c | 90 +++++++++++++++++++++++---------------
>  2 files changed, 56 insertions(+), 36 deletions(-)

Reviewed-by: Arseniy Krasnov <avkrasnov@salutedevices.com>

> 
> diff --git a/tools/testing/vsock/util.h b/tools/testing/vsock/util.h
> index fe31f267e67e..e5407677ce05 100644
> --- a/tools/testing/vsock/util.h
> +++ b/tools/testing/vsock/util.h
> @@ -42,6 +42,8 @@ int vsock_stream_accept(unsigned int cid, unsigned int port,
>  int vsock_seqpacket_accept(unsigned int cid, unsigned int port,
>  			   struct sockaddr_vm *clientaddrp);
>  void vsock_wait_remote_close(int fd);
> +void send_buf(int fd, const void *buf, size_t len, int flags,
> +	      ssize_t expected_ret);
>  void recv_buf(int fd, void *buf, size_t len, int flags, ssize_t expected_ret);
>  void send_byte(int fd, int expected_ret, int flags);
>  void recv_byte(int fd, int expected_ret, int flags);
> diff --git a/tools/testing/vsock/util.c b/tools/testing/vsock/util.c
> index 2826902706e8..6779d5008b27 100644
> --- a/tools/testing/vsock/util.c
> +++ b/tools/testing/vsock/util.c
> @@ -211,6 +211,59 @@ int vsock_seqpacket_accept(unsigned int cid, unsigned int port,
>  	return vsock_accept(cid, port, clientaddrp, SOCK_SEQPACKET);
>  }
>  
> +/* Transmit bytes from a buffer and check the return value.
> + *
> + * expected_ret:
> + *  <0 Negative errno (for testing errors)
> + *   0 End-of-file
> + *  >0 Success (bytes successfully written)
> + */
> +void send_buf(int fd, const void *buf, size_t len, int flags,
> +	      ssize_t expected_ret)
> +{
> +	ssize_t nwritten = 0;
> +	ssize_t ret;
> +
> +	timeout_begin(TIMEOUT);
> +	do {
> +		ret = send(fd, buf + nwritten, len - nwritten, flags);
> +		timeout_check("send");
> +
> +		if (ret == 0 || (ret < 0 && errno != EINTR))
> +			break;
> +
> +		nwritten += ret;
> +	} while (nwritten < len);
> +	timeout_end();
> +
> +	if (expected_ret < 0) {
> +		if (ret != -1) {
> +			fprintf(stderr, "bogus send(2) return value %zd (expected %zd)\n",
> +				ret, expected_ret);
> +			exit(EXIT_FAILURE);
> +		}
> +		if (errno != -expected_ret) {
> +			perror("send");
> +			exit(EXIT_FAILURE);
> +		}
> +		return;
> +	}
> +
> +	if (ret < 0) {
> +		perror("send");
> +		exit(EXIT_FAILURE);
> +	}
> +
> +	if (nwritten != expected_ret) {
> +		if (ret == 0)
> +			fprintf(stderr, "unexpected EOF while sending bytes\n");
> +
> +		fprintf(stderr, "bogus send(2) bytes written %zd (expected %zd)\n",
> +			nwritten, expected_ret);
> +		exit(EXIT_FAILURE);
> +	}
> +}
> +
>  /* Receive bytes in a buffer and check the return value.
>   *
>   * expected_ret:
> @@ -273,43 +326,8 @@ void recv_buf(int fd, void *buf, size_t len, int flags, ssize_t expected_ret)
>  void send_byte(int fd, int expected_ret, int flags)
>  {
>  	const uint8_t byte = 'A';
> -	ssize_t nwritten;
> -
> -	timeout_begin(TIMEOUT);
> -	do {
> -		nwritten = send(fd, &byte, sizeof(byte), flags);
> -		timeout_check("write");
> -	} while (nwritten < 0 && errno == EINTR);
> -	timeout_end();
> -
> -	if (expected_ret < 0) {
> -		if (nwritten != -1) {
> -			fprintf(stderr, "bogus send(2) return value %zd\n",
> -				nwritten);
> -			exit(EXIT_FAILURE);
> -		}
> -		if (errno != -expected_ret) {
> -			perror("write");
> -			exit(EXIT_FAILURE);
> -		}
> -		return;
> -	}
>  
> -	if (nwritten < 0) {
> -		perror("write");
> -		exit(EXIT_FAILURE);
> -	}
> -	if (nwritten == 0) {
> -		if (expected_ret == 0)
> -			return;
> -
> -		fprintf(stderr, "unexpected EOF while sending byte\n");
> -		exit(EXIT_FAILURE);
> -	}
> -	if (nwritten != sizeof(byte)) {
> -		fprintf(stderr, "bogus send(2) return value %zd\n", nwritten);
> -		exit(EXIT_FAILURE);
> -	}
> +	send_buf(fd, &byte, sizeof(byte), flags, expected_ret);
>  }
>  
>  /* Receive one byte and check the return value.

[PATCH net-next 4/5] vsock/test: use send_buf() in vsock_test.c

[Stefano Garzarella]

We have a very common pattern used in vsock_test that we can
now replace with the new send_buf().

This allows us to reuse the code we already had to check the
actual return value and wait for all the bytes to be sent with
an appropriate timeout.

Signed-off-by: Stefano Garzarella <sgarzare@redhat.com>
---
 tools/testing/vsock/vsock_test.c | 75 ++++----------------------------
 1 file changed, 9 insertions(+), 66 deletions(-)

diff --git a/tools/testing/vsock/vsock_test.c b/tools/testing/vsock/vsock_test.c
index d1dcbaeb477a..b18acbaf92e2 100644
--- a/tools/testing/vsock/vsock_test.c
+++ b/tools/testing/vsock/vsock_test.c
@@ -261,7 +261,6 @@ static void test_msg_peek_client(const struct test_opts *opts,
 				 bool seqpacket)
 {
 	unsigned char buf[MSG_PEEK_BUF_LEN];
-	ssize_t send_size;
 	int fd;
 	int i;
 
@@ -280,17 +279,7 @@ static void test_msg_peek_client(const struct test_opts *opts,
 
 	control_expectln("SRVREADY");
 
-	send_size = send(fd, buf, sizeof(buf), 0);
-
-	if (send_size < 0) {
-		perror("send");
-		exit(EXIT_FAILURE);
-	}
-
-	if (send_size != sizeof(buf)) {
-		fprintf(stderr, "Invalid send size %zi\n", send_size);
-		exit(EXIT_FAILURE);
-	}
+	send_buf(fd, buf, sizeof(buf), 0, sizeof(buf));
 
 	close(fd);
 }
@@ -385,7 +374,6 @@ static void test_seqpacket_msg_bounds_client(const struct test_opts *opts)
 	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;
@@ -413,17 +401,7 @@ static void test_seqpacket_msg_bounds_client(const struct test_opts *opts)
 			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);
-		}
+		send_buf(fd, buf, buf_size, flags, buf_size);
 
 		/*
 		 * Hash sum is computed at both client and server in
@@ -524,10 +502,7 @@ static void test_seqpacket_msg_trunc_client(const struct test_opts *opts)
 		exit(EXIT_FAILURE);
 	}
 
-	if (send(fd, buf, sizeof(buf), 0) != sizeof(buf)) {
-		perror("send failed");
-		exit(EXIT_FAILURE);
-	}
+	send_buf(fd, buf, sizeof(buf), 0, sizeof(buf));
 
 	control_writeln("SENDDONE");
 	close(fd);
@@ -649,7 +624,6 @@ 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;
-	ssize_t send_size;
 	socklen_t len;
 	void *data;
 	int fd;
@@ -676,18 +650,7 @@ static void test_seqpacket_bigmsg_client(const struct test_opts *opts)
 		exit(EXIT_FAILURE);
 	}
 
-	send_size = send(fd, data, sock_buf_size, 0);
-	if (send_size != -1) {
-		fprintf(stderr, "expected 'send(2)' failure, got %zi\n",
-			send_size);
-		exit(EXIT_FAILURE);
-	}
-
-	if (errno != EMSGSIZE) {
-		fprintf(stderr, "expected EMSGSIZE in 'errno', got %i\n",
-			errno);
-		exit(EXIT_FAILURE);
-	}
+	send_buf(fd, data, sock_buf_size, 0, -EMSGSIZE);
 
 	control_writeln("CLISENT");
 
@@ -741,15 +704,9 @@ static void test_seqpacket_invalid_rec_buffer_client(const struct test_opts *opt
 	memset(buf1, BUF_PATTERN_1, buf_size);
 	memset(buf2, BUF_PATTERN_2, buf_size);
 
-	if (send(fd, buf1, buf_size, 0) != buf_size) {
-		perror("send failed");
-		exit(EXIT_FAILURE);
-	}
+	send_buf(fd, buf1, buf_size, 0, buf_size);
 
-	if (send(fd, buf2, buf_size, 0) != buf_size) {
-		perror("send failed");
-		exit(EXIT_FAILURE);
-	}
+	send_buf(fd, buf2, buf_size, 0, buf_size);
 
 	close(fd);
 }
@@ -972,7 +929,6 @@ static void test_inv_buf_client(const struct test_opts *opts, bool stream)
 static void test_inv_buf_server(const struct test_opts *opts, bool stream)
 {
 	unsigned char data[INV_BUF_TEST_DATA_LEN] = {0};
-	ssize_t res;
 	int fd;
 
 	if (stream)
@@ -985,11 +941,7 @@ static void test_inv_buf_server(const struct test_opts *opts, bool stream)
 		exit(EXIT_FAILURE);
 	}
 
-	res = send(fd, data, sizeof(data), 0);
-	if (res != sizeof(data)) {
-		fprintf(stderr, "unexpected send(2) result %zi\n", res);
-		exit(EXIT_FAILURE);
-	}
+	send_buf(fd, data, sizeof(data), 0, sizeof(data));
 
 	control_writeln("SENDDONE");
 
@@ -1023,7 +975,6 @@ static void test_seqpacket_inv_buf_server(const struct test_opts *opts)
 
 static void test_stream_virtio_skb_merge_client(const struct test_opts *opts)
 {
-	ssize_t res;
 	int fd;
 
 	fd = vsock_stream_connect(opts->peer_cid, 1234);
@@ -1033,22 +984,14 @@ static void test_stream_virtio_skb_merge_client(const struct test_opts *opts)
 	}
 
 	/* Send first skbuff. */
-	res = send(fd, HELLO_STR, strlen(HELLO_STR), 0);
-	if (res != strlen(HELLO_STR)) {
-		fprintf(stderr, "unexpected send(2) result %zi\n", res);
-		exit(EXIT_FAILURE);
-	}
+	send_buf(fd, HELLO_STR, strlen(HELLO_STR), 0, strlen(HELLO_STR));
 
 	control_writeln("SEND0");
 	/* Peer reads part of first skbuff. */
 	control_expectln("REPLY0");
 
 	/* Send second skbuff, it will be appended to the first. */
-	res = send(fd, WORLD_STR, strlen(WORLD_STR), 0);
-	if (res != strlen(WORLD_STR)) {
-		fprintf(stderr, "unexpected send(2) result %zi\n", res);
-		exit(EXIT_FAILURE);
-	}
+	send_buf(fd, WORLD_STR, strlen(WORLD_STR), 0, strlen(WORLD_STR));
 
 	control_writeln("SEND1");
 	/* Peer reads merged skbuff packet. */
-- 
2.41.0

Re: [PATCH net-next 4/5] vsock/test: use send_buf() in vsock_test.c

[Arseniy Krasnov]

On 15.09.2023 15:14, Stefano Garzarella wrote:
> We have a very common pattern used in vsock_test that we can
> now replace with the new send_buf().
> 
> This allows us to reuse the code we already had to check the
> actual return value and wait for all the bytes to be sent with
> an appropriate timeout.
> 
> Signed-off-by: Stefano Garzarella <sgarzare@redhat.com>
> ---
>  tools/testing/vsock/vsock_test.c | 75 ++++----------------------------
>  1 file changed, 9 insertions(+), 66 deletions(-)

Reviewed-by: Arseniy Krasnov <avkrasnov@salutedevices.com>

> 
> diff --git a/tools/testing/vsock/vsock_test.c b/tools/testing/vsock/vsock_test.c
> index d1dcbaeb477a..b18acbaf92e2 100644
> --- a/tools/testing/vsock/vsock_test.c
> +++ b/tools/testing/vsock/vsock_test.c
> @@ -261,7 +261,6 @@ static void test_msg_peek_client(const struct test_opts *opts,
>  				 bool seqpacket)
>  {
>  	unsigned char buf[MSG_PEEK_BUF_LEN];
> -	ssize_t send_size;
>  	int fd;
>  	int i;
>  
> @@ -280,17 +279,7 @@ static void test_msg_peek_client(const struct test_opts *opts,
>  
>  	control_expectln("SRVREADY");
>  
> -	send_size = send(fd, buf, sizeof(buf), 0);
> -
> -	if (send_size < 0) {
> -		perror("send");
> -		exit(EXIT_FAILURE);
> -	}
> -
> -	if (send_size != sizeof(buf)) {
> -		fprintf(stderr, "Invalid send size %zi\n", send_size);
> -		exit(EXIT_FAILURE);
> -	}
> +	send_buf(fd, buf, sizeof(buf), 0, sizeof(buf));
>  
>  	close(fd);
>  }
> @@ -385,7 +374,6 @@ static void test_seqpacket_msg_bounds_client(const struct test_opts *opts)
>  	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;
> @@ -413,17 +401,7 @@ static void test_seqpacket_msg_bounds_client(const struct test_opts *opts)
>  			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);
> -		}
> +		send_buf(fd, buf, buf_size, flags, buf_size);
>  
>  		/*
>  		 * Hash sum is computed at both client and server in
> @@ -524,10 +502,7 @@ static void test_seqpacket_msg_trunc_client(const struct test_opts *opts)
>  		exit(EXIT_FAILURE);
>  	}
>  
> -	if (send(fd, buf, sizeof(buf), 0) != sizeof(buf)) {
> -		perror("send failed");
> -		exit(EXIT_FAILURE);
> -	}
> +	send_buf(fd, buf, sizeof(buf), 0, sizeof(buf));
>  
>  	control_writeln("SENDDONE");
>  	close(fd);
> @@ -649,7 +624,6 @@ 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;
> -	ssize_t send_size;
>  	socklen_t len;
>  	void *data;
>  	int fd;
> @@ -676,18 +650,7 @@ static void test_seqpacket_bigmsg_client(const struct test_opts *opts)
>  		exit(EXIT_FAILURE);
>  	}
>  
> -	send_size = send(fd, data, sock_buf_size, 0);
> -	if (send_size != -1) {
> -		fprintf(stderr, "expected 'send(2)' failure, got %zi\n",
> -			send_size);
> -		exit(EXIT_FAILURE);
> -	}
> -
> -	if (errno != EMSGSIZE) {
> -		fprintf(stderr, "expected EMSGSIZE in 'errno', got %i\n",
> -			errno);
> -		exit(EXIT_FAILURE);
> -	}
> +	send_buf(fd, data, sock_buf_size, 0, -EMSGSIZE);
>  
>  	control_writeln("CLISENT");
>  
> @@ -741,15 +704,9 @@ static void test_seqpacket_invalid_rec_buffer_client(const struct test_opts *opt
>  	memset(buf1, BUF_PATTERN_1, buf_size);
>  	memset(buf2, BUF_PATTERN_2, buf_size);
>  
> -	if (send(fd, buf1, buf_size, 0) != buf_size) {
> -		perror("send failed");
> -		exit(EXIT_FAILURE);
> -	}
> +	send_buf(fd, buf1, buf_size, 0, buf_size);
>  
> -	if (send(fd, buf2, buf_size, 0) != buf_size) {
> -		perror("send failed");
> -		exit(EXIT_FAILURE);
> -	}
> +	send_buf(fd, buf2, buf_size, 0, buf_size);
>  
>  	close(fd);
>  }
> @@ -972,7 +929,6 @@ static void test_inv_buf_client(const struct test_opts *opts, bool stream)
>  static void test_inv_buf_server(const struct test_opts *opts, bool stream)
>  {
>  	unsigned char data[INV_BUF_TEST_DATA_LEN] = {0};
> -	ssize_t res;
>  	int fd;
>  
>  	if (stream)
> @@ -985,11 +941,7 @@ static void test_inv_buf_server(const struct test_opts *opts, bool stream)
>  		exit(EXIT_FAILURE);
>  	}
>  
> -	res = send(fd, data, sizeof(data), 0);
> -	if (res != sizeof(data)) {
> -		fprintf(stderr, "unexpected send(2) result %zi\n", res);
> -		exit(EXIT_FAILURE);
> -	}
> +	send_buf(fd, data, sizeof(data), 0, sizeof(data));
>  
>  	control_writeln("SENDDONE");
>  
> @@ -1023,7 +975,6 @@ static void test_seqpacket_inv_buf_server(const struct test_opts *opts)
>  
>  static void test_stream_virtio_skb_merge_client(const struct test_opts *opts)
>  {
> -	ssize_t res;
>  	int fd;
>  
>  	fd = vsock_stream_connect(opts->peer_cid, 1234);
> @@ -1033,22 +984,14 @@ static void test_stream_virtio_skb_merge_client(const struct test_opts *opts)
>  	}
>  
>  	/* Send first skbuff. */
> -	res = send(fd, HELLO_STR, strlen(HELLO_STR), 0);
> -	if (res != strlen(HELLO_STR)) {
> -		fprintf(stderr, "unexpected send(2) result %zi\n", res);
> -		exit(EXIT_FAILURE);
> -	}
> +	send_buf(fd, HELLO_STR, strlen(HELLO_STR), 0, strlen(HELLO_STR));
>  
>  	control_writeln("SEND0");
>  	/* Peer reads part of first skbuff. */
>  	control_expectln("REPLY0");
>  
>  	/* Send second skbuff, it will be appended to the first. */
> -	res = send(fd, WORLD_STR, strlen(WORLD_STR), 0);
> -	if (res != strlen(WORLD_STR)) {
> -		fprintf(stderr, "unexpected send(2) result %zi\n", res);
> -		exit(EXIT_FAILURE);
> -	}
> +	send_buf(fd, WORLD_STR, strlen(WORLD_STR), 0, strlen(WORLD_STR));
>  
>  	control_writeln("SEND1");
>  	/* Peer reads merged skbuff packet. */

[PATCH net-next 5/5] vsock/test: track bytes in MSG_PEEK test for SOCK_SEQPACKET

[Stefano Garzarella]

The test was a bit complicated to read.
Added variables to keep track of the bytes read and to be read
in each step. Also some comments.

The test is unchanged.

Signed-off-by: Stefano Garzarella <sgarzare@redhat.com>
---
 tools/testing/vsock/vsock_test.c | 14 +++++++++++---
 1 file changed, 11 insertions(+), 3 deletions(-)

diff --git a/tools/testing/vsock/vsock_test.c b/tools/testing/vsock/vsock_test.c
index b18acbaf92e2..5743dcae2350 100644
--- a/tools/testing/vsock/vsock_test.c
+++ b/tools/testing/vsock/vsock_test.c
@@ -1002,6 +1002,7 @@ static void test_stream_virtio_skb_merge_client(const struct test_opts *opts)
 
 static void test_stream_virtio_skb_merge_server(const struct test_opts *opts)
 {
+	size_t read = 0, to_read;
 	unsigned char buf[64];
 	int fd;
 
@@ -1014,14 +1015,21 @@ static void test_stream_virtio_skb_merge_server(const struct test_opts *opts)
 	control_expectln("SEND0");
 
 	/* Read skbuff partially. */
-	recv_buf(fd, buf, 2, 0, 2);
+	to_read = 2;
+	recv_buf(fd, buf + read, to_read, 0, to_read);
+	read += to_read;
 
 	control_writeln("REPLY0");
 	control_expectln("SEND1");
 
-	recv_buf(fd, buf + 2, 8, 0, 8);
+	/* Read the rest of both buffers */
+	to_read = strlen(HELLO_STR WORLD_STR) - read;
+	recv_buf(fd, buf + read, to_read, 0, to_read);
+	read += to_read;
 
-	recv_buf(fd, buf, sizeof(buf) - 8 - 2, MSG_DONTWAIT, -EAGAIN);
+	/* No more bytes should be there */
+	to_read = sizeof(buf) - read;
+	recv_buf(fd, buf + read, to_read, MSG_DONTWAIT, -EAGAIN);
 
 	if (memcmp(buf, HELLO_STR WORLD_STR, strlen(HELLO_STR WORLD_STR))) {
 		fprintf(stderr, "pattern mismatch\n");
-- 
2.41.0

Re: [PATCH net-next 5/5] vsock/test: track bytes in MSG_PEEK test for SOCK_SEQPACKET

[Arseniy Krasnov]

Small remark on 'Subject' - this is not MSG_PEEK test, it is test for sk_buff merging.

Considering that:

Reviewed-by: Arseniy Krasnov <avkrasnov@salutedevices.com>

Thanks, Arseniy

On 15.09.2023 15:14, Stefano Garzarella wrote:
> The test was a bit complicated to read.
> Added variables to keep track of the bytes read and to be read
> in each step. Also some comments.
> 
> The test is unchanged.
> 
> Signed-off-by: Stefano Garzarella <sgarzare@redhat.com>
> ---
>  tools/testing/vsock/vsock_test.c | 14 +++++++++++---
>  1 file changed, 11 insertions(+), 3 deletions(-)
> 
> diff --git a/tools/testing/vsock/vsock_test.c b/tools/testing/vsock/vsock_test.c
> index b18acbaf92e2..5743dcae2350 100644
> --- a/tools/testing/vsock/vsock_test.c
> +++ b/tools/testing/vsock/vsock_test.c
> @@ -1002,6 +1002,7 @@ static void test_stream_virtio_skb_merge_client(const struct test_opts *opts)
>  
>  static void test_stream_virtio_skb_merge_server(const struct test_opts *opts)
>  {
> +	size_t read = 0, to_read;
>  	unsigned char buf[64];
>  	int fd;
>  
> @@ -1014,14 +1015,21 @@ static void test_stream_virtio_skb_merge_server(const struct test_opts *opts)
>  	control_expectln("SEND0");
>  
>  	/* Read skbuff partially. */
> -	recv_buf(fd, buf, 2, 0, 2);
> +	to_read = 2;
> +	recv_buf(fd, buf + read, to_read, 0, to_read);
> +	read += to_read;
>  
>  	control_writeln("REPLY0");
>  	control_expectln("SEND1");
>  
> -	recv_buf(fd, buf + 2, 8, 0, 8);
> +	/* Read the rest of both buffers */
> +	to_read = strlen(HELLO_STR WORLD_STR) - read;
> +	recv_buf(fd, buf + read, to_read, 0, to_read);
> +	read += to_read;
>  
> -	recv_buf(fd, buf, sizeof(buf) - 8 - 2, MSG_DONTWAIT, -EAGAIN);
> +	/* No more bytes should be there */
> +	to_read = sizeof(buf) - read;
> +	recv_buf(fd, buf + read, to_read, MSG_DONTWAIT, -EAGAIN);
>  
>  	if (memcmp(buf, HELLO_STR WORLD_STR, strlen(HELLO_STR WORLD_STR))) {
>  		fprintf(stderr, "pattern mismatch\n");

Re: [PATCH net-next 0/5] vsock/test: add recv_buf()/send_buf() utility functions and some improvements

[Arseniy Krasnov]

On 15.09.2023 15:14, Stefano Garzarella wrote:
> We recently found that some tests were failing [1].
> 
> The problem was that we were not waiting for all the bytes correctly,
> so we had a partial read. I had initially suggested using MSG_WAITALL,
> but this could have timeout problems.
> 
> Since we already had send_byte() and recv_byte() that handled the timeout,
> but also the expected return value, I moved that code to two new functions
> that we can now use to send/receive generic buffers.
> 
> The last commit is just an improvement to a test I found difficult to
> understand while using the new functions.
> 
> @Arseniy a review and some testing are really appreciated :-)

Thanks for this cleanup of the tests, I reviewed this patchset and run local
testing, seems everything is ok! :)

Thanks, Arseniy

> 
> [1] https://lore.kernel.org/netdev/63xflnwiohdfo6m3vnrrxgv2ulplencpwug5qqacugqh7xxpu3@tsczkuqgwurb/
> 
> Stefano Garzarella (5):
>   vsock/test: add recv_buf() utility function
>   vsock/test: use recv_buf() in vsock_test.c
>   vsock/test: add send_buf() utility function
>   vsock/test: use send_buf() in vsock_test.c
>   vsock/test: track bytes in MSG_PEEK test for SOCK_SEQPACKET
> 
>  tools/testing/vsock/util.h       |   3 +
>  tools/testing/vsock/util.c       | 124 ++++++++++++--------
>  tools/testing/vsock/vsock_test.c | 187 ++++++-------------------------
>  3 files changed, 117 insertions(+), 197 deletions(-)
> 

Re: [PATCH net-next 0/5] vsock/test: add recv_buf()/send_buf() utility functions and some improvements

[patchwork-bot+netdevbpf]

Hello:

This series was applied to netdev/net-next.git (main)
by David S. Miller <davem@davemloft.net>:

On Fri, 15 Sep 2023 14:14:47 +0200 you wrote:
> We recently found that some tests were failing [1].
> 
> The problem was that we were not waiting for all the bytes correctly,
> so we had a partial read. I had initially suggested using MSG_WAITALL,
> but this could have timeout problems.
> 
> Since we already had send_byte() and recv_byte() that handled the timeout,
> but also the expected return value, I moved that code to two new functions
> that we can now use to send/receive generic buffers.
> 
> [...]

Here is the summary with links:
  - [net-next,1/5] vsock/test: add recv_buf() utility function
    (no matching commit)
  - [net-next,2/5] vsock/test: use recv_buf() in vsock_test.c
    https://git.kernel.org/netdev/net-next/c/a0bcb8357716
  - [net-next,3/5] vsock/test: add send_buf() utility function
    (no matching commit)
  - [net-next,4/5] vsock/test: use send_buf() in vsock_test.c
    https://git.kernel.org/netdev/net-next/c/2a8548a9bb4c
  - [net-next,5/5] vsock/test: track bytes in MSG_PEEK test for SOCK_SEQPACKET
    https://git.kernel.org/netdev/net-next/c/bc7bea452d32

You are awesome, thank you!
-- 
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html