diff mbox series

[net-next,v1,3/3] af_vsock: Assign the vsock transport considering the vsock address flag

Message ID 20201201152505.19445-4-andraprs@amazon.com
State Superseded
Headers show
Series vsock: Add flag field in the vsock address | expand

Commit Message

Paraschiv, Andra-Irina Dec. 1, 2020, 3:25 p.m. UTC
The vsock flag has been 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 flag (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 vsock flag is set. For the other use
cases, the vsock transport assignment logic is not changed.

Signed-off-by: Andra Paraschiv <andraprs@amazon.com>
---
 net/vmw_vsock/af_vsock.c | 15 +++++++++++----
 1 file changed, 11 insertions(+), 4 deletions(-)

Comments

Stefano Garzarella Dec. 1, 2020, 4:23 p.m. UTC | #1
On Tue, Dec 01, 2020 at 05:25:05PM +0200, Andra Paraschiv wrote:
>The vsock flag has been 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 flag (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 vsock flag is set. For the other use
>cases, the vsock transport assignment logic is not changed.
>
>Signed-off-by: Andra Paraschiv <andraprs@amazon.com>
>---
> net/vmw_vsock/af_vsock.c | 15 +++++++++++----
> 1 file changed, 11 insertions(+), 4 deletions(-)
>
>diff --git a/net/vmw_vsock/af_vsock.c b/net/vmw_vsock/af_vsock.c
>index d10916ab45267..bafc1cb20abd4 100644
>--- a/net/vmw_vsock/af_vsock.c
>+++ b/net/vmw_vsock/af_vsock.c
>@@ -419,16 +419,21 @@ static void vsock_deassign_transport(struct vsock_sock *vsk)
>  * (e.g. during the connect() or when a connection request on a listener
>  * socket is received).
>  * 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 will use host->guest transport;
>+ *  - remote flag == VMADDR_FLAG_SIBLING_VMS_COMMUNICATION, will always
>+ *    forward the vsock packets to the host and use guest->host transport;
>+ *  - otherwise, going forward with the remote flag default value:
>+ *    - 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 or h2g is not loaded, 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)
> {
> 	const struct vsock_transport *new_transport;
> 	struct sock *sk = sk_vsock(vsk);
> 	unsigned int remote_cid = vsk->remote_addr.svm_cid;
>+	unsigned short remote_flag = vsk->remote_addr.svm_flag;
> 	int ret;
>
> 	switch (sk->sk_type) {
>@@ -438,6 +443,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_flag == VMADDR_FLAG_SIBLING_VMS_COMMUNICATION)

Others flags can be added, so here we should use the bitwise AND 
operator to check if this flag is set.

And what about merging with the next if clause?


Thanks,
Stefano

>+			new_transport = transport_g2h;
> 		else if (remote_cid <= VMADDR_CID_HOST || 
> 		!transport_h2g)
> 			new_transport = transport_g2h;
> 		else
>-- 
>2.20.1 (Apple Git-117)
>
Paraschiv, Andra-Irina Dec. 1, 2020, 7:06 p.m. UTC | #2
On 01/12/2020 18:23, Stefano Garzarella wrote:
>
> On Tue, Dec 01, 2020 at 05:25:05PM +0200, Andra Paraschiv wrote:
>> The vsock flag has been 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 flag (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 vsock flag is set. For the other use
>> cases, the vsock transport assignment logic is not changed.
>>
>> Signed-off-by: Andra Paraschiv <andraprs@amazon.com>
>> ---
>> net/vmw_vsock/af_vsock.c | 15 +++++++++++----
>> 1 file changed, 11 insertions(+), 4 deletions(-)
>>
>> diff --git a/net/vmw_vsock/af_vsock.c b/net/vmw_vsock/af_vsock.c
>> index d10916ab45267..bafc1cb20abd4 100644
>> --- a/net/vmw_vsock/af_vsock.c
>> +++ b/net/vmw_vsock/af_vsock.c
>> @@ -419,16 +419,21 @@ static void vsock_deassign_transport(struct 
>> vsock_sock *vsk)
>>  * (e.g. during the connect() or when a connection request on a listener
>>  * socket is received).
>>  * 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 will use host->guest transport;
>> + *  - remote flag == VMADDR_FLAG_SIBLING_VMS_COMMUNICATION, will always
>> + *    forward the vsock packets to the host and use guest->host 
>> transport;
>> + *  - otherwise, going forward with the remote flag default value:
>> + *    - 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 or h2g is not loaded, 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)
>> {
>>       const struct vsock_transport *new_transport;
>>       struct sock *sk = sk_vsock(vsk);
>>       unsigned int remote_cid = vsk->remote_addr.svm_cid;
>> +      unsigned short remote_flag = vsk->remote_addr.svm_flag;
>>       int ret;
>>
>>       switch (sk->sk_type) {
>> @@ -438,6 +443,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_flag == 
>> VMADDR_FLAG_SIBLING_VMS_COMMUNICATION)
>
> Others flags can be added, so here we should use the bitwise AND
> operator to check if this flag is set.
>
> And what about merging with the next if clause?
>

Indeed, I'll update the codebase to use the bitwise operator. Then I can 
also merge all the checks corresponding to the g2h transport in a single 
if block.

Thanks,
Andra

>
>> +                      new_transport = transport_g2h;
>>               else if (remote_cid <= VMADDR_CID_HOST ||
>>               !transport_h2g)
>>                       new_transport = transport_g2h;
>>               else
>> -- 
>> 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.
diff mbox series

Patch

diff --git a/net/vmw_vsock/af_vsock.c b/net/vmw_vsock/af_vsock.c
index d10916ab45267..bafc1cb20abd4 100644
--- a/net/vmw_vsock/af_vsock.c
+++ b/net/vmw_vsock/af_vsock.c
@@ -419,16 +419,21 @@  static void vsock_deassign_transport(struct vsock_sock *vsk)
  * (e.g. during the connect() or when a connection request on a listener
  * socket is received).
  * 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 will use host->guest transport;
+ *  - remote flag == VMADDR_FLAG_SIBLING_VMS_COMMUNICATION, will always
+ *    forward the vsock packets to the host and use guest->host transport;
+ *  - otherwise, going forward with the remote flag default value:
+ *    - 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 or h2g is not loaded, 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)
 {
 	const struct vsock_transport *new_transport;
 	struct sock *sk = sk_vsock(vsk);
 	unsigned int remote_cid = vsk->remote_addr.svm_cid;
+	unsigned short remote_flag = vsk->remote_addr.svm_flag;
 	int ret;
 
 	switch (sk->sk_type) {
@@ -438,6 +443,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_flag == VMADDR_FLAG_SIBLING_VMS_COMMUNICATION)
+			new_transport = transport_g2h;
 		else if (remote_cid <= VMADDR_CID_HOST || !transport_h2g)
 			new_transport = transport_g2h;
 		else