[RFC] add support for async packed ring dequeue

[RFC] add support for async packed ring dequeue

[Cheng Jiang]

This RFC patch implements packed ring dequeue data path for asynchronous
vhost. It's based on the RFC patch:
http://patchwork.dpdk.org/project/dpdk/cover/20220310065407.17145-1-xuan.ding@intel.com/

Signed-off-by: Cheng Jiang <cheng1.jiang@intel.com>
---
 lib/vhost/virtio_net.c | 217 ++++++++++++++++++++++++++++++++++++-----
 1 file changed, 191 insertions(+), 26 deletions(-)

diff --git a/lib/vhost/virtio_net.c b/lib/vhost/virtio_net.c
index 3816caca79..4e6ea935c9 100644
--- a/lib/vhost/virtio_net.c
+++ b/lib/vhost/virtio_net.c
@@ -3312,7 +3312,7 @@ async_desc_to_mbuf(struct virtio_net *dev, struct vhost_virtqueue *vq,
 }
 
 static __rte_always_inline uint16_t
-async_poll_dequeue_completed_split(struct virtio_net *dev, uint16_t queue_id,
+async_poll_dequeue_completed(struct virtio_net *dev, uint16_t queue_id,
 		struct rte_mbuf **pkts, uint16_t count, uint16_t dma_id,
 		uint16_t vchan_id, bool legacy_ol_flags)
 {
@@ -3330,7 +3330,7 @@ async_poll_dequeue_completed_split(struct virtio_net *dev, uint16_t queue_id,
 	from = start_idx;
 	while (vq->async->pkts_cmpl_flag[from] && count--) {
 		vq->async->pkts_cmpl_flag[from] = false;
-		from = (from + 1) & (vq->size - 1);
+		from = (from + 1) % vq->size;
 		nr_cpl_pkts++;
 	}
 
@@ -3338,7 +3338,7 @@ async_poll_dequeue_completed_split(struct virtio_net *dev, uint16_t queue_id,
 		return 0;
 
 	for (i = 0; i < nr_cpl_pkts; i++) {
-		from = (start_idx + i) & (vq->size - 1);
+		from = (start_idx + i) % vq->size;
 		pkts[i] = pkts_info[from].mbuf;
 
 		if (virtio_net_with_host_offload(dev))
@@ -3347,10 +3347,14 @@ async_poll_dequeue_completed_split(struct virtio_net *dev, uint16_t queue_id,
 	}
 
 	/* write back completed descs to used ring and update used idx */
-	write_back_completed_descs_split(vq, nr_cpl_pkts);
-	__atomic_add_fetch(&vq->used->idx, nr_cpl_pkts, __ATOMIC_RELEASE);
-	vhost_vring_call_split(dev, vq);
-
+	if (vq_is_packed(dev)) {
+		write_back_completed_descs_packed(vq, nr_cpl_pkts);
+		vhost_vring_call_packed(dev, vq);
+	} else {
+		write_back_completed_descs_split(vq, nr_cpl_pkts);
+		__atomic_add_fetch(&vq->used->idx, nr_cpl_pkts, __ATOMIC_RELEASE);
+		vhost_vring_call_split(dev, vq);
+	}
 	vq->async->pkts_inflight_n -= nr_cpl_pkts;
 
 	return nr_cpl_pkts;
@@ -3486,8 +3490,8 @@ virtio_dev_tx_async_split(struct virtio_net *dev, struct vhost_virtqueue *vq,
 
 out:
 	/* DMA device may serve other queues, unconditionally check completed. */
-	nr_done_pkts = async_poll_dequeue_completed_split(dev, queue_id, pkts, pkts_size,
-							  dma_id, vchan_id, legacy_ol_flags);
+	nr_done_pkts = async_poll_dequeue_completed(dev, queue_id, pkts, pkts_size,
+						dma_id, vchan_id, legacy_ol_flags);
 
 	return nr_done_pkts;
 }
@@ -3514,6 +3518,170 @@ virtio_dev_tx_async_split_compliant(struct virtio_net *dev,
 				pkts, count, dma_id, vchan_id, false);
 }
 
+static __rte_always_inline void
+vhost_async_shadow_dequeue_single_packed(struct vhost_virtqueue *vq, uint16_t buf_id)
+{
+	struct vhost_async *async = vq->async;
+	uint16_t idx = async->buffer_idx_packed;
+
+	async->buffers_packed[idx].id = buf_id;
+	async->buffers_packed[idx].len = 0;
+	async->buffers_packed[idx].count = 1;
+
+	async->buffer_idx_packed++;
+	if (async->buffer_idx_packed >= vq->size)
+		async->buffer_idx_packed -= vq->size;
+
+}
+
+static __rte_always_inline int
+virtio_dev_tx_async_single_packed(struct virtio_net *dev,
+			struct vhost_virtqueue *vq,
+			struct rte_mempool *mbuf_pool,
+			struct rte_mbuf *pkts,
+			struct virtio_net_hdr *nethdr)
+{
+	int err;
+	uint16_t buf_id, desc_count = 0;
+	uint16_t nr_vec = 0;
+	uint32_t buf_len;
+	struct buf_vector buf_vec[BUF_VECTOR_MAX];
+	static bool allocerr_warned;
+
+	if (unlikely(fill_vec_buf_packed(dev, vq, vq->last_avail_idx, &desc_count,
+					 buf_vec, &nr_vec, &buf_id, &buf_len,
+					 VHOST_ACCESS_RO) < 0))
+		return -1;
+
+	if (unlikely(virtio_dev_pktmbuf_prep(dev, pkts, buf_len))) {
+		if (!allocerr_warned) {
+			VHOST_LOG_DATA(ERR, "Failed mbuf alloc of size %d from %s on %s.\n",
+				buf_len, mbuf_pool->name, dev->ifname);
+			allocerr_warned = true;
+		}
+		return -1;
+	}
+
+	err = async_desc_to_mbuf(dev, vq, buf_vec, nr_vec, pkts, mbuf_pool, nethdr);
+	if (unlikely(err)) {
+		rte_pktmbuf_free(pkts);
+		if (!allocerr_warned) {
+			VHOST_LOG_DATA(ERR, "Failed to copy desc to mbuf on %s.\n", dev->ifname);
+			allocerr_warned = true;
+		}
+		return -1;
+	}
+
+	/* update async shadow packed ring */
+	vhost_async_shadow_dequeue_single_packed(vq, buf_id);
+
+	return err;
+}
+
+static __rte_always_inline uint16_t
+virtio_dev_tx_async_packed(struct virtio_net *dev, struct vhost_virtqueue *vq, uint16_t queue_id,
+		struct rte_mempool *mbuf_pool, struct rte_mbuf **pkts,
+		uint16_t count, uint16_t dma_id, uint16_t dma_vchan, bool legacy_ol_flags)
+{
+	uint16_t pkt_idx;
+	uint16_t slot_idx = 0;
+	uint16_t nr_done_pkts = 0;
+	uint16_t pkt_err = 0;
+	uint32_t n_xfer;
+	struct vhost_async *async = vq->async;
+	struct async_inflight_info *pkts_info = async->pkts_info;
+	struct rte_mbuf *pkts_prealloc[MAX_PKT_BURST];
+
+	VHOST_LOG_DATA(DEBUG, "(%d) about to dequeue %u buffers\n", dev->vid, count);
+
+	async_iter_reset(async);
+
+	if (rte_pktmbuf_alloc_bulk(mbuf_pool, pkts_prealloc, count))
+		goto out;
+
+	for (pkt_idx = 0; pkt_idx < count; pkt_idx++) {
+		struct rte_mbuf *pkt = pkts_prealloc[pkt_idx];
+
+		rte_prefetch0(&vq->desc_packed[vq->last_avail_idx]);
+
+		slot_idx = (async->pkts_idx + pkt_idx) % vq->size;
+		if (unlikely(virtio_dev_tx_async_single_packed(dev, vq, mbuf_pool, pkt,
+			&pkts_info[slot_idx].nethdr))) {
+			rte_pktmbuf_free_bulk(&pkts_prealloc[pkt_idx], count - pkt_idx);
+			break;
+		}
+
+		pkts_info[slot_idx].mbuf = pkt;
+
+		vq_inc_last_avail_packed(vq, 1);
+
+	}
+
+	n_xfer = vhost_async_dma_transfer(dev, vq, dma_id, dma_vchan, async->pkts_idx,
+					async->iov_iter, pkt_idx);
+
+	async->pkts_inflight_n += n_xfer;
+
+	pkt_err = pkt_idx - n_xfer;
+
+	if (unlikely(pkt_err)) {
+		pkt_idx -= pkt_err;
+
+		/**
+		 * recover DMA-copy related structures and free pktmbuf for DMA-error pkts.
+		 */
+		if (async->buffer_idx_packed >= pkt_err)
+			async->buffer_idx_packed -= pkt_err;
+		else
+			async->buffer_idx_packed += vq->size - pkt_err;
+
+		while (pkt_err-- > 0) {
+			rte_pktmbuf_free(pkts_info[slot_idx % vq->size].mbuf);
+			slot_idx--;
+		}
+
+		/* recover available ring */
+		if (vq->last_avail_idx >= pkt_err) {
+			vq->last_avail_idx -= pkt_err;
+		} else {
+			vq->last_avail_idx += vq->size - pkt_err;
+			vq->avail_wrap_counter ^= 1;
+		}
+	}
+
+	async->pkts_idx += pkt_idx;
+	if (async->pkts_idx >= vq->size)
+		async->pkts_idx -= vq->size;
+
+out:
+	nr_done_pkts = async_poll_dequeue_completed(dev, queue_id, pkts, count,
+					dma_id, dma_vchan, legacy_ol_flags);
+
+	return nr_done_pkts;
+}
+
+__rte_noinline
+static uint16_t
+virtio_dev_tx_async_packed_legacy(struct virtio_net *dev,
+		struct vhost_virtqueue *vq, uint16_t queue_id,
+		struct rte_mempool *mbuf_pool, struct rte_mbuf **pkts,
+		uint16_t count, uint16_t dma_id, uint16_t dma_vchan)
+{
+	return virtio_dev_tx_async_packed(dev, vq, queue_id, mbuf_pool,
+				pkts, count, dma_id, dma_vchan, true);
+}
+
+__rte_noinline
+static uint16_t
+virtio_dev_tx_async_packed_compliant(struct virtio_net *dev,
+		struct vhost_virtqueue *vq, uint16_t queue_id,
+		struct rte_mempool *mbuf_pool, struct rte_mbuf **pkts,
+		uint16_t count, uint16_t dma_id, uint16_t dma_vchan)
+{
+	return virtio_dev_tx_async_packed(dev, vq, queue_id, mbuf_pool,
+				pkts, count, dma_id, dma_vchan, false);
+}
+
 uint16_t
 rte_vhost_async_try_dequeue_burst(int vid, uint16_t queue_id,
 	struct rte_mempool *mbuf_pool, struct rte_mbuf **pkts, uint16_t count,
@@ -3612,25 +3780,22 @@ rte_vhost_async_try_dequeue_burst(int vid, uint16_t queue_id,
 		count -= 1;
 	}
 
-	if (unlikely(vq_is_packed(dev))) {
-		static bool not_support_pack_log;
-		if (!not_support_pack_log) {
-			VHOST_LOG_DATA(ERR,
-				"(%s) %s: async dequeue does not support packed ring.\n",
-				dev->ifname, __func__);
-			not_support_pack_log = true;
-		}
-		count = 0;
-		goto out;
+	if (vq_is_packed(dev)) {
+		if (dev->flags & VIRTIO_DEV_LEGACY_OL_FLAGS)
+			count = virtio_dev_tx_async_packed_legacy(dev, vq, queue_id,
+					mbuf_pool, pkts, count, dma_id, vchan_id);
+		else
+			count = virtio_dev_tx_async_packed_compliant(dev, vq, queue_id,
+					mbuf_pool, pkts, count, dma_id, vchan_id);
+	} else {
+		if (dev->flags & VIRTIO_DEV_LEGACY_OL_FLAGS)
+			count = virtio_dev_tx_async_split_legacy(dev, vq, queue_id,
+					mbuf_pool, pkts, count, dma_id, vchan_id);
+		else
+			count = virtio_dev_tx_async_split_compliant(dev, vq, queue_id,
+					mbuf_pool, pkts, count, dma_id, vchan_id);
 	}
 
-	if (dev->flags & VIRTIO_DEV_LEGACY_OL_FLAGS)
-		count = virtio_dev_tx_async_split_legacy(dev, vq, queue_id,
-				mbuf_pool, pkts, count, dma_id, vchan_id);
-	else
-		count = virtio_dev_tx_async_split_compliant(dev, vq, queue_id,
-				mbuf_pool, pkts, count, dma_id, vchan_id);
-
 	*nr_inflight = vq->async->pkts_inflight_n;
 
 out:
-- 
2.25.1

[RFC v3] add support for async vhost packed ring dequeue

[Cheng Jiang]

This RFC patch implements packed ring dequeue data path for asynchronous
vhost.

Signed-off-by: Cheng Jiang <cheng1.jiang@intel.com>
---
 lib/vhost/virtio_net.c | 217 ++++++++++++++++++++++++++++++++++++-----
 1 file changed, 191 insertions(+), 26 deletions(-)

diff --git a/lib/vhost/virtio_net.c b/lib/vhost/virtio_net.c
index 68a26eb17d..9c8964c876 100644
--- a/lib/vhost/virtio_net.c
+++ b/lib/vhost/virtio_net.c
@@ -3240,7 +3240,7 @@ rte_vhost_dequeue_burst(int vid, uint16_t queue_id,
 }
 
 static __rte_always_inline uint16_t
-async_poll_dequeue_completed_split(struct virtio_net *dev, struct vhost_virtqueue *vq,
+async_poll_dequeue_completed(struct virtio_net *dev, struct vhost_virtqueue *vq,
 		struct rte_mbuf **pkts, uint16_t count, int16_t dma_id,
 		uint16_t vchan_id, bool legacy_ol_flags)
 {
@@ -3255,7 +3255,7 @@ async_poll_dequeue_completed_split(struct virtio_net *dev, struct vhost_virtqueu
 	from = start_idx;
 	while (vq->async->pkts_cmpl_flag[from] && count--) {
 		vq->async->pkts_cmpl_flag[from] = false;
-		from = (from + 1) & (vq->size - 1);
+		from = (from + 1) % vq->size;
 		nr_cpl_pkts++;
 	}
 
@@ -3263,7 +3263,7 @@ async_poll_dequeue_completed_split(struct virtio_net *dev, struct vhost_virtqueu
 		return 0;
 
 	for (i = 0; i < nr_cpl_pkts; i++) {
-		from = (start_idx + i) & (vq->size - 1);
+		from = (start_idx + i) % vq->size;
 		pkts[i] = pkts_info[from].mbuf;
 
 		if (virtio_net_with_host_offload(dev))
@@ -3272,10 +3272,14 @@ async_poll_dequeue_completed_split(struct virtio_net *dev, struct vhost_virtqueu
 	}
 
 	/* write back completed descs to used ring and update used idx */
-	write_back_completed_descs_split(vq, nr_cpl_pkts);
-	__atomic_add_fetch(&vq->used->idx, nr_cpl_pkts, __ATOMIC_RELEASE);
-	vhost_vring_call_split(dev, vq);
-
+	if (vq_is_packed(dev)) {
+		write_back_completed_descs_packed(vq, nr_cpl_pkts);
+		vhost_vring_call_packed(dev, vq);
+	} else {
+		write_back_completed_descs_split(vq, nr_cpl_pkts);
+		__atomic_add_fetch(&vq->used->idx, nr_cpl_pkts, __ATOMIC_RELEASE);
+		vhost_vring_call_split(dev, vq);
+	}
 	vq->async->pkts_inflight_n -= nr_cpl_pkts;
 
 	return nr_cpl_pkts;
@@ -3412,8 +3416,8 @@ virtio_dev_tx_async_split(struct virtio_net *dev, struct vhost_virtqueue *vq,
 
 out:
 	/* DMA device may serve other queues, unconditionally check completed. */
-	nr_done_pkts = async_poll_dequeue_completed_split(dev, vq, pkts, pkts_size,
-							  dma_id, vchan_id, legacy_ol_flags);
+	nr_done_pkts = async_poll_dequeue_completed(dev, vq, pkts, pkts_size,
+							dma_id, vchan_id, legacy_ol_flags);
 
 	return nr_done_pkts;
 }
@@ -3440,6 +3444,170 @@ virtio_dev_tx_async_split_compliant(struct virtio_net *dev,
 				pkts, count, dma_id, vchan_id, false);
 }
 
+static __rte_always_inline void
+vhost_async_shadow_dequeue_single_packed(struct vhost_virtqueue *vq, uint16_t buf_id)
+{
+	struct vhost_async *async = vq->async;
+	uint16_t idx = async->buffer_idx_packed;
+
+	async->buffers_packed[idx].id = buf_id;
+	async->buffers_packed[idx].len = 0;
+	async->buffers_packed[idx].count = 1;
+
+	async->buffer_idx_packed++;
+	if (async->buffer_idx_packed >= vq->size)
+		async->buffer_idx_packed -= vq->size;
+
+}
+
+static __rte_always_inline int
+virtio_dev_tx_async_single_packed(struct virtio_net *dev,
+			struct vhost_virtqueue *vq,
+			struct rte_mempool *mbuf_pool,
+			struct rte_mbuf *pkts,
+			uint16_t slot_idx,
+			bool legacy_ol_flags)
+{
+	int err;
+	uint16_t buf_id, desc_count = 0;
+	uint16_t nr_vec = 0;
+	uint32_t buf_len;
+	struct buf_vector buf_vec[BUF_VECTOR_MAX];
+	static bool allocerr_warned;
+
+	if (unlikely(fill_vec_buf_packed(dev, vq, vq->last_avail_idx, &desc_count,
+					 buf_vec, &nr_vec, &buf_id, &buf_len,
+					 VHOST_ACCESS_RO) < 0))
+		return -1;
+
+	if (unlikely(virtio_dev_pktmbuf_prep(dev, pkts, buf_len))) {
+		if (!allocerr_warned) {
+			VHOST_LOG_DATA(ERR, "Failed mbuf alloc of size %d from %s on %s.\n",
+				buf_len, mbuf_pool->name, dev->ifname);
+			allocerr_warned = true;
+		}
+		return -1;
+	}
+
+	err = desc_to_mbuf(dev, vq, buf_vec, nr_vec, pkts, mbuf_pool,
+		legacy_ol_flags, slot_idx, true);
+	if (unlikely(err)) {
+		rte_pktmbuf_free(pkts);
+		if (!allocerr_warned) {
+			VHOST_LOG_DATA(ERR, "Failed to copy desc to mbuf on %s.\n", dev->ifname);
+			allocerr_warned = true;
+		}
+		return -1;
+	}
+
+	/* update async shadow packed ring */
+	vhost_async_shadow_dequeue_single_packed(vq, buf_id);
+
+	return err;
+}
+
+static __rte_always_inline uint16_t
+virtio_dev_tx_async_packed(struct virtio_net *dev, struct vhost_virtqueue *vq,
+		struct rte_mempool *mbuf_pool, struct rte_mbuf **pkts,
+		uint16_t count, uint16_t dma_id, uint16_t vchan_id, bool legacy_ol_flags)
+{
+	uint16_t pkt_idx;
+	uint16_t slot_idx = 0;
+	uint16_t nr_done_pkts = 0;
+	uint16_t pkt_err = 0;
+	uint32_t n_xfer;
+	struct vhost_async *async = vq->async;
+	struct async_inflight_info *pkts_info = async->pkts_info;
+	struct rte_mbuf *pkts_prealloc[MAX_PKT_BURST];
+
+	VHOST_LOG_DATA(DEBUG, "(%d) about to dequeue %u buffers\n", dev->vid, count);
+
+	async_iter_reset(async);
+
+	if (rte_pktmbuf_alloc_bulk(mbuf_pool, pkts_prealloc, count))
+		goto out;
+
+	for (pkt_idx = 0; pkt_idx < count; pkt_idx++) {
+		struct rte_mbuf *pkt = pkts_prealloc[pkt_idx];
+
+		rte_prefetch0(&vq->desc_packed[vq->last_avail_idx]);
+
+		slot_idx = (async->pkts_idx + pkt_idx) % vq->size;
+		if (unlikely(virtio_dev_tx_async_single_packed(dev, vq, mbuf_pool, pkt,
+				slot_idx, legacy_ol_flags))) {
+			rte_pktmbuf_free_bulk(&pkts_prealloc[pkt_idx], count - pkt_idx);
+			break;
+		}
+
+		pkts_info[slot_idx].mbuf = pkt;
+
+		vq_inc_last_avail_packed(vq, 1);
+
+	}
+
+	n_xfer = vhost_async_dma_transfer(dev, vq, dma_id, vchan_id, async->pkts_idx,
+					async->iov_iter, pkt_idx);
+
+	async->pkts_inflight_n += n_xfer;
+
+	pkt_err = pkt_idx - n_xfer;
+
+	if (unlikely(pkt_err)) {
+		pkt_idx -= pkt_err;
+
+		/**
+		 * recover DMA-copy related structures and free pktmbuf for DMA-error pkts.
+		 */
+		if (async->buffer_idx_packed >= pkt_err)
+			async->buffer_idx_packed -= pkt_err;
+		else
+			async->buffer_idx_packed += vq->size - pkt_err;
+
+		while (pkt_err-- > 0) {
+			rte_pktmbuf_free(pkts_info[slot_idx % vq->size].mbuf);
+			slot_idx--;
+		}
+
+		/* recover available ring */
+		if (vq->last_avail_idx >= pkt_err) {
+			vq->last_avail_idx -= pkt_err;
+		} else {
+			vq->last_avail_idx += vq->size - pkt_err;
+			vq->avail_wrap_counter ^= 1;
+		}
+	}
+
+	async->pkts_idx += pkt_idx;
+	if (async->pkts_idx >= vq->size)
+		async->pkts_idx -= vq->size;
+
+out:
+	nr_done_pkts = async_poll_dequeue_completed(dev, vq, pkts, count,
+					dma_id, vchan_id, legacy_ol_flags);
+
+	return nr_done_pkts;
+}
+
+__rte_noinline
+static uint16_t
+virtio_dev_tx_async_packed_legacy(struct virtio_net *dev, struct vhost_virtqueue *vq,
+		struct rte_mempool *mbuf_pool, struct rte_mbuf **pkts,
+		uint16_t count, uint16_t dma_id, uint16_t vchan_id)
+{
+	return virtio_dev_tx_async_packed(dev, vq, mbuf_pool,
+				pkts, count, dma_id, vchan_id, true);
+}
+
+__rte_noinline
+static uint16_t
+virtio_dev_tx_async_packed_compliant(struct virtio_net *dev, struct vhost_virtqueue *vq,
+		struct rte_mempool *mbuf_pool, struct rte_mbuf **pkts,
+		uint16_t count, uint16_t dma_id, uint16_t vchan_id)
+{
+	return virtio_dev_tx_async_packed(dev, vq, mbuf_pool,
+				pkts, count, dma_id, vchan_id, false);
+}
+
 uint16_t
 rte_vhost_async_try_dequeue_burst(int vid, uint16_t queue_id,
 	struct rte_mempool *mbuf_pool, struct rte_mbuf **pkts, uint16_t count,
@@ -3542,25 +3710,22 @@ rte_vhost_async_try_dequeue_burst(int vid, uint16_t queue_id,
 		count -= 1;
 	}
 
-	if (unlikely(vq_is_packed(dev))) {
-		static bool not_support_pack_log;
-		if (!not_support_pack_log) {
-			VHOST_LOG_DATA(ERR,
-				"(%s) %s: async dequeue does not support packed ring.\n",
-				dev->ifname, __func__);
-			not_support_pack_log = true;
-		}
-		count = 0;
-		goto out;
+	if (vq_is_packed(dev)) {
+		if (dev->flags & VIRTIO_DEV_LEGACY_OL_FLAGS)
+			count = virtio_dev_tx_async_packed_legacy(dev, vq, mbuf_pool,
+					pkts, count, dma_id, vchan_id);
+		else
+			count = virtio_dev_tx_async_packed_compliant(dev, vq, mbuf_pool,
+					pkts, count, dma_id, vchan_id);
+	} else {
+		if (dev->flags & VIRTIO_DEV_LEGACY_OL_FLAGS)
+			count = virtio_dev_tx_async_split_legacy(dev, vq, mbuf_pool,
+					pkts, count, dma_id, vchan_id);
+		else
+			count = virtio_dev_tx_async_split_compliant(dev, vq, mbuf_pool,
+					pkts, count, dma_id, vchan_id);
 	}
 
-	if (dev->flags & VIRTIO_DEV_LEGACY_OL_FLAGS)
-		count = virtio_dev_tx_async_split_legacy(dev, vq, mbuf_pool, pkts,
-							 count, dma_id, vchan_id);
-	else
-		count = virtio_dev_tx_async_split_compliant(dev, vq, mbuf_pool, pkts,
-							    count, dma_id, vchan_id);
-
 	*nr_inflight = vq->async->pkts_inflight_n;
 
 out:
-- 
2.35.1

Re: [RFC v3] add support for async vhost packed ring dequeue

[Maxime Coquelin]

Hi Cheng,

On 5/30/22 06:56, Cheng Jiang wrote:
> This RFC patch implements packed ring dequeue data path for asynchronous
> vhost.

Please remove RFC in the next revision.

> Signed-off-by: Cheng Jiang <cheng1.jiang@intel.com>
> ---
>   lib/vhost/virtio_net.c | 217 ++++++++++++++++++++++++++++++++++++-----
>   1 file changed, 191 insertions(+), 26 deletions(-)
> 
> diff --git a/lib/vhost/virtio_net.c b/lib/vhost/virtio_net.c

...

> +
> +static __rte_always_inline int
> +virtio_dev_tx_async_single_packed(struct virtio_net *dev,
> +			struct vhost_virtqueue *vq,
> +			struct rte_mempool *mbuf_pool,
> +			struct rte_mbuf *pkts,
> +			uint16_t slot_idx,
> +			bool legacy_ol_flags)
> +{
> +	int err;
> +	uint16_t buf_id, desc_count = 0;
> +	uint16_t nr_vec = 0;
> +	uint32_t buf_len;
> +	struct buf_vector buf_vec[BUF_VECTOR_MAX];
> +	static bool allocerr_warned;
> +
> +	if (unlikely(fill_vec_buf_packed(dev, vq, vq->last_avail_idx, &desc_count,
> +					 buf_vec, &nr_vec, &buf_id, &buf_len,
> +					 VHOST_ACCESS_RO) < 0))
> +		return -1;
> +
> +	if (unlikely(virtio_dev_pktmbuf_prep(dev, pkts, buf_len))) {
> +		if (!allocerr_warned) {
> +			VHOST_LOG_DATA(ERR, "Failed mbuf alloc of size %d from %s on %s.\n",
> +				buf_len, mbuf_pool->name, dev->ifname);

We have defined a common format for all Vhost logs to ease logs
filtering:
VHOST_LOG_DATA(ERR, "(%s) Failed mbuf alloc of size %d from %s.\n", 	
		dev->ifname, buf_len, mbuf_pool->name);

Could you please fix it here and everywhere else in the patch?

With above minor comments fixed, feel free to add:

Reviewed-by: Maxime Coquelin <maxime.coquelin@redhat.com>

Thanks,
Maxime

RE: [RFC v3] add support for async vhost packed ring dequeue

[Jiang, Cheng1]

> -----Original Message-----
> From: Maxime Coquelin <maxime.coquelin@redhat.com>
> Sent: Thursday, June 9, 2022 9:04 PM
> To: Jiang, Cheng1 <cheng1.jiang@intel.com>; Xia, Chenbo
> <chenbo.xia@intel.com>
> Cc: dev@dpdk.org; Hu, Jiayu <jiayu.hu@intel.com>; Ding, Xuan
> <xuan.ding@intel.com>; Ma, WenwuX <wenwux.ma@intel.com>; Wang,
> YuanX <yuanx.wang@intel.com>; Yang, YvonneX <yvonnex.yang@intel.com>
> Subject: Re: [RFC v3] add support for async vhost packed ring dequeue
> 
> Hi Cheng,
> 
> On 5/30/22 06:56, Cheng Jiang wrote:
> > This RFC patch implements packed ring dequeue data path for
> > asynchronous vhost.
> 
> Please remove RFC in the next revision.

Sure.

> 
> > Signed-off-by: Cheng Jiang <cheng1.jiang@intel.com>
> > ---
> >   lib/vhost/virtio_net.c | 217
> ++++++++++++++++++++++++++++++++++++-----
> >   1 file changed, 191 insertions(+), 26 deletions(-)
> >
> > diff --git a/lib/vhost/virtio_net.c b/lib/vhost/virtio_net.c
> 
> ...
> 
> > +
> > +static __rte_always_inline int
> > +virtio_dev_tx_async_single_packed(struct virtio_net *dev,
> > +			struct vhost_virtqueue *vq,
> > +			struct rte_mempool *mbuf_pool,
> > +			struct rte_mbuf *pkts,
> > +			uint16_t slot_idx,
> > +			bool legacy_ol_flags)
> > +{
> > +	int err;
> > +	uint16_t buf_id, desc_count = 0;
> > +	uint16_t nr_vec = 0;
> > +	uint32_t buf_len;
> > +	struct buf_vector buf_vec[BUF_VECTOR_MAX];
> > +	static bool allocerr_warned;
> > +
> > +	if (unlikely(fill_vec_buf_packed(dev, vq, vq->last_avail_idx,
> &desc_count,
> > +					 buf_vec, &nr_vec, &buf_id,
> &buf_len,
> > +					 VHOST_ACCESS_RO) < 0))
> > +		return -1;
> > +
> > +	if (unlikely(virtio_dev_pktmbuf_prep(dev, pkts, buf_len))) {
> > +		if (!allocerr_warned) {
> > +			VHOST_LOG_DATA(ERR, "Failed mbuf alloc of size %d
> from %s on %s.\n",
> > +				buf_len, mbuf_pool->name, dev->ifname);
> 
> We have defined a common format for all Vhost logs to ease logs
> filtering:
> VHOST_LOG_DATA(ERR, "(%s) Failed mbuf alloc of size %d from %s.\n",
> 
> 		dev->ifname, buf_len, mbuf_pool->name);
> 
> Could you please fix it here and everywhere else in the patch?
> 
> With above minor comments fixed, feel free to add:
> 
> Reviewed-by: Maxime Coquelin <maxime.coquelin@redhat.com>
> 
> Thanks,
> Maxime

Sure, got it. I'll fix it.

Thanks a lot.
Cheng