2020-12-11 13:39:21

by Paraschiv, Andra-Irina

[permalink] [raw]
Subject: [PATCH net-next v3 0/4] vsock: Add flags field in the vsock address

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.

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/[email protected]/

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/[email protected]/

---

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.


2020-12-11 13:39:52

by Paraschiv, Andra-Irina

[permalink] [raw]
Subject: [PATCH net-next v3 2/4] vm_sockets: Add VMADDR_FLAG_TO_HOST vsock flag

Add VMADDR_FLAG_TO_HOST vsock flag that is used to setup a vsock
connection where all the packets are forwarded to the host.

Then, using this type of vsock channel, vsock communication between
sibling VMs can be built on top of it.

Changelog

v2 -> v3

* Update comments to mention when the flag is set in the connect and
listen paths.

v1 -> v2

* New patch in v2, it was split from the first patch in the series.
* Remove the default value for the vsock flags field.
* Update the naming for the vsock flag to "VMADDR_FLAG_TO_HOST".

Signed-off-by: Andra Paraschiv <[email protected]>
---
include/uapi/linux/vm_sockets.h | 20 ++++++++++++++++++++
1 file changed, 20 insertions(+)

diff --git a/include/uapi/linux/vm_sockets.h b/include/uapi/linux/vm_sockets.h
index 619f8e9d55ca4..c99ed29602345 100644
--- a/include/uapi/linux/vm_sockets.h
+++ b/include/uapi/linux/vm_sockets.h
@@ -114,6 +114,26 @@

#define VMADDR_CID_HOST 2

+/* The current default use case for the vsock channel is the following:
+ * 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.
+ *
+ * Set this flag value in the sockaddr_vm corresponding field if the vsock
+ * packets need to be always forwarded to the host. Using this behavior,
+ * vsock communication between sibling VMs can be setup.
+ *
+ * This way can explicitly distinguish between vsock channels created for
+ * different use cases, such as nested VMs (or local communication between
+ * guest and host) and sibling VMs.
+ *
+ * The flag can be set in the connect logic in the user space application flow.
+ * In the listen logic (from kernel space) the flag is set on the remote peer
+ * address. This happens for an incoming connection when it is routed from the
+ * host and comes from the guest (local CID and remote CID > VMADDR_CID_HOST).
+ */
+#define VMADDR_FLAG_TO_HOST 0x0001
+
/* Invalid vSockets version. */

#define VM_SOCKETS_INVALID_VERSION -1U
--
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.

2020-12-11 17:59:02

by Stefano Garzarella

[permalink] [raw]
Subject: Re: [PATCH net-next v3 2/4] vm_sockets: Add VMADDR_FLAG_TO_HOST vsock flag

On Fri, Dec 11, 2020 at 12:32:39PM +0200, Andra Paraschiv wrote:
>Add VMADDR_FLAG_TO_HOST vsock flag that is used to setup a vsock
>connection where all the packets are forwarded to the host.
>
>Then, using this type of vsock channel, vsock communication between
>sibling VMs can be built on top of it.
>
>Changelog
>
>v2 -> v3
>
>* Update comments to mention when the flag is set in the connect and
> listen paths.
>
>v1 -> v2
>
>* New patch in v2, it was split from the first patch in the series.
>* Remove the default value for the vsock flags field.
>* Update the naming for the vsock flag to "VMADDR_FLAG_TO_HOST".
>
>Signed-off-by: Andra Paraschiv <[email protected]>
>---
> include/uapi/linux/vm_sockets.h | 20 ++++++++++++++++++++
> 1 file changed, 20 insertions(+)

Reviewed-by: Stefano Garzarella <[email protected]>

>
>diff --git a/include/uapi/linux/vm_sockets.h b/include/uapi/linux/vm_sockets.h
>index 619f8e9d55ca4..c99ed29602345 100644
>--- a/include/uapi/linux/vm_sockets.h
>+++ b/include/uapi/linux/vm_sockets.h
>@@ -114,6 +114,26 @@
>
> #define VMADDR_CID_HOST 2
>
>+/* The current default use case for the vsock channel is the following:
>+ * 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.
>+ *
>+ * Set this flag value in the sockaddr_vm corresponding field if the vsock
>+ * packets need to be always forwarded to the host. Using this behavior,
>+ * vsock communication between sibling VMs can be setup.
>+ *
>+ * This way can explicitly distinguish between vsock channels created for
>+ * different use cases, such as nested VMs (or local communication between
>+ * guest and host) and sibling VMs.
>+ *
>+ * The flag can be set in the connect logic in the user space application flow.
>+ * In the listen logic (from kernel space) the flag is set on the remote peer
>+ * address. This happens for an incoming connection when it is routed from the
>+ * host and comes from the guest (local CID and remote CID > VMADDR_CID_HOST).
>+ */
>+#define VMADDR_FLAG_TO_HOST 0x0001
>+
> /* Invalid vSockets version. */
>
> #define VM_SOCKETS_INVALID_VERSION -1U
>--
>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.
>

2020-12-11 17:59:37

by Stefano Garzarella

[permalink] [raw]
Subject: Re: [PATCH net-next v3 0/4] vsock: Add flags field in the vsock address

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!

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.
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/[email protected]/
>
>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/[email protected]/
>
>---
>
>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.
>

2020-12-11 19:02:47

by Paraschiv, Andra-Irina

[permalink] [raw]
Subject: Re: [PATCH net-next v3 0/4] vsock: Add flags field in the vsock address



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/[email protected]/
>>
>> 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/[email protected]/
>>
>> ---
>>
>> 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.

2020-12-13 16:20:17

by Jakub Kicinski

[permalink] [raw]
Subject: Re: [PATCH net-next v3 0/4] vsock: Add flags field in the vsock address

On Fri, 11 Dec 2020 16:24:13 +0100 Stefano Garzarella wrote:
> 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!
>
> 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.
> 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;

Strictly speaking this check should be superseded by the check below
(AKA removed). We used to check svm_zero[0], with the new field added
this now checks svm_zero[2]. Old applications may have not initialized
svm_zero[2] (we're talking about binary compatibility here, apps built
with old headers).

> + if (addr->svm_flags & ~svm_valid_flags)
> + return -EINVAL;

The flags should also probably be one byte (we can define a "more
flags" flag to unlock further bytes) - otherwise on big endian the
new flag will fall into svm_zero[1] so the v3 improvements are moot
for big endian, right?

> return 0;
> }
> EXPORT_SYMBOL_GPL(vsock_addr_validate);

2020-12-14 10:09:12

by Stefano Garzarella

[permalink] [raw]
Subject: Re: [PATCH net-next v3 0/4] vsock: Add flags field in the vsock address

On Sat, Dec 12, 2020 at 09:16:08AM -0800, Jakub Kicinski wrote:
>On Fri, 11 Dec 2020 16:24:13 +0100 Stefano Garzarella wrote:
>> 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!
>>
>> 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.
>> 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;
>
>Strictly speaking this check should be superseded by the check below
>(AKA removed). We used to check svm_zero[0], with the new field added
>this now checks svm_zero[2]. Old applications may have not initialized
>svm_zero[2] (we're talking about binary compatibility here, apps built
>with old headers).
>
>> + if (addr->svm_flags & ~svm_valid_flags)
>> + return -EINVAL;
>
>The flags should also probably be one byte (we can define a "more
>flags" flag to unlock further bytes) - otherwise on big endian the
>new flag will fall into svm_zero[1] so the v3 improvements are moot
>for big endian, right?

Right, I assumed the entire svm_zero[] was zeroed out, but we can't be
sure.

So, I agree to change the svm_flags to 1 byte (__u8), and remove the
superseded check that you pointed out.
With these changes we should be fully binary compatibility.

Thanks,
Stefano

>
>> return 0;
>> }
>> EXPORT_SYMBOL_GPL(vsock_addr_validate);
>

2020-12-14 16:23:52

by Paraschiv, Andra-Irina

[permalink] [raw]
Subject: Re: [PATCH net-next v3 0/4] vsock: Add flags field in the vsock address



On 14/12/2020 10:13, Stefano Garzarella wrote:
>
> On Sat, Dec 12, 2020 at 09:16:08AM -0800, Jakub Kicinski wrote:
>> On Fri, 11 Dec 2020 16:24:13 +0100 Stefano Garzarella wrote:
>>> 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!
>>>
>>> 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.
>>> 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;
>>
>> Strictly speaking this check should be superseded by the check below
>> (AKA removed). We used to check svm_zero[0], with the new field added
>> this now checks svm_zero[2]. Old applications may have not initialized
>> svm_zero[2] (we're talking about binary compatibility here, apps built
>> with old headers).
>>
>>>      +       if (addr->svm_flags & ~svm_valid_flags)
>>>      +               return -EINVAL;
>>
>> The flags should also probably be one byte (we can define a "more
>> flags" flag to unlock further bytes) - otherwise on big endian the
>> new flag will fall into svm_zero[1] so the v3 improvements are moot
>> for big endian, right?
>
> Right, I assumed the entire svm_zero[] was zeroed out, but we can't be
> sure.
>
> So, I agree to change the svm_flags to 1 byte (__u8), and remove the
> superseded check that you pointed out.
> With these changes we should be fully binary compatibility.
>

Here we go, sent out v4:

https://lore.kernel.org/lkml/[email protected]/

Thank you both.

Andra

>>
>>>              return 0;
>>>       }
>>>       EXPORT_SYMBOL_GPL(vsock_addr_validate);
>>
>




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.