[PATCH v2 0/4] vsock: cancel connect packets when failing to connect

[PATCH v2 0/4] vsock: cancel connect packets when failing to connect

[Peng Tao]

Currently, if a connect call fails on a signal or timeout (e.g., guest is still
in the process of starting up), we'll just return to caller and leave the connect
packet queued and they are sent even though the connection is considered a failure,
which can confuse applications with unwanted false connect attempt.

The patchset enables vsock (both host and guest) to cancel queued packets when
a connect attempt is considered to fail.

v2 changelog:
  - fix queued_replies counting and resume tx/rx when necessary

Peng Tao (4):
  vsock: track pkt owner vsock
  vhost-vsock: add pkt cancel capability
  vsock: add pkt cancel capability
  vsock: cancel packets when failing to connect

 drivers/vhost/vsock.c                   | 41 ++++++++++++++++++++++++++++++++
 include/linux/virtio_vsock.h            | 12 ++++++++++
 net/vmw_vsock/af_vsock.c                |  7 ++++++
 net/vmw_vsock/virtio_transport.c        | 42 +++++++++++++++++++++++++++++++++
 net/vmw_vsock/virtio_transport_common.c | 14 +++++------
 5 files changed, 109 insertions(+), 7 deletions(-)

-- 
2.7.4

[PATCH v2 1/4] vsock: track pkt owner vsock

[Peng Tao]

So that we can cancel a queued pkt later if necessary.

Signed-off-by: Peng Tao <bergwolf@gmail.com>
---
 include/linux/virtio_vsock.h            | 2 ++
 net/vmw_vsock/virtio_transport_common.c | 7 +++++++
 2 files changed, 9 insertions(+)

diff --git a/include/linux/virtio_vsock.h b/include/linux/virtio_vsock.h
index 9638bfe..6dd3242 100644
--- a/include/linux/virtio_vsock.h
+++ b/include/linux/virtio_vsock.h
@@ -48,6 +48,7 @@ struct virtio_vsock_pkt {
 	struct virtio_vsock_hdr	hdr;
 	struct work_struct work;
 	struct list_head list;
+	struct vsock_sock *vsk;
 	void *buf;
 	u32 len;
 	u32 off;
@@ -56,6 +57,7 @@ struct virtio_vsock_pkt {
 
 struct virtio_vsock_pkt_info {
 	u32 remote_cid, remote_port;
+	struct vsock_sock *vsk;
 	struct msghdr *msg;
 	u32 pkt_len;
 	u16 type;
diff --git a/net/vmw_vsock/virtio_transport_common.c b/net/vmw_vsock/virtio_transport_common.c
index a53b3a1..cc1eeb5 100644
--- a/net/vmw_vsock/virtio_transport_common.c
+++ b/net/vmw_vsock/virtio_transport_common.c
@@ -57,6 +57,7 @@ virtio_transport_alloc_pkt(struct virtio_vsock_pkt_info *info,
 	pkt->len		= len;
 	pkt->hdr.len		= cpu_to_le32(len);
 	pkt->reply		= info->reply;
+	pkt->vsk		= info->vsk;
 
 	if (info->msg && len > 0) {
 		pkt->buf = kmalloc(len, GFP_KERNEL);
@@ -180,6 +181,7 @@ static int virtio_transport_send_credit_update(struct vsock_sock *vsk,
 	struct virtio_vsock_pkt_info info = {
 		.op = VIRTIO_VSOCK_OP_CREDIT_UPDATE,
 		.type = type,
+		.vsk = vsk,
 	};
 
 	return virtio_transport_send_pkt_info(vsk, &info);
@@ -519,6 +521,7 @@ int virtio_transport_connect(struct vsock_sock *vsk)
 	struct virtio_vsock_pkt_info info = {
 		.op = VIRTIO_VSOCK_OP_REQUEST,
 		.type = VIRTIO_VSOCK_TYPE_STREAM,
+		.vsk = vsk,
 	};
 
 	return virtio_transport_send_pkt_info(vsk, &info);
@@ -534,6 +537,7 @@ int virtio_transport_shutdown(struct vsock_sock *vsk, int mode)
 			  VIRTIO_VSOCK_SHUTDOWN_RCV : 0) |
 			 (mode & SEND_SHUTDOWN ?
 			  VIRTIO_VSOCK_SHUTDOWN_SEND : 0),
+		.vsk = vsk,
 	};
 
 	return virtio_transport_send_pkt_info(vsk, &info);
@@ -560,6 +564,7 @@ virtio_transport_stream_enqueue(struct vsock_sock *vsk,
 		.type = VIRTIO_VSOCK_TYPE_STREAM,
 		.msg = msg,
 		.pkt_len = len,
+		.vsk = vsk,
 	};
 
 	return virtio_transport_send_pkt_info(vsk, &info);
@@ -581,6 +586,7 @@ static int virtio_transport_reset(struct vsock_sock *vsk,
 		.op = VIRTIO_VSOCK_OP_RST,
 		.type = VIRTIO_VSOCK_TYPE_STREAM,
 		.reply = !!pkt,
+		.vsk = vsk,
 	};
 
 	/* Send RST only if the original pkt is not a RST pkt */
@@ -826,6 +832,7 @@ virtio_transport_send_response(struct vsock_sock *vsk,
 		.remote_cid = le32_to_cpu(pkt->hdr.src_cid),
 		.remote_port = le32_to_cpu(pkt->hdr.src_port),
 		.reply = true,
+		.vsk = vsk,
 	};
 
 	return virtio_transport_send_pkt_info(vsk, &info);
-- 
2.7.4

[PATCH v2 2/4] vhost-vsock: add pkt cancel capability

[Peng Tao]

To allow canceling all packets of a connection.

Signed-off-by: Peng Tao <bergwolf@gmail.com>
---
 drivers/vhost/vsock.c        | 41 +++++++++++++++++++++++++++++++++++++++++
 include/linux/virtio_vsock.h |  3 +++
 2 files changed, 44 insertions(+)

diff --git a/drivers/vhost/vsock.c b/drivers/vhost/vsock.c
index a504e2e0..d01e4a4 100644
--- a/drivers/vhost/vsock.c
+++ b/drivers/vhost/vsock.c
@@ -218,6 +218,46 @@ vhost_transport_send_pkt(struct virtio_vsock_pkt *pkt)
 	return len;
 }
 
+static int
+vhost_transport_cancel_pkt(struct vsock_sock *vsk)
+{
+	struct vhost_vsock *vsock;
+	struct virtio_vsock_pkt *pkt, *n;
+	int cnt = 0;
+	LIST_HEAD(freeme);
+
+	/* Find the vhost_vsock according to guest context id  */
+	vsock = vhost_vsock_get(vsk->remote_addr.svm_cid);
+	if (!vsock)
+		return -ENODEV;
+
+	spin_lock_bh(&vsock->send_pkt_list_lock);
+	list_for_each_entry_safe(pkt, n, &vsock->send_pkt_list, list) {
+		if (pkt->vsk != vsk)
+			continue;
+		list_move(&pkt->list, &freeme);
+	}
+	spin_unlock_bh(&vsock->send_pkt_list_lock);
+
+	list_for_each_entry_safe(pkt, n, &freeme, list) {
+		if (pkt->reply)
+			cnt++;
+		list_del(&pkt->list);
+		virtio_transport_free_pkt(pkt);
+	}
+
+	if (cnt) {
+		struct vhost_virtqueue *tx_vq = &vsock->vqs[VSOCK_VQ_TX];
+		int new_cnt;
+
+		new_cnt = atomic_sub_return(cnt, &vsock->queued_replies);
+		if (new_cnt + cnt >= tx_vq->num && new_cnt < tx_vq->num)
+			vhost_poll_queue(&tx_vq->poll);
+	}
+
+	return 0;
+}
+
 static struct virtio_vsock_pkt *
 vhost_vsock_alloc_pkt(struct vhost_virtqueue *vq,
 		      unsigned int out, unsigned int in)
@@ -698,6 +738,7 @@ static struct virtio_transport vhost_transport = {
 	},
 
 	.send_pkt = vhost_transport_send_pkt,
+	.cancel_pkt = vhost_transport_cancel_pkt,
 };
 
 static int __init vhost_vsock_init(void)
diff --git a/include/linux/virtio_vsock.h b/include/linux/virtio_vsock.h
index 6dd3242..b92e88d 100644
--- a/include/linux/virtio_vsock.h
+++ b/include/linux/virtio_vsock.h
@@ -72,6 +72,9 @@ struct virtio_transport {
 
 	/* Takes ownership of the packet */
 	int (*send_pkt)(struct virtio_vsock_pkt *pkt);
+
+	/* Cancel packets belonging the same vsock */
+	int (*cancel_pkt)(struct vsock_sock *vsk);
 };
 
 ssize_t
-- 
2.7.4

[PATCH v2 3/4] vsock: add pkt cancel capability

[Peng Tao]

Signed-off-by: Peng Tao <bergwolf@gmail.com>
---
 net/vmw_vsock/virtio_transport.c | 42 ++++++++++++++++++++++++++++++++++++++++
 1 file changed, 42 insertions(+)

diff --git a/net/vmw_vsock/virtio_transport.c b/net/vmw_vsock/virtio_transport.c
index 936d7ee..a5f3833 100644
--- a/net/vmw_vsock/virtio_transport.c
+++ b/net/vmw_vsock/virtio_transport.c
@@ -170,6 +170,47 @@ virtio_transport_send_pkt(struct virtio_vsock_pkt *pkt)
 	return len;
 }
 
+static int
+virtio_transport_cancel_pkt(struct vsock_sock *vsk)
+{
+	struct virtio_vsock *vsock;
+	struct virtio_vsock_pkt *pkt, *n;
+	int cnt = 0;
+	LIST_HEAD(freeme);
+
+	vsock = virtio_vsock_get();
+	if (!vsock) {
+		return -ENODEV;
+	}
+
+	spin_lock_bh(&vsock->send_pkt_list_lock);
+	list_for_each_entry_safe(pkt, n, &vsock->send_pkt_list, list) {
+		if (pkt->vsk != vsk)
+			continue;
+		list_move(&pkt->list, &freeme);
+	}
+	spin_unlock_bh(&vsock->send_pkt_list_lock);
+
+	list_for_each_entry_safe(pkt, n, &freeme, list) {
+		if (pkt->reply)
+			cnt++;
+		list_del(&pkt->list);
+		virtio_transport_free_pkt(pkt);
+	}
+
+	if (cnt) {
+		struct virtqueue *rx_vq = vsock->vqs[VSOCK_VQ_RX];
+		int new_cnt;
+
+		new_cnt = atomic_sub_return(cnt, &vsock->queued_replies);
+		if (new_cnt + cnt >= virtqueue_get_vring_size(rx_vq) &&
+		    new_cnt < virtqueue_get_vring_size(rx_vq))
+			queue_work(virtio_vsock_workqueue, &vsock->rx_work);
+	}
+
+	return 0;
+}
+
 static void virtio_vsock_rx_fill(struct virtio_vsock *vsock)
 {
 	int buf_len = VIRTIO_VSOCK_DEFAULT_RX_BUF_SIZE;
@@ -453,6 +494,7 @@ static struct virtio_transport virtio_transport = {
 	},
 
 	.send_pkt = virtio_transport_send_pkt,
+	.cancel_pkt = virtio_transport_cancel_pkt,
 };
 
 static int virtio_vsock_probe(struct virtio_device *vdev)
-- 
2.7.4

[PATCH v2 4/4] vsock: cancel packets when failing to connect

[Peng Tao]

Otherwise we'll leave the packets queued until releasing vsock device.
E.g., if guest is slow to start up, resulting ETIMEDOUT on connect, guest
will get the connect requests from failed host sockets.

Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
Signed-off-by: Peng Tao <bergwolf@gmail.com>
---
 include/linux/virtio_vsock.h            | 7 +++++++
 net/vmw_vsock/af_vsock.c                | 7 +++++++
 net/vmw_vsock/virtio_transport_common.c | 7 -------
 3 files changed, 14 insertions(+), 7 deletions(-)

diff --git a/include/linux/virtio_vsock.h b/include/linux/virtio_vsock.h
index b92e88d..ff6850a 100644
--- a/include/linux/virtio_vsock.h
+++ b/include/linux/virtio_vsock.h
@@ -156,4 +156,11 @@ void virtio_transport_inc_tx_pkt(struct virtio_vsock_sock *vvs, struct virtio_vs
 u32 virtio_transport_get_credit(struct virtio_vsock_sock *vvs, u32 wanted);
 void virtio_transport_put_credit(struct virtio_vsock_sock *vvs, u32 credit);
 
+static inline const struct virtio_transport *virtio_transport_get_ops(void)
+{
+	const struct vsock_transport *t = vsock_core_get_transport();
+
+	return container_of(t, struct virtio_transport, transport);
+}
+
 #endif /* _LINUX_VIRTIO_VSOCK_H */
diff --git a/net/vmw_vsock/af_vsock.c b/net/vmw_vsock/af_vsock.c
index 8a398b3..ebb50d6 100644
--- a/net/vmw_vsock/af_vsock.c
+++ b/net/vmw_vsock/af_vsock.c
@@ -104,6 +104,7 @@
 #include <linux/unistd.h>
 #include <linux/wait.h>
 #include <linux/workqueue.h>
+#include <linux/virtio_vsock.h>
 #include <net/sock.h>
 #include <net/af_vsock.h>
 
@@ -1105,6 +1106,7 @@ static void vsock_connect_timeout(struct work_struct *work)
 {
 	struct sock *sk;
 	struct vsock_sock *vsk;
+	int cancel = 0;
 
 	vsk = container_of(work, struct vsock_sock, dwork.work);
 	sk = sk_vsock(vsk);
@@ -1115,8 +1117,11 @@ static void vsock_connect_timeout(struct work_struct *work)
 		sk->sk_state = SS_UNCONNECTED;
 		sk->sk_err = ETIMEDOUT;
 		sk->sk_error_report(sk);
+		cancel = 1;
 	}
 	release_sock(sk);
+	if (cancel)
+		virtio_transport_get_ops()->cancel_pkt(vsk);
 
 	sock_put(sk);
 }
@@ -1223,11 +1228,13 @@ static int vsock_stream_connect(struct socket *sock, struct sockaddr *addr,
 			err = sock_intr_errno(timeout);
 			sk->sk_state = SS_UNCONNECTED;
 			sock->state = SS_UNCONNECTED;
+			virtio_transport_get_ops()->cancel_pkt(vsk);
 			goto out_wait;
 		} else if (timeout == 0) {
 			err = -ETIMEDOUT;
 			sk->sk_state = SS_UNCONNECTED;
 			sock->state = SS_UNCONNECTED;
+			virtio_transport_get_ops()->cancel_pkt(vsk);
 			goto out_wait;
 		}
 
diff --git a/net/vmw_vsock/virtio_transport_common.c b/net/vmw_vsock/virtio_transport_common.c
index cc1eeb5..72c5dff 100644
--- a/net/vmw_vsock/virtio_transport_common.c
+++ b/net/vmw_vsock/virtio_transport_common.c
@@ -25,13 +25,6 @@
 /* How long to wait for graceful shutdown of a connection */
 #define VSOCK_CLOSE_TIMEOUT (8 * HZ)
 
-static const struct virtio_transport *virtio_transport_get_ops(void)
-{
-	const struct vsock_transport *t = vsock_core_get_transport();
-
-	return container_of(t, struct virtio_transport, transport);
-}
-
 struct virtio_vsock_pkt *
 virtio_transport_alloc_pkt(struct virtio_vsock_pkt_info *info,
 			   size_t len,
-- 
2.7.4

Re: [PATCH v2 4/4] vsock: cancel packets when failing to connect

[Stefan Hajnoczi]

[-- Attachment #1.1: Type: text/plain, Size: 4076 bytes --]

On Wed, Dec 07, 2016 at 11:14:12PM +0800, Peng Tao wrote:
> Otherwise we'll leave the packets queued until releasing vsock device.
> E.g., if guest is slow to start up, resulting ETIMEDOUT on connect, guest
> will get the connect requests from failed host sockets.
> 
> Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
> Signed-off-by: Peng Tao <bergwolf@gmail.com>
> ---
>  include/linux/virtio_vsock.h            | 7 +++++++
>  net/vmw_vsock/af_vsock.c                | 7 +++++++
>  net/vmw_vsock/virtio_transport_common.c | 7 -------
>  3 files changed, 14 insertions(+), 7 deletions(-)
> 
> diff --git a/include/linux/virtio_vsock.h b/include/linux/virtio_vsock.h
> index b92e88d..ff6850a 100644
> --- a/include/linux/virtio_vsock.h
> +++ b/include/linux/virtio_vsock.h
> @@ -156,4 +156,11 @@ void virtio_transport_inc_tx_pkt(struct virtio_vsock_sock *vvs, struct virtio_vs
>  u32 virtio_transport_get_credit(struct virtio_vsock_sock *vvs, u32 wanted);
>  void virtio_transport_put_credit(struct virtio_vsock_sock *vvs, u32 credit);
>  
> +static inline const struct virtio_transport *virtio_transport_get_ops(void)
> +{
> +	const struct vsock_transport *t = vsock_core_get_transport();
> +
> +	return container_of(t, struct virtio_transport, transport);
> +}
> +
>  #endif /* _LINUX_VIRTIO_VSOCK_H */
> diff --git a/net/vmw_vsock/af_vsock.c b/net/vmw_vsock/af_vsock.c
> index 8a398b3..ebb50d6 100644
> --- a/net/vmw_vsock/af_vsock.c
> +++ b/net/vmw_vsock/af_vsock.c
> @@ -104,6 +104,7 @@
>  #include <linux/unistd.h>
>  #include <linux/wait.h>
>  #include <linux/workqueue.h>
> +#include <linux/virtio_vsock.h>
>  #include <net/sock.h>
>  #include <net/af_vsock.h>
>  
> @@ -1105,6 +1106,7 @@ static void vsock_connect_timeout(struct work_struct *work)
>  {
>  	struct sock *sk;
>  	struct vsock_sock *vsk;
> +	int cancel = 0;
>  
>  	vsk = container_of(work, struct vsock_sock, dwork.work);
>  	sk = sk_vsock(vsk);
> @@ -1115,8 +1117,11 @@ static void vsock_connect_timeout(struct work_struct *work)
>  		sk->sk_state = SS_UNCONNECTED;
>  		sk->sk_err = ETIMEDOUT;
>  		sk->sk_error_report(sk);
> +		cancel = 1;
>  	}
>  	release_sock(sk);
> +	if (cancel)
> +		virtio_transport_get_ops()->cancel_pkt(vsk);

This doesn't work with the VMCI transport.  Remember af_vsock.c is
common code shared by all transports.

You need to add a struct vsock_transport->cancel_pkt() callback instead
os a struct virtio_transport->cancel_pkt() callback.  And you need to
handle the case where cancel_pkt == NULL if you don't implement it for
VMCI.

>  
>  	sock_put(sk);
>  }
> @@ -1223,11 +1228,13 @@ static int vsock_stream_connect(struct socket *sock, struct sockaddr *addr,
>  			err = sock_intr_errno(timeout);
>  			sk->sk_state = SS_UNCONNECTED;
>  			sock->state = SS_UNCONNECTED;
> +			virtio_transport_get_ops()->cancel_pkt(vsk);
>  			goto out_wait;
>  		} else if (timeout == 0) {
>  			err = -ETIMEDOUT;
>  			sk->sk_state = SS_UNCONNECTED;
>  			sock->state = SS_UNCONNECTED;
> +			virtio_transport_get_ops()->cancel_pkt(vsk);
>  			goto out_wait;
>  		}
>  
> diff --git a/net/vmw_vsock/virtio_transport_common.c b/net/vmw_vsock/virtio_transport_common.c
> index cc1eeb5..72c5dff 100644
> --- a/net/vmw_vsock/virtio_transport_common.c
> +++ b/net/vmw_vsock/virtio_transport_common.c
> @@ -25,13 +25,6 @@
>  /* How long to wait for graceful shutdown of a connection */
>  #define VSOCK_CLOSE_TIMEOUT (8 * HZ)
>  
> -static const struct virtio_transport *virtio_transport_get_ops(void)
> -{
> -	const struct vsock_transport *t = vsock_core_get_transport();
> -
> -	return container_of(t, struct virtio_transport, transport);
> -}
> -
>  struct virtio_vsock_pkt *
>  virtio_transport_alloc_pkt(struct virtio_vsock_pkt_info *info,
>  			   size_t len,
> -- 
> 2.7.4
> 
> --
> To unsubscribe from this list: send the line "unsubscribe kvm" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 455 bytes --]

[-- Attachment #2: Type: text/plain, Size: 183 bytes --]

_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

Re: [PATCH v2 1/4] vsock: track pkt owner vsock

[Stefan Hajnoczi]

[-- Attachment #1.1: Type: text/plain, Size: 1013 bytes --]

On Wed, Dec 07, 2016 at 11:14:09PM +0800, Peng Tao wrote:
> So that we can cancel a queued pkt later if necessary.
> 
> Signed-off-by: Peng Tao <bergwolf@gmail.com>
> ---
>  include/linux/virtio_vsock.h            | 2 ++
>  net/vmw_vsock/virtio_transport_common.c | 7 +++++++
>  2 files changed, 9 insertions(+)
> 
> diff --git a/include/linux/virtio_vsock.h b/include/linux/virtio_vsock.h
> index 9638bfe..6dd3242 100644
> --- a/include/linux/virtio_vsock.h
> +++ b/include/linux/virtio_vsock.h
> @@ -48,6 +48,7 @@ struct virtio_vsock_pkt {
>  	struct virtio_vsock_hdr	hdr;
>  	struct work_struct work;
>  	struct list_head list;
> +	struct vsock_sock *vsk;

To prevent future bugs, please add a comment here:
/* socket refcnt not held, only use for cancellation */

This field is just an opaque token used for cancellation rather than a
struct vsock_sock pointer that we are allowed to dereference.  You could
change this field to void *cancel_token to make the code harder to
misuse.

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 455 bytes --]

[-- Attachment #2: Type: text/plain, Size: 183 bytes --]

_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

Re: [PATCH v2 2/4] vhost-vsock: add pkt cancel capability

[Stefan Hajnoczi]

[-- Attachment #1.1: Type: text/plain, Size: 383 bytes --]

On Wed, Dec 07, 2016 at 11:14:10PM +0800, Peng Tao wrote:
> To allow canceling all packets of a connection.
> 
> Signed-off-by: Peng Tao <bergwolf@gmail.com>
> ---
>  drivers/vhost/vsock.c        | 41 +++++++++++++++++++++++++++++++++++++++++
>  include/linux/virtio_vsock.h |  3 +++
>  2 files changed, 44 insertions(+)

Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 455 bytes --]

[-- Attachment #2: Type: text/plain, Size: 183 bytes --]

_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

Re: [PATCH v2 3/4] vsock: add pkt cancel capability

[Stefan Hajnoczi]

[-- Attachment #1.1: Type: text/plain, Size: 281 bytes --]

On Wed, Dec 07, 2016 at 11:14:11PM +0800, Peng Tao wrote:
> Signed-off-by: Peng Tao <bergwolf@gmail.com>
> ---
>  net/vmw_vsock/virtio_transport.c | 42 ++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 42 insertions(+)

Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 455 bytes --]

[-- Attachment #2: Type: text/plain, Size: 183 bytes --]

_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

Re: [PATCH v2 4/4] vsock: cancel packets when failing to connect

[Peng Tao]

On Thu, Dec 8, 2016 at 5:24 PM, Stefan Hajnoczi <stefanha@gmail.com> wrote:
> On Wed, Dec 07, 2016 at 11:14:12PM +0800, Peng Tao wrote:
>> Otherwise we'll leave the packets queued until releasing vsock device.
>> E.g., if guest is slow to start up, resulting ETIMEDOUT on connect, guest
>> will get the connect requests from failed host sockets.
>>
>> Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
>> Signed-off-by: Peng Tao <bergwolf@gmail.com>
>> ---
>>  include/linux/virtio_vsock.h            | 7 +++++++
>>  net/vmw_vsock/af_vsock.c                | 7 +++++++
>>  net/vmw_vsock/virtio_transport_common.c | 7 -------
>>  3 files changed, 14 insertions(+), 7 deletions(-)
>>
>> diff --git a/include/linux/virtio_vsock.h b/include/linux/virtio_vsock.h
>> index b92e88d..ff6850a 100644
>> --- a/include/linux/virtio_vsock.h
>> +++ b/include/linux/virtio_vsock.h
>> @@ -156,4 +156,11 @@ void virtio_transport_inc_tx_pkt(struct virtio_vsock_sock *vvs, struct virtio_vs
>>  u32 virtio_transport_get_credit(struct virtio_vsock_sock *vvs, u32 wanted);
>>  void virtio_transport_put_credit(struct virtio_vsock_sock *vvs, u32 credit);
>>
>> +static inline const struct virtio_transport *virtio_transport_get_ops(void)
>> +{
>> +     const struct vsock_transport *t = vsock_core_get_transport();
>> +
>> +     return container_of(t, struct virtio_transport, transport);
>> +}
>> +
>>  #endif /* _LINUX_VIRTIO_VSOCK_H */
>> diff --git a/net/vmw_vsock/af_vsock.c b/net/vmw_vsock/af_vsock.c
>> index 8a398b3..ebb50d6 100644
>> --- a/net/vmw_vsock/af_vsock.c
>> +++ b/net/vmw_vsock/af_vsock.c
>> @@ -104,6 +104,7 @@
>>  #include <linux/unistd.h>
>>  #include <linux/wait.h>
>>  #include <linux/workqueue.h>
>> +#include <linux/virtio_vsock.h>
>>  #include <net/sock.h>
>>  #include <net/af_vsock.h>
>>
>> @@ -1105,6 +1106,7 @@ static void vsock_connect_timeout(struct work_struct *work)
>>  {
>>       struct sock *sk;
>>       struct vsock_sock *vsk;
>> +     int cancel = 0;
>>
>>       vsk = container_of(work, struct vsock_sock, dwork.work);
>>       sk = sk_vsock(vsk);
>> @@ -1115,8 +1117,11 @@ static void vsock_connect_timeout(struct work_struct *work)
>>               sk->sk_state = SS_UNCONNECTED;
>>               sk->sk_err = ETIMEDOUT;
>>               sk->sk_error_report(sk);
>> +             cancel = 1;
>>       }
>>       release_sock(sk);
>> +     if (cancel)
>> +             virtio_transport_get_ops()->cancel_pkt(vsk);
>
> This doesn't work with the VMCI transport.  Remember af_vsock.c is
> common code shared by all transports.
>
> You need to add a struct vsock_transport->cancel_pkt() callback instead
> os a struct virtio_transport->cancel_pkt() callback.  And you need to
> handle the case where cancel_pkt == NULL if you don't implement it for
> VMCI.
>
I see. Thanks for reviewing! I'll fix it and resend later.

Cheers,
Tao

Re: [PATCH v2 1/4] vsock: track pkt owner vsock

[Peng Tao]

On Thu, Dec 8, 2016 at 5:30 PM, Stefan Hajnoczi <stefanha@gmail.com> wrote:
> On Wed, Dec 07, 2016 at 11:14:09PM +0800, Peng Tao wrote:
>> So that we can cancel a queued pkt later if necessary.
>>
>> Signed-off-by: Peng Tao <bergwolf@gmail.com>
>> ---
>>  include/linux/virtio_vsock.h            | 2 ++
>>  net/vmw_vsock/virtio_transport_common.c | 7 +++++++
>>  2 files changed, 9 insertions(+)
>>
>> diff --git a/include/linux/virtio_vsock.h b/include/linux/virtio_vsock.h
>> index 9638bfe..6dd3242 100644
>> --- a/include/linux/virtio_vsock.h
>> +++ b/include/linux/virtio_vsock.h
>> @@ -48,6 +48,7 @@ struct virtio_vsock_pkt {
>>       struct virtio_vsock_hdr hdr;
>>       struct work_struct work;
>>       struct list_head list;
>> +     struct vsock_sock *vsk;
>
> To prevent future bugs, please add a comment here:
> /* socket refcnt not held, only use for cancellation */
>
> This field is just an opaque token used for cancellation rather than a
> struct vsock_sock pointer that we are allowed to dereference.  You could
> change this field to void *cancel_token to make the code harder to
> misuse.
Will do. Thanks!

Cheers,
Tao