2020-12-04 17:06:56

by Paraschiv, Andra-Irina

[permalink] [raw]
Subject: [PATCH net-next v2 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 "svm_reserved1" field
has been repurposed to be the flags field. The value of the flags will then be
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.

Thank you.

Andra

---

Patch Series Changelog

The patch series is built on top of v5.10-rc6.

GitHub repo branch for the latest version of the patch series:

* https://github.com/andraprs/linux/tree/vsock-flag-sibling-comm-v2

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: Include 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 | 17 ++++++++++++++++-
net/vmw_vsock/af_vsock.c | 21 +++++++++++++++++++--
2 files changed, 35 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-04 17:07:54

by Paraschiv, Andra-Irina

[permalink] [raw]
Subject: [PATCH net-next v2 4/4] af_vsock: Assign the vsock transport considering the vsock address flags

The vsock flags field can be set in the connect and (listen) receive
paths.

When the vsock transport is assigned, the remote CID is used to
distinguish between types of connection.

Use the vsock flags value (in addition to the CID) from the remote
address to decide which vsock transport to assign. For the sibling VMs
use case, all the vsock packets need to be forwarded to the host, so
always assign the guest->host transport if the VMADDR_FLAG_TO_HOST flag
is set. For the other use cases, the vsock transport assignment logic is
not changed.

Changelog

v1 -> v2

* Use bitwise operator to check the vsock flag.
* Use the updated "VMADDR_FLAG_TO_HOST" flag naming.
* Merge the checks for the g2h transport assignment in one "if" block.

Signed-off-by: Andra Paraschiv <[email protected]>
---
net/vmw_vsock/af_vsock.c | 9 +++++++--
1 file changed, 7 insertions(+), 2 deletions(-)

diff --git a/net/vmw_vsock/af_vsock.c b/net/vmw_vsock/af_vsock.c
index 83d035eab0b05..66e643c3b5f85 100644
--- a/net/vmw_vsock/af_vsock.c
+++ b/net/vmw_vsock/af_vsock.c
@@ -421,7 +421,8 @@ static void vsock_deassign_transport(struct vsock_sock *vsk)
* The vsk->remote_addr is used to decide which transport to use:
* - remote CID == VMADDR_CID_LOCAL or g2h->local_cid or VMADDR_CID_HOST if
* g2h is not loaded, will use local transport;
- * - remote CID <= VMADDR_CID_HOST will use guest->host transport;
+ * - remote CID <= VMADDR_CID_HOST or h2g is not loaded or remote flags field
+ * includes VMADDR_FLAG_TO_HOST flag value, will use guest->host transport;
* - remote CID > VMADDR_CID_HOST will use host->guest transport;
*/
int vsock_assign_transport(struct vsock_sock *vsk, struct vsock_sock *psk)
@@ -429,6 +430,7 @@ int vsock_assign_transport(struct vsock_sock *vsk, struct vsock_sock *psk)
const struct vsock_transport *new_transport;
struct sock *sk = sk_vsock(vsk);
unsigned int remote_cid = vsk->remote_addr.svm_cid;
+ unsigned short remote_flags;
int ret;

/* If the packet is coming with the source and destination CIDs higher
@@ -443,6 +445,8 @@ int vsock_assign_transport(struct vsock_sock *vsk, struct vsock_sock *psk)
vsk->remote_addr.svm_cid > VMADDR_CID_HOST)
vsk->remote_addr.svm_flags |= VMADDR_FLAG_TO_HOST;

+ remote_flags = vsk->remote_addr.svm_flags;
+
switch (sk->sk_type) {
case SOCK_DGRAM:
new_transport = transport_dgram;
@@ -450,7 +454,8 @@ int vsock_assign_transport(struct vsock_sock *vsk, struct vsock_sock *psk)
case SOCK_STREAM:
if (vsock_use_local_transport(remote_cid))
new_transport = transport_local;
- else if (remote_cid <= VMADDR_CID_HOST || !transport_h2g)
+ else if (remote_cid <= VMADDR_CID_HOST || !transport_h2g ||
+ (remote_flags & VMADDR_FLAG_TO_HOST) == VMADDR_FLAG_TO_HOST)
new_transport = transport_g2h;
else
new_transport = transport_h2g;
--
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-07 10:06:06

by Stefano Garzarella

[permalink] [raw]
Subject: Re: [PATCH net-next v2 4/4] af_vsock: Assign the vsock transport considering the vsock address flags

On Fri, Dec 04, 2020 at 07:02:35PM +0200, Andra Paraschiv wrote:
>The vsock flags field can be set in the connect and (listen) receive
>paths.
>
>When the vsock transport is assigned, the remote CID is used to
>distinguish between types of connection.
>
>Use the vsock flags value (in addition to the CID) from the remote
>address to decide which vsock transport to assign. For the sibling VMs
>use case, all the vsock packets need to be forwarded to the host, so
>always assign the guest->host transport if the VMADDR_FLAG_TO_HOST flag
>is set. For the other use cases, the vsock transport assignment logic is
>not changed.
>
>Changelog
>
>v1 -> v2
>
>* Use bitwise operator to check the vsock flag.
>* Use the updated "VMADDR_FLAG_TO_HOST" flag naming.
>* Merge the checks for the g2h transport assignment in one "if" block.
>
>Signed-off-by: Andra Paraschiv <[email protected]>
>---
> net/vmw_vsock/af_vsock.c | 9 +++++++--
> 1 file changed, 7 insertions(+), 2 deletions(-)
>
>diff --git a/net/vmw_vsock/af_vsock.c b/net/vmw_vsock/af_vsock.c
>index 83d035eab0b05..66e643c3b5f85 100644
>--- a/net/vmw_vsock/af_vsock.c
>+++ b/net/vmw_vsock/af_vsock.c
>@@ -421,7 +421,8 @@ static void vsock_deassign_transport(struct vsock_sock *vsk)
> * The vsk->remote_addr is used to decide which transport to use:
> * - remote CID == VMADDR_CID_LOCAL or g2h->local_cid or VMADDR_CID_HOST if
> * g2h is not loaded, will use local transport;
>- * - remote CID <= VMADDR_CID_HOST will use guest->host transport;
>+ * - remote CID <= VMADDR_CID_HOST or h2g is not loaded or remote flags field
>+ * includes VMADDR_FLAG_TO_HOST flag value, will use guest->host transport;
> * - remote CID > VMADDR_CID_HOST will use host->guest transport;
> */
> int vsock_assign_transport(struct vsock_sock *vsk, struct vsock_sock *psk)
>@@ -429,6 +430,7 @@ int vsock_assign_transport(struct vsock_sock *vsk, struct vsock_sock *psk)
> const struct vsock_transport *new_transport;
> struct sock *sk = sk_vsock(vsk);
> unsigned int remote_cid = vsk->remote_addr.svm_cid;
>+ unsigned short remote_flags;
> int ret;
>
> /* If the packet is coming with the source and destination CIDs higher
>@@ -443,6 +445,8 @@ int vsock_assign_transport(struct vsock_sock *vsk, struct vsock_sock *psk)
> vsk->remote_addr.svm_cid > VMADDR_CID_HOST)
> vsk->remote_addr.svm_flags |= VMADDR_FLAG_TO_HOST;
>
>+ remote_flags = vsk->remote_addr.svm_flags;
>+
> switch (sk->sk_type) {
> case SOCK_DGRAM:
> new_transport = transport_dgram;
>@@ -450,7 +454,8 @@ int vsock_assign_transport(struct vsock_sock *vsk, struct vsock_sock *psk)
> case SOCK_STREAM:
> if (vsock_use_local_transport(remote_cid))
> new_transport = transport_local;
>- else if (remote_cid <= VMADDR_CID_HOST || !transport_h2g)
>+ else if (remote_cid <= VMADDR_CID_HOST || !transport_h2g ||
>+ (remote_flags & VMADDR_FLAG_TO_HOST) == VMADDR_FLAG_TO_HOST)

Maybe "remote_flags & VMADDR_FLAG_TO_HOST" should be enough, but the
patch is okay:

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

> new_transport = transport_g2h;
> else
> new_transport = transport_h2g;
>--
>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-07 10:08:54

by Stefano Garzarella

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

Hi Andra,

On Fri, Dec 04, 2020 at 07:02:31PM +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 "svm_reserved1" field
>has been repurposed to be the flags field. The value of the flags will then be
>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 series seems in a good shape, I left some minor comments.
I run my test suite (vsock_test, iperf3, nc) with nested VMs (QEMU/KVM),
and everything looks good.

Note: I'll be offline today and tomorrow, so I may miss followups.

Thanks,
Stefano

2020-12-07 19:22:29

by Paraschiv, Andra-Irina

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



On 07/12/2020 12:05, Stefano Garzarella wrote:
>
> Hi Andra,
>
> On Fri, Dec 04, 2020 at 07:02:31PM +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
>> "svm_reserved1" field
>> has been repurposed to be the flags field. The value of the flags
>> will then be
>> 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 series seems in a good shape, I left some minor comments.
> I run my test suite (vsock_test, iperf3, nc) with nested VMs (QEMU/KVM),
> and everything looks good.

Thanks, Stefano, for review and checking it out for the nested case as well.

I'll send out v3 including the addressed feedback and the Rb tags.

>
> Note: I'll be offline today and tomorrow, so I may miss followups.

Ok, np, thanks for the heads-up.

Andra



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-07 23:59:11

by Paraschiv, Andra-Irina

[permalink] [raw]
Subject: Re: [PATCH net-next v2 4/4] af_vsock: Assign the vsock transport considering the vsock address flags



On 07/12/2020 12:00, Stefano Garzarella wrote:
>
> On Fri, Dec 04, 2020 at 07:02:35PM +0200, Andra Paraschiv wrote:
>> The vsock flags field can be set in the connect and (listen) receive
>> paths.
>>
>> When the vsock transport is assigned, the remote CID is used to
>> distinguish between types of connection.
>>
>> Use the vsock flags value (in addition to the CID) from the remote
>> address to decide which vsock transport to assign. For the sibling VMs
>> use case, all the vsock packets need to be forwarded to the host, so
>> always assign the guest->host transport if the VMADDR_FLAG_TO_HOST flag
>> is set. For the other use cases, the vsock transport assignment logic is
>> not changed.
>>
>> Changelog
>>
>> v1 -> v2
>>
>> * Use bitwise operator to check the vsock flag.
>> * Use the updated "VMADDR_FLAG_TO_HOST" flag naming.
>> * Merge the checks for the g2h transport assignment in one "if" block.
>>
>> Signed-off-by: Andra Paraschiv <[email protected]>
>> ---
>> net/vmw_vsock/af_vsock.c | 9 +++++++--
>> 1 file changed, 7 insertions(+), 2 deletions(-)
>>
>> diff --git a/net/vmw_vsock/af_vsock.c b/net/vmw_vsock/af_vsock.c
>> index 83d035eab0b05..66e643c3b5f85 100644
>> --- a/net/vmw_vsock/af_vsock.c
>> +++ b/net/vmw_vsock/af_vsock.c
>> @@ -421,7 +421,8 @@ static void vsock_deassign_transport(struct
>> vsock_sock *vsk)
>>  * The vsk->remote_addr is used to decide which transport to use:
>>  *  - remote CID == VMADDR_CID_LOCAL or g2h->local_cid or
>> VMADDR_CID_HOST if
>>  *    g2h is not loaded, will use local transport;
>> - *  - remote CID <= VMADDR_CID_HOST will use guest->host transport;
>> + *  - remote CID <= VMADDR_CID_HOST or h2g is not loaded or remote
>> flags field
>> + *    includes VMADDR_FLAG_TO_HOST flag value, will use guest->host
>> transport;
>>  *  - remote CID > VMADDR_CID_HOST will use host->guest transport;
>>  */
>> int vsock_assign_transport(struct vsock_sock *vsk, struct vsock_sock
>> *psk)
>> @@ -429,6 +430,7 @@ int vsock_assign_transport(struct vsock_sock
>> *vsk, struct vsock_sock *psk)
>>       const struct vsock_transport *new_transport;
>>       struct sock *sk = sk_vsock(vsk);
>>       unsigned int remote_cid = vsk->remote_addr.svm_cid;
>> +      unsigned short remote_flags;
>>       int ret;
>>
>>       /* If the packet is coming with the source and destination CIDs
>> higher
>> @@ -443,6 +445,8 @@ int vsock_assign_transport(struct vsock_sock
>> *vsk, struct vsock_sock *psk)
>>           vsk->remote_addr.svm_cid > VMADDR_CID_HOST)
>>               vsk->remote_addr.svm_flags |= VMADDR_FLAG_TO_HOST;
>>
>> +      remote_flags = vsk->remote_addr.svm_flags;
>> +
>>       switch (sk->sk_type) {
>>       case SOCK_DGRAM:
>>               new_transport = transport_dgram;
>> @@ -450,7 +454,8 @@ int vsock_assign_transport(struct vsock_sock
>> *vsk, struct vsock_sock *psk)
>>       case SOCK_STREAM:
>>               if (vsock_use_local_transport(remote_cid))
>>                       new_transport = transport_local;
>> -              else if (remote_cid <= VMADDR_CID_HOST || !transport_h2g)
>> +              else if (remote_cid <= VMADDR_CID_HOST ||
>> !transport_h2g ||
>> +                       (remote_flags & VMADDR_FLAG_TO_HOST) ==
>> VMADDR_FLAG_TO_HOST)
>
> Maybe "remote_flags & VMADDR_FLAG_TO_HOST" should be enough, but the
> patch is okay:
>
> Reviewed-by: Stefano Garzarella <[email protected]>

Done, updated to have only the bitwise logic, without the comparison.

Thanks,
Andra

>
>>                       new_transport = transport_g2h;
>>               else
>>                       new_transport = transport_h2g;
>> --
>> 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.