RE: [PATCH v2] virtio/vsock: fix the transport to work with VMADDR_CID_ANY

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:

quoted

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 <redacted>
---
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>

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:

quoted

On Thu, Nov 25, 2021 at 08:18:23PM -0500, Wei Wang wrote:

quoted

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 <redacted>
---
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>