diff mbox series

[net] vsock: forward all packets to the host when no H2G is registered

Message ID 20201112133837.34183-1-sgarzare@redhat.com (mailing list archive)
State Accepted
Commit 65b422d9b61ba12c08150784e8012fa1892ad03e
Delegated to: Netdev Maintainers
Headers show
Series [net] vsock: forward all packets to the host when no H2G is registered | 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
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 success total: 0 errors, 0 warnings, 0 checks, 8 lines checked
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

Stefano Garzarella Nov. 12, 2020, 1:38 p.m. UTC
Before commit c0cfa2d8a788 ("vsock: add multi-transports support"),
if a G2H transport was loaded (e.g. virtio transport), every packets
was forwarded to the host, regardless of the destination CID.
The H2G transports implemented until then (vhost-vsock, VMCI) always
responded with an error, if the destination CID was not
VMADDR_CID_HOST.

From that commit, we are using the remote CID to decide which
transport to use, so packets with remote CID > VMADDR_CID_HOST(2)
are sent only through H2G transport. If no H2G is available, packets
are discarded directly in the guest.

Some use cases (e.g. Nitro Enclaves [1]) rely on the old behaviour
to implement sibling VMs communication, so we restore the old
behavior when no H2G is registered.
It will be up to the host to discard packets if the destination is
not the right one. As it was already implemented before adding
multi-transport support.

Tested with nested QEMU/KVM by me and Nitro Enclaves by Andra.

[1] Documentation/virt/ne_overview.rst

Cc: Jorgen Hansen <jhansen@vmware.com>
Cc: Dexuan Cui <decui@microsoft.com>
Fixes: c0cfa2d8a788 ("vsock: add multi-transports support")
Reported-by: Andra Paraschiv <andraprs@amazon.com>
Tested-by: Andra Paraschiv <andraprs@amazon.com>
Signed-off-by: Stefano Garzarella <sgarzare@redhat.com>
---
 net/vmw_vsock/af_vsock.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

patchwork-bot+netdevbpf@kernel.org Nov. 14, 2020, 7:50 p.m. UTC | #1
Hello:

This patch was applied to netdev/net.git (refs/heads/master):

On Thu, 12 Nov 2020 14:38:37 +0100 you wrote:
> Before commit c0cfa2d8a788 ("vsock: add multi-transports support"),
> if a G2H transport was loaded (e.g. virtio transport), every packets
> was forwarded to the host, regardless of the destination CID.
> The H2G transports implemented until then (vhost-vsock, VMCI) always
> responded with an error, if the destination CID was not
> VMADDR_CID_HOST.
> 
> [...]

Here is the summary with links:
  - [net] vsock: forward all packets to the host when no H2G is registered
    https://git.kernel.org/netdev/net/c/65b422d9b61b

You are awesome, thank you!
--
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html
Stefan Hajnoczi Nov. 19, 2020, 2:03 p.m. UTC | #2
On Thu, Nov 12, 2020 at 02:38:37PM +0100, Stefano Garzarella wrote:
> Before commit c0cfa2d8a788 ("vsock: add multi-transports support"),
> if a G2H transport was loaded (e.g. virtio transport), every packets
> was forwarded to the host, regardless of the destination CID.
> The H2G transports implemented until then (vhost-vsock, VMCI) always
> responded with an error, if the destination CID was not
> VMADDR_CID_HOST.
> 
> From that commit, we are using the remote CID to decide which
> transport to use, so packets with remote CID > VMADDR_CID_HOST(2)
> are sent only through H2G transport. If no H2G is available, packets
> are discarded directly in the guest.
> 
> Some use cases (e.g. Nitro Enclaves [1]) rely on the old behaviour
> to implement sibling VMs communication, so we restore the old
> behavior when no H2G is registered.
> It will be up to the host to discard packets if the destination is
> not the right one. As it was already implemented before adding
> multi-transport support.
> 
> Tested with nested QEMU/KVM by me and Nitro Enclaves by Andra.
> 
> [1] Documentation/virt/ne_overview.rst
> 
> Cc: Jorgen Hansen <jhansen@vmware.com>
> Cc: Dexuan Cui <decui@microsoft.com>
> Fixes: c0cfa2d8a788 ("vsock: add multi-transports support")
> Reported-by: Andra Paraschiv <andraprs@amazon.com>
> Tested-by: Andra Paraschiv <andraprs@amazon.com>
> Signed-off-by: Stefano Garzarella <sgarzare@redhat.com>
> ---
>  net/vmw_vsock/af_vsock.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)

Acked-by: Stefan Hajnoczi <stefanha@redhat.com>
Alexander Graf Nov. 19, 2020, 2:25 p.m. UTC | #3
On 19.11.20 15:03, Stefan Hajnoczi wrote:
> On Thu, Nov 12, 2020 at 02:38:37PM +0100, Stefano Garzarella wrote:
>> Before commit c0cfa2d8a788 ("vsock: add multi-transports support"),
>> if a G2H transport was loaded (e.g. virtio transport), every packets
>> was forwarded to the host, regardless of the destination CID.
>> The H2G transports implemented until then (vhost-vsock, VMCI) always
>> responded with an error, if the destination CID was not
>> VMADDR_CID_HOST.
>>
>>  From that commit, we are using the remote CID to decide which
>> transport to use, so packets with remote CID > VMADDR_CID_HOST(2)
>> are sent only through H2G transport. If no H2G is available, packets
>> are discarded directly in the guest.
>>
>> Some use cases (e.g. Nitro Enclaves [1]) rely on the old behaviour
>> to implement sibling VMs communication, so we restore the old
>> behavior when no H2G is registered.
>> It will be up to the host to discard packets if the destination is
>> not the right one. As it was already implemented before adding
>> multi-transport support.
>>
>> Tested with nested QEMU/KVM by me and Nitro Enclaves by Andra.
>>
>> [1] Documentation/virt/ne_overview.rst
>>
>> Cc: Jorgen Hansen <jhansen@vmware.com>
>> Cc: Dexuan Cui <decui@microsoft.com>
>> Fixes: c0cfa2d8a788 ("vsock: add multi-transports support")
>> Reported-by: Andra Paraschiv <andraprs@amazon.com>
>> Tested-by: Andra Paraschiv <andraprs@amazon.com>
>> Signed-off-by: Stefano Garzarella <sgarzare@redhat.com>
>> ---
>>   net/vmw_vsock/af_vsock.c | 2 +-
>>   1 file changed, 1 insertion(+), 1 deletion(-)
> Acked-by: Stefan Hajnoczi <stefanha@redhat.com>


Is there anything we have to do to also get this into the affected 
stable trees? :)

Alex





Amazon Development Center Germany GmbH
Krausenstr. 38
10117 Berlin
Geschaeftsfuehrung: Christian Schlaeger, Jonathan Weiss
Eingetragen am Amtsgericht Charlottenburg unter HRB 149173 B
Sitz: Berlin
Ust-ID: DE 289 237 879
Stefano Garzarella Nov. 19, 2020, 2:41 p.m. UTC | #4
On Thu, Nov 19, 2020 at 03:25:42PM +0100, Alexander Graf wrote:
>
>On 19.11.20 15:03, Stefan Hajnoczi wrote:
>>On Thu, Nov 12, 2020 at 02:38:37PM +0100, Stefano Garzarella wrote:
>>>Before commit c0cfa2d8a788 ("vsock: add multi-transports support"),
>>>if a G2H transport was loaded (e.g. virtio transport), every packets
>>>was forwarded to the host, regardless of the destination CID.
>>>The H2G transports implemented until then (vhost-vsock, VMCI) always
>>>responded with an error, if the destination CID was not
>>>VMADDR_CID_HOST.
>>>
>>> From that commit, we are using the remote CID to decide which
>>>transport to use, so packets with remote CID > VMADDR_CID_HOST(2)
>>>are sent only through H2G transport. If no H2G is available, packets
>>>are discarded directly in the guest.
>>>
>>>Some use cases (e.g. Nitro Enclaves [1]) rely on the old behaviour
>>>to implement sibling VMs communication, so we restore the old
>>>behavior when no H2G is registered.
>>>It will be up to the host to discard packets if the destination is
>>>not the right one. As it was already implemented before adding
>>>multi-transport support.
>>>
>>>Tested with nested QEMU/KVM by me and Nitro Enclaves by Andra.
>>>
>>>[1] Documentation/virt/ne_overview.rst
>>>
>>>Cc: Jorgen Hansen <jhansen@vmware.com>
>>>Cc: Dexuan Cui <decui@microsoft.com>
>>>Fixes: c0cfa2d8a788 ("vsock: add multi-transports support")
>>>Reported-by: Andra Paraschiv <andraprs@amazon.com>
>>>Tested-by: Andra Paraschiv <andraprs@amazon.com>
>>>Signed-off-by: Stefano Garzarella <sgarzare@redhat.com>
>>>---
>>>  net/vmw_vsock/af_vsock.c | 2 +-
>>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>Acked-by: Stefan Hajnoczi <stefanha@redhat.com>
>
>
>Is there anything we have to do to also get this into the affected 
>stable trees? :)
>

The patch is already queued by Jakub in the netdev stable queue:
https://patchwork.kernel.org/bundle/netdev/stable/?series=382773&state=*

Stefano
diff mbox series

Patch

diff --git a/net/vmw_vsock/af_vsock.c b/net/vmw_vsock/af_vsock.c
index b4d7b8aba003..d10916ab4526 100644
--- a/net/vmw_vsock/af_vsock.c
+++ b/net/vmw_vsock/af_vsock.c
@@ -438,7 +438,7 @@  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)
+		else if (remote_cid <= VMADDR_CID_HOST || !transport_h2g)
 			new_transport = transport_g2h;
 		else
 			new_transport = transport_h2g;