[RESEND PATCH net-next v4 3/4] test/vsock: Add retry mechanism to ioctl wrapper

Xuewei Niu posted 4 patches 3 months, 1 week ago
There is a newer version of this series
[RESEND PATCH net-next v4 3/4] test/vsock: Add retry mechanism to ioctl wrapper
Posted by Xuewei Niu 3 months, 1 week ago
Wrap the ioctl in `ioctl_int()`, which takes a pointer to the actual
int value and an expected int value. The function will not return until
either the ioctl returns the expected value or a timeout occurs, thus
avoiding immediate failure.

Signed-off-by: Xuewei Niu <niuxuewei.nxw@antgroup.com>
---
 tools/testing/vsock/util.c | 32 +++++++++++++++++++++++---------
 tools/testing/vsock/util.h |  1 +
 2 files changed, 24 insertions(+), 9 deletions(-)

diff --git a/tools/testing/vsock/util.c b/tools/testing/vsock/util.c
index 0c7e9cbcbc85..481c395227e4 100644
--- a/tools/testing/vsock/util.c
+++ b/tools/testing/vsock/util.c
@@ -16,6 +16,7 @@
 #include <unistd.h>
 #include <assert.h>
 #include <sys/epoll.h>
+#include <sys/ioctl.h>
 #include <sys/mman.h>
 #include <linux/sockios.h>
 
@@ -97,28 +98,41 @@ void vsock_wait_remote_close(int fd)
 	close(epollfd);
 }
 
-/* Wait until transport reports no data left to be sent.
- * Return false if transport does not implement the unsent_bytes() callback.
+/* Wait until ioctl gives an expected int value.
+ * Return false if the op is not supported.
  */
-bool vsock_wait_sent(int fd)
+bool vsock_ioctl_int(int fd, unsigned long op, int *actual, int expected)
 {
-	int ret, sock_bytes_unsent;
+	int ret;
+	char name[32];
+
+	snprintf(name, sizeof(name), "ioctl(%lu)", op);
 
 	timeout_begin(TIMEOUT);
 	do {
-		ret = ioctl(fd, SIOCOUTQ, &sock_bytes_unsent);
+		ret = ioctl(fd, op, actual);
 		if (ret < 0) {
 			if (errno == EOPNOTSUPP)
 				break;
 
-			perror("ioctl(SIOCOUTQ)");
+			perror(name);
 			exit(EXIT_FAILURE);
 		}
-		timeout_check("SIOCOUTQ");
-	} while (sock_bytes_unsent != 0);
+		timeout_check(name);
+	} while (*actual != expected);
 	timeout_end();
 
-	return !ret;
+	return ret >= 0;
+}
+
+/* Wait until transport reports no data left to be sent.
+ * Return false if transport does not implement the unsent_bytes() callback.
+ */
+bool vsock_wait_sent(int fd)
+{
+	int sock_bytes_unsent;
+
+	return vsock_ioctl_int(fd, SIOCOUTQ, &sock_bytes_unsent, 0);
 }
 
 /* Create socket <type>, bind to <cid, port> and return the file descriptor. */
diff --git a/tools/testing/vsock/util.h b/tools/testing/vsock/util.h
index 5e2db67072d5..d59581f68d61 100644
--- a/tools/testing/vsock/util.h
+++ b/tools/testing/vsock/util.h
@@ -54,6 +54,7 @@ int vsock_stream_listen(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);
+bool vsock_ioctl_int(int fd, unsigned long op, int *actual, int expected);
 bool vsock_wait_sent(int fd);
 void send_buf(int fd, const void *buf, size_t len, int flags,
 	      ssize_t expected_ret);
-- 
2.34.1
Re: [RESEND PATCH net-next v4 3/4] test/vsock: Add retry mechanism to ioctl wrapper
Posted by Stefano Garzarella 3 months, 1 week ago
On Mon, Jun 30, 2025 at 03:57:26PM +0800, Xuewei Niu wrote:
>Wrap the ioctl in `ioctl_int()`, which takes a pointer to the actual
>int value and an expected int value. The function will not return until
>either the ioctl returns the expected value or a timeout occurs, thus
>avoiding immediate failure.
>
>Signed-off-by: Xuewei Niu <niuxuewei.nxw@antgroup.com>
>---
> tools/testing/vsock/util.c | 32 +++++++++++++++++++++++---------
> tools/testing/vsock/util.h |  1 +
> 2 files changed, 24 insertions(+), 9 deletions(-)
>
>diff --git a/tools/testing/vsock/util.c b/tools/testing/vsock/util.c
>index 0c7e9cbcbc85..481c395227e4 100644
>--- a/tools/testing/vsock/util.c
>+++ b/tools/testing/vsock/util.c
>@@ -16,6 +16,7 @@
> #include <unistd.h>
> #include <assert.h>
> #include <sys/epoll.h>
>+#include <sys/ioctl.h>
> #include <sys/mman.h>
> #include <linux/sockios.h>
>
>@@ -97,28 +98,41 @@ void vsock_wait_remote_close(int fd)
> 	close(epollfd);
> }
>
>-/* Wait until transport reports no data left to be sent.
>- * Return false if transport does not implement the unsent_bytes() 
>callback.
>+/* Wait until ioctl gives an expected int value.
>+ * Return false if the op is not supported.
>  */
>-bool vsock_wait_sent(int fd)
>+bool vsock_ioctl_int(int fd, unsigned long op, int *actual, int expected)

Why we need the `actual` parameter?

> {
>-	int ret, sock_bytes_unsent;
>+	int ret;
>+	char name[32];
>+
>+	snprintf(name, sizeof(name), "ioctl(%lu)", op);
>
> 	timeout_begin(TIMEOUT);
> 	do {
>-		ret = ioctl(fd, SIOCOUTQ, &sock_bytes_unsent);
>+		ret = ioctl(fd, op, actual);
> 		if (ret < 0) {
> 			if (errno == EOPNOTSUPP)
> 				break;
>
>-			perror("ioctl(SIOCOUTQ)");
>+			perror(name);
> 			exit(EXIT_FAILURE);
> 		}
>-		timeout_check("SIOCOUTQ");
>-	} while (sock_bytes_unsent != 0);
>+		timeout_check(name);
>+	} while (*actual != expected);
> 	timeout_end();
>
>-	return !ret;
>+	return ret >= 0;
>+}
>+
>+/* Wait until transport reports no data left to be sent.
>+ * Return false if transport does not implement the unsent_bytes() callback.
>+ */
>+bool vsock_wait_sent(int fd)
>+{
>+	int sock_bytes_unsent;
>+
>+	return vsock_ioctl_int(fd, SIOCOUTQ, &sock_bytes_unsent, 0);
> }
>
> /* Create socket <type>, bind to <cid, port> and return the file descriptor. */
>diff --git a/tools/testing/vsock/util.h b/tools/testing/vsock/util.h
>index 5e2db67072d5..d59581f68d61 100644
>--- a/tools/testing/vsock/util.h
>+++ b/tools/testing/vsock/util.h
>@@ -54,6 +54,7 @@ int vsock_stream_listen(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);
>+bool vsock_ioctl_int(int fd, unsigned long op, int *actual, int expected);
> bool vsock_wait_sent(int fd);
> void send_buf(int fd, const void *buf, size_t len, int flags,
> 	      ssize_t expected_ret);
>-- 
>2.34.1
>
Re: [RESEND PATCH net-next v4 3/4] test/vsock: Add retry mechanism to ioctl wrapper
Posted by Xuewei Niu 3 months, 1 week ago
Resend: the previous message was rejected due to HTML
Resend: forgot to reply all...

> On Mon, Jun 30, 2025 at 03:57:26PM +0800, Xuewei Niu wrote:
> >Wrap the ioctl in `ioctl_int()`, which takes a pointer to the actual
> >int value and an expected int value. The function will not return until
> >either the ioctl returns the expected value or a timeout occurs, thus
> >avoiding immediate failure.
> >
> >Signed-off-by: Xuewei Niu <niuxuewei.nxw@antgroup.com>
> >---
> > tools/testing/vsock/util.c | 32 +++++++++++++++++++++++---------
> > tools/testing/vsock/util.h |  1 +
> > 2 files changed, 24 insertions(+), 9 deletions(-)
> >
> >diff --git a/tools/testing/vsock/util.c b/tools/testing/vsock/util.c
> >index 0c7e9cbcbc85..481c395227e4 100644
> >--- a/tools/testing/vsock/util.c
> >+++ b/tools/testing/vsock/util.c
> >@@ -16,6 +16,7 @@
> > #include <unistd.h>
> > #include <assert.h>
> > #include <sys/epoll.h>
> >+#include <sys/ioctl.h>
> > #include <sys/mman.h>
> > #include <linux/sockios.h>
> >
> >@@ -97,28 +98,41 @@ void vsock_wait_remote_close(int fd)
> > 	close(epollfd);
> > }
> >
> >-/* Wait until transport reports no data left to be sent.
> >- * Return false if transport does not implement the unsent_bytes() 
> >callback.
> >+/* Wait until ioctl gives an expected int value.
> >+ * Return false if the op is not supported.
> >  */
> >-bool vsock_wait_sent(int fd)
> >+bool vsock_ioctl_int(int fd, unsigned long op, int *actual, int expected)
> 
> Why we need the `actual` parameter?

We can exit early `if (*actual == expected)`, and the `expected` can be any integer.
I also make it to be a pointer, because the caller might need to have the actual value.

Thanks,
Xuewei
 
> > {
> >-	int ret, sock_bytes_unsent;
> >+	int ret;
> >+	char name[32];
> >+
> >+	snprintf(name, sizeof(name), "ioctl(%lu)", op);
> >
> > 	timeout_begin(TIMEOUT);
> > 	do {
> >-		ret = ioctl(fd, SIOCOUTQ, &sock_bytes_unsent);
> >+		ret = ioctl(fd, op, actual);
> > 		if (ret < 0) {
> > 			if (errno == EOPNOTSUPP)
> > 				break;
> >
> >-			perror("ioctl(SIOCOUTQ)");
> >+			perror(name);
> > 			exit(EXIT_FAILURE);
> > 		}
> >-		timeout_check("SIOCOUTQ");
> >-	} while (sock_bytes_unsent != 0);
> >+		timeout_check(name);
> >+	} while (*actual != expected);
> > 	timeout_end();
> >
> >-	return !ret;
> >+	return ret >= 0;
> >+}
> >+
> >+/* Wait until transport reports no data left to be sent.
> >+ * Return false if transport does not implement the unsent_bytes() callback.
> >+ */
> >+bool vsock_wait_sent(int fd)
> >+{
> >+	int sock_bytes_unsent;
> >+
> >+	return vsock_ioctl_int(fd, SIOCOUTQ, &sock_bytes_unsent, 0);
> > }
> >
> > /* Create socket <type>, bind to <cid, port> and return the file descriptor. */
> >diff --git a/tools/testing/vsock/util.h b/tools/testing/vsock/util.h
> >index 5e2db67072d5..d59581f68d61 100644
> >--- a/tools/testing/vsock/util.h
> >+++ b/tools/testing/vsock/util.h
> >@@ -54,6 +54,7 @@ int vsock_stream_listen(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);
> >+bool vsock_ioctl_int(int fd, unsigned long op, int *actual, int expected);
> > bool vsock_wait_sent(int fd);
> > void send_buf(int fd, const void *buf, size_t len, int flags,
> > 	      ssize_t expected_ret);
> >-- 
> >2.34.1
> >
Re: [RESEND PATCH net-next v4 3/4] test/vsock: Add retry mechanism to ioctl wrapper
Posted by Stefano Garzarella 3 months, 1 week ago
On Thu, Jul 03, 2025 at 11:05:14AM +0800, Xuewei Niu wrote:
>Resend: the previous message was rejected due to HTML
>Resend: forgot to reply all...
>
>> On Mon, Jun 30, 2025 at 03:57:26PM +0800, Xuewei Niu wrote:
>> >Wrap the ioctl in `ioctl_int()`, which takes a pointer to the actual
>> >int value and an expected int value. The function will not return until
>> >either the ioctl returns the expected value or a timeout occurs, thus
>> >avoiding immediate failure.
>> >
>> >Signed-off-by: Xuewei Niu <niuxuewei.nxw@antgroup.com>
>> >---
>> > tools/testing/vsock/util.c | 32 +++++++++++++++++++++++---------
>> > tools/testing/vsock/util.h |  1 +
>> > 2 files changed, 24 insertions(+), 9 deletions(-)
>> >
>> >diff --git a/tools/testing/vsock/util.c b/tools/testing/vsock/util.c
>> >index 0c7e9cbcbc85..481c395227e4 100644
>> >--- a/tools/testing/vsock/util.c
>> >+++ b/tools/testing/vsock/util.c
>> >@@ -16,6 +16,7 @@
>> > #include <unistd.h>
>> > #include <assert.h>
>> > #include <sys/epoll.h>
>> >+#include <sys/ioctl.h>
>> > #include <sys/mman.h>
>> > #include <linux/sockios.h>
>> >
>> >@@ -97,28 +98,41 @@ void vsock_wait_remote_close(int fd)
>> > 	close(epollfd);
>> > }
>> >
>> >-/* Wait until transport reports no data left to be sent.
>> >- * Return false if transport does not implement the unsent_bytes()
>> >callback.
>> >+/* Wait until ioctl gives an expected int value.
>> >+ * Return false if the op is not supported.
>> >  */
>> >-bool vsock_wait_sent(int fd)
>> >+bool vsock_ioctl_int(int fd, unsigned long op, int *actual, int expected)
>>
>> Why we need the `actual` parameter?
>
>We can exit early `if (*actual == expected)`, and the `expected` can be any integer.
>I also make it to be a pointer, because the caller might need to have the actual value.

IIUC this function return true if `*actual == expected` or false if 
there was an error, so I don't see the point of aving `actual`, since it 
can only be equal to `expected` if it returns true, or invalid if it 
returs false.

Thanks,
Stefano

>
>Thanks,
>Xuewei
>
>> > {
>> >-	int ret, sock_bytes_unsent;
>> >+	int ret;
>> >+	char name[32];
>> >+
>> >+	snprintf(name, sizeof(name), "ioctl(%lu)", op);
>> >
>> > 	timeout_begin(TIMEOUT);
>> > 	do {
>> >-		ret = ioctl(fd, SIOCOUTQ, &sock_bytes_unsent);
>> >+		ret = ioctl(fd, op, actual);
>> > 		if (ret < 0) {
>> > 			if (errno == EOPNOTSUPP)
>> > 				break;
>> >
>> >-			perror("ioctl(SIOCOUTQ)");
>> >+			perror(name);
>> > 			exit(EXIT_FAILURE);
>> > 		}
>> >-		timeout_check("SIOCOUTQ");
>> >-	} while (sock_bytes_unsent != 0);
>> >+		timeout_check(name);
>> >+	} while (*actual != expected);
>> > 	timeout_end();
>> >
>> >-	return !ret;
>> >+	return ret >= 0;
>> >+}
>> >+
>> >+/* Wait until transport reports no data left to be sent.
>> >+ * Return false if transport does not implement the unsent_bytes() callback.
>> >+ */
>> >+bool vsock_wait_sent(int fd)
>> >+{
>> >+	int sock_bytes_unsent;
>> >+
>> >+	return vsock_ioctl_int(fd, SIOCOUTQ, &sock_bytes_unsent, 0);
>> > }
>> >
>> > /* Create socket <type>, bind to <cid, port> and return the file descriptor. */
>> >diff --git a/tools/testing/vsock/util.h b/tools/testing/vsock/util.h
>> >index 5e2db67072d5..d59581f68d61 100644
>> >--- a/tools/testing/vsock/util.h
>> >+++ b/tools/testing/vsock/util.h
>> >@@ -54,6 +54,7 @@ int vsock_stream_listen(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);
>> >+bool vsock_ioctl_int(int fd, unsigned long op, int *actual, int expected);
>> > bool vsock_wait_sent(int fd);
>> > void send_buf(int fd, const void *buf, size_t len, int flags,
>> > 	      ssize_t expected_ret);
>> >--
>> >2.34.1
>> >
>
Re: [RESEND PATCH net-next v4 3/4] test/vsock: Add retry mechanism to ioctl wrapper
Posted by Xuewei Niu 3 months, 1 week ago
> On Thu, Jul 03, 2025 at 11:05:14AM +0800, Xuewei Niu wrote:
> >Resend: the previous message was rejected due to HTML
> >Resend: forgot to reply all...
> >
> >> On Mon, Jun 30, 2025 at 03:57:26PM +0800, Xuewei Niu wrote:
> >> >Wrap the ioctl in `ioctl_int()`, which takes a pointer to the actual
> >> >int value and an expected int value. The function will not return until
> >> >either the ioctl returns the expected value or a timeout occurs, thus
> >> >avoiding immediate failure.
> >> >
> >> >Signed-off-by: Xuewei Niu <niuxuewei.nxw@antgroup.com>
> >> >---
> >> > tools/testing/vsock/util.c | 32 +++++++++++++++++++++++---------
> >> > tools/testing/vsock/util.h |  1 +
> >> > 2 files changed, 24 insertions(+), 9 deletions(-)
> >> >
> >> >diff --git a/tools/testing/vsock/util.c b/tools/testing/vsock/util.c
> >> >index 0c7e9cbcbc85..481c395227e4 100644
> >> >--- a/tools/testing/vsock/util.c
> >> >+++ b/tools/testing/vsock/util.c
> >> >@@ -16,6 +16,7 @@
> >> > #include <unistd.h>
> >> > #include <assert.h>
> >> > #include <sys/epoll.h>
> >> >+#include <sys/ioctl.h>
> >> > #include <sys/mman.h>
> >> > #include <linux/sockios.h>
> >> >
> >> >@@ -97,28 +98,41 @@ void vsock_wait_remote_close(int fd)
> >> > 	close(epollfd);
> >> > }
> >> >
> >> >-/* Wait until transport reports no data left to be sent.
> >> >- * Return false if transport does not implement the unsent_bytes()
> >> >callback.
> >> >+/* Wait until ioctl gives an expected int value.
> >> >+ * Return false if the op is not supported.
> >> >  */
> >> >-bool vsock_wait_sent(int fd)
> >> >+bool vsock_ioctl_int(int fd, unsigned long op, int *actual, int expected)
> >>
> >> Why we need the `actual` parameter?
> >
> >We can exit early `if (*actual == expected)`, and the `expected` can be any integer.
> >I also make it to be a pointer, because the caller might need to have the actual value.
> 
> IIUC this function return true if `*actual == expected` or false if 
> there was an error, so I don't see the point of aving `actual`, since it 
> can only be equal to `expected` if it returns true, or invalid if it 
> returs false.

Nice catch! I'll remove it in v5.

Thanks,
Xuewei

> Thanks,
> Stefano
> 
> >
> >Thanks,
> >Xuewei
> >
> >> > {
> >> >-	int ret, sock_bytes_unsent;
> >> >+	int ret;
> >> >+	char name[32];
> >> >+
> >> >+	snprintf(name, sizeof(name), "ioctl(%lu)", op);
> >> >
> >> > 	timeout_begin(TIMEOUT);
> >> > 	do {
> >> >-		ret = ioctl(fd, SIOCOUTQ, &sock_bytes_unsent);
> >> >+		ret = ioctl(fd, op, actual);
> >> > 		if (ret < 0) {
> >> > 			if (errno == EOPNOTSUPP)
> >> > 				break;
> >> >
> >> >-			perror("ioctl(SIOCOUTQ)");
> >> >+			perror(name);
> >> > 			exit(EXIT_FAILURE);
> >> > 		}
> >> >-		timeout_check("SIOCOUTQ");
> >> >-	} while (sock_bytes_unsent != 0);
> >> >+		timeout_check(name);
> >> >+	} while (*actual != expected);
> >> > 	timeout_end();
> >> >
> >> >-	return !ret;
> >> >+	return ret >= 0;
> >> >+}
> >> >+
> >> >+/* Wait until transport reports no data left to be sent.
> >> >+ * Return false if transport does not implement the unsent_bytes() callback.
> >> >+ */
> >> >+bool vsock_wait_sent(int fd)
> >> >+{
> >> >+	int sock_bytes_unsent;
> >> >+
> >> >+	return vsock_ioctl_int(fd, SIOCOUTQ, &sock_bytes_unsent, 0);
> >> > }
> >> >
> >> > /* Create socket <type>, bind to <cid, port> and return the file descriptor. */
> >> >diff --git a/tools/testing/vsock/util.h b/tools/testing/vsock/util.h
> >> >index 5e2db67072d5..d59581f68d61 100644
> >> >--- a/tools/testing/vsock/util.h
> >> >+++ b/tools/testing/vsock/util.h
> >> >@@ -54,6 +54,7 @@ int vsock_stream_listen(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);
> >> >+bool vsock_ioctl_int(int fd, unsigned long op, int *actual, int expected);
> >> > bool vsock_wait_sent(int fd);
> >> > void send_buf(int fd, const void *buf, size_t len, int flags,
> >> > 	      ssize_t expected_ret);
> >> >--
> >> >2.34.1
> >> >
> >