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.

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.).
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).

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