Messages in this thread | | | Subject | Re: [PATCH net-next v1 1/3] vm_sockets: Include flag field in the vsock address data structure | From | "Paraschiv, Andra-Irina" <> | Date | Tue, 1 Dec 2020 20:15:04 +0200 |
| |
On 01/12/2020 18:09, Stefano Garzarella wrote: > > On Tue, Dec 01, 2020 at 05:25:03PM +0200, Andra Paraschiv wrote: >> vsock enables communication between virtual machines and the host they >> are running on. With the multi transport support (guest->host and >> host->guest), nested VMs can also use vsock channels for communication. >> >> In addition to this, by default, all the vsock packets are forwarded to >> the host, if no host->guest transport is loaded. This behavior can be >> implicitly used for enabling vsock communication between sibling VMs. >> >> Add a flag field in the vsock address data structure that can be used to >> explicitly mark the vsock connection as being targeted for a certain >> type of communication. This way, can distinguish between nested VMs and >> sibling VMs use cases and can also setup them at the same time. Till >> now, could either have nested VMs or sibling VMs at a time using the >> vsock communication stack. >> >> Use the already available "svm_reserved1" field and mark it as a flag >> field instead. This flag can be set when initializing the vsock address >> variable used for the connect() call. > > Maybe we can split this patch in 2 patches, one to rename the svm_flag > and one to add the new flags.
Sure, I can split this in 2 patches, to have a bit more separation of duties.
> >> >> Signed-off-by: Andra Paraschiv <andraprs@amazon.com> >> --- >> include/uapi/linux/vm_sockets.h | 18 +++++++++++++++++- >> 1 file changed, 17 insertions(+), 1 deletion(-) >> >> diff --git a/include/uapi/linux/vm_sockets.h >> b/include/uapi/linux/vm_sockets.h >> index fd0ed7221645d..58da5a91413ac 100644 >> --- a/include/uapi/linux/vm_sockets.h >> +++ b/include/uapi/linux/vm_sockets.h >> @@ -114,6 +114,22 @@ >> >> #define VMADDR_CID_HOST 2 >> >> +/* This sockaddr_vm flag value covers the current default use case: >> + * local vsock communication between guest and host and nested VMs >> setup. >> + * In addition to this, implicitly, the vsock packets are forwarded >> to the host >> + * if no host->guest vsock transport is set. >> + */ >> +#define VMADDR_FLAG_DEFAULT_COMMUNICATION 0x0000 > > I think we don't need this macro, since the next one can be used to > check if it a sibling communication (flag 0x1 set) or not (flag 0x1 > not set).
Right, that's not particularly the use of the flag value, as by default comes as 0. It was more for sharing the cases this covers. But I can remove the define and keep this kind of info, with regard to the default case, in the commit message / comments.
> >> + >> +/* Set this flag value in the sockaddr_vm corresponding field if the >> vsock >> + * channel needs to be setup between two sibling VMs running on the >> same host. >> + * This way can explicitly distinguish between vsock channels >> created for nested >> + * VMs (or local communication between guest and host) and the ones >> created for >> + * sibling VMs. And vsock channels for multiple use cases (nested / >> sibling VMs) >> + * can be setup at the same time. >> + */ >> +#define VMADDR_FLAG_SIBLING_VMS_COMMUNICATION 0x0001 > > What do you think if we shorten in VMADDR_FLAG_SIBLING? >
Yup, this seems ok as well for me. I'll update the naming.
Thanks, Andra
> >> + >> /* Invalid vSockets version. */ >> >> #define VM_SOCKETS_INVALID_VERSION -1U >> @@ -145,7 +161,7 @@ >> >> struct sockaddr_vm { >> __kernel_sa_family_t svm_family; >> - unsigned short svm_reserved1; >> + unsigned short svm_flag; >> unsigned int svm_port; >> unsigned int svm_cid; >> unsigned char svm_zero[sizeof(struct sockaddr) - >> -- >> 2.20.1 (Apple Git-117) >> >> >> >> >> Amazon Development Center (Romania) S.R.L. registered office: 27A Sf. >> Lazar Street, UBC5, floor 2, Iasi, Iasi County, 700045, Romania. >> Registered in Romania. Registration number J22/2621/2005. >> >
Amazon Development Center (Romania) S.R.L. registered office: 27A Sf. Lazar Street, UBC5, floor 2, Iasi, Iasi County, 700045, Romania. Registered in Romania. Registration number J22/2621/2005.
| |