linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v2] virtio/vsock: fix the transport to work with VMADDR_CID_ANY
@ 2021-11-26  1:18 Wei Wang
  2021-11-26  8:53 ` Stefano Garzarella
  0 siblings, 1 reply; 4+ messages in thread
From: Wei Wang @ 2021-11-26  1:18 UTC (permalink / raw)
  To: mst, stefanha, sgarzare, davem, linux-kernel, virtualization; +Cc: Wei Wang

The VMADDR_CID_ANY flag used by a socket means that the socket isn't bound
to any specific CID. For example, a host vsock server may want to be bound
with VMADDR_CID_ANY, so that a guest vsock client can connect to the host
server with CID=VMADDR_CID_HOST (i.e. 2), and meanwhile, a host vsock
client can connect to the same local server with CID=VMADDR_CID_LOCAL
(i.e. 1).

The current implementation sets the destination socket's svm_cid to a
fixed CID value after the first client's connection, which isn't an
expected operation. For example, if the guest client first connects to the
host server, the server's svm_cid gets set to VMADDR_CID_HOST, then other
host clients won't be able to connect to the server anymore.

Reproduce steps:
1. Run the host server:
   socat VSOCK-LISTEN:1234,fork -
2. Run a guest client to connect to the host server:
   socat - VSOCK-CONNECT:2:1234
3. Run a host client to connect to the host server:
   socat - VSOCK-CONNECT:1:1234

Without this patch, step 3. above fails to connect, and socat complains
"socat[1720] E connect(5, AF=40 cid:1 port:1234, 16): Connection
reset by peer".
With this patch, the above works well.

Fixes: c0cfa2d8a788 ("vsock: add multi-transports support")
Signed-off-by: Wei Wang <wei.w.wang@intel.com>
---
 net/vmw_vsock/virtio_transport_common.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/net/vmw_vsock/virtio_transport_common.c b/net/vmw_vsock/virtio_transport_common.c
index 59ee1be5a6dd..ec2c2afbf0d0 100644
--- a/net/vmw_vsock/virtio_transport_common.c
+++ b/net/vmw_vsock/virtio_transport_common.c
@@ -1299,7 +1299,8 @@ void virtio_transport_recv_pkt(struct virtio_transport *t,
 	space_available = virtio_transport_space_update(sk, pkt);
 
 	/* Update CID in case it has changed after a transport reset event */
-	vsk->local_addr.svm_cid = dst.svm_cid;
+	if (vsk->local_addr.svm_cid != VMADDR_CID_ANY)
+		vsk->local_addr.svm_cid = dst.svm_cid;
 
 	if (space_available)
 		sk->sk_write_space(sk);

base-commit: 5f53fa508db098c9d372423a6dac31c8a5679cdf
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 4+ messages in thread

* Re: [PATCH v2] virtio/vsock: fix the transport to work with VMADDR_CID_ANY
  2021-11-26  1:18 [PATCH v2] virtio/vsock: fix the transport to work with VMADDR_CID_ANY Wei Wang
@ 2021-11-26  8:53 ` Stefano Garzarella
  2021-11-30  1:34   ` Wang, Wei W
  0 siblings, 1 reply; 4+ messages in thread
From: Stefano Garzarella @ 2021-11-26  8:53 UTC (permalink / raw)
  To: Wei Wang; +Cc: mst, stefanha, davem, linux-kernel, virtualization

On Thu, Nov 25, 2021 at 08:18:23PM -0500, Wei Wang wrote:
>The VMADDR_CID_ANY flag used by a socket means that the socket isn't bound
>to any specific CID. For example, a host vsock server may want to be bound
>with VMADDR_CID_ANY, so that a guest vsock client can connect to the host
>server with CID=VMADDR_CID_HOST (i.e. 2), and meanwhile, a host vsock
>client can connect to the same local server with CID=VMADDR_CID_LOCAL
>(i.e. 1).
>
>The current implementation sets the destination socket's svm_cid to a
>fixed CID value after the first client's connection, which isn't an
>expected operation. For example, if the guest client first connects to the
>host server, the server's svm_cid gets set to VMADDR_CID_HOST, then other
>host clients won't be able to connect to the server anymore.
>
>Reproduce steps:
>1. Run the host server:
>   socat VSOCK-LISTEN:1234,fork -
>2. Run a guest client to connect to the host server:
>   socat - VSOCK-CONNECT:2:1234
>3. Run a host client to connect to the host server:
>   socat - VSOCK-CONNECT:1:1234
>
>Without this patch, step 3. above fails to connect, and socat complains
>"socat[1720] E connect(5, AF=40 cid:1 port:1234, 16): Connection
>reset by peer".
>With this patch, the above works well.
>
>Fixes: c0cfa2d8a788 ("vsock: add multi-transports support")
>Signed-off-by: Wei Wang <wei.w.wang@intel.com>
>---
> net/vmw_vsock/virtio_transport_common.c | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)

Usually fixes for net/vmw_vsock/* are applied through the net tree 
(netdev@vger.kernel.org) that seems not CCed. Please use 
./scripts/get_maintainer.pl next time.

Maybe this one can be queued by Michael, let's wait a bit, otherwise 
please resend CCing netdev and using "net" tag.

Anyway the patch LGTM:

Reviewed-by: Stefano Garzarella <sgarzare@redhat.com>


^ permalink raw reply	[flat|nested] 4+ messages in thread

* RE: [PATCH v2] virtio/vsock: fix the transport to work with VMADDR_CID_ANY
  2021-11-26  8:53 ` Stefano Garzarella
@ 2021-11-30  1:34   ` Wang, Wei W
  2021-11-30 23:21     ` Michael S. Tsirkin
  0 siblings, 1 reply; 4+ messages in thread
From: Wang, Wei W @ 2021-11-30  1:34 UTC (permalink / raw)
  To: Stefano Garzarella
  Cc: mst, stefanha, davem, linux-kernel, virtualization, netdev

Hi Michael,

Do you plan to merge this patch through your tree?
If not, I'll resend to have it applied to the net tree.

Thanks,
Wei

On Friday, November 26, 2021 4:54 PM, Stefano Garzarella wrote:
> On Thu, Nov 25, 2021 at 08:18:23PM -0500, Wei Wang wrote:
> >The VMADDR_CID_ANY flag used by a socket means that the socket isn't
> >bound to any specific CID. For example, a host vsock server may want to
> >be bound with VMADDR_CID_ANY, so that a guest vsock client can connect
> >to the host server with CID=VMADDR_CID_HOST (i.e. 2), and meanwhile, a
> >host vsock client can connect to the same local server with
> >CID=VMADDR_CID_LOCAL (i.e. 1).
> >
> >The current implementation sets the destination socket's svm_cid to a
> >fixed CID value after the first client's connection, which isn't an
> >expected operation. For example, if the guest client first connects to
> >the host server, the server's svm_cid gets set to VMADDR_CID_HOST, then
> >other host clients won't be able to connect to the server anymore.
> >
> >Reproduce steps:
> >1. Run the host server:
> >   socat VSOCK-LISTEN:1234,fork -
> >2. Run a guest client to connect to the host server:
> >   socat - VSOCK-CONNECT:2:1234
> >3. Run a host client to connect to the host server:
> >   socat - VSOCK-CONNECT:1:1234
> >
> >Without this patch, step 3. above fails to connect, and socat complains
> >"socat[1720] E connect(5, AF=40 cid:1 port:1234, 16): Connection reset
> >by peer".
> >With this patch, the above works well.
> >
> >Fixes: c0cfa2d8a788 ("vsock: add multi-transports support")
> >Signed-off-by: Wei Wang <wei.w.wang@intel.com>
> >---
> > net/vmw_vsock/virtio_transport_common.c | 3 ++-
> > 1 file changed, 2 insertions(+), 1 deletion(-)
> 
> Usually fixes for net/vmw_vsock/* are applied through the net tree
> (netdev@vger.kernel.org) that seems not CCed. Please
> use ./scripts/get_maintainer.pl next time.
> 
> Maybe this one can be queued by Michael, let's wait a bit, otherwise please
> resend CCing netdev and using "net" tag.
> 
> Anyway the patch LGTM:
> 
> Reviewed-by: Stefano Garzarella <sgarzare@redhat.com>


^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH v2] virtio/vsock: fix the transport to work with VMADDR_CID_ANY
  2021-11-30  1:34   ` Wang, Wei W
@ 2021-11-30 23:21     ` Michael S. Tsirkin
  0 siblings, 0 replies; 4+ messages in thread
From: Michael S. Tsirkin @ 2021-11-30 23:21 UTC (permalink / raw)
  To: Wang, Wei W
  Cc: Stefano Garzarella, stefanha, davem, linux-kernel,
	virtualization, netdev

On Tue, Nov 30, 2021 at 01:34:20AM +0000, Wang, Wei W wrote:
> Hi Michael,
> 
> Do you plan to merge this patch through your tree?
> If not, I'll resend to have it applied to the net tree.
> 
> Thanks,
> Wei


Sure, I'll merge it. Thanks!

> On Friday, November 26, 2021 4:54 PM, Stefano Garzarella wrote:
> > On Thu, Nov 25, 2021 at 08:18:23PM -0500, Wei Wang wrote:
> > >The VMADDR_CID_ANY flag used by a socket means that the socket isn't
> > >bound to any specific CID. For example, a host vsock server may want to
> > >be bound with VMADDR_CID_ANY, so that a guest vsock client can connect
> > >to the host server with CID=VMADDR_CID_HOST (i.e. 2), and meanwhile, a
> > >host vsock client can connect to the same local server with
> > >CID=VMADDR_CID_LOCAL (i.e. 1).
> > >
> > >The current implementation sets the destination socket's svm_cid to a
> > >fixed CID value after the first client's connection, which isn't an
> > >expected operation. For example, if the guest client first connects to
> > >the host server, the server's svm_cid gets set to VMADDR_CID_HOST, then
> > >other host clients won't be able to connect to the server anymore.
> > >
> > >Reproduce steps:
> > >1. Run the host server:
> > >   socat VSOCK-LISTEN:1234,fork -
> > >2. Run a guest client to connect to the host server:
> > >   socat - VSOCK-CONNECT:2:1234
> > >3. Run a host client to connect to the host server:
> > >   socat - VSOCK-CONNECT:1:1234
> > >
> > >Without this patch, step 3. above fails to connect, and socat complains
> > >"socat[1720] E connect(5, AF=40 cid:1 port:1234, 16): Connection reset
> > >by peer".
> > >With this patch, the above works well.
> > >
> > >Fixes: c0cfa2d8a788 ("vsock: add multi-transports support")
> > >Signed-off-by: Wei Wang <wei.w.wang@intel.com>
> > >---
> > > net/vmw_vsock/virtio_transport_common.c | 3 ++-
> > > 1 file changed, 2 insertions(+), 1 deletion(-)
> > 
> > Usually fixes for net/vmw_vsock/* are applied through the net tree
> > (netdev@vger.kernel.org) that seems not CCed. Please
> > use ./scripts/get_maintainer.pl next time.
> > 
> > Maybe this one can be queued by Michael, let's wait a bit, otherwise please
> > resend CCing netdev and using "net" tag.
> > 
> > Anyway the patch LGTM:
> > 
> > Reviewed-by: Stefano Garzarella <sgarzare@redhat.com>


^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2021-11-30 23:22 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-11-26  1:18 [PATCH v2] virtio/vsock: fix the transport to work with VMADDR_CID_ANY Wei Wang
2021-11-26  8:53 ` Stefano Garzarella
2021-11-30  1:34   ` Wang, Wei W
2021-11-30 23:21     ` Michael S. Tsirkin

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).