Re: [PATCH RFC net v2 3/3] vsock: Fix IOCTL_VM_SOCKETS_GET_LOCAL_CID to check also `transport_local`
From: Michal Luczaj <hidden>
Date: 2025-06-25 21:24:10
Also in:
lkml, virtualization
On 6/25/25 10:54, Stefano Garzarella wrote:
On Fri, Jun 20, 2025 at 09:52:45PM +0200, Michal Luczaj wrote:quoted
Support returning VMADDR_CID_LOCAL in case no other vsock transport is available. Fixes: 0e12190578d0 ("vsock: add local transport support in the vsock core") Suggested-by: Stefano Garzarella <sgarzare@redhat.com> Signed-off-by: Michal Luczaj <redacted> --- man vsock(7) mentions IOCTL_VM_SOCKETS_GET_LOCAL_CID vs. VMADDR_CID_LOCAL: Ioctls ... IOCTL_VM_SOCKETS_GET_LOCAL_CID ... Consider using VMADDR_CID_ANY when binding instead of getting the local CID with IOCTL_VM_SOCKETS_GET_LOCAL_CID. Local communication .... The local CID obtained with IOCTL_VM_SOCKETS_GET_LOCAL_CID can be used for the same purpose, but it is preferable to use VMADDR_CID_LOCAL. I was wondering it that would need some rewriting, since we're adding VMADDR_CID_LOCAL as a possible ioctl's return value.IIRC the reason was, that if we have for example a G2H module loaded, the ioctl returns the CID of that module (e.g. 42). So, we can use both 42 and VMADDR_CID_LOCAL to do the loopback communication, but we encourage to always use VMADDR_CID_LOCAL. With this change we basically don't change that, but we change the fact that if there is only the loopback module loaded, before the ioctl returned VMADDR_CID_ANY, while now it returns LOCAL rightly. So, IMO we are fine.
All right, got it.
quoted
--- net/vmw_vsock/af_vsock.c | 2 ++ 1 file changed, 2 insertions(+)diff --git a/net/vmw_vsock/af_vsock.c b/net/vmw_vsock/af_vsock.c index a1b1073a2c89f865fcdb58b38d8e7feffcf1544f..4bdb4016bd14d790f3d217d5063be64a1553b194 100644 --- a/net/vmw_vsock/af_vsock.c +++ b/net/vmw_vsock/af_vsock.c@@ -2577,6 +2577,8 @@ static long vsock_dev_do_ioctl(struct file *filp,cid = vsock_transport_local_cid(&transport_g2h); if (cid == VMADDR_CID_ANY) cid = vsock_transport_local_cid(&transport_h2g); + if (cid == VMADDR_CID_ANY && transport_local) + cid = VMADDR_CID_LOCAL;why not `cid = vsock_transport_local_cid(&transport_local)` like for H2G?
Sure, can do. I've assumed transport_local would always have a local CID of VMADDR_CID_LOCAL. So taking mutex and going through a callback function to get VMADDR_CID_LOCAL seemed superfluous. But I get it, if you want to have it symmetrical with the other vsock_transport_local_cid()s. Thanks, Michal