Re: [RFC PATCH 5/5] vsock: Add an ioctl request to get all CIDs

[Stefano Garzarella <sgarzare@xxxxxxxxxx>]

On Fri, May 17, 2024 at 10:46:07PM GMT, Xuewei Niu wrote:
> The new request is called `IOCTL_VM_SOCKETS_GET_LOCAL_CIDS`. And the old
> one, `IOCTL_VM_SOCKETS_GET_LOCAL_CID` is retained.
>
> For the transport that supports multi-devices:
>
> * `IOCTL_VM_SOCKETS_GET_LOCAL_CID` returns "-1";

What about returning the default CID (lower prio)?

> * `IOCTL_VM_SOCKETS_GET_LOCAL_CIDS` returns a vector of CIDS. The usage 
> is
> shown as following.
>
> ```
> struct vsock_local_cids local_cids;
> if ((ret = ioctl(fd, IOCTL_VM_SOCKETS_GET_LOCAL_CIDS, &local_cids))) {
>    perror("failed to get cids");
>    exit(1);
> }
> for (i = 0; i<local_cids.nr; i++) {
>    if (i == (local_cids.nr - 1))
>        printf("%u", local_cids.data[i]);
>    else
>        printf("%u,", local_cids.data[i]);
> }
> ```
>
> Signed-off-by: Xuewei Niu <niuxuewei.nxw@xxxxxxxxxxxx>
> ---
> include/net/af_vsock.h          |  7 +++++++
> include/uapi/linux/vm_sockets.h |  8 ++++++++
> net/vmw_vsock/af_vsock.c        | 19 +++++++++++++++++++
> 3 files changed, 34 insertions(+)
>
> diff --git a/include/net/af_vsock.h b/include/net/af_vsock.h
> index 25f7dc3d602d..2febc816e388 100644
> --- a/include/net/af_vsock.h
> +++ b/include/net/af_vsock.h
> @@ -264,4 +264,11 @@ static inline bool vsock_msgzerocopy_allow(const struct vsock_transport *t)
> {
> 	return t->msgzerocopy_allow && t->msgzerocopy_allow();
> }
> +
> +/**** IOCTL ****/
> +/* Type of return value of IOCTL_VM_SOCKETS_GET_LOCAL_CIDS. */
> +struct vsock_local_cids {
> +	int nr;
> +	unsigned int data[MAX_VSOCK_NUM];
> +};
> #endif /* __AF_VSOCK_H__ */
> diff --git a/include/uapi/linux/vm_sockets.h b/include/uapi/linux/vm_sockets.h
> index 36ca5023293a..01f73fb7af5a 100644
> --- a/include/uapi/linux/vm_sockets.h
> +++ b/include/uapi/linux/vm_sockets.h
> @@ -195,8 +195,16 @@ struct sockaddr_vm {
>
> #define MAX_VSOCK_NUM 16

Okay, now I see why you need this in the UAPI, but pleace try to follow
other defines.

What about VM_SOCKETS_MAX_DEVS ?

> +/* Return actual context id if the transport not support vsock
> + * multi-devices. Otherwise, return `-1U`.
> + */
> +
> #define IOCTL_VM_SOCKETS_GET_LOCAL_CID		_IO(7, 0xb9)
>
> +/* Only available in transports that support multiple devices. */
> +
> +#define IOCTL_VM_SOCKETS_GET_LOCAL_CIDS     _IOR(7, 0xba, struct vsock_local_cids)
> +
> /* MSG_ZEROCOPY notifications are encoded in the standard error format,
>  * sock_extended_err. See Documentation/networking/msg_zerocopy.rst in
>  * kernel source tree for more details.
> diff --git a/net/vmw_vsock/af_vsock.c b/net/vmw_vsock/af_vsock.c
> index 3b34be802bf2..2ea2ff52f15b 100644
> --- a/net/vmw_vsock/af_vsock.c
> +++ b/net/vmw_vsock/af_vsock.c
> @@ -2454,6 +2454,7 @@ static long vsock_dev_do_ioctl(struct file *filp,
> 	u32 __user *p = ptr;
> 	u32 cid = VMADDR_CID_ANY;
> 	int retval = 0;
> +	struct vsock_local_cids local_cids;
>
> 	switch (cmd) {
> 	case IOCTL_VM_SOCKETS_GET_LOCAL_CID:
> @@ -2469,6 +2470,24 @@ static long vsock_dev_do_ioctl(struct file *filp,
> 			retval = -EFAULT;
> 		break;
>
> +	case IOCTL_VM_SOCKETS_GET_LOCAL_CIDS:
> +		if (!transport_g2h || !transport_g2h->get_local_cids)
> +			goto fault;
> +
> +		rcu_read_lock();
> +		local_cids.nr = transport_g2h->get_local_cids(local_cids.data);
> +		rcu_read_unlock();
> +
> +		if (local_cids.nr < 0 ||
> +		    copy_to_user(p, &local_cids, sizeof(local_cids)))
> +			goto fault;
> +
> +		break;
> +
> +fault:
> +		retval = -EFAULT;
> +		break;
> +
> 	default:
> 		retval = -ENOIOCTLCMD;
> 	}
> --
> 2.34.1