[PATCH 0/4] Add multi-cid support for vsock driver

[PATCH 0/4] Add multi-cid support for vsock driver

[fuguancheng]

This patchset enables the user to specify additional CIDS for host and
guest when booting up the guest machine. The guest's additional CIDS cannot
be repeated, and can be used to communicate with the host. The user can
also choose to specify a set of additional host cids, which can be
used to communicate with the guest who specify them. The original
CID(VHOST_DEFAULT_CID) is still available for host. The guest cid field is
deleted.

To ensure that multiple guest CID maps to the same vhost_vsock struct,
a struct called vhost_vsock_ref is added.  The function of vhost_vsock_ref
is simply used to allow multiple guest CIDS map to the
same vhost_vsock struct.

If not specified, the host and guest will now use the first CID specified
in the array for connect operation. If the host or guest wants to use
one specific CID, the bind operation can be performed before the connect
operation so that the vsock_auto_bind operation can be avoided.

Hypervisors such as qemu needs to be modified to use this feature. The
required changes including at least the following:
1. Invoke the modified ioctl call with the request code
VHOST_VSOCK_SET_GUEST_CID. Also see struct multi_cid_message for
arguments used in this ioctl call.
2. Write new arguments to the emulated device config space.
3. Modify the layout of the data written to the device config space.
See struct virtio_vsock_config for reference.

I have tested this setup with iperf3.  The communication between host
and guest using original CID or additional CIDS worked normally.
Not tested in extreme conditions where memory is insufficient.

Linux kernel newbies here, any suggestions are welcomed.
Thanks in advance!

fuguancheng (4):
  VSOCK DRIVER: Add multi-cid support for guest
  VSOCK DRIVER: support communication using additional guest cid
  VSOCK DRIVER: support specifying additional cids for host
  VSOCK DRIVER: support communication using host additional cids

 drivers/vhost/vsock.c                   | 338 ++++++++++++++++++++++++++++----
 include/net/af_vsock.h                  |   5 +
 include/uapi/linux/vhost.h              |   9 +
 include/uapi/linux/virtio_vsock.h       |   8 +-
 net/vmw_vsock/af_vsock.c                |  28 ++-
 net/vmw_vsock/virtio_transport.c        | 129 +++++++++++-
 net/vmw_vsock/virtio_transport_common.c |   5 +-
 net/vmw_vsock/vsock_loopback.c          |   8 +
 8 files changed, 471 insertions(+), 59 deletions(-)

-- 
2.11.0

[PATCH 1/4] VSOCK DRIVER: Add multi-cid support for guest

[fuguancheng]

This patch allowes the user to specify multiple additional CIDS
for the guest that can be used for communication between host
and guest.

The guest reads the additional cids from the device config space.
The device config space layout can be found at uapi/linux/virtio_vsock.h
The existing ioctl call for device VHOST_VIRTIO with request code
VHOST_VSOCK_SET_GUEST_CID is modified to notify the host for the
additional guest CIDS.

Signed-off-by: fuguancheng <fuguancheng@bytedance.com>
---
 drivers/vhost/vhost.h             |   5 ++
 drivers/vhost/vsock.c             | 173 +++++++++++++++++++++++++++++---------
 include/net/af_vsock.h            |   1 +
 include/uapi/linux/vhost.h        |   7 ++
 include/uapi/linux/virtio_vsock.h |   3 +-
 net/vmw_vsock/af_vsock.c          |   6 +-
 net/vmw_vsock/virtio_transport.c  |  72 ++++++++++++++--
 net/vmw_vsock/vsock_loopback.c    |   8 ++
 8 files changed, 222 insertions(+), 53 deletions(-)

diff --git a/drivers/vhost/vhost.h b/drivers/vhost/vhost.h
index 638bb640d6b4..52bd143ccf0c 100644
--- a/drivers/vhost/vhost.h
+++ b/drivers/vhost/vhost.h
@@ -25,6 +25,11 @@ struct vhost_work {
 	unsigned long		flags;
 };
 
+struct multi_cid_message {
+	u32 number_cid;
+	u64 *cid;
+};
+
 /* Poll a file (eventfd or socket) */
 /* Note: there's nothing vhost specific about this structure. */
 struct vhost_poll {
diff --git a/drivers/vhost/vsock.c b/drivers/vhost/vsock.c
index f249622ef11b..f66c87de91b8 100644
--- a/drivers/vhost/vsock.c
+++ b/drivers/vhost/vsock.c
@@ -43,12 +43,25 @@ enum {
 static DEFINE_MUTEX(vhost_vsock_mutex);
 static DEFINE_READ_MOSTLY_HASHTABLE(vhost_vsock_hash, 8);
 
+struct vhost_vsock_ref {
+	struct vhost_vsock *vsock;
+	struct hlist_node ref_hash;
+	u32 cid;
+};
+
+static bool vhost_transport_contain_cid(u32 cid)
+{
+	if (cid == VHOST_VSOCK_DEFAULT_HOST_CID)
+		return true;
+	return false;
+}
+
 struct vhost_vsock {
 	struct vhost_dev dev;
 	struct vhost_virtqueue vqs[2];
 
 	/* Link to global vhost_vsock_hash, writes use vhost_vsock_mutex */
-	struct hlist_node hash;
+	struct vhost_vsock_ref *ref_list;
 
 	struct vhost_work send_pkt_work;
 	spinlock_t send_pkt_list_lock;
@@ -56,7 +69,8 @@ struct vhost_vsock {
 
 	atomic_t queued_replies;
 
-	u32 guest_cid;
+	u32 *cids;
+	u32 num_cid;
 	bool seqpacket_allow;
 };
 
@@ -70,23 +84,49 @@ static u32 vhost_transport_get_local_cid(void)
  */
 static struct vhost_vsock *vhost_vsock_get(u32 guest_cid)
 {
-	struct vhost_vsock *vsock;
+	struct vhost_vsock_ref *ref;
 
-	hash_for_each_possible_rcu(vhost_vsock_hash, vsock, hash, guest_cid) {
-		u32 other_cid = vsock->guest_cid;
+	hash_for_each_possible_rcu(vhost_vsock_hash, ref, ref_hash, guest_cid) {
+		u32 other_cid = ref->cid;
 
 		/* Skip instances that have no CID yet */
 		if (other_cid == 0)
 			continue;
 
 		if (other_cid == guest_cid)
-			return vsock;
+			return ref->vsock;
 
 	}
 
 	return NULL;
 }
 
+static int check_if_cid_valid(u64 guest_cid, struct vhost_vsock *vsock)
+{
+	struct vhost_vsock *other;
+
+	if (guest_cid <= VMADDR_CID_HOST || guest_cid == U32_MAX)
+		return -EINVAL;
+
+	/* 64-bit CIDs are not yet supported */
+	if (guest_cid > U32_MAX)
+		return -EINVAL;
+	/* Refuse if CID is assigned to the guest->host transport (i.e. nested
+	 * VM), to make the loopback work.
+	 */
+	if (vsock_find_cid(guest_cid))
+		return -EADDRINUSE;
+	/* Refuse if CID is already in use */
+	mutex_lock(&vhost_vsock_mutex);
+	other = vhost_vsock_get(guest_cid);
+	if (other) {
+		mutex_unlock(&vhost_vsock_mutex);
+		return -EADDRINUSE;
+	}
+	mutex_unlock(&vhost_vsock_mutex);
+	return 0;
+}
+
 static void
 vhost_transport_do_send_pkt(struct vhost_vsock *vsock,
 			    struct vhost_virtqueue *vq)
@@ -427,6 +467,7 @@ static struct virtio_transport vhost_transport = {
 		.module                   = THIS_MODULE,
 
 		.get_local_cid            = vhost_transport_get_local_cid,
+		.contain_cid              = vhost_transport_contain_cid,
 
 		.init                     = virtio_transport_do_socket_init,
 		.destruct                 = virtio_transport_destruct,
@@ -542,9 +583,9 @@ static void vhost_vsock_handle_tx_kick(struct vhost_work *work)
 		virtio_transport_deliver_tap_pkt(pkt);
 
 		/* Only accept correctly addressed packets */
-		if (le64_to_cpu(pkt->hdr.src_cid) == vsock->guest_cid &&
-		    le64_to_cpu(pkt->hdr.dst_cid) ==
-		    vhost_transport_get_local_cid())
+		if (vsock->num_cid > 0 &&
+		    (pkt->hdr.src_cid) == vsock->cids[0] &&
+		    le64_to_cpu(pkt->hdr.dst_cid) == vhost_transport_get_local_cid())
 			virtio_transport_recv_pkt(&vhost_transport, pkt);
 		else
 			virtio_transport_free_pkt(pkt);
@@ -655,6 +696,10 @@ static int vhost_vsock_stop(struct vhost_vsock *vsock)
 
 static void vhost_vsock_free(struct vhost_vsock *vsock)
 {
+	if (vsock->ref_list)
+		kvfree(vsock->ref_list);
+	if (vsock->cids)
+		kvfree(vsock->cids);
 	kvfree(vsock);
 }
 
@@ -677,7 +722,9 @@ static int vhost_vsock_dev_open(struct inode *inode, struct file *file)
 		goto out;
 	}
 
-	vsock->guest_cid = 0; /* no CID assigned yet */
+	vsock->ref_list = NULL;
+	vsock->cids = NULL;
+	vsock->num_cid = 0;
 
 	atomic_set(&vsock->queued_replies, 0);
 
@@ -739,11 +786,14 @@ static void vhost_vsock_reset_orphans(struct sock *sk)
 
 static int vhost_vsock_dev_release(struct inode *inode, struct file *file)
 {
+	int index;
 	struct vhost_vsock *vsock = file->private_data;
 
 	mutex_lock(&vhost_vsock_mutex);
-	if (vsock->guest_cid)
-		hash_del_rcu(&vsock->hash);
+	if (vsock->num_cid) {
+		for (index = 0; index < vsock->num_cid; index++)
+			hash_del_rcu(&vsock->ref_list[index].ref_hash);
+	}
 	mutex_unlock(&vhost_vsock_mutex);
 
 	/* Wait for other CPUs to finish using vsock */
@@ -774,41 +824,80 @@ static int vhost_vsock_dev_release(struct inode *inode, struct file *file)
 	return 0;
 }
 
-static int vhost_vsock_set_cid(struct vhost_vsock *vsock, u64 guest_cid)
+static int vhost_vsock_set_cid(struct vhost_vsock *vsock, u64 __user *cids, u32 number_cid)
 {
-	struct vhost_vsock *other;
+	u64 cid;
+	int i, ret;
 
-	/* Refuse reserved CIDs */
-	if (guest_cid <= VMADDR_CID_HOST ||
-	    guest_cid == U32_MAX)
+	if (number_cid <= 0)
 		return -EINVAL;
-
-	/* 64-bit CIDs are not yet supported */
-	if (guest_cid > U32_MAX)
-		return -EINVAL;
-
-	/* Refuse if CID is assigned to the guest->host transport (i.e. nested
-	 * VM), to make the loopback work.
-	 */
-	if (vsock_find_cid(guest_cid))
-		return -EADDRINUSE;
-
-	/* Refuse if CID is already in use */
-	mutex_lock(&vhost_vsock_mutex);
-	other = vhost_vsock_get(guest_cid);
-	if (other && other != vsock) {
+	/* delete the old CIDs. */
+	if (vsock->num_cid) {
+		mutex_lock(&vhost_vsock_mutex);
+		for (i = 0; i < vsock->num_cid; i++)
+			hash_del_rcu(&vsock->ref_list[i].ref_hash);
 		mutex_unlock(&vhost_vsock_mutex);
-		return -EADDRINUSE;
+		kvfree(vsock->ref_list);
+		vsock->ref_list = NULL;
+		kvfree(vsock->cids);
+		vsock->cids = NULL;
+	}
+	vsock->num_cid = number_cid;
+	vsock->cids = kmalloc_array(vsock->num_cid, sizeof(u32),
+				    GFP_KERNEL | __GFP_RETRY_MAYFAIL);
+	if (!vsock->cids) {
+		vsock->num_cid = 0;
+		ret = -ENOMEM;
+		goto out;
+	}
+	vsock->ref_list = kvmalloc_array(vsock->num_cid, sizeof(*vsock->ref_list),
+			       GFP_KERNEL | __GFP_RETRY_MAYFAIL);
+	if (!vsock->ref_list) {
+		vsock->num_cid = 0;
+		ret = -ENOMEM;
+		goto out;
 	}
 
-	if (vsock->guest_cid)
-		hash_del_rcu(&vsock->hash);
-
-	vsock->guest_cid = guest_cid;
-	hash_add_rcu(vhost_vsock_hash, &vsock->hash, vsock->guest_cid);
-	mutex_unlock(&vhost_vsock_mutex);
+	for (i = 0; i < number_cid; i++) {
+		if (copy_from_user(&cid, cids + i, sizeof(cid))) {
+			/* record where we failed, to clean up the ref in hash table. */
+			vsock->num_cid = i;
+			ret = -EFAULT;
+			goto out;
+		}
+		ret = check_if_cid_valid(cid, vsock);
+		if (ret) {
+			vsock->num_cid = i;
+			goto out;
+		}
 
+		vsock->cids[i] = (u32)cid;
+		vsock->ref_list[i].cid = vsock->cids[i];
+		vsock->ref_list[i].vsock = vsock;
+		mutex_lock(&vhost_vsock_mutex);
+		hash_add_rcu(vhost_vsock_hash, &vsock->ref_list[i].ref_hash,
+			     vsock->cids[i]);
+		mutex_unlock(&vhost_vsock_mutex);
+	}
 	return 0;
+
+out:
+	/* Handle the memory release here. */
+	if (vsock->num_cid) {
+		mutex_lock(&vhost_vsock_mutex);
+		for (i = 0; i < vsock->num_cid; i++)
+			hash_del_rcu(&vsock->ref_list[i].ref_hash);
+		mutex_unlock(&vhost_vsock_mutex);
+		vsock->num_cid = 0;
+	}
+	if (vsock->ref_list)
+		kvfree(vsock->ref_list);
+	if (vsock->cids)
+		kvfree(vsock->cids);
+	/* Set it to null to prevent double release. */
+	vsock->ref_list = NULL;
+	vsock->cids = NULL;
+	return ret;
 }
 
 static int vhost_vsock_set_features(struct vhost_vsock *vsock, u64 features)
@@ -852,16 +941,16 @@ static long vhost_vsock_dev_ioctl(struct file *f, unsigned int ioctl,
 {
 	struct vhost_vsock *vsock = f->private_data;
 	void __user *argp = (void __user *)arg;
-	u64 guest_cid;
 	u64 features;
 	int start;
 	int r;
+	struct multi_cid_message cid_message;
 
 	switch (ioctl) {
 	case VHOST_VSOCK_SET_GUEST_CID:
-		if (copy_from_user(&guest_cid, argp, sizeof(guest_cid)))
+		if (copy_from_user(&cid_message, argp, sizeof(cid_message)))
 			return -EFAULT;
-		return vhost_vsock_set_cid(vsock, guest_cid);
+		return vhost_vsock_set_cid(vsock, cid_message.cid, cid_message.number_cid);
 	case VHOST_VSOCK_SET_RUNNING:
 		if (copy_from_user(&start, argp, sizeof(start)))
 			return -EFAULT;
diff --git a/include/net/af_vsock.h b/include/net/af_vsock.h
index ab207677e0a8..d0fc08fb9cac 100644
--- a/include/net/af_vsock.h
+++ b/include/net/af_vsock.h
@@ -170,6 +170,7 @@ struct vsock_transport {
 
 	/* Addressing. */
 	u32 (*get_local_cid)(void);
+	bool (*contain_cid)(u32 cid);
 };
 
 /**** CORE ****/
diff --git a/include/uapi/linux/vhost.h b/include/uapi/linux/vhost.h
index c998860d7bbc..a3ea99f6fc7f 100644
--- a/include/uapi/linux/vhost.h
+++ b/include/uapi/linux/vhost.h
@@ -17,6 +17,13 @@
 
 #define VHOST_FILE_UNBIND -1
 
+/* structs used for hypervisors to send cid info. */
+
+struct multi_cid_message {
+	u32 number_cid;
+	u64 *cid;
+};
+
 /* ioctls */
 
 #define VHOST_VIRTIO 0xAF
diff --git a/include/uapi/linux/virtio_vsock.h b/include/uapi/linux/virtio_vsock.h
index 3dd3555b2740..0afc14446b01 100644
--- a/include/uapi/linux/virtio_vsock.h
+++ b/include/uapi/linux/virtio_vsock.h
@@ -42,7 +42,8 @@
 #define VIRTIO_VSOCK_F_SEQPACKET	1	/* SOCK_SEQPACKET supported */
 
 struct virtio_vsock_config {
-	__le64 guest_cid;
+	__le32 number_cid;
+	__le64 cids[];
 } __attribute__((packed));
 
 enum virtio_vsock_event_id {
diff --git a/net/vmw_vsock/af_vsock.c b/net/vmw_vsock/af_vsock.c
index 3e02cc3b24f8..4e1fbe74013f 100644
--- a/net/vmw_vsock/af_vsock.c
+++ b/net/vmw_vsock/af_vsock.c
@@ -507,13 +507,13 @@ EXPORT_SYMBOL_GPL(vsock_assign_transport);
 
 bool vsock_find_cid(unsigned int cid)
 {
-	if (transport_g2h && cid == transport_g2h->get_local_cid())
+	if (transport_g2h && transport_g2h->contain_cid(cid))
 		return true;
 
-	if (transport_h2g && cid == VMADDR_CID_HOST)
+	if (transport_h2g && transport_h2g->contain_cid(cid))
 		return true;
 
-	if (transport_local && cid == VMADDR_CID_LOCAL)
+	if (transport_local && transport_local->contain_cid(cid))
 		return true;
 
 	return false;
diff --git a/net/vmw_vsock/virtio_transport.c b/net/vmw_vsock/virtio_transport.c
index e0c2c992ad9c..5f256a57d9ae 100644
--- a/net/vmw_vsock/virtio_transport.c
+++ b/net/vmw_vsock/virtio_transport.c
@@ -61,10 +61,41 @@ struct virtio_vsock {
 	bool event_run;
 	struct virtio_vsock_event event_list[8];
 
-	u32 guest_cid;
+	/* The following fields are used to hold additional cids given by the hypervisor
+	 * such as qemu.
+	 */
+	u32 number_cid;
+	u32 *cids;
+
 	bool seqpacket_allow;
 };
 
+static bool virtio_transport_contain_cid(u32 cid)
+{
+	struct virtio_vsock *vsock;
+	bool ret;
+	u32 num_cid;
+
+	num_cid = 0;
+	rcu_read_lock();
+	vsock = rcu_dereference(the_virtio_vsock);
+	if (!vsock || !vsock->number_cid) {
+		ret = false;
+		goto out_rcu;
+	}
+
+	for (num_cid = 0; num_cid < vsock->number_cid; num_cid++) {
+		if (vsock->cids[num_cid] == cid) {
+			ret = true;
+			goto out_rcu;
+		}
+	}
+	ret = false;
+out_rcu:
+	rcu_read_unlock();
+	return ret;
+}
+
 static u32 virtio_transport_get_local_cid(void)
 {
 	struct virtio_vsock *vsock;
@@ -72,12 +103,12 @@ static u32 virtio_transport_get_local_cid(void)
 
 	rcu_read_lock();
 	vsock = rcu_dereference(the_virtio_vsock);
-	if (!vsock) {
+	if (!vsock || !vsock->number_cid) {
 		ret = VMADDR_CID_ANY;
 		goto out_rcu;
 	}
 
-	ret = vsock->guest_cid;
+	ret = vsock->cids[0];
 out_rcu:
 	rcu_read_unlock();
 	return ret;
@@ -176,7 +207,7 @@ virtio_transport_send_pkt(struct virtio_vsock_pkt *pkt)
 		goto out_rcu;
 	}
 
-	if (le64_to_cpu(pkt->hdr.dst_cid) == vsock->guest_cid) {
+	if (le64_to_cpu(pkt->hdr.dst_cid) == vsock->cids[0]) {
 		virtio_transport_free_pkt(pkt);
 		len = -ENODEV;
 		goto out_rcu;
@@ -368,10 +399,33 @@ static void virtio_vsock_update_guest_cid(struct virtio_vsock *vsock)
 {
 	struct virtio_device *vdev = vsock->vdev;
 	__le64 guest_cid;
+	__le32 number_cid;
+	u32 index;
 
-	vdev->config->get(vdev, offsetof(struct virtio_vsock_config, guest_cid),
-			  &guest_cid, sizeof(guest_cid));
-	vsock->guest_cid = le64_to_cpu(guest_cid);
+	vdev->config->get(vdev, offsetof(struct virtio_vsock_config, number_cid),
+			  &number_cid, sizeof(number_cid));
+	vsock->number_cid = le32_to_cpu(number_cid);
+
+	/* number_cid must be greater than 0 in the config space
+	 * to use this feature.
+	 */
+	if (vsock->number_cid > 0) {
+		vsock->cids = kmalloc_array(vsock->number_cid, sizeof(u32), GFP_KERNEL);
+		if (!vsock->cids) {
+			/* Space allocated failed, reset number_cid to 0.
+			 * only use the original guest_cid.
+			 */
+			vsock->number_cid = 0;
+		}
+	}
+
+	for (index = 0; index < vsock->number_cid; index++) {
+		vdev->config->get(vdev,
+				  offsetof(struct virtio_vsock_config, cids)
+				  + index * sizeof(uint64_t),
+				  &guest_cid, sizeof(guest_cid));
+		vsock->cids[index] = le64_to_cpu(guest_cid);
+	}
 }
 
 /* event_lock must be held */
@@ -451,6 +505,7 @@ static struct virtio_transport virtio_transport = {
 		.module                   = THIS_MODULE,
 
 		.get_local_cid            = virtio_transport_get_local_cid,
+		.contain_cid              = virtio_transport_contain_cid,
 
 		.init                     = virtio_transport_do_socket_init,
 		.destruct                 = virtio_transport_destruct,
@@ -594,6 +649,8 @@ static int virtio_vsock_probe(struct virtio_device *vdev)
 	}
 
 	vsock->vdev = vdev;
+	vsock->cids = NULL;
+	vsock->number_cid = 0;
 
 	ret = virtio_find_vqs(vsock->vdev, VSOCK_VQ_MAX,
 			      vsock->vqs, callbacks, names,
@@ -713,6 +770,7 @@ static void virtio_vsock_remove(struct virtio_device *vdev)
 
 	mutex_unlock(&the_virtio_vsock_mutex);
 
+	kfree(vsock->cids);
 	kfree(vsock);
 }
 
diff --git a/net/vmw_vsock/vsock_loopback.c b/net/vmw_vsock/vsock_loopback.c
index 169a8cf65b39..3abbbaff34eb 100644
--- a/net/vmw_vsock/vsock_loopback.c
+++ b/net/vmw_vsock/vsock_loopback.c
@@ -63,6 +63,13 @@ static int vsock_loopback_cancel_pkt(struct vsock_sock *vsk)
 	return 0;
 }
 
+static bool vsock_loopback_contain_cid(u32 cid)
+{
+	if (cid == VMADDR_CID_LOCAL)
+		return true;
+	return false;
+}
+
 static bool vsock_loopback_seqpacket_allow(u32 remote_cid);
 
 static struct virtio_transport loopback_transport = {
@@ -70,6 +77,7 @@ static struct virtio_transport loopback_transport = {
 		.module                   = THIS_MODULE,
 
 		.get_local_cid            = vsock_loopback_get_local_cid,
+		.contain_cid              = vsock_loopback_contain_cid,
 
 		.init                     = virtio_transport_do_socket_init,
 		.destruct                 = virtio_transport_destruct,
-- 
2.11.0

Re: [PATCH 1/4] VSOCK DRIVER: Add multi-cid support for guest

[Michael S. Tsirkin]

On Mon, Aug 02, 2021 at 08:07:17PM +0800, fuguancheng wrote:
> diff --git a/include/uapi/linux/vhost.h b/include/uapi/linux/vhost.h
> index c998860d7bbc..a3ea99f6fc7f 100644
> --- a/include/uapi/linux/vhost.h
> +++ b/include/uapi/linux/vhost.h
> @@ -17,6 +17,13 @@
>  
>  #define VHOST_FILE_UNBIND -1
>  
> +/* structs used for hypervisors to send cid info. */
> +
> +struct multi_cid_message {
> +	u32 number_cid;
> +	u64 *cid;
> +};
> +
>  /* ioctls */
>  
>  #define VHOST_VIRTIO 0xAF


In this case, a kernel pointer in a UAPI struct is suspicious.
So is padding after number_cid.

-- 
MST

Re: [PATCH 1/4] VSOCK DRIVER: Add multi-cid support for guest

[Michael S. Tsirkin]

On Mon, Aug 02, 2021 at 08:07:17PM +0800, fuguancheng wrote:
> diff --git a/include/uapi/linux/virtio_vsock.h b/include/uapi/linux/virtio_vsock.h
> index 3dd3555b2740..0afc14446b01 100644
> --- a/include/uapi/linux/virtio_vsock.h
> +++ b/include/uapi/linux/virtio_vsock.h
> @@ -42,7 +42,8 @@
>  #define VIRTIO_VSOCK_F_SEQPACKET	1	/* SOCK_SEQPACKET supported */
>  
>  struct virtio_vsock_config {
> -	__le64 guest_cid;
> +	__le32 number_cid;
> +	__le64 cids[];
>  } __attribute__((packed));

any host/guest interface change needs to copy the virtio TC.
packing here is a bad idea imho, just add explicit padding.

-- 
MST

Re: [PATCH 1/4] VSOCK DRIVER: Add multi-cid support for guest

[Michael S. Tsirkin]

On Mon, Aug 02, 2021 at 08:07:17PM +0800, fuguancheng wrote:
> This patch allowes the user to specify multiple additional CIDS
> for the guest that can be used for communication between host
> and guest.
> 
> The guest reads the additional cids from the device config space.
> The device config space layout can be found at uapi/linux/virtio_vsock.h
> The existing ioctl call for device VHOST_VIRTIO with request code
> VHOST_VSOCK_SET_GUEST_CID is modified to notify the host for the
> additional guest CIDS.
> 
> Signed-off-by: fuguancheng <fuguancheng@bytedance.com>
> ---
>  drivers/vhost/vhost.h             |   5 ++
>  drivers/vhost/vsock.c             | 173 +++++++++++++++++++++++++++++---------
>  include/net/af_vsock.h            |   1 +
>  include/uapi/linux/vhost.h        |   7 ++
>  include/uapi/linux/virtio_vsock.h |   3 +-
>  net/vmw_vsock/af_vsock.c          |   6 +-
>  net/vmw_vsock/virtio_transport.c  |  72 ++++++++++++++--
>  net/vmw_vsock/vsock_loopback.c    |   8 ++
>  8 files changed, 222 insertions(+), 53 deletions(-)
> 
> diff --git a/drivers/vhost/vhost.h b/drivers/vhost/vhost.h
> index 638bb640d6b4..52bd143ccf0c 100644
> --- a/drivers/vhost/vhost.h
> +++ b/drivers/vhost/vhost.h
> @@ -25,6 +25,11 @@ struct vhost_work {
>  	unsigned long		flags;
>  };
>  
> +struct multi_cid_message {
> +	u32 number_cid;
> +	u64 *cid;
> +};
> +
>  /* Poll a file (eventfd or socket) */
>  /* Note: there's nothing vhost specific about this structure. */
>  struct vhost_poll {
> diff --git a/drivers/vhost/vsock.c b/drivers/vhost/vsock.c
> index f249622ef11b..f66c87de91b8 100644
> --- a/drivers/vhost/vsock.c
> +++ b/drivers/vhost/vsock.c
> @@ -43,12 +43,25 @@ enum {
>  static DEFINE_MUTEX(vhost_vsock_mutex);
>  static DEFINE_READ_MOSTLY_HASHTABLE(vhost_vsock_hash, 8);
>  
> +struct vhost_vsock_ref {
> +	struct vhost_vsock *vsock;
> +	struct hlist_node ref_hash;
> +	u32 cid;
> +};
> +
> +static bool vhost_transport_contain_cid(u32 cid)
> +{
> +	if (cid == VHOST_VSOCK_DEFAULT_HOST_CID)
> +		return true;
> +	return false;
> +}
> +
>  struct vhost_vsock {
>  	struct vhost_dev dev;
>  	struct vhost_virtqueue vqs[2];
>  
>  	/* Link to global vhost_vsock_hash, writes use vhost_vsock_mutex */
> -	struct hlist_node hash;
> +	struct vhost_vsock_ref *ref_list;
>  
>  	struct vhost_work send_pkt_work;
>  	spinlock_t send_pkt_list_lock;
> @@ -56,7 +69,8 @@ struct vhost_vsock {
>  
>  	atomic_t queued_replies;
>  
> -	u32 guest_cid;
> +	u32 *cids;
> +	u32 num_cid;
>  	bool seqpacket_allow;
>  };
>  
> @@ -70,23 +84,49 @@ static u32 vhost_transport_get_local_cid(void)
>   */
>  static struct vhost_vsock *vhost_vsock_get(u32 guest_cid)
>  {
> -	struct vhost_vsock *vsock;
> +	struct vhost_vsock_ref *ref;
>  
> -	hash_for_each_possible_rcu(vhost_vsock_hash, vsock, hash, guest_cid) {
> -		u32 other_cid = vsock->guest_cid;
> +	hash_for_each_possible_rcu(vhost_vsock_hash, ref, ref_hash, guest_cid) {
> +		u32 other_cid = ref->cid;
>  
>  		/* Skip instances that have no CID yet */
>  		if (other_cid == 0)
>  			continue;
>  
>  		if (other_cid == guest_cid)
> -			return vsock;
> +			return ref->vsock;
>  
>  	}
>  
>  	return NULL;
>  }
>  
> +static int check_if_cid_valid(u64 guest_cid, struct vhost_vsock *vsock)
> +{
> +	struct vhost_vsock *other;
> +
> +	if (guest_cid <= VMADDR_CID_HOST || guest_cid == U32_MAX)
> +		return -EINVAL;
> +
> +	/* 64-bit CIDs are not yet supported */
> +	if (guest_cid > U32_MAX)
> +		return -EINVAL;
> +	/* Refuse if CID is assigned to the guest->host transport (i.e. nested
> +	 * VM), to make the loopback work.
> +	 */
> +	if (vsock_find_cid(guest_cid))
> +		return -EADDRINUSE;
> +	/* Refuse if CID is already in use */
> +	mutex_lock(&vhost_vsock_mutex);
> +	other = vhost_vsock_get(guest_cid);
> +	if (other) {
> +		mutex_unlock(&vhost_vsock_mutex);
> +		return -EADDRINUSE;
> +	}
> +	mutex_unlock(&vhost_vsock_mutex);
> +	return 0;
> +}
> +
>  static void
>  vhost_transport_do_send_pkt(struct vhost_vsock *vsock,
>  			    struct vhost_virtqueue *vq)
> @@ -427,6 +467,7 @@ static struct virtio_transport vhost_transport = {
>  		.module                   = THIS_MODULE,
>  
>  		.get_local_cid            = vhost_transport_get_local_cid,
> +		.contain_cid              = vhost_transport_contain_cid,
>  
>  		.init                     = virtio_transport_do_socket_init,
>  		.destruct                 = virtio_transport_destruct,
> @@ -542,9 +583,9 @@ static void vhost_vsock_handle_tx_kick(struct vhost_work *work)
>  		virtio_transport_deliver_tap_pkt(pkt);
>  
>  		/* Only accept correctly addressed packets */
> -		if (le64_to_cpu(pkt->hdr.src_cid) == vsock->guest_cid &&
> -		    le64_to_cpu(pkt->hdr.dst_cid) ==
> -		    vhost_transport_get_local_cid())
> +		if (vsock->num_cid > 0 &&
> +		    (pkt->hdr.src_cid) == vsock->cids[0] &&
> +		    le64_to_cpu(pkt->hdr.dst_cid) == vhost_transport_get_local_cid())
>  			virtio_transport_recv_pkt(&vhost_transport, pkt);
>  		else
>  			virtio_transport_free_pkt(pkt);
> @@ -655,6 +696,10 @@ static int vhost_vsock_stop(struct vhost_vsock *vsock)
>  
>  static void vhost_vsock_free(struct vhost_vsock *vsock)
>  {
> +	if (vsock->ref_list)
> +		kvfree(vsock->ref_list);
> +	if (vsock->cids)
> +		kvfree(vsock->cids);
>  	kvfree(vsock);
>  }
>  
> @@ -677,7 +722,9 @@ static int vhost_vsock_dev_open(struct inode *inode, struct file *file)
>  		goto out;
>  	}
>  
> -	vsock->guest_cid = 0; /* no CID assigned yet */
> +	vsock->ref_list = NULL;
> +	vsock->cids = NULL;
> +	vsock->num_cid = 0;
>  
>  	atomic_set(&vsock->queued_replies, 0);
>  
> @@ -739,11 +786,14 @@ static void vhost_vsock_reset_orphans(struct sock *sk)
>  
>  static int vhost_vsock_dev_release(struct inode *inode, struct file *file)
>  {
> +	int index;
>  	struct vhost_vsock *vsock = file->private_data;
>  
>  	mutex_lock(&vhost_vsock_mutex);
> -	if (vsock->guest_cid)
> -		hash_del_rcu(&vsock->hash);
> +	if (vsock->num_cid) {
> +		for (index = 0; index < vsock->num_cid; index++)
> +			hash_del_rcu(&vsock->ref_list[index].ref_hash);
> +	}
>  	mutex_unlock(&vhost_vsock_mutex);
>  
>  	/* Wait for other CPUs to finish using vsock */
> @@ -774,41 +824,80 @@ static int vhost_vsock_dev_release(struct inode *inode, struct file *file)
>  	return 0;
>  }
>  
> -static int vhost_vsock_set_cid(struct vhost_vsock *vsock, u64 guest_cid)
> +static int vhost_vsock_set_cid(struct vhost_vsock *vsock, u64 __user *cids, u32 number_cid)
>  {
> -	struct vhost_vsock *other;
> +	u64 cid;
> +	int i, ret;
>  
> -	/* Refuse reserved CIDs */
> -	if (guest_cid <= VMADDR_CID_HOST ||
> -	    guest_cid == U32_MAX)
> +	if (number_cid <= 0)
>  		return -EINVAL;
> -
> -	/* 64-bit CIDs are not yet supported */
> -	if (guest_cid > U32_MAX)
> -		return -EINVAL;
> -
> -	/* Refuse if CID is assigned to the guest->host transport (i.e. nested
> -	 * VM), to make the loopback work.
> -	 */
> -	if (vsock_find_cid(guest_cid))
> -		return -EADDRINUSE;
> -
> -	/* Refuse if CID is already in use */
> -	mutex_lock(&vhost_vsock_mutex);
> -	other = vhost_vsock_get(guest_cid);
> -	if (other && other != vsock) {
> +	/* delete the old CIDs. */
> +	if (vsock->num_cid) {
> +		mutex_lock(&vhost_vsock_mutex);
> +		for (i = 0; i < vsock->num_cid; i++)
> +			hash_del_rcu(&vsock->ref_list[i].ref_hash);
>  		mutex_unlock(&vhost_vsock_mutex);
> -		return -EADDRINUSE;
> +		kvfree(vsock->ref_list);
> +		vsock->ref_list = NULL;
> +		kvfree(vsock->cids);
> +		vsock->cids = NULL;
> +	}
> +	vsock->num_cid = number_cid;
> +	vsock->cids = kmalloc_array(vsock->num_cid, sizeof(u32),
> +				    GFP_KERNEL | __GFP_RETRY_MAYFAIL);
> +	if (!vsock->cids) {
> +		vsock->num_cid = 0;
> +		ret = -ENOMEM;
> +		goto out;
> +	}
> +	vsock->ref_list = kvmalloc_array(vsock->num_cid, sizeof(*vsock->ref_list),
> +			       GFP_KERNEL | __GFP_RETRY_MAYFAIL);
> +	if (!vsock->ref_list) {
> +		vsock->num_cid = 0;
> +		ret = -ENOMEM;
> +		goto out;
>  	}
>  
> -	if (vsock->guest_cid)
> -		hash_del_rcu(&vsock->hash);
> -
> -	vsock->guest_cid = guest_cid;
> -	hash_add_rcu(vhost_vsock_hash, &vsock->hash, vsock->guest_cid);
> -	mutex_unlock(&vhost_vsock_mutex);
> +	for (i = 0; i < number_cid; i++) {
> +		if (copy_from_user(&cid, cids + i, sizeof(cid))) {
> +			/* record where we failed, to clean up the ref in hash table. */
> +			vsock->num_cid = i;
> +			ret = -EFAULT;
> +			goto out;
> +		}
> +		ret = check_if_cid_valid(cid, vsock);
> +		if (ret) {
> +			vsock->num_cid = i;
> +			goto out;
> +		}
>  
> +		vsock->cids[i] = (u32)cid;
> +		vsock->ref_list[i].cid = vsock->cids[i];
> +		vsock->ref_list[i].vsock = vsock;
> +		mutex_lock(&vhost_vsock_mutex);
> +		hash_add_rcu(vhost_vsock_hash, &vsock->ref_list[i].ref_hash,
> +			     vsock->cids[i]);
> +		mutex_unlock(&vhost_vsock_mutex);
> +	}
>  	return 0;
> +
> +out:
> +	/* Handle the memory release here. */
> +	if (vsock->num_cid) {
> +		mutex_lock(&vhost_vsock_mutex);
> +		for (i = 0; i < vsock->num_cid; i++)
> +			hash_del_rcu(&vsock->ref_list[i].ref_hash);
> +		mutex_unlock(&vhost_vsock_mutex);
> +		vsock->num_cid = 0;
> +	}
> +	if (vsock->ref_list)
> +		kvfree(vsock->ref_list);
> +	if (vsock->cids)
> +		kvfree(vsock->cids);
> +	/* Set it to null to prevent double release. */
> +	vsock->ref_list = NULL;
> +	vsock->cids = NULL;
> +	return ret;
>  }
>  
>  static int vhost_vsock_set_features(struct vhost_vsock *vsock, u64 features)
> @@ -852,16 +941,16 @@ static long vhost_vsock_dev_ioctl(struct file *f, unsigned int ioctl,
>  {
>  	struct vhost_vsock *vsock = f->private_data;
>  	void __user *argp = (void __user *)arg;
> -	u64 guest_cid;
>  	u64 features;
>  	int start;
>  	int r;
> +	struct multi_cid_message cid_message;
>  
>  	switch (ioctl) {
>  	case VHOST_VSOCK_SET_GUEST_CID:
> -		if (copy_from_user(&guest_cid, argp, sizeof(guest_cid)))
> +		if (copy_from_user(&cid_message, argp, sizeof(cid_message)))
>  			return -EFAULT;
> -		return vhost_vsock_set_cid(vsock, guest_cid);
> +		return vhost_vsock_set_cid(vsock, cid_message.cid, cid_message.number_cid);
>  	case VHOST_VSOCK_SET_RUNNING:
>  		if (copy_from_user(&start, argp, sizeof(start)))
>  			return -EFAULT;
> diff --git a/include/net/af_vsock.h b/include/net/af_vsock.h
> index ab207677e0a8..d0fc08fb9cac 100644
> --- a/include/net/af_vsock.h
> +++ b/include/net/af_vsock.h
> @@ -170,6 +170,7 @@ struct vsock_transport {
>  
>  	/* Addressing. */
>  	u32 (*get_local_cid)(void);
> +	bool (*contain_cid)(u32 cid);
>  };
>  
>  /**** CORE ****/
> diff --git a/include/uapi/linux/vhost.h b/include/uapi/linux/vhost.h
> index c998860d7bbc..a3ea99f6fc7f 100644
> --- a/include/uapi/linux/vhost.h
> +++ b/include/uapi/linux/vhost.h
> @@ -17,6 +17,13 @@
>  
>  #define VHOST_FILE_UNBIND -1
>  
> +/* structs used for hypervisors to send cid info. */
> +
> +struct multi_cid_message {
> +	u32 number_cid;
> +	u64 *cid;
> +};
> +
>  /* ioctls */
>  
>  #define VHOST_VIRTIO 0xAF
> diff --git a/include/uapi/linux/virtio_vsock.h b/include/uapi/linux/virtio_vsock.h
> index 3dd3555b2740..0afc14446b01 100644
> --- a/include/uapi/linux/virtio_vsock.h
> +++ b/include/uapi/linux/virtio_vsock.h
> @@ -42,7 +42,8 @@
>  #define VIRTIO_VSOCK_F_SEQPACKET	1	/* SOCK_SEQPACKET supported */
>  
>  struct virtio_vsock_config {
> -	__le64 guest_cid;
> +	__le32 number_cid;
> +	__le64 cids[];

Config space should be generally limited to ~256 bytes.
That is < 32 cids. Enough? I would implement an interface where
you write a number and read back a cid, instead.


>  } __attribute__((packed));
>

You want a feature bit for this.

  
>  enum virtio_vsock_event_id {
> diff --git a/net/vmw_vsock/af_vsock.c b/net/vmw_vsock/af_vsock.c
> index 3e02cc3b24f8..4e1fbe74013f 100644
> --- a/net/vmw_vsock/af_vsock.c
> +++ b/net/vmw_vsock/af_vsock.c
> @@ -507,13 +507,13 @@ EXPORT_SYMBOL_GPL(vsock_assign_transport);
>  
>  bool vsock_find_cid(unsigned int cid)
>  {
> -	if (transport_g2h && cid == transport_g2h->get_local_cid())
> +	if (transport_g2h && transport_g2h->contain_cid(cid))
>  		return true;
>  
> -	if (transport_h2g && cid == VMADDR_CID_HOST)
> +	if (transport_h2g && transport_h2g->contain_cid(cid))
>  		return true;
>  
> -	if (transport_local && cid == VMADDR_CID_LOCAL)
> +	if (transport_local && transport_local->contain_cid(cid))
>  		return true;
>  
>  	return false;
> diff --git a/net/vmw_vsock/virtio_transport.c b/net/vmw_vsock/virtio_transport.c
> index e0c2c992ad9c..5f256a57d9ae 100644
> --- a/net/vmw_vsock/virtio_transport.c
> +++ b/net/vmw_vsock/virtio_transport.c
> @@ -61,10 +61,41 @@ struct virtio_vsock {
>  	bool event_run;
>  	struct virtio_vsock_event event_list[8];
>  
> -	u32 guest_cid;
> +	/* The following fields are used to hold additional cids given by the hypervisor
> +	 * such as qemu.
> +	 */
> +	u32 number_cid;
> +	u32 *cids;
> +
>  	bool seqpacket_allow;
>  };
>  
> +static bool virtio_transport_contain_cid(u32 cid)
> +{
> +	struct virtio_vsock *vsock;
> +	bool ret;
> +	u32 num_cid;
> +
> +	num_cid = 0;
> +	rcu_read_lock();
> +	vsock = rcu_dereference(the_virtio_vsock);
> +	if (!vsock || !vsock->number_cid) {
> +		ret = false;
> +		goto out_rcu;
> +	}
> +
> +	for (num_cid = 0; num_cid < vsock->number_cid; num_cid++) {
> +		if (vsock->cids[num_cid] == cid) {
> +			ret = true;
> +			goto out_rcu;
> +		}
> +	}
> +	ret = false;
> +out_rcu:
> +	rcu_read_unlock();
> +	return ret;
> +}
> +
>  static u32 virtio_transport_get_local_cid(void)
>  {
>  	struct virtio_vsock *vsock;
> @@ -72,12 +103,12 @@ static u32 virtio_transport_get_local_cid(void)
>  
>  	rcu_read_lock();
>  	vsock = rcu_dereference(the_virtio_vsock);
> -	if (!vsock) {
> +	if (!vsock || !vsock->number_cid) {
>  		ret = VMADDR_CID_ANY;
>  		goto out_rcu;
>  	}
>  
> -	ret = vsock->guest_cid;
> +	ret = vsock->cids[0];
>  out_rcu:
>  	rcu_read_unlock();
>  	return ret;
> @@ -176,7 +207,7 @@ virtio_transport_send_pkt(struct virtio_vsock_pkt *pkt)
>  		goto out_rcu;
>  	}
>  
> -	if (le64_to_cpu(pkt->hdr.dst_cid) == vsock->guest_cid) {
> +	if (le64_to_cpu(pkt->hdr.dst_cid) == vsock->cids[0]) {
>  		virtio_transport_free_pkt(pkt);
>  		len = -ENODEV;
>  		goto out_rcu;
> @@ -368,10 +399,33 @@ static void virtio_vsock_update_guest_cid(struct virtio_vsock *vsock)
>  {
>  	struct virtio_device *vdev = vsock->vdev;
>  	__le64 guest_cid;
> +	__le32 number_cid;
> +	u32 index;
>  
> -	vdev->config->get(vdev, offsetof(struct virtio_vsock_config, guest_cid),
> -			  &guest_cid, sizeof(guest_cid));
> -	vsock->guest_cid = le64_to_cpu(guest_cid);
> +	vdev->config->get(vdev, offsetof(struct virtio_vsock_config, number_cid),
> +			  &number_cid, sizeof(number_cid));

need to handle existing devices without the feature.

> +	vsock->number_cid = le32_to_cpu(number_cid);
> +
> +	/* number_cid must be greater than 0 in the config space
> +	 * to use this feature.
> +	 */
> +	if (vsock->number_cid > 0) {
> +		vsock->cids = kmalloc_array(vsock->number_cid, sizeof(u32), GFP_KERNEL);
> +		if (!vsock->cids) {
> +			/* Space allocated failed, reset number_cid to 0.
> +			 * only use the original guest_cid.
> +			 */
> +			vsock->number_cid = 0;
> +		}
> +	}
> +
> +	for (index = 0; index < vsock->number_cid; index++) {
> +		vdev->config->get(vdev,
> +				  offsetof(struct virtio_vsock_config, cids)
> +				  + index * sizeof(uint64_t),
> +				  &guest_cid, sizeof(guest_cid));
> +		vsock->cids[index] = le64_to_cpu(guest_cid);

You just drop high bits here. Unlikely to behave well if they
are not 0.


> +	}
>  }
>  
>  /* event_lock must be held */
> @@ -451,6 +505,7 @@ static struct virtio_transport virtio_transport = {
>  		.module                   = THIS_MODULE,
>  
>  		.get_local_cid            = virtio_transport_get_local_cid,
> +		.contain_cid              = virtio_transport_contain_cid,
>  
>  		.init                     = virtio_transport_do_socket_init,
>  		.destruct                 = virtio_transport_destruct,
> @@ -594,6 +649,8 @@ static int virtio_vsock_probe(struct virtio_device *vdev)
>  	}
>  
>  	vsock->vdev = vdev;
> +	vsock->cids = NULL;
> +	vsock->number_cid = 0;
>  
>  	ret = virtio_find_vqs(vsock->vdev, VSOCK_VQ_MAX,
>  			      vsock->vqs, callbacks, names,
> @@ -713,6 +770,7 @@ static void virtio_vsock_remove(struct virtio_device *vdev)
>  
>  	mutex_unlock(&the_virtio_vsock_mutex);
>  
> +	kfree(vsock->cids);
>  	kfree(vsock);
>  }
>  
> diff --git a/net/vmw_vsock/vsock_loopback.c b/net/vmw_vsock/vsock_loopback.c
> index 169a8cf65b39..3abbbaff34eb 100644
> --- a/net/vmw_vsock/vsock_loopback.c
> +++ b/net/vmw_vsock/vsock_loopback.c
> @@ -63,6 +63,13 @@ static int vsock_loopback_cancel_pkt(struct vsock_sock *vsk)
>  	return 0;
>  }
>  
> +static bool vsock_loopback_contain_cid(u32 cid)
> +{
> +	if (cid == VMADDR_CID_LOCAL)
> +		return true;
> +	return false;
> +}
> +
>  static bool vsock_loopback_seqpacket_allow(u32 remote_cid);
>  
>  static struct virtio_transport loopback_transport = {
> @@ -70,6 +77,7 @@ static struct virtio_transport loopback_transport = {
>  		.module                   = THIS_MODULE,
>  
>  		.get_local_cid            = vsock_loopback_get_local_cid,
> +		.contain_cid              = vsock_loopback_contain_cid,
>  
>  		.init                     = virtio_transport_do_socket_init,
>  		.destruct                 = virtio_transport_destruct,
> -- 
> 2.11.0
> 
> 

[PATCH 2/4] VSOCK DRIVER: support communication using additional guest cid

[fuguancheng]

Changes in this patch are made to allow the guest communicate
with the host using the additional cids specified when
creating the guest.

In original settings, the packet sent with the additional CIDS will
be rejected when received by the host, the newly added function
vhost_vsock_contain_cid will fix this error.

Now that we have multiple CIDS, the VMADDR_CID_ANY now behaves like
this:
1. The client will use the first available cid specified in the cids
array if VMADDR_CID_ANY is used.
2. The host will still use the original default CID.
3. If a guest server binds to VMADDR_CID_ANY, then the server can
choose to connect to any of the available CIDs for this guest.

Signed-off-by: fuguancheng <fuguancheng@bytedance.com>
---
 drivers/vhost/vsock.c                   | 14 +++++++++++++-
 net/vmw_vsock/af_vsock.c                |  2 +-
 net/vmw_vsock/virtio_transport_common.c |  5 ++++-
 3 files changed, 18 insertions(+), 3 deletions(-)

diff --git a/drivers/vhost/vsock.c b/drivers/vhost/vsock.c
index f66c87de91b8..013f8ebf8189 100644
--- a/drivers/vhost/vsock.c
+++ b/drivers/vhost/vsock.c
@@ -74,6 +74,18 @@ struct vhost_vsock {
 	bool seqpacket_allow;
 };
 
+static bool
+vhost_vsock_contain_cid(struct vhost_vsock *vsock, u32 cid)
+{
+	u32 index;
+
+	for (index = 0; index < vsock->num_cid; index++) {
+		if (cid == vsock->cids[index])
+			return true;
+	}
+	return false;
+}
+
 static u32 vhost_transport_get_local_cid(void)
 {
 	return VHOST_VSOCK_DEFAULT_HOST_CID;
@@ -584,7 +596,7 @@ static void vhost_vsock_handle_tx_kick(struct vhost_work *work)
 
 		/* Only accept correctly addressed packets */
 		if (vsock->num_cid > 0 &&
-		    (pkt->hdr.src_cid) == vsock->cids[0] &&
+			vhost_vsock_contain_cid(vsock, pkt->hdr.src_cid) &&
 		    le64_to_cpu(pkt->hdr.dst_cid) == vhost_transport_get_local_cid())
 			virtio_transport_recv_pkt(&vhost_transport, pkt);
 		else
diff --git a/net/vmw_vsock/af_vsock.c b/net/vmw_vsock/af_vsock.c
index 4e1fbe74013f..c22ae7101e55 100644
--- a/net/vmw_vsock/af_vsock.c
+++ b/net/vmw_vsock/af_vsock.c
@@ -251,7 +251,7 @@ static struct sock *__vsock_find_connected_socket(struct sockaddr_vm *src,
 	list_for_each_entry(vsk, vsock_connected_sockets(src, dst),
 			    connected_table) {
 		if (vsock_addr_equals_addr(src, &vsk->remote_addr) &&
-		    dst->svm_port == vsk->local_addr.svm_port) {
+		    vsock_addr_equals_addr(&vsk->local_addr, dst)) {
 			return sk_vsock(vsk);
 		}
 	}
diff --git a/net/vmw_vsock/virtio_transport_common.c b/net/vmw_vsock/virtio_transport_common.c
index 169ba8b72a63..cb45e2f801f1 100644
--- a/net/vmw_vsock/virtio_transport_common.c
+++ b/net/vmw_vsock/virtio_transport_common.c
@@ -197,7 +197,10 @@ static int virtio_transport_send_pkt_info(struct vsock_sock *vsk,
 	if (unlikely(!t_ops))
 		return -EFAULT;
 
-	src_cid = t_ops->transport.get_local_cid();
+	if (vsk->local_addr.svm_cid != VMADDR_CID_ANY)
+		src_cid = vsk->local_addr.svm_cid;
+	else
+		src_cid = t_ops->transport.get_local_cid();
 	src_port = vsk->local_addr.svm_port;
 	if (!info->remote_cid) {
 		dst_cid	= vsk->remote_addr.svm_cid;
-- 
2.11.0

Re: [PATCH 2/4] VSOCK DRIVER: support communication using additional guest cid

[Michael S. Tsirkin]

On Mon, Aug 02, 2021 at 08:07:18PM +0800, fuguancheng wrote:
> Changes in this patch are made to allow the guest communicate
> with the host using the additional cids specified when
> creating the guest.
> 
> In original settings, the packet sent with the additional CIDS will
> be rejected when received by the host, the newly added function
> vhost_vsock_contain_cid will fix this error.
> 
> Now that we have multiple CIDS, the VMADDR_CID_ANY now behaves like
> this:
> 1. The client will use the first available cid specified in the cids
> array if VMADDR_CID_ANY is used.
> 2. The host will still use the original default CID.
> 3. If a guest server binds to VMADDR_CID_ANY, then the server can
> choose to connect to any of the available CIDs for this guest.
> 
> Signed-off-by: fuguancheng <fuguancheng@bytedance.com>
> ---
>  drivers/vhost/vsock.c                   | 14 +++++++++++++-
>  net/vmw_vsock/af_vsock.c                |  2 +-
>  net/vmw_vsock/virtio_transport_common.c |  5 ++++-
>  3 files changed, 18 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/vhost/vsock.c b/drivers/vhost/vsock.c
> index f66c87de91b8..013f8ebf8189 100644
> --- a/drivers/vhost/vsock.c
> +++ b/drivers/vhost/vsock.c
> @@ -74,6 +74,18 @@ struct vhost_vsock {
>  	bool seqpacket_allow;
>  };
>  
> +static bool
> +vhost_vsock_contain_cid(struct vhost_vsock *vsock, u32 cid)
> +{
> +	u32 index;
> +
> +	for (index = 0; index < vsock->num_cid; index++) {
> +		if (cid == vsock->cids[index])
> +			return true;
> +	}
> +	return false;
> +}
> +
>  static u32 vhost_transport_get_local_cid(void)
>  {
>  	return VHOST_VSOCK_DEFAULT_HOST_CID;

Doing this linear scan on data path is not going to scale
well with lots of CIDs.

> @@ -584,7 +596,7 @@ static void vhost_vsock_handle_tx_kick(struct vhost_work *work)
>  
>  		/* Only accept correctly addressed packets */
>  		if (vsock->num_cid > 0 &&
> -		    (pkt->hdr.src_cid) == vsock->cids[0] &&
> +			vhost_vsock_contain_cid(vsock, pkt->hdr.src_cid) &&
>  		    le64_to_cpu(pkt->hdr.dst_cid) == vhost_transport_get_local_cid())
>  			virtio_transport_recv_pkt(&vhost_transport, pkt);
>  		else
> diff --git a/net/vmw_vsock/af_vsock.c b/net/vmw_vsock/af_vsock.c
> index 4e1fbe74013f..c22ae7101e55 100644
> --- a/net/vmw_vsock/af_vsock.c
> +++ b/net/vmw_vsock/af_vsock.c
> @@ -251,7 +251,7 @@ static struct sock *__vsock_find_connected_socket(struct sockaddr_vm *src,
>  	list_for_each_entry(vsk, vsock_connected_sockets(src, dst),
>  			    connected_table) {
>  		if (vsock_addr_equals_addr(src, &vsk->remote_addr) &&
> -		    dst->svm_port == vsk->local_addr.svm_port) {
> +		    vsock_addr_equals_addr(&vsk->local_addr, dst)) {
>  			return sk_vsock(vsk);
>  		}
>  	}
> diff --git a/net/vmw_vsock/virtio_transport_common.c b/net/vmw_vsock/virtio_transport_common.c
> index 169ba8b72a63..cb45e2f801f1 100644
> --- a/net/vmw_vsock/virtio_transport_common.c
> +++ b/net/vmw_vsock/virtio_transport_common.c
> @@ -197,7 +197,10 @@ static int virtio_transport_send_pkt_info(struct vsock_sock *vsk,
>  	if (unlikely(!t_ops))
>  		return -EFAULT;
>  
> -	src_cid = t_ops->transport.get_local_cid();
> +	if (vsk->local_addr.svm_cid != VMADDR_CID_ANY)
> +		src_cid = vsk->local_addr.svm_cid;
> +	else
> +		src_cid = t_ops->transport.get_local_cid();
>  	src_port = vsk->local_addr.svm_port;
>  	if (!info->remote_cid) {
>  		dst_cid	= vsk->remote_addr.svm_cid;
> -- 
> 2.11.0
> 
> 

[PATCH 3/4] VSOCK DRIVER: support specifying additional cids for host

[fuguancheng]

This packet allows the user to specify multiple additional CIDS for host
that can be used to communicate with a guest in the future. The host get
its additional cid through the ioctl call with
request code VHOST_VSOCK_SET_GUEST_CID.

Guest also knows the additional cids for host so that it can check the
received packet to see whether the packet should be received or rejected.

Guest gets host's additional cids from the device config space, so
hypervisors that emulate the device needs to be changed to use this
feature. The data layout of the device config space can be found at
include/uapi/linux/virtio_vsock.h

Signed-off-by: fuguancheng <fuguancheng@bytedance.com>
---
 drivers/vhost/vhost.h             |   5 --
 drivers/vhost/vsock.c             | 134 ++++++++++++++++++++++++++++++++++++--
 include/uapi/linux/vhost.h        |   2 +
 include/uapi/linux/virtio_vsock.h |   5 ++
 net/vmw_vsock/virtio_transport.c  |  27 ++++++++
 5 files changed, 161 insertions(+), 12 deletions(-)

diff --git a/drivers/vhost/vhost.h b/drivers/vhost/vhost.h
index 52bd143ccf0c..638bb640d6b4 100644
--- a/drivers/vhost/vhost.h
+++ b/drivers/vhost/vhost.h
@@ -25,11 +25,6 @@ struct vhost_work {
 	unsigned long		flags;
 };
 
-struct multi_cid_message {
-	u32 number_cid;
-	u64 *cid;
-};
-
 /* Poll a file (eventfd or socket) */
 /* Note: there's nothing vhost specific about this structure. */
 struct vhost_poll {
diff --git a/drivers/vhost/vsock.c b/drivers/vhost/vsock.c
index 013f8ebf8189..f5d9b9f06ba5 100644
--- a/drivers/vhost/vsock.c
+++ b/drivers/vhost/vsock.c
@@ -39,10 +39,16 @@ enum {
 	VHOST_VSOCK_BACKEND_FEATURES = (1ULL << VHOST_BACKEND_F_IOTLB_MSG_V2)
 };
 
+typedef struct vhost_vsock *(*get_vhost_vsock)(u32);
+
 /* Used to track all the vhost_vsock instances on the system. */
 static DEFINE_MUTEX(vhost_vsock_mutex);
 static DEFINE_READ_MOSTLY_HASHTABLE(vhost_vsock_hash, 8);
 
+/* Used to track all the valid host cids for vhost_vsock in system. */
+static DEFINE_MUTEX(valid_host_mutex);
+static DEFINE_READ_MOSTLY_HASHTABLE(valid_host_hash, 8);
+
 struct vhost_vsock_ref {
 	struct vhost_vsock *vsock;
 	struct hlist_node ref_hash;
@@ -65,12 +71,21 @@ struct vhost_vsock {
 
 	struct vhost_work send_pkt_work;
 	spinlock_t send_pkt_list_lock;
-	struct list_head send_pkt_list;	/* host->guest pending packets */
+	struct list_head send_pkt_list; /* host->guest pending packets */
 
 	atomic_t queued_replies;
 
 	u32 *cids;
 	u32 num_cid;
+
+	/* num_host_cid indicates how many host cids are considered valid for this guest. */
+	/* Additional cids are stored in hostcids. */
+	u32 num_host_cid;
+	u32 *hostcids;
+
+	/* Link to table valid_host_hash, writes use valid_hash_lock. */
+	struct vhost_vsock_ref *valid_cid_list;
+
 	bool seqpacket_allow;
 };
 
@@ -113,7 +128,24 @@ static struct vhost_vsock *vhost_vsock_get(u32 guest_cid)
 	return NULL;
 }
 
-static int check_if_cid_valid(u64 guest_cid, struct vhost_vsock *vsock)
+/* Callers that dereference the return value must hold vhost_vsock_mutex or the
+ * RCU read lock.
+ */
+static struct vhost_vsock *valid_vhost_get(u32 host_cid)
+{
+	struct vhost_vsock_ref *ref;
+	/* Iterate through the hash table to prevent two vhost_vsock use the same host cid. */
+	hash_for_each_possible_rcu(valid_host_hash, ref, ref_hash, host_cid) {
+		u32 other_cid = ref->cid;
+
+		if (other_cid == host_cid)
+			return ref->vsock;
+	}
+
+	return NULL;
+}
+
+static int check_if_cid_valid(u64 guest_cid, struct vhost_vsock *vsock, get_vhost_vsock func)
 {
 	struct vhost_vsock *other;
 
@@ -130,7 +162,7 @@ static int check_if_cid_valid(u64 guest_cid, struct vhost_vsock *vsock)
 		return -EADDRINUSE;
 	/* Refuse if CID is already in use */
 	mutex_lock(&vhost_vsock_mutex);
-	other = vhost_vsock_get(guest_cid);
+	other = func(guest_cid);
 	if (other) {
 		mutex_unlock(&vhost_vsock_mutex);
 		return -EADDRINUSE;
@@ -712,6 +744,10 @@ static void vhost_vsock_free(struct vhost_vsock *vsock)
 		kvfree(vsock->ref_list);
 	if (vsock->cids)
 		kvfree(vsock->cids);
+	if (vsock->valid_cid_list)
+		kvfree(vsock->valid_cid_list);
+	if (vsock->hostcids)
+		kvfree(vsock->hostcids);
 	kvfree(vsock);
 }
 
@@ -738,6 +774,10 @@ static int vhost_vsock_dev_open(struct inode *inode, struct file *file)
 	vsock->cids = NULL;
 	vsock->num_cid = 0;
 
+	vsock->valid_cid_list = NULL;
+	vsock->hostcids = NULL;
+	vsock->num_host_cid = 0;
+
 	atomic_set(&vsock->queued_replies, 0);
 
 	vqs[VSOCK_VQ_TX] = &vsock->vqs[VSOCK_VQ_TX];
@@ -808,6 +848,13 @@ static int vhost_vsock_dev_release(struct inode *inode, struct file *file)
 	}
 	mutex_unlock(&vhost_vsock_mutex);
 
+	mutex_lock(&valid_host_mutex);
+	if (vsock->num_host_cid) {
+		for (index = 0; index < vsock->num_host_cid; index++)
+			hash_del_rcu(&vsock->valid_cid_list[index].ref_hash);
+	}
+	mutex_unlock(&valid_host_mutex);
+
 	/* Wait for other CPUs to finish using vsock */
 	synchronize_rcu();
 
@@ -836,14 +883,19 @@ static int vhost_vsock_dev_release(struct inode *inode, struct file *file)
 	return 0;
 }
 
-static int vhost_vsock_set_cid(struct vhost_vsock *vsock, u64 __user *cids, u32 number_cid)
+static int vhost_vsock_set_cid(struct vhost_vsock *vsock, u64 __user *cids,
+			       u32 number_cid, u64 __user *hostcids,
+			       u32 number_host_cid)
 {
 	u64 cid;
 	int i, ret;
 
+	/* num_host_cid = 0 is allowed for that
+	 * we can use the default host cid.
+	 */
 	if (number_cid <= 0)
 		return -EINVAL;
-	/* delete the old CIDs. */
+	/* delete the old guest CIDs. */
 	if (vsock->num_cid) {
 		mutex_lock(&vhost_vsock_mutex);
 		for (i = 0; i < vsock->num_cid; i++)
@@ -854,6 +906,19 @@ static int vhost_vsock_set_cid(struct vhost_vsock *vsock, u64 __user *cids, u32
 		kvfree(vsock->cids);
 		vsock->cids = NULL;
 	}
+
+	/* delete old host CIDS related to this vsock. */
+	if (vsock->num_host_cid) {
+		mutex_lock(&valid_host_mutex);
+		for (i = 0; i < vsock->num_host_cid; i++)
+			hash_del_rcu(&vsock->valid_cid_list[i].ref_hash);
+		mutex_unlock(&valid_host_mutex);
+		kvfree(vsock->valid_cid_list);
+		vsock->valid_cid_list = NULL;
+		kvfree(vsock->hostcids);
+		vsock->valid_cid_list = NULL;
+	}
+
 	vsock->num_cid = number_cid;
 	vsock->cids = kmalloc_array(vsock->num_cid, sizeof(u32),
 				    GFP_KERNEL | __GFP_RETRY_MAYFAIL);
@@ -870,6 +935,22 @@ static int vhost_vsock_set_cid(struct vhost_vsock *vsock, u64 __user *cids, u32
 		goto out;
 	}
 
+	vsock->num_host_cid = number_host_cid;
+	vsock->hostcids = kmalloc_array(vsock->num_host_cid, sizeof(u32),
+				    GFP_KERNEL | __GFP_RETRY_MAYFAIL);
+	if (!vsock->hostcids) {
+		vsock->num_host_cid = 0;
+		ret = -ENOMEM;
+		goto out;
+	}
+	vsock->valid_cid_list = kvmalloc_array(vsock->num_host_cid, sizeof(*vsock->ref_list),
+			       GFP_KERNEL | __GFP_RETRY_MAYFAIL);
+	if (!vsock->valid_cid_list) {
+		vsock->num_host_cid = 0;
+		ret = -ENOMEM;
+		goto out;
+	}
+
 	for (i = 0; i < number_cid; i++) {
 		if (copy_from_user(&cid, cids + i, sizeof(cid))) {
 			/* record where we failed, to clean up the ref in hash table. */
@@ -877,7 +958,7 @@ static int vhost_vsock_set_cid(struct vhost_vsock *vsock, u64 __user *cids, u32
 			ret = -EFAULT;
 			goto out;
 		}
-		ret = check_if_cid_valid(cid, vsock);
+		ret = check_if_cid_valid(cid, vsock, vhost_vsock_get);
 		if (ret) {
 			vsock->num_cid = i;
 			goto out;
@@ -891,6 +972,28 @@ static int vhost_vsock_set_cid(struct vhost_vsock *vsock, u64 __user *cids, u32
 			     vsock->cids[i]);
 		mutex_unlock(&vhost_vsock_mutex);
 	}
+
+	for (i = 0; i < number_host_cid; i++) {
+		if (copy_from_user(&cid, hostcids + i, sizeof(cid))) {
+			vsock->num_host_cid = i;
+			ret = -EFAULT;
+			goto out;
+		}
+		ret = check_if_cid_valid(cid, vsock, valid_vhost_get);
+		if (ret) {
+			vsock->num_host_cid = i;
+			goto out;
+		}
+
+		vsock->hostcids[i] = (u32)cid;
+		vsock->valid_cid_list[i].cid = vsock->hostcids[i];
+		vsock->valid_cid_list[i].vsock = vsock;
+		mutex_lock(&valid_host_mutex);
+		hash_add_rcu(valid_host_hash,
+		     &vsock->valid_cid_list[i].ref_hash,
+		     vsock->hostcids[i]);
+		mutex_unlock(&valid_host_mutex);
+	}
 	return 0;
 
 out:
@@ -902,13 +1005,27 @@ static int vhost_vsock_set_cid(struct vhost_vsock *vsock, u64 __user *cids, u32
 		mutex_unlock(&vhost_vsock_mutex);
 		vsock->num_cid = 0;
 	}
+
+	if (vsock->num_host_cid) {
+		mutex_lock(&valid_host_mutex);
+		for (i = 0; i < vsock->num_host_cid; i++)
+			hash_del_rcu(&vsock->valid_cid_list[i].ref_hash);
+		mutex_unlock(&valid_host_mutex);
+		vsock->num_host_cid = 0;
+	}
 	if (vsock->ref_list)
 		kvfree(vsock->ref_list);
 	if (vsock->cids)
 		kvfree(vsock->cids);
+	if (vsock->valid_cid_list)
+		kvfree(vsock->valid_cid_list);
+	if (vsock->hostcids)
+		kvfree(vsock->hostcids);
 	/* Set it to null to prevent double release. */
 	vsock->ref_list = NULL;
 	vsock->cids = NULL;
+	vsock->valid_cid_list = NULL;
+	vsock->hostcids = NULL;
 	return ret;
 }
 
@@ -962,7 +1079,10 @@ static long vhost_vsock_dev_ioctl(struct file *f, unsigned int ioctl,
 	case VHOST_VSOCK_SET_GUEST_CID:
 		if (copy_from_user(&cid_message, argp, sizeof(cid_message)))
 			return -EFAULT;
-		return vhost_vsock_set_cid(vsock, cid_message.cid, cid_message.number_cid);
+		return vhost_vsock_set_cid(vsock, cid_message.cid,
+					   cid_message.number_cid,
+					   cid_message.hostcid,
+					   cid_message.number_host_cid);
 	case VHOST_VSOCK_SET_RUNNING:
 		if (copy_from_user(&start, argp, sizeof(start)))
 			return -EFAULT;
diff --git a/include/uapi/linux/vhost.h b/include/uapi/linux/vhost.h
index a3ea99f6fc7f..e2639d7ce375 100644
--- a/include/uapi/linux/vhost.h
+++ b/include/uapi/linux/vhost.h
@@ -22,6 +22,8 @@
 struct multi_cid_message {
 	u32 number_cid;
 	u64 *cid;
+	u32 number_host_cid;
+	u64 *hostcid;
 };
 
 /* ioctls */
diff --git a/include/uapi/linux/virtio_vsock.h b/include/uapi/linux/virtio_vsock.h
index 0afc14446b01..18c54bfdcbf2 100644
--- a/include/uapi/linux/virtio_vsock.h
+++ b/include/uapi/linux/virtio_vsock.h
@@ -41,8 +41,13 @@
 /* The feature bitmap for virtio vsock */
 #define VIRTIO_VSOCK_F_SEQPACKET	1	/* SOCK_SEQPACKET supported */
 
+/* For values stored in cids, the first "number_cid" values
+ * are used for guest additional cid.
+ * The last "number_host_cid" values are used for host additional cid.
+ */
 struct virtio_vsock_config {
 	__le32 number_cid;
+	__le32 number_host_cid;
 	__le64 cids[];
 } __attribute__((packed));
 
diff --git a/net/vmw_vsock/virtio_transport.c b/net/vmw_vsock/virtio_transport.c
index 5f256a57d9ae..c552bc60e539 100644
--- a/net/vmw_vsock/virtio_transport.c
+++ b/net/vmw_vsock/virtio_transport.c
@@ -67,6 +67,9 @@ struct virtio_vsock {
 	u32 number_cid;
 	u32 *cids;
 
+	u32 number_host_cid;
+	u32 *host_cids;
+
 	bool seqpacket_allow;
 };
 
@@ -400,11 +403,16 @@ static void virtio_vsock_update_guest_cid(struct virtio_vsock *vsock)
 	struct virtio_device *vdev = vsock->vdev;
 	__le64 guest_cid;
 	__le32 number_cid;
+	__le64 host_cid;
+	__le32 number_host_cid;
 	u32 index;
 
 	vdev->config->get(vdev, offsetof(struct virtio_vsock_config, number_cid),
 			  &number_cid, sizeof(number_cid));
+	vdev->config->get(vdev, offsetof(struct virtio_vsock_config, number_host_cid),
+			  &number_host_cid, sizeof(number_host_cid));
 	vsock->number_cid = le32_to_cpu(number_cid);
+	vsock->number_host_cid = le32_to_cpu(number_host_cid);
 
 	/* number_cid must be greater than 0 in the config space
 	 * to use this feature.
@@ -419,6 +427,16 @@ static void virtio_vsock_update_guest_cid(struct virtio_vsock *vsock)
 		}
 	}
 
+	if (vsock->number_host_cid > 0) {
+		vsock->host_cids = kmalloc_array(vsock->number_host_cid, sizeof(u32), GFP_KERNEL);
+		if (!vsock->host_cids) {
+			/* Space allocated failed, reset number_cid to 0.
+			 * only use the original guest_cid.
+			 */
+			vsock->number_host_cid = 0;
+		}
+	}
+
 	for (index = 0; index < vsock->number_cid; index++) {
 		vdev->config->get(vdev,
 				  offsetof(struct virtio_vsock_config, cids)
@@ -426,6 +444,14 @@ static void virtio_vsock_update_guest_cid(struct virtio_vsock *vsock)
 				  &guest_cid, sizeof(guest_cid));
 		vsock->cids[index] = le64_to_cpu(guest_cid);
 	}
+
+	for (index = index; index < vsock->number_cid + vsock->number_host_cid; index++) {
+		vdev->config->get(vdev,
+				  offsetof(struct virtio_vsock_config, cids)
+				  + index * sizeof(uint64_t),
+				  &host_cid, sizeof(host_cid));
+		vsock->host_cids[index - vsock->number_cid] = le64_to_cpu(host_cid);
+	}
 }
 
 /* event_lock must be held */
@@ -771,6 +797,7 @@ static void virtio_vsock_remove(struct virtio_device *vdev)
 	mutex_unlock(&the_virtio_vsock_mutex);
 
 	kfree(vsock->cids);
+	kfree(vsock->host_cids);
 	kfree(vsock);
 }
 
-- 
2.11.0

[PATCH 4/4] VSOCK DRIVER: support communication using host additional cids

[fuguancheng]

This patch allows the user to use the additional host CIDS to communicate
with the guest.  As server, the host can bind to any CIDS as long as
the cid can be mapped to one guest.

The VHOST_DEFAULT_CID can be used as normal.

As client, when connect to a remote server, if no address is specified to
be used, then it will use the first cid in the array. If the user wants
to use a specific cid, then the user can perfrom bind before the connect
operation, so that vsock_auto_bind will not be performed.

The patch depends on the previous patch which enables hypervisors such as
qemu to specify multiple cids for host and guest.

Signed-off-by: fuguancheng <fuguancheng@bytedance.com>
---
 drivers/vhost/vsock.c            | 39 ++++++++++++++++++++++++++++++++++++++-
 include/net/af_vsock.h           |  4 ++++
 net/vmw_vsock/af_vsock.c         | 20 ++++++++++++++------
 net/vmw_vsock/virtio_transport.c | 30 ++++++++++++++++++++++++++++++
 4 files changed, 86 insertions(+), 7 deletions(-)

diff --git a/drivers/vhost/vsock.c b/drivers/vhost/vsock.c
index f5d9b9f06ba5..104fcdea2dd7 100644
--- a/drivers/vhost/vsock.c
+++ b/drivers/vhost/vsock.c
@@ -57,8 +57,22 @@ struct vhost_vsock_ref {
 
 static bool vhost_transport_contain_cid(u32 cid)
 {
+	unsigned int index;
+	struct vhost_vsock_ref *ref;
+
 	if (cid == VHOST_VSOCK_DEFAULT_HOST_CID)
 		return true;
+
+	mutex_lock(&valid_host_mutex);
+	hash_for_each(valid_host_hash, index, ref, ref_hash) {
+		u32 other_cid = ref->cid;
+
+		if (other_cid == cid) {
+			mutex_unlock(&valid_host_mutex);
+			return true;
+		}
+	}
+	mutex_unlock(&valid_host_mutex);
 	return false;
 }
 
@@ -101,6 +115,21 @@ vhost_vsock_contain_cid(struct vhost_vsock *vsock, u32 cid)
 	return false;
 }
 
+/* Check if a cid is valid for the pkt to be received. */
+static bool
+vhost_vsock_contain_host_cid(struct vhost_vsock *vsock, u32 dst_cid)
+{
+	uint32_t index;
+
+	if (dst_cid == VHOST_VSOCK_DEFAULT_HOST_CID)
+		return true;
+	for (index = 0; index < vsock->num_host_cid; index++) {
+		if (vsock->hostcids[index] == dst_cid)
+			return true;
+	}
+	return false;
+}
+
 static u32 vhost_transport_get_local_cid(void)
 {
 	return VHOST_VSOCK_DEFAULT_HOST_CID;
@@ -128,6 +157,13 @@ static struct vhost_vsock *vhost_vsock_get(u32 guest_cid)
 	return NULL;
 }
 
+/* This function checks if the cid is used by one of the guests. */
+static bool
+vhost_transport_contain_opposite_cid(u32 cid)
+{
+	return vhost_vsock_get(cid) != NULL;
+}
+
 /* Callers that dereference the return value must hold vhost_vsock_mutex or the
  * RCU read lock.
  */
@@ -512,6 +548,7 @@ static struct virtio_transport vhost_transport = {
 
 		.get_local_cid            = vhost_transport_get_local_cid,
 		.contain_cid              = vhost_transport_contain_cid,
+		.contain_opposite_cid     = vhost_transport_contain_opposite_cid,
 
 		.init                     = virtio_transport_do_socket_init,
 		.destruct                 = virtio_transport_destruct,
@@ -629,7 +666,7 @@ static void vhost_vsock_handle_tx_kick(struct vhost_work *work)
 		/* Only accept correctly addressed packets */
 		if (vsock->num_cid > 0 &&
 			vhost_vsock_contain_cid(vsock, pkt->hdr.src_cid) &&
-		    le64_to_cpu(pkt->hdr.dst_cid) == vhost_transport_get_local_cid())
+		    vhost_vsock_contain_host_cid(vsock, le64_to_cpu(pkt->hdr.dst_cid)))
 			virtio_transport_recv_pkt(&vhost_transport, pkt);
 		else
 			virtio_transport_free_pkt(pkt);
diff --git a/include/net/af_vsock.h b/include/net/af_vsock.h
index d0fc08fb9cac..739ac9aaff8f 100644
--- a/include/net/af_vsock.h
+++ b/include/net/af_vsock.h
@@ -171,6 +171,10 @@ struct vsock_transport {
 	/* Addressing. */
 	u32 (*get_local_cid)(void);
 	bool (*contain_cid)(u32 cid);
+	/* For transport_g2h, this checks if the cid is used by its host. */
+	/* For transport_h2g, this checks if the cid is used by one of its guests. */
+	/* This function is set to NULL for loopback_transport. */
+	bool (*contain_opposite_cid)(u32 cid);
 };
 
 /**** CORE ****/
diff --git a/net/vmw_vsock/af_vsock.c b/net/vmw_vsock/af_vsock.c
index c22ae7101e55..d3037ee885be 100644
--- a/net/vmw_vsock/af_vsock.c
+++ b/net/vmw_vsock/af_vsock.c
@@ -397,9 +397,9 @@ static bool vsock_use_local_transport(unsigned int remote_cid)
 		return true;
 
 	if (transport_g2h) {
-		return remote_cid == transport_g2h->get_local_cid();
+		return transport_g2h->contain_cid(remote_cid);
 	} else {
-		return remote_cid == VMADDR_CID_HOST;
+		return transport_h2g->contain_cid(remote_cid);
 	}
 }
 
@@ -423,7 +423,9 @@ static void vsock_deassign_transport(struct vsock_sock *vsk)
  *    g2h is not loaded, will use local transport;
  *  - remote CID <= VMADDR_CID_HOST or h2g is not loaded or remote flags field
  *    includes VMADDR_FLAG_TO_HOST flag value, will use guest->host transport;
- *  - remote CID > VMADDR_CID_HOST will use host->guest transport;
+ *  - remote CID > VMADDR_CID_HOST will use host->guest transport if
+ *    guest->host transport is not loaded.  Otherwise, if guest->host transport
+ *    contains the remote_cid, then use the guest->host transport.
  */
 int vsock_assign_transport(struct vsock_sock *vsk, struct vsock_sock *psk)
 {
@@ -434,15 +436,18 @@ int vsock_assign_transport(struct vsock_sock *vsk, struct vsock_sock *psk)
 	int ret;
 
 	/* If the packet is coming with the source and destination CIDs higher
-	 * than VMADDR_CID_HOST, then a vsock channel where all the packets are
+	 * than VMADDR_CID_HOST, and the source and destination CIDs are not
+	 * used by the host, then a vsock channel where all the packets are
 	 * forwarded to the host should be established. Then the host will
 	 * need to forward the packets to the guest.
 	 *
 	 * The flag is set on the (listen) receive path (psk is not NULL). On
 	 * the connect path the flag can be set by the user space application.
 	 */
-	if (psk && vsk->local_addr.svm_cid > VMADDR_CID_HOST &&
-	    vsk->remote_addr.svm_cid > VMADDR_CID_HOST)
+	if (psk && transport_h2g && vsk->local_addr.svm_cid > VMADDR_CID_HOST &&
+	    !transport_h2g->contain_cid(vsk->local_addr.svm_cid) &&
+	    vsk->remote_addr.svm_cid > VMADDR_CID_HOST &&
+	    !transport_h2g->contain_cid(vsk->remote_addr.svm_cid))
 		vsk->remote_addr.svm_flags |= VMADDR_FLAG_TO_HOST;
 
 	remote_flags = vsk->remote_addr.svm_flags;
@@ -458,6 +463,9 @@ int vsock_assign_transport(struct vsock_sock *vsk, struct vsock_sock *psk)
 		else if (remote_cid <= VMADDR_CID_HOST || !transport_h2g ||
 			 (remote_flags & VMADDR_FLAG_TO_HOST))
 			new_transport = transport_g2h;
+		else if (remote_cid > VMADDR_CID_HOST && transport_g2h &&
+			 transport_g2h->contain_opposite_cid(remote_cid))
+			new_transport = transport_g2h;
 		else
 			new_transport = transport_h2g;
 		break;
diff --git a/net/vmw_vsock/virtio_transport.c b/net/vmw_vsock/virtio_transport.c
index c552bc60e539..0c4a2f03318c 100644
--- a/net/vmw_vsock/virtio_transport.c
+++ b/net/vmw_vsock/virtio_transport.c
@@ -99,6 +99,35 @@ static bool virtio_transport_contain_cid(u32 cid)
 	return ret;
 }
 
+/* This function checks if the transport_g2h is using the cid. */
+static bool virtio_transport_contain_opposite_cid(u32 cid)
+{
+	struct virtio_vsock *vsock;
+	bool ret;
+	u32 num_host_cid;
+
+	if (cid == VMADDR_CID_HOST)
+		return true;
+	num_host_cid = 0;
+	rcu_read_lock();
+	vsock = rcu_dereference(the_virtio_vsock);
+	if (!vsock || vsock->number_host_cid == 0) {
+		ret = false;
+		goto out_rcu;
+	}
+
+	for (num_host_cid = 0; num_host_cid < vsock->number_host_cid; num_host_cid++) {
+		if (vsock->host_cids[num_host_cid] == cid) {
+			ret = true;
+			goto out_rcu;
+		}
+	}
+	ret = false;
+out_rcu:
+	rcu_read_unlock();
+	return ret;
+}
+
 static u32 virtio_transport_get_local_cid(void)
 {
 	struct virtio_vsock *vsock;
@@ -532,6 +561,7 @@ static struct virtio_transport virtio_transport = {
 
 		.get_local_cid            = virtio_transport_get_local_cid,
 		.contain_cid              = virtio_transport_contain_cid,
+		.contain_opposite_cid     = virtio_transport_contain_opposite_cid,
 
 		.init                     = virtio_transport_do_socket_init,
 		.destruct                 = virtio_transport_destruct,
-- 
2.11.0

Re: [PATCH 0/4] Add multi-cid support for vsock driver

[Stefano Garzarella]

On Mon, Aug 02, 2021 at 08:07:16PM +0800, fuguancheng wrote:
>This patchset enables the user to specify additional CIDS for host and
>guest when booting up the guest machine. The guest's additional CIDS cannot
>be repeated, and can be used to communicate with the host. The user can
>also choose to specify a set of additional host cids, which can be
>used to communicate with the guest who specify them. The original
>CID(VHOST_DEFAULT_CID) is still available for host. The guest cid field is
>deleted.
>
>To ensure that multiple guest CID maps to the same vhost_vsock struct,
>a struct called vhost_vsock_ref is added.  The function of vhost_vsock_ref
>is simply used to allow multiple guest CIDS map to the
>same vhost_vsock struct.
>
>If not specified, the host and guest will now use the first CID specified
>in the array for connect operation. If the host or guest wants to use
>one specific CID, the bind operation can be performed before the connect
>operation so that the vsock_auto_bind operation can be avoided.
>
>Hypervisors such as qemu needs to be modified to use this feature. The
>required changes including at least the following:
>1. Invoke the modified ioctl call with the request code
>VHOST_VSOCK_SET_GUEST_CID. Also see struct multi_cid_message for
>arguments used in this ioctl call.
>2. Write new arguments to the emulated device config space.
>3. Modify the layout of the data written to the device config space.
>See struct virtio_vsock_config for reference.

Can you please describe a use case?

vsock was created to be zero configuration, we're complicating enough 
here, we should have a particular reason.

Also I gave a quick view and it seems to me that you change 
virtio_vsock_config, are you sure it works if one of the two peers 
doesn't support multiple CIDs?

Maybe we'd need a new feature bit, and we'd definitely need to discuss 
specification changes with virtio-comment@lists.oasis-open.org first.

How does the guest or host applications know which CIDs are assigned to 
them?

Please use the RFC tag if the patches are not in good shape.
Patches seem hard to review, please avoid adding code that is removed 
later (e.g.  multi_cid_message), and try not to break the backward 
compatibility.

Thanks,
Stefano

Re: [External] Re: [PATCH 0/4] Add multi-cid support for vsock driver

[Stefano Garzarella]

On Wed, Aug 04, 2021 at 03:09:41PM +0800, 傅关丞 wrote:
>Sorry I cannot figure out a good use case.
>
>It is normal for a host to have multiple ip addresses used for
>communication.
>So I thought it might be nice to have both  host and guest use multiple
>CIDs for communication.
>I know this is not a very strong argument.

Maybe there could be a use case for guests (which I don't see now), but 
for the host it seems pointless. The strength of vsock is that the guest 
knows that using CID=2 always reaches the host.

Moreover we have recently merged VMADDR_FLAG_TO_HOST that when set 
allows you to forward any packet to the host, regardless of the CID (not 
yet supported by vhost-vsock).

>
>The vsock driver does not work if one of the two peers doesn't support
>multiple CIDs.

This is absolutely to be avoided.

I think the virtio device feature negotiation can help here.

>
>I have a possible solution here, but there may be some problems with it
>that I haven't noticed.
>
>Hypervisors will use different ways to send CIDs setup to the kernel based
>on their vsock setup.
>
>------For host-------
>If host vsock driver supports multi-cid, the hypervisor will use the
>modified VHOST_VSOCK_SET_GUEST_CID call to set its CIDs.
>Otherwise, the original call is used.
>
>------For guest-------
>Now the virtio_vsock_config looks like this:
>u64 guest_cid
>u32 num_guest_cid;
>u32 num_host_cid;
>u32 index;
>u64 cid;
>
>If the guest vsock driver supports multi-cid, it will read num_guest_cid
>and num_host_cid from the device config space.
>Then it writes an index register, which is the cid it wants to read.  After
>hypervisors handle this issue, it can read the cid
>from the cid register.
>
>If it does not support multi-cid, it will just read the guest_cid from the
>config space, which should work just fine.
>

Why not add a new device feature to enable or disable multi-cid?


>
>-------Communication--------
>For communication issues, we might need to use a new feature bit.  Let's
>call it VHOST_VSOCK_SUPPORT_MULTI_CID.
>The basic idea is that this feature bit is set when both host and guest
>support using multiple CIDs.  After negotiation, if the feature bit
>is set, the host can use all the CIDs specified to communicate with the
>guest.  Otherwise, the first cid passed in will
>be used as the guest_cid to communicate with guests.

I think the same feature bit can be used for the virtio_vsock_config, 
no?

>
>Also, if the bit is set for guests, all the CIDs can be used to communicate
>with the host.  Otherwise, the first cid with index 0 will be
>used as the guest_cid while the VMADDR_HOST_CID will be used for host cid.

We already have VMADDR_FLAG_TO_HOST to forward all packets to the host, 
we only need to support in some way in vhost-vsock.

Thanks,
Stefano

Re: [PATCH 0/4] Add multi-cid support for vsock driver

[Michael S. Tsirkin]

On Mon, Aug 02, 2021 at 08:07:16PM +0800, fuguancheng wrote:
> This patchset enables the user to specify additional CIDS for host and
> guest when booting up the guest machine. The guest's additional CIDS cannot
> be repeated, and can be used to communicate with the host. The user can
> also choose to specify a set of additional host cids, which can be
> used to communicate with the guest who specify them. The original
> CID(VHOST_DEFAULT_CID) is still available for host. The guest cid field is
> deleted.
> 
> To ensure that multiple guest CID maps to the same vhost_vsock struct,
> a struct called vhost_vsock_ref is added.  The function of vhost_vsock_ref
> is simply used to allow multiple guest CIDS map to the
> same vhost_vsock struct.
> 
> If not specified, the host and guest will now use the first CID specified
> in the array for connect operation. If the host or guest wants to use
> one specific CID, the bind operation can be performed before the connect
> operation so that the vsock_auto_bind operation can be avoided.
> 
> Hypervisors such as qemu needs to be modified to use this feature. The
> required changes including at least the following:
> 1. Invoke the modified ioctl call with the request code
> VHOST_VSOCK_SET_GUEST_CID. Also see struct multi_cid_message for
> arguments used in this ioctl call.
> 2. Write new arguments to the emulated device config space.
> 3. Modify the layout of the data written to the device config space.
> See struct virtio_vsock_config for reference.
> 
> I have tested this setup with iperf3.  The communication between host
> and guest using original CID or additional CIDS worked normally.
> Not tested in extreme conditions where memory is insufficient.
> 
> Linux kernel newbies here, any suggestions are welcomed.
> Thanks in advance!

Could you supply a bit info about the motivation for this feature?
I wonder whether it's be better to have multiple VQs
instead of tweaking the CID in the message header.


> fuguancheng (4):
>   VSOCK DRIVER: Add multi-cid support for guest
>   VSOCK DRIVER: support communication using additional guest cid
>   VSOCK DRIVER: support specifying additional cids for host
>   VSOCK DRIVER: support communication using host additional cids
> 
>  drivers/vhost/vsock.c                   | 338 ++++++++++++++++++++++++++++----
>  include/net/af_vsock.h                  |   5 +
>  include/uapi/linux/vhost.h              |   9 +
>  include/uapi/linux/virtio_vsock.h       |   8 +-
>  net/vmw_vsock/af_vsock.c                |  28 ++-
>  net/vmw_vsock/virtio_transport.c        | 129 +++++++++++-
>  net/vmw_vsock/virtio_transport_common.c |   5 +-
>  net/vmw_vsock/vsock_loopback.c          |   8 +
>  8 files changed, 471 insertions(+), 59 deletions(-)
> 
> -- 
> 2.11.0
> 
>