Re: [PATCH RFC net v2 3/3] vsock: Fix IOCTL_VM_SOCKETS_GET_LOCAL_CID to check also `transport_local`
On 6/27/25 10:10, Stefano Garzarella wrote:
> On Wed, Jun 25, 2025 at 11:23:54PM +0200, Michal Luczaj wrote:
>> On 6/25/25 10:54, Stefano Garzarella wrote:
>>> On Fri, Jun 20, 2025 at 09:52:45PM +0200, Michal Luczaj wrote:
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
Signed-off-by: Michal Luczaj
---
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.
>>
---
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.
>
> Yeah, BTW for transport_h2g is the same, they always should return
> VMADDR_CID_HOST, so I think we should be symmetrical.
Heh, I've missed that VMADDR_CID_HOST completely :)
Thanks,
Michal
Re: [PATCH RFC net v2 3/3] vsock: Fix IOCTL_VM_SOCKETS_GET_LOCAL_CID to check also `transport_local`
On Wed, Jun 25, 2025 at 11:23:54PM +0200, Michal Luczaj wrote:
On 6/25/25 10:54, Stefano Garzarella wrote:
On Fri, Jun 20, 2025 at 09:52:45PM +0200, Michal Luczaj wrote:
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
Signed-off-by: Michal Luczaj
---
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.
---
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.
Yeah, BTW for transport_h2g is the same, they always should return
VMADDR_CID_HOST, so I think we should be symmetrical.
Thanks,
Stefano
Re: [PATCH RFC net v2 3/3] vsock: Fix IOCTL_VM_SOCKETS_GET_LOCAL_CID to check also `transport_local`
On 6/25/25 10:54, Stefano Garzarella wrote:
> On Fri, Jun 20, 2025 at 09:52:45PM +0200, Michal Luczaj wrote:
>> 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
>> Signed-off-by: Michal Luczaj
>> ---
>> 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.
>> ---
>> 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
Re: [PATCH RFC net v2 3/3] vsock: Fix IOCTL_VM_SOCKETS_GET_LOCAL_CID to check also `transport_local`
On Fri, Jun 20, 2025 at 09:52:45PM +0200, Michal Luczaj wrote:
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
Signed-off-by: Michal Luczaj
---
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.
---
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?
Thanks,
Stefano
if (put_user(cid, p) != 0)
retval = -EFAULT;
--
2.49.0
