Messages in this thread | | | Subject | Re: [PATCH net-next v3 0/4] vsock: Add flags field in the vsock address | From | "Paraschiv, Andra-Irina" <> | Date | Fri, 11 Dec 2020 19:46:10 +0200 |
| |
On 11/12/2020 17:24, Stefano Garzarella wrote: > > Hi Andra, > > On Fri, Dec 11, 2020 at 12:32:37PM +0200, Andra Paraschiv wrote: >> vsock enables communication between virtual machines and the host >> they are >> running on. Nested VMs can be setup to use vsock channels, as the multi >> transport support has been available in the mainline since the v5.5 >> Linux kernel >> has been released. >> >> Implicitly, if no host->guest vsock transport is loaded, all the >> vsock packets >> are forwarded to the host. This behavior can be used to setup >> communication >> channels between sibling VMs that are running on the same host. One >> example can >> be the vsock channels that can be established within AWS Nitro Enclaves >> (see Documentation/virt/ne_overview.rst). >> >> To be able to explicitly mark a connection as being used for a >> certain use case, >> add a flags field in the vsock address data structure. The value of >> the flags >> field is taken into consideration when the vsock transport is >> assigned. This way >> can distinguish between different use cases, such as nested VMs / local >> communication and sibling VMs. >> >> The flags field can be set in the user space application connect >> logic. On the >> listen path, the field can be set in the kernel space logic. >> > > I reviewed all the patches and they are in a good shape!
Hi Stefano,
Thanks for the overall review and for the reconfirmation of the Rb for the vsock address data structure changes.
> > Maybe the last thing to add is a flags check in the > vsock_addr_validate(), to avoid that flags that we don't know how to > handle are specified.
I can add this validation as a new patch in the series, next revision.
Thanks, Andra
> > For example if in the future we add new flags that this version of the > kernel is not able to satisfy, we should return an error to the > application. > > I mean something like this: > > diff --git a/net/vmw_vsock/vsock_addr.c b/net/vmw_vsock/vsock_addr.c > index 909de26cb0e7..73bb1d2fa526 100644 > --- a/net/vmw_vsock/vsock_addr.c > +++ b/net/vmw_vsock/vsock_addr.c > @@ -22,6 +22,8 @@ EXPORT_SYMBOL_GPL(vsock_addr_init); > > int vsock_addr_validate(const struct sockaddr_vm *addr) > { > + unsigned short svm_valid_flags = VMADDR_FLAG_TO_HOST; > + > if (!addr) > return -EFAULT; > > @@ -31,6 +33,9 @@ int vsock_addr_validate(const struct sockaddr_vm > *addr) > if (addr->svm_zero[0] != 0) > return -EINVAL; > > + if (addr->svm_flags & ~svm_valid_flags) > + return -EINVAL; > + > return 0; > } > EXPORT_SYMBOL_GPL(vsock_addr_validate); > > > Thanks, > Stefano > >> Thank you. >> >> Andra >> >> --- >> >> Patch Series Changelog >> >> The patch series is built on top of v5.10-rc7. >> >> GitHub repo branch for the latest version of the patch series: >> >> * https://github.com/andraprs/linux/tree/vsock-flag-sibling-comm-v3 >> >> v2 -> v3 >> >> * Rebase on top of v5.10-rc7. >> * Add "svm_flags" as a new field, not reusing "svm_reserved1". >> * Update comments to mention when the "VMADDR_FLAG_TO_HOST" flag is >> set in the >> connect and listen paths. >> * Update bitwise check logic to not compare result to the flag value. >> * v2: >> https://lore.kernel.org/lkml/20201204170235.84387-1-andraprs@amazon.com/ >> >> v1 -> v2 >> >> * Update the vsock flag naming to "VMADDR_FLAG_TO_HOST". >> * Use bitwise operators to setup and check the vsock flag. >> * Set the vsock flag on the receive path in the vsock transport >> assignment >> logic. >> * Merge the checks for the g2h transport assignment in one "if" block. >> * v1: >> https://lore.kernel.org/lkml/20201201152505.19445-1-andraprs@amazon.com/ >> >> --- >> >> Andra Paraschiv (4): >> vm_sockets: Add flags field in the vsock address data structure >> vm_sockets: Add VMADDR_FLAG_TO_HOST vsock flag >> af_vsock: Set VMADDR_FLAG_TO_HOST flag on the receive path >> af_vsock: Assign the vsock transport considering the vsock address >> flags >> >> include/uapi/linux/vm_sockets.h | 25 ++++++++++++++++++++++++- >> net/vmw_vsock/af_vsock.c | 21 +++++++++++++++++++-- >> 2 files changed, 43 insertions(+), 3 deletions(-) >> >> -- >> 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.
| |