Skip to contentSkip to conversation

examples/vhost: sample code refactor

PATCH Superseded Cheng Jiang 4 participants 44 messages Dec 18, 2020 – Jan 27, 2021

lore
Share & tools
lore
#0 lore raw

cover examples/vhost: sample code refactor

Refactor the vhost sample code. Add ioat ring space count and check in ioat callback, optimize vhost data path for batch enqueue, and refactor vhost async data path.

Cheng Jiang (3):
examples/vhost: add ioat ring space count and check examples/vhost: optimize vhost data path for batch examples/vhost: refactor vhost async data path

 examples/vhost/ioat.c |  15 ++---
 examples/vhost/main.c | 144 ++++++++++++++++++++++++++++++------------
 examples/vhost/main.h |   2 +-
 3 files changed, 110 insertions(+), 51 deletions(-)

--
2.29.2

#1 lore raw

PATCH 1/3 examples/vhost: add ioat ring space count and check

Add ioat ring space count and check, if ioat ring space is not enough for the next async vhost packet enqueue, then just return to prevent enqueue failure.

---
 examples/vhost/ioat.c | 15 +++++++--------
 1 file changed, 7 insertions(+), 8 deletions(-)
examples/vhost/ioat.c +7 -8
index 71d8a1f1f5..b0b04aa453 100644
@@ -17,6 +17,7 @@ struct packet_tracker {
 	unsigned short next_read;
 	unsigned short next_write;
 	unsigned short last_remain;
+	unsigned short ioat_space;
 };
 
 struct packet_tracker cb_tracker[MAX_VHOST_DEVICE];
@@ -113,7 +114,7 @@ open_ioat(const char *value)
 			goto out;
 		}
 		rte_rawdev_start(dev_id);
-
+		cb_tracker[dev_id].ioat_space = IOAT_RING_SIZE;
 		dma_info->nr++;
 		i++;
 	}
@@ -140,13 +141,9 @@ ioat_transfer_data_cb(int vid, uint16_t queue_id,
 			src = descs[i_desc].src;
 			dst = descs[i_desc].dst;
 			i_seg = 0;
+			if (cb_tracker[dev_id].ioat_space < src->nr_segs)
+				break;
 			while (i_seg < src->nr_segs) {
-				/*
-				 * TODO: Assuming that the ring space of the
-				 * IOAT device is large enough, so there is no
-				 * error here, and the actual error handling
-				 * will be added later.
-				 */
 				rte_ioat_enqueue_copy(dev_id,
 					(uintptr_t)(src->iov[i_seg].iov_base)
 						+ src->offset,
@@ -158,7 +155,8 @@ ioat_transfer_data_cb(int vid, uint16_t queue_id,
 				i_seg++;
 			}
 			write &= mask;
-			cb_tracker[dev_id].size_track[write] = i_seg;
+			cb_tracker[dev_id].size_track[write] = src->nr_segs;
+			cb_tracker[dev_id].ioat_space -= src->nr_segs;
 			write++;
 		}
 	} else {
@@ -186,6 +184,7 @@ ioat_check_completed_copies_cb(int vid, uint16_t queue_id,
 		int dev_id = dma_bind[vid].dmas[queue_id * 2
 				+ VIRTIO_RXQ].dev_id;
 		n_seg = rte_ioat_completed_ops(dev_id, 255, dump, dump);
+		cb_tracker[dev_id].ioat_space += n_seg;
 		n_seg += cb_tracker[dev_id].last_remain;
 		if (!n_seg)
 			return 0;
Signature · 1 line
2.29.2
#2 lore raw

PATCH 2/3 examples/vhost: optimize vhost data path for batch

In reply to Cheng Jiang

Change the vm2vm data path to batch enqueue for better performance.

---
 examples/vhost/main.c | 84 ++++++++++++++++++++++++++++++++++++++-----
 1 file changed, 75 insertions(+), 9 deletions(-)
examples/vhost/main.c +75 -9
index 8d8c3038bf..28226a4ff7 100644
@@ -182,6 +182,11 @@ struct mbuf_table {
 /* TX queue for each data core. */
 struct mbuf_table lcore_tx_queue[RTE_MAX_LCORE];
 
+static uint64_t vhost_tsc[MAX_VHOST_DEVICE];
+
+/* TX queue for each vhost device. */
+struct mbuf_table vhost_m_table[MAX_VHOST_DEVICE];
+
 #define MBUF_TABLE_DRAIN_TSC	((rte_get_tsc_hz() + US_PER_S - 1) \
 				 / US_PER_S * BURST_TX_DRAIN_US)
 #define VLAN_HLEN       4
@@ -804,6 +809,13 @@ unlink_vmdq(struct vhost_dev *vdev)
 	}
 }
 
+static inline void
+free_pkts(struct rte_mbuf **pkts, uint16_t n)
+{
+	while (n--)
+		rte_pktmbuf_free(pkts[n]);
+}
+
 static __rte_always_inline void
 virtio_xmit(struct vhost_dev *dst_vdev, struct vhost_dev *src_vdev,
 	    struct rte_mbuf *m)
@@ -837,6 +849,40 @@ virtio_xmit(struct vhost_dev *dst_vdev, struct vhost_dev *src_vdev,
 	}
 }
 
+static __rte_always_inline void
+drain_vhost(struct vhost_dev *dst_vdev, struct rte_mbuf **m, uint16_t nr_xmit)
+{
+	uint16_t ret, nr_cpl;
+	struct rte_mbuf *m_cpl[MAX_PKT_BURST];
+
+	if (builtin_net_driver) {
+		ret = vs_enqueue_pkts(dst_vdev, VIRTIO_RXQ, m, nr_xmit);
+	} else if (async_vhost_driver) {
+		ret = rte_vhost_submit_enqueue_burst(dst_vdev->vid, VIRTIO_RXQ,
+						m, nr_xmit);
+		dst_vdev->nr_async_pkts += ret;
+		free_pkts(&m[ret], nr_xmit - ret);
+
+		while (likely(dst_vdev->nr_async_pkts)) {
+			nr_cpl = rte_vhost_poll_enqueue_completed(dst_vdev->vid,
+					VIRTIO_RXQ, m_cpl, MAX_PKT_BURST);
+			dst_vdev->nr_async_pkts -= nr_cpl;
+			free_pkts(m_cpl, nr_cpl);
+		}
+	} else {
+		ret = rte_vhost_enqueue_burst(dst_vdev->vid, VIRTIO_RXQ,
+						m, nr_xmit);
+	}
+
+	if (enable_stats) {
+		rte_atomic64_add(&dst_vdev->stats.rx_total_atomic, nr_xmit);
+		rte_atomic64_add(&dst_vdev->stats.rx_atomic, ret);
+	}
+
+	if (!async_vhost_driver)
+		free_pkts(m, nr_xmit);
+}
+
 /*
  * Check if the packet destination MAC address is for a local device. If so then put
  * the packet on that devices RX queue. If not then return.
@@ -846,6 +892,7 @@ virtio_tx_local(struct vhost_dev *vdev, struct rte_mbuf *m)
 {
 	struct rte_ether_hdr *pkt_hdr;
 	struct vhost_dev *dst_vdev;
+	struct mbuf_table *vhost_txq;
 
 	pkt_hdr = rte_pktmbuf_mtod(m, struct rte_ether_hdr *);
 
@@ -869,7 +916,19 @@ virtio_tx_local(struct vhost_dev *vdev, struct rte_mbuf *m)
 		return 0;
 	}
 
-	virtio_xmit(dst_vdev, vdev, m);
+	vhost_txq = &vhost_m_table[dst_vdev->vid];
+	vhost_txq->m_table[vhost_txq->len++] = m;
+
+	if (enable_stats) {
+		vdev->stats.tx_total++;
+		vdev->stats.tx++;
+	}
+
+	if (unlikely(vhost_txq->len == MAX_PKT_BURST)) {
+		drain_vhost(dst_vdev, vhost_txq->m_table, MAX_PKT_BURST);
+		vhost_txq->len = 0;
+		vhost_tsc[dst_vdev->vid] = rte_rdtsc();
+	}
 	return 0;
 }
 
@@ -940,13 +999,6 @@ static void virtio_tx_offload(struct rte_mbuf *m)
 	tcp_hdr->cksum = get_psd_sum(l3_hdr, m->ol_flags);
 }
 
-static inline void
-free_pkts(struct rte_mbuf **pkts, uint16_t n)
-{
-	while (n--)
-		rte_pktmbuf_free(pkts[n]);
-}
-
 static __rte_always_inline void
 do_drain_mbuf_table(struct mbuf_table *tx_q)
 {
@@ -986,7 +1038,6 @@ virtio_tx_route(struct vhost_dev *vdev, struct rte_mbuf *m, uint16_t vlan_tag)
 
 	/*check if destination is local VM*/
 	if ((vm2vm_mode == VM2VM_SOFTWARE) && (virtio_tx_local(vdev, m) == 0)) {
-		rte_pktmbuf_free(m);
 		return;
 	}
 
@@ -1144,8 +1195,10 @@ static __rte_always_inline void
 drain_virtio_tx(struct vhost_dev *vdev)
 {
 	struct rte_mbuf *pkts[MAX_PKT_BURST];
+	struct mbuf_table *vhost_txq;
 	uint16_t count;
 	uint16_t i;
+	uint64_t cur_tsc;
 
 	if (builtin_net_driver) {
 		count = vs_dequeue_pkts(vdev, VIRTIO_TXQ, mbuf_pool,
@@ -1163,6 +1216,19 @@ drain_virtio_tx(struct vhost_dev *vdev)
 
 	for (i = 0; i < count; ++i)
 		virtio_tx_route(vdev, pkts[i], vlan_tags[vdev->vid]);
+
+	vhost_txq = &vhost_m_table[vdev->vid];
+	cur_tsc = rte_rdtsc();
+	if (unlikely(cur_tsc - vhost_tsc[vdev->vid] > MBUF_TABLE_DRAIN_TSC)) {
+		if (vhost_txq->len) {
+			RTE_LOG_DP(DEBUG, VHOST_DATA,
+				"Vhost tX queue drained after timeout with burst size %u\n",
+				vhost_txq->len);
+			drain_vhost(vdev, vhost_txq->m_table, vhost_txq->len);
+			vhost_txq->len = 0;
+			vhost_tsc[vdev->vid] = cur_tsc;
+		}
+	}
 }
 
 /*
Signature · 1 line
2.29.2
#3 lore raw

PATCH 3/3 examples/vhost: refactor vhost async data path

In reply to Cheng Jiang

Support latest async vhost api, refactor vhost async data path, and clean some codes.

---
 examples/vhost/main.c | 88 ++++++++++++++++++++-----------------------
 examples/vhost/main.h |  2 +-
 2 files changed, 42 insertions(+), 48 deletions(-)
examples/vhost/main.c +41 -47
index 28226a4ff7..0113147876 100644
@@ -817,26 +817,26 @@ free_pkts(struct rte_mbuf **pkts, uint16_t n)
 }
 
 static __rte_always_inline void
-virtio_xmit(struct vhost_dev *dst_vdev, struct vhost_dev *src_vdev,
+complete_async_pkts(struct vhost_dev *vdev)
+{
+	struct rte_mbuf *p_cpl[MAX_PKT_BURST];
+	uint16_t complete_count;
+
+	complete_count = rte_vhost_poll_enqueue_completed(vdev->vid,
+					VIRTIO_RXQ, p_cpl, MAX_PKT_BURST);
+	rte_atomic16_sub(&vdev->nr_async_pkts, complete_count);
+	if (complete_count)
+		free_pkts(p_cpl, complete_count);
+}
+
+static __rte_always_inline void
+sync_virtio_xmit(struct vhost_dev *dst_vdev, struct vhost_dev *src_vdev,
 	    struct rte_mbuf *m)
 {
 	uint16_t ret;
-	struct rte_mbuf *m_cpl[1];
 
 	if (builtin_net_driver) {
 		ret = vs_enqueue_pkts(dst_vdev, VIRTIO_RXQ, &m, 1);
-	} else if (async_vhost_driver) {
-		ret = rte_vhost_submit_enqueue_burst(dst_vdev->vid, VIRTIO_RXQ,
-						&m, 1);
-
-		if (likely(ret))
-			dst_vdev->nr_async_pkts++;
-
-		while (likely(dst_vdev->nr_async_pkts)) {
-			if (rte_vhost_poll_enqueue_completed(dst_vdev->vid,
-					VIRTIO_RXQ, m_cpl, 1))
-				dst_vdev->nr_async_pkts--;
-		}
 	} else {
 		ret = rte_vhost_enqueue_burst(dst_vdev->vid, VIRTIO_RXQ, &m, 1);
 	}
@@ -850,25 +850,25 @@ virtio_xmit(struct vhost_dev *dst_vdev, struct vhost_dev *src_vdev,
 }
 
 static __rte_always_inline void
-drain_vhost(struct vhost_dev *dst_vdev, struct rte_mbuf **m, uint16_t nr_xmit)
+drain_vhost(struct vhost_dev *dst_vdev)
 {
-	uint16_t ret, nr_cpl;
-	struct rte_mbuf *m_cpl[MAX_PKT_BURST];
+	uint16_t ret;
+	uint16_t nr_xmit = vhost_m_table[dst_vdev->vid].len;
+	struct rte_mbuf **m = vhost_m_table[dst_vdev->vid].m_table;
 
 	if (builtin_net_driver) {
 		ret = vs_enqueue_pkts(dst_vdev, VIRTIO_RXQ, m, nr_xmit);
 	} else if (async_vhost_driver) {
+		uint32_t cpu_cpl_nr;
+		struct rte_mbuf *m_cpu_cpl[nr_xmit];
+		complete_async_pkts(dst_vdev);
+		while (rte_atomic16_read(&dst_vdev->nr_async_pkts) >= 128)
+			complete_async_pkts(dst_vdev);
+
 		ret = rte_vhost_submit_enqueue_burst(dst_vdev->vid, VIRTIO_RXQ,
-						m, nr_xmit);
-		dst_vdev->nr_async_pkts += ret;
+					m, nr_xmit, m_cpu_cpl, &cpu_cpl_nr);
+		rte_atomic16_add(&dst_vdev->nr_async_pkts, ret - cpu_cpl_nr);
 		free_pkts(&m[ret], nr_xmit - ret);
-
-		while (likely(dst_vdev->nr_async_pkts)) {
-			nr_cpl = rte_vhost_poll_enqueue_completed(dst_vdev->vid,
-					VIRTIO_RXQ, m_cpl, MAX_PKT_BURST);
-			dst_vdev->nr_async_pkts -= nr_cpl;
-			free_pkts(m_cpl, nr_cpl);
-		}
 	} else {
 		ret = rte_vhost_enqueue_burst(dst_vdev->vid, VIRTIO_RXQ,
 						m, nr_xmit);
@@ -925,7 +925,7 @@ virtio_tx_local(struct vhost_dev *vdev, struct rte_mbuf *m)
 	}
 
 	if (unlikely(vhost_txq->len == MAX_PKT_BURST)) {
-		drain_vhost(dst_vdev, vhost_txq->m_table, MAX_PKT_BURST);
+		drain_vhost(dst_vdev);
 		vhost_txq->len = 0;
 		vhost_tsc[dst_vdev->vid] = rte_rdtsc();
 	}
@@ -1031,7 +1031,7 @@ virtio_tx_route(struct vhost_dev *vdev, struct rte_mbuf *m, uint16_t vlan_tag)
 
 		TAILQ_FOREACH(vdev2, &vhost_dev_list, global_vdev_entry) {
 			if (vdev2 != vdev)
-				virtio_xmit(vdev2, vdev, m);
+				sync_virtio_xmit(vdev2, vdev, m);
 		}
 		goto queue2nic;
 	}
@@ -1124,31 +1124,17 @@ drain_mbuf_table(struct mbuf_table *tx_q)
 	}
 }
 
-static __rte_always_inline void
-complete_async_pkts(struct vhost_dev *vdev, uint16_t qid)
-{
-	struct rte_mbuf *p_cpl[MAX_PKT_BURST];
-	uint16_t complete_count;
-
-	complete_count = rte_vhost_poll_enqueue_completed(vdev->vid,
-						qid, p_cpl, MAX_PKT_BURST);
-	vdev->nr_async_pkts -= complete_count;
-	if (complete_count)
-		free_pkts(p_cpl, complete_count);
-}
-
 static __rte_always_inline void
 drain_eth_rx(struct vhost_dev *vdev)
 {
 	uint16_t rx_count, enqueue_count;
+	uint32_t cpu_cpl_nr;
 	struct rte_mbuf *pkts[MAX_PKT_BURST];
+	struct rte_mbuf *m_cpu_cpl[MAX_PKT_BURST];
 
 	rx_count = rte_eth_rx_burst(ports[0], vdev->vmdq_rx_q,
 				    pkts, MAX_PKT_BURST);
 
-	while (likely(vdev->nr_async_pkts))
-		complete_async_pkts(vdev, VIRTIO_RXQ);
-
 	if (!rx_count)
 		return;
 
@@ -1170,13 +1156,21 @@ drain_eth_rx(struct vhost_dev *vdev)
 		}
 	}
 
+	complete_async_pkts(vdev);
+	while (rte_atomic16_read(&vdev->nr_async_pkts) >= 128)
+		complete_async_pkts(vdev);
+
 	if (builtin_net_driver) {
 		enqueue_count = vs_enqueue_pkts(vdev, VIRTIO_RXQ,
 						pkts, rx_count);
 	} else if (async_vhost_driver) {
 		enqueue_count = rte_vhost_submit_enqueue_burst(vdev->vid,
-					VIRTIO_RXQ, pkts, rx_count);
-		vdev->nr_async_pkts += enqueue_count;
+					VIRTIO_RXQ, pkts, rx_count,
+					m_cpu_cpl, &cpu_cpl_nr);
+		rte_atomic16_add(&vdev->nr_async_pkts,
+					enqueue_count - cpu_cpl_nr);
+		if (cpu_cpl_nr)
+			free_pkts(m_cpu_cpl, cpu_cpl_nr);
 	} else {
 		enqueue_count = rte_vhost_enqueue_burst(vdev->vid, VIRTIO_RXQ,
 						pkts, rx_count);
@@ -1224,7 +1218,7 @@ drain_virtio_tx(struct vhost_dev *vdev)
 			RTE_LOG_DP(DEBUG, VHOST_DATA,
 				"Vhost tX queue drained after timeout with burst size %u\n",
 				vhost_txq->len);
-			drain_vhost(vdev, vhost_txq->m_table, vhost_txq->len);
+			drain_vhost(vdev);
 			vhost_txq->len = 0;
 			vhost_tsc[vdev->vid] = cur_tsc;
 		}
examples/vhost/main.h +1 -1
index 4317b6ae81..d33ddb411b 100644
@@ -51,7 +51,7 @@ struct vhost_dev {
 	uint64_t features;
 	size_t hdr_len;
 	uint16_t nr_vrings;
-	uint16_t nr_async_pkts;
+	rte_atomic16_t nr_async_pkts;
 	struct rte_vhost_memory *mem;
 	struct device_statistics stats;
 	TAILQ_ENTRY(vhost_dev) global_vdev_entry;
Signature · 1 line
2.29.2
#4 lore raw

v2 cover examples/vhost: sample code refactor

In reply to Cheng Jiang

Refactor the vhost sample code. Add ioat ring space count and check in ioat callback, optimize vhost data path for batch enqueue, replase rte_atomicNN_xxx to atomic_XXX and refactor vhost async data path.
---
v2:
* optimized patch structure
* optimized git log
* replased rte_atomicNN_xxx to atomic_XXX

Cheng Jiang (2):
examples/vhost: add ioat ring space count and check examples/vhost: refactor vhost data path

 examples/vhost/ioat.c |  15 ++--
 examples/vhost/main.c | 163 +++++++++++++++++++++++++++++-------------
 examples/vhost/main.h |   7 +-
 3 files changed, 125 insertions(+), 60 deletions(-)

--
2.29.2

#5 lore raw

PATCH v2 1/2 examples/vhost: add ioat ring space count and check

Add ioat ring space count and check, if ioat ring space is not enough for the next async vhost packet enqueue, then just return to prevent enqueue failure.

---
 examples/vhost/ioat.c | 15 +++++++--------
 1 file changed, 7 insertions(+), 8 deletions(-)
examples/vhost/ioat.c +7 -8
index 71d8a1f1f5..b0b04aa453 100644
@@ -17,6 +17,7 @@ struct packet_tracker {
 	unsigned short next_read;
 	unsigned short next_write;
 	unsigned short last_remain;
+	unsigned short ioat_space;
 };
 
 struct packet_tracker cb_tracker[MAX_VHOST_DEVICE];
@@ -113,7 +114,7 @@ open_ioat(const char *value)
 			goto out;
 		}
 		rte_rawdev_start(dev_id);
-
+		cb_tracker[dev_id].ioat_space = IOAT_RING_SIZE;
 		dma_info->nr++;
 		i++;
 	}
@@ -140,13 +141,9 @@ ioat_transfer_data_cb(int vid, uint16_t queue_id,
 			src = descs[i_desc].src;
 			dst = descs[i_desc].dst;
 			i_seg = 0;
+			if (cb_tracker[dev_id].ioat_space < src->nr_segs)
+				break;
 			while (i_seg < src->nr_segs) {
-				/*
-				 * TODO: Assuming that the ring space of the
-				 * IOAT device is large enough, so there is no
-				 * error here, and the actual error handling
-				 * will be added later.
-				 */
 				rte_ioat_enqueue_copy(dev_id,
 					(uintptr_t)(src->iov[i_seg].iov_base)
 						+ src->offset,
@@ -158,7 +155,8 @@ ioat_transfer_data_cb(int vid, uint16_t queue_id,
 				i_seg++;
 			}
 			write &= mask;
-			cb_tracker[dev_id].size_track[write] = i_seg;
+			cb_tracker[dev_id].size_track[write] = src->nr_segs;
+			cb_tracker[dev_id].ioat_space -= src->nr_segs;
 			write++;
 		}
 	} else {
@@ -186,6 +184,7 @@ ioat_check_completed_copies_cb(int vid, uint16_t queue_id,
 		int dev_id = dma_bind[vid].dmas[queue_id * 2
 				+ VIRTIO_RXQ].dev_id;
 		n_seg = rte_ioat_completed_ops(dev_id, 255, dump, dump);
+		cb_tracker[dev_id].ioat_space += n_seg;
 		n_seg += cb_tracker[dev_id].last_remain;
 		if (!n_seg)
 			return 0;
Signature · 1 line
2.29.2
#6 lore raw

PATCH v2 2/2 examples/vhost: refactor vhost data path

In reply to Cheng Jiang

Change the vm2vm data path to batch enqueue for better performance. Support latest async vhost API, refactor vhost async data path, replase rte_atomicNN_xxx to atomic_XXX and clean some codes.

---
 examples/vhost/main.c | 163 +++++++++++++++++++++++++++++-------------
 examples/vhost/main.h |   7 +-
 2 files changed, 118 insertions(+), 52 deletions(-)
examples/vhost/main.c +114 -49
index 8d8c3038bf..d400939a2d 100644
@@ -179,9 +179,18 @@ struct mbuf_table {
 	struct rte_mbuf *m_table[MAX_PKT_BURST];
 };
 
+struct vhost_bufftable {
+	uint32_t len;
+	uint64_t pre_tsc;
+	struct rte_mbuf *m_table[MAX_PKT_BURST];
+};
+
 /* TX queue for each data core. */
 struct mbuf_table lcore_tx_queue[RTE_MAX_LCORE];
 
+/* TX queue for each vhost device. */
+struct vhost_bufftable vhost_bufftable[RTE_MAX_LCORE * MAX_VHOST_DEVICE];
+
 #define MBUF_TABLE_DRAIN_TSC	((rte_get_tsc_hz() + US_PER_S - 1) \
 				 / US_PER_S * BURST_TX_DRAIN_US)
 #define VLAN_HLEN       4
@@ -804,39 +813,82 @@ unlink_vmdq(struct vhost_dev *vdev)
 	}
 }
 
+static inline void
+free_pkts(struct rte_mbuf **pkts, uint16_t n)
+{
+	while (n--)
+		rte_pktmbuf_free(pkts[n]);
+}
+
 static __rte_always_inline void
-virtio_xmit(struct vhost_dev *dst_vdev, struct vhost_dev *src_vdev,
+complete_async_pkts(struct vhost_dev *vdev)
+{
+	struct rte_mbuf *p_cpl[MAX_PKT_BURST];
+	uint16_t complete_count;
+
+	complete_count = rte_vhost_poll_enqueue_completed(vdev->vid,
+					VIRTIO_RXQ, p_cpl, MAX_PKT_BURST);
+	if (complete_count) {
+		atomic_fetch_sub(&vdev->nr_async_pkts, complete_count);
+		free_pkts(p_cpl, complete_count);
+	}
+}
+
+static __rte_always_inline void
+sync_virtio_xmit(struct vhost_dev *dst_vdev, struct vhost_dev *src_vdev,
 	    struct rte_mbuf *m)
 {
 	uint16_t ret;
-	struct rte_mbuf *m_cpl[1];
 
 	if (builtin_net_driver) {
 		ret = vs_enqueue_pkts(dst_vdev, VIRTIO_RXQ, &m, 1);
-	} else if (async_vhost_driver) {
-		ret = rte_vhost_submit_enqueue_burst(dst_vdev->vid, VIRTIO_RXQ,
-						&m, 1);
-
-		if (likely(ret))
-			dst_vdev->nr_async_pkts++;
-
-		while (likely(dst_vdev->nr_async_pkts)) {
-			if (rte_vhost_poll_enqueue_completed(dst_vdev->vid,
-					VIRTIO_RXQ, m_cpl, 1))
-				dst_vdev->nr_async_pkts--;
-		}
 	} else {
 		ret = rte_vhost_enqueue_burst(dst_vdev->vid, VIRTIO_RXQ, &m, 1);
 	}
 
 	if (enable_stats) {
-		rte_atomic64_inc(&dst_vdev->stats.rx_total_atomic);
-		rte_atomic64_add(&dst_vdev->stats.rx_atomic, ret);
+		atomic_fetch_add(&dst_vdev->stats.rx_total_atomic, 1);
+		atomic_fetch_add(&dst_vdev->stats.rx_atomic, ret);
 		src_vdev->stats.tx_total++;
 		src_vdev->stats.tx += ret;
 	}
 }
 
+static __rte_always_inline void
+drain_vhost(struct vhost_dev *vdev)
+{
+	uint16_t ret;
+	uint64_t queue_id = rte_lcore_id() * MAX_VHOST_DEVICE + vdev->vid;
+	uint16_t nr_xmit = vhost_bufftable[queue_id].len;
+	struct rte_mbuf **m = vhost_bufftable[queue_id].m_table;
+
+	if (builtin_net_driver) {
+		ret = vs_enqueue_pkts(vdev, VIRTIO_RXQ, m, nr_xmit);
+	} else if (async_vhost_driver) {
+		uint32_t cpu_cpl_nr;
+		struct rte_mbuf *m_cpu_cpl[nr_xmit];
+		complete_async_pkts(vdev);
+		ret = rte_vhost_submit_enqueue_burst(vdev->vid, VIRTIO_RXQ,
+					m, nr_xmit, m_cpu_cpl, &cpu_cpl_nr);
+		atomic_fetch_add(&vdev->nr_async_pkts, ret - cpu_cpl_nr);
+		if (cpu_cpl_nr)
+			free_pkts(m_cpu_cpl, cpu_cpl_nr);
+		if (nr_xmit - ret)
+			free_pkts(&m[ret], nr_xmit - ret);
+	} else {
+		ret = rte_vhost_enqueue_burst(vdev->vid, VIRTIO_RXQ,
+						m, nr_xmit);
+	}
+
+	if (enable_stats) {
+		atomic_fetch_add(&vdev->stats.rx_total_atomic, nr_xmit);
+		atomic_fetch_add(&vdev->stats.rx_atomic, ret);
+	}
+
+	if (!async_vhost_driver)
+		free_pkts(m, nr_xmit);
+}
+
 /*
  * Check if the packet destination MAC address is for a local device. If so then put
  * the packet on that devices RX queue. If not then return.
@@ -846,7 +898,8 @@ virtio_tx_local(struct vhost_dev *vdev, struct rte_mbuf *m)
 {
 	struct rte_ether_hdr *pkt_hdr;
 	struct vhost_dev *dst_vdev;
-
+	struct vhost_bufftable *vhost_txq;
+	const uint16_t lcore_id = rte_lcore_id();
 	pkt_hdr = rte_pktmbuf_mtod(m, struct rte_ether_hdr *);
 
 	dst_vdev = find_vhost_dev(&pkt_hdr->d_addr);
@@ -869,7 +922,20 @@ virtio_tx_local(struct vhost_dev *vdev, struct rte_mbuf *m)
 		return 0;
 	}
 
-	virtio_xmit(dst_vdev, vdev, m);
+	vhost_txq = &vhost_bufftable[lcore_id * MAX_VHOST_DEVICE
+					+ dst_vdev->vid];
+	vhost_txq->m_table[vhost_txq->len++] = m;
+
+	if (enable_stats) {
+		vdev->stats.tx_total++;
+		vdev->stats.tx++;
+	}
+
+	if (unlikely(vhost_txq->len == MAX_PKT_BURST)) {
+		drain_vhost(dst_vdev);
+		vhost_txq->len = 0;
+		vhost_txq->pre_tsc = rte_rdtsc();
+	}
 	return 0;
 }
 
@@ -940,13 +1006,6 @@ static void virtio_tx_offload(struct rte_mbuf *m)
 	tcp_hdr->cksum = get_psd_sum(l3_hdr, m->ol_flags);
 }
 
-static inline void
-free_pkts(struct rte_mbuf **pkts, uint16_t n)
-{
-	while (n--)
-		rte_pktmbuf_free(pkts[n]);
-}
-
 static __rte_always_inline void
 do_drain_mbuf_table(struct mbuf_table *tx_q)
 {
@@ -979,14 +1038,13 @@ virtio_tx_route(struct vhost_dev *vdev, struct rte_mbuf *m, uint16_t vlan_tag)
 
 		TAILQ_FOREACH(vdev2, &vhost_dev_list, global_vdev_entry) {
 			if (vdev2 != vdev)
-				virtio_xmit(vdev2, vdev, m);
+				sync_virtio_xmit(vdev2, vdev, m);
 		}
 		goto queue2nic;
 	}
 
 	/*check if destination is local VM*/
 	if ((vm2vm_mode == VM2VM_SOFTWARE) && (virtio_tx_local(vdev, m) == 0)) {
-		rte_pktmbuf_free(m);
 		return;
 	}
 
@@ -1073,19 +1131,6 @@ drain_mbuf_table(struct mbuf_table *tx_q)
 	}
 }
 
-static __rte_always_inline void
-complete_async_pkts(struct vhost_dev *vdev, uint16_t qid)
-{
-	struct rte_mbuf *p_cpl[MAX_PKT_BURST];
-	uint16_t complete_count;
-
-	complete_count = rte_vhost_poll_enqueue_completed(vdev->vid,
-						qid, p_cpl, MAX_PKT_BURST);
-	vdev->nr_async_pkts -= complete_count;
-	if (complete_count)
-		free_pkts(p_cpl, complete_count);
-}
-
 static __rte_always_inline void
 drain_eth_rx(struct vhost_dev *vdev)
 {
@@ -1095,9 +1140,6 @@ drain_eth_rx(struct vhost_dev *vdev)
 	rx_count = rte_eth_rx_burst(ports[0], vdev->vmdq_rx_q,
 				    pkts, MAX_PKT_BURST);
 
-	while (likely(vdev->nr_async_pkts))
-		complete_async_pkts(vdev, VIRTIO_RXQ);
-
 	if (!rx_count)
 		return;
 
@@ -1123,17 +1165,26 @@ drain_eth_rx(struct vhost_dev *vdev)
 		enqueue_count = vs_enqueue_pkts(vdev, VIRTIO_RXQ,
 						pkts, rx_count);
 	} else if (async_vhost_driver) {
+		uint32_t cpu_cpl_nr;
+		struct rte_mbuf *m_cpu_cpl[MAX_PKT_BURST];
+		complete_async_pkts(vdev);
 		enqueue_count = rte_vhost_submit_enqueue_burst(vdev->vid,
-					VIRTIO_RXQ, pkts, rx_count);
-		vdev->nr_async_pkts += enqueue_count;
+					VIRTIO_RXQ, pkts, rx_count,
+					m_cpu_cpl, &cpu_cpl_nr);
+		atomic_fetch_add(&vdev->nr_async_pkts,
+					enqueue_count - cpu_cpl_nr);
+		if (enqueue_count > cpu_cpl_nr)
+			free_pkts(m_cpu_cpl, cpu_cpl_nr);
+		free_pkts(&pkts[enqueue_count], rx_count - enqueue_count);
+
 	} else {
 		enqueue_count = rte_vhost_enqueue_burst(vdev->vid, VIRTIO_RXQ,
 						pkts, rx_count);
 	}
 
 	if (enable_stats) {
-		rte_atomic64_add(&vdev->stats.rx_total_atomic, rx_count);
-		rte_atomic64_add(&vdev->stats.rx_atomic, enqueue_count);
+		atomic_fetch_add(&vdev->stats.rx_total_atomic, rx_count);
+		atomic_fetch_add(&vdev->stats.rx_atomic, enqueue_count);
 	}
 
 	if (!async_vhost_driver)
@@ -1144,8 +1195,11 @@ static __rte_always_inline void
 drain_virtio_tx(struct vhost_dev *vdev)
 {
 	struct rte_mbuf *pkts[MAX_PKT_BURST];
+	struct vhost_bufftable *vhost_txq;
+	const uint16_t lcore_id = rte_lcore_id();
 	uint16_t count;
 	uint16_t i;
+	uint64_t cur_tsc;
 
 	if (builtin_net_driver) {
 		count = vs_dequeue_pkts(vdev, VIRTIO_TXQ, mbuf_pool,
@@ -1163,6 +1217,17 @@ drain_virtio_tx(struct vhost_dev *vdev)
 
 	for (i = 0; i < count; ++i)
 		virtio_tx_route(vdev, pkts[i], vlan_tags[vdev->vid]);
+
+	vhost_txq = &vhost_bufftable[lcore_id * MAX_VHOST_DEVICE + vdev->vid];
+	cur_tsc = rte_rdtsc();
+	if (unlikely(cur_tsc - vhost_txq->pre_tsc > MBUF_TABLE_DRAIN_TSC)) {
+		RTE_LOG_DP(DEBUG, VHOST_DATA,
+			"Vhost tX queue drained after timeout with burst size %u\n",
+			vhost_txq->len);
+		drain_vhost(vdev);
+		vhost_txq->len = 0;
+		vhost_txq->pre_tsc = cur_tsc;
+	}
 }
 
 /*
@@ -1392,8 +1457,8 @@ print_stats(__rte_unused void *arg)
 			tx         = vdev->stats.tx;
 			tx_dropped = tx_total - tx;
 
-			rx_total   = rte_atomic64_read(&vdev->stats.rx_total_atomic);
-			rx         = rte_atomic64_read(&vdev->stats.rx_atomic);
+			rx_total   = atomic_load(&vdev->stats.rx_total_atomic);
+			rx         = atomic_load(&vdev->stats.rx_atomic);
 			rx_dropped = rx_total - rx;
 
 			printf("Statistics for device %d\n"
examples/vhost/main.h +4 -3
index 4317b6ae81..6aa798a3e2 100644
@@ -8,6 +8,7 @@
 #include <sys/queue.h>
 
 #include <rte_ether.h>
+#include <stdatomic.h>
 
 /* Macros for printing using RTE_LOG */
 #define RTE_LOGTYPE_VHOST_CONFIG RTE_LOGTYPE_USER1
@@ -21,8 +22,8 @@ enum {VIRTIO_RXQ, VIRTIO_TXQ, VIRTIO_QNUM};
 struct device_statistics {
 	uint64_t	tx;
 	uint64_t	tx_total;
-	rte_atomic64_t	rx_atomic;
-	rte_atomic64_t	rx_total_atomic;
+	atomic_int_least64_t	rx_atomic;
+	atomic_int_least64_t	rx_total_atomic;
 };
 
 struct vhost_queue {
@@ -51,7 +52,7 @@ struct vhost_dev {
 	uint64_t features;
 	size_t hdr_len;
 	uint16_t nr_vrings;
-	uint16_t nr_async_pkts;
+	atomic_int_least16_t nr_async_pkts;
 	struct rte_vhost_memory *mem;
 	struct device_statistics stats;
 	TAILQ_ENTRY(vhost_dev) global_vdev_entry;
Signature · 1 line
2.29.2
#7 lore raw

v3 cover examples/vhost: sample code refactor

In reply to Cheng Jiang

Refactor the vhost sample code. Add ioat ring space count and check in ioat callback, optimize vhost data path for batch enqueue, replase rte_atomicNN_xxx to atomic_XXX and refactor vhost async data path.
---
v3:
* added some variable initiation
* cleaned some codes

v2:
* optimized patch structure
* optimized git log
* replased rte_atomicNN_xxx to atomic_XXX

Cheng Jiang (2):
examples/vhost: add ioat ring space count and check examples/vhost: refactor vhost data path

 examples/vhost/ioat.c |  15 ++--
 examples/vhost/main.c | 168 ++++++++++++++++++++++++++++++------------
 examples/vhost/main.h |   7 +-
 3 files changed, 130 insertions(+), 60 deletions(-)

--
2.29.2

#8 lore raw

PATCH v3 1/2 examples/vhost: add ioat ring space count and check

Add ioat ring space count and check, if ioat ring space is not enough for the next async vhost packet enqueue, then just return to prevent enqueue failure.

---
 examples/vhost/ioat.c | 15 +++++++--------
 1 file changed, 7 insertions(+), 8 deletions(-)
examples/vhost/ioat.c +7 -8
index 71d8a1f1f..b0b04aa45 100644
@@ -17,6 +17,7 @@ struct packet_tracker {
 	unsigned short next_read;
 	unsigned short next_write;
 	unsigned short last_remain;
+	unsigned short ioat_space;
 };
 
 struct packet_tracker cb_tracker[MAX_VHOST_DEVICE];
@@ -113,7 +114,7 @@ open_ioat(const char *value)
 			goto out;
 		}
 		rte_rawdev_start(dev_id);
-
+		cb_tracker[dev_id].ioat_space = IOAT_RING_SIZE;
 		dma_info->nr++;
 		i++;
 	}
@@ -140,13 +141,9 @@ ioat_transfer_data_cb(int vid, uint16_t queue_id,
 			src = descs[i_desc].src;
 			dst = descs[i_desc].dst;
 			i_seg = 0;
+			if (cb_tracker[dev_id].ioat_space < src->nr_segs)
+				break;
 			while (i_seg < src->nr_segs) {
-				/*
-				 * TODO: Assuming that the ring space of the
-				 * IOAT device is large enough, so there is no
-				 * error here, and the actual error handling
-				 * will be added later.
-				 */
 				rte_ioat_enqueue_copy(dev_id,
 					(uintptr_t)(src->iov[i_seg].iov_base)
 						+ src->offset,
@@ -158,7 +155,8 @@ ioat_transfer_data_cb(int vid, uint16_t queue_id,
 				i_seg++;
 			}
 			write &= mask;
-			cb_tracker[dev_id].size_track[write] = i_seg;
+			cb_tracker[dev_id].size_track[write] = src->nr_segs;
+			cb_tracker[dev_id].ioat_space -= src->nr_segs;
 			write++;
 		}
 	} else {
@@ -186,6 +184,7 @@ ioat_check_completed_copies_cb(int vid, uint16_t queue_id,
 		int dev_id = dma_bind[vid].dmas[queue_id * 2
 				+ VIRTIO_RXQ].dev_id;
 		n_seg = rte_ioat_completed_ops(dev_id, 255, dump, dump);
+		cb_tracker[dev_id].ioat_space += n_seg;
 		n_seg += cb_tracker[dev_id].last_remain;
 		if (!n_seg)
 			return 0;
Signature · 1 line
2.29.2
#9 lore raw

PATCH v3 2/2 examples/vhost: refactor vhost data path

In reply to Cheng Jiang

Change the vm2vm data path to batch enqueue for better performance. Support latest async vhost API, refactor vhost async data path, replase rte_atomicNN_xxx to atomic_XXX and clean some codes.

---
 examples/vhost/main.c | 168 ++++++++++++++++++++++++++++++------------
 examples/vhost/main.h |   7 +-
 2 files changed, 123 insertions(+), 52 deletions(-)
examples/vhost/main.c +119 -49
index 8d8c3038b..efc044b28 100644
@@ -179,9 +179,18 @@ struct mbuf_table {
 	struct rte_mbuf *m_table[MAX_PKT_BURST];
 };
 
+struct vhost_bufftable {
+	uint32_t len;
+	uint64_t pre_tsc;
+	struct rte_mbuf *m_table[MAX_PKT_BURST];
+};
+
 /* TX queue for each data core. */
 struct mbuf_table lcore_tx_queue[RTE_MAX_LCORE];
 
+/* TX queue for each vhost device. */
+struct vhost_bufftable vhost_bufftable[RTE_MAX_LCORE * MAX_VHOST_DEVICE];
+
 #define MBUF_TABLE_DRAIN_TSC	((rte_get_tsc_hz() + US_PER_S - 1) \
 				 / US_PER_S * BURST_TX_DRAIN_US)
 #define VLAN_HLEN       4
@@ -804,39 +813,84 @@ unlink_vmdq(struct vhost_dev *vdev)
 	}
 }
 
+static inline void
+free_pkts(struct rte_mbuf **pkts, uint16_t n)
+{
+	while (n--)
+		rte_pktmbuf_free(pkts[n]);
+}
+
 static __rte_always_inline void
-virtio_xmit(struct vhost_dev *dst_vdev, struct vhost_dev *src_vdev,
+complete_async_pkts(struct vhost_dev *vdev)
+{
+	struct rte_mbuf *p_cpl[MAX_PKT_BURST];
+	uint16_t complete_count;
+
+	complete_count = rte_vhost_poll_enqueue_completed(vdev->vid,
+					VIRTIO_RXQ, p_cpl, MAX_PKT_BURST);
+	if (complete_count) {
+		atomic_fetch_sub(&vdev->nr_async_pkts, complete_count);
+		free_pkts(p_cpl, complete_count);
+	}
+}
+
+static __rte_always_inline void
+sync_virtio_xmit(struct vhost_dev *dst_vdev, struct vhost_dev *src_vdev,
 	    struct rte_mbuf *m)
 {
 	uint16_t ret;
-	struct rte_mbuf *m_cpl[1];
 
 	if (builtin_net_driver) {
 		ret = vs_enqueue_pkts(dst_vdev, VIRTIO_RXQ, &m, 1);
-	} else if (async_vhost_driver) {
-		ret = rte_vhost_submit_enqueue_burst(dst_vdev->vid, VIRTIO_RXQ,
-						&m, 1);
-
-		if (likely(ret))
-			dst_vdev->nr_async_pkts++;
-
-		while (likely(dst_vdev->nr_async_pkts)) {
-			if (rte_vhost_poll_enqueue_completed(dst_vdev->vid,
-					VIRTIO_RXQ, m_cpl, 1))
-				dst_vdev->nr_async_pkts--;
-		}
 	} else {
 		ret = rte_vhost_enqueue_burst(dst_vdev->vid, VIRTIO_RXQ, &m, 1);
 	}
 
 	if (enable_stats) {
-		rte_atomic64_inc(&dst_vdev->stats.rx_total_atomic);
-		rte_atomic64_add(&dst_vdev->stats.rx_atomic, ret);
+		atomic_fetch_add(&dst_vdev->stats.rx_total_atomic, 1);
+		atomic_fetch_add(&dst_vdev->stats.rx_atomic, ret);
 		src_vdev->stats.tx_total++;
 		src_vdev->stats.tx += ret;
 	}
 }
 
+static __rte_always_inline void
+drain_vhost(struct vhost_dev *vdev)
+{
+	uint16_t ret;
+	uint64_t queue_id = rte_lcore_id() * MAX_VHOST_DEVICE + vdev->vid;
+	uint16_t nr_xmit = vhost_bufftable[queue_id].len;
+	struct rte_mbuf **m = vhost_bufftable[queue_id].m_table;
+
+	if (builtin_net_driver) {
+		ret = vs_enqueue_pkts(vdev, VIRTIO_RXQ, m, nr_xmit);
+	} else if (async_vhost_driver) {
+		uint32_t cpu_cpl_nr = 0;
+		uint16_t enqueue_fail = 0;
+		struct rte_mbuf *m_cpu_cpl[nr_xmit];
+		complete_async_pkts(vdev);
+		ret = rte_vhost_submit_enqueue_burst(vdev->vid, VIRTIO_RXQ,
+					m, nr_xmit, m_cpu_cpl, &cpu_cpl_nr);
+		atomic_fetch_add(&vdev->nr_async_pkts, ret - cpu_cpl_nr);
+		if (cpu_cpl_nr)
+			free_pkts(m_cpu_cpl, cpu_cpl_nr);
+		enqueue_fail = nr_xmit - ret;
+		if (enqueue_fail)
+			free_pkts(&m[ret], nr_xmit - ret);
+	} else {
+		ret = rte_vhost_enqueue_burst(vdev->vid, VIRTIO_RXQ,
+						m, nr_xmit);
+	}
+
+	if (enable_stats) {
+		atomic_fetch_add(&vdev->stats.rx_total_atomic, nr_xmit);
+		atomic_fetch_add(&vdev->stats.rx_atomic, ret);
+	}
+
+	if (!async_vhost_driver)
+		free_pkts(m, nr_xmit);
+}
+
 /*
  * Check if the packet destination MAC address is for a local device. If so then put
  * the packet on that devices RX queue. If not then return.
@@ -846,7 +900,8 @@ virtio_tx_local(struct vhost_dev *vdev, struct rte_mbuf *m)
 {
 	struct rte_ether_hdr *pkt_hdr;
 	struct vhost_dev *dst_vdev;
-
+	struct vhost_bufftable *vhost_txq;
+	const uint16_t lcore_id = rte_lcore_id();
 	pkt_hdr = rte_pktmbuf_mtod(m, struct rte_ether_hdr *);
 
 	dst_vdev = find_vhost_dev(&pkt_hdr->d_addr);
@@ -869,7 +924,20 @@ virtio_tx_local(struct vhost_dev *vdev, struct rte_mbuf *m)
 		return 0;
 	}
 
-	virtio_xmit(dst_vdev, vdev, m);
+	vhost_txq = &vhost_bufftable[lcore_id * MAX_VHOST_DEVICE
+					+ dst_vdev->vid];
+	vhost_txq->m_table[vhost_txq->len++] = m;
+
+	if (enable_stats) {
+		vdev->stats.tx_total++;
+		vdev->stats.tx++;
+	}
+
+	if (unlikely(vhost_txq->len == MAX_PKT_BURST)) {
+		drain_vhost(dst_vdev);
+		vhost_txq->len = 0;
+		vhost_txq->pre_tsc = rte_rdtsc();
+	}
 	return 0;
 }
 
@@ -940,13 +1008,6 @@ static void virtio_tx_offload(struct rte_mbuf *m)
 	tcp_hdr->cksum = get_psd_sum(l3_hdr, m->ol_flags);
 }
 
-static inline void
-free_pkts(struct rte_mbuf **pkts, uint16_t n)
-{
-	while (n--)
-		rte_pktmbuf_free(pkts[n]);
-}
-
 static __rte_always_inline void
 do_drain_mbuf_table(struct mbuf_table *tx_q)
 {
@@ -979,14 +1040,13 @@ virtio_tx_route(struct vhost_dev *vdev, struct rte_mbuf *m, uint16_t vlan_tag)
 
 		TAILQ_FOREACH(vdev2, &vhost_dev_list, global_vdev_entry) {
 			if (vdev2 != vdev)
-				virtio_xmit(vdev2, vdev, m);
+				sync_virtio_xmit(vdev2, vdev, m);
 		}
 		goto queue2nic;
 	}
 
 	/*check if destination is local VM*/
 	if ((vm2vm_mode == VM2VM_SOFTWARE) && (virtio_tx_local(vdev, m) == 0)) {
-		rte_pktmbuf_free(m);
 		return;
 	}
 
@@ -1073,19 +1133,6 @@ drain_mbuf_table(struct mbuf_table *tx_q)
 	}
 }
 
-static __rte_always_inline void
-complete_async_pkts(struct vhost_dev *vdev, uint16_t qid)
-{
-	struct rte_mbuf *p_cpl[MAX_PKT_BURST];
-	uint16_t complete_count;
-
-	complete_count = rte_vhost_poll_enqueue_completed(vdev->vid,
-						qid, p_cpl, MAX_PKT_BURST);
-	vdev->nr_async_pkts -= complete_count;
-	if (complete_count)
-		free_pkts(p_cpl, complete_count);
-}
-
 static __rte_always_inline void
 drain_eth_rx(struct vhost_dev *vdev)
 {
@@ -1095,9 +1142,6 @@ drain_eth_rx(struct vhost_dev *vdev)
 	rx_count = rte_eth_rx_burst(ports[0], vdev->vmdq_rx_q,
 				    pkts, MAX_PKT_BURST);
 
-	while (likely(vdev->nr_async_pkts))
-		complete_async_pkts(vdev, VIRTIO_RXQ);
-
 	if (!rx_count)
 		return;
 
@@ -1123,17 +1167,29 @@ drain_eth_rx(struct vhost_dev *vdev)
 		enqueue_count = vs_enqueue_pkts(vdev, VIRTIO_RXQ,
 						pkts, rx_count);
 	} else if (async_vhost_driver) {
+		uint32_t cpu_cpl_nr = 0;
+		uint16_t enqueue_fail = 0;
+		struct rte_mbuf *m_cpu_cpl[MAX_PKT_BURST];
+		complete_async_pkts(vdev);
 		enqueue_count = rte_vhost_submit_enqueue_burst(vdev->vid,
-					VIRTIO_RXQ, pkts, rx_count);
-		vdev->nr_async_pkts += enqueue_count;
+					VIRTIO_RXQ, pkts, rx_count,
+					m_cpu_cpl, &cpu_cpl_nr);
+		atomic_fetch_add(&vdev->nr_async_pkts,
+					enqueue_count - cpu_cpl_nr);
+		if (cpu_cpl_nr)
+			free_pkts(m_cpu_cpl, cpu_cpl_nr);
+		enqueue_fail = rx_count - enqueue_count;
+		if (enqueue_fail)
+			free_pkts(&pkts[enqueue_count], enqueue_fail);
+
 	} else {
 		enqueue_count = rte_vhost_enqueue_burst(vdev->vid, VIRTIO_RXQ,
 						pkts, rx_count);
 	}
 
 	if (enable_stats) {
-		rte_atomic64_add(&vdev->stats.rx_total_atomic, rx_count);
-		rte_atomic64_add(&vdev->stats.rx_atomic, enqueue_count);
+		atomic_fetch_add(&vdev->stats.rx_total_atomic, rx_count);
+		atomic_fetch_add(&vdev->stats.rx_atomic, enqueue_count);
 	}
 
 	if (!async_vhost_driver)
@@ -1144,8 +1200,11 @@ static __rte_always_inline void
 drain_virtio_tx(struct vhost_dev *vdev)
 {
 	struct rte_mbuf *pkts[MAX_PKT_BURST];
+	struct vhost_bufftable *vhost_txq;
+	const uint16_t lcore_id = rte_lcore_id();
 	uint16_t count;
 	uint16_t i;
+	uint64_t cur_tsc;
 
 	if (builtin_net_driver) {
 		count = vs_dequeue_pkts(vdev, VIRTIO_TXQ, mbuf_pool,
@@ -1163,6 +1222,17 @@ drain_virtio_tx(struct vhost_dev *vdev)
 
 	for (i = 0; i < count; ++i)
 		virtio_tx_route(vdev, pkts[i], vlan_tags[vdev->vid]);
+
+	vhost_txq = &vhost_bufftable[lcore_id * MAX_VHOST_DEVICE + vdev->vid];
+	cur_tsc = rte_rdtsc();
+	if (unlikely(cur_tsc - vhost_txq->pre_tsc > MBUF_TABLE_DRAIN_TSC)) {
+		RTE_LOG_DP(DEBUG, VHOST_DATA,
+			"Vhost tX queue drained after timeout with burst size %u\n",
+			vhost_txq->len);
+		drain_vhost(vdev);
+		vhost_txq->len = 0;
+		vhost_txq->pre_tsc = cur_tsc;
+	}
 }
 
 /*
@@ -1392,8 +1462,8 @@ print_stats(__rte_unused void *arg)
 			tx         = vdev->stats.tx;
 			tx_dropped = tx_total - tx;
 
-			rx_total   = rte_atomic64_read(&vdev->stats.rx_total_atomic);
-			rx         = rte_atomic64_read(&vdev->stats.rx_atomic);
+			rx_total   = atomic_load(&vdev->stats.rx_total_atomic);
+			rx         = atomic_load(&vdev->stats.rx_atomic);
 			rx_dropped = rx_total - rx;
 
 			printf("Statistics for device %d\n"
examples/vhost/main.h +4 -3
index 4317b6ae8..6aa798a3e 100644
@@ -8,6 +8,7 @@
 #include <sys/queue.h>
 
 #include <rte_ether.h>
+#include <stdatomic.h>
 
 /* Macros for printing using RTE_LOG */
 #define RTE_LOGTYPE_VHOST_CONFIG RTE_LOGTYPE_USER1
@@ -21,8 +22,8 @@ enum {VIRTIO_RXQ, VIRTIO_TXQ, VIRTIO_QNUM};
 struct device_statistics {
 	uint64_t	tx;
 	uint64_t	tx_total;
-	rte_atomic64_t	rx_atomic;
-	rte_atomic64_t	rx_total_atomic;
+	atomic_int_least64_t	rx_atomic;
+	atomic_int_least64_t	rx_total_atomic;
 };
 
 struct vhost_queue {
@@ -51,7 +52,7 @@ struct vhost_dev {
 	uint64_t features;
 	size_t hdr_len;
 	uint16_t nr_vrings;
-	uint16_t nr_async_pkts;
+	atomic_int_least16_t nr_async_pkts;
 	struct rte_vhost_memory *mem;
 	struct device_statistics stats;
 	TAILQ_ENTRY(vhost_dev) global_vdev_entry;
Signature · 1 line
2.29.2
#10 lore raw

v4 cover examples/vhost: sample code refactor

In reply to Cheng Jiang

Refactor the vhost sample code. Add ioat ring space count and check in ioat callback, optimize vhost data path for batch enqueue, replase rte_atomicNN_xxx to atomic_XXX and refactor vhost async data path.
---
v4:
* improved code structure
* improved vhost enqueue buffer memory allocation
* cleaned some codes

v3:
* added some variable initiation
* cleaned some codes

v2:
* optimized patch structure
* optimized git log
* replased rte_atomicNN_xxx to atomic_XXX

Cheng Jiang (2):
examples/vhost: add ioat ring space count and check examples/vhost: refactor vhost data path

 examples/vhost/ioat.c |  15 ++--
 examples/vhost/main.c | 202 +++++++++++++++++++++++++++++++-----------
 examples/vhost/main.h |   7 +-
 3 files changed, 161 insertions(+), 63 deletions(-)

--
2.29.2

#11 lore raw

PATCH v4 1/2 examples/vhost: add ioat ring space count and check

Add ioat ring space count and check, if ioat ring space is not enough for the next async vhost packet enqueue, then just return to prevent enqueue failure.

---
 examples/vhost/ioat.c | 15 +++++++--------
 1 file changed, 7 insertions(+), 8 deletions(-)
examples/vhost/ioat.c +7 -8
index 71d8a1f1f..b0b04aa45 100644
@@ -17,6 +17,7 @@ struct packet_tracker {
 	unsigned short next_read;
 	unsigned short next_write;
 	unsigned short last_remain;
+	unsigned short ioat_space;
 };
 
 struct packet_tracker cb_tracker[MAX_VHOST_DEVICE];
@@ -113,7 +114,7 @@ open_ioat(const char *value)
 			goto out;
 		}
 		rte_rawdev_start(dev_id);
-
+		cb_tracker[dev_id].ioat_space = IOAT_RING_SIZE;
 		dma_info->nr++;
 		i++;
 	}
@@ -140,13 +141,9 @@ ioat_transfer_data_cb(int vid, uint16_t queue_id,
 			src = descs[i_desc].src;
 			dst = descs[i_desc].dst;
 			i_seg = 0;
+			if (cb_tracker[dev_id].ioat_space < src->nr_segs)
+				break;
 			while (i_seg < src->nr_segs) {
-				/*
-				 * TODO: Assuming that the ring space of the
-				 * IOAT device is large enough, so there is no
-				 * error here, and the actual error handling
-				 * will be added later.
-				 */
 				rte_ioat_enqueue_copy(dev_id,
 					(uintptr_t)(src->iov[i_seg].iov_base)
 						+ src->offset,
@@ -158,7 +155,8 @@ ioat_transfer_data_cb(int vid, uint16_t queue_id,
 				i_seg++;
 			}
 			write &= mask;
-			cb_tracker[dev_id].size_track[write] = i_seg;
+			cb_tracker[dev_id].size_track[write] = src->nr_segs;
+			cb_tracker[dev_id].ioat_space -= src->nr_segs;
 			write++;
 		}
 	} else {
@@ -186,6 +184,7 @@ ioat_check_completed_copies_cb(int vid, uint16_t queue_id,
 		int dev_id = dma_bind[vid].dmas[queue_id * 2
 				+ VIRTIO_RXQ].dev_id;
 		n_seg = rte_ioat_completed_ops(dev_id, 255, dump, dump);
+		cb_tracker[dev_id].ioat_space += n_seg;
 		n_seg += cb_tracker[dev_id].last_remain;
 		if (!n_seg)
 			return 0;
Signature · 1 line
2.29.2
#12 lore raw

Re: [dpdk-dev] [PATCH v4 1/2] examples/vhost: add ioat ring space count and check

Hi Cheng,

Quoted 65 lines

-----Original Message-----
From: Jiang, Cheng1 <[email protected]>
Sent: Friday, December 25, 2020 4:07 PM
To: [email protected]; Xia, Chenbo <[email protected]>
Cc: [email protected]; Hu, Jiayu <[email protected]>; Yang, YvonneX <[email protected]>; Jiang, Cheng1 <[email protected]>
Subject: [PATCH v4 1/2] examples/vhost: add ioat ring space count and check

Add ioat ring space count and check, if ioat ring space is not enough for the next async vhost packet enqueue, then just return to prevent enqueue failure.

---
 examples/vhost/ioat.c | 15 +++++++--------
 1 file changed, 7 insertions(+), 8 deletions(-)
examples/vhost/ioat.c +6 -8
index 71d8a1f1f..b0b04aa45 100644
@@ -17,6 +17,7 @@ struct packet_tracker {
 	unsigned short next_read;
 	unsigned short next_write;
 	unsigned short last_remain;
+	unsigned short ioat_space;
 };
 
 struct packet_tracker cb_tracker[MAX_VHOST_DEVICE];
@@ -113,7 +114,7 @@ open_ioat(const char *value)
 			goto out;
 		}
 		rte_rawdev_start(dev_id);
-
+		cb_tracker[dev_id].ioat_space = IOAT_RING_SIZE;
 		dma_info->nr++;
 		i++;
 	}
@@ -140,13 +141,9 @@ ioat_transfer_data_cb(int vid, uint16_t queue_id,
 			src = descs[i_desc].src;
 			dst = descs[i_desc].dst;
 			i_seg = 0;
+			if (cb_tracker[dev_id].ioat_space < src->nr_segs)
+				break;
 			while (i_seg < src->nr_segs) {
-				/*
-				 * TODO: Assuming that the ring space of the
-				 * IOAT device is large enough, so there is no
-				 * error here, and the actual error handling
-				 * will be added later.
-				 */
 				rte_ioat_enqueue_copy(dev_id,
 					(uintptr_t)(src->iov[i_seg].iov_base)
 						+ src->offset,
@@ -158,7 +155,8 @@ ioat_transfer_data_cb(int vid, uint16_t queue_id,
 				i_seg++;
 			}
 			write &= mask;
-			cb_tracker[dev_id].size_track[write] = i_seg;
+			cb_tracker[dev_id].size_track[write] = src->nr_segs;
+			cb_tracker[dev_id].ioat_space -= src->nr_segs;
 			write++;
 		}
 	} else {

examples/vhost/ioat.c ioat_check_completed_copies_cb(int vid, uint16_t · line 188

Quoted 5 lines
examples/vhost/ioat.c +1 -0
@@ -186,6 +184,7 @@ ioat_check_completed_copies_cb(int vid, uint16_t
 queue_id,
 		int dev_id = dma_bind[vid].dmas[queue_id * 2
 				+ VIRTIO_RXQ].dev_id;
 		n_seg = rte_ioat_completed_ops(dev_id, 255, dump, dump);
+		cb_tracker[dev_id].ioat_space += n_seg;

rte_ioat_completed_ops() may fail. In this case, its return value is -1, which will cause n_seg to 65534.

Thanks,
Jiayu

Quoted 5 lines
examples/vhost/ioat.c +0 -0
 		n_seg += cb_tracker[dev_id].last_remain;
 		if (!n_seg)

return 0;
--
2.29.2

#13 lore raw

Re: [dpdk-dev] [PATCH v4 1/2] examples/vhost: add ioat ring space count and check

Hi Jiayu,

Quoted 67 lines

-----Original Message-----
From: Hu, Jiayu <[email protected]>
Sent: Monday, December 28, 2020 10:51 AM
To: Jiang, Cheng1 <[email protected]>; [email protected]; Xia, Chenbo <[email protected]>
Cc: [email protected]; Yang, YvonneX <[email protected]>
Subject: RE: [PATCH v4 1/2] examples/vhost: add ioat ring space count and check

Hi Cheng,

Quoted 56 lines

-----Original Message-----
From: Jiang, Cheng1 <[email protected]>
Sent: Friday, December 25, 2020 4:07 PM
To: [email protected]; Xia, Chenbo <[email protected]>
Cc: [email protected]; Hu, Jiayu <[email protected]>; Yang, YvonneX <[email protected]>; Jiang, Cheng1 <[email protected]>
Subject: [PATCH v4 1/2] examples/vhost: add ioat ring space count and check

Add ioat ring space count and check, if ioat ring space is not enough for the next async vhost packet enqueue, then just return to prevent enqueue failure.

---
 examples/vhost/ioat.c | 15 +++++++--------
 1 file changed, 7 insertions(+), 8 deletions(-)
examples/vhost/ioat.c index renamed from examples/vhost/ioat.c +0 -0

71d8a1f1f..b0b04aa45 100644

examples/vhost/ioat.c +3 -7
@@ -17,6 +17,7 @@ struct packet_tracker {  unsigned short next_read;
 unsigned short next_write;  unsigned short last_remain;
+unsigned short ioat_space;
 };
 
 struct packet_tracker cb_tracker[MAX_VHOST_DEVICE]; @@ -113,7 +114,7
 @@ open_ioat(const char *value)  goto out;  }
 rte_rawdev_start(dev_id);
-
+cb_tracker[dev_id].ioat_space = IOAT_RING_SIZE;
 dma_info->nr++;
 i++;
 }
@@ -140,13 +141,9 @@ ioat_transfer_data_cb(int vid, uint16_t queue_id,
 src = descs[i_desc].src;  dst = descs[i_desc].dst;  i_seg = 0;
+if (cb_tracker[dev_id].ioat_space < src->nr_segs) break;
 while (i_seg < src->nr_segs) {
-/*
- * TODO: Assuming that the ring space of the
- * IOAT device is large enough, so there is no
- * error here, and the actual error handling
- * will be added later.
- */
 rte_ioat_enqueue_copy(dev_id,
 (uintptr_t)(src->iov[i_seg].iov_base)
 + src->offset,
@@ -158,7 +155,8 @@ ioat_transfer_data_cb(int vid, uint16_t queue_id,

i_seg++; } write &= mask; -cb_tracker[dev_id].size_track[write] = i_seg;

examples/vhost/ioat.c +2 -0
+cb_tracker[dev_id].size_track[write] = src->nr_segs;
+cb_tracker[dev_id].ioat_space -= src->nr_segs;
 write++;
 }
 } else {
Quoted 8 lines
Quoted 5 lines
examples/vhost/ioat.c +0 -0
@@ -186,6 +184,7 @@ ioat_check_completed_copies_cb(int vid, uint16_t

queue_id, int dev_id = dma_bind[vid].dmas[queue_id * 2 + VIRTIO_RXQ].dev_id; n_seg = rte_ioat_completed_ops(dev_id, 255, dump, dump);
+cb_tracker[dev_id].ioat_space += n_seg;

rte_ioat_completed_ops() may fail. In this case, its return value is -1, which will cause n_seg to 65534.

I will change it in the next version.

Thanks.
Cheng

Quoted 8 lines

Thanks,
Jiayu

Quoted 3 lines

n_seg += cb_tracker[dev_id].last_remain; if (!n_seg) return 0;
--
2.29.2

#14 lore raw

PATCH v4 2/2 examples/vhost: refactor vhost data path

In reply to Cheng Jiang

Change the vm2vm data path to batch enqueue for better performance. Support latest async vhost API, refactor vhost async data path, replase rte_atomicNN_xxx to atomic_XXX and clean some codes.

---
 examples/vhost/main.c | 202 +++++++++++++++++++++++++++++++-----------
 examples/vhost/main.h |   7 +-
 2 files changed, 154 insertions(+), 55 deletions(-)
examples/vhost/main.c +150 -52
index 8d8c3038b..3ea12a474 100644
@@ -179,9 +179,18 @@ struct mbuf_table {
 	struct rte_mbuf *m_table[MAX_PKT_BURST];
 };
 
+struct vhost_bufftable {
+	uint32_t len;
+	uint64_t pre_tsc;
+	struct rte_mbuf *m_table[MAX_PKT_BURST];
+};
+
 /* TX queue for each data core. */
 struct mbuf_table lcore_tx_queue[RTE_MAX_LCORE];
 
+/* TX queue for each vhost device. */
+struct vhost_bufftable *vhost_txbuff[RTE_MAX_LCORE * MAX_VHOST_DEVICE];
+
 #define MBUF_TABLE_DRAIN_TSC	((rte_get_tsc_hz() + US_PER_S - 1) \
 				 / US_PER_S * BURST_TX_DRAIN_US)
 #define VLAN_HLEN       4
@@ -804,39 +813,114 @@ unlink_vmdq(struct vhost_dev *vdev)
 	}
 }
 
+static inline void
+free_pkts(struct rte_mbuf **pkts, uint16_t n)
+{
+	while (n--)
+		rte_pktmbuf_free(pkts[n]);
+}
+
 static __rte_always_inline void
-virtio_xmit(struct vhost_dev *dst_vdev, struct vhost_dev *src_vdev,
+complete_async_pkts(struct vhost_dev *vdev)
+{
+	struct rte_mbuf *p_cpl[MAX_PKT_BURST];
+	uint16_t complete_count;
+
+	complete_count = rte_vhost_poll_enqueue_completed(vdev->vid,
+					VIRTIO_RXQ, p_cpl, MAX_PKT_BURST);
+	if (complete_count) {
+		atomic_fetch_sub(&vdev->nr_async_pkts, complete_count);
+		free_pkts(p_cpl, complete_count);
+	}
+}
+
+static __rte_always_inline void
+sync_virtio_xmit(struct vhost_dev *dst_vdev, struct vhost_dev *src_vdev,
 	    struct rte_mbuf *m)
 {
 	uint16_t ret;
-	struct rte_mbuf *m_cpl[1];
 
 	if (builtin_net_driver) {
 		ret = vs_enqueue_pkts(dst_vdev, VIRTIO_RXQ, &m, 1);
-	} else if (async_vhost_driver) {
-		ret = rte_vhost_submit_enqueue_burst(dst_vdev->vid, VIRTIO_RXQ,
-						&m, 1);
-
-		if (likely(ret))
-			dst_vdev->nr_async_pkts++;
-
-		while (likely(dst_vdev->nr_async_pkts)) {
-			if (rte_vhost_poll_enqueue_completed(dst_vdev->vid,
-					VIRTIO_RXQ, m_cpl, 1))
-				dst_vdev->nr_async_pkts--;
-		}
 	} else {
 		ret = rte_vhost_enqueue_burst(dst_vdev->vid, VIRTIO_RXQ, &m, 1);
 	}
 
 	if (enable_stats) {
-		rte_atomic64_inc(&dst_vdev->stats.rx_total_atomic);
-		rte_atomic64_add(&dst_vdev->stats.rx_atomic, ret);
+		atomic_fetch_add(&dst_vdev->stats.rx_total_atomic, 1);
+		atomic_fetch_add(&dst_vdev->stats.rx_atomic, ret);
 		src_vdev->stats.tx_total++;
 		src_vdev->stats.tx += ret;
 	}
 }
 
+static __rte_always_inline void
+drain_vhost(struct vhost_dev *vdev)
+{
+	uint16_t ret;
+	uint64_t queue_id = rte_lcore_id() * MAX_VHOST_DEVICE + vdev->vid;
+	uint16_t nr_xmit = vhost_txbuff[queue_id]->len;
+	struct rte_mbuf **m = vhost_txbuff[queue_id]->m_table;
+
+	if (builtin_net_driver) {
+		ret = vs_enqueue_pkts(vdev, VIRTIO_RXQ, m, nr_xmit);
+	} else if (async_vhost_driver) {
+		uint32_t cpu_cpl_nr = 0;
+		uint16_t enqueue_fail = 0;
+		struct rte_mbuf *m_cpu_cpl[nr_xmit];
+
+		complete_async_pkts(vdev);
+		ret = rte_vhost_submit_enqueue_burst(vdev->vid, VIRTIO_RXQ,
+					m, nr_xmit, m_cpu_cpl, &cpu_cpl_nr);
+		atomic_fetch_add(&vdev->nr_async_pkts, ret - cpu_cpl_nr);
+
+		if (cpu_cpl_nr)
+			free_pkts(m_cpu_cpl, cpu_cpl_nr);
+
+		enqueue_fail = nr_xmit - ret;
+		if (enqueue_fail)
+			free_pkts(&m[ret], nr_xmit - ret);
+	} else {
+		ret = rte_vhost_enqueue_burst(vdev->vid, VIRTIO_RXQ,
+						m, nr_xmit);
+	}
+
+	if (enable_stats) {
+		atomic_fetch_add(&vdev->stats.rx_total_atomic, nr_xmit);
+		atomic_fetch_add(&vdev->stats.rx_atomic, ret);
+	}
+
+	if (!async_vhost_driver)
+		free_pkts(m, nr_xmit);
+}
+
+static __rte_always_inline void
+drain_vhost_table(void)
+{
+	const uint16_t lcore_id = rte_lcore_id();
+	struct vhost_bufftable *vhost_txq;
+	struct vhost_dev *vdev;
+	uint64_t cur_tsc;
+
+	TAILQ_FOREACH(vdev, &lcore_info[lcore_id].vdev_list, lcore_vdev_entry) {
+		if (!vdev->remove) {
+			vhost_txq = vhost_txbuff[lcore_id * MAX_VHOST_DEVICE
+					+ vdev->vid];
+			cur_tsc = rte_rdtsc();
+
+			if (unlikely(cur_tsc - vhost_txq->pre_tsc
+					> MBUF_TABLE_DRAIN_TSC)) {
+				RTE_LOG_DP(DEBUG, VHOST_DATA,
+					"Vhost tX queue drained after timeout with burst size %u\n",
+					vhost_txq->len);
+				drain_vhost(vdev);
+				vhost_txq->len = 0;
+				vhost_txq->pre_tsc = cur_tsc;
+			}
+		}
+	}
+}
+
 /*
  * Check if the packet destination MAC address is for a local device. If so then put
  * the packet on that devices RX queue. If not then return.
@@ -846,7 +930,8 @@ virtio_tx_local(struct vhost_dev *vdev, struct rte_mbuf *m)
 {
 	struct rte_ether_hdr *pkt_hdr;
 	struct vhost_dev *dst_vdev;
-
+	struct vhost_bufftable *vhost_txq;
+	const uint16_t lcore_id = rte_lcore_id();
 	pkt_hdr = rte_pktmbuf_mtod(m, struct rte_ether_hdr *);
 
 	dst_vdev = find_vhost_dev(&pkt_hdr->d_addr);
@@ -869,7 +954,19 @@ virtio_tx_local(struct vhost_dev *vdev, struct rte_mbuf *m)
 		return 0;
 	}
 
-	virtio_xmit(dst_vdev, vdev, m);
+	vhost_txq = vhost_txbuff[lcore_id * MAX_VHOST_DEVICE + dst_vdev->vid];
+	vhost_txq->m_table[vhost_txq->len++] = m;
+
+	if (enable_stats) {
+		vdev->stats.tx_total++;
+		vdev->stats.tx++;
+	}
+
+	if (unlikely(vhost_txq->len == MAX_PKT_BURST)) {
+		drain_vhost(dst_vdev);
+		vhost_txq->len = 0;
+		vhost_txq->pre_tsc = rte_rdtsc();
+	}
 	return 0;
 }
 
@@ -940,13 +1037,6 @@ static void virtio_tx_offload(struct rte_mbuf *m)
 	tcp_hdr->cksum = get_psd_sum(l3_hdr, m->ol_flags);
 }
 
-static inline void
-free_pkts(struct rte_mbuf **pkts, uint16_t n)
-{
-	while (n--)
-		rte_pktmbuf_free(pkts[n]);
-}
-
 static __rte_always_inline void
 do_drain_mbuf_table(struct mbuf_table *tx_q)
 {
@@ -979,16 +1069,14 @@ virtio_tx_route(struct vhost_dev *vdev, struct rte_mbuf *m, uint16_t vlan_tag)
 
 		TAILQ_FOREACH(vdev2, &vhost_dev_list, global_vdev_entry) {
 			if (vdev2 != vdev)
-				virtio_xmit(vdev2, vdev, m);
+				sync_virtio_xmit(vdev2, vdev, m);
 		}
 		goto queue2nic;
 	}
 
 	/*check if destination is local VM*/
-	if ((vm2vm_mode == VM2VM_SOFTWARE) && (virtio_tx_local(vdev, m) == 0)) {
-		rte_pktmbuf_free(m);
+	if ((vm2vm_mode == VM2VM_SOFTWARE) && (virtio_tx_local(vdev, m) == 0))
 		return;
-	}
 
 	if (unlikely(vm2vm_mode == VM2VM_HARDWARE)) {
 		if (unlikely(find_local_dest(vdev, m, &offset,
@@ -1073,19 +1161,6 @@ drain_mbuf_table(struct mbuf_table *tx_q)
 	}
 }
 
-static __rte_always_inline void
-complete_async_pkts(struct vhost_dev *vdev, uint16_t qid)
-{
-	struct rte_mbuf *p_cpl[MAX_PKT_BURST];
-	uint16_t complete_count;
-
-	complete_count = rte_vhost_poll_enqueue_completed(vdev->vid,
-						qid, p_cpl, MAX_PKT_BURST);
-	vdev->nr_async_pkts -= complete_count;
-	if (complete_count)
-		free_pkts(p_cpl, complete_count);
-}
-
 static __rte_always_inline void
 drain_eth_rx(struct vhost_dev *vdev)
 {
@@ -1095,9 +1170,6 @@ drain_eth_rx(struct vhost_dev *vdev)
 	rx_count = rte_eth_rx_burst(ports[0], vdev->vmdq_rx_q,
 				    pkts, MAX_PKT_BURST);
 
-	while (likely(vdev->nr_async_pkts))
-		complete_async_pkts(vdev, VIRTIO_RXQ);
-
 	if (!rx_count)
 		return;
 
@@ -1123,17 +1195,31 @@ drain_eth_rx(struct vhost_dev *vdev)
 		enqueue_count = vs_enqueue_pkts(vdev, VIRTIO_RXQ,
 						pkts, rx_count);
 	} else if (async_vhost_driver) {
+		uint32_t cpu_cpl_nr = 0;
+		uint16_t enqueue_fail = 0;
+		struct rte_mbuf *m_cpu_cpl[MAX_PKT_BURST];
+
+		complete_async_pkts(vdev);
 		enqueue_count = rte_vhost_submit_enqueue_burst(vdev->vid,
-					VIRTIO_RXQ, pkts, rx_count);
-		vdev->nr_async_pkts += enqueue_count;
+					VIRTIO_RXQ, pkts, rx_count,
+					m_cpu_cpl, &cpu_cpl_nr);
+		atomic_fetch_add(&vdev->nr_async_pkts,
+					enqueue_count - cpu_cpl_nr);
+		if (cpu_cpl_nr)
+			free_pkts(m_cpu_cpl, cpu_cpl_nr);
+
+		enqueue_fail = rx_count - enqueue_count;
+		if (enqueue_fail)
+			free_pkts(&pkts[enqueue_count], enqueue_fail);
+
 	} else {
 		enqueue_count = rte_vhost_enqueue_burst(vdev->vid, VIRTIO_RXQ,
 						pkts, rx_count);
 	}
 
 	if (enable_stats) {
-		rte_atomic64_add(&vdev->stats.rx_total_atomic, rx_count);
-		rte_atomic64_add(&vdev->stats.rx_atomic, enqueue_count);
+		atomic_fetch_add(&vdev->stats.rx_total_atomic, rx_count);
+		atomic_fetch_add(&vdev->stats.rx_atomic, enqueue_count);
 	}
 
 	if (!async_vhost_driver)
@@ -1202,7 +1288,7 @@ switch_worker(void *arg __rte_unused)
 
 	while(1) {
 		drain_mbuf_table(tx_q);
-
+		drain_vhost_table();
 		/*
 		 * Inform the configuration core that we have exited the
 		 * linked list and that no devices are in use if requested.
@@ -1298,6 +1384,7 @@ static int
 new_device(int vid)
 {
 	int lcore, core_add = 0;
+	uint16_t i;
 	uint32_t device_num_min = num_devices;
 	struct vhost_dev *vdev;
 	vdev = rte_zmalloc("vhost device", sizeof(*vdev), RTE_CACHE_LINE_SIZE);
@@ -1309,6 +1396,13 @@ new_device(int vid)
 	}
 	vdev->vid = vid;
 
+	for (i = 0; i < RTE_MAX_LCORE; i++) {
+		vhost_txbuff[i * MAX_VHOST_DEVICE + vid]
+			= rte_zmalloc("vhost bufftable",
+				sizeof(struct vhost_bufftable),
+				RTE_CACHE_LINE_SIZE);
+	}
+
 	if (builtin_net_driver)
 		vs_vhost_net_setup(vdev);
 
@@ -1343,12 +1437,15 @@ new_device(int vid)
 	if (async_vhost_driver) {
 		struct rte_vhost_async_features f;
 		struct rte_vhost_async_channel_ops channel_ops;
+
 		if (strncmp(dma_type, "ioat", 4) == 0) {
 			channel_ops.transfer_data = ioat_transfer_data_cb;
 			channel_ops.check_completed_copies =
 				ioat_check_completed_copies_cb;
+
 			f.async_inorder = 1;
 			f.async_threshold = 256;
+
 			return rte_vhost_async_channel_register(vid, VIRTIO_RXQ,
 				f.intval, &channel_ops);
 		}
@@ -1392,8 +1489,8 @@ print_stats(__rte_unused void *arg)
 			tx         = vdev->stats.tx;
 			tx_dropped = tx_total - tx;
 
-			rx_total   = rte_atomic64_read(&vdev->stats.rx_total_atomic);
-			rx         = rte_atomic64_read(&vdev->stats.rx_atomic);
+			rx_total   = atomic_load(&vdev->stats.rx_total_atomic);
+			rx         = atomic_load(&vdev->stats.rx_atomic);
 			rx_dropped = rx_total - rx;
 
 			printf("Statistics for device %d\n"
@@ -1592,6 +1689,7 @@ main(int argc, char *argv[])
 	/* Register vhost user driver to handle vhost messages. */
 	for (i = 0; i < nb_sockets; i++) {
 		char *file = socket_files + i * PATH_MAX;
+
 		if (async_vhost_driver)
 			flags = flags | RTE_VHOST_USER_ASYNC_COPY;
 
examples/vhost/main.h +4 -3
index 4317b6ae8..6aa798a3e 100644
@@ -8,6 +8,7 @@
 #include <sys/queue.h>
 
 #include <rte_ether.h>
+#include <stdatomic.h>
 
 /* Macros for printing using RTE_LOG */
 #define RTE_LOGTYPE_VHOST_CONFIG RTE_LOGTYPE_USER1
@@ -21,8 +22,8 @@ enum {VIRTIO_RXQ, VIRTIO_TXQ, VIRTIO_QNUM};
 struct device_statistics {
 	uint64_t	tx;
 	uint64_t	tx_total;
-	rte_atomic64_t	rx_atomic;
-	rte_atomic64_t	rx_total_atomic;
+	atomic_int_least64_t	rx_atomic;
+	atomic_int_least64_t	rx_total_atomic;
 };
 
 struct vhost_queue {
@@ -51,7 +52,7 @@ struct vhost_dev {
 	uint64_t features;
 	size_t hdr_len;
 	uint16_t nr_vrings;
-	uint16_t nr_async_pkts;
+	atomic_int_least16_t nr_async_pkts;
 	struct rte_vhost_memory *mem;
 	struct device_statistics stats;
 	TAILQ_ENTRY(vhost_dev) global_vdev_entry;
Signature · 1 line
2.29.2
#15 lore raw

Re: [dpdk-dev] [PATCH v4 2/2] examples/vhost: refactor vhost data path

Hi Cheng,

Some comments are inline.

Thanks,
Jiayu

Quoted 11 lines

-----Original Message-----
From: Jiang, Cheng1 <[email protected]>
Sent: Friday, December 25, 2020 4:07 PM
To: [email protected]; Xia, Chenbo <[email protected]>
Cc: [email protected]; Hu, Jiayu <[email protected]>; Yang, YvonneX <[email protected]>; Jiang, Cheng1 <[email protected]>
Subject: [PATCH v4 2/2] examples/vhost: refactor vhost data path

Change the vm2vm data path to batch enqueue for better performance. Support latest async vhost API, refactor vhost async data path, replase rte_atomicNN_xxx to atomic_XXX and clean some codes.

Typo: replase -> replace

Quoted 15 lines
---
 examples/vhost/main.c | 202 +++++++++++++++++++++++++++++++-----------
 examples/vhost/main.h |   7 +-
 2 files changed, 154 insertions(+), 55 deletions(-)
examples/vhost/main.c +0 -0
index 8d8c3038b..3ea12a474 100644
@@ -179,9 +179,18 @@ struct mbuf_table {
 	struct rte_mbuf *m_table[MAX_PKT_BURST];
 };
 

examples/vhost/main.c struct mbuf_table { · line 191

Quoted 10 lines
examples/vhost/main.c +7 -0
+struct vhost_bufftable {
+	uint32_t len;
+	uint64_t pre_tsc;
+	struct rte_mbuf *m_table[MAX_PKT_BURST];
+};
+
 /* TX queue for each data core. */
 struct mbuf_table lcore_tx_queue[RTE_MAX_LCORE];
 
+/* TX queue for each vhost device. */

Every lcore maintains a TX buffer for every vhost device, which is to batch pkts to enqueue for higher performance. I suggest you to update the description of vhost_txbuff above, as it is not very clear.

Quoted 77 lines
examples/vhost/main.c +2 -0
+struct vhost_bufftable *vhost_txbuff[RTE_MAX_LCORE *
 MAX_VHOST_DEVICE];
+
 #define MBUF_TABLE_DRAIN_TSC	((rte_get_tsc_hz() + US_PER_S - 1) \
 				 / US_PER_S * BURST_TX_DRAIN_US)

#define VLAN_HLEN 4

examples/vhost/main.c +29 -16
@@ -804,39 +813,114 @@ unlink_vmdq(struct vhost_dev *vdev)
 	}
 }
 
+static inline void
+free_pkts(struct rte_mbuf **pkts, uint16_t n)
+{
+	while (n--)
+		rte_pktmbuf_free(pkts[n]);
+}
+
 static __rte_always_inline void
-virtio_xmit(struct vhost_dev *dst_vdev, struct vhost_dev *src_vdev,
+complete_async_pkts(struct vhost_dev *vdev)
+{
+	struct rte_mbuf *p_cpl[MAX_PKT_BURST];
+	uint16_t complete_count;
+
+	complete_count = rte_vhost_poll_enqueue_completed(vdev->vid,
+					VIRTIO_RXQ, p_cpl,
 MAX_PKT_BURST);
+	if (complete_count) {
+		atomic_fetch_sub(&vdev->nr_async_pkts, complete_count);
+		free_pkts(p_cpl, complete_count);
+	}
+}
+
+static __rte_always_inline void
+sync_virtio_xmit(struct vhost_dev *dst_vdev, struct vhost_dev *src_vdev,
 	    struct rte_mbuf *m)
 {
 	uint16_t ret;
-	struct rte_mbuf *m_cpl[1];
 
 	if (builtin_net_driver) {
 		ret = vs_enqueue_pkts(dst_vdev, VIRTIO_RXQ, &m, 1);
-	} else if (async_vhost_driver) {
-		ret = rte_vhost_submit_enqueue_burst(dst_vdev->vid,
 VIRTIO_RXQ,
-						&m, 1);
-
-		if (likely(ret))
-			dst_vdev->nr_async_pkts++;
-
-		while (likely(dst_vdev->nr_async_pkts)) {
-			if (rte_vhost_poll_enqueue_completed(dst_vdev-
 >vid,
-					VIRTIO_RXQ, m_cpl, 1))
-				dst_vdev->nr_async_pkts--;
-		}
 	} else {
 		ret = rte_vhost_enqueue_burst(dst_vdev->vid, VIRTIO_RXQ,
 &m, 1);
 	}
 
 	if (enable_stats) {
-		rte_atomic64_inc(&dst_vdev->stats.rx_total_atomic);
-		rte_atomic64_add(&dst_vdev->stats.rx_atomic, ret);
+		atomic_fetch_add(&dst_vdev->stats.rx_total_atomic, 1);
+		atomic_fetch_add(&dst_vdev->stats.rx_atomic, ret);
 		src_vdev->stats.tx_total++;
 		src_vdev->stats.tx += ret;
 	}
 }
 
+static __rte_always_inline void
+drain_vhost(struct vhost_dev *vdev)
+{
+	uint16_t ret;
+	uint64_t queue_id = rte_lcore_id() * MAX_VHOST_DEVICE + vdev-
Quoted 1 line

vid;

examples/vhost/main.c unlink_vmdq(struct vhost_dev *vdev) · line 863

Quoted 2 lines
examples/vhost/main.c +2 -0
+	uint16_t nr_xmit = vhost_txbuff[queue_id]->len;
+	struct rte_mbuf **m = vhost_txbuff[queue_id]->m_table;

"queue_id" is not a very good name, as it's not the queue id of vhost device, but a buffer index which holds pkts to enqueue.

Quoted 43 lines
examples/vhost/main.c +10 -0
+
+	if (builtin_net_driver) {
+		ret = vs_enqueue_pkts(vdev, VIRTIO_RXQ, m, nr_xmit);
+	} else if (async_vhost_driver) {
+		uint32_t cpu_cpl_nr = 0;
+		uint16_t enqueue_fail = 0;
+		struct rte_mbuf *m_cpu_cpl[nr_xmit];
+
+		complete_async_pkts(vdev);
+		ret = rte_vhost_submit_enqueue_burst(vdev->vid,

VIRTIO_RXQ,

examples/vhost/main.c +32 -0
+					m, nr_xmit, m_cpu_cpl, &cpu_cpl_nr);
+		atomic_fetch_add(&vdev->nr_async_pkts, ret - cpu_cpl_nr);
+
+		if (cpu_cpl_nr)
+			free_pkts(m_cpu_cpl, cpu_cpl_nr);
+
+		enqueue_fail = nr_xmit - ret;
+		if (enqueue_fail)
+			free_pkts(&m[ret], nr_xmit - ret);
+	} else {
+		ret = rte_vhost_enqueue_burst(vdev->vid, VIRTIO_RXQ,
+						m, nr_xmit);
+	}
+
+	if (enable_stats) {
+		atomic_fetch_add(&vdev->stats.rx_total_atomic, nr_xmit);
+		atomic_fetch_add(&vdev->stats.rx_atomic, ret);
+	}
+
+	if (!async_vhost_driver)
+		free_pkts(m, nr_xmit);
+}
+
+static __rte_always_inline void
+drain_vhost_table(void)
+{
+	const uint16_t lcore_id = rte_lcore_id();
+	struct vhost_bufftable *vhost_txq;
+	struct vhost_dev *vdev;
+	uint64_t cur_tsc;
+
+	TAILQ_FOREACH(vdev, &lcore_info[lcore_id].vdev_list,
Quoted 1 line

lcore_vdev_entry) {

A lcore may have pkts to enqueue for any vhost device, as it's decided by mac address. So drain_vhost_table() shouldn't just process vhost ports allocated to its lcore, but all vhost ports that have un-sent packets.

examples/vhost/main.c unlink_vmdq(struct vhost_dev *vdev)

Quoted 10 lines
examples/vhost/main.c +2 -0
+		if (!vdev->remove) {
+			vhost_txq = vhost_txbuff[lcore_id *

MAX_VHOST_DEVICE

examples/vhost/main.c +7 -0
+					+ vdev->vid];
+			cur_tsc = rte_rdtsc();
+
+			if (unlikely(cur_tsc - vhost_txq->pre_tsc
+					> MBUF_TABLE_DRAIN_TSC)) {
+				RTE_LOG_DP(DEBUG, VHOST_DATA,
+					"Vhost tX queue drained after

"tX" -> "TX"

Quoted 15 lines

timeout with burst size %u\n",

examples/vhost/main.c +9 -0
+					vhost_txq->len);
+				drain_vhost(vdev);
+				vhost_txq->len = 0;
+				vhost_txq->pre_tsc = cur_tsc;
+			}
+		}
+	}
+}
+
 /*
  * Check if the packet destination MAC address is for a local device. If so then

put
* the packet on that devices RX queue. If not then return.

examples/vhost/main.c virtio_tx_local(struct vhost_dev *vdev, struct · line 935

Quoted 7 lines
examples/vhost/main.c +2 -1
@@ -846,7 +930,8 @@ virtio_tx_local(struct vhost_dev *vdev, struct
 rte_mbuf *m)
 {
 	struct rte_ether_hdr *pkt_hdr;
 	struct vhost_dev *dst_vdev;
-
+	struct vhost_bufftable *vhost_txq;
+	const uint16_t lcore_id = rte_lcore_id();

Why use "const"?

Quoted 150 lines
examples/vhost/main.c +0 -0
 	pkt_hdr = rte_pktmbuf_mtod(m, struct rte_ether_hdr *);
 
examples/vhost/main.c +13 -1
 	dst_vdev = find_vhost_dev(&pkt_hdr->d_addr);
@@ -869,7 +954,19 @@ virtio_tx_local(struct vhost_dev *vdev, struct
 rte_mbuf *m)
 		return 0;
 	}
 
-	virtio_xmit(dst_vdev, vdev, m);
+	vhost_txq = vhost_txbuff[lcore_id * MAX_VHOST_DEVICE + dst_vdev-
 >vid];
+	vhost_txq->m_table[vhost_txq->len++] = m;
+
+	if (enable_stats) {
+		vdev->stats.tx_total++;
+		vdev->stats.tx++;
+	}
+
+	if (unlikely(vhost_txq->len == MAX_PKT_BURST)) {
+		drain_vhost(dst_vdev);
+		vhost_txq->len = 0;
+		vhost_txq->pre_tsc = rte_rdtsc();
+	}
 	return 0;

}

examples/vhost/main.c +20 -31
@@ -940,13 +1037,6 @@ static void virtio_tx_offload(struct rte_mbuf *m)
 	tcp_hdr->cksum = get_psd_sum(l3_hdr, m->ol_flags);
 }
 
-static inline void
-free_pkts(struct rte_mbuf **pkts, uint16_t n)
-{
-	while (n--)
-		rte_pktmbuf_free(pkts[n]);
-}
-
 static __rte_always_inline void
 do_drain_mbuf_table(struct mbuf_table *tx_q)
 {
@@ -979,16 +1069,14 @@ virtio_tx_route(struct vhost_dev *vdev, struct
 rte_mbuf *m, uint16_t vlan_tag)
 
 		TAILQ_FOREACH(vdev2, &vhost_dev_list, global_vdev_entry)
 {
 			if (vdev2 != vdev)
-				virtio_xmit(vdev2, vdev, m);
+				sync_virtio_xmit(vdev2, vdev, m);
 		}
 		goto queue2nic;
 	}
 
 	/*check if destination is local VM*/
-	if ((vm2vm_mode == VM2VM_SOFTWARE) && (virtio_tx_local(vdev,
 m) == 0)) {
-		rte_pktmbuf_free(m);
+	if ((vm2vm_mode == VM2VM_SOFTWARE) && (virtio_tx_local(vdev,
 m) == 0))
 		return;
-	}
 
 	if (unlikely(vm2vm_mode == VM2VM_HARDWARE)) {
 		if (unlikely(find_local_dest(vdev, m, &offset,
@@ -1073,19 +1161,6 @@ drain_mbuf_table(struct mbuf_table *tx_q)
 	}
 }
 
-static __rte_always_inline void
-complete_async_pkts(struct vhost_dev *vdev, uint16_t qid)
-{
-	struct rte_mbuf *p_cpl[MAX_PKT_BURST];
-	uint16_t complete_count;
-
-	complete_count = rte_vhost_poll_enqueue_completed(vdev->vid,
-						qid, p_cpl, MAX_PKT_BURST);
-	vdev->nr_async_pkts -= complete_count;
-	if (complete_count)
-		free_pkts(p_cpl, complete_count);
-}
-
 static __rte_always_inline void
 drain_eth_rx(struct vhost_dev *vdev)
 {
@@ -1095,9 +1170,6 @@ drain_eth_rx(struct vhost_dev *vdev)
 	rx_count = rte_eth_rx_burst(ports[0], vdev->vmdq_rx_q,
 				    pkts, MAX_PKT_BURST);
 
-	while (likely(vdev->nr_async_pkts))
-		complete_async_pkts(vdev, VIRTIO_RXQ);
-
 	if (!rx_count)
 		return;
 
@@ -1123,17 +1195,31 @@ drain_eth_rx(struct vhost_dev *vdev)
 		enqueue_count = vs_enqueue_pkts(vdev, VIRTIO_RXQ,
 						pkts, rx_count);
 	} else if (async_vhost_driver) {
+		uint32_t cpu_cpl_nr = 0;
+		uint16_t enqueue_fail = 0;
+		struct rte_mbuf *m_cpu_cpl[MAX_PKT_BURST];
+
+		complete_async_pkts(vdev);
 		enqueue_count = rte_vhost_submit_enqueue_burst(vdev-
 >vid,
-					VIRTIO_RXQ, pkts, rx_count);
-		vdev->nr_async_pkts += enqueue_count;
+					VIRTIO_RXQ, pkts, rx_count,
+					m_cpu_cpl, &cpu_cpl_nr);
+		atomic_fetch_add(&vdev->nr_async_pkts,
+					enqueue_count - cpu_cpl_nr);
+		if (cpu_cpl_nr)
+			free_pkts(m_cpu_cpl, cpu_cpl_nr);
+
+		enqueue_fail = rx_count - enqueue_count;
+		if (enqueue_fail)
+			free_pkts(&pkts[enqueue_count], enqueue_fail);
+
 	} else {
 		enqueue_count = rte_vhost_enqueue_burst(vdev->vid,
 VIRTIO_RXQ,
 						pkts, rx_count);
 	}
 
 	if (enable_stats) {
-		rte_atomic64_add(&vdev->stats.rx_total_atomic, rx_count);
-		rte_atomic64_add(&vdev->stats.rx_atomic, enqueue_count);
+		atomic_fetch_add(&vdev->stats.rx_total_atomic, rx_count);
+		atomic_fetch_add(&vdev->stats.rx_atomic, enqueue_count);
 	}
examples/vhost/main.c +2 -1
 	if (!async_vhost_driver)
@@ -1202,7 +1288,7 @@ switch_worker(void *arg __rte_unused)
 
 	while(1) {
 		drain_mbuf_table(tx_q);
-
+		drain_vhost_table();
 		/*
 		 * Inform the configuration core that we have exited the
 		 * linked list and that no devices are in use if requested.
@@ -1298,6 +1384,7 @@ static int
 new_device(int vid)
 {
 	int lcore, core_add = 0;
+	uint16_t i;
 	uint32_t device_num_min = num_devices;
 	struct vhost_dev *vdev;
 	vdev = rte_zmalloc("vhost device", sizeof(*vdev),

RTE_CACHE_LINE_SIZE);

examples/vhost/main.c new_device(int vid) · line 1403

Quoted 8 lines
examples/vhost/main.c +5 -0
@@ -1309,6 +1396,13 @@ new_device(int vid)
 	}
 	vdev->vid = vid;
 
+	for (i = 0; i < RTE_MAX_LCORE; i++) {
+		vhost_txbuff[i * MAX_VHOST_DEVICE + vid]
+			= rte_zmalloc("vhost bufftable",
+				sizeof(struct vhost_bufftable),
+				RTE_CACHE_LINE_SIZE);

Rte_zmalloc() may fail. Need to handle failure.

Quoted 77 lines
examples/vhost/main.c +5 -0
+	}
+
 	if (builtin_net_driver)
 		vs_vhost_net_setup(vdev);
 
@@ -1343,12 +1437,15 @@ new_device(int vid)
 	if (async_vhost_driver) {
 		struct rte_vhost_async_features f;
 		struct rte_vhost_async_channel_ops channel_ops;
+
 		if (strncmp(dma_type, "ioat", 4) == 0) {
 			channel_ops.transfer_data = ioat_transfer_data_cb;
 			channel_ops.check_completed_copies =
 				ioat_check_completed_copies_cb;
+
 			f.async_inorder = 1;
 			f.async_threshold = 256;
+
 			return rte_vhost_async_channel_register(vid,
 VIRTIO_RXQ,
 				f.intval, &channel_ops);
examples/vhost/main.c +2 -2
 		}
@@ -1392,8 +1489,8 @@ print_stats(__rte_unused void *arg)
 			tx         = vdev->stats.tx;
 			tx_dropped = tx_total - tx;
 
-			rx_total   = rte_atomic64_read(&vdev-
 >stats.rx_total_atomic);
-			rx         = rte_atomic64_read(&vdev->stats.rx_atomic);
+			rx_total   = atomic_load(&vdev-
 >stats.rx_total_atomic);
+			rx         = atomic_load(&vdev->stats.rx_atomic);
 			rx_dropped = rx_total - rx;
examples/vhost/main.c +1 -0
 			printf("Statistics for device %d\n"
@@ -1592,6 +1689,7 @@ main(int argc, char *argv[])
 	/* Register vhost user driver to handle vhost messages. */
 	for (i = 0; i < nb_sockets; i++) {
 		char *file = socket_files + i * PATH_MAX;
+
 		if (async_vhost_driver)
 			flags = flags | RTE_VHOST_USER_ASYNC_COPY;
 
examples/vhost/main.h +4 -4
index 4317b6ae8..6aa798a3e 100644
@@ -8,6 +8,7 @@
 #include <sys/queue.h>
 
 #include <rte_ether.h>
+#include <stdatomic.h>
 
 /* Macros for printing using RTE_LOG */
 #define RTE_LOGTYPE_VHOST_CONFIG RTE_LOGTYPE_USER1
@@ -21,8 +22,8 @@ enum {VIRTIO_RXQ, VIRTIO_TXQ, VIRTIO_QNUM};
 struct device_statistics {
 	uint64_t	tx;
 	uint64_t	tx_total;
-	rte_atomic64_t	rx_atomic;
-	rte_atomic64_t	rx_total_atomic;
+	atomic_int_least64_t	rx_atomic;
+	atomic_int_least64_t	rx_total_atomic;
 };
 
 struct vhost_queue {
@@ -51,7 +52,7 @@ struct vhost_dev {
 	uint64_t features;
 	size_t hdr_len;
 	uint16_t nr_vrings;
-	uint16_t nr_async_pkts;
+	atomic_int_least16_t nr_async_pkts;
 	struct rte_vhost_memory *mem;
 	struct device_statistics stats;
 	TAILQ_ENTRY(vhost_dev) global_vdev_entry;
--

2.29.2

#16 lore raw

Re: [dpdk-dev] [PATCH v4 2/2] examples/vhost: refactor vhost data path

Hi Jiayu,

Quoted 22 lines

-----Original Message-----
From: Hu, Jiayu <[email protected]>
Sent: Monday, December 28, 2020 12:04 PM
To: Jiang, Cheng1 <[email protected]>; [email protected]; Xia, Chenbo <[email protected]>
Cc: [email protected]; Yang, YvonneX <[email protected]>
Subject: RE: [PATCH v4 2/2] examples/vhost: refactor vhost data path

Hi Cheng,

Some comments are inline.

Thanks,
Jiayu

Quoted 8 lines

-----Original Message-----
From: Jiang, Cheng1 <[email protected]>
Sent: Friday, December 25, 2020 4:07 PM
To: [email protected]; Xia, Chenbo <[email protected]>
Cc: [email protected]; Hu, Jiayu <[email protected]>; Yang, YvonneX <[email protected]>; Jiang, Cheng1 <[email protected]>
Subject: [PATCH v4 2/2] examples/vhost: refactor vhost data path

Quoted 5 lines
Quoted 3 lines

Change the vm2vm data path to batch enqueue for better performance. Support latest async vhost API, refactor vhost async data path, replase rte_atomicNN_xxx to atomic_XXX and clean some codes.

Typo: replase -> replace

I'll fix it in the next version.

Quoted 24 lines
Quoted 5 lines
---
 examples/vhost/main.c | 202 +++++++++++++++++++++++++++++++-----

------

Quoted 18 lines
 examples/vhost/main.h |   7 +-
 2 files changed, 154 insertions(+), 55 deletions(-)
examples/vhost/main.c index renamed from examples/vhost/main.c +0 -0

8d8c3038b..3ea12a474 100644

examples/vhost/main.c +5 -0
@@ -179,9 +179,18 @@ struct mbuf_table {  struct rte_mbuf
 *m_table[MAX_PKT_BURST];  };
 
+struct vhost_bufftable {
+uint32_t len;
+uint64_t pre_tsc;
+struct rte_mbuf *m_table[MAX_PKT_BURST]; };
+
 /* TX queue for each data core. */
 struct mbuf_table lcore_tx_queue[RTE_MAX_LCORE];
 
Quoted 6 lines

examples/vhost/main.c struct mbuf_table { · line 191

Quoted 1 line
examples/vhost/main.c +1 -0
+/* TX queue for each vhost device. */

Every lcore maintains a TX buffer for every vhost device, which is to batch pkts to enqueue for higher performance.
I suggest you to update the description of vhost_txbuff above, as it is not very clear.

Sure, will add some comments in the next version.

Quoted 67 lines
Quoted 67 lines
examples/vhost/main.c +18 -2
+struct vhost_bufftable *vhost_txbuff[RTE_MAX_LCORE *
 MAX_VHOST_DEVICE];
+
 #define MBUF_TABLE_DRAIN_TSC((rte_get_tsc_hz() + US_PER_S - 1) \
  / US_PER_S * BURST_TX_DRAIN_US)
 #define VLAN_HLEN       4
@@ -804,39 +813,114 @@ unlink_vmdq(struct vhost_dev *vdev)  }  }
 
+static inline void
+free_pkts(struct rte_mbuf **pkts, uint16_t n) { while (n--)
+rte_pktmbuf_free(pkts[n]); }
+
 static __rte_always_inline void
-virtio_xmit(struct vhost_dev *dst_vdev, struct vhost_dev *src_vdev,
+complete_async_pkts(struct vhost_dev *vdev) { struct rte_mbuf
+*p_cpl[MAX_PKT_BURST]; uint16_t complete_count;
+
+complete_count = rte_vhost_poll_enqueue_completed(vdev->vid,
+VIRTIO_RXQ, p_cpl,
 MAX_PKT_BURST);
+if (complete_count) {
+atomic_fetch_sub(&vdev->nr_async_pkts, complete_count);
+free_pkts(p_cpl, complete_count); } }
+
+static __rte_always_inline void
+sync_virtio_xmit(struct vhost_dev *dst_vdev, struct vhost_dev
+*src_vdev,
     struct rte_mbuf *m)
 {
 uint16_t ret;
-struct rte_mbuf *m_cpl[1];
 
 if (builtin_net_driver) {
 ret = vs_enqueue_pkts(dst_vdev, VIRTIO_RXQ, &m, 1); -} else if

(async_vhost_driver) { -ret =
rte_vhost_submit_enqueue_burst(dst_vdev->vid,
VIRTIO_RXQ,
-&m, 1);

examples/vhost/main.c +0 -5
-
-if (likely(ret))
-dst_vdev->nr_async_pkts++;
-
-while (likely(dst_vdev->nr_async_pkts)) { -if

(rte_vhost_poll_enqueue_completed(dst_vdev-

Quoted 1 line

vid,

examples/vhost/main.c +7 -5
-VIRTIO_RXQ, m_cpl, 1))
-dst_vdev->nr_async_pkts--;
-}
 } else {
 ret = rte_vhost_enqueue_burst(dst_vdev->vid, VIRTIO_RXQ, &m, 1);  }
 
 if (enable_stats) {
-rte_atomic64_inc(&dst_vdev->stats.rx_total_atomic);
-rte_atomic64_add(&dst_vdev->stats.rx_atomic, ret);
+atomic_fetch_add(&dst_vdev->stats.rx_total_atomic, 1);
+atomic_fetch_add(&dst_vdev->stats.rx_atomic, ret);
 src_vdev->stats.tx_total++;
 src_vdev->stats.tx += ret;
 }
 }
 
+static __rte_always_inline void
+drain_vhost(struct vhost_dev *vdev)
+{
+uint16_t ret;
+uint64_t queue_id = rte_lcore_id() * MAX_VHOST_DEVICE + vdev-
Quoted 6 lines

examples/vhost/main.c

Quoted 3 lines
Quoted 1 line

vid;

examples/vhost/main.c +2 -0
+uint16_t nr_xmit = vhost_txbuff[queue_id]->len; struct rte_mbuf **m =
+vhost_txbuff[queue_id]->m_table;

"queue_id" is not a very good name, as it's not the queue id of vhost device, but a buffer index which holds pkts to enqueue.

Sure, I will change it to buff_idx.

Quoted 34 lines
Quoted 34 lines
examples/vhost/main.c +8 -0
+
+if (builtin_net_driver) {
+ret = vs_enqueue_pkts(vdev, VIRTIO_RXQ, m, nr_xmit); } else if
+(async_vhost_driver) { uint32_t cpu_cpl_nr = 0; uint16_t enqueue_fail
+= 0; struct rte_mbuf *m_cpu_cpl[nr_xmit];
+
+complete_async_pkts(vdev);
+ret = rte_vhost_submit_enqueue_burst(vdev->vid,

VIRTIO_RXQ,

examples/vhost/main.c +24 -0
+m, nr_xmit, m_cpu_cpl, &cpu_cpl_nr);
+atomic_fetch_add(&vdev->nr_async_pkts, ret - cpu_cpl_nr);
+
+if (cpu_cpl_nr)
+free_pkts(m_cpu_cpl, cpu_cpl_nr);
+
+enqueue_fail = nr_xmit - ret;
+if (enqueue_fail)
+free_pkts(&m[ret], nr_xmit - ret);
+} else {
+ret = rte_vhost_enqueue_burst(vdev->vid, VIRTIO_RXQ, m, nr_xmit); }
+
+if (enable_stats) {
+atomic_fetch_add(&vdev->stats.rx_total_atomic, nr_xmit);
+atomic_fetch_add(&vdev->stats.rx_atomic, ret); }
+
+if (!async_vhost_driver)
+free_pkts(m, nr_xmit);
+}
+
+static __rte_always_inline void
+drain_vhost_table(void)
+{
+const uint16_t lcore_id = rte_lcore_id(); struct vhost_bufftable
Quoted 8 lines
Quoted 4 lines
examples/vhost/main.c +3 -0
+*vhost_txq; struct vhost_dev *vdev; uint64_t cur_tsc;
+
+TAILQ_FOREACH(vdev, &lcore_info[lcore_id].vdev_list,

lcore_vdev_entry) {

A lcore may have pkts to enqueue for any vhost device, as it's decided by mac address. So drain_vhost_table() shouldn't just process vhost ports allocated to its lcore, but all vhost ports that have un-sent packets.

Sure, I will fix it in the next version.

Quoted 13 lines

examples/vhost/main.c unlink_vmdq(struct vhost_dev *vdev)

Quoted 11 lines
examples/vhost/main.c +2 -0
+if (!vdev->remove) {
+vhost_txq = vhost_txbuff[lcore_id *

MAX_VHOST_DEVICE
++ vdev->vid];

examples/vhost/main.c +6 -0
+cur_tsc = rte_rdtsc();
+
+if (unlikely(cur_tsc - vhost_txq->pre_tsc
+> MBUF_TABLE_DRAIN_TSC)) {
+RTE_LOG_DP(DEBUG, VHOST_DATA,
+"Vhost tX queue drained after

"tX" -> "TX"

I will fix it in the next version.

Quoted 15 lines
Quoted 15 lines

timeout with burst size %u\n",

examples/vhost/main.c +9 -0
+vhost_txq->len);
+drain_vhost(vdev);
+vhost_txq->len = 0;
+vhost_txq->pre_tsc = cur_tsc;
+}
+}
+}
+}
+
 /*
  * Check if the packet destination MAC address is for a local device.

If so then put
* the packet on that devices RX queue. If not then return.

Quoted 8 lines

examples/vhost/main.c virtio_tx_local(struct vhost_dev *vdev, struct rte_mbuf *m) · line 934

Quoted 6 lines
examples/vhost/main.c +0 -0
@@ -846,7 +930,8 @@ virtio_tx_local(struct vhost_dev *vdev, struct

rte_mbuf *m) { struct rte_ether_hdr *pkt_hdr; struct vhost_dev *dst_vdev;

examples/vhost/main.c +2 -1
-
+struct vhost_bufftable *vhost_txq;
+const uint16_t lcore_id = rte_lcore_id();

Why use "const"?

No particular reason, maybe I just forget to delete it. I will fix it in the next version.

Quoted 134 lines

examples/vhost/main.c drain_eth_rx(struct vhost_dev *vdev) rx_count

Quoted 70 lines

pkt_hdr = rte_pktmbuf_mtod(m, struct rte_ether_hdr *);

dst_vdev = find_vhost_dev(&pkt_hdr->d_addr); @@ -869,7 +954,19 @@
virtio_tx_local(struct vhost_dev *vdev, struct rte_mbuf *m) return 0; }

examples/vhost/main.c +1 -1
-virtio_xmit(dst_vdev, vdev, m);
+vhost_txq = vhost_txbuff[lcore_id * MAX_VHOST_DEVICE + dst_vdev-
Quoted 1 line

vid];

examples/vhost/main.c +10 -4
+vhost_txq->m_table[vhost_txq->len++] = m;
+
+if (enable_stats) {
+vdev->stats.tx_total++;
+vdev->stats.tx++;
+}
+
+if (unlikely(vhost_txq->len == MAX_PKT_BURST)) {
+drain_vhost(dst_vdev); vhost_txq->len = 0; vhost_txq->pre_tsc =
+rte_rdtsc(); }
 return 0;
 }
@@ -940,13 +1037,6 @@ static void virtio_tx_offload(struct rte_mbuf
 *m)  tcp_hdr->cksum = get_psd_sum(l3_hdr, m->ol_flags);  }
 
-static inline void
-free_pkts(struct rte_mbuf **pkts, uint16_t n) -{ -while (n--)
-rte_pktmbuf_free(pkts[n]); -}
-
 static __rte_always_inline void
 do_drain_mbuf_table(struct mbuf_table *tx_q)  { @@ -979,16 +1069,14
 @@ virtio_tx_route(struct vhost_dev *vdev, struct rte_mbuf *m,
 uint16_t vlan_tag)
 
 TAILQ_FOREACH(vdev2, &vhost_dev_list, global_vdev_entry) {  if (vdev2

!= vdev) -virtio_xmit(vdev2, vdev, m);

examples/vhost/main.c +1 -1
+sync_virtio_xmit(vdev2, vdev, m);
 }
 goto queue2nic;
 }
 
 /*check if destination is local VM*/
-if ((vm2vm_mode == VM2VM_SOFTWARE) && (virtio_tx_local(vdev,

m) == 0)) {

examples/vhost/main.c +1 -1
-rte_pktmbuf_free(m);
+if ((vm2vm_mode == VM2VM_SOFTWARE) && (virtio_tx_local(vdev,

m) == 0))
return;
-}

if (unlikely(vm2vm_mode == VM2VM_HARDWARE)) { if
(unlikely(find_local_dest(vdev, m, &offset, @@ -1073,19 +1161,6 @@ drain_mbuf_table(struct mbuf_table *tx_q) } }

examples/vhost/main.c +0 -2
-static __rte_always_inline void
-complete_async_pkts(struct vhost_dev *vdev, uint16_t qid) -{ -struct

rte_mbuf *p_cpl[MAX_PKT_BURST]; -uint16_t complete_count;

examples/vhost/main.c +0 -8
-
-complete_count = rte_vhost_poll_enqueue_completed(vdev->vid,
-qid, p_cpl, MAX_PKT_BURST);
-vdev->nr_async_pkts -= complete_count;
-if (complete_count)
-free_pkts(p_cpl, complete_count);
-}
-
 static __rte_always_inline void
 drain_eth_rx(struct vhost_dev *vdev)
 {
@@ -1095,9 +1170,6 @@ drain_eth_rx(struct vhost_dev *vdev)  rx_count

=

Quoted 63 lines
examples/vhost/main.c +19 -8
 rte_eth_rx_burst(ports[0], vdev->vmdq_rx_q,
     pkts, MAX_PKT_BURST);
 
-while (likely(vdev->nr_async_pkts))
-complete_async_pkts(vdev, VIRTIO_RXQ);
-
 if (!rx_count)
 return;
 
@@ -1123,17 +1195,31 @@ drain_eth_rx(struct vhost_dev *vdev)
 enqueue_count = vs_enqueue_pkts(vdev, VIRTIO_RXQ,  pkts, rx_count);  }
 else if (async_vhost_driver) {
+uint32_t cpu_cpl_nr = 0;
+uint16_t enqueue_fail = 0;
+struct rte_mbuf *m_cpu_cpl[MAX_PKT_BURST];
+
+complete_async_pkts(vdev);
 enqueue_count = rte_vhost_submit_enqueue_burst(vdev-
 >vid,
-VIRTIO_RXQ, pkts, rx_count);
-vdev->nr_async_pkts += enqueue_count;
+VIRTIO_RXQ, pkts, rx_count,
+m_cpu_cpl, &cpu_cpl_nr);
+atomic_fetch_add(&vdev->nr_async_pkts,
+enqueue_count - cpu_cpl_nr);
+if (cpu_cpl_nr)
+free_pkts(m_cpu_cpl, cpu_cpl_nr);
+
+enqueue_fail = rx_count - enqueue_count; if (enqueue_fail)
+free_pkts(&pkts[enqueue_count], enqueue_fail);
+
 } else {
 enqueue_count = rte_vhost_enqueue_burst(vdev->vid,
 VIRTIO_RXQ,
 pkts, rx_count);
 }
 
 if (enable_stats) {
-rte_atomic64_add(&vdev->stats.rx_total_atomic, rx_count);
-rte_atomic64_add(&vdev->stats.rx_atomic, enqueue_count);
+atomic_fetch_add(&vdev->stats.rx_total_atomic, rx_count);
+atomic_fetch_add(&vdev->stats.rx_atomic, enqueue_count);
 }
 
 if (!async_vhost_driver)
@@ -1202,7 +1288,7 @@ switch_worker(void *arg __rte_unused)
 
 while(1) {
 drain_mbuf_table(tx_q);
-
+drain_vhost_table();
 /*
  * Inform the configuration core that we have exited the
  * linked list and that no devices are in use if requested.
@@ -1298,6 +1384,7 @@ static int
 new_device(int vid)
 {
 int lcore, core_add = 0;
+uint16_t i;
 uint32_t device_num_min = num_devices;  struct vhost_dev *vdev;  vdev

= rte_zmalloc("vhost device", sizeof(*vdev), RTE_CACHE_LINE_SIZE); @@ -1309,6 +1396,13 @@ new_device(int vid) } vdev->vid = vid;

Quoted 5 lines

examples/vhost/main.c

Quoted 3 lines
examples/vhost/main.c +3 -0
+for (i = 0; i < RTE_MAX_LCORE; i++) { vhost_txbuff[i *
+MAX_VHOST_DEVICE + vid] = rte_zmalloc("vhost bufftable",
+sizeof(struct vhost_bufftable), RTE_CACHE_LINE_SIZE);

Rte_zmalloc() may fail. Need to handle failure.

Sure, I will add failure handler in the next version.

Thanks.
Cheng

Quoted 74 lines
Quoted 52 lines
examples/vhost/main.c +2 -0
+}
+
 if (builtin_net_driver)
 vs_vhost_net_setup(vdev);
@@ -1343,12 +1437,15 @@ new_device(int vid)  if (async_vhost_driver) {

struct rte_vhost_async_features f; struct rte_vhost_async_channel_ops channel_ops;
+
if (strncmp(dma_type, "ioat", 4) == 0) { channel_ops.transfer_data =
ioat_transfer_data_cb; channel_ops.check_completed_copies = ioat_check_completed_copies_cb;

examples/vhost/main.c +2 -0
+
 f.async_inorder = 1;
 f.async_threshold = 256;
+
 return rte_vhost_async_channel_register(vid,

VIRTIO_RXQ,
f.intval, &channel_ops);
}

examples/vhost/main.c +2 -2
@@ -1392,8 +1489,8 @@ print_stats(__rte_unused void *arg)
 tx         = vdev->stats.tx;
 tx_dropped = tx_total - tx;
 
-rx_total   = rte_atomic64_read(&vdev-
 >stats.rx_total_atomic);
-rx         = rte_atomic64_read(&vdev->stats.rx_atomic);
+rx_total   = atomic_load(&vdev-
 >stats.rx_total_atomic);
+rx         = atomic_load(&vdev->stats.rx_atomic);
 rx_dropped = rx_total - rx;

printf("Statistics for device %d\n"

examples/vhost/main.c +1 -0
@@ -1592,6 +1689,7 @@ main(int argc, char *argv[])
 /* Register vhost user driver to handle vhost messages. */  for (i =
 0; i < nb_sockets; i++) {  char *file = socket_files + i * PATH_MAX;
+
 if (async_vhost_driver)
 flags = flags | RTE_VHOST_USER_ASYNC_COPY;
 
examples/vhost/main.h index renamed from examples/vhost/main.h +0 -0

4317b6ae8..6aa798a3e 100644

examples/vhost/main.h +1 -0
@@ -8,6 +8,7 @@
 #include <sys/queue.h>
 
 #include <rte_ether.h>
+#include <stdatomic.h>
 
 /* Macros for printing using RTE_LOG */  #define
Quoted 1 line

RTE_LOGTYPE_VHOST_CONFIG RTE_LOGTYPE_USER1 @@ -21,8 +22,8 @@

enum

Quoted 19 lines

{VIRTIO_RXQ, VIRTIO_TXQ, VIRTIO_QNUM}; struct device_statistics { uint64_ttx; uint64_ttx_total; -rte_atomic64_trx_atomic;

examples/vhost/main.h +3 -3
-rte_atomic64_trx_total_atomic;
+atomic_int_least64_trx_atomic;
+atomic_int_least64_trx_total_atomic;
 };
 
 struct vhost_queue {
@@ -51,7 +52,7 @@ struct vhost_dev {
 uint64_t features;
 size_t hdr_len;
 uint16_t nr_vrings;
-uint16_t nr_async_pkts;
+atomic_int_least16_t nr_async_pkts;
 struct rte_vhost_memory *mem;
 struct device_statistics stats;
 TAILQ_ENTRY(vhost_dev) global_vdev_entry;
--

2.29.2

#17 lore raw

v5 cover examples/vhost: sample code refactor

In reply to Cheng Jiang

Refactor the vhost sample code. Add ioat ring space count and check in ioat callback, optimize vhost data path for batch enqueue, replace rte_atomicNN_xxx to atomic_XXX and refactor vhost async data path.
---
v5:
* added vhost enqueue buffer free when destroy a vhost device
* added rte_ioat_completed_ops() fail handler
* changed the behavior of drain_vhost_table() function
* changed some variable names
* changed some variable definition
* added rte_zmalloc() fail handler
* added some comments
* fixed some typos

v4:
* improved code structure
* improved vhost enqueue buffer memory allocation
* cleaned some codes

v3:
* added some variable initiation
* cleaned some codes

v2:
* optimized patch structure
* optimized git log
* replaced rte_atomicNN_xxx to atomic_XXX

Cheng Jiang (2):
examples/vhost: add ioat ring space count and check examples/vhost: refactor vhost data path

 examples/vhost/ioat.c |  22 +++--
 examples/vhost/main.c | 214 ++++++++++++++++++++++++++++++++----------
 examples/vhost/main.h |   7 +-
 3 files changed, 178 insertions(+), 65 deletions(-)

--
2.29.2

#18 lore raw

PATCH v5 1/2 examples/vhost: add ioat ring space count and check

Add ioat ring space count and check, if ioat ring space is not enough for the next async vhost packet enqueue, then just return to prevent enqueue failure. Add rte_ioat_completed_ops() fail handler.

---
 examples/vhost/ioat.c | 22 ++++++++++++----------
 1 file changed, 12 insertions(+), 10 deletions(-)
examples/vhost/ioat.c +12 -10
index 71d8a1f1f..679d1e2f5 100644
@@ -17,6 +17,7 @@ struct packet_tracker {
 	unsigned short next_read;
 	unsigned short next_write;
 	unsigned short last_remain;
+	unsigned short ioat_space;
 };
 
 struct packet_tracker cb_tracker[MAX_VHOST_DEVICE];
@@ -113,7 +114,7 @@ open_ioat(const char *value)
 			goto out;
 		}
 		rte_rawdev_start(dev_id);
-
+		cb_tracker[dev_id].ioat_space = IOAT_RING_SIZE;
 		dma_info->nr++;
 		i++;
 	}
@@ -140,13 +141,9 @@ ioat_transfer_data_cb(int vid, uint16_t queue_id,
 			src = descs[i_desc].src;
 			dst = descs[i_desc].dst;
 			i_seg = 0;
+			if (cb_tracker[dev_id].ioat_space < src->nr_segs)
+				break;
 			while (i_seg < src->nr_segs) {
-				/*
-				 * TODO: Assuming that the ring space of the
-				 * IOAT device is large enough, so there is no
-				 * error here, and the actual error handling
-				 * will be added later.
-				 */
 				rte_ioat_enqueue_copy(dev_id,
 					(uintptr_t)(src->iov[i_seg].iov_base)
 						+ src->offset,
@@ -158,7 +155,8 @@ ioat_transfer_data_cb(int vid, uint16_t queue_id,
 				i_seg++;
 			}
 			write &= mask;
-			cb_tracker[dev_id].size_track[write] = i_seg;
+			cb_tracker[dev_id].size_track[write] = src->nr_segs;
+			cb_tracker[dev_id].ioat_space -= src->nr_segs;
 			write++;
 		}
 	} else {
@@ -178,17 +176,21 @@ ioat_check_completed_copies_cb(int vid, uint16_t queue_id,
 {
 	if (!opaque_data) {
 		uintptr_t dump[255];
-		unsigned short n_seg;
+		int n_seg;
 		unsigned short read, write;
 		unsigned short nb_packet = 0;
 		unsigned short mask = MAX_ENQUEUED_SIZE - 1;
 		unsigned short i;
+
 		int dev_id = dma_bind[vid].dmas[queue_id * 2
 				+ VIRTIO_RXQ].dev_id;
 		n_seg = rte_ioat_completed_ops(dev_id, 255, dump, dump);
-		n_seg += cb_tracker[dev_id].last_remain;
 		if (!n_seg)
 			return 0;
+
+		cb_tracker[dev_id].ioat_space += n_seg;
+		n_seg += cb_tracker[dev_id].last_remain;
+
 		read = cb_tracker[dev_id].next_read;
 		write = cb_tracker[dev_id].next_write;
 		for (i = 0; i < max_packets; i++) {
Signature · 1 line
2.29.2
#19 lore raw

PATCH v5 2/2 examples/vhost: refactor vhost data path

In reply to Cheng Jiang

Change the vm2vm data path to batch enqueue for better performance. Support latest async vhost API, refactor vhost async data path, replace rte_atomicNN_xxx to atomic_XXX and clean some codes.

---
 examples/vhost/main.c | 214 ++++++++++++++++++++++++++++++++----------
 examples/vhost/main.h |   7 +-
 2 files changed, 166 insertions(+), 55 deletions(-)
examples/vhost/main.c +162 -52
index 8d8c3038b..45976c93c 100644
@@ -179,9 +179,22 @@ struct mbuf_table {
 	struct rte_mbuf *m_table[MAX_PKT_BURST];
 };
 
+struct vhost_bufftable {
+	uint32_t len;
+	uint64_t pre_tsc;
+	struct rte_mbuf *m_table[MAX_PKT_BURST];
+};
+
 /* TX queue for each data core. */
 struct mbuf_table lcore_tx_queue[RTE_MAX_LCORE];
 
+/*
+ * Vhost TX buffer for each data core.
+ * Every data core maintains a TX buffer for every vhost device,
+ * which is used for batch pkts enqueue for higher performance.
+ */
+struct vhost_bufftable *vhost_txbuff[RTE_MAX_LCORE * MAX_VHOST_DEVICE];
+
 #define MBUF_TABLE_DRAIN_TSC	((rte_get_tsc_hz() + US_PER_S - 1) \
 				 / US_PER_S * BURST_TX_DRAIN_US)
 #define VLAN_HLEN       4
@@ -804,39 +817,112 @@ unlink_vmdq(struct vhost_dev *vdev)
 	}
 }
 
+static inline void
+free_pkts(struct rte_mbuf **pkts, uint16_t n)
+{
+	while (n--)
+		rte_pktmbuf_free(pkts[n]);
+}
+
 static __rte_always_inline void
-virtio_xmit(struct vhost_dev *dst_vdev, struct vhost_dev *src_vdev,
+complete_async_pkts(struct vhost_dev *vdev)
+{
+	struct rte_mbuf *p_cpl[MAX_PKT_BURST];
+	uint16_t complete_count;
+
+	complete_count = rte_vhost_poll_enqueue_completed(vdev->vid,
+					VIRTIO_RXQ, p_cpl, MAX_PKT_BURST);
+	if (complete_count) {
+		atomic_fetch_sub(&vdev->nr_async_pkts, complete_count);
+		free_pkts(p_cpl, complete_count);
+	}
+}
+
+static __rte_always_inline void
+sync_virtio_xmit(struct vhost_dev *dst_vdev, struct vhost_dev *src_vdev,
 	    struct rte_mbuf *m)
 {
 	uint16_t ret;
-	struct rte_mbuf *m_cpl[1];
 
 	if (builtin_net_driver) {
 		ret = vs_enqueue_pkts(dst_vdev, VIRTIO_RXQ, &m, 1);
-	} else if (async_vhost_driver) {
-		ret = rte_vhost_submit_enqueue_burst(dst_vdev->vid, VIRTIO_RXQ,
-						&m, 1);
-
-		if (likely(ret))
-			dst_vdev->nr_async_pkts++;
-
-		while (likely(dst_vdev->nr_async_pkts)) {
-			if (rte_vhost_poll_enqueue_completed(dst_vdev->vid,
-					VIRTIO_RXQ, m_cpl, 1))
-				dst_vdev->nr_async_pkts--;
-		}
 	} else {
 		ret = rte_vhost_enqueue_burst(dst_vdev->vid, VIRTIO_RXQ, &m, 1);
 	}
 
 	if (enable_stats) {
-		rte_atomic64_inc(&dst_vdev->stats.rx_total_atomic);
-		rte_atomic64_add(&dst_vdev->stats.rx_atomic, ret);
+		atomic_fetch_add(&dst_vdev->stats.rx_total_atomic, 1);
+		atomic_fetch_add(&dst_vdev->stats.rx_atomic, ret);
 		src_vdev->stats.tx_total++;
 		src_vdev->stats.tx += ret;
 	}
 }
 
+static __rte_always_inline void
+drain_vhost(struct vhost_dev *vdev)
+{
+	uint16_t ret;
+	uint64_t buff_idx = rte_lcore_id() * MAX_VHOST_DEVICE + vdev->vid;
+	uint16_t nr_xmit = vhost_txbuff[buff_idx]->len;
+	struct rte_mbuf **m = vhost_txbuff[buff_idx]->m_table;
+
+	if (builtin_net_driver) {
+		ret = vs_enqueue_pkts(vdev, VIRTIO_RXQ, m, nr_xmit);
+	} else if (async_vhost_driver) {
+		uint32_t cpu_cpl_nr = 0;
+		uint16_t enqueue_fail = 0;
+		struct rte_mbuf *m_cpu_cpl[nr_xmit];
+
+		complete_async_pkts(vdev);
+		ret = rte_vhost_submit_enqueue_burst(vdev->vid, VIRTIO_RXQ,
+					m, nr_xmit, m_cpu_cpl, &cpu_cpl_nr);
+		atomic_fetch_add(&vdev->nr_async_pkts, ret - cpu_cpl_nr);
+
+		if (cpu_cpl_nr)
+			free_pkts(m_cpu_cpl, cpu_cpl_nr);
+
+		enqueue_fail = nr_xmit - ret;
+		if (enqueue_fail)
+			free_pkts(&m[ret], nr_xmit - ret);
+	} else {
+		ret = rte_vhost_enqueue_burst(vdev->vid, VIRTIO_RXQ,
+						m, nr_xmit);
+	}
+
+	if (enable_stats) {
+		atomic_fetch_add(&vdev->stats.rx_total_atomic, nr_xmit);
+		atomic_fetch_add(&vdev->stats.rx_atomic, ret);
+	}
+
+	if (!async_vhost_driver)
+		free_pkts(m, nr_xmit);
+}
+
+static __rte_always_inline void
+drain_vhost_table(void)
+{
+	uint16_t lcore_id = rte_lcore_id();
+	struct vhost_bufftable *vhost_txq;
+	struct vhost_dev *vdev;
+	uint64_t cur_tsc;
+
+	TAILQ_FOREACH(vdev, &vhost_dev_list, global_vdev_entry) {
+		vhost_txq = vhost_txbuff[lcore_id * MAX_VHOST_DEVICE
+						+ vdev->vid];
+
+		cur_tsc = rte_rdtsc();
+		if (unlikely(cur_tsc - vhost_txq->pre_tsc
+				> MBUF_TABLE_DRAIN_TSC)) {
+			RTE_LOG_DP(DEBUG, VHOST_DATA,
+				"Vhost TX queue drained after timeout with burst size %u\n",
+				vhost_txq->len);
+			drain_vhost(vdev);
+			vhost_txq->len = 0;
+			vhost_txq->pre_tsc = cur_tsc;
+		}
+	}
+}
+
 /*
  * Check if the packet destination MAC address is for a local device. If so then put
  * the packet on that devices RX queue. If not then return.
@@ -846,7 +932,8 @@ virtio_tx_local(struct vhost_dev *vdev, struct rte_mbuf *m)
 {
 	struct rte_ether_hdr *pkt_hdr;
 	struct vhost_dev *dst_vdev;
-
+	struct vhost_bufftable *vhost_txq;
+	uint16_t lcore_id = rte_lcore_id();
 	pkt_hdr = rte_pktmbuf_mtod(m, struct rte_ether_hdr *);
 
 	dst_vdev = find_vhost_dev(&pkt_hdr->d_addr);
@@ -869,7 +956,19 @@ virtio_tx_local(struct vhost_dev *vdev, struct rte_mbuf *m)
 		return 0;
 	}
 
-	virtio_xmit(dst_vdev, vdev, m);
+	vhost_txq = vhost_txbuff[lcore_id * MAX_VHOST_DEVICE + dst_vdev->vid];
+	vhost_txq->m_table[vhost_txq->len++] = m;
+
+	if (enable_stats) {
+		vdev->stats.tx_total++;
+		vdev->stats.tx++;
+	}
+
+	if (unlikely(vhost_txq->len == MAX_PKT_BURST)) {
+		drain_vhost(dst_vdev);
+		vhost_txq->len = 0;
+		vhost_txq->pre_tsc = rte_rdtsc();
+	}
 	return 0;
 }
 
@@ -940,13 +1039,6 @@ static void virtio_tx_offload(struct rte_mbuf *m)
 	tcp_hdr->cksum = get_psd_sum(l3_hdr, m->ol_flags);
 }
 
-static inline void
-free_pkts(struct rte_mbuf **pkts, uint16_t n)
-{
-	while (n--)
-		rte_pktmbuf_free(pkts[n]);
-}
-
 static __rte_always_inline void
 do_drain_mbuf_table(struct mbuf_table *tx_q)
 {
@@ -979,16 +1071,14 @@ virtio_tx_route(struct vhost_dev *vdev, struct rte_mbuf *m, uint16_t vlan_tag)
 
 		TAILQ_FOREACH(vdev2, &vhost_dev_list, global_vdev_entry) {
 			if (vdev2 != vdev)
-				virtio_xmit(vdev2, vdev, m);
+				sync_virtio_xmit(vdev2, vdev, m);
 		}
 		goto queue2nic;
 	}
 
 	/*check if destination is local VM*/
-	if ((vm2vm_mode == VM2VM_SOFTWARE) && (virtio_tx_local(vdev, m) == 0)) {
-		rte_pktmbuf_free(m);
+	if ((vm2vm_mode == VM2VM_SOFTWARE) && (virtio_tx_local(vdev, m) == 0))
 		return;
-	}
 
 	if (unlikely(vm2vm_mode == VM2VM_HARDWARE)) {
 		if (unlikely(find_local_dest(vdev, m, &offset,
@@ -1073,19 +1163,6 @@ drain_mbuf_table(struct mbuf_table *tx_q)
 	}
 }
 
-static __rte_always_inline void
-complete_async_pkts(struct vhost_dev *vdev, uint16_t qid)
-{
-	struct rte_mbuf *p_cpl[MAX_PKT_BURST];
-	uint16_t complete_count;
-
-	complete_count = rte_vhost_poll_enqueue_completed(vdev->vid,
-						qid, p_cpl, MAX_PKT_BURST);
-	vdev->nr_async_pkts -= complete_count;
-	if (complete_count)
-		free_pkts(p_cpl, complete_count);
-}
-
 static __rte_always_inline void
 drain_eth_rx(struct vhost_dev *vdev)
 {
@@ -1095,9 +1172,6 @@ drain_eth_rx(struct vhost_dev *vdev)
 	rx_count = rte_eth_rx_burst(ports[0], vdev->vmdq_rx_q,
 				    pkts, MAX_PKT_BURST);
 
-	while (likely(vdev->nr_async_pkts))
-		complete_async_pkts(vdev, VIRTIO_RXQ);
-
 	if (!rx_count)
 		return;
 
@@ -1123,17 +1197,31 @@ drain_eth_rx(struct vhost_dev *vdev)
 		enqueue_count = vs_enqueue_pkts(vdev, VIRTIO_RXQ,
 						pkts, rx_count);
 	} else if (async_vhost_driver) {
+		uint32_t cpu_cpl_nr = 0;
+		uint16_t enqueue_fail = 0;
+		struct rte_mbuf *m_cpu_cpl[MAX_PKT_BURST];
+
+		complete_async_pkts(vdev);
 		enqueue_count = rte_vhost_submit_enqueue_burst(vdev->vid,
-					VIRTIO_RXQ, pkts, rx_count);
-		vdev->nr_async_pkts += enqueue_count;
+					VIRTIO_RXQ, pkts, rx_count,
+					m_cpu_cpl, &cpu_cpl_nr);
+		atomic_fetch_add(&vdev->nr_async_pkts,
+					enqueue_count - cpu_cpl_nr);
+		if (cpu_cpl_nr)
+			free_pkts(m_cpu_cpl, cpu_cpl_nr);
+
+		enqueue_fail = rx_count - enqueue_count;
+		if (enqueue_fail)
+			free_pkts(&pkts[enqueue_count], enqueue_fail);
+
 	} else {
 		enqueue_count = rte_vhost_enqueue_burst(vdev->vid, VIRTIO_RXQ,
 						pkts, rx_count);
 	}
 
 	if (enable_stats) {
-		rte_atomic64_add(&vdev->stats.rx_total_atomic, rx_count);
-		rte_atomic64_add(&vdev->stats.rx_atomic, enqueue_count);
+		atomic_fetch_add(&vdev->stats.rx_total_atomic, rx_count);
+		atomic_fetch_add(&vdev->stats.rx_atomic, enqueue_count);
 	}
 
 	if (!async_vhost_driver)
@@ -1202,7 +1290,7 @@ switch_worker(void *arg __rte_unused)
 
 	while(1) {
 		drain_mbuf_table(tx_q);
-
+		drain_vhost_table();
 		/*
 		 * Inform the configuration core that we have exited the
 		 * linked list and that no devices are in use if requested.
@@ -1243,6 +1331,7 @@ destroy_device(int vid)
 {
 	struct vhost_dev *vdev = NULL;
 	int lcore;
+	uint16_t i;
 
 	TAILQ_FOREACH(vdev, &vhost_dev_list, global_vdev_entry) {
 		if (vdev->vid == vid)
@@ -1256,6 +1345,9 @@ destroy_device(int vid)
 		rte_pause();
 	}
 
+	for (i = 0; i < RTE_MAX_LCORE; i++)
+		rte_free(vhost_txbuff[i * MAX_VHOST_DEVICE + vid]);
+
 	if (builtin_net_driver)
 		vs_vhost_net_remove(vdev);
 
@@ -1298,6 +1390,7 @@ static int
 new_device(int vid)
 {
 	int lcore, core_add = 0;
+	uint16_t i;
 	uint32_t device_num_min = num_devices;
 	struct vhost_dev *vdev;
 	vdev = rte_zmalloc("vhost device", sizeof(*vdev), RTE_CACHE_LINE_SIZE);
@@ -1309,6 +1402,19 @@ new_device(int vid)
 	}
 	vdev->vid = vid;
 
+	for (i = 0; i < RTE_MAX_LCORE; i++) {
+		vhost_txbuff[i * MAX_VHOST_DEVICE + vid]
+			= rte_zmalloc("vhost bufftable",
+				sizeof(struct vhost_bufftable),
+				RTE_CACHE_LINE_SIZE);
+
+		if (vhost_txbuff[i * MAX_VHOST_DEVICE + vid] == NULL) {
+			RTE_LOG(INFO, VHOST_DATA,
+			  "(%d) couldn't allocate memory for vhost TX\n", vid);
+			return -1;
+		}
+	}
+
 	if (builtin_net_driver)
 		vs_vhost_net_setup(vdev);
 
@@ -1343,12 +1449,15 @@ new_device(int vid)
 	if (async_vhost_driver) {
 		struct rte_vhost_async_features f;
 		struct rte_vhost_async_channel_ops channel_ops;
+
 		if (strncmp(dma_type, "ioat", 4) == 0) {
 			channel_ops.transfer_data = ioat_transfer_data_cb;
 			channel_ops.check_completed_copies =
 				ioat_check_completed_copies_cb;
+
 			f.async_inorder = 1;
 			f.async_threshold = 256;
+
 			return rte_vhost_async_channel_register(vid, VIRTIO_RXQ,
 				f.intval, &channel_ops);
 		}
@@ -1392,8 +1501,8 @@ print_stats(__rte_unused void *arg)
 			tx         = vdev->stats.tx;
 			tx_dropped = tx_total - tx;
 
-			rx_total   = rte_atomic64_read(&vdev->stats.rx_total_atomic);
-			rx         = rte_atomic64_read(&vdev->stats.rx_atomic);
+			rx_total   = atomic_load(&vdev->stats.rx_total_atomic);
+			rx         = atomic_load(&vdev->stats.rx_atomic);
 			rx_dropped = rx_total - rx;
 
 			printf("Statistics for device %d\n"
@@ -1592,6 +1701,7 @@ main(int argc, char *argv[])
 	/* Register vhost user driver to handle vhost messages. */
 	for (i = 0; i < nb_sockets; i++) {
 		char *file = socket_files + i * PATH_MAX;
+
 		if (async_vhost_driver)
 			flags = flags | RTE_VHOST_USER_ASYNC_COPY;
 
examples/vhost/main.h +4 -3
index 4317b6ae8..6aa798a3e 100644
@@ -8,6 +8,7 @@
 #include <sys/queue.h>
 
 #include <rte_ether.h>
+#include <stdatomic.h>
 
 /* Macros for printing using RTE_LOG */
 #define RTE_LOGTYPE_VHOST_CONFIG RTE_LOGTYPE_USER1
@@ -21,8 +22,8 @@ enum {VIRTIO_RXQ, VIRTIO_TXQ, VIRTIO_QNUM};
 struct device_statistics {
 	uint64_t	tx;
 	uint64_t	tx_total;
-	rte_atomic64_t	rx_atomic;
-	rte_atomic64_t	rx_total_atomic;
+	atomic_int_least64_t	rx_atomic;
+	atomic_int_least64_t	rx_total_atomic;
 };
 
 struct vhost_queue {
@@ -51,7 +52,7 @@ struct vhost_dev {
 	uint64_t features;
 	size_t hdr_len;
 	uint16_t nr_vrings;
-	uint16_t nr_async_pkts;
+	atomic_int_least16_t nr_async_pkts;
 	struct rte_vhost_memory *mem;
 	struct device_statistics stats;
 	TAILQ_ENTRY(vhost_dev) global_vdev_entry;
Signature · 1 line
2.29.2
#20 lore raw

v6 cover examples/vhost: sample code refactor

In reply to Cheng Jiang

Refactor the vhost sample code. Add ioat ring space count and check in ioat callback, optimize vhost data path for batch enqueue, replace rte_atomicNN_xxx to atomic_XXX and refactor vhost async data path.
---
v6:
* adjusted the value of MAX_ENQUEUED_SIZE in ioat.h

v5:
* added vhost enqueue buffer free when destroy a vhost device
* added rte_ioat_completed_ops() fail handler
* changed the behavior of drain_vhost_table() function
* changed some variable names
* changed some variable definition
* added rte_zmalloc() fail handler
* added some comments
* fixed some typos

v4:
* improved code structure
* improved vhost enqueue buffer memory allocation
* cleaned some codes

v3:
* added some variable initiation
* cleaned some codes

v2:
* optimized patch structure
* optimized git log
* replaced rte_atomicNN_xxx to atomic_XXX

Cheng Jiang (2):
examples/vhost: add ioat ring space count and check examples/vhost: refactor vhost data path

 examples/vhost/ioat.c |  22 +++--
 examples/vhost/ioat.h |   2 +-
 examples/vhost/main.c | 214 ++++++++++++++++++++++++++++++++----------
 examples/vhost/main.h |   7 +-
 4 files changed, 179 insertions(+), 66 deletions(-)

--
2.29.2

#21 lore raw

PATCH v6 1/2 examples/vhost: add ioat ring space count and check

Add ioat ring space count and check, if ioat ring space is not enough for the next async vhost packet enqueue, then just return to prevent enqueue failure. Add rte_ioat_completed_ops() fail handler.

---
 examples/vhost/ioat.c | 22 ++++++++++++----------
 1 file changed, 12 insertions(+), 10 deletions(-)
examples/vhost/ioat.c +12 -10
index 71d8a1f1f5..679d1e2f58 100644
@@ -17,6 +17,7 @@ struct packet_tracker {
 	unsigned short next_read;
 	unsigned short next_write;
 	unsigned short last_remain;
+	unsigned short ioat_space;
 };
 
 struct packet_tracker cb_tracker[MAX_VHOST_DEVICE];
@@ -113,7 +114,7 @@ open_ioat(const char *value)
 			goto out;
 		}
 		rte_rawdev_start(dev_id);
-
+		cb_tracker[dev_id].ioat_space = IOAT_RING_SIZE;
 		dma_info->nr++;
 		i++;
 	}
@@ -140,13 +141,9 @@ ioat_transfer_data_cb(int vid, uint16_t queue_id,
 			src = descs[i_desc].src;
 			dst = descs[i_desc].dst;
 			i_seg = 0;
+			if (cb_tracker[dev_id].ioat_space < src->nr_segs)
+				break;
 			while (i_seg < src->nr_segs) {
-				/*
-				 * TODO: Assuming that the ring space of the
-				 * IOAT device is large enough, so there is no
-				 * error here, and the actual error handling
-				 * will be added later.
-				 */
 				rte_ioat_enqueue_copy(dev_id,
 					(uintptr_t)(src->iov[i_seg].iov_base)
 						+ src->offset,
@@ -158,7 +155,8 @@ ioat_transfer_data_cb(int vid, uint16_t queue_id,
 				i_seg++;
 			}
 			write &= mask;
-			cb_tracker[dev_id].size_track[write] = i_seg;
+			cb_tracker[dev_id].size_track[write] = src->nr_segs;
+			cb_tracker[dev_id].ioat_space -= src->nr_segs;
 			write++;
 		}
 	} else {
@@ -178,17 +176,21 @@ ioat_check_completed_copies_cb(int vid, uint16_t queue_id,
 {
 	if (!opaque_data) {
 		uintptr_t dump[255];
-		unsigned short n_seg;
+		int n_seg;
 		unsigned short read, write;
 		unsigned short nb_packet = 0;
 		unsigned short mask = MAX_ENQUEUED_SIZE - 1;
 		unsigned short i;
+
 		int dev_id = dma_bind[vid].dmas[queue_id * 2
 				+ VIRTIO_RXQ].dev_id;
 		n_seg = rte_ioat_completed_ops(dev_id, 255, dump, dump);
-		n_seg += cb_tracker[dev_id].last_remain;
 		if (!n_seg)
 			return 0;
+
+		cb_tracker[dev_id].ioat_space += n_seg;
+		n_seg += cb_tracker[dev_id].last_remain;
+
 		read = cb_tracker[dev_id].next_read;
 		write = cb_tracker[dev_id].next_write;
 		for (i = 0; i < max_packets; i++) {
Signature · 1 line
2.29.2
#22 lore raw

Re: [dpdk-dev] [PATCH v6 1/2] examples/vhost: add ioat ring space count and check

Hi Cheng,

Quoted 76 lines

-----Original Message-----
From: Jiang, Cheng1 <[email protected]>
Sent: Monday, January 4, 2021 12:58 PM
To: [email protected]; Xia, Chenbo <[email protected]>
Cc: [email protected]; Hu, Jiayu <[email protected]>; Yang, YvonneX <[email protected]>; Wang, Yinan <[email protected]>; Jiang, Cheng1 <[email protected]>
Subject: [PATCH v6 1/2] examples/vhost: add ioat ring space count and check

Add ioat ring space count and check, if ioat ring space is not enough for the next async vhost packet enqueue, then just return to prevent enqueue failure. Add rte_ioat_completed_ops() fail handler.

---
 examples/vhost/ioat.c | 22 ++++++++++++----------
 1 file changed, 12 insertions(+), 10 deletions(-)
examples/vhost/ioat.c +7 -9
index 71d8a1f1f5..679d1e2f58 100644
@@ -17,6 +17,7 @@ struct packet_tracker {
 	unsigned short next_read;
 	unsigned short next_write;
 	unsigned short last_remain;
+	unsigned short ioat_space;
 };
 
 struct packet_tracker cb_tracker[MAX_VHOST_DEVICE];
@@ -113,7 +114,7 @@ open_ioat(const char *value)
 			goto out;
 		}
 		rte_rawdev_start(dev_id);
-
+		cb_tracker[dev_id].ioat_space = IOAT_RING_SIZE;
 		dma_info->nr++;
 		i++;
 	}
@@ -140,13 +141,9 @@ ioat_transfer_data_cb(int vid, uint16_t queue_id,
 			src = descs[i_desc].src;
 			dst = descs[i_desc].dst;
 			i_seg = 0;
+			if (cb_tracker[dev_id].ioat_space < src->nr_segs)
+				break;
 			while (i_seg < src->nr_segs) {
-				/*
-				 * TODO: Assuming that the ring space of the
-				 * IOAT device is large enough, so there is no
-				 * error here, and the actual error handling
-				 * will be added later.
-				 */
 				rte_ioat_enqueue_copy(dev_id,
 					(uintptr_t)(src->iov[i_seg].iov_base)
 						+ src->offset,
@@ -158,7 +155,8 @@ ioat_transfer_data_cb(int vid, uint16_t queue_id,
 				i_seg++;
 			}
 			write &= mask;
-			cb_tracker[dev_id].size_track[write] = i_seg;
+			cb_tracker[dev_id].size_track[write] = src->nr_segs;
+			cb_tracker[dev_id].ioat_space -= src->nr_segs;
 			write++;
 		}
 	} else {
@@ -178,17 +176,21 @@ ioat_check_completed_copies_cb(int vid, uint16_t
 queue_id,
 {
 	if (!opaque_data) {
 		uintptr_t dump[255];
-		unsigned short n_seg;
+		int n_seg;
 		unsigned short read, write;
 		unsigned short nb_packet = 0;
 		unsigned short mask = MAX_ENQUEUED_SIZE - 1;
 		unsigned short i;

examples/vhost/ioat.c ioat_check_completed_copies_cb(int vid, uint16_t · line 193

Quoted 10 lines
examples/vhost/ioat.c +4 -1
+
 		int dev_id = dma_bind[vid].dmas[queue_id * 2
 				+ VIRTIO_RXQ].dev_id;
 		n_seg = rte_ioat_completed_ops(dev_id, 255, dump, dump);
-		n_seg += cb_tracker[dev_id].last_remain;
 		if (!n_seg)
 			return 0;
+
+		cb_tracker[dev_id].ioat_space += n_seg;
+		n_seg += cb_tracker[dev_id].last_remain;

When error happens in rte_ioat_completed_ops(), where n_seg is -1, the value of "!n_seg" is false (0) and it can still pass the check of "if".

Thanks,
Jiayu

Quoted 6 lines
examples/vhost/ioat.c +1 -0
+
 		read = cb_tracker[dev_id].next_read;
 		write = cb_tracker[dev_id].next_write;

for (i = 0; i < max_packets; i++) {
--
2.29.2

#23 lore raw

Re: [dpdk-dev] [PATCH v6 1/2] examples/vhost: add ioat ring space count and check

Hi Jiayu,

Quoted 71 lines

-----Original Message-----
From: Hu, Jiayu <[email protected]>
Sent: Tuesday, January 5, 2021 9:20 AM
To: Jiang, Cheng1 <[email protected]>; [email protected]; Xia, Chenbo <[email protected]>
Cc: [email protected]; Yang, YvonneX <[email protected]>; Wang, Yinan <[email protected]>
Subject: RE: [PATCH v6 1/2] examples/vhost: add ioat ring space count and check

Hi Cheng,

examples/vhost/ioat.c ioat_check_completed_copies_cb(int vid,

Quoted 58 lines

-----Original Message-----
From: Jiang, Cheng1 <[email protected]>
Sent: Monday, January 4, 2021 12:58 PM
To: [email protected]; Xia, Chenbo <[email protected]>
Cc: [email protected]; Hu, Jiayu <[email protected]>; Yang, YvonneX <[email protected]>; Wang, Yinan <[email protected]>; Jiang, Cheng1 <[email protected]>
Subject: [PATCH v6 1/2] examples/vhost: add ioat ring space count and check

Add ioat ring space count and check, if ioat ring space is not enough for the next async vhost packet enqueue, then just return to prevent enqueue failure. Add rte_ioat_completed_ops() fail handler.

---
 examples/vhost/ioat.c | 22 ++++++++++++----------
 1 file changed, 12 insertions(+), 10 deletions(-)
examples/vhost/ioat.c index renamed from examples/vhost/ioat.c +0 -0

71d8a1f1f5..679d1e2f58 100644

examples/vhost/ioat.c +3 -7
@@ -17,6 +17,7 @@ struct packet_tracker {  unsigned short next_read;
 unsigned short next_write;  unsigned short last_remain;
+unsigned short ioat_space;
 };
 
 struct packet_tracker cb_tracker[MAX_VHOST_DEVICE]; @@ -113,7 +114,7
 @@ open_ioat(const char *value)  goto out;  }
 rte_rawdev_start(dev_id);
-
+cb_tracker[dev_id].ioat_space = IOAT_RING_SIZE;
 dma_info->nr++;
 i++;
 }
@@ -140,13 +141,9 @@ ioat_transfer_data_cb(int vid, uint16_t queue_id,
 src = descs[i_desc].src;  dst = descs[i_desc].dst;  i_seg = 0;
+if (cb_tracker[dev_id].ioat_space < src->nr_segs) break;
 while (i_seg < src->nr_segs) {
-/*
- * TODO: Assuming that the ring space of the
- * IOAT device is large enough, so there is no
- * error here, and the actual error handling
- * will be added later.
- */
 rte_ioat_enqueue_copy(dev_id,
 (uintptr_t)(src->iov[i_seg].iov_base)
 + src->offset,
@@ -158,7 +155,8 @@ ioat_transfer_data_cb(int vid, uint16_t queue_id,

i_seg++; } write &= mask; -cb_tracker[dev_id].size_track[write] = i_seg;

examples/vhost/ioat.c +2 -0
+cb_tracker[dev_id].size_track[write] = src->nr_segs;
+cb_tracker[dev_id].ioat_space -= src->nr_segs;
 write++;
 }
 } else {
@@ -178,17 +176,21 @@ ioat_check_completed_copies_cb(int vid,

uint16_t

Quoted 17 lines

examples/vhost/ioat.c

Quoted 13 lines

queue_id, { if (!opaque_data) { uintptr_t dump[255]; -unsigned short n_seg;

examples/vhost/ioat.c +2 -0
+int n_seg;
 unsigned short read, write;
 unsigned short nb_packet = 0;
 unsigned short mask = MAX_ENQUEUED_SIZE - 1;  unsigned short i;
+
 int dev_id = dma_bind[vid].dmas[queue_id * 2  + VIRTIO_RXQ].dev_id;

n_seg = rte_ioat_completed_ops(dev_id, 255, dump, dump); -n_seg += cb_tracker[dev_id].last_remain; if (!n_seg) return 0;

examples/vhost/ioat.c +3 -0
+
+cb_tracker[dev_id].ioat_space += n_seg; n_seg +=
+cb_tracker[dev_id].last_remain;

When error happens in rte_ioat_completed_ops(), where n_seg is -1, the value of "!n_seg" is false (0) and it can still pass the check of "if".

You are right, I think I just missed that
I will fix it in the next version, thanks a lot.

Cheng

Quoted 9 lines

Thanks,
Jiayu

Quoted 6 lines

+
read = cb_tracker[dev_id].next_read;
write = cb_tracker[dev_id].next_write; for (i = 0; i < max_packets;
i++) {
--
2.29.2

#24 lore raw

PATCH v6 2/2 examples/vhost: refactor vhost data path

In reply to Cheng Jiang

Change the vm2vm data path to batch enqueue for better performance. Support latest async vhost API, refactor vhost async data path, replace rte_atomicNN_xxx to atomic_XXX and clean some codes.

---
 examples/vhost/ioat.h |   2 +-
 examples/vhost/main.c | 214 ++++++++++++++++++++++++++++++++----------
 examples/vhost/main.h |   7 +-
 3 files changed, 167 insertions(+), 56 deletions(-)
examples/vhost/ioat.h +1 -1
index d6e1e2e079..0a1dbb8117 100644
@@ -11,7 +11,7 @@
 
 #define MAX_VHOST_DEVICE 1024
 #define IOAT_RING_SIZE 4096
-#define MAX_ENQUEUED_SIZE 256
+#define MAX_ENQUEUED_SIZE 512
 
 struct dma_info {
 	struct rte_pci_addr addr;
examples/vhost/main.c +162 -52

355 lines not on this page · raw message

examples/vhost/main.h +4 -3
index 4317b6ae81..6aa798a3e2 100644
@@ -8,6 +8,7 @@
 #include <sys/queue.h>
 
 #include <rte_ether.h>
+#include <stdatomic.h>
 
 /* Macros for printing using RTE_LOG */
 #define RTE_LOGTYPE_VHOST_CONFIG RTE_LOGTYPE_USER1
@@ -21,8 +22,8 @@ enum {VIRTIO_RXQ, VIRTIO_TXQ, VIRTIO_QNUM};
 struct device_statistics {
 	uint64_t	tx;
 	uint64_t	tx_total;
-	rte_atomic64_t	rx_atomic;
-	rte_atomic64_t	rx_total_atomic;
+	atomic_int_least64_t	rx_atomic;
+	atomic_int_least64_t	rx_total_atomic;
 };
 
 struct vhost_queue {
@@ -51,7 +52,7 @@ struct vhost_dev {
 	uint64_t features;
 	size_t hdr_len;
 	uint16_t nr_vrings;
-	uint16_t nr_async_pkts;
+	atomic_int_least16_t nr_async_pkts;
 	struct rte_vhost_memory *mem;
 	struct device_statistics stats;
 	TAILQ_ENTRY(vhost_dev) global_vdev_entry;
Signature · 1 line
2.29.2
Reviewed-by on v6 2/2
#25 lore raw

Re: [dpdk-dev] [PATCH v6 2/2] examples/vhost: refactor vhost data path

Quoted 14 lines

-----Original Message-----
From: Jiang, Cheng1 <[email protected]>
Sent: Monday, January 4, 2021 12:58 PM
To: [email protected]; Xia, Chenbo <[email protected]>
Cc: [email protected]; Hu, Jiayu <[email protected]>; Yang, YvonneX <[email protected]>; Wang, Yinan <[email protected]>; Jiang, Cheng1 <[email protected]>
Subject: [PATCH v6 2/2] examples/vhost: refactor vhost data path

Change the vm2vm data path to batch enqueue for better performance. Support latest async vhost API, refactor vhost async data path, replace rte_atomicNN_xxx to atomic_XXX and clean some codes.

Quoted 7 lines
---
 examples/vhost/ioat.h |   2 +-
 examples/vhost/main.c | 214 ++++++++++++++++++++++++++++++++----------
 examples/vhost/main.h |   7 +-
 3 files changed, 167 insertions(+), 56 deletions(-)
#26 lore raw

v7 cover examples/vhost: sample code refactor

In reply to Cheng Jiang

Refactor the vhost sample code. Add ioat ring space count and check in ioat callback, optimize vhost data path for batch enqueue, replace rte_atomicNN_xxx to atomic_XXX and refactor vhost async data path.
---
v7:
* fixed rte_ioat_completed_ops() fail handler issue

v6:
* adjusted the value of MAX_ENQUEUED_SIZE in ioat.h

v5:
* added vhost enqueue buffer free when destroy a vhost device
* added rte_ioat_completed_ops() fail handler
* changed the behavior of drain_vhost_table() function
* changed some variable names
* changed some variable definition
* added rte_zmalloc() fail handler
* added some comments
* fixed some typos

v4:
* improved code structure
* improved vhost enqueue buffer memory allocation
* cleaned some codes

v3:
* added some variable initiation
* cleaned some codes

v2:
* optimized patch structure
* optimized git log
* replaced rte_atomicNN_xxx to atomic_XXX

Cheng Jiang (2):
examples/vhost: add ioat ring space count and check examples/vhost: refactor vhost data path

 examples/vhost/ioat.c |  24 ++---
 examples/vhost/ioat.h |   2 +-
 examples/vhost/main.c | 214 ++++++++++++++++++++++++++++++++----------
 examples/vhost/main.h |   7 +-
 4 files changed, 180 insertions(+), 67 deletions(-)

--
2.29.2

#27 lore raw

PATCH v7 1/2 examples/vhost: add ioat ring space count and check

Add ioat ring space count and check, if ioat ring space is not enough for the next async vhost packet enqueue, then just return to prevent enqueue failure. Add rte_ioat_completed_ops() fail handler.

---
 examples/vhost/ioat.c | 24 +++++++++++++-----------
 1 file changed, 13 insertions(+), 11 deletions(-)
examples/vhost/ioat.c +13 -11
index 71d8a1f1f5..dbad28d43e 100644
@@ -17,6 +17,7 @@ struct packet_tracker {
 	unsigned short next_read;
 	unsigned short next_write;
 	unsigned short last_remain;
+	unsigned short ioat_space;
 };
 
 struct packet_tracker cb_tracker[MAX_VHOST_DEVICE];
@@ -113,7 +114,7 @@ open_ioat(const char *value)
 			goto out;
 		}
 		rte_rawdev_start(dev_id);
-
+		cb_tracker[dev_id].ioat_space = IOAT_RING_SIZE;
 		dma_info->nr++;
 		i++;
 	}
@@ -140,13 +141,9 @@ ioat_transfer_data_cb(int vid, uint16_t queue_id,
 			src = descs[i_desc].src;
 			dst = descs[i_desc].dst;
 			i_seg = 0;
+			if (cb_tracker[dev_id].ioat_space < src->nr_segs)
+				break;
 			while (i_seg < src->nr_segs) {
-				/*
-				 * TODO: Assuming that the ring space of the
-				 * IOAT device is large enough, so there is no
-				 * error here, and the actual error handling
-				 * will be added later.
-				 */
 				rte_ioat_enqueue_copy(dev_id,
 					(uintptr_t)(src->iov[i_seg].iov_base)
 						+ src->offset,
@@ -158,7 +155,8 @@ ioat_transfer_data_cb(int vid, uint16_t queue_id,
 				i_seg++;
 			}
 			write &= mask;
-			cb_tracker[dev_id].size_track[write] = i_seg;
+			cb_tracker[dev_id].size_track[write] = src->nr_segs;
+			cb_tracker[dev_id].ioat_space -= src->nr_segs;
 			write++;
 		}
 	} else {
@@ -178,17 +176,21 @@ ioat_check_completed_copies_cb(int vid, uint16_t queue_id,
 {
 	if (!opaque_data) {
 		uintptr_t dump[255];
-		unsigned short n_seg;
+		int n_seg;
 		unsigned short read, write;
 		unsigned short nb_packet = 0;
 		unsigned short mask = MAX_ENQUEUED_SIZE - 1;
 		unsigned short i;
+
 		int dev_id = dma_bind[vid].dmas[queue_id * 2
 				+ VIRTIO_RXQ].dev_id;
 		n_seg = rte_ioat_completed_ops(dev_id, 255, dump, dump);
-		n_seg += cb_tracker[dev_id].last_remain;
-		if (!n_seg)
+		if (n_seg <= 0)
 			return 0;
+
+		cb_tracker[dev_id].ioat_space += n_seg;
+		n_seg += cb_tracker[dev_id].last_remain;
+
 		read = cb_tracker[dev_id].next_read;
 		write = cb_tracker[dev_id].next_write;
 		for (i = 0; i < max_packets; i++) {
Signature · 1 line
2.29.2
Reviewed-by on v7 1/2
#28 lore raw

Re: [dpdk-dev] [PATCH v7 1/2] examples/vhost: add ioat ring space count and check

Quoted 93 lines

-----Original Message-----
From: Jiang, Cheng1 <[email protected]>
Sent: Tuesday, January 5, 2021 10:15 AM
To: [email protected]; Xia, Chenbo <[email protected]>
Cc: [email protected]; Hu, Jiayu <[email protected]>; Yang, YvonneX <[email protected]>; Wang, Yinan <[email protected]>; Jiang, Cheng1 <[email protected]>
Subject: [PATCH v7 1/2] examples/vhost: add ioat ring space count and check

Add ioat ring space count and check, if ioat ring space is not enough for the next async vhost packet enqueue, then just return to prevent enqueue failure. Add rte_ioat_completed_ops() fail handler.

---
 examples/vhost/ioat.c | 24 +++++++++++++-----------
 1 file changed, 13 insertions(+), 11 deletions(-)
examples/vhost/ioat.c +13 -11
index 71d8a1f1f5..dbad28d43e 100644
@@ -17,6 +17,7 @@ struct packet_tracker {
 	unsigned short next_read;
 	unsigned short next_write;
 	unsigned short last_remain;
+	unsigned short ioat_space;
 };
 
 struct packet_tracker cb_tracker[MAX_VHOST_DEVICE];
@@ -113,7 +114,7 @@ open_ioat(const char *value)
 			goto out;
 		}
 		rte_rawdev_start(dev_id);
-
+		cb_tracker[dev_id].ioat_space = IOAT_RING_SIZE;
 		dma_info->nr++;
 		i++;
 	}
@@ -140,13 +141,9 @@ ioat_transfer_data_cb(int vid, uint16_t queue_id,
 			src = descs[i_desc].src;
 			dst = descs[i_desc].dst;
 			i_seg = 0;
+			if (cb_tracker[dev_id].ioat_space < src->nr_segs)
+				break;
 			while (i_seg < src->nr_segs) {
-				/*
-				 * TODO: Assuming that the ring space of the
-				 * IOAT device is large enough, so there is no
-				 * error here, and the actual error handling
-				 * will be added later.
-				 */
 				rte_ioat_enqueue_copy(dev_id,
 					(uintptr_t)(src->iov[i_seg].iov_base)
 						+ src->offset,
@@ -158,7 +155,8 @@ ioat_transfer_data_cb(int vid, uint16_t queue_id,
 				i_seg++;
 			}
 			write &= mask;
-			cb_tracker[dev_id].size_track[write] = i_seg;
+			cb_tracker[dev_id].size_track[write] = src->nr_segs;
+			cb_tracker[dev_id].ioat_space -= src->nr_segs;
 			write++;
 		}
 	} else {
@@ -178,17 +176,21 @@ ioat_check_completed_copies_cb(int vid, uint16_t
 queue_id,
 {
 	if (!opaque_data) {
 		uintptr_t dump[255];
-		unsigned short n_seg;
+		int n_seg;
 		unsigned short read, write;
 		unsigned short nb_packet = 0;
 		unsigned short mask = MAX_ENQUEUED_SIZE - 1;
 		unsigned short i;
+
 		int dev_id = dma_bind[vid].dmas[queue_id * 2
 				+ VIRTIO_RXQ].dev_id;
 		n_seg = rte_ioat_completed_ops(dev_id, 255, dump, dump);
-		n_seg += cb_tracker[dev_id].last_remain;
-		if (!n_seg)
+		if (n_seg <= 0)
 			return 0;
+
+		cb_tracker[dev_id].ioat_space += n_seg;
+		n_seg += cb_tracker[dev_id].last_remain;
+
 		read = cb_tracker[dev_id].next_read;
 		write = cb_tracker[dev_id].next_write;

for (i = 0; i < max_packets; i++) {
--
2.29.2

#29 lore raw

PATCH v7 2/2 examples/vhost: refactor vhost data path

In reply to Cheng Jiang

Change the vm2vm data path to batch enqueue for better performance. Support latest async vhost API, refactor vhost async data path, replace rte_atomicNN_xxx to atomic_XXX and clean some codes.

---
 examples/vhost/ioat.h |   2 +-
 examples/vhost/main.c | 214 ++++++++++++++++++++++++++++++++----------
 examples/vhost/main.h |   7 +-
 3 files changed, 167 insertions(+), 56 deletions(-)
examples/vhost/ioat.h +1 -1
index d6e1e2e079..0a1dbb8117 100644
@@ -11,7 +11,7 @@
 
 #define MAX_VHOST_DEVICE 1024
 #define IOAT_RING_SIZE 4096
-#define MAX_ENQUEUED_SIZE 256
+#define MAX_ENQUEUED_SIZE 512
 
 struct dma_info {
 	struct rte_pci_addr addr;
examples/vhost/main.c +162 -52

355 lines not on this page · raw message

examples/vhost/main.h +4 -3
index 4317b6ae81..6aa798a3e2 100644
@@ -8,6 +8,7 @@
 #include <sys/queue.h>
 
 #include <rte_ether.h>
+#include <stdatomic.h>
 
 /* Macros for printing using RTE_LOG */
 #define RTE_LOGTYPE_VHOST_CONFIG RTE_LOGTYPE_USER1
@@ -21,8 +22,8 @@ enum {VIRTIO_RXQ, VIRTIO_TXQ, VIRTIO_QNUM};
 struct device_statistics {
 	uint64_t	tx;
 	uint64_t	tx_total;
-	rte_atomic64_t	rx_atomic;
-	rte_atomic64_t	rx_total_atomic;
+	atomic_int_least64_t	rx_atomic;
+	atomic_int_least64_t	rx_total_atomic;
 };
 
 struct vhost_queue {
@@ -51,7 +52,7 @@ struct vhost_dev {
 	uint64_t features;
 	size_t hdr_len;
 	uint16_t nr_vrings;
-	uint16_t nr_async_pkts;
+	atomic_int_least16_t nr_async_pkts;
 	struct rte_vhost_memory *mem;
 	struct device_statistics stats;
 	TAILQ_ENTRY(vhost_dev) global_vdev_entry;
Signature · 1 line
2.29.2
Tested-by on the v7 series
#30 lore raw
In reply to Cheng Jiang

Regards,
Ling Wei

Quoted 56 lines

-----Original Message-----
From: dev <[email protected]> On Behalf Of Cheng Jiang
Sent: Tuesday, January 5, 2021 10:15 AM
To: [email protected]; Xia, Chenbo <[email protected]>
Cc: [email protected]; Hu, Jiayu <[email protected]>; Yang, YvonneX <[email protected]>; Wang, Yinan <[email protected]>; Jiang, Cheng1 <[email protected]>
Subject: [dpdk-dev] [PATCH v7 0/2] examples/vhost: sample code refactor

Refactor the vhost sample code. Add ioat ring space count and check in ioat callback, optimize vhost data path for batch enqueue, replace rte_atomicNN_xxx to atomic_XXX and refactor vhost async data path.
---
v7:
* fixed rte_ioat_completed_ops() fail handler issue

v6:
* adjusted the value of MAX_ENQUEUED_SIZE in ioat.h

v5:
* added vhost enqueue buffer free when destroy a vhost device
* added rte_ioat_completed_ops() fail handler
* changed the behavior of drain_vhost_table() function
* changed some variable names
* changed some variable definition
* added rte_zmalloc() fail handler
* added some comments
* fixed some typos

v4:
* improved code structure
* improved vhost enqueue buffer memory allocation
* cleaned some codes

v3:
* added some variable initiation
* cleaned some codes

v2:
* optimized patch structure
* optimized git log
* replaced rte_atomicNN_xxx to atomic_XXX

Cheng Jiang (2):
examples/vhost: add ioat ring space count and check examples/vhost: refactor vhost data path

 examples/vhost/ioat.c |  24 ++---
 examples/vhost/ioat.h |   2 +-
 examples/vhost/main.c | 214 ++++++++++++++++++++++++++++++++-----
-----
 examples/vhost/main.h |   7 +-
 4 files changed, 180 insertions(+), 67 deletions(-)

--
2.29.2

#31 lore raw

v8 cover examples/vhost: sample code refactor

In reply to Cheng Jiang

Refactor the vhost sample code. Add ioat ring space count and check in ioat callback, optimize vhost data path for batch enqueue, replace rte_atomicNN_xxx to atomic_XXX and refactor vhost async data path.
---
v8:
* rebased codes

v7:
* fixed rte_ioat_completed_ops() fail handler issue

v6:
* adjusted the value of MAX_ENQUEUED_SIZE in ioat.h

v5:
* added vhost enqueue buffer free when destroy a vhost device
* added rte_ioat_completed_ops() fail handler
* changed the behavior of drain_vhost_table() function
* changed some variable names
* changed some variable definition
* added rte_zmalloc() fail handler
* added some comments
* fixed some typos

v4:
* improved code structure
* improved vhost enqueue buffer memory allocation
* cleaned some codes

v3:
* added some variable initiation
* cleaned some codes

v2:
* optimized patch structure
* optimized git log
* replaced rte_atomicNN_xxx to atomic_XXX

Cheng Jiang (2):
examples/vhost: add ioat ring space count and check examples/vhost: refactor vhost data path

 examples/vhost/ioat.c |  24 +++--
 examples/vhost/ioat.h |   2 +-
 examples/vhost/main.c | 226 ++++++++++++++++++++++++++++++------------
 examples/vhost/main.h |   7 +-
 4 files changed, 181 insertions(+), 78 deletions(-)

--
2.29.2

#32 lore raw

PATCH v8 1/2 examples/vhost: add ioat ring space count and check

Add ioat ring space count and check, if ioat ring space is not enough for the next async vhost packet enqueue, then just return to prevent enqueue failure. Add rte_ioat_completed_ops() fail handler.

---
 examples/vhost/ioat.c | 24 +++++++++++++-----------
 1 file changed, 13 insertions(+), 11 deletions(-)
examples/vhost/ioat.c +13 -11
index 71d8a1f1f..dbad28d43 100644
@@ -17,6 +17,7 @@ struct packet_tracker {
 	unsigned short next_read;
 	unsigned short next_write;
 	unsigned short last_remain;
+	unsigned short ioat_space;
 };
 
 struct packet_tracker cb_tracker[MAX_VHOST_DEVICE];
@@ -113,7 +114,7 @@ open_ioat(const char *value)
 			goto out;
 		}
 		rte_rawdev_start(dev_id);
-
+		cb_tracker[dev_id].ioat_space = IOAT_RING_SIZE;
 		dma_info->nr++;
 		i++;
 	}
@@ -140,13 +141,9 @@ ioat_transfer_data_cb(int vid, uint16_t queue_id,
 			src = descs[i_desc].src;
 			dst = descs[i_desc].dst;
 			i_seg = 0;
+			if (cb_tracker[dev_id].ioat_space < src->nr_segs)
+				break;
 			while (i_seg < src->nr_segs) {
-				/*
-				 * TODO: Assuming that the ring space of the
-				 * IOAT device is large enough, so there is no
-				 * error here, and the actual error handling
-				 * will be added later.
-				 */
 				rte_ioat_enqueue_copy(dev_id,
 					(uintptr_t)(src->iov[i_seg].iov_base)
 						+ src->offset,
@@ -158,7 +155,8 @@ ioat_transfer_data_cb(int vid, uint16_t queue_id,
 				i_seg++;
 			}
 			write &= mask;
-			cb_tracker[dev_id].size_track[write] = i_seg;
+			cb_tracker[dev_id].size_track[write] = src->nr_segs;
+			cb_tracker[dev_id].ioat_space -= src->nr_segs;
 			write++;
 		}
 	} else {
@@ -178,17 +176,21 @@ ioat_check_completed_copies_cb(int vid, uint16_t queue_id,
 {
 	if (!opaque_data) {
 		uintptr_t dump[255];
-		unsigned short n_seg;
+		int n_seg;
 		unsigned short read, write;
 		unsigned short nb_packet = 0;
 		unsigned short mask = MAX_ENQUEUED_SIZE - 1;
 		unsigned short i;
+
 		int dev_id = dma_bind[vid].dmas[queue_id * 2
 				+ VIRTIO_RXQ].dev_id;
 		n_seg = rte_ioat_completed_ops(dev_id, 255, dump, dump);
-		n_seg += cb_tracker[dev_id].last_remain;
-		if (!n_seg)
+		if (n_seg <= 0)
 			return 0;
+
+		cb_tracker[dev_id].ioat_space += n_seg;
+		n_seg += cb_tracker[dev_id].last_remain;
+
 		read = cb_tracker[dev_id].next_read;
 		write = cb_tracker[dev_id].next_write;
 		for (i = 0; i < max_packets; i++) {

--
2.29.2

Reviewed-by on v8 1/2
#33 lore raw

Re: [dpdk-dev] [PATCH v8 1/2] examples/vhost: add ioat ring space count and check

Quoted 10 lines · Cheng Jiang

Add ioat ring space count and check, if ioat ring space is not enough for the next async vhost packet enqueue, then just return to prevent enqueue failure. Add rte_ioat_completed_ops() fail handler.

---
 examples/vhost/ioat.c | 24 +++++++++++++-----------
 1 file changed, 13 insertions(+), 11 deletions(-)

Thanks,
Maxime

#34 lore raw

PATCH v8 2/2 examples/vhost: refactor vhost data path

In reply to Cheng Jiang

Change the vm2vm data path to batch enqueue for better performance. Support latest async vhost API, refactor vhost async data path, replace rte_atomicNN_xxx to atomic_XXX and clean some codes.

---
 examples/vhost/ioat.h |   2 +-
 examples/vhost/main.c | 226 ++++++++++++++++++++++++++++++------------
 examples/vhost/main.h |   7 +-
 3 files changed, 168 insertions(+), 67 deletions(-)
examples/vhost/ioat.h +1 -1
index d6e1e2e07..0a1dbb811 100644
@@ -11,7 +11,7 @@
 
 #define MAX_VHOST_DEVICE 1024
 #define IOAT_RING_SIZE 4096
-#define MAX_ENQUEUED_SIZE 256
+#define MAX_ENQUEUED_SIZE 512
 
 struct dma_info {
 	struct rte_pci_addr addr;
examples/vhost/main.c +163 -63

369 lines not on this page · raw message

examples/vhost/main.h +4 -3
index 4317b6ae8..6aa798a3e 100644
@@ -8,6 +8,7 @@
 #include <sys/queue.h>
 
 #include <rte_ether.h>
+#include <stdatomic.h>
 
 /* Macros for printing using RTE_LOG */
 #define RTE_LOGTYPE_VHOST_CONFIG RTE_LOGTYPE_USER1
@@ -21,8 +22,8 @@ enum {VIRTIO_RXQ, VIRTIO_TXQ, VIRTIO_QNUM};
 struct device_statistics {
 	uint64_t	tx;
 	uint64_t	tx_total;
-	rte_atomic64_t	rx_atomic;
-	rte_atomic64_t	rx_total_atomic;
+	atomic_int_least64_t	rx_atomic;
+	atomic_int_least64_t	rx_total_atomic;
 };
 
 struct vhost_queue {
@@ -51,7 +52,7 @@ struct vhost_dev {
 	uint64_t features;
 	size_t hdr_len;
 	uint16_t nr_vrings;
-	uint16_t nr_async_pkts;
+	atomic_int_least16_t nr_async_pkts;
 	struct rte_vhost_memory *mem;
 	struct device_statistics stats;
 	TAILQ_ENTRY(vhost_dev) global_vdev_entry;

--
2.29.2

#35 lore raw

Re: [dpdk-dev] [PATCH v8 2/2] examples/vhost: refactor vhost data path

Quoted 3 lines · Cheng Jiang

Change the vm2vm data path to batch enqueue for better performance. Support latest async vhost API, refactor vhost async data path, replace rte_atomicNN_xxx to atomic_XXX and clean some codes.

Wouldn't it be better to use GCC/Clang C11 atmoic built-ins like all other code is being migrated to it? (i.e. __atomic_XXX)

Quoted 445 lines
---
 examples/vhost/ioat.h |   2 +-
 examples/vhost/main.c | 226 ++++++++++++++++++++++++++++++------------
 examples/vhost/main.h |   7 +-
 3 files changed, 168 insertions(+), 67 deletions(-)
examples/vhost/ioat.h +1 -1
index d6e1e2e07..0a1dbb811 100644
@@ -11,7 +11,7 @@
 
 #define MAX_VHOST_DEVICE 1024
 #define IOAT_RING_SIZE 4096
-#define MAX_ENQUEUED_SIZE 256
+#define MAX_ENQUEUED_SIZE 512
 
 struct dma_info {
 	struct rte_pci_addr addr;
examples/vhost/main.c +163 -63
index 22309977c..45976c93c 100644
@@ -179,9 +179,22 @@ struct mbuf_table {
 	struct rte_mbuf *m_table[MAX_PKT_BURST];
 };
 
+struct vhost_bufftable {
+	uint32_t len;
+	uint64_t pre_tsc;
+	struct rte_mbuf *m_table[MAX_PKT_BURST];
+};
+
 /* TX queue for each data core. */
 struct mbuf_table lcore_tx_queue[RTE_MAX_LCORE];
 
+/*
+ * Vhost TX buffer for each data core.
+ * Every data core maintains a TX buffer for every vhost device,
+ * which is used for batch pkts enqueue for higher performance.
+ */
+struct vhost_bufftable *vhost_txbuff[RTE_MAX_LCORE * MAX_VHOST_DEVICE];
+
 #define MBUF_TABLE_DRAIN_TSC	((rte_get_tsc_hz() + US_PER_S - 1) \
 				 / US_PER_S * BURST_TX_DRAIN_US)
 #define VLAN_HLEN       4
@@ -804,43 +817,112 @@ unlink_vmdq(struct vhost_dev *vdev)
 	}
 }
 
+static inline void
+free_pkts(struct rte_mbuf **pkts, uint16_t n)
+{
+	while (n--)
+		rte_pktmbuf_free(pkts[n]);
+}
+
 static __rte_always_inline void
-virtio_xmit(struct vhost_dev *dst_vdev, struct vhost_dev *src_vdev,
+complete_async_pkts(struct vhost_dev *vdev)
+{
+	struct rte_mbuf *p_cpl[MAX_PKT_BURST];
+	uint16_t complete_count;
+
+	complete_count = rte_vhost_poll_enqueue_completed(vdev->vid,
+					VIRTIO_RXQ, p_cpl, MAX_PKT_BURST);
+	if (complete_count) {
+		atomic_fetch_sub(&vdev->nr_async_pkts, complete_count);
+		free_pkts(p_cpl, complete_count);
+	}
+}
+
+static __rte_always_inline void
+sync_virtio_xmit(struct vhost_dev *dst_vdev, struct vhost_dev *src_vdev,
 	    struct rte_mbuf *m)
 {
 	uint16_t ret;
-	struct rte_mbuf *m_cpl[1], *comp_pkt;
-	uint32_t nr_comp = 0;
 
 	if (builtin_net_driver) {
 		ret = vs_enqueue_pkts(dst_vdev, VIRTIO_RXQ, &m, 1);
-	} else if (async_vhost_driver) {
-		ret = rte_vhost_submit_enqueue_burst(dst_vdev->vid, VIRTIO_RXQ,
-						&m, 1, &comp_pkt, &nr_comp);
-		if (nr_comp == 1)
-			goto done;
-
-		if (likely(ret))
-			dst_vdev->nr_async_pkts++;
-
-		while (likely(dst_vdev->nr_async_pkts)) {
-			if (rte_vhost_poll_enqueue_completed(dst_vdev->vid,
-					VIRTIO_RXQ, m_cpl, 1))
-				dst_vdev->nr_async_pkts--;
-		}
 	} else {
 		ret = rte_vhost_enqueue_burst(dst_vdev->vid, VIRTIO_RXQ, &m, 1);
 	}
 
-done:
 	if (enable_stats) {
-		rte_atomic64_inc(&dst_vdev->stats.rx_total_atomic);
-		rte_atomic64_add(&dst_vdev->stats.rx_atomic, ret);
+		atomic_fetch_add(&dst_vdev->stats.rx_total_atomic, 1);
+		atomic_fetch_add(&dst_vdev->stats.rx_atomic, ret);
 		src_vdev->stats.tx_total++;
 		src_vdev->stats.tx += ret;
 	}
 }
 
+static __rte_always_inline void
+drain_vhost(struct vhost_dev *vdev)
+{
+	uint16_t ret;
+	uint64_t buff_idx = rte_lcore_id() * MAX_VHOST_DEVICE + vdev->vid;
+	uint16_t nr_xmit = vhost_txbuff[buff_idx]->len;
+	struct rte_mbuf **m = vhost_txbuff[buff_idx]->m_table;
+
+	if (builtin_net_driver) {
+		ret = vs_enqueue_pkts(vdev, VIRTIO_RXQ, m, nr_xmit);
+	} else if (async_vhost_driver) {
+		uint32_t cpu_cpl_nr = 0;
+		uint16_t enqueue_fail = 0;
+		struct rte_mbuf *m_cpu_cpl[nr_xmit];
+
+		complete_async_pkts(vdev);
+		ret = rte_vhost_submit_enqueue_burst(vdev->vid, VIRTIO_RXQ,
+					m, nr_xmit, m_cpu_cpl, &cpu_cpl_nr);
+		atomic_fetch_add(&vdev->nr_async_pkts, ret - cpu_cpl_nr);
+
+		if (cpu_cpl_nr)
+			free_pkts(m_cpu_cpl, cpu_cpl_nr);
+
+		enqueue_fail = nr_xmit - ret;
+		if (enqueue_fail)
+			free_pkts(&m[ret], nr_xmit - ret);
+	} else {
+		ret = rte_vhost_enqueue_burst(vdev->vid, VIRTIO_RXQ,
+						m, nr_xmit);
+	}
+
+	if (enable_stats) {
+		atomic_fetch_add(&vdev->stats.rx_total_atomic, nr_xmit);
+		atomic_fetch_add(&vdev->stats.rx_atomic, ret);
+	}
+
+	if (!async_vhost_driver)
+		free_pkts(m, nr_xmit);
+}
+
+static __rte_always_inline void
+drain_vhost_table(void)
+{
+	uint16_t lcore_id = rte_lcore_id();
+	struct vhost_bufftable *vhost_txq;
+	struct vhost_dev *vdev;
+	uint64_t cur_tsc;
+
+	TAILQ_FOREACH(vdev, &vhost_dev_list, global_vdev_entry) {
+		vhost_txq = vhost_txbuff[lcore_id * MAX_VHOST_DEVICE
+						+ vdev->vid];
+
+		cur_tsc = rte_rdtsc();
+		if (unlikely(cur_tsc - vhost_txq->pre_tsc
+				> MBUF_TABLE_DRAIN_TSC)) {
+			RTE_LOG_DP(DEBUG, VHOST_DATA,
+				"Vhost TX queue drained after timeout with burst size %u\n",
+				vhost_txq->len);
+			drain_vhost(vdev);
+			vhost_txq->len = 0;
+			vhost_txq->pre_tsc = cur_tsc;
+		}
+	}
+}
+
 /*
  * Check if the packet destination MAC address is for a local device. If so then put
  * the packet on that devices RX queue. If not then return.
@@ -850,7 +932,8 @@ virtio_tx_local(struct vhost_dev *vdev, struct rte_mbuf *m)
 {
 	struct rte_ether_hdr *pkt_hdr;
 	struct vhost_dev *dst_vdev;
-
+	struct vhost_bufftable *vhost_txq;
+	uint16_t lcore_id = rte_lcore_id();
 	pkt_hdr = rte_pktmbuf_mtod(m, struct rte_ether_hdr *);
 
 	dst_vdev = find_vhost_dev(&pkt_hdr->d_addr);
@@ -873,7 +956,19 @@ virtio_tx_local(struct vhost_dev *vdev, struct rte_mbuf *m)
 		return 0;
 	}
 
-	virtio_xmit(dst_vdev, vdev, m);
+	vhost_txq = vhost_txbuff[lcore_id * MAX_VHOST_DEVICE + dst_vdev->vid];
+	vhost_txq->m_table[vhost_txq->len++] = m;
+
+	if (enable_stats) {
+		vdev->stats.tx_total++;
+		vdev->stats.tx++;
+	}
+
+	if (unlikely(vhost_txq->len == MAX_PKT_BURST)) {
+		drain_vhost(dst_vdev);
+		vhost_txq->len = 0;
+		vhost_txq->pre_tsc = rte_rdtsc();
+	}
 	return 0;
 }
 
@@ -944,13 +1039,6 @@ static void virtio_tx_offload(struct rte_mbuf *m)
 	tcp_hdr->cksum = get_psd_sum(l3_hdr, m->ol_flags);
 }
 
-static inline void
-free_pkts(struct rte_mbuf **pkts, uint16_t n)
-{
-	while (n--)
-		rte_pktmbuf_free(pkts[n]);
-}
-
 static __rte_always_inline void
 do_drain_mbuf_table(struct mbuf_table *tx_q)
 {
@@ -983,16 +1071,14 @@ virtio_tx_route(struct vhost_dev *vdev, struct rte_mbuf *m, uint16_t vlan_tag)
 
 		TAILQ_FOREACH(vdev2, &vhost_dev_list, global_vdev_entry) {
 			if (vdev2 != vdev)
-				virtio_xmit(vdev2, vdev, m);
+				sync_virtio_xmit(vdev2, vdev, m);
 		}
 		goto queue2nic;
 	}
 
 	/*check if destination is local VM*/
-	if ((vm2vm_mode == VM2VM_SOFTWARE) && (virtio_tx_local(vdev, m) == 0)) {
-		rte_pktmbuf_free(m);
+	if ((vm2vm_mode == VM2VM_SOFTWARE) && (virtio_tx_local(vdev, m) == 0))
 		return;
-	}
 
 	if (unlikely(vm2vm_mode == VM2VM_HARDWARE)) {
 		if (unlikely(find_local_dest(vdev, m, &offset,
@@ -1077,32 +1163,15 @@ drain_mbuf_table(struct mbuf_table *tx_q)
 	}
 }
 
-static __rte_always_inline void
-complete_async_pkts(struct vhost_dev *vdev, uint16_t qid)
-{
-	struct rte_mbuf *p_cpl[MAX_PKT_BURST];
-	uint16_t complete_count;
-
-	complete_count = rte_vhost_poll_enqueue_completed(vdev->vid,
-						qid, p_cpl, MAX_PKT_BURST);
-	vdev->nr_async_pkts -= complete_count;
-	if (complete_count)
-		free_pkts(p_cpl, complete_count);
-}
-
 static __rte_always_inline void
 drain_eth_rx(struct vhost_dev *vdev)
 {
 	uint16_t rx_count, enqueue_count;
-	struct rte_mbuf *pkts[MAX_PKT_BURST], *comp_pkts[MAX_PKT_BURST];
-	uint32_t nr_comp = 0;
+	struct rte_mbuf *pkts[MAX_PKT_BURST];
 
 	rx_count = rte_eth_rx_burst(ports[0], vdev->vmdq_rx_q,
 				    pkts, MAX_PKT_BURST);
 
-	while (likely(vdev->nr_async_pkts))
-		complete_async_pkts(vdev, VIRTIO_RXQ);
-
 	if (!rx_count)
 		return;
 
@@ -1128,22 +1197,31 @@ drain_eth_rx(struct vhost_dev *vdev)
 		enqueue_count = vs_enqueue_pkts(vdev, VIRTIO_RXQ,
 						pkts, rx_count);
 	} else if (async_vhost_driver) {
+		uint32_t cpu_cpl_nr = 0;
+		uint16_t enqueue_fail = 0;
+		struct rte_mbuf *m_cpu_cpl[MAX_PKT_BURST];
+
+		complete_async_pkts(vdev);
 		enqueue_count = rte_vhost_submit_enqueue_burst(vdev->vid,
-					VIRTIO_RXQ, pkts, rx_count, comp_pkts,
-					&nr_comp);
-		if (nr_comp > 0) {
-			free_pkts(comp_pkts, nr_comp);
-			enqueue_count -= nr_comp;
-		}
-		vdev->nr_async_pkts += enqueue_count;
+					VIRTIO_RXQ, pkts, rx_count,
+					m_cpu_cpl, &cpu_cpl_nr);
+		atomic_fetch_add(&vdev->nr_async_pkts,
+					enqueue_count - cpu_cpl_nr);
+		if (cpu_cpl_nr)
+			free_pkts(m_cpu_cpl, cpu_cpl_nr);
+
+		enqueue_fail = rx_count - enqueue_count;
+		if (enqueue_fail)
+			free_pkts(&pkts[enqueue_count], enqueue_fail);
+
 	} else {
 		enqueue_count = rte_vhost_enqueue_burst(vdev->vid, VIRTIO_RXQ,
 						pkts, rx_count);
 	}
 
 	if (enable_stats) {
-		rte_atomic64_add(&vdev->stats.rx_total_atomic, rx_count);
-		rte_atomic64_add(&vdev->stats.rx_atomic, enqueue_count);
+		atomic_fetch_add(&vdev->stats.rx_total_atomic, rx_count);
+		atomic_fetch_add(&vdev->stats.rx_atomic, enqueue_count);
 	}
 
 	if (!async_vhost_driver)
@@ -1212,7 +1290,7 @@ switch_worker(void *arg __rte_unused)
 
 	while(1) {
 		drain_mbuf_table(tx_q);
-
+		drain_vhost_table();
 		/*
 		 * Inform the configuration core that we have exited the
 		 * linked list and that no devices are in use if requested.
@@ -1253,6 +1331,7 @@ destroy_device(int vid)
 {
 	struct vhost_dev *vdev = NULL;
 	int lcore;
+	uint16_t i;
 
 	TAILQ_FOREACH(vdev, &vhost_dev_list, global_vdev_entry) {
 		if (vdev->vid == vid)
@@ -1266,6 +1345,9 @@ destroy_device(int vid)
 		rte_pause();
 	}
 
+	for (i = 0; i < RTE_MAX_LCORE; i++)
+		rte_free(vhost_txbuff[i * MAX_VHOST_DEVICE + vid]);
+
 	if (builtin_net_driver)
 		vs_vhost_net_remove(vdev);
 
@@ -1308,6 +1390,7 @@ static int
 new_device(int vid)
 {
 	int lcore, core_add = 0;
+	uint16_t i;
 	uint32_t device_num_min = num_devices;
 	struct vhost_dev *vdev;
 	vdev = rte_zmalloc("vhost device", sizeof(*vdev), RTE_CACHE_LINE_SIZE);
@@ -1319,6 +1402,19 @@ new_device(int vid)
 	}
 	vdev->vid = vid;
 
+	for (i = 0; i < RTE_MAX_LCORE; i++) {
+		vhost_txbuff[i * MAX_VHOST_DEVICE + vid]
+			= rte_zmalloc("vhost bufftable",
+				sizeof(struct vhost_bufftable),
+				RTE_CACHE_LINE_SIZE);
+
+		if (vhost_txbuff[i * MAX_VHOST_DEVICE + vid] == NULL) {
+			RTE_LOG(INFO, VHOST_DATA,
+			  "(%d) couldn't allocate memory for vhost TX\n", vid);
+			return -1;
+		}
+	}
+
 	if (builtin_net_driver)
 		vs_vhost_net_setup(vdev);
 
@@ -1353,12 +1449,15 @@ new_device(int vid)
 	if (async_vhost_driver) {
 		struct rte_vhost_async_features f;
 		struct rte_vhost_async_channel_ops channel_ops;
+
 		if (strncmp(dma_type, "ioat", 4) == 0) {
 			channel_ops.transfer_data = ioat_transfer_data_cb;
 			channel_ops.check_completed_copies =
 				ioat_check_completed_copies_cb;
+
 			f.async_inorder = 1;
 			f.async_threshold = 256;
+
 			return rte_vhost_async_channel_register(vid, VIRTIO_RXQ,
 				f.intval, &channel_ops);
 		}
@@ -1402,8 +1501,8 @@ print_stats(__rte_unused void *arg)
 			tx         = vdev->stats.tx;
 			tx_dropped = tx_total - tx;
 
-			rx_total   = rte_atomic64_read(&vdev->stats.rx_total_atomic);
-			rx         = rte_atomic64_read(&vdev->stats.rx_atomic);
+			rx_total   = atomic_load(&vdev->stats.rx_total_atomic);
+			rx         = atomic_load(&vdev->stats.rx_atomic);
 			rx_dropped = rx_total - rx;
 
 			printf("Statistics for device %d\n"
@@ -1602,6 +1701,7 @@ main(int argc, char *argv[])
 	/* Register vhost user driver to handle vhost messages. */
 	for (i = 0; i < nb_sockets; i++) {
 		char *file = socket_files + i * PATH_MAX;
+
 		if (async_vhost_driver)
 			flags = flags | RTE_VHOST_USER_ASYNC_COPY;
 
examples/vhost/main.h +4 -4
index 4317b6ae8..6aa798a3e 100644
@@ -8,6 +8,7 @@
 #include <sys/queue.h>
 
 #include <rte_ether.h>
+#include <stdatomic.h>
 
 /* Macros for printing using RTE_LOG */
 #define RTE_LOGTYPE_VHOST_CONFIG RTE_LOGTYPE_USER1
@@ -21,8 +22,8 @@ enum {VIRTIO_RXQ, VIRTIO_TXQ, VIRTIO_QNUM};
 struct device_statistics {
 	uint64_t	tx;
 	uint64_t	tx_total;
-	rte_atomic64_t	rx_atomic;
-	rte_atomic64_t	rx_total_atomic;
+	atomic_int_least64_t	rx_atomic;
+	atomic_int_least64_t	rx_total_atomic;
 };
 
 struct vhost_queue {
@@ -51,7 +52,7 @@ struct vhost_dev {
 	uint64_t features;
 	size_t hdr_len;
 	uint16_t nr_vrings;
-	uint16_t nr_async_pkts;
+	atomic_int_least16_t nr_async_pkts;
 	struct rte_vhost_memory *mem;
 	struct device_statistics stats;
 	TAILQ_ENTRY(vhost_dev) global_vdev_entry;
--

2.29.2

#36 lore raw

Re: [dpdk-dev] [PATCH v8 2/2] examples/vhost: refactor vhost data path

Hi,

Quoted 11 lines

-----Original Message-----
From: Maxime Coquelin <[email protected]>
Sent: Monday, January 11, 2021 10:25 PM
To: Jiang, Cheng1 <[email protected]>; Xia, Chenbo <[email protected]>
Cc: [email protected]; Hu, Jiayu <[email protected]>; Yang, YvonneX <[email protected]>; Wang, Yinan <[email protected]>
Subject: Re: [PATCH v8 2/2] examples/vhost: refactor vhost data path

Quoted 7 lines
Quoted 3 lines · Cheng Jiang

Change the vm2vm data path to batch enqueue for better performance. Support latest async vhost API, refactor vhost async data path, replace rte_atomicNN_xxx to atomic_XXX and clean some codes.

Wouldn't it be better to use GCC/Clang C11 atmoic built-ins like all other code is being migrated to it? (i.e. __atomic_XXX)

Sure, it will be fixed in the next version.

Thanks,
Cheng

Quoted 467 lines
Quoted 6 lines
---
 examples/vhost/ioat.h |   2 +-
 examples/vhost/main.c | 226 ++++++++++++++++++++++++++++++------

------

Quoted 50 lines
 examples/vhost/main.h |   7 +-
 3 files changed, 168 insertions(+), 67 deletions(-)
examples/vhost/ioat.h index renamed from examples/vhost/ioat.h +0 -0

d6e1e2e07..0a1dbb811 100644

examples/vhost/ioat.h +1 -1
@@ -11,7 +11,7 @@
 
 #define MAX_VHOST_DEVICE 1024
 #define IOAT_RING_SIZE 4096
-#define MAX_ENQUEUED_SIZE 256
+#define MAX_ENQUEUED_SIZE 512
 
 struct dma_info {
 	struct rte_pci_addr addr;
examples/vhost/main.c index renamed from examples/vhost/main.c +0 -0

22309977c..45976c93c 100644

examples/vhost/main.c +17 -0
@@ -179,9 +179,22 @@ struct mbuf_table {
 	struct rte_mbuf *m_table[MAX_PKT_BURST];  };
 
+struct vhost_bufftable {
+	uint32_t len;
+	uint64_t pre_tsc;
+	struct rte_mbuf *m_table[MAX_PKT_BURST]; };
+
 /* TX queue for each data core. */
 struct mbuf_table lcore_tx_queue[RTE_MAX_LCORE];
 
+/*
+ * Vhost TX buffer for each data core.
+ * Every data core maintains a TX buffer for every vhost device,
+ * which is used for batch pkts enqueue for higher performance.
+ */
+struct vhost_bufftable *vhost_txbuff[RTE_MAX_LCORE *
+MAX_VHOST_DEVICE];
+
 #define MBUF_TABLE_DRAIN_TSC	((rte_get_tsc_hz() + US_PER_S - 1) \
 				 / US_PER_S * BURST_TX_DRAIN_US)
 #define VLAN_HLEN       4
@@ -804,43 +817,112 @@ unlink_vmdq(struct vhost_dev *vdev)
 	}
 }
 
+static inline void
+free_pkts(struct rte_mbuf **pkts, uint16_t n) {
+	while (n--)
+		rte_pktmbuf_free(pkts[n]);

examples/vhost/main.c unlink_vmdq(struct vhost_dev *vdev) · line 832

Quoted 10 lines
examples/vhost/main.c +8 -1
+}
+
 static __rte_always_inline void
-virtio_xmit(struct vhost_dev *dst_vdev, struct vhost_dev *src_vdev,
+complete_async_pkts(struct vhost_dev *vdev) {
+	struct rte_mbuf *p_cpl[MAX_PKT_BURST];
+	uint16_t complete_count;
+
+	complete_count = rte_vhost_poll_enqueue_completed(vdev->vid,
+					VIRTIO_RXQ, p_cpl,

MAX_PKT_BURST);

Quoted 9 lines
examples/vhost/main.c +9 -0
+	if (complete_count) {
+		atomic_fetch_sub(&vdev->nr_async_pkts, complete_count);
+		free_pkts(p_cpl, complete_count);
+	}
+}
+
+static __rte_always_inline void
+sync_virtio_xmit(struct vhost_dev *dst_vdev, struct vhost_dev
+*src_vdev,

examples/vhost/main.c unlink_vmdq(struct vhost_dev *vdev)

Quoted 10 lines
examples/vhost/main.c +0 -4
 	    struct rte_mbuf *m)
 {
 	uint16_t ret;
-	struct rte_mbuf *m_cpl[1], *comp_pkt;
-	uint32_t nr_comp = 0;
 
 	if (builtin_net_driver) {
 		ret = vs_enqueue_pkts(dst_vdev, VIRTIO_RXQ, &m, 1);
-	} else if (async_vhost_driver) {
-		ret = rte_vhost_submit_enqueue_burst(dst_vdev->vid,

VIRTIO_RXQ,

examples/vhost/main.c unlink_vmdq(struct vhost_dev *vdev) · line 819

Quoted 1 line
examples/vhost/main.c +0 -1
-						&m, 1, &comp_pkt,

&nr_comp);

Quoted 4 lines
examples/vhost/main.c +0 -4
-		if (nr_comp == 1)
-			goto done;
-
-		if (likely(ret))

examples/vhost/main.c unlink_vmdq(struct vhost_dev *vdev)

Quoted 10 lines
examples/vhost/main.c +0 -7
-			dst_vdev->nr_async_pkts++;
-
-		while (likely(dst_vdev->nr_async_pkts)) {
-			if (rte_vhost_poll_enqueue_completed(dst_vdev-
 vid,
-					VIRTIO_RXQ, m_cpl, 1))
-				dst_vdev->nr_async_pkts--;
-		}
 	} else {
 		ret = rte_vhost_enqueue_burst(dst_vdev->vid, VIRTIO_RXQ,

&m, 1);

Quoted 21 lines
examples/vhost/main.c +9 -3
 	}
 
-done:
 	if (enable_stats) {
-		rte_atomic64_inc(&dst_vdev->stats.rx_total_atomic);
-		rte_atomic64_add(&dst_vdev->stats.rx_atomic, ret);
+		atomic_fetch_add(&dst_vdev->stats.rx_total_atomic, 1);
+		atomic_fetch_add(&dst_vdev->stats.rx_atomic, ret);
 		src_vdev->stats.tx_total++;
 		src_vdev->stats.tx += ret;
 	}
 }
 
+static __rte_always_inline void
+drain_vhost(struct vhost_dev *vdev)
+{
+	uint16_t ret;
+	uint64_t buff_idx = rte_lcore_id() * MAX_VHOST_DEVICE + vdev-
 vid;
+	uint16_t nr_xmit = vhost_txbuff[buff_idx]->len;
+	struct rte_mbuf **m = vhost_txbuff[buff_idx]->m_table;

examples/vhost/main.c unlink_vmdq(struct vhost_dev *vdev)

Quoted 10 lines
examples/vhost/main.c +10 -0
+
+	if (builtin_net_driver) {
+		ret = vs_enqueue_pkts(vdev, VIRTIO_RXQ, m, nr_xmit);
+	} else if (async_vhost_driver) {
+		uint32_t cpu_cpl_nr = 0;
+		uint16_t enqueue_fail = 0;
+		struct rte_mbuf *m_cpu_cpl[nr_xmit];
+
+		complete_async_pkts(vdev);
+		ret = rte_vhost_submit_enqueue_burst(vdev->vid,

VIRTIO_RXQ,

examples/vhost/main.c unlink_vmdq(struct vhost_dev *vdev) · line 879

Quoted 1 line
examples/vhost/main.c +1 -0
+					m, nr_xmit, m_cpu_cpl,

&cpu_cpl_nr);

Quoted 29 lines
examples/vhost/main.c +29 -0
+		atomic_fetch_add(&vdev->nr_async_pkts, ret - cpu_cpl_nr);
+
+		if (cpu_cpl_nr)
+			free_pkts(m_cpu_cpl, cpu_cpl_nr);
+
+		enqueue_fail = nr_xmit - ret;
+		if (enqueue_fail)
+			free_pkts(&m[ret], nr_xmit - ret);
+	} else {
+		ret = rte_vhost_enqueue_burst(vdev->vid, VIRTIO_RXQ,
+						m, nr_xmit);
+	}
+
+	if (enable_stats) {
+		atomic_fetch_add(&vdev->stats.rx_total_atomic, nr_xmit);
+		atomic_fetch_add(&vdev->stats.rx_atomic, ret);
+	}
+
+	if (!async_vhost_driver)
+		free_pkts(m, nr_xmit);
+}
+
+static __rte_always_inline void
+drain_vhost_table(void)
+{
+	uint16_t lcore_id = rte_lcore_id();
+	struct vhost_bufftable *vhost_txq;
+	struct vhost_dev *vdev;
+	uint64_t cur_tsc;

examples/vhost/main.c unlink_vmdq(struct vhost_dev *vdev)

Quoted 10 lines
examples/vhost/main.c +10 -0
+
+	TAILQ_FOREACH(vdev, &vhost_dev_list, global_vdev_entry) {
+		vhost_txq = vhost_txbuff[lcore_id * MAX_VHOST_DEVICE
+						+ vdev->vid];
+
+		cur_tsc = rte_rdtsc();
+		if (unlikely(cur_tsc - vhost_txq->pre_tsc
+				> MBUF_TABLE_DRAIN_TSC)) {
+			RTE_LOG_DP(DEBUG, VHOST_DATA,
+				"Vhost TX queue drained after timeout with

burst size %u\n",

examples/vhost/main.c unlink_vmdq(struct vhost_dev *vdev)

Quoted 10 lines
examples/vhost/main.c +8 -0
+				vhost_txq->len);
+			drain_vhost(vdev);
+			vhost_txq->len = 0;
+			vhost_txq->pre_tsc = cur_tsc;
+		}
+	}
+}
+
 /*
  * Check if the packet destination MAC address is for a local device. If so

then put

examples/vhost/main.c virtio_tx_local(struct vhost_dev *vdev, struct · line 939

Quoted 11 lines

* the packet on that devices RX queue. If not then return.

examples/vhost/main.c +2 -1
@@ -850,7 +932,8 @@ virtio_tx_local(struct vhost_dev *vdev, struct
 rte_mbuf *m)  {
 	struct rte_ether_hdr *pkt_hdr;
 	struct vhost_dev *dst_vdev;
-
+	struct vhost_bufftable *vhost_txq;
+	uint16_t lcore_id = rte_lcore_id();
 	pkt_hdr = rte_pktmbuf_mtod(m, struct rte_ether_hdr *);
 
 	dst_vdev = find_vhost_dev(&pkt_hdr->d_addr); @@ -873,7 +956,19

@@

examples/vhost/main.c virtio_tx_local(struct vhost_dev *vdev, struct rte_mbuf *m)

Quoted 6 lines

virtio_tx_local(struct vhost_dev *vdev, struct rte_mbuf *m)

examples/vhost/main.c +1 -1
 		return 0;
 	}
 
-	virtio_xmit(dst_vdev, vdev, m);
+	vhost_txq = vhost_txbuff[lcore_id * MAX_VHOST_DEVICE +

dst_vdev->vid];

Quoted 20 lines
examples/vhost/main.c +12 -2
+	vhost_txq->m_table[vhost_txq->len++] = m;
+
+	if (enable_stats) {
+		vdev->stats.tx_total++;
+		vdev->stats.tx++;
+	}
+
+	if (unlikely(vhost_txq->len == MAX_PKT_BURST)) {
+		drain_vhost(dst_vdev);
+		vhost_txq->len = 0;
+		vhost_txq->pre_tsc = rte_rdtsc();
+	}
 	return 0;
 }
@@ -944,13 +1039,6 @@ static void virtio_tx_offload(struct rte_mbuf *m)
 	tcp_hdr->cksum = get_psd_sum(l3_hdr, m->ol_flags);  }
 
-static inline void
-free_pkts(struct rte_mbuf **pkts, uint16_t n) -{

examples/vhost/main.c static void virtio_tx_offload(struct rte_mbuf *m)

Quoted 10 lines
examples/vhost/main.c +0 -4
-	while (n--)
-		rte_pktmbuf_free(pkts[n]);
-}
-
 static __rte_always_inline void
 do_drain_mbuf_table(struct mbuf_table *tx_q)  { @@ -983,16 +1071,14
 @@ virtio_tx_route(struct vhost_dev *vdev, struct rte_mbuf *m,
 uint16_t vlan_tag)
 
 		TAILQ_FOREACH(vdev2, &vhost_dev_list, global_vdev_entry)

{

examples/vhost/main.c virtio_tx_route(struct vhost_dev *vdev, struct rte_mbuf *m, uint16_t vlan_tag)

Quoted 9 lines
examples/vhost/main.c +1 -2
 			if (vdev2 != vdev)
-				virtio_xmit(vdev2, vdev, m);
+				sync_virtio_xmit(vdev2, vdev, m);
 		}
 		goto queue2nic;
 	}
 
 	/*check if destination is local VM*/
-	if ((vm2vm_mode == VM2VM_SOFTWARE) && (virtio_tx_local(vdev,

m) == 0)) {

examples/vhost/main.c

Quoted 2 lines
examples/vhost/main.c +1 -1
-		rte_pktmbuf_free(m);
+	if ((vm2vm_mode == VM2VM_SOFTWARE) && (virtio_tx_local(vdev,

m) ==

examples/vhost/main.c virtio_tx_route(struct vhost_dev *vdev, struct rte_mbuf *m, uint16_t vlan_tag)

Quoted 6 lines

+0))

examples/vhost/main.c +0 -1
 		return;
-	}
 
 	if (unlikely(vm2vm_mode == VM2VM_HARDWARE)) {
 		if (unlikely(find_local_dest(vdev, m, &offset, @@ -1077,32

+1163,15

Quoted 11 lines

@@ drain_mbuf_table(struct mbuf_table *tx_q)

examples/vhost/main.c +0 -7
 	}
 }
 
-static __rte_always_inline void
-complete_async_pkts(struct vhost_dev *vdev, uint16_t qid) -{
-	struct rte_mbuf *p_cpl[MAX_PKT_BURST];
-	uint16_t complete_count;
-
-	complete_count = rte_vhost_poll_enqueue_completed(vdev->vid,
-						qid, p_cpl, MAX_PKT_BURST);

examples/vhost/main.c drain_mbuf_table(struct mbuf_table *tx_q)

Quoted 10 lines
examples/vhost/main.c +0 -6
-	vdev->nr_async_pkts -= complete_count;
-	if (complete_count)
-		free_pkts(p_cpl, complete_count);
-}
-
 static __rte_always_inline void
 drain_eth_rx(struct vhost_dev *vdev)
 {
 	uint16_t rx_count, enqueue_count;
-	struct rte_mbuf *pkts[MAX_PKT_BURST],

*comp_pkts[MAX_PKT_BURST];

Quoted 14 lines
examples/vhost/main.c +1 -4
-	uint32_t nr_comp = 0;
+	struct rte_mbuf *pkts[MAX_PKT_BURST];
 
 	rx_count = rte_eth_rx_burst(ports[0], vdev->vmdq_rx_q,
 				    pkts, MAX_PKT_BURST);
 
-	while (likely(vdev->nr_async_pkts))
-		complete_async_pkts(vdev, VIRTIO_RXQ);
-
 	if (!rx_count)
 		return;
@@ -1128,22 +1197,31 @@ drain_eth_rx(struct vhost_dev *vdev)
 		enqueue_count = vs_enqueue_pkts(vdev, VIRTIO_RXQ,

examples/vhost/main.c drain_eth_rx(struct vhost_dev *vdev) · line 1133

Quoted 10 lines
examples/vhost/main.c +5 -1
 						pkts, rx_count);
 	} else if (async_vhost_driver) {
+		uint32_t cpu_cpl_nr = 0;
+		uint16_t enqueue_fail = 0;
+		struct rte_mbuf *m_cpu_cpl[MAX_PKT_BURST];
+
+		complete_async_pkts(vdev);
 		enqueue_count = rte_vhost_submit_enqueue_burst(vdev-
 vid,
-					VIRTIO_RXQ, pkts, rx_count,

comp_pkts,

Quoted 9 lines
examples/vhost/main.c +3 -6
-					&nr_comp);
-		if (nr_comp > 0) {
-			free_pkts(comp_pkts, nr_comp);
-			enqueue_count -= nr_comp;
-		}
-		vdev->nr_async_pkts += enqueue_count;
+					VIRTIO_RXQ, pkts, rx_count,
+					m_cpu_cpl, &cpu_cpl_nr);
+		atomic_fetch_add(&vdev->nr_async_pkts,

examples/vhost/main.c drain_eth_rx(struct vhost_dev *vdev)

Quoted 10 lines
examples/vhost/main.c +8 -0
+					enqueue_count - cpu_cpl_nr);
+		if (cpu_cpl_nr)
+			free_pkts(m_cpu_cpl, cpu_cpl_nr);
+
+		enqueue_fail = rx_count - enqueue_count;
+		if (enqueue_fail)
+			free_pkts(&pkts[enqueue_count], enqueue_fail);
+
 	} else {
 		enqueue_count = rte_vhost_enqueue_burst(vdev->vid,

VIRTIO_RXQ,

examples/vhost/main.c drain_eth_rx(struct vhost_dev *vdev)

Quoted 6 lines
examples/vhost/main.c +0 -2
 						pkts, rx_count);
 	}
 
 	if (enable_stats) {
-		rte_atomic64_add(&vdev->stats.rx_total_atomic, rx_count);
-		rte_atomic64_add(&vdev->stats.rx_atomic,

enqueue_count);

examples/vhost/main.c drain_eth_rx(struct vhost_dev *vdev) · line 1225

Quoted 2 lines
examples/vhost/main.c +2 -0
+		atomic_fetch_add(&vdev->stats.rx_total_atomic, rx_count);
+		atomic_fetch_add(&vdev->stats.rx_atomic,

enqueue_count);

Quoted 61 lines
examples/vhost/main.c +0 -0
 	}
 
examples/vhost/main.c +6 -1
 	if (!async_vhost_driver)
@@ -1212,7 +1290,7 @@ switch_worker(void *arg __rte_unused)
 
 	while(1) {
 		drain_mbuf_table(tx_q);
-
+		drain_vhost_table();
 		/*
 		 * Inform the configuration core that we have exited the
 		 * linked list and that no devices are in use if requested.
@@ -1253,6 +1331,7 @@ destroy_device(int vid)  {
 	struct vhost_dev *vdev = NULL;
 	int lcore;
+	uint16_t i;
 
 	TAILQ_FOREACH(vdev, &vhost_dev_list, global_vdev_entry) {
 		if (vdev->vid == vid)
@@ -1266,6 +1345,9 @@ destroy_device(int vid)
 		rte_pause();
 	}
 
+	for (i = 0; i < RTE_MAX_LCORE; i++)
+		rte_free(vhost_txbuff[i * MAX_VHOST_DEVICE + vid]);
+
 	if (builtin_net_driver)
 		vs_vhost_net_remove(vdev);
 
@@ -1308,6 +1390,7 @@ static int
 new_device(int vid)
 {
 	int lcore, core_add = 0;
+	uint16_t i;
 	uint32_t device_num_min = num_devices;
 	struct vhost_dev *vdev;
 	vdev = rte_zmalloc("vhost device", sizeof(*vdev),

RTE_CACHE_LINE_SIZE); @@ -1319,6 +1402,19 @@ new_device(int vid)

examples/vhost/main.c +13 -0
 	}
 	vdev->vid = vid;
 
+	for (i = 0; i < RTE_MAX_LCORE; i++) {
+		vhost_txbuff[i * MAX_VHOST_DEVICE + vid]
+			= rte_zmalloc("vhost bufftable",
+				sizeof(struct vhost_bufftable),
+				RTE_CACHE_LINE_SIZE);
+
+		if (vhost_txbuff[i * MAX_VHOST_DEVICE + vid] == NULL) {
+			RTE_LOG(INFO, VHOST_DATA,
+			  "(%d) couldn't allocate memory for vhost TX\n", vid);
+			return -1;
+		}
+	}
+
 	if (builtin_net_driver)
 		vs_vhost_net_setup(vdev);
@@ -1353,12 +1449,15 @@ new_device(int vid)
 	if (async_vhost_driver) {
 		struct rte_vhost_async_features f;
 		struct rte_vhost_async_channel_ops channel_ops;

examples/vhost/main.c new_device(int vid) · line 1461

Quoted 10 lines
examples/vhost/main.c +3 -0
+
 		if (strncmp(dma_type, "ioat", 4) == 0) {
 			channel_ops.transfer_data = ioat_transfer_data_cb;
 			channel_ops.check_completed_copies =
 				ioat_check_completed_copies_cb;
+
 			f.async_inorder = 1;
 			f.async_threshold = 256;
+
 			return rte_vhost_async_channel_register(vid,

VIRTIO_RXQ,

Quoted 34 lines
examples/vhost/main.c +2 -2
 				f.intval, &channel_ops);
 		}
@@ -1402,8 +1501,8 @@ print_stats(__rte_unused void *arg)
 			tx         = vdev->stats.tx;
 			tx_dropped = tx_total - tx;
 
-			rx_total   = rte_atomic64_read(&vdev-
 stats.rx_total_atomic);
-			rx         = rte_atomic64_read(&vdev->stats.rx_atomic);
+			rx_total   = atomic_load(&vdev-
 stats.rx_total_atomic);
+			rx         = atomic_load(&vdev->stats.rx_atomic);
 			rx_dropped = rx_total - rx;
examples/vhost/main.c +1 -0
 			printf("Statistics for device %d\n"
@@ -1602,6 +1701,7 @@ main(int argc, char *argv[])
 	/* Register vhost user driver to handle vhost messages. */
 	for (i = 0; i < nb_sockets; i++) {
 		char *file = socket_files + i * PATH_MAX;
+
 		if (async_vhost_driver)
 			flags = flags | RTE_VHOST_USER_ASYNC_COPY;
 
examples/vhost/main.h index renamed from examples/vhost/main.h +0 -0

4317b6ae8..6aa798a3e 100644

examples/vhost/main.h +1 -0
@@ -8,6 +8,7 @@
 #include <sys/queue.h>
 
 #include <rte_ether.h>
+#include <stdatomic.h>
 
 /* Macros for printing using RTE_LOG */  #define
Quoted 1 line

RTE_LOGTYPE_VHOST_CONFIG RTE_LOGTYPE_USER1 @@ -21,8 +22,8 @@

enum

Quoted 22 lines

{VIRTIO_RXQ, VIRTIO_TXQ, VIRTIO_QNUM}; struct device_statistics {

examples/vhost/main.h +3 -4
 	uint64_t	tx;
 	uint64_t	tx_total;
-	rte_atomic64_t	rx_atomic;
-	rte_atomic64_t	rx_total_atomic;
+	atomic_int_least64_t	rx_atomic;
+	atomic_int_least64_t	rx_total_atomic;
 };
 
 struct vhost_queue {
@@ -51,7 +52,7 @@ struct vhost_dev {
 	uint64_t features;
 	size_t hdr_len;
 	uint16_t nr_vrings;
-	uint16_t nr_async_pkts;
+	atomic_int_least16_t nr_async_pkts;
 	struct rte_vhost_memory *mem;
 	struct device_statistics stats;
 	TAILQ_ENTRY(vhost_dev) global_vdev_entry;
--

2.29.2

#37 lore raw

v9 cover examples/vhost: sample code refactor

In reply to Cheng Jiang

Refactor the vhost sample code. Add ioat ring space count and check in ioat callback, optimize vhost data path for batch enqueue, replace rte_atomicNN_xxx to __atomic_XXX and refactor vhost async data path.
---
v9:
* migrated atomic_XXX to __atomic_XXX and updated git log

v8:
* rebased codes

v7:
* fixed rte_ioat_completed_ops() fail handler issue

v6:
* adjusted the value of MAX_ENQUEUED_SIZE in ioat.h

v5:
* added vhost enqueue buffer free when destroy a vhost device
* added rte_ioat_completed_ops() fail handler
* changed the behavior of drain_vhost_table() function
* changed some variable names
* changed some variable definition
* added rte_zmalloc() fail handler
* added some comments
* fixed some typos

v4:
* improved code structure
* improved vhost enqueue buffer memory allocation
* cleaned some codes

v3:
* added some variable initiation
* cleaned some codes

v2:
* optimized patch structure
* optimized git log
* replaced rte_atomicNN_xxx to atomic_XXX

Cheng Jiang (2):
examples/vhost: add ioat ring space count and check examples/vhost: refactor vhost data path

 examples/vhost/ioat.c |  24 +++--
 examples/vhost/ioat.h |   2 +-
 examples/vhost/main.c | 238 ++++++++++++++++++++++++++++++------------
 examples/vhost/main.h |   6 +-
 4 files changed, 191 insertions(+), 79 deletions(-)

--
2.29.2

#38 lore raw

PATCH v9 1/2 examples/vhost: add ioat ring space count and check

Add ioat ring space count and check, if ioat ring space is not enough for the next async vhost packet enqueue, then just return to prevent enqueue failure. Add rte_ioat_completed_ops() fail handler.

---
 examples/vhost/ioat.c | 24 +++++++++++++-----------
 1 file changed, 13 insertions(+), 11 deletions(-)
examples/vhost/ioat.c +13 -11
index 71d8a1f1f..dbad28d43 100644
@@ -17,6 +17,7 @@ struct packet_tracker {
 	unsigned short next_read;
 	unsigned short next_write;
 	unsigned short last_remain;
+	unsigned short ioat_space;
 };
 
 struct packet_tracker cb_tracker[MAX_VHOST_DEVICE];
@@ -113,7 +114,7 @@ open_ioat(const char *value)
 			goto out;
 		}
 		rte_rawdev_start(dev_id);
-
+		cb_tracker[dev_id].ioat_space = IOAT_RING_SIZE;
 		dma_info->nr++;
 		i++;
 	}
@@ -140,13 +141,9 @@ ioat_transfer_data_cb(int vid, uint16_t queue_id,
 			src = descs[i_desc].src;
 			dst = descs[i_desc].dst;
 			i_seg = 0;
+			if (cb_tracker[dev_id].ioat_space < src->nr_segs)
+				break;
 			while (i_seg < src->nr_segs) {
-				/*
-				 * TODO: Assuming that the ring space of the
-				 * IOAT device is large enough, so there is no
-				 * error here, and the actual error handling
-				 * will be added later.
-				 */
 				rte_ioat_enqueue_copy(dev_id,
 					(uintptr_t)(src->iov[i_seg].iov_base)
 						+ src->offset,
@@ -158,7 +155,8 @@ ioat_transfer_data_cb(int vid, uint16_t queue_id,
 				i_seg++;
 			}
 			write &= mask;
-			cb_tracker[dev_id].size_track[write] = i_seg;
+			cb_tracker[dev_id].size_track[write] = src->nr_segs;
+			cb_tracker[dev_id].ioat_space -= src->nr_segs;
 			write++;
 		}
 	} else {
@@ -178,17 +176,21 @@ ioat_check_completed_copies_cb(int vid, uint16_t queue_id,
 {
 	if (!opaque_data) {
 		uintptr_t dump[255];
-		unsigned short n_seg;
+		int n_seg;
 		unsigned short read, write;
 		unsigned short nb_packet = 0;
 		unsigned short mask = MAX_ENQUEUED_SIZE - 1;
 		unsigned short i;
+
 		int dev_id = dma_bind[vid].dmas[queue_id * 2
 				+ VIRTIO_RXQ].dev_id;
 		n_seg = rte_ioat_completed_ops(dev_id, 255, dump, dump);
-		n_seg += cb_tracker[dev_id].last_remain;
-		if (!n_seg)
+		if (n_seg <= 0)
 			return 0;
+
+		cb_tracker[dev_id].ioat_space += n_seg;
+		n_seg += cb_tracker[dev_id].last_remain;
+
 		read = cb_tracker[dev_id].next_read;
 		write = cb_tracker[dev_id].next_write;
 		for (i = 0; i < max_packets; i++) {

--
2.29.2

Reviewed-by on v9 1/2
#39 lore raw

Re: [dpdk-dev] [PATCH v9 1/2] examples/vhost: add ioat ring space count and check

Quoted 66 lines · Cheng Jiang

Add ioat ring space count and check, if ioat ring space is not enough for the next async vhost packet enqueue, then just return to prevent enqueue failure. Add rte_ioat_completed_ops() fail handler.

---
 examples/vhost/ioat.c | 24 +++++++++++++-----------
 1 file changed, 13 insertions(+), 11 deletions(-)
examples/vhost/ioat.c +7 -9
index 71d8a1f1f..dbad28d43 100644
@@ -17,6 +17,7 @@ struct packet_tracker {
 	unsigned short next_read;
 	unsigned short next_write;
 	unsigned short last_remain;
+	unsigned short ioat_space;
 };
 
 struct packet_tracker cb_tracker[MAX_VHOST_DEVICE];
@@ -113,7 +114,7 @@ open_ioat(const char *value)
 			goto out;
 		}
 		rte_rawdev_start(dev_id);
-
+		cb_tracker[dev_id].ioat_space = IOAT_RING_SIZE;
 		dma_info->nr++;
 		i++;
 	}
@@ -140,13 +141,9 @@ ioat_transfer_data_cb(int vid, uint16_t queue_id,
 			src = descs[i_desc].src;
 			dst = descs[i_desc].dst;
 			i_seg = 0;
+			if (cb_tracker[dev_id].ioat_space < src->nr_segs)
+				break;
 			while (i_seg < src->nr_segs) {
-				/*
-				 * TODO: Assuming that the ring space of the
-				 * IOAT device is large enough, so there is no
-				 * error here, and the actual error handling
-				 * will be added later.
-				 */
 				rte_ioat_enqueue_copy(dev_id,
 					(uintptr_t)(src->iov[i_seg].iov_base)
 						+ src->offset,
@@ -158,7 +155,8 @@ ioat_transfer_data_cb(int vid, uint16_t queue_id,
 				i_seg++;
 			}
 			write &= mask;
-			cb_tracker[dev_id].size_track[write] = i_seg;
+			cb_tracker[dev_id].size_track[write] = src->nr_segs;
+			cb_tracker[dev_id].ioat_space -= src->nr_segs;
 			write++;
 		}
 	} else {
@@ -178,17 +176,21 @@ ioat_check_completed_copies_cb(int vid, uint16_t queue_id,
 {
 	if (!opaque_data) {
 		uintptr_t dump[255];
-		unsigned short n_seg;
+		int n_seg;
 		unsigned short read, write;
 		unsigned short nb_packet = 0;

examples/vhost/ioat.c ioat_check_completed_copies_cb(int vid, uint16_t queue_id, · line 189

Quoted 10 lines
examples/vhost/ioat.c +2 -2
 		unsigned short mask = MAX_ENQUEUED_SIZE - 1;
 		unsigned short i;
+
 		int dev_id = dma_bind[vid].dmas[queue_id * 2
 				+ VIRTIO_RXQ].dev_id;
 		n_seg = rte_ioat_completed_ops(dev_id, 255, dump, dump);
-		n_seg += cb_tracker[dev_id].last_remain;
-		if (!n_seg)
+		if (n_seg <= 0)
 			return 0;

In a separate patch, it might make sense to propagate the error if rte_ioat_completed_ops return -1.

Maxime

Quoted 10 lines
examples/vhost/ioat.c +4 -1
+
+		cb_tracker[dev_id].ioat_space += n_seg;
+		n_seg += cb_tracker[dev_id].last_remain;
+
 		read = cb_tracker[dev_id].next_read;
 		write = cb_tracker[dev_id].next_write;
 		for (i = 0; i < max_packets; i++) {
--

2.29.2

#40 lore raw

Re: [dpdk-dev] [PATCH v9 1/2] examples/vhost: add ioat ring space count and check

Hi,

Quoted 85 lines

-----Original Message-----
From: Maxime Coquelin <[email protected]>
Sent: Thursday, January 21, 2021 8:35 PM
To: Jiang, Cheng1 <[email protected]>; Xia, Chenbo <[email protected]>
Cc: [email protected]; Hu, Jiayu <[email protected]>; Yang, YvonneX <[email protected]>; Wang, Yinan <[email protected]>
Subject: Re: [PATCH v9 1/2] examples/vhost: add ioat ring space count and check

examples/vhost/ioat.c ioat_check_completed_copies_cb(int vid,

Quoted 59 lines · Cheng Jiang

Add ioat ring space count and check, if ioat ring space is not enough for the next async vhost packet enqueue, then just return to prevent enqueue failure. Add rte_ioat_completed_ops() fail handler.

---
 examples/vhost/ioat.c | 24 +++++++++++++-----------
 1 file changed, 13 insertions(+), 11 deletions(-)
examples/vhost/ioat.c index renamed from examples/vhost/ioat.c +0 -0

71d8a1f1f..dbad28d43 100644

examples/vhost/ioat.c +1 -0
@@ -17,6 +17,7 @@ struct packet_tracker {
 	unsigned short next_read;
 	unsigned short next_write;
 	unsigned short last_remain;
+	unsigned short ioat_space;
 };
 
 struct packet_tracker cb_tracker[MAX_VHOST_DEVICE]; @@ -113,7 +114,7

@@ open_ioat(const char *value)

examples/vhost/ioat.c +5 -8
 			goto out;
 		}
 		rte_rawdev_start(dev_id);
-
+		cb_tracker[dev_id].ioat_space = IOAT_RING_SIZE;
 		dma_info->nr++;
 		i++;
 	}
@@ -140,13 +141,9 @@ ioat_transfer_data_cb(int vid, uint16_t queue_id,
 			src = descs[i_desc].src;
 			dst = descs[i_desc].dst;
 			i_seg = 0;
+			if (cb_tracker[dev_id].ioat_space < src->nr_segs)
+				break;
 			while (i_seg < src->nr_segs) {
-				/*
-				 * TODO: Assuming that the ring space of the
-				 * IOAT device is large enough, so there is no
-				 * error here, and the actual error handling
-				 * will be added later.
-				 */
 				rte_ioat_enqueue_copy(dev_id,
 					(uintptr_t)(src->iov[i_seg].iov_base)
 						+ src->offset,
@@ -158,7 +155,8 @@ ioat_transfer_data_cb(int vid, uint16_t queue_id,
 				i_seg++;
 			}
 			write &= mask;
-			cb_tracker[dev_id].size_track[write] = i_seg;
+			cb_tracker[dev_id].size_track[write] = src->nr_segs;
+			cb_tracker[dev_id].ioat_space -= src->nr_segs;
 			write++;
 		}
 	} else {
@@ -178,17 +176,21 @@ ioat_check_completed_copies_cb(int vid,

uint16_t

Quoted 12 lines
examples/vhost/ioat.c +2 -1
 queue_id,  {
 	if (!opaque_data) {
 		uintptr_t dump[255];
-		unsigned short n_seg;
+		int n_seg;
 		unsigned short read, write;
 		unsigned short nb_packet = 0;
 		unsigned short mask = MAX_ENQUEUED_SIZE - 1;
 		unsigned short i;
+
 		int dev_id = dma_bind[vid].dmas[queue_id * 2
 				+ VIRTIO_RXQ].dev_id;
Quoted 8 lines

examples/vhost/ioat.c ioat_check_completed_copies_cb(int vid, uint16_t queue_id, · line 189

Quoted 5 lines
examples/vhost/ioat.c +1 -2
 		n_seg = rte_ioat_completed_ops(dev_id, 255, dump, dump);
-		n_seg += cb_tracker[dev_id].last_remain;
-		if (!n_seg)
+		if (n_seg <= 0)
 			return 0;

In a separate patch, it might make sense to propagate the error if rte_ioat_completed_ops return -1.

Sure, I'll send a patch to fix it when this patch set is merged.

Thanks,
Cheng

Quoted 14 lines

Maxime

Quoted 10 lines
examples/vhost/ioat.c +4 -1
+
+		cb_tracker[dev_id].ioat_space += n_seg;
+		n_seg += cb_tracker[dev_id].last_remain;
+
 		read = cb_tracker[dev_id].next_read;
 		write = cb_tracker[dev_id].next_write;
 		for (i = 0; i < max_packets; i++) {
--

2.29.2

#41 lore raw

PATCH v9 2/2 examples/vhost: refactor vhost data path

In reply to Cheng Jiang

Change the vm2vm data path to batch enqueue for better performance. Support latest async vhost API, refactor vhost async data path, replace rte_atomicNN_xxx to __atomic_XXX and clean some codes.

---
 examples/vhost/ioat.h |   2 +-
 examples/vhost/main.c | 238 ++++++++++++++++++++++++++++++------------
 examples/vhost/main.h |   6 +-
 3 files changed, 178 insertions(+), 68 deletions(-)
examples/vhost/ioat.h +1 -1
index d6e1e2e07..0a1dbb811 100644
@@ -11,7 +11,7 @@
 
 #define MAX_VHOST_DEVICE 1024
 #define IOAT_RING_SIZE 4096
-#define MAX_ENQUEUED_SIZE 256
+#define MAX_ENQUEUED_SIZE 512
 
 struct dma_info {
 	struct rte_pci_addr addr;
examples/vhost/main.c +174 -64

387 lines not on this page · raw message

examples/vhost/main.h +3 -3
index 4317b6ae8..2d6c05fd7 100644
@@ -21,8 +21,8 @@ enum {VIRTIO_RXQ, VIRTIO_TXQ, VIRTIO_QNUM};
 struct device_statistics {
 	uint64_t	tx;
 	uint64_t	tx_total;
-	rte_atomic64_t	rx_atomic;
-	rte_atomic64_t	rx_total_atomic;
+	uint64_t	rx_atomic;
+	uint64_t	rx_total_atomic;
 };
 
 struct vhost_queue {
@@ -51,7 +51,7 @@ struct vhost_dev {
 	uint64_t features;
 	size_t hdr_len;
 	uint16_t nr_vrings;
-	uint16_t nr_async_pkts;
+	uint64_t nr_async_pkts;
 	struct rte_vhost_memory *mem;
 	struct device_statistics stats;
 	TAILQ_ENTRY(vhost_dev) global_vdev_entry;
Signature · 1 line
2.29.2
Reviewed-by on v9 2/2
#42 lore raw

Re: [dpdk-dev] [PATCH v9 2/2] examples/vhost: refactor vhost data path

Quoted 4 lines · Cheng Jiang

Change the vm2vm data path to batch enqueue for better performance. Support latest async vhost API, refactor vhost async data path, replace rte_atomicNN_xxx to __atomic_XXX and clean some codes.

Quoted 8 lines
---
 examples/vhost/ioat.h |   2 +-
 examples/vhost/main.c | 238 ++++++++++++++++++++++++++++++------------
 examples/vhost/main.h |   6 +-
 3 files changed, 178 insertions(+), 68 deletions(-)

Thanks,
Maxime

#43 lore raw
In reply to Cheng Jiang
Quoted 44 lines · Cheng Jiang

Refactor the vhost sample code. Add ioat ring space count and check in ioat callback, optimize vhost data path for batch enqueue, replace rte_atomicNN_xxx to __atomic_XXX and refactor vhost async data path.
---
v9:
* migrated atomic_XXX to __atomic_XXX and updated git log

v8:
* rebased codes

v7:
* fixed rte_ioat_completed_ops() fail handler issue

v6:
* adjusted the value of MAX_ENQUEUED_SIZE in ioat.h

v5:
* added vhost enqueue buffer free when destroy a vhost device
* added rte_ioat_completed_ops() fail handler
* changed the behavior of drain_vhost_table() function
* changed some variable names
* changed some variable definition
* added rte_zmalloc() fail handler
* added some comments
* fixed some typos

v4:
* improved code structure
* improved vhost enqueue buffer memory allocation
* cleaned some codes

v3:
* added some variable initiation
* cleaned some codes

v2:
* optimized patch structure
* optimized git log
* replaced rte_atomicNN_xxx to atomic_XXX

Cheng Jiang (2):
examples/vhost: add ioat ring space count and check examples/vhost: refactor vhost data path

Quoted 9 lines
 examples/vhost/ioat.c |  24 +++--
 examples/vhost/ioat.h |   2 +-
 examples/vhost/main.c | 238 ++++++++++++++++++++++++++++++------------
 examples/vhost/main.h |   6 +-
 4 files changed, 191 insertions(+), 79 deletions(-)

--
2.29.2

Applied to dpdk-next-virtio/main.

Thanks,
Maxime