diff mbox series

[net-next,v2,4/4] af_vsock: Assign the vsock transport considering the vsock address flags

Message ID 20201204170235.84387-5-andraprs@amazon.com (mailing list archive)
State Changes Requested
Delegated to: Netdev Maintainers
Headers show
Series vsock: Add flags field in the vsock address | expand

Checks

Context Check Description
netdev/cover_letter success Link
netdev/fixes_present success Link
netdev/patch_count success Link
netdev/tree_selection success Clearly marked for net-next
netdev/subject_prefix success Link
netdev/source_inline success Was 0 now: 0
netdev/verify_signedoff success Link
netdev/module_param success Was 0 now: 0
netdev/build_32bit success Errors and warnings before: 0 this patch: 0
netdev/kdoc success Errors and warnings before: 0 this patch: 0
netdev/verify_fixes success Link
netdev/checkpatch warning WARNING: line length of 85 exceeds 80 columns
netdev/build_allmodconfig_warn success Errors and warnings before: 0 this patch: 0
netdev/header_inline success Link
netdev/stable success Stable not CCed

Commit Message

Paraschiv, Andra-Irina Dec. 4, 2020, 5:02 p.m. UTC
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 <andraprs@amazon.com>
---
 net/vmw_vsock/af_vsock.c | 9 +++++++--
 1 file changed, 7 insertions(+), 2 deletions(-)

Comments

Stefano Garzarella Dec. 7, 2020, 10 a.m. UTC | #1
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 <andraprs@amazon.com>
>---
> 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 <sgarzare@redhat.com>

> 			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.
>
Paraschiv, Andra-Irina Dec. 7, 2020, 7:51 p.m. UTC | #2
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 <andraprs@amazon.com>
>> ---
>> 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 <sgarzare@redhat.com>

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.
diff mbox series

Patch

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;