These tests:
    "SOCK_STREAM ioctl(SIOCOUTQ) 0 unsent bytes"
    "SOCK_SEQPACKET ioctl(SIOCOUTQ) 0 unsent bytes"
output: "Unexpected 'SIOCOUTQ' value, expected 0, got 64 (CLIENT)".

They test that the SIOCOUTQ ioctl reports 0 unsent bytes after the data
have been received by the other side. However, sometimes there is a delay
in updating this "unsent bytes" counter, and the test fails even though
the counter properly goes to 0 several milliseconds later.

The delay occurs in the kernel because the used buffer notification
callback virtio_vsock_tx_done(), called upon receipt of the data by the
other side, doesn't update the counter itself. It delegates that to
a kernel thread (via vsock->tx_work). Sometimes that thread is delayed
more than the test expects.

Change the test to try SIOCOUTQ several times with small delays in between.

Signed-off-by: Konstantin Shkolnyy <kshk@linux.ibm.com>
---
 tools/testing/vsock/vsock_test.c | 26 ++++++++++++++++----------
 1 file changed, 16 insertions(+), 10 deletions(-)

diff --git a/tools/testing/vsock/vsock_test.c b/tools/testing/vsock/vsock_test.c
index d0f6d253ac72..143f1cba2d18 100644
--- a/tools/testing/vsock/vsock_test.c
+++ b/tools/testing/vsock/vsock_test.c
@@ -1264,21 +1264,27 @@ static void test_unsent_bytes_client(const struct test_opts *opts, int type)
 	send_buf(fd, buf, sizeof(buf), 0, sizeof(buf));
 	control_expectln("RECEIVED");
 
-	ret = ioctl(fd, SIOCOUTQ, &sock_bytes_unsent);
-	if (ret < 0) {
-		if (errno == EOPNOTSUPP) {
-			fprintf(stderr, "Test skipped, SIOCOUTQ not supported.\n");
-		} else {
+	/* SIOCOUTQ isn't guaranteed to instantly track sent data */
+	for (int i = 0; i < 10; i++) {
+		ret = ioctl(fd, SIOCOUTQ, &sock_bytes_unsent);
+		if (ret == 0 && sock_bytes_unsent == 0)
+			goto success;
+
+		if (ret < 0) {
+			if (errno == EOPNOTSUPP) {
+				fprintf(stderr, "Test skipped, SIOCOUTQ not supported.\n");
+				goto success;
+			}
 			perror("ioctl");
 			exit(EXIT_FAILURE);
 		}
-	} else if (ret == 0 && sock_bytes_unsent != 0) {
-		fprintf(stderr,
-			"Unexpected 'SIOCOUTQ' value, expected 0, got %i\n",
-			sock_bytes_unsent);
-		exit(EXIT_FAILURE);
+		usleep(10 * 1000);
 	}
 
+	fprintf(stderr, "Unexpected 'SIOCOUTQ' value, expected 0, got %i\n",
+		sock_bytes_unsent);
+	exit(EXIT_FAILURE);
+success:
 	close(fd);
 }
 
-- 
2.34.1

On Wed, May 07, 2025 at 06:48:33AM -0500, Konstantin Shkolnyy wrote:
>These tests:
>    "SOCK_STREAM ioctl(SIOCOUTQ) 0 unsent bytes"
>    "SOCK_SEQPACKET ioctl(SIOCOUTQ) 0 unsent bytes"
>output: "Unexpected 'SIOCOUTQ' value, expected 0, got 64 (CLIENT)".
>
>They test that the SIOCOUTQ ioctl reports 0 unsent bytes after the data
>have been received by the other side. However, sometimes there is a delay
>in updating this "unsent bytes" counter, and the test fails even though
>the counter properly goes to 0 several milliseconds later.
>
>The delay occurs in the kernel because the used buffer notification
>callback virtio_vsock_tx_done(), called upon receipt of the data by the
>other side, doesn't update the counter itself. It delegates that to
>a kernel thread (via vsock->tx_work). Sometimes that thread is delayed
>more than the test expects.
>
>Change the test to try SIOCOUTQ several times with small delays in between.
>
>Signed-off-by: Konstantin Shkolnyy <kshk@linux.ibm.com>
>---
> tools/testing/vsock/vsock_test.c | 26 ++++++++++++++++----------
> 1 file changed, 16 insertions(+), 10 deletions(-)
>
>diff --git a/tools/testing/vsock/vsock_test.c b/tools/testing/vsock/vsock_test.c
>index d0f6d253ac72..143f1cba2d18 100644
>--- a/tools/testing/vsock/vsock_test.c
>+++ b/tools/testing/vsock/vsock_test.c
>@@ -1264,21 +1264,27 @@ static void test_unsent_bytes_client(const struct test_opts *opts, int type)
> 	send_buf(fd, buf, sizeof(buf), 0, sizeof(buf));
> 	control_expectln("RECEIVED");
>
>-	ret = ioctl(fd, SIOCOUTQ, &sock_bytes_unsent);
>-	if (ret < 0) {
>-		if (errno == EOPNOTSUPP) {
>-			fprintf(stderr, "Test skipped, SIOCOUTQ not supported.\n");
>-		} else {
>+	/* SIOCOUTQ isn't guaranteed to instantly track sent data */
>+	for (int i = 0; i < 10; i++) {
>+		ret = ioctl(fd, SIOCOUTQ, &sock_bytes_unsent);
>+		if (ret == 0 && sock_bytes_unsent == 0)
>+			goto success;
>+
>+		if (ret < 0) {
>+			if (errno == EOPNOTSUPP) {
>+				fprintf(stderr, "Test skipped, SIOCOUTQ not supported.\n");
>+				goto success;
>+			}
> 			perror("ioctl");
> 			exit(EXIT_FAILURE);
> 		}
>-	} else if (ret == 0 && sock_bytes_unsent != 0) {
>-		fprintf(stderr,
>-			"Unexpected 'SIOCOUTQ' value, expected 0, got %i\n",
>-			sock_bytes_unsent);
>-		exit(EXIT_FAILURE);
>+		usleep(10 * 1000);
> 	}
>
>+	fprintf(stderr, "Unexpected 'SIOCOUTQ' value, expected 0, got %i\n",
>+		sock_bytes_unsent);
>+	exit(EXIT_FAILURE);
>+success:
> 	close(fd);

I worked on something similar but I didn't yet send it.

I like the delay you put, but I prefer to use the timeout stuff we have
to retry, like I did here:

@@ -1264,20 +1270,25 @@ static void test_unsent_bytes_client(const struct test_opts *op
ts, int type)
         send_buf(fd, buf, sizeof(buf), 0, sizeof(buf));
         control_expectln("RECEIVED");

-       ret = ioctl(fd, SIOCOUTQ, &sock_bytes_unsent);
-       if (ret < 0) {
-               if (errno == EOPNOTSUPP) {
-                       fprintf(stderr, "Test skipped, SIOCOUTQ not supported.\n");
-               } else {
-                       perror("ioctl");
-                       exit(EXIT_FAILURE);
+       /* Although we have a control message, we are not sure that the vsock
+        * transport has sent us notification that the buffer has been copied
+        * and cleared, so in some cases we may still see unsent bytes.
+        * Better to do a few iterations to be sure.
+        */
+       timeout_begin(TIMEOUT);
+       do {
+               ret = ioctl(fd, SIOCOUTQ, &sock_bytes_unsent);
+               if (ret < 0) {
+                       if (errno == EOPNOTSUPP) {
+                               fprintf(stderr, "Test skipped, SIOCOUTQ not supported.\n");
+                               break;
+                       } else {
+                               perror("ioctl");
+                               exit(EXIT_FAILURE);
+                       }
                 }
-       } else if (ret == 0 && sock_bytes_unsent != 0) {
-               fprintf(stderr,
-                       "Unexpected 'SIOCOUTQ' value, expected 0, got %i\n",
-                       sock_bytes_unsent);
-               exit(EXIT_FAILURE);
-       }
+       } while (sock_bytes_unsent != 0);
+       timeout_end();


What about combining the two?

Thanks,
Stefano