Re: [PATCH v7] virtio-vsock: Add support for multi devices

On Mon, Jun 23, 2025 at 08:14:07PM +0800, Xuewei Niu wrote:
>> On Mon, Jun 23, 2025 at 06:35:59PM +0800, Xuewei Niu wrote:
>> >> On Mon, Jun 23, 2025 at 04:48:33PM +0800, Xuewei Niu wrote:
>> >> >> On Thu, Jun 19, 2025 at 01:10:33PM +0800, Xuewei Niu wrote:
>> >> >> >> > From: Xuewei Niu <[email protected]>
>> >> >> >> > Sent: 19 June 2025 08:57 AM
>> >> >> >> >
>> >> >> >> > Hi Parav,
>> >> >> >> >
>> >> >> >> > Could you please take a look at the diagram in [1]?
>> >> >> >> >
>> >> >> >> > IIUC, for VM0, there are two groups, and for VM1, there is only one group.
>> >> >> >> > Am I right? If yes, I think the group concept is reasonable but we don't need
>> >> >> >> > at this time.
>> >> >> >> >
>> >> >> >> > I think the first thing is to figure out how to pick the right group.
>> >> >> >> >
>> >> >> >> > Standard socket doesn't provide a way to access the group information.
>> >> >> >> >
>> >> >> >> > Source and destination are from `bind()` and `connect()`, respectively. If we
>> >> >> >> > don't call `bind()`, only the destination is known.
>> >> >> >> >
>> >> >> >> > However, only destination is not enough to find the group. For example, the
>> >> >> >> > well-known CIDs (e.g. 2) are valid for all groups.
>> >> >> >> >
>> >> >> >> > 1:
>> >> >> >> > https://lore.kernel.org/virtio-comment/[email protected]/
>> >> >> >>
>> >> >> >> Right. Sock addressing scheme is naïve presently to select the group.
>> >> >> >> Not sure when/how you or others plan to do.
>> >> >> >> This is transport layer problem to solve (not to confuse with transport = pci/mmio etc).
>> >> >>
>> >> >>
>> >> >> >>
>> >> >> >> However, at device level, we should have the construct of grouping.
>> >> >> >> Without this construct, all devices will be part of single group and one will not be able to build the group concept later.
>> >> >> >> So even if you don't need it explicitly now, grouping the device is what you need when connect() is called.
>> >> >> >>
>> >> >> >> So I was imagining a relatively simple scheme:
>> >> >> >> For example, virtio device level, some kind of group id is present.
>> >> >> >> So two devices which has same group id, are part of single group.
>> >> >> >> An example group id format can be a UUID.
>> >> >> >>
>> >> >> >> And this is completely optional for devices to implement.
>> >> >> >> Generic enough and usable beyond just vsock device in other use cases
>> >> >> >> we discussed in past.
>> >> >> >
>> >> >> >Fair enough.
>> >> >> >
>> >> >> >@Stefano, could you please take a look at this? I'd love to have some
>> >> >> >input
>> >> >> >from you.
>> >> >> >
>> >> >> >A brief summary of the idea is: The config space will be extended to
>> >> >> >include a group id. The devices with the same group id are considered
>> >> >> >to be
>> >> >> >in the same group.
>> >> >>
>> >> >> Thanks for the summary, but please avoid top posting, otherwise is very
>> >> >> hard to follow the discussion :-(
>> >> >> https://subspace.kernel.org/etiquette.html#do-not-top-post-when-replying
>> >> >
>> >> >Sorry, I'll avoid it in the future.
>> >> >
>> >> >> I like the idea of groups. What is not clear to me, is how groups will
>> >> >> allow the driver to select the default output device when the source
>> >> >> socket is not bind to any source CID.
>> >> >
>> >> >Well, we did discuss, but we need your input.
>> >> >
>> >> >I said in the thread [1] based on the standard socket API, the driver can't
>> >> >pick a group. Parav [2] suggested that the group, as a basic concept,
>> >> >should be present even if we are unable to use it.
>> >> >
>> >> >IMHO, we might use "{group_id}-{cid}" as the vsock addressing scheme to use
>> >> >the concept. But it is a very big change, leading to incompatibility with
>> >> >the existing apps.
>> >>
>> >> I still don't understand how the group_id will work :-( and how will
>> >> allow the driver to pick the default device.
>> >
>> >FYI, here is trying to explain what is group and why it is needed, but I
>> >agree with you about the idea of "types".
>> >
>> >I posted a diagram in the thread [1]. I think the group is something like a
>> >"namespace". I think it is reasonable, but my concerns are compatibility
>> >and complexity.
>> >
>> >1: https://lore.kernel.org/virtio-comment/[email protected]/
>> >
>> >> This is the `sockaddr_vm`, so we should be careful of extending it:
>> >>
>> >> struct sockaddr_vm {
>> >> 	__kernel_sa_family_t svm_family;
>> >> 	unsigned short svm_reserved1;
>> >> 	unsigned int svm_port;
>> >> 	unsigned int svm_cid;
>> >>
>> >> #define VMADDR_FLAG_TO_HOST 0x01
>> >>
>> >> 	__u8 svm_flags;
>> >> 	unsigned char svm_zero[sizeof(struct sockaddr) -
>> >> 			       sizeof(sa_family_t) -
>> >> 			       sizeof(unsigned short) -
>> >> 			       sizeof(unsigned int) -
>> >> 			       sizeof(unsigned int) -
>> >> 			       sizeof(__u8)];
>> >> };
>> >>
>> >> >I think it might be beyond the scope of this patch, and would make the
>> >> >vsock more complex. The current conclusion is that we will keep the concept
>> >> >of grouping as a placeholder, but we will not use it.
>> >>
>> >> IMO we should first clarify better what we want to support.
>> >> As I already suggested some months ago, IMHO supporting any number of
>> >> vsock devices for a VM it's not really needed for your goal and I can't
>> >> see other use cases where a virtio-net device can't be use. Just a
>> >> reminder, vsock is not a network device, is more a P2P device where we
>> >> want to keep the configuration in the guest as simpler as possible (we
>> >> don't want to run ARP, DHCP, etc.).
>> >
>> >Agree that.
>> >
>> >> Till now vsock was more used just for guest-host communication, but
>> >> recently it was extended to communicate with sibling VMs.
>> >>
>> >> IIUC your use case, we just need to support different type of vsock
>> >> devices attached to the VM. With "type" I mean type of address handled.
>> >> I think we can define 3 types based on the CID we have:
>> >> - hypervisor: VMADDR_CID_HYPERVISOR(0)
>> >> - host: VMADDR_CID_HOST(2)
>> >> - sibling: CID >=3
>> >>
>> >> The vhost-vsock device handles only VMADDR_CID_HOST, so it doesn't allow
>> >> to reach from the guest any other CIDs.
>> >> The vhost-user-vsock recently started to support sibling VMs.
>> >> The tcp-over-vsock of libkrun should use VMADDR_CID_HYPERVISOR(0).
>> >>
>> >> So, IMHO we should define new features or config flags that a device can
>> >> expose depending on which address is able to handle.
>> >> Also, in order to avoid to overcomplicate vsock, we should allow only
>> >> one device for each type (a single device should support multiple types,
>> >> but only one device can be registered for a type).
>> >
>> >I think only one device can be registered for sibling and hypervisor. And
>> >it is possible to have multiple devices for host. Otherwise, the goals of
>> >this patch will be not achieved.
>>
>> Why?
>>
>> IMO, as I already wrote, the libkrun service should use CID=0.
>
>As we before discussed, if there is only one backend for the libkrun
>service, we can use cid=0. I totally agree with you. So let us put this
>case aside first.
>
>> How you will handle multiple devices for host? How can the guest know
>> which device to use to reach HOST(2)?
>
>Do `bind()` explicitly, and a device with matching cid will be picked up.

Okay, but why you need 2 devices to communicate with the same CID, host 
in this case (CID=2)?
IMO use the source CID to multiplex a socket at destination is not 
great. But I can be wrong.

>
>> IMO is easier to have the multiple device for sibling (e.g. the device
>> can advertise which dest CID its supports), but not for host.
>
>I think one device for sibling is enough.

I also think one should be enough, but IMO I think it might make more 
sense to have multiple devices in this case, where each device can 
handle a pool of CIDs, then the hard part will be figuring out how to 
allocate the CIDs, etc. so yes, I agree that having one device even in 
this case is the easiest thing.

>For example, dev1 is enough for
>communication with dev3 and dev4. So we don't need dev2 for sibling in the
>first VM, right?

Just a note, vhost-vsock is not allowing any sibling communication.
vhost-user-vsock can do it, but we don't want to bring any support in 
vhost-vsock to not overcomplicate it (again it will become like a 
network switch, requiring firewalls, etc.)

>
>+──kernel(vhost-vsock)──────────────────────────────────────────────+
>│+──────────────+ +──────────────+ +──────────────+ +──────────────+│
>││  dev1(cid1)  │ │  dev2(cid2)  │ │  dev3(cid3)  │ │  dev4(cid4)  ││
>│+──────────────+ +──────────────+ +──────────────+ +──────────────+│
>+────────▲────────────────▲────────────────▲────────────────▲───────+
>         │                │                │                │
>         │                │          ┌─────┘                │
>         │                │          │                      │
>+────────┴───────────+    │ +────────┴───────────+ +────────┴───────────+
>│+──────────────+ VM0│    │ │+──────────────+ VM1│ │+──────────────+ VM2│
>││dev1(cid1,def)│    │    │ ││dev3(cid3,def)│    │ ││dev4(cid4,def)│    │
>│+──────────────+    │    │ │+──────────────+    │ │+──────────────+    │
>│+──────────────+    │    │ │                    │ │                    │
>││  dev2(cid2)  │────┼────┘ │                    │ │                    │
>│+──────────────+    │      │                    │ │                    │
>+────────────────────+      +────────────────────+ +────────────────────+
>
>The things go different for devices for host:
>
>1. For dev1, it is used for sibling (dev1 <-> dev4), and the host
>(src_cid=cid1, dst_cid=2) means the real host (not a userapp), where
>doesn't show in the diagram.

This is not going to happen (see above).

>2. For dev2 (src_cid=cid2), the host (dst_cid=2) is userapp1;
>3. For dev3 (src_cid=cid3), the host (dst_cid=2) is userapp2.
>
>+──kernel(vhost-vsock)──────────────────────────────────────────────+
>│+──────────────+                                   +──────────────+│
>││  dev1(cid1)  │                                   │  dev4(cid4)  ││
>│+──────────────+                                   +──────────────+│
>+────────▲──────────────────────────────────────────────────▲───────+
>         │                                                  │
>+────────┴───────────+     +────────────────────+           │
>│+──────────────+ VM0│     │+──────────────+    │           │
>││dev1(cid1,def)│    │┌────▶│  dev2(cid2)  │    │  +────────┴───────────+
>│+──────────────+    ││    │+──────────────+    │  │+──────────────+ VM2│
>│+──────────────+    ││    │            userapp1│  ││dev4(cid4,def)│    │
>││  dev2(cid2)  │────┼┘    +────────────────────+  │+──────────────+    │
>│+──────────────+  vhost-user-vsock─────────────+  │                    │
>│+──────────────+    │     │+──────────────+    │  │                    │
>││  dev3(cid3)  │────┼─────▶│  dev3(cid3)  │    │  │                    │
>│+──────────────+    │     │+──────────────+    │  +────────────────────+
>+────────────────────+     │            userapp2│
>                           +────────────────────+
>

I'm really confused with `cid1`, `cid2`, etc. Are they any number >= 3?
I'd suggest to use real value (e.g. cid=42).

So what dest CID the VM0 is supposed to use to talk with userapp1 and 
userapp2? In both cases CID=2, right?

Why you need 2 vhost-user-vsock devices?
Can you just have a single one and have the application 
connecting/listing on different port? (which is the sense of the port, 
multiplexing application on the destination)

Thanks,
Stefano