Skip to contentSkip to conversation

VSOCK: support fill data to mergeable rx buffer in host

PATCH Superseded jiangyiwen 2 participants 15 messages Nov 5, 2018 – Nov 8, 2018

Subsystems (6)

v1 of 2 Superseded Latest is v2 →

lore
Share & tools
lore

jiangyiwen

author
#0 lore raw

PATCH 2/5 VSOCK: support fill data to mergeable rx buffer in host

When vhost support VIRTIO_VSOCK_F_MRG_RXBUF feature, it will merge big packet into rx vq.

---
 drivers/vhost/vsock.c             | 117 +++++++++++++++++++++++++++++++-------
 include/linux/virtio_vsock.h      |   1 +
 include/uapi/linux/virtio_vsock.h |   5 ++
 3 files changed, 102 insertions(+), 21 deletions(-)
drivers/vhost/vsock.c +96 -21
index 34bc3ab..648be39 100644
@@ -22,7 +22,8 @@
 #define VHOST_VSOCK_DEFAULT_HOST_CID	2
 
 enum {
-	VHOST_VSOCK_FEATURES = VHOST_FEATURES,
+	VHOST_VSOCK_FEATURES = VHOST_FEATURES |
+			(1ULL << VIRTIO_VSOCK_F_MRG_RXBUF),
 };
 
 /* Used to track all the vhost_vsock instances on the system. */
@@ -80,6 +81,68 @@ static struct vhost_vsock *vhost_vsock_get(u32 guest_cid)
 	return vsock;
 }
 
+static int get_rx_bufs(struct vhost_virtqueue *vq,
+		struct vring_used_elem *heads, int datalen,
+		unsigned *iovcount, unsigned int quota)
+{
+	unsigned int out, in;
+	int seg = 0;
+	int headcount = 0;
+	unsigned d;
+	int ret;
+	/*
+	 * len is always initialized before use since we are always called with
+	 * datalen > 0.
+	 */
+	u32 uninitialized_var(len);
+
+	while (datalen > 0 && headcount < quota) {
+		if (unlikely(seg >= UIO_MAXIOV)) {
+			ret = -ENOBUFS;
+			goto err;
+		}
+
+		ret = vhost_get_vq_desc(vq, vq->iov + seg,
+				ARRAY_SIZE(vq->iov) - seg, &out,
+				&in, NULL, NULL);
+		if (unlikely(ret < 0))
+			goto err;
+
+		d = ret;
+		if (d == vq->num) {
+			ret = 0;
+			goto err;
+		}
+
+		if (unlikely(out || in <= 0)) {
+			vq_err(vq, "unexpected descriptor format for RX: "
+					"out %d, in %d\n", out, in);
+			ret = -EINVAL;
+			goto err;
+		}
+
+		heads[headcount].id = cpu_to_vhost32(vq, d);
+		len = iov_length(vq->iov + seg, in);
+		heads[headcount].len = cpu_to_vhost32(vq, len);
+		datalen -= len;
+		++headcount;
+		seg += in;
+	}
+
+	heads[headcount - 1].len = cpu_to_vhost32(vq, len + datalen);
+	*iovcount = seg;
+
+	/* Detect overrun */
+	if (unlikely(datalen > 0)) {
+		ret = UIO_MAXIOV + 1;
+		goto err;
+	}
+	return headcount;
+err:
+	vhost_discard_vq_desc(vq, headcount);
+	return ret;
+}
+
 static void
 vhost_transport_do_send_pkt(struct vhost_vsock *vsock,
 			    struct vhost_virtqueue *vq)
@@ -87,22 +150,34 @@ static struct vhost_vsock *vhost_vsock_get(u32 guest_cid)
 	struct vhost_virtqueue *tx_vq = &vsock->vqs[VSOCK_VQ_TX];
 	bool added = false;
 	bool restart_tx = false;
+	int mergeable;
+	size_t vsock_hlen;
 
 	mutex_lock(&vq->mutex);
 
 	if (!vq->private_data)
 		goto out;
 
+	mergeable = vhost_has_feature(vq, VIRTIO_VSOCK_F_MRG_RXBUF);
+	/*
+	 * Guest fill page for rx vq in mergeable case, so it will not
+	 * allocate pkt structure, we should reserve size of pkt in advance.
+	 */
+	if (likely(mergeable))
+		vsock_hlen = sizeof(struct virtio_vsock_pkt);
+	else
+		vsock_hlen = sizeof(struct virtio_vsock_hdr);
+
 	/* Avoid further vmexits, we're already processing the virtqueue */
 	vhost_disable_notify(&vsock->dev, vq);
 
 	for (;;) {
 		struct virtio_vsock_pkt *pkt;
 		struct iov_iter iov_iter;
-		unsigned out, in;
+		unsigned out = 0, in = 0;
 		size_t nbytes;
 		size_t len;
-		int head;
+		s16 headcount;
 
 		spin_lock_bh(&vsock->send_pkt_list_lock);
 		if (list_empty(&vsock->send_pkt_list)) {
@@ -116,16 +191,9 @@ static struct vhost_vsock *vhost_vsock_get(u32 guest_cid)
 		list_del_init(&pkt->list);
 		spin_unlock_bh(&vsock->send_pkt_list_lock);
 
-		head = vhost_get_vq_desc(vq, vq->iov, ARRAY_SIZE(vq->iov),
-					 &out, &in, NULL, NULL);
-		if (head < 0) {
-			spin_lock_bh(&vsock->send_pkt_list_lock);
-			list_add(&pkt->list, &vsock->send_pkt_list);
-			spin_unlock_bh(&vsock->send_pkt_list_lock);
-			break;
-		}
-
-		if (head == vq->num) {
+		headcount = get_rx_bufs(vq, vq->heads, vsock_hlen + pkt->len,
+				&in, likely(mergeable) ? UIO_MAXIOV : 1);
+		if (headcount <= 0) {
 			spin_lock_bh(&vsock->send_pkt_list_lock);
 			list_add(&pkt->list, &vsock->send_pkt_list);
 			spin_unlock_bh(&vsock->send_pkt_list_lock);
@@ -133,19 +201,13 @@ static struct vhost_vsock *vhost_vsock_get(u32 guest_cid)
 			/* We cannot finish yet if more buffers snuck in while
 			 * re-enabling notify.
 			 */
-			if (unlikely(vhost_enable_notify(&vsock->dev, vq))) {
+			if (!headcount && unlikely(vhost_enable_notify(&vsock->dev, vq))) {
 				vhost_disable_notify(&vsock->dev, vq);
 				continue;
 			}
 			break;
 		}
 
-		if (out) {
-			virtio_transport_free_pkt(pkt);
-			vq_err(vq, "Expected 0 output buffers, got %u\n", out);
-			break;
-		}
-
 		len = iov_length(&vq->iov[out], in);
 		iov_iter_init(&iov_iter, READ, &vq->iov[out], in, len);
 
@@ -156,6 +218,19 @@ static struct vhost_vsock *vhost_vsock_get(u32 guest_cid)
 			break;
 		}
 
+		if (likely(mergeable)) {
+			pkt->mrg_rxbuf_hdr.num_buffers = cpu_to_le16(headcount);
+			nbytes = copy_to_iter(&pkt->mrg_rxbuf_hdr,
+					sizeof(pkt->mrg_rxbuf_hdr), &iov_iter);
+			if (nbytes != sizeof(pkt->mrg_rxbuf_hdr)) {
+				virtio_transport_free_pkt(pkt);
+				vq_err(vq, "Faulted on copying rxbuf hdr\n");
+				break;
+			}
+			iov_iter_advance(&iov_iter, (vsock_hlen -
+					sizeof(pkt->mrg_rxbuf_hdr) - sizeof(pkt->hdr)));
+		}
+
 		nbytes = copy_to_iter(pkt->buf, pkt->len, &iov_iter);
 		if (nbytes != pkt->len) {
 			virtio_transport_free_pkt(pkt);
@@ -163,7 +238,7 @@ static struct vhost_vsock *vhost_vsock_get(u32 guest_cid)
 			break;
 		}
 
-		vhost_add_used(vq, head, sizeof(pkt->hdr) + pkt->len);
+		vhost_add_used_n(vq, vq->heads, headcount);
 		added = true;
 
 		if (pkt->reply) {
include/linux/virtio_vsock.h +1 -0
index bf84418..da9e1fe 100644
@@ -50,6 +50,7 @@ struct virtio_vsock_sock {
 
 struct virtio_vsock_pkt {
 	struct virtio_vsock_hdr	hdr;
+	struct virtio_vsock_mrg_rxbuf_hdr mrg_rxbuf_hdr;
 	struct work_struct work;
 	struct list_head list;
 	/* socket refcnt not held, only use for cancellation */
include/uapi/linux/virtio_vsock.h +5 -0
index 1d57ed3..2292f30 100644
@@ -63,6 +63,11 @@ struct virtio_vsock_hdr {
 	__le32	fwd_cnt;
 } __attribute__((packed));
 
+/* It add mergeable rx buffers feature */
+struct virtio_vsock_mrg_rxbuf_hdr {
+	__le16  num_buffers;    /* number of mergeable rx buffers */
+} __attribute__((packed));
+
 enum virtio_vsock_type {
 	VIRTIO_VSOCK_TYPE_STREAM = 1,
 };
Signature · 1 line
1.8.3.1
#1 lore raw
Quoted 79 lines · jiangyiwen

When vhost support VIRTIO_VSOCK_F_MRG_RXBUF feature, it will merge big packet into rx vq.

---
  drivers/vhost/vsock.c             | 117 +++++++++++++++++++++++++++++++-------
  include/linux/virtio_vsock.h      |   1 +
  include/uapi/linux/virtio_vsock.h |   5 ++
  3 files changed, 102 insertions(+), 21 deletions(-)
drivers/vhost/vsock.c +53 -1
index 34bc3ab..648be39 100644
@@ -22,7 +22,8 @@
  #define VHOST_VSOCK_DEFAULT_HOST_CID	2
 
  enum {
-	VHOST_VSOCK_FEATURES = VHOST_FEATURES,
+	VHOST_VSOCK_FEATURES = VHOST_FEATURES |
+			(1ULL << VIRTIO_VSOCK_F_MRG_RXBUF),
  };
 
  /* Used to track all the vhost_vsock instances on the system. */
@@ -80,6 +81,68 @@ static struct vhost_vsock *vhost_vsock_get(u32 guest_cid)
  	return vsock;
  }
 
+static int get_rx_bufs(struct vhost_virtqueue *vq,
+		struct vring_used_elem *heads, int datalen,
+		unsigned *iovcount, unsigned int quota)
+{
+	unsigned int out, in;
+	int seg = 0;
+	int headcount = 0;
+	unsigned d;
+	int ret;
+	/*
+	 * len is always initialized before use since we are always called with
+	 * datalen > 0.
+	 */
+	u32 uninitialized_var(len);
+
+	while (datalen > 0 && headcount < quota) {
+		if (unlikely(seg >= UIO_MAXIOV)) {
+			ret = -ENOBUFS;
+			goto err;
+		}
+
+		ret = vhost_get_vq_desc(vq, vq->iov + seg,
+				ARRAY_SIZE(vq->iov) - seg, &out,
+				&in, NULL, NULL);
+		if (unlikely(ret < 0))
+			goto err;
+
+		d = ret;
+		if (d == vq->num) {
+			ret = 0;
+			goto err;
+		}
+
+		if (unlikely(out || in <= 0)) {
+			vq_err(vq, "unexpected descriptor format for RX: "
+					"out %d, in %d\n", out, in);
+			ret = -EINVAL;
+			goto err;
+		}
+
+		heads[headcount].id = cpu_to_vhost32(vq, d);
+		len = iov_length(vq->iov + seg, in);
+		heads[headcount].len = cpu_to_vhost32(vq, len);
+		datalen -= len;
+		++headcount;
+		seg += in;
+	}
+
+	heads[headcount - 1].len = cpu_to_vhost32(vq, len + datalen);
+	*iovcount = seg;
+

drivers/vhost/vsock.c static struct vhost_vsock *vhost_vsock_get(u32 guest_cid) · line 144

Quoted 10 lines
drivers/vhost/vsock.c +10 -0
+	/* Detect overrun */
+	if (unlikely(datalen > 0)) {
+		ret = UIO_MAXIOV + 1;
+		goto err;
+	}
+	return headcount;
+err:
+	vhost_discard_vq_desc(vq, headcount);
+	return ret;
+}

Seems duplicated with the one used by vhost-net.

In packed virtqueue implementation, I plan to move this to vhost.c.

Quoted 103 lines
drivers/vhost/vsock.c +32 -19
+
  static void
  vhost_transport_do_send_pkt(struct vhost_vsock *vsock,
  			    struct vhost_virtqueue *vq)
@@ -87,22 +150,34 @@ static struct vhost_vsock *vhost_vsock_get(u32 guest_cid)
  	struct vhost_virtqueue *tx_vq = &vsock->vqs[VSOCK_VQ_TX];
  	bool added = false;
  	bool restart_tx = false;
+	int mergeable;
+	size_t vsock_hlen;
 
  	mutex_lock(&vq->mutex);
 
  	if (!vq->private_data)
  		goto out;
 
+	mergeable = vhost_has_feature(vq, VIRTIO_VSOCK_F_MRG_RXBUF);
+	/*
+	 * Guest fill page for rx vq in mergeable case, so it will not
+	 * allocate pkt structure, we should reserve size of pkt in advance.
+	 */
+	if (likely(mergeable))
+		vsock_hlen = sizeof(struct virtio_vsock_pkt);
+	else
+		vsock_hlen = sizeof(struct virtio_vsock_hdr);
+
  	/* Avoid further vmexits, we're already processing the virtqueue */
  	vhost_disable_notify(&vsock->dev, vq);
 
  	for (;;) {
  		struct virtio_vsock_pkt *pkt;
  		struct iov_iter iov_iter;
-		unsigned out, in;
+		unsigned out = 0, in = 0;
  		size_t nbytes;
  		size_t len;
-		int head;
+		s16 headcount;
 
  		spin_lock_bh(&vsock->send_pkt_list_lock);
  		if (list_empty(&vsock->send_pkt_list)) {
@@ -116,16 +191,9 @@ static struct vhost_vsock *vhost_vsock_get(u32 guest_cid)
  		list_del_init(&pkt->list);
  		spin_unlock_bh(&vsock->send_pkt_list_lock);
 
-		head = vhost_get_vq_desc(vq, vq->iov, ARRAY_SIZE(vq->iov),
-					 &out, &in, NULL, NULL);
-		if (head < 0) {
-			spin_lock_bh(&vsock->send_pkt_list_lock);
-			list_add(&pkt->list, &vsock->send_pkt_list);
-			spin_unlock_bh(&vsock->send_pkt_list_lock);
-			break;
-		}
-
-		if (head == vq->num) {
+		headcount = get_rx_bufs(vq, vq->heads, vsock_hlen + pkt->len,
+				&in, likely(mergeable) ? UIO_MAXIOV : 1);
+		if (headcount <= 0) {
  			spin_lock_bh(&vsock->send_pkt_list_lock);
  			list_add(&pkt->list, &vsock->send_pkt_list);
  			spin_unlock_bh(&vsock->send_pkt_list_lock);
@@ -133,19 +201,13 @@ static struct vhost_vsock *vhost_vsock_get(u32 guest_cid)
  			/* We cannot finish yet if more buffers snuck in while
  			 * re-enabling notify.
  			 */
-			if (unlikely(vhost_enable_notify(&vsock->dev, vq))) {
+			if (!headcount && unlikely(vhost_enable_notify(&vsock->dev, vq))) {
  				vhost_disable_notify(&vsock->dev, vq);
  				continue;
  			}
  			break;
  		}
 
-		if (out) {
-			virtio_transport_free_pkt(pkt);
-			vq_err(vq, "Expected 0 output buffers, got %u\n", out);
-			break;
-		}
-
  		len = iov_length(&vq->iov[out], in);
  		iov_iter_init(&iov_iter, READ, &vq->iov[out], in, len);
 
@@ -156,6 +218,19 @@ static struct vhost_vsock *vhost_vsock_get(u32 guest_cid)
  			break;
  		}
 
+		if (likely(mergeable)) {
+			pkt->mrg_rxbuf_hdr.num_buffers = cpu_to_le16(headcount);
+			nbytes = copy_to_iter(&pkt->mrg_rxbuf_hdr,
+					sizeof(pkt->mrg_rxbuf_hdr), &iov_iter);
+			if (nbytes != sizeof(pkt->mrg_rxbuf_hdr)) {
+				virtio_transport_free_pkt(pkt);
+				vq_err(vq, "Faulted on copying rxbuf hdr\n");
+				break;
+			}
+			iov_iter_advance(&iov_iter, (vsock_hlen -
+					sizeof(pkt->mrg_rxbuf_hdr) - sizeof(pkt->hdr)));
+		}
+
  		nbytes = copy_to_iter(pkt->buf, pkt->len, &iov_iter);
  		if (nbytes != pkt->len) {
  			virtio_transport_free_pkt(pkt);

drivers/vhost/vsock.c static struct vhost_vsock *vhost_vsock_get(u32 guest_cid) · line 242

Quoted 6 lines
drivers/vhost/vsock.c +1 -1
@@ -163,7 +238,7 @@ static struct vhost_vsock *vhost_vsock_get(u32 guest_cid)
  			break;
  		}
 
-		vhost_add_used(vq, head, sizeof(pkt->hdr) + pkt->len);
+		vhost_add_used_n(vq, vq->heads, headcount);
  		added = true;

Looks rather similar to vhost-net mergeable rx buffer implementation. Another proof of using vhost-net.

Thanks

Quoted 30 lines
drivers/vhost/vsock.c +0 -0
  		if (pkt->reply) {
include/linux/virtio_vsock.h +1 -0
index bf84418..da9e1fe 100644
@@ -50,6 +50,7 @@ struct virtio_vsock_sock {
 
  struct virtio_vsock_pkt {
  	struct virtio_vsock_hdr	hdr;
+	struct virtio_vsock_mrg_rxbuf_hdr mrg_rxbuf_hdr;
  	struct work_struct work;
  	struct list_head list;
  	/* socket refcnt not held, only use for cancellation */
include/uapi/linux/virtio_vsock.h +5 -0
index 1d57ed3..2292f30 100644
@@ -63,6 +63,11 @@ struct virtio_vsock_hdr {
  	__le32	fwd_cnt;
  } __attribute__((packed));
 
+/* It add mergeable rx buffers feature */
+struct virtio_vsock_mrg_rxbuf_hdr {
+	__le16  num_buffers;    /* number of mergeable rx buffers */
+} __attribute__((packed));
+
  enum virtio_vsock_type {
  	VIRTIO_VSOCK_TYPE_STREAM = 1,
  };
author
#2 lore raw
Quoted 95 lines · Jason Wang
Quoted 79 lines · jiangyiwen

When vhost support VIRTIO_VSOCK_F_MRG_RXBUF feature, it will merge big packet into rx vq.

---
  drivers/vhost/vsock.c             | 117 +++++++++++++++++++++++++++++++-------
  include/linux/virtio_vsock.h      |   1 +
  include/uapi/linux/virtio_vsock.h |   5 ++
  3 files changed, 102 insertions(+), 21 deletions(-)
drivers/vhost/vsock.c +53 -1
index 34bc3ab..648be39 100644
@@ -22,7 +22,8 @@
  #define VHOST_VSOCK_DEFAULT_HOST_CID    2
 
  enum {
-    VHOST_VSOCK_FEATURES = VHOST_FEATURES,
+    VHOST_VSOCK_FEATURES = VHOST_FEATURES |
+            (1ULL << VIRTIO_VSOCK_F_MRG_RXBUF),
  };
 
  /* Used to track all the vhost_vsock instances on the system. */
@@ -80,6 +81,68 @@ static struct vhost_vsock *vhost_vsock_get(u32 guest_cid)
      return vsock;
  }
 
+static int get_rx_bufs(struct vhost_virtqueue *vq,
+        struct vring_used_elem *heads, int datalen,
+        unsigned *iovcount, unsigned int quota)
+{
+    unsigned int out, in;
+    int seg = 0;
+    int headcount = 0;
+    unsigned d;
+    int ret;
+    /*
+     * len is always initialized before use since we are always called with
+     * datalen > 0.
+     */
+    u32 uninitialized_var(len);
+
+    while (datalen > 0 && headcount < quota) {
+        if (unlikely(seg >= UIO_MAXIOV)) {
+            ret = -ENOBUFS;
+            goto err;
+        }
+
+        ret = vhost_get_vq_desc(vq, vq->iov + seg,
+                ARRAY_SIZE(vq->iov) - seg, &out,
+                &in, NULL, NULL);
+        if (unlikely(ret < 0))
+            goto err;
+
+        d = ret;
+        if (d == vq->num) {
+            ret = 0;
+            goto err;
+        }
+
+        if (unlikely(out || in <= 0)) {
+            vq_err(vq, "unexpected descriptor format for RX: "
+                    "out %d, in %d\n", out, in);
+            ret = -EINVAL;
+            goto err;
+        }
+
+        heads[headcount].id = cpu_to_vhost32(vq, d);
+        len = iov_length(vq->iov + seg, in);
+        heads[headcount].len = cpu_to_vhost32(vq, len);
+        datalen -= len;
+        ++headcount;
+        seg += in;
+    }
+
+    heads[headcount - 1].len = cpu_to_vhost32(vq, len + datalen);
+    *iovcount = seg;
+

drivers/vhost/vsock.c static struct vhost_vsock *vhost_vsock_get(u32 guest_cid) · line 144

Quoted 10 lines
drivers/vhost/vsock.c +10 -0
+    /* Detect overrun */
+    if (unlikely(datalen > 0)) {
+        ret = UIO_MAXIOV + 1;
+        goto err;
+    }
+    return headcount;
+err:
+    vhost_discard_vq_desc(vq, headcount);
+    return ret;
+}

Seems duplicated with the one used by vhost-net.

Quoted 2 lines

In packed virtqueue implementation, I plan to move this to vhost.c.

Yes, this code is full copied from vhost-net, if it can be packed into vhost.c, it would be great.

Quoted 107 lines
Quoted 107 lines
drivers/vhost/vsock.c +32 -19
+
  static void
  vhost_transport_do_send_pkt(struct vhost_vsock *vsock,
                  struct vhost_virtqueue *vq)
@@ -87,22 +150,34 @@ static struct vhost_vsock *vhost_vsock_get(u32 guest_cid)
      struct vhost_virtqueue *tx_vq = &vsock->vqs[VSOCK_VQ_TX];
      bool added = false;
      bool restart_tx = false;
+    int mergeable;
+    size_t vsock_hlen;
 
      mutex_lock(&vq->mutex);
 
      if (!vq->private_data)
          goto out;
 
+    mergeable = vhost_has_feature(vq, VIRTIO_VSOCK_F_MRG_RXBUF);
+    /*
+     * Guest fill page for rx vq in mergeable case, so it will not
+     * allocate pkt structure, we should reserve size of pkt in advance.
+     */
+    if (likely(mergeable))
+        vsock_hlen = sizeof(struct virtio_vsock_pkt);
+    else
+        vsock_hlen = sizeof(struct virtio_vsock_hdr);
+
      /* Avoid further vmexits, we're already processing the virtqueue */
      vhost_disable_notify(&vsock->dev, vq);
 
      for (;;) {
          struct virtio_vsock_pkt *pkt;
          struct iov_iter iov_iter;
-        unsigned out, in;
+        unsigned out = 0, in = 0;
          size_t nbytes;
          size_t len;
-        int head;
+        s16 headcount;
 
          spin_lock_bh(&vsock->send_pkt_list_lock);
          if (list_empty(&vsock->send_pkt_list)) {
@@ -116,16 +191,9 @@ static struct vhost_vsock *vhost_vsock_get(u32 guest_cid)
          list_del_init(&pkt->list);
          spin_unlock_bh(&vsock->send_pkt_list_lock);
 
-        head = vhost_get_vq_desc(vq, vq->iov, ARRAY_SIZE(vq->iov),
-                     &out, &in, NULL, NULL);
-        if (head < 0) {
-            spin_lock_bh(&vsock->send_pkt_list_lock);
-            list_add(&pkt->list, &vsock->send_pkt_list);
-            spin_unlock_bh(&vsock->send_pkt_list_lock);
-            break;
-        }
-
-        if (head == vq->num) {
+        headcount = get_rx_bufs(vq, vq->heads, vsock_hlen + pkt->len,
+                &in, likely(mergeable) ? UIO_MAXIOV : 1);
+        if (headcount <= 0) {
              spin_lock_bh(&vsock->send_pkt_list_lock);
              list_add(&pkt->list, &vsock->send_pkt_list);
              spin_unlock_bh(&vsock->send_pkt_list_lock);
@@ -133,19 +201,13 @@ static struct vhost_vsock *vhost_vsock_get(u32 guest_cid)
              /* We cannot finish yet if more buffers snuck in while
               * re-enabling notify.
               */
-            if (unlikely(vhost_enable_notify(&vsock->dev, vq))) {
+            if (!headcount && unlikely(vhost_enable_notify(&vsock->dev, vq))) {
                  vhost_disable_notify(&vsock->dev, vq);
                  continue;
              }
              break;
          }
 
-        if (out) {
-            virtio_transport_free_pkt(pkt);
-            vq_err(vq, "Expected 0 output buffers, got %u\n", out);
-            break;
-        }
-
          len = iov_length(&vq->iov[out], in);
          iov_iter_init(&iov_iter, READ, &vq->iov[out], in, len);
 
@@ -156,6 +218,19 @@ static struct vhost_vsock *vhost_vsock_get(u32 guest_cid)
              break;
          }
 
+        if (likely(mergeable)) {
+            pkt->mrg_rxbuf_hdr.num_buffers = cpu_to_le16(headcount);
+            nbytes = copy_to_iter(&pkt->mrg_rxbuf_hdr,
+                    sizeof(pkt->mrg_rxbuf_hdr), &iov_iter);
+            if (nbytes != sizeof(pkt->mrg_rxbuf_hdr)) {
+                virtio_transport_free_pkt(pkt);
+                vq_err(vq, "Faulted on copying rxbuf hdr\n");
+                break;
+            }
+            iov_iter_advance(&iov_iter, (vsock_hlen -
+                    sizeof(pkt->mrg_rxbuf_hdr) - sizeof(pkt->hdr)));
+        }
+
          nbytes = copy_to_iter(pkt->buf, pkt->len, &iov_iter);
          if (nbytes != pkt->len) {
              virtio_transport_free_pkt(pkt);
@@ -163,7 +238,7 @@ static struct vhost_vsock *vhost_vsock_get(u32 guest_cid)
              break;
          }
 
Quoted 8 lines

drivers/vhost/vsock.c static struct vhost_vsock *vhost_vsock_get(u32 guest_cid) · line 242

Quoted 3 lines
drivers/vhost/vsock.c +1 -1
-        vhost_add_used(vq, head, sizeof(pkt->hdr) + pkt->len);
+        vhost_add_used_n(vq, vq->heads, headcount);
          added = true;

Looks rather similar to vhost-net mergeable rx buffer implementation. Another proof of using vhost-net.

Thanks

Yes.

Quoted 35 lines
Quoted 22 lines
drivers/vhost/vsock.c +0 -0
          if (pkt->reply) {
include/linux/virtio_vsock.h +1 -0
index bf84418..da9e1fe 100644
@@ -50,6 +50,7 @@ struct virtio_vsock_sock {
 
  struct virtio_vsock_pkt {
      struct virtio_vsock_hdr    hdr;
+    struct virtio_vsock_mrg_rxbuf_hdr mrg_rxbuf_hdr;
      struct work_struct work;
      struct list_head list;
      /* socket refcnt not held, only use for cancellation */
include/uapi/linux/virtio_vsock.h +0 -0
index 1d57ed3..2292f30 100644
@@ -63,6 +63,11 @@ struct virtio_vsock_hdr {
      __le32    fwd_cnt;

include/uapi/linux/virtio_vsock.h struct virtio_vsock_hdr { · line 73

Quoted 10 lines
include/uapi/linux/virtio_vsock.h +5 -0
  } __attribute__((packed));
 
+/* It add mergeable rx buffers feature */
+struct virtio_vsock_mrg_rxbuf_hdr {
+    __le16  num_buffers;    /* number of mergeable rx buffers */
+} __attribute__((packed));
+
  enum virtio_vsock_type {
      VIRTIO_VSOCK_TYPE_STREAM = 1,
  };

.

#3 lore raw
Quoted 7 lines · jiangyiwen
Quoted 4 lines

Seems duplicated with the one used by vhost-net.

In packed virtqueue implementation, I plan to move this to vhost.c.

Yes, this code is full copied from vhost-net, if it can be packed into vhost.c, it would be great.

If you try to reuse vhost-net, you don't even need to care about this :)

Thanks

author
#4 lore raw
Quoted 10 lines · Jason Wang
Quoted 7 lines · jiangyiwen
Quoted 4 lines

Seems duplicated with the one used by vhost-net.

In packed virtqueue implementation, I plan to move this to vhost.c.

Yes, this code is full copied from vhost-net, if it can be packed into vhost.c, it would be great.

Quoted 7 lines

If you try to reuse vhost-net, you don't even need to care about this :)

Thanks

.

Hi Jason,

Thank your advice, I will consider your idea. But I don't know what's stefan's suggestion? It seems that he doesn't care much about this community. :(

I still hope this community can have some vitality.

#5 lore raw
Quoted 14 lines · jiangyiwen
Quoted 13 lines · Jason Wang
Quoted 7 lines · jiangyiwen
Quoted 4 lines

Seems duplicated with the one used by vhost-net.

In packed virtqueue implementation, I plan to move this to vhost.c.

Yes, this code is full copied from vhost-net, if it can be packed into vhost.c, it would be great.

If you try to reuse vhost-net, you don't even need to care about this:)

Thanks

Quoted 7 lines
Quoted 2 lines

.

Hi Jason,

Thank your advice, I will consider your idea. But I don't know what's stefan's suggestion? It seems that he doesn't care much about this community.:(

I think not. He is probably busy these days.

Quoted 3 lines

I still hope this community can have some vitality.

Let's wait for few more days for the possible comments from Stefan or Michael. But I do prefer to unify the virtio networking datapath which will be easier to be extended and maintained.

Thanks

author
#6 lore raw
Quoted 32 lines · Jason Wang
Quoted 14 lines · jiangyiwen
Quoted 13 lines · Jason Wang
Quoted 7 lines · jiangyiwen
Quoted 4 lines

Seems duplicated with the one used by vhost-net.

In packed virtqueue implementation, I plan to move this to vhost.c.

Yes, this code is full copied from vhost-net, if it can be packed into vhost.c, it would be great.

If you try to reuse vhost-net, you don't even need to care about this:)

Thanks

Quoted 7 lines
Quoted 2 lines

.

Hi Jason,

Thank your advice, I will consider your idea. But I don't know what's stefan's suggestion? It seems that he doesn't care much about this community.:(

I think not. He is probably busy these days.

Quoted 3 lines

I still hope this community can have some vitality.

Quoted 7 lines

Let's wait for few more days for the possible comments from Stefan or Michael. But I do prefer to unify the virtio networking datapath which will be easier to be extended and maintained.

Thanks

.

Hi Jason,

Actually vsock use virtio-net as transport path should be a better idea, I will try to consider the new idea.

Thanks,
Yiwen.

#7 lore raw
Quoted 35 lines · jiangyiwen
Quoted 25 lines · Jason Wang
Quoted 14 lines · jiangyiwen
Quoted 13 lines · Jason Wang
Quoted 7 lines · jiangyiwen
Quoted 4 lines

Seems duplicated with the one used by vhost-net.

In packed virtqueue implementation, I plan to move this to vhost.c.

Yes, this code is full copied from vhost-net, if it can be packed into vhost.c, it would be great.

If you try to reuse vhost-net, you don't even need to care about this:)

Thanks

Quoted 7 lines
Quoted 2 lines

.

Hi Jason,

Thank your advice, I will consider your idea. But I don't know what's stefan's suggestion? It seems that he doesn't care much about this community.:(

I think not. He is probably busy these days.

Quoted 9 lines
Quoted 2 lines

I still hope this community can have some vitality.

Let's wait for few more days for the possible comments from Stefan or Michael. But I do prefer to unify the virtio networking datapath which will be easier to be extended and maintained.

Thanks

.

Quoted 8 lines

Hi Jason,

Actually vsock use virtio-net as transport path should be a better idea, I will try to consider the new idea.

Thanks,
Yiwen.

Good to know this and thanks!

#8 lore raw
In reply to jiangyiwen
Quoted 35 lines · jiangyiwen
Quoted 25 lines · Jason Wang
Quoted 14 lines · jiangyiwen
Quoted 13 lines · Jason Wang
Quoted 7 lines · jiangyiwen
Quoted 4 lines

Seems duplicated with the one used by vhost-net.

In packed virtqueue implementation, I plan to move this to vhost.c.

Yes, this code is full copied from vhost-net, if it can be packed into vhost.c, it would be great.

If you try to reuse vhost-net, you don't even need to care about this:)

Thanks

Quoted 7 lines
Quoted 2 lines

.

Hi Jason,

Thank your advice, I will consider your idea. But I don't know what's stefan's suggestion? It seems that he doesn't care much about this community.:(

I think not. He is probably busy these days.

Quoted 9 lines
Quoted 2 lines

I still hope this community can have some vitality.

Let's wait for few more days for the possible comments from Stefan or Michael. But I do prefer to unify the virtio networking datapath which will be easier to be extended and maintained.

Thanks

.

Quoted 8 lines

Hi Jason,

Actually vsock use virtio-net as transport path should be a better idea, I will try to consider the new idea.

Thanks,
Yiwen.

Good to know this and thanks!

_______________________________________________
Virtualization mailing list
[email protected]
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

author
#9 lore raw
In reply to Jason Wang
Quoted 32 lines · Jason Wang
Quoted 14 lines · jiangyiwen
Quoted 13 lines · Jason Wang
Quoted 7 lines · jiangyiwen
Quoted 4 lines

Seems duplicated with the one used by vhost-net.

In packed virtqueue implementation, I plan to move this to vhost.c.

Yes, this code is full copied from vhost-net, if it can be packed into vhost.c, it would be great.

If you try to reuse vhost-net, you don't even need to care about this:)

Thanks

Quoted 7 lines
Quoted 2 lines

.

Hi Jason,

Thank your advice, I will consider your idea. But I don't know what's stefan's suggestion? It seems that he doesn't care much about this community.:(

I think not. He is probably busy these days.

Quoted 3 lines

I still hope this community can have some vitality.

Quoted 7 lines

Let's wait for few more days for the possible comments from Stefan or Michael. But I do prefer to unify the virtio networking datapath which will be easier to be extended and maintained.

Thanks

.

Hi Jason,

Actually vsock use virtio-net as transport path should be a better idea, I will try to consider the new idea.

Thanks,
Yiwen.

_______________________________________________
Virtualization mailing list
[email protected]
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

#10 lore raw
In reply to jiangyiwen
Quoted 14 lines · jiangyiwen
Quoted 13 lines · Jason Wang
Quoted 7 lines · jiangyiwen
Quoted 4 lines

Seems duplicated with the one used by vhost-net.

In packed virtqueue implementation, I plan to move this to vhost.c.

Yes, this code is full copied from vhost-net, if it can be packed into vhost.c, it would be great.

If you try to reuse vhost-net, you don't even need to care about this:)

Thanks

Quoted 7 lines
Quoted 2 lines

.

Hi Jason,

Thank your advice, I will consider your idea. But I don't know what's stefan's suggestion? It seems that he doesn't care much about this community.:(

I think not. He is probably busy these days.

Quoted 3 lines

I still hope this community can have some vitality.

Let's wait for few more days for the possible comments from Stefan or Michael. But I do prefer to unify the virtio networking datapath which will be easier to be extended and maintained.

Thanks

_______________________________________________
Virtualization mailing list
[email protected]
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

author
#11 lore raw
In reply to Jason Wang
Quoted 10 lines · Jason Wang
Quoted 7 lines · jiangyiwen
Quoted 4 lines

Seems duplicated with the one used by vhost-net.

In packed virtqueue implementation, I plan to move this to vhost.c.

Yes, this code is full copied from vhost-net, if it can be packed into vhost.c, it would be great.

Quoted 7 lines

If you try to reuse vhost-net, you don't even need to care about this :)

Thanks

.

Hi Jason,

Thank your advice, I will consider your idea. But I don't know what's stefan's suggestion? It seems that he doesn't care much about this community. :(

I still hope this community can have some vitality.

_______________________________________________
Virtualization mailing list
[email protected]
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

#12 lore raw
In reply to jiangyiwen
Quoted 7 lines · jiangyiwen
Quoted 4 lines

Seems duplicated with the one used by vhost-net.

In packed virtqueue implementation, I plan to move this to vhost.c.

Yes, this code is full copied from vhost-net, if it can be packed into vhost.c, it would be great.

If you try to reuse vhost-net, you don't even need to care about this :)

Thanks

_______________________________________________
Virtualization mailing list
[email protected]
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

author
#13 lore raw
In reply to Jason Wang
Quoted 95 lines · Jason Wang
Quoted 79 lines · jiangyiwen

When vhost support VIRTIO_VSOCK_F_MRG_RXBUF feature, it will merge big packet into rx vq.

---
  drivers/vhost/vsock.c             | 117 +++++++++++++++++++++++++++++++-------
  include/linux/virtio_vsock.h      |   1 +
  include/uapi/linux/virtio_vsock.h |   5 ++
  3 files changed, 102 insertions(+), 21 deletions(-)
drivers/vhost/vsock.c +53 -1
index 34bc3ab..648be39 100644
@@ -22,7 +22,8 @@
  #define VHOST_VSOCK_DEFAULT_HOST_CID    2
 
  enum {
-    VHOST_VSOCK_FEATURES = VHOST_FEATURES,
+    VHOST_VSOCK_FEATURES = VHOST_FEATURES |
+            (1ULL << VIRTIO_VSOCK_F_MRG_RXBUF),
  };
 
  /* Used to track all the vhost_vsock instances on the system. */
@@ -80,6 +81,68 @@ static struct vhost_vsock *vhost_vsock_get(u32 guest_cid)
      return vsock;
  }
 
+static int get_rx_bufs(struct vhost_virtqueue *vq,
+        struct vring_used_elem *heads, int datalen,
+        unsigned *iovcount, unsigned int quota)
+{
+    unsigned int out, in;
+    int seg = 0;
+    int headcount = 0;
+    unsigned d;
+    int ret;
+    /*
+     * len is always initialized before use since we are always called with
+     * datalen > 0.
+     */
+    u32 uninitialized_var(len);
+
+    while (datalen > 0 && headcount < quota) {
+        if (unlikely(seg >= UIO_MAXIOV)) {
+            ret = -ENOBUFS;
+            goto err;
+        }
+
+        ret = vhost_get_vq_desc(vq, vq->iov + seg,
+                ARRAY_SIZE(vq->iov) - seg, &out,
+                &in, NULL, NULL);
+        if (unlikely(ret < 0))
+            goto err;
+
+        d = ret;
+        if (d == vq->num) {
+            ret = 0;
+            goto err;
+        }
+
+        if (unlikely(out || in <= 0)) {
+            vq_err(vq, "unexpected descriptor format for RX: "
+                    "out %d, in %d\n", out, in);
+            ret = -EINVAL;
+            goto err;
+        }
+
+        heads[headcount].id = cpu_to_vhost32(vq, d);
+        len = iov_length(vq->iov + seg, in);
+        heads[headcount].len = cpu_to_vhost32(vq, len);
+        datalen -= len;
+        ++headcount;
+        seg += in;
+    }
+
+    heads[headcount - 1].len = cpu_to_vhost32(vq, len + datalen);
+    *iovcount = seg;
+

drivers/vhost/vsock.c static struct vhost_vsock *vhost_vsock_get(u32 guest_cid) · line 144

Quoted 10 lines
drivers/vhost/vsock.c +10 -0
+    /* Detect overrun */
+    if (unlikely(datalen > 0)) {
+        ret = UIO_MAXIOV + 1;
+        goto err;
+    }
+    return headcount;
+err:
+    vhost_discard_vq_desc(vq, headcount);
+    return ret;
+}

Seems duplicated with the one used by vhost-net.

Quoted 2 lines

In packed virtqueue implementation, I plan to move this to vhost.c.

Yes, this code is full copied from vhost-net, if it can be packed into vhost.c, it would be great.

Quoted 107 lines
Quoted 107 lines
drivers/vhost/vsock.c +32 -19
+
  static void
  vhost_transport_do_send_pkt(struct vhost_vsock *vsock,
                  struct vhost_virtqueue *vq)
@@ -87,22 +150,34 @@ static struct vhost_vsock *vhost_vsock_get(u32 guest_cid)
      struct vhost_virtqueue *tx_vq = &vsock->vqs[VSOCK_VQ_TX];
      bool added = false;
      bool restart_tx = false;
+    int mergeable;
+    size_t vsock_hlen;
 
      mutex_lock(&vq->mutex);
 
      if (!vq->private_data)
          goto out;
 
+    mergeable = vhost_has_feature(vq, VIRTIO_VSOCK_F_MRG_RXBUF);
+    /*
+     * Guest fill page for rx vq in mergeable case, so it will not
+     * allocate pkt structure, we should reserve size of pkt in advance.
+     */
+    if (likely(mergeable))
+        vsock_hlen = sizeof(struct virtio_vsock_pkt);
+    else
+        vsock_hlen = sizeof(struct virtio_vsock_hdr);
+
      /* Avoid further vmexits, we're already processing the virtqueue */
      vhost_disable_notify(&vsock->dev, vq);
 
      for (;;) {
          struct virtio_vsock_pkt *pkt;
          struct iov_iter iov_iter;
-        unsigned out, in;
+        unsigned out = 0, in = 0;
          size_t nbytes;
          size_t len;
-        int head;
+        s16 headcount;
 
          spin_lock_bh(&vsock->send_pkt_list_lock);
          if (list_empty(&vsock->send_pkt_list)) {
@@ -116,16 +191,9 @@ static struct vhost_vsock *vhost_vsock_get(u32 guest_cid)
          list_del_init(&pkt->list);
          spin_unlock_bh(&vsock->send_pkt_list_lock);
 
-        head = vhost_get_vq_desc(vq, vq->iov, ARRAY_SIZE(vq->iov),
-                     &out, &in, NULL, NULL);
-        if (head < 0) {
-            spin_lock_bh(&vsock->send_pkt_list_lock);
-            list_add(&pkt->list, &vsock->send_pkt_list);
-            spin_unlock_bh(&vsock->send_pkt_list_lock);
-            break;
-        }
-
-        if (head == vq->num) {
+        headcount = get_rx_bufs(vq, vq->heads, vsock_hlen + pkt->len,
+                &in, likely(mergeable) ? UIO_MAXIOV : 1);
+        if (headcount <= 0) {
              spin_lock_bh(&vsock->send_pkt_list_lock);
              list_add(&pkt->list, &vsock->send_pkt_list);
              spin_unlock_bh(&vsock->send_pkt_list_lock);
@@ -133,19 +201,13 @@ static struct vhost_vsock *vhost_vsock_get(u32 guest_cid)
              /* We cannot finish yet if more buffers snuck in while
               * re-enabling notify.
               */
-            if (unlikely(vhost_enable_notify(&vsock->dev, vq))) {
+            if (!headcount && unlikely(vhost_enable_notify(&vsock->dev, vq))) {
                  vhost_disable_notify(&vsock->dev, vq);
                  continue;
              }
              break;
          }
 
-        if (out) {
-            virtio_transport_free_pkt(pkt);
-            vq_err(vq, "Expected 0 output buffers, got %u\n", out);
-            break;
-        }
-
          len = iov_length(&vq->iov[out], in);
          iov_iter_init(&iov_iter, READ, &vq->iov[out], in, len);
 
@@ -156,6 +218,19 @@ static struct vhost_vsock *vhost_vsock_get(u32 guest_cid)
              break;
          }
 
+        if (likely(mergeable)) {
+            pkt->mrg_rxbuf_hdr.num_buffers = cpu_to_le16(headcount);
+            nbytes = copy_to_iter(&pkt->mrg_rxbuf_hdr,
+                    sizeof(pkt->mrg_rxbuf_hdr), &iov_iter);
+            if (nbytes != sizeof(pkt->mrg_rxbuf_hdr)) {
+                virtio_transport_free_pkt(pkt);
+                vq_err(vq, "Faulted on copying rxbuf hdr\n");
+                break;
+            }
+            iov_iter_advance(&iov_iter, (vsock_hlen -
+                    sizeof(pkt->mrg_rxbuf_hdr) - sizeof(pkt->hdr)));
+        }
+
          nbytes = copy_to_iter(pkt->buf, pkt->len, &iov_iter);
          if (nbytes != pkt->len) {
              virtio_transport_free_pkt(pkt);
@@ -163,7 +238,7 @@ static struct vhost_vsock *vhost_vsock_get(u32 guest_cid)
              break;
          }
 
Quoted 8 lines

drivers/vhost/vsock.c static struct vhost_vsock *vhost_vsock_get(u32 guest_cid) · line 242

Quoted 3 lines
drivers/vhost/vsock.c +1 -1
-        vhost_add_used(vq, head, sizeof(pkt->hdr) + pkt->len);
+        vhost_add_used_n(vq, vq->heads, headcount);
          added = true;

Looks rather similar to vhost-net mergeable rx buffer implementation. Another proof of using vhost-net.

Thanks

Yes.

Quoted 24 lines
Quoted 24 lines
drivers/vhost/vsock.c +0 -0
          if (pkt->reply) {
include/linux/virtio_vsock.h +1 -0
index bf84418..da9e1fe 100644
@@ -50,6 +50,7 @@ struct virtio_vsock_sock {
 
  struct virtio_vsock_pkt {
      struct virtio_vsock_hdr    hdr;
+    struct virtio_vsock_mrg_rxbuf_hdr mrg_rxbuf_hdr;
      struct work_struct work;
      struct list_head list;
      /* socket refcnt not held, only use for cancellation */
include/uapi/linux/virtio_vsock.h +0 -0
index 1d57ed3..2292f30 100644
@@ -63,6 +63,11 @@ struct virtio_vsock_hdr {
      __le32    fwd_cnt;
  } __attribute__((packed));
 
Quoted 11 lines

include/uapi/linux/virtio_vsock.h struct virtio_vsock_hdr { · line 73

Quoted 8 lines
include/uapi/linux/virtio_vsock.h +5 -0
+/* It add mergeable rx buffers feature */
+struct virtio_vsock_mrg_rxbuf_hdr {
+    __le16  num_buffers;    /* number of mergeable rx buffers */
+} __attribute__((packed));
+
  enum virtio_vsock_type {
      VIRTIO_VSOCK_TYPE_STREAM = 1,
  };

.

_______________________________________________
Virtualization mailing list
[email protected]
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

#14 lore raw
In reply to jiangyiwen
Quoted 79 lines · jiangyiwen

When vhost support VIRTIO_VSOCK_F_MRG_RXBUF feature, it will merge big packet into rx vq.

---
  drivers/vhost/vsock.c             | 117 +++++++++++++++++++++++++++++++-------
  include/linux/virtio_vsock.h      |   1 +
  include/uapi/linux/virtio_vsock.h |   5 ++
  3 files changed, 102 insertions(+), 21 deletions(-)
drivers/vhost/vsock.c +53 -1
index 34bc3ab..648be39 100644
@@ -22,7 +22,8 @@
  #define VHOST_VSOCK_DEFAULT_HOST_CID	2
 
  enum {
-	VHOST_VSOCK_FEATURES = VHOST_FEATURES,
+	VHOST_VSOCK_FEATURES = VHOST_FEATURES |
+			(1ULL << VIRTIO_VSOCK_F_MRG_RXBUF),
  };
 
  /* Used to track all the vhost_vsock instances on the system. */
@@ -80,6 +81,68 @@ static struct vhost_vsock *vhost_vsock_get(u32 guest_cid)
  	return vsock;
  }
 
+static int get_rx_bufs(struct vhost_virtqueue *vq,
+		struct vring_used_elem *heads, int datalen,
+		unsigned *iovcount, unsigned int quota)
+{
+	unsigned int out, in;
+	int seg = 0;
+	int headcount = 0;
+	unsigned d;
+	int ret;
+	/*
+	 * len is always initialized before use since we are always called with
+	 * datalen > 0.
+	 */
+	u32 uninitialized_var(len);
+
+	while (datalen > 0 && headcount < quota) {
+		if (unlikely(seg >= UIO_MAXIOV)) {
+			ret = -ENOBUFS;
+			goto err;
+		}
+
+		ret = vhost_get_vq_desc(vq, vq->iov + seg,
+				ARRAY_SIZE(vq->iov) - seg, &out,
+				&in, NULL, NULL);
+		if (unlikely(ret < 0))
+			goto err;
+
+		d = ret;
+		if (d == vq->num) {
+			ret = 0;
+			goto err;
+		}
+
+		if (unlikely(out || in <= 0)) {
+			vq_err(vq, "unexpected descriptor format for RX: "
+					"out %d, in %d\n", out, in);
+			ret = -EINVAL;
+			goto err;
+		}
+
+		heads[headcount].id = cpu_to_vhost32(vq, d);
+		len = iov_length(vq->iov + seg, in);
+		heads[headcount].len = cpu_to_vhost32(vq, len);
+		datalen -= len;
+		++headcount;
+		seg += in;
+	}
+
+	heads[headcount - 1].len = cpu_to_vhost32(vq, len + datalen);
+	*iovcount = seg;
+

drivers/vhost/vsock.c static struct vhost_vsock *vhost_vsock_get(u32 guest_cid) · line 144

Quoted 10 lines
drivers/vhost/vsock.c +10 -0
+	/* Detect overrun */
+	if (unlikely(datalen > 0)) {
+		ret = UIO_MAXIOV + 1;
+		goto err;
+	}
+	return headcount;
+err:
+	vhost_discard_vq_desc(vq, headcount);
+	return ret;
+}

Seems duplicated with the one used by vhost-net.

In packed virtqueue implementation, I plan to move this to vhost.c.

Quoted 103 lines
drivers/vhost/vsock.c +32 -19
+
  static void
  vhost_transport_do_send_pkt(struct vhost_vsock *vsock,
  			    struct vhost_virtqueue *vq)
@@ -87,22 +150,34 @@ static struct vhost_vsock *vhost_vsock_get(u32 guest_cid)
  	struct vhost_virtqueue *tx_vq = &vsock->vqs[VSOCK_VQ_TX];
  	bool added = false;
  	bool restart_tx = false;
+	int mergeable;
+	size_t vsock_hlen;
 
  	mutex_lock(&vq->mutex);
 
  	if (!vq->private_data)
  		goto out;
 
+	mergeable = vhost_has_feature(vq, VIRTIO_VSOCK_F_MRG_RXBUF);
+	/*
+	 * Guest fill page for rx vq in mergeable case, so it will not
+	 * allocate pkt structure, we should reserve size of pkt in advance.
+	 */
+	if (likely(mergeable))
+		vsock_hlen = sizeof(struct virtio_vsock_pkt);
+	else
+		vsock_hlen = sizeof(struct virtio_vsock_hdr);
+
  	/* Avoid further vmexits, we're already processing the virtqueue */
  	vhost_disable_notify(&vsock->dev, vq);
 
  	for (;;) {
  		struct virtio_vsock_pkt *pkt;
  		struct iov_iter iov_iter;
-		unsigned out, in;
+		unsigned out = 0, in = 0;
  		size_t nbytes;
  		size_t len;
-		int head;
+		s16 headcount;
 
  		spin_lock_bh(&vsock->send_pkt_list_lock);
  		if (list_empty(&vsock->send_pkt_list)) {
@@ -116,16 +191,9 @@ static struct vhost_vsock *vhost_vsock_get(u32 guest_cid)
  		list_del_init(&pkt->list);
  		spin_unlock_bh(&vsock->send_pkt_list_lock);
 
-		head = vhost_get_vq_desc(vq, vq->iov, ARRAY_SIZE(vq->iov),
-					 &out, &in, NULL, NULL);
-		if (head < 0) {
-			spin_lock_bh(&vsock->send_pkt_list_lock);
-			list_add(&pkt->list, &vsock->send_pkt_list);
-			spin_unlock_bh(&vsock->send_pkt_list_lock);
-			break;
-		}
-
-		if (head == vq->num) {
+		headcount = get_rx_bufs(vq, vq->heads, vsock_hlen + pkt->len,
+				&in, likely(mergeable) ? UIO_MAXIOV : 1);
+		if (headcount <= 0) {
  			spin_lock_bh(&vsock->send_pkt_list_lock);
  			list_add(&pkt->list, &vsock->send_pkt_list);
  			spin_unlock_bh(&vsock->send_pkt_list_lock);
@@ -133,19 +201,13 @@ static struct vhost_vsock *vhost_vsock_get(u32 guest_cid)
  			/* We cannot finish yet if more buffers snuck in while
  			 * re-enabling notify.
  			 */
-			if (unlikely(vhost_enable_notify(&vsock->dev, vq))) {
+			if (!headcount && unlikely(vhost_enable_notify(&vsock->dev, vq))) {
  				vhost_disable_notify(&vsock->dev, vq);
  				continue;
  			}
  			break;
  		}
 
-		if (out) {
-			virtio_transport_free_pkt(pkt);
-			vq_err(vq, "Expected 0 output buffers, got %u\n", out);
-			break;
-		}
-
  		len = iov_length(&vq->iov[out], in);
  		iov_iter_init(&iov_iter, READ, &vq->iov[out], in, len);
 
@@ -156,6 +218,19 @@ static struct vhost_vsock *vhost_vsock_get(u32 guest_cid)
  			break;
  		}
 
+		if (likely(mergeable)) {
+			pkt->mrg_rxbuf_hdr.num_buffers = cpu_to_le16(headcount);
+			nbytes = copy_to_iter(&pkt->mrg_rxbuf_hdr,
+					sizeof(pkt->mrg_rxbuf_hdr), &iov_iter);
+			if (nbytes != sizeof(pkt->mrg_rxbuf_hdr)) {
+				virtio_transport_free_pkt(pkt);
+				vq_err(vq, "Faulted on copying rxbuf hdr\n");
+				break;
+			}
+			iov_iter_advance(&iov_iter, (vsock_hlen -
+					sizeof(pkt->mrg_rxbuf_hdr) - sizeof(pkt->hdr)));
+		}
+
  		nbytes = copy_to_iter(pkt->buf, pkt->len, &iov_iter);
  		if (nbytes != pkt->len) {
  			virtio_transport_free_pkt(pkt);

drivers/vhost/vsock.c static struct vhost_vsock *vhost_vsock_get(u32 guest_cid) · line 242

Quoted 6 lines
drivers/vhost/vsock.c +1 -1
@@ -163,7 +238,7 @@ static struct vhost_vsock *vhost_vsock_get(u32 guest_cid)
  			break;
  		}
 
-		vhost_add_used(vq, head, sizeof(pkt->hdr) + pkt->len);
+		vhost_add_used_n(vq, vq->heads, headcount);
  		added = true;

Looks rather similar to vhost-net mergeable rx buffer implementation. Another proof of using vhost-net.

Thanks

Quoted 20 lines
drivers/vhost/vsock.c +0 -0
  		if (pkt->reply) {
include/linux/virtio_vsock.h +1 -0
index bf84418..da9e1fe 100644
@@ -50,6 +50,7 @@ struct virtio_vsock_sock {
 
  struct virtio_vsock_pkt {
  	struct virtio_vsock_hdr	hdr;
+	struct virtio_vsock_mrg_rxbuf_hdr mrg_rxbuf_hdr;
  	struct work_struct work;
  	struct list_head list;
  	/* socket refcnt not held, only use for cancellation */
include/uapi/linux/virtio_vsock.h +0 -0
index 1d57ed3..2292f30 100644
@@ -63,6 +63,11 @@ struct virtio_vsock_hdr {
  	__le32	fwd_cnt;

include/uapi/linux/virtio_vsock.h struct virtio_vsock_hdr { · line 73

Quoted 10 lines
include/uapi/linux/virtio_vsock.h +5 -0
  } __attribute__((packed));
 
+/* It add mergeable rx buffers feature */
+struct virtio_vsock_mrg_rxbuf_hdr {
+	__le16  num_buffers;    /* number of mergeable rx buffers */
+} __attribute__((packed));
+
  enum virtio_vsock_type {
  	VIRTIO_VSOCK_TYPE_STREAM = 1,
  };

_______________________________________________
Virtualization mailing list
[email protected]
https://lists.linuxfoundation.org/mailman/listinfo/virtualization