On Tue, Apr 02, 2024 at 05:05:37PM +0200, Luigi Leonardi wrote:
This add support for ioctl(s) for SOCK_STREAM SOCK_SEQPACKET and SOCK_DGRAM in AF_VSOCK. The only ioctl available is SIOCOUTQ/TIOCOUTQ, which returns the number of unsent bytes in the socket. This information is transport-specific and is delegated to them using a callback. Suggested-by: Daan De Meyer <daan.j.demeyer@xxxxxxxxx> Signed-off-by: Luigi Leonardi <luigi.leonardi@xxxxxxxxxxx> --- include/net/af_vsock.h | 1 + net/vmw_vsock/af_vsock.c | 42 +++++++++++++++++++++++++++++++++++++--- 2 files changed, 40 insertions(+), 3 deletions(-) diff --git a/include/net/af_vsock.h b/include/net/af_vsock.h index 535701efc1e5..cd4311abd3c9 100644 --- a/include/net/af_vsock.h +++ b/include/net/af_vsock.h @@ -137,6 +137,7 @@ struct vsock_transport { u64 (*stream_rcvhiwat)(struct vsock_sock *); bool (*stream_is_active)(struct vsock_sock *); bool (*stream_allow)(u32 cid, u32 port); + int (*stream_bytes_unsent)(struct vsock_sock *vsk); /* SEQ_PACKET. */ ssize_t (*seqpacket_dequeue)(struct vsock_sock *vsk, struct msghdr *msg, diff --git a/net/vmw_vsock/af_vsock.c b/net/vmw_vsock/af_vsock.c index 54ba7316f808..991e9edfa743 100644 --- a/net/vmw_vsock/af_vsock.c +++ b/net/vmw_vsock/af_vsock.c @@ -112,6 +112,7 @@ #include <net/sock.h> #include <net/af_vsock.h> #include <uapi/linux/vm_sockets.h> +#include <uapi/asm-generic/ioctls.h> static int __vsock_bind(struct sock *sk, struct sockaddr_vm *addr); static void vsock_sk_destruct(struct sock *sk); @@ -1292,6 +1293,41 @@ int vsock_dgram_recvmsg(struct socket *sock, struct msghdr *msg, } EXPORT_SYMBOL_GPL(vsock_dgram_recvmsg); +static int vsock_do_ioctl(struct socket *sock, unsigned int cmd, + int __user *arg) +{ + struct sock *sk = sock->sk; + int retval = -EOPNOTSUPP; + struct vsock_sock *vsk; + + vsk = vsock_sk(sk); + + switch (cmd) { + case SIOCOUTQ: + if (vsk->transport->stream_bytes_unsent) { + if (sk->sk_state == TCP_LISTEN)
Maybe we should do this check only if `sock_type_connectible(sk->sk_type)` is true.
+ return -EINVAL; + retval = vsk->transport->stream_bytes_unsent(vsk);
IIUC `stream_bytes_unsent()` is used for any type of socket, so I suggest using a name more generic, something like `unsent_bytes()`.
+ } + break; + default: + retval = -EOPNOTSUPP;
Should we return -ENOIOCTLCMD in this case?
+ } + + if (retval >= 0) {
IMHO mixing the the ioctl() return value and the value itself is confusing. I also suggest to move the put_user() in the `case SIOCOUTQ`, other ioctls may no needs any value to copy to userspace.
+ put_user(retval, arg);
put_user() can fail, we should return the failure to the user.
+ retval = 0; + } + + return retval; +} + +static int vsock_ioctl(struct socket *sock, unsigned int cmd, + unsigned long arg) +{ + return vsock_do_ioctl(sock, cmd, (int __user *)arg); +} + static const struct proto_ops vsock_dgram_ops = { .family = PF_VSOCK, .owner = THIS_MODULE, @@ -1302,7 +1338,7 @@ static const struct proto_ops vsock_dgram_ops = { .accept = sock_no_accept, .getname = vsock_getname, .poll = vsock_poll, - .ioctl = sock_no_ioctl, + .ioctl = vsock_ioctl, .listen = sock_no_listen, .shutdown = vsock_shutdown, .sendmsg = vsock_dgram_sendmsg, @@ -2286,7 +2322,7 @@ static const struct proto_ops vsock_stream_ops = { .accept = vsock_accept, .getname = vsock_getname, .poll = vsock_poll, - .ioctl = sock_no_ioctl, + .ioctl = vsock_ioctl, .listen = vsock_listen, .shutdown = vsock_shutdown, .setsockopt = vsock_connectible_setsockopt, @@ -2308,7 +2344,7 @@ static const struct proto_ops vsock_seqpacket_ops = { .accept = vsock_accept, .getname = vsock_getname, .poll = vsock_poll, - .ioctl = sock_no_ioctl, + .ioctl = vsock_ioctl, .listen = vsock_listen, .shutdown = vsock_shutdown, .setsockopt = vsock_connectible_setsockopt, -- 2.34.1