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

> > 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.
> 
> Then, we can get rid of the group concept.
> 
> Big thanks for your constructive ideas :)
> 
> Thanks,
> Xuewei

Well, I think it can impose some limitations to make things simpler:

1. The default device is used to VM-VM (sibling).
2. If there is an exclusive device for HV-VM, then use it. Otherwise, use
the default device.
3. Other devices are allowed to VM-Host only.

Thanks,
Xuewei

> > If we don't want this limitation, I think we need to overcomplicate 
> > vsock and define some kind of discovery algorithm that the guest driver 
> > should run to understand which CIDs are reachable for each device.
> > But I'm not sure we really want this; if this is a use case, the guest 
> > should use a virtio-net device that already support this pretty well, so 
> > before going on this direction, we should define better why we want to 
> > complicate vsock, instead of using a net device.
> > 
> > Thanks,
> > Stefano