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

[Stefano Garzarella]

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@xxxxxxxxxxxx>
> > >---
> > > 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
> > >