[PATCH net-next v2 1/3] vsock: Add support for SIOCINQ ioctl

Xuewei Niu posted 3 patches 3 months, 4 weeks ago
There is a newer version of this series
[PATCH net-next v2 1/3] vsock: Add support for SIOCINQ ioctl
Posted by Xuewei Niu 3 months, 4 weeks ago
This patch adds support for SIOCINQ ioctl, which returns the number of
bytes unread in the socket.

Signed-off-by: Xuewei Niu <niuxuewei.nxw@antgroup.com>
---
 include/net/af_vsock.h   |  2 ++
 net/vmw_vsock/af_vsock.c | 22 ++++++++++++++++++++++
 2 files changed, 24 insertions(+)

diff --git a/include/net/af_vsock.h b/include/net/af_vsock.h
index d56e6e135158..723a886253ba 100644
--- a/include/net/af_vsock.h
+++ b/include/net/af_vsock.h
@@ -171,6 +171,8 @@ struct vsock_transport {
 
 	/* SIOCOUTQ ioctl */
 	ssize_t (*unsent_bytes)(struct vsock_sock *vsk);
+	/* SIOCINQ ioctl */
+	ssize_t (*unread_bytes)(struct vsock_sock *vsk);
 
 	/* Shutdown. */
 	int (*shutdown)(struct vsock_sock *, int);
diff --git a/net/vmw_vsock/af_vsock.c b/net/vmw_vsock/af_vsock.c
index 2e7a3034e965..466b1ebadbbc 100644
--- a/net/vmw_vsock/af_vsock.c
+++ b/net/vmw_vsock/af_vsock.c
@@ -1389,6 +1389,28 @@ static int vsock_do_ioctl(struct socket *sock, unsigned int cmd,
 	vsk = vsock_sk(sk);
 
 	switch (cmd) {
+	case SIOCINQ: {
+		ssize_t n_bytes;
+
+		if (!vsk->transport || !vsk->transport->unread_bytes) {
+			ret = -EOPNOTSUPP;
+			break;
+		}
+
+		if (sock_type_connectible(sk->sk_type) &&
+		    sk->sk_state == TCP_LISTEN) {
+			ret = -EINVAL;
+			break;
+		}
+
+		n_bytes = vsk->transport->unread_bytes(vsk);
+		if (n_bytes < 0) {
+			ret = n_bytes;
+			break;
+		}
+		ret = put_user(n_bytes, arg);
+		break;
+	}
 	case SIOCOUTQ: {
 		ssize_t n_bytes;
 
-- 
2.34.1
Re: [PATCH net-next v2 1/3] vsock: Add support for SIOCINQ ioctl
Posted by Stefano Garzarella 3 months, 3 weeks ago
On Fri, Jun 13, 2025 at 11:11:50AM +0800, Xuewei Niu wrote:
>This patch adds support for SIOCINQ ioctl, which returns the number of
>bytes unread in the socket.
>
>Signed-off-by: Xuewei Niu <niuxuewei.nxw@antgroup.com>
>---
> include/net/af_vsock.h   |  2 ++
> net/vmw_vsock/af_vsock.c | 22 ++++++++++++++++++++++
> 2 files changed, 24 insertions(+)
>
>diff --git a/include/net/af_vsock.h b/include/net/af_vsock.h
>index d56e6e135158..723a886253ba 100644
>--- a/include/net/af_vsock.h
>+++ b/include/net/af_vsock.h
>@@ -171,6 +171,8 @@ struct vsock_transport {
>
> 	/* SIOCOUTQ ioctl */
> 	ssize_t (*unsent_bytes)(struct vsock_sock *vsk);
>+	/* SIOCINQ ioctl */
>+	ssize_t (*unread_bytes)(struct vsock_sock *vsk);

Instead of adding a new callback, can we just use 
`vsock_stream_has_data()` ?

Maybe adjusting it or changing something in the transports, but for 
virtio-vsock, it seems to me it does exactly what the new 
`virtio_transport_unread_bytes()` does, right?

Thanks,
Stefano

>
> 	/* Shutdown. */
> 	int (*shutdown)(struct vsock_sock *, int);
>diff --git a/net/vmw_vsock/af_vsock.c b/net/vmw_vsock/af_vsock.c
>index 2e7a3034e965..466b1ebadbbc 100644
>--- a/net/vmw_vsock/af_vsock.c
>+++ b/net/vmw_vsock/af_vsock.c
>@@ -1389,6 +1389,28 @@ static int vsock_do_ioctl(struct socket *sock, unsigned int cmd,
> 	vsk = vsock_sk(sk);
>
> 	switch (cmd) {
>+	case SIOCINQ: {
>+		ssize_t n_bytes;
>+
>+		if (!vsk->transport || !vsk->transport->unread_bytes) {
>+			ret = -EOPNOTSUPP;
>+			break;
>+		}
>+
>+		if (sock_type_connectible(sk->sk_type) &&
>+		    sk->sk_state == TCP_LISTEN) {
>+			ret = -EINVAL;
>+			break;
>+		}
>+
>+		n_bytes = vsk->transport->unread_bytes(vsk);
>+		if (n_bytes < 0) {
>+			ret = n_bytes;
>+			break;
>+		}
>+		ret = put_user(n_bytes, arg);
>+		break;
>+	}
> 	case SIOCOUTQ: {
> 		ssize_t n_bytes;
>
>-- 
>2.34.1
>
Re: [PATCH net-next v2 1/3] vsock: Add support for SIOCINQ ioctl
Posted by Xuewei Niu 3 months, 3 weeks ago
> On Fri, Jun 13, 2025 at 11:11:50AM +0800, Xuewei Niu wrote:
> >This patch adds support for SIOCINQ ioctl, which returns the number of
> >bytes unread in the socket.
> >
> >Signed-off-by: Xuewei Niu <niuxuewei.nxw@antgroup.com>
> >---
> > include/net/af_vsock.h   |  2 ++
> > net/vmw_vsock/af_vsock.c | 22 ++++++++++++++++++++++
> > 2 files changed, 24 insertions(+)
> >
> >diff --git a/include/net/af_vsock.h b/include/net/af_vsock.h
> >index d56e6e135158..723a886253ba 100644
> >--- a/include/net/af_vsock.h
> >+++ b/include/net/af_vsock.h
> >@@ -171,6 +171,8 @@ struct vsock_transport {
> >
> > 	/* SIOCOUTQ ioctl */
> > 	ssize_t (*unsent_bytes)(struct vsock_sock *vsk);
> >+	/* SIOCINQ ioctl */
> >+	ssize_t (*unread_bytes)(struct vsock_sock *vsk);
> 
> Instead of adding a new callback, can we just use 
> `vsock_stream_has_data()` ?
>
> Maybe adjusting it or changing something in the transports, but for 
> virtio-vsock, it seems to me it does exactly what the new 
> `virtio_transport_unread_bytes()` does, right?

Sorry, I forgot to update this.

I am curious that is there a plan to implement dgram support in
virtio-vsock? If yes, adding a new callback is the right way to go. I
deadly hope to see that feature. If no, will do in the next.

Thanks,
Xuewei

> Thanks,
> Stefano
> 
> >
> > 	/* Shutdown. */
> > 	int (*shutdown)(struct vsock_sock *, int);
> >diff --git a/net/vmw_vsock/af_vsock.c b/net/vmw_vsock/af_vsock.c
> >index 2e7a3034e965..466b1ebadbbc 100644
> >--- a/net/vmw_vsock/af_vsock.c
> >+++ b/net/vmw_vsock/af_vsock.c
> >@@ -1389,6 +1389,28 @@ static int vsock_do_ioctl(struct socket *sock, unsigned int cmd,
> > 	vsk = vsock_sk(sk);
> >
> > 	switch (cmd) {
> >+	case SIOCINQ: {
> >+		ssize_t n_bytes;
> >+
> >+		if (!vsk->transport || !vsk->transport->unread_bytes) {
> >+			ret = -EOPNOTSUPP;
> >+			break;
> >+		}
> >+
> >+		if (sock_type_connectible(sk->sk_type) &&
> >+		    sk->sk_state == TCP_LISTEN) {
> >+			ret = -EINVAL;
> >+			break;
> >+		}
> >+
> >+		n_bytes = vsk->transport->unread_bytes(vsk);
> >+		if (n_bytes < 0) {
> >+			ret = n_bytes;
> >+			break;
> >+		}
> >+		ret = put_user(n_bytes, arg);
> >+		break;
> >+	}
> > 	case SIOCOUTQ: {
> > 		ssize_t n_bytes;
> >
> >-- 
> >2.34.1
> >
Re: [PATCH net-next v2 1/3] vsock: Add support for SIOCINQ ioctl
Posted by Stefano Garzarella 3 months, 3 weeks ago
On Mon, Jun 16, 2025 at 10:28:22PM +0800, Xuewei Niu wrote:
>> On Fri, Jun 13, 2025 at 11:11:50AM +0800, Xuewei Niu wrote:
>> >This patch adds support for SIOCINQ ioctl, which returns the number of
>> >bytes unread in the socket.
>> >
>> >Signed-off-by: Xuewei Niu <niuxuewei.nxw@antgroup.com>
>> >---
>> > include/net/af_vsock.h   |  2 ++
>> > net/vmw_vsock/af_vsock.c | 22 ++++++++++++++++++++++
>> > 2 files changed, 24 insertions(+)
>> >
>> >diff --git a/include/net/af_vsock.h b/include/net/af_vsock.h
>> >index d56e6e135158..723a886253ba 100644
>> >--- a/include/net/af_vsock.h
>> >+++ b/include/net/af_vsock.h
>> >@@ -171,6 +171,8 @@ struct vsock_transport {
>> >
>> > 	/* SIOCOUTQ ioctl */
>> > 	ssize_t (*unsent_bytes)(struct vsock_sock *vsk);
>> >+	/* SIOCINQ ioctl */
>> >+	ssize_t (*unread_bytes)(struct vsock_sock *vsk);
>>
>> Instead of adding a new callback, can we just use
>> `vsock_stream_has_data()` ?
>>
>> Maybe adjusting it or changing something in the transports, but for
>> virtio-vsock, it seems to me it does exactly what the new
>> `virtio_transport_unread_bytes()` does, right?
>
>Sorry, I forgot to update this.

Don't worry.

>
>I am curious that is there a plan to implement dgram support in
>virtio-vsock? If yes, adding a new callback is the right way to go. I
>deadly hope to see that feature. If no, will do in the next.

I don't know the status, there were folks working on it, but I didn't 
see updates.

IMO we can deal with it later, since also this patch will not work as it 
is with datagram since you're checking if the socket is "connectible" 
and also a state. And maybe we also need some 
"vsock_datagram_has_data()" anyway, so let's do this when we will have 
the support. For now let's reuse what we have as much as we can.

Thanks.
Stefano

>
>Thanks,
>Xuewei
>
>> Thanks,
>> Stefano
>>
>> >
>> > 	/* Shutdown. */
>> > 	int (*shutdown)(struct vsock_sock *, int);
>> >diff --git a/net/vmw_vsock/af_vsock.c b/net/vmw_vsock/af_vsock.c
>> >index 2e7a3034e965..466b1ebadbbc 100644
>> >--- a/net/vmw_vsock/af_vsock.c
>> >+++ b/net/vmw_vsock/af_vsock.c
>> >@@ -1389,6 +1389,28 @@ static int vsock_do_ioctl(struct socket *sock, unsigned int cmd,
>> > 	vsk = vsock_sk(sk);
>> >
>> > 	switch (cmd) {
>> >+	case SIOCINQ: {
>> >+		ssize_t n_bytes;
>> >+
>> >+		if (!vsk->transport || !vsk->transport->unread_bytes) {
>> >+			ret = -EOPNOTSUPP;
>> >+			break;
>> >+		}
>> >+
>> >+		if (sock_type_connectible(sk->sk_type) &&
>> >+		    sk->sk_state == TCP_LISTEN) {
>> >+			ret = -EINVAL;
>> >+			break;
>> >+		}
>> >+
>> >+		n_bytes = vsk->transport->unread_bytes(vsk);
>> >+		if (n_bytes < 0) {
>> >+			ret = n_bytes;
>> >+			break;
>> >+		}
>> >+		ret = put_user(n_bytes, arg);
>> >+		break;
>> >+	}
>> > 	case SIOCOUTQ: {
>> > 		ssize_t n_bytes;
>> >
>> >--
>> >2.34.1
>> >
>
Re: [PATCH net-next v2 1/3] vsock: Add support for SIOCINQ ioctl
Posted by Luigi Leonardi 3 months, 3 weeks ago
On Fri, Jun 13, 2025 at 11:11:50AM +0800, Xuewei Niu wrote:
>This patch adds support for SIOCINQ ioctl, which returns the number of
>bytes unread in the socket.
>
>Signed-off-by: Xuewei Niu <niuxuewei.nxw@antgroup.com>
>---
> include/net/af_vsock.h   |  2 ++
> net/vmw_vsock/af_vsock.c | 22 ++++++++++++++++++++++
> 2 files changed, 24 insertions(+)
>
>diff --git a/include/net/af_vsock.h b/include/net/af_vsock.h
>index d56e6e135158..723a886253ba 100644
>--- a/include/net/af_vsock.h
>+++ b/include/net/af_vsock.h
>@@ -171,6 +171,8 @@ struct vsock_transport {
>
> 	/* SIOCOUTQ ioctl */
> 	ssize_t (*unsent_bytes)(struct vsock_sock *vsk);
>+	/* SIOCINQ ioctl */
>+	ssize_t (*unread_bytes)(struct vsock_sock *vsk);
>
> 	/* Shutdown. */
> 	int (*shutdown)(struct vsock_sock *, int);
>diff --git a/net/vmw_vsock/af_vsock.c b/net/vmw_vsock/af_vsock.c
>index 2e7a3034e965..466b1ebadbbc 100644
>--- a/net/vmw_vsock/af_vsock.c
>+++ b/net/vmw_vsock/af_vsock.c
>@@ -1389,6 +1389,28 @@ static int vsock_do_ioctl(struct socket *sock, unsigned int cmd,
> 	vsk = vsock_sk(sk);
>
> 	switch (cmd) {
>+	case SIOCINQ: {
>+		ssize_t n_bytes;
>+
>+		if (!vsk->transport || !vsk->transport->unread_bytes) {
>+			ret = -EOPNOTSUPP;
>+			break;
>+		}
>+
>+		if (sock_type_connectible(sk->sk_type) &&
>+		    sk->sk_state == TCP_LISTEN) {
>+			ret = -EINVAL;
>+			break;
>+		}
>+
>+		n_bytes = vsk->transport->unread_bytes(vsk);
>+		if (n_bytes < 0) {
>+			ret = n_bytes;
>+			break;
>+		}
>+		ret = put_user(n_bytes, arg);
>+		break;
>+	}
> 	case SIOCOUTQ: {
> 		ssize_t n_bytes;
>
>-- 
>2.34.1
>

Reviewed-by: Luigi Leonardi <leonardi@redhat.com>
Re: [PATCH net-next v2 1/3] vsock: Add support for SIOCINQ ioctl
Posted by Luigi Leonardi 3 months, 3 weeks ago
On Mon, Jun 16, 2025 at 03:42:53PM +0200, Luigi Leonardi wrote:
>On Fri, Jun 13, 2025 at 11:11:50AM +0800, Xuewei Niu wrote:
>>This patch adds support for SIOCINQ ioctl, which returns the number of
>>bytes unread in the socket.
>>
>>Signed-off-by: Xuewei Niu <niuxuewei.nxw@antgroup.com>
>>---
>>include/net/af_vsock.h   |  2 ++
>>net/vmw_vsock/af_vsock.c | 22 ++++++++++++++++++++++
>>2 files changed, 24 insertions(+)
>>
>>diff --git a/include/net/af_vsock.h b/include/net/af_vsock.h
>>index d56e6e135158..723a886253ba 100644
>>--- a/include/net/af_vsock.h
>>+++ b/include/net/af_vsock.h
>>@@ -171,6 +171,8 @@ struct vsock_transport {
>>
>>	/* SIOCOUTQ ioctl */
>>	ssize_t (*unsent_bytes)(struct vsock_sock *vsk);
>>+	/* SIOCINQ ioctl */
>>+	ssize_t (*unread_bytes)(struct vsock_sock *vsk);
>>
>>	/* Shutdown. */
>>	int (*shutdown)(struct vsock_sock *, int);
>>diff --git a/net/vmw_vsock/af_vsock.c b/net/vmw_vsock/af_vsock.c
>>index 2e7a3034e965..466b1ebadbbc 100644
>>--- a/net/vmw_vsock/af_vsock.c
>>+++ b/net/vmw_vsock/af_vsock.c
>>@@ -1389,6 +1389,28 @@ static int vsock_do_ioctl(struct socket *sock, unsigned int cmd,
>>	vsk = vsock_sk(sk);
>>
>>	switch (cmd) {
>>+	case SIOCINQ: {
>>+		ssize_t n_bytes;
>>+
>>+		if (!vsk->transport || !vsk->transport->unread_bytes) {
>>+			ret = -EOPNOTSUPP;
>>+			break;
>>+		}
>>+
>>+		if (sock_type_connectible(sk->sk_type) &&
>>+		    sk->sk_state == TCP_LISTEN) {
>>+			ret = -EINVAL;
>>+			break;
>>+		}
>>+
>>+		n_bytes = vsk->transport->unread_bytes(vsk);
>>+		if (n_bytes < 0) {
>>+			ret = n_bytes;
>>+			break;
>>+		}
>>+		ret = put_user(n_bytes, arg);
>>+		break;
>>+	}
>>	case SIOCOUTQ: {
>>		ssize_t n_bytes;
>>
>>-- 
>>2.34.1
>>
>
>Reviewed-by: Luigi Leonardi <leonardi@redhat.com>

Stefano is totally right, reusing `virtio_transport_unread_bytes` is a 
good idea.

nit: commit message should use 'imperative' language [1]. "This patch 
adds" should be avoided.

Sorry for the confusion.

Thanks,
Luigi

[1]https://www.kernel.org/doc/html/latest/process/submitting-patches.html#describe-your-changes
Re: [PATCH net-next v2 1/3] vsock: Add support for SIOCINQ ioctl
Posted by Xuewei Niu 3 months, 3 weeks ago
> On Mon, Jun 16, 2025 at 03:42:53PM +0200, Luigi Leonardi wrote:
> >On Fri, Jun 13, 2025 at 11:11:50AM +0800, Xuewei Niu wrote:
> >>This patch adds support for SIOCINQ ioctl, which returns the number of
> >>bytes unread in the socket.
> >>
> >>Signed-off-by: Xuewei Niu <niuxuewei.nxw@antgroup.com>
> >>---
> >>include/net/af_vsock.h   |  2 ++
> >>net/vmw_vsock/af_vsock.c | 22 ++++++++++++++++++++++
> >>2 files changed, 24 insertions(+)
> >>
> >>diff --git a/include/net/af_vsock.h b/include/net/af_vsock.h
> >>index d56e6e135158..723a886253ba 100644
> >>--- a/include/net/af_vsock.h
> >>+++ b/include/net/af_vsock.h
> >>@@ -171,6 +171,8 @@ struct vsock_transport {
> >>
> >>	/* SIOCOUTQ ioctl */
> >>	ssize_t (*unsent_bytes)(struct vsock_sock *vsk);
> >>+	/* SIOCINQ ioctl */
> >>+	ssize_t (*unread_bytes)(struct vsock_sock *vsk);
> >>
> >>	/* Shutdown. */
> >>	int (*shutdown)(struct vsock_sock *, int);
> >>diff --git a/net/vmw_vsock/af_vsock.c b/net/vmw_vsock/af_vsock.c
> >>index 2e7a3034e965..466b1ebadbbc 100644
> >>--- a/net/vmw_vsock/af_vsock.c
> >>+++ b/net/vmw_vsock/af_vsock.c
> >>@@ -1389,6 +1389,28 @@ static int vsock_do_ioctl(struct socket *sock, unsigned int cmd,
> >>	vsk = vsock_sk(sk);
> >>
> >>	switch (cmd) {
> >>+	case SIOCINQ: {
> >>+		ssize_t n_bytes;
> >>+
> >>+		if (!vsk->transport || !vsk->transport->unread_bytes) {
> >>+			ret = -EOPNOTSUPP;
> >>+			break;
> >>+		}
> >>+
> >>+		if (sock_type_connectible(sk->sk_type) &&
> >>+		    sk->sk_state == TCP_LISTEN) {
> >>+			ret = -EINVAL;
> >>+			break;
> >>+		}
> >>+
> >>+		n_bytes = vsk->transport->unread_bytes(vsk);
> >>+		if (n_bytes < 0) {
> >>+			ret = n_bytes;
> >>+			break;
> >>+		}
> >>+		ret = put_user(n_bytes, arg);
> >>+		break;
> >>+	}
> >>	case SIOCOUTQ: {
> >>		ssize_t n_bytes;
> >>
> >>-- 
> >>2.34.1
> >>
> >
> >Reviewed-by: Luigi Leonardi <leonardi@redhat.com>
> 
> Stefano is totally right, reusing `virtio_transport_unread_bytes` is a 
> good idea.
> 
> nit: commit message should use 'imperative' language [1]. "This patch 
> adds" should be avoided.
> 
> Sorry for the confusion.
> 
> Thanks,
> Luigi
> 
> [1]https://www.kernel.org/doc/html/latest/process/submitting-patches.html#describe-your-changes

Thanks for pointing out. I'll update the commit message following the
guidelines.

Thanks,
Xuewei