* [dpdk-dev] [RFC 00/14] vhost: clean-up and simplify async implementation
@ 2021-10-07 21:59 Maxime Coquelin
2021-10-07 22:00 ` [dpdk-dev] [RFC 01/14] vhost: move async data in a dedicated structure Maxime Coquelin
` (15 more replies)
0 siblings, 16 replies; 21+ messages in thread
From: Maxime Coquelin @ 2021-10-07 21:59 UTC (permalink / raw)
To: dev, chenbo.xia, jiayu.hu, yuanx.wang, wenwux.ma,
bruce.richardson, john.mcnamara
Cc: Maxime Coquelin
This series aims at cleaning and simplifying the async
enqueue path. I think it makes the code easier to
understand, and is necessary before integrating new
changes.
I may have more reworks to propose in next revisions,
but I wanted to share my current status early so that
you have time to review/test it.
It is only compile-tested, as I don't have HW with IOAT
support to test it.
Maxime Coquelin (14):
vhost: move async data in a dedicated structure
vhost: hide inflight async structure
vhost: simplify async IO vectors
vhost: simplify async IO vectors iterators
vhost: remove async batch threshold
vhost: introduce specific iovec structure
vhost: remove useless fields in async iterator struct
vhost: improve IO vector logic
vhost: remove notion of async descriptor
vhost: simplify async enqueue completion
vhost: simplify getting the first in-flight index
vhost: prepare async for mbuf to desc refactoring
vhost: prepare sync for mbuf to desc refactoring
vhost: merge sync and async mbuf to desc filling
examples/vhost/ioat.c | 30 +-
examples/vhost/ioat.h | 2 +-
lib/vhost/rte_vhost_async.h | 42 +--
lib/vhost/vhost.c | 129 +++----
lib/vhost/vhost.h | 67 ++--
lib/vhost/vhost_user.c | 4 +-
lib/vhost/virtio_net.c | 697 ++++++++++++++----------------------
7 files changed, 379 insertions(+), 592 deletions(-)
--
2.31.1
^ permalink raw reply [flat|nested] 21+ messages in thread
* [dpdk-dev] [RFC 01/14] vhost: move async data in a dedicated structure
2021-10-07 21:59 [dpdk-dev] [RFC 00/14] vhost: clean-up and simplify async implementation Maxime Coquelin
@ 2021-10-07 22:00 ` Maxime Coquelin
2021-10-14 3:24 ` Hu, Jiayu
2021-10-07 22:00 ` [dpdk-dev] [RFC 02/14] vhost: hide inflight async structure Maxime Coquelin
` (14 subsequent siblings)
15 siblings, 1 reply; 21+ messages in thread
From: Maxime Coquelin @ 2021-10-07 22:00 UTC (permalink / raw)
To: dev, chenbo.xia, jiayu.hu, yuanx.wang, wenwux.ma,
bruce.richardson, john.mcnamara
Cc: Maxime Coquelin
This patch moves async-related metadata from vhost_virtqueue
to a dedicated struct. It makes it clear which fields are
async related, and also saves some memory when async feature
is not in use.
Signed-off-by: Maxime Coquelin <maxime.coquelin@redhat.com>
---
lib/vhost/vhost.c | 129 ++++++++++++++++-------------------------
lib/vhost/vhost.h | 53 ++++++++---------
lib/vhost/vhost_user.c | 4 +-
lib/vhost/virtio_net.c | 114 +++++++++++++++++++-----------------
4 files changed, 140 insertions(+), 160 deletions(-)
diff --git a/lib/vhost/vhost.c b/lib/vhost/vhost.c
index 9540522dac..58f72b633c 100644
--- a/lib/vhost/vhost.c
+++ b/lib/vhost/vhost.c
@@ -340,19 +340,15 @@ cleanup_device(struct virtio_net *dev, int destroy)
static void
vhost_free_async_mem(struct vhost_virtqueue *vq)
{
- rte_free(vq->async_pkts_info);
+ rte_free(vq->async->pkts_info);
- rte_free(vq->async_buffers_packed);
- vq->async_buffers_packed = NULL;
- rte_free(vq->async_descs_split);
- vq->async_descs_split = NULL;
+ rte_free(vq->async->buffers_packed);
+ vq->async->buffers_packed = NULL;
+ rte_free(vq->async->descs_split);
+ vq->async->descs_split = NULL;
- rte_free(vq->it_pool);
- rte_free(vq->vec_pool);
-
- vq->async_pkts_info = NULL;
- vq->it_pool = NULL;
- vq->vec_pool = NULL;
+ rte_free(vq->async);
+ vq->async = NULL;
}
void
@@ -1629,77 +1625,63 @@ async_channel_register(int vid, uint16_t queue_id,
{
struct virtio_net *dev = get_device(vid);
struct vhost_virtqueue *vq = dev->virtqueue[queue_id];
+ struct vhost_async *async;
+ int node = vq->numa_node;
- if (unlikely(vq->async_registered)) {
+ if (unlikely(vq->async)) {
VHOST_LOG_CONFIG(ERR,
- "async register failed: channel already registered "
- "(vid %d, qid: %d)\n", vid, queue_id);
+ "async register failed: already registered (vid %d, qid: %d)\n",
+ vid, queue_id);
return -1;
}
- vq->async_pkts_info = rte_malloc_socket(NULL,
- vq->size * sizeof(struct async_inflight_info),
- RTE_CACHE_LINE_SIZE, vq->numa_node);
- if (!vq->async_pkts_info) {
- vhost_free_async_mem(vq);
- VHOST_LOG_CONFIG(ERR,
- "async register failed: cannot allocate memory for async_pkts_info "
- "(vid %d, qid: %d)\n", vid, queue_id);
+ async = rte_zmalloc_socket(NULL, sizeof(struct vhost_async), 0, node);
+ if (!async) {
+ VHOST_LOG_CONFIG(ERR, "failed to allocate async metadata (vid %d, qid: %d)\n",
+ vid, queue_id);
return -1;
}
- vq->it_pool = rte_malloc_socket(NULL,
- VHOST_MAX_ASYNC_IT * sizeof(struct rte_vhost_iov_iter),
- RTE_CACHE_LINE_SIZE, vq->numa_node);
- if (!vq->it_pool) {
- vhost_free_async_mem(vq);
- VHOST_LOG_CONFIG(ERR,
- "async register failed: cannot allocate memory for it_pool "
- "(vid %d, qid: %d)\n", vid, queue_id);
- return -1;
- }
-
- vq->vec_pool = rte_malloc_socket(NULL,
- VHOST_MAX_ASYNC_VEC * sizeof(struct iovec),
- RTE_CACHE_LINE_SIZE, vq->numa_node);
- if (!vq->vec_pool) {
- vhost_free_async_mem(vq);
- VHOST_LOG_CONFIG(ERR,
- "async register failed: cannot allocate memory for vec_pool "
- "(vid %d, qid: %d)\n", vid, queue_id);
- return -1;
+ async->pkts_info = rte_malloc_socket(NULL, vq->size * sizeof(struct async_inflight_info),
+ RTE_CACHE_LINE_SIZE, node);
+ if (async->pkts_info) {
+ VHOST_LOG_CONFIG(ERR, "failed to allocate async_pkts_info (vid %d, qid: %d)\n",
+ vid, queue_id);
+ goto out_free_async;
}
if (vq_is_packed(dev)) {
- vq->async_buffers_packed = rte_malloc_socket(NULL,
- vq->size * sizeof(struct vring_used_elem_packed),
- RTE_CACHE_LINE_SIZE, vq->numa_node);
- if (!vq->async_buffers_packed) {
- vhost_free_async_mem(vq);
- VHOST_LOG_CONFIG(ERR,
- "async register failed: cannot allocate memory for async buffers "
- "(vid %d, qid: %d)\n", vid, queue_id);
- return -1;
+ async->buffers_packed = rte_malloc_socket(NULL,
+ vq->size * sizeof(struct vring_used_elem_packed),
+ RTE_CACHE_LINE_SIZE, node);
+ if (!async->buffers_packed) {
+ VHOST_LOG_CONFIG(ERR, "failed to allocate async buffers (vid %d, qid: %d)\n",
+ vid, queue_id);
+ goto out_free_inflight;
}
} else {
- vq->async_descs_split = rte_malloc_socket(NULL,
- vq->size * sizeof(struct vring_used_elem),
- RTE_CACHE_LINE_SIZE, vq->numa_node);
- if (!vq->async_descs_split) {
- vhost_free_async_mem(vq);
- VHOST_LOG_CONFIG(ERR,
- "async register failed: cannot allocate memory for async descs "
- "(vid %d, qid: %d)\n", vid, queue_id);
- return -1;
+ async->descs_split = rte_malloc_socket(NULL,
+ vq->size * sizeof(struct vring_used_elem),
+ RTE_CACHE_LINE_SIZE, node);
+ if (!async->descs_split) {
+ VHOST_LOG_CONFIG(ERR, "failed to allocate async descs (vid %d, qid: %d)\n",
+ vid, queue_id);
+ goto out_free_inflight;
}
}
- vq->async_ops.check_completed_copies = ops->check_completed_copies;
- vq->async_ops.transfer_data = ops->transfer_data;
+ async->ops.check_completed_copies = ops->check_completed_copies;
+ async->ops.transfer_data = ops->transfer_data;
- vq->async_registered = true;
+ vq->async = async;
return 0;
+out_free_inflight:
+ rte_free(async->pkts_info);
+out_free_async:
+ rte_free(async);
+
+ return -1;
}
int
@@ -1793,7 +1775,7 @@ rte_vhost_async_channel_unregister(int vid, uint16_t queue_id)
ret = 0;
- if (!vq->async_registered)
+ if (!vq->async)
return ret;
if (!rte_spinlock_trylock(&vq->access_lock)) {
@@ -1802,7 +1784,7 @@ rte_vhost_async_channel_unregister(int vid, uint16_t queue_id)
return -1;
}
- if (vq->async_pkts_inflight_n) {
+ if (vq->async->pkts_inflight_n) {
VHOST_LOG_CONFIG(ERR, "Failed to unregister async channel. "
"async inflight packets must be completed before unregistration.\n");
ret = -1;
@@ -1810,11 +1792,6 @@ rte_vhost_async_channel_unregister(int vid, uint16_t queue_id)
}
vhost_free_async_mem(vq);
-
- vq->async_ops.transfer_data = NULL;
- vq->async_ops.check_completed_copies = NULL;
- vq->async_registered = false;
-
out:
rte_spinlock_unlock(&vq->access_lock);
@@ -1838,10 +1815,10 @@ rte_vhost_async_channel_unregister_thread_unsafe(int vid, uint16_t queue_id)
if (vq == NULL)
return -1;
- if (!vq->async_registered)
+ if (!vq->async)
return 0;
- if (vq->async_pkts_inflight_n) {
+ if (vq->async->pkts_inflight_n) {
VHOST_LOG_CONFIG(ERR, "Failed to unregister async channel. "
"async inflight packets must be completed before unregistration.\n");
return -1;
@@ -1849,10 +1826,6 @@ rte_vhost_async_channel_unregister_thread_unsafe(int vid, uint16_t queue_id)
vhost_free_async_mem(vq);
- vq->async_ops.transfer_data = NULL;
- vq->async_ops.check_completed_copies = NULL;
- vq->async_registered = false;
-
return 0;
}
@@ -1874,7 +1847,7 @@ rte_vhost_async_get_inflight(int vid, uint16_t queue_id)
if (vq == NULL)
return ret;
- if (!vq->async_registered)
+ if (!vq->async)
return ret;
if (!rte_spinlock_trylock(&vq->access_lock)) {
@@ -1883,7 +1856,7 @@ rte_vhost_async_get_inflight(int vid, uint16_t queue_id)
return ret;
}
- ret = vq->async_pkts_inflight_n;
+ ret = vq->async->pkts_inflight_n;
rte_spinlock_unlock(&vq->access_lock);
return ret;
diff --git a/lib/vhost/vhost.h b/lib/vhost/vhost.h
index 1e56311725..ba33c6a69d 100644
--- a/lib/vhost/vhost.h
+++ b/lib/vhost/vhost.h
@@ -119,6 +119,32 @@ struct vring_used_elem_packed {
uint32_t count;
};
+struct vhost_async {
+ /* operation callbacks for DMA */
+ struct rte_vhost_async_channel_ops ops;
+
+ struct rte_vhost_iov_iter it_pool[VHOST_MAX_ASYNC_IT];
+ struct iovec vec_pool[VHOST_MAX_ASYNC_VEC];
+
+ /* data transfer status */
+ struct async_inflight_info *pkts_info;
+ uint16_t pkts_idx;
+ uint16_t pkts_inflight_n;
+ uint16_t last_pkts_n;
+ union {
+ struct vring_used_elem *descs_split;
+ struct vring_used_elem_packed *buffers_packed;
+ };
+ union {
+ uint16_t desc_idx_split;
+ uint16_t buffer_idx_packed;
+ };
+ union {
+ uint16_t last_desc_idx_split;
+ uint16_t last_buffer_idx_packed;
+ };
+};
+
/**
* Structure contains variables relevant to RX/TX virtqueues.
*/
@@ -193,32 +219,7 @@ struct vhost_virtqueue {
struct rte_vhost_resubmit_info *resubmit_inflight;
uint64_t global_counter;
- /* operation callbacks for async dma */
- struct rte_vhost_async_channel_ops async_ops;
-
- struct rte_vhost_iov_iter *it_pool;
- struct iovec *vec_pool;
-
- /* async data transfer status */
- struct async_inflight_info *async_pkts_info;
- uint16_t async_pkts_idx;
- uint16_t async_pkts_inflight_n;
- uint16_t async_last_pkts_n;
- union {
- struct vring_used_elem *async_descs_split;
- struct vring_used_elem_packed *async_buffers_packed;
- };
- union {
- uint16_t async_desc_idx_split;
- uint16_t async_buffer_idx_packed;
- };
- union {
- uint16_t last_async_desc_idx_split;
- uint16_t last_async_buffer_idx_packed;
- };
-
- /* vq async features */
- bool async_registered;
+ struct vhost_async *async;
int notif_enable;
#define VIRTIO_UNINITIALIZED_NOTIF (-1)
diff --git a/lib/vhost/vhost_user.c b/lib/vhost/vhost_user.c
index 5a894ca0cc..dad4463d45 100644
--- a/lib/vhost/vhost_user.c
+++ b/lib/vhost/vhost_user.c
@@ -2140,8 +2140,8 @@ vhost_user_set_vring_enable(struct virtio_net **pdev,
"set queue enable: %d to qp idx: %d\n",
enable, index);
- if (enable && dev->virtqueue[index]->async_registered) {
- if (dev->virtqueue[index]->async_pkts_inflight_n) {
+ if (enable && dev->virtqueue[index]->async) {
+ if (dev->virtqueue[index]->async->pkts_inflight_n) {
VHOST_LOG_CONFIG(ERR, "failed to enable vring. "
"async inflight packets must be completed first\n");
return RTE_VHOST_MSG_RESULT_ERR;
diff --git a/lib/vhost/virtio_net.c b/lib/vhost/virtio_net.c
index e481906113..a109c2a316 100644
--- a/lib/vhost/virtio_net.c
+++ b/lib/vhost/virtio_net.c
@@ -1510,12 +1510,13 @@ virtio_dev_rx_async_submit_split(struct virtio_net *dev,
uint16_t num_buffers;
uint16_t avail_head;
- struct rte_vhost_iov_iter *it_pool = vq->it_pool;
- struct iovec *vec_pool = vq->vec_pool;
+ struct vhost_async *async = vq->async;
+ struct rte_vhost_iov_iter *it_pool = async->it_pool;
+ struct iovec *vec_pool = async->vec_pool;
struct rte_vhost_async_desc tdes[MAX_PKT_BURST];
struct iovec *src_iovec = vec_pool;
struct iovec *dst_iovec = vec_pool + (VHOST_MAX_ASYNC_VEC >> 1);
- struct async_inflight_info *pkts_info = vq->async_pkts_info;
+ struct async_inflight_info *pkts_info = async->pkts_info;
uint32_t n_pkts = 0, pkt_err = 0;
int32_t n_xfer;
uint16_t segs_await = 0;
@@ -1556,7 +1557,7 @@ virtio_dev_rx_async_submit_split(struct virtio_net *dev,
async_fill_desc(&tdes[pkt_burst_idx++], &it_pool[it_idx],
&it_pool[it_idx + 1]);
- slot_idx = (vq->async_pkts_idx + pkt_idx) & (vq->size - 1);
+ slot_idx = (async->pkts_idx + pkt_idx) & (vq->size - 1);
pkts_info[slot_idx].descs = num_buffers;
pkts_info[slot_idx].mbuf = pkts[pkt_idx];
@@ -1574,7 +1575,7 @@ virtio_dev_rx_async_submit_split(struct virtio_net *dev,
if (unlikely(pkt_burst_idx >= VHOST_ASYNC_BATCH_THRESHOLD ||
((VHOST_MAX_ASYNC_VEC >> 1) - segs_await <
BUF_VECTOR_MAX))) {
- n_xfer = vq->async_ops.transfer_data(dev->vid,
+ n_xfer = async->ops.transfer_data(dev->vid,
queue_id, tdes, 0, pkt_burst_idx);
if (likely(n_xfer >= 0)) {
n_pkts = n_xfer;
@@ -1606,7 +1607,7 @@ virtio_dev_rx_async_submit_split(struct virtio_net *dev,
}
if (pkt_burst_idx) {
- n_xfer = vq->async_ops.transfer_data(dev->vid, queue_id, tdes, 0, pkt_burst_idx);
+ n_xfer = async->ops.transfer_data(dev->vid, queue_id, tdes, 0, pkt_burst_idx);
if (likely(n_xfer >= 0)) {
n_pkts = n_xfer;
} else {
@@ -1638,15 +1639,15 @@ virtio_dev_rx_async_submit_split(struct virtio_net *dev,
/* keep used descriptors */
if (likely(vq->shadow_used_idx)) {
- uint16_t to = vq->async_desc_idx_split & (vq->size - 1);
+ uint16_t to = async->desc_idx_split & (vq->size - 1);
store_dma_desc_info_split(vq->shadow_used_split,
- vq->async_descs_split, vq->size, 0, to,
+ async->descs_split, vq->size, 0, to,
vq->shadow_used_idx);
- vq->async_desc_idx_split += vq->shadow_used_idx;
- vq->async_pkts_idx += pkt_idx;
- vq->async_pkts_inflight_n += pkt_idx;
+ async->desc_idx_split += vq->shadow_used_idx;
+ async->pkts_idx += pkt_idx;
+ async->pkts_inflight_n += pkt_idx;
vq->shadow_used_idx = 0;
}
@@ -1798,7 +1799,7 @@ dma_error_handler_packed(struct vhost_virtqueue *vq, uint16_t slot_idx,
{
uint16_t descs_err = 0;
uint16_t buffers_err = 0;
- struct async_inflight_info *pkts_info = vq->async_pkts_info;
+ struct async_inflight_info *pkts_info = vq->async->pkts_info;
*pkt_idx -= nr_err;
/* calculate the sum of buffers and descs of DMA-error packets. */
@@ -1829,12 +1830,13 @@ virtio_dev_rx_async_submit_packed(struct virtio_net *dev,
uint16_t num_buffers;
uint16_t num_descs;
- struct rte_vhost_iov_iter *it_pool = vq->it_pool;
- struct iovec *vec_pool = vq->vec_pool;
+ struct vhost_async *async = vq->async;
+ struct rte_vhost_iov_iter *it_pool = async->it_pool;
+ struct iovec *vec_pool = async->vec_pool;
struct rte_vhost_async_desc tdes[MAX_PKT_BURST];
struct iovec *src_iovec = vec_pool;
struct iovec *dst_iovec = vec_pool + (VHOST_MAX_ASYNC_VEC >> 1);
- struct async_inflight_info *pkts_info = vq->async_pkts_info;
+ struct async_inflight_info *pkts_info = async->pkts_info;
uint32_t n_pkts = 0, pkt_err = 0;
uint16_t slot_idx = 0;
uint16_t segs_await = 0;
@@ -1851,7 +1853,7 @@ virtio_dev_rx_async_submit_packed(struct virtio_net *dev,
&it_pool[it_idx], &it_pool[it_idx + 1]) < 0))
break;
- slot_idx = (vq->async_pkts_idx + pkt_idx) % vq->size;
+ slot_idx = (async->pkts_idx + pkt_idx) % vq->size;
async_fill_desc(&tdes[pkt_burst_idx++], &it_pool[it_idx],
&it_pool[it_idx + 1]);
@@ -1873,7 +1875,7 @@ virtio_dev_rx_async_submit_packed(struct virtio_net *dev,
*/
if (unlikely(pkt_burst_idx >= VHOST_ASYNC_BATCH_THRESHOLD ||
((VHOST_MAX_ASYNC_VEC >> 1) - segs_await < BUF_VECTOR_MAX))) {
- n_xfer = vq->async_ops.transfer_data(dev->vid,
+ n_xfer = async->ops.transfer_data(dev->vid,
queue_id, tdes, 0, pkt_burst_idx);
if (likely(n_xfer >= 0)) {
n_pkts = n_xfer;
@@ -1904,7 +1906,7 @@ virtio_dev_rx_async_submit_packed(struct virtio_net *dev,
} while (pkt_idx < count);
if (pkt_burst_idx) {
- n_xfer = vq->async_ops.transfer_data(dev->vid, queue_id, tdes, 0, pkt_burst_idx);
+ n_xfer = async->ops.transfer_data(dev->vid, queue_id, tdes, 0, pkt_burst_idx);
if (likely(n_xfer >= 0)) {
n_pkts = n_xfer;
} else {
@@ -1922,20 +1924,20 @@ virtio_dev_rx_async_submit_packed(struct virtio_net *dev,
if (likely(vq->shadow_used_idx)) {
/* keep used descriptors. */
- store_dma_desc_info_packed(vq->shadow_used_packed, vq->async_buffers_packed,
- vq->size, 0, vq->async_buffer_idx_packed,
+ store_dma_desc_info_packed(vq->shadow_used_packed, async->buffers_packed,
+ vq->size, 0, async->buffer_idx_packed,
vq->shadow_used_idx);
- vq->async_buffer_idx_packed += vq->shadow_used_idx;
- if (vq->async_buffer_idx_packed >= vq->size)
- vq->async_buffer_idx_packed -= vq->size;
+ async->buffer_idx_packed += vq->shadow_used_idx;
+ if (async->buffer_idx_packed >= vq->size)
+ async->buffer_idx_packed -= vq->size;
- vq->async_pkts_idx += pkt_idx;
- if (vq->async_pkts_idx >= vq->size)
- vq->async_pkts_idx -= vq->size;
+ async->pkts_idx += pkt_idx;
+ if (async->pkts_idx >= vq->size)
+ async->pkts_idx -= vq->size;
vq->shadow_used_idx = 0;
- vq->async_pkts_inflight_n += pkt_idx;
+ async->pkts_inflight_n += pkt_idx;
}
return pkt_idx;
@@ -1944,28 +1946,29 @@ virtio_dev_rx_async_submit_packed(struct virtio_net *dev,
static __rte_always_inline void
write_back_completed_descs_split(struct vhost_virtqueue *vq, uint16_t n_descs)
{
+ struct vhost_async *async = vq->async;
uint16_t nr_left = n_descs;
uint16_t nr_copy;
uint16_t to, from;
do {
- from = vq->last_async_desc_idx_split & (vq->size - 1);
+ from = async->last_desc_idx_split & (vq->size - 1);
nr_copy = nr_left + from <= vq->size ? nr_left : vq->size - from;
to = vq->last_used_idx & (vq->size - 1);
if (to + nr_copy <= vq->size) {
- rte_memcpy(&vq->used->ring[to], &vq->async_descs_split[from],
+ rte_memcpy(&vq->used->ring[to], &async->descs_split[from],
nr_copy * sizeof(struct vring_used_elem));
} else {
uint16_t size = vq->size - to;
- rte_memcpy(&vq->used->ring[to], &vq->async_descs_split[from],
+ rte_memcpy(&vq->used->ring[to], &async->descs_split[from],
size * sizeof(struct vring_used_elem));
- rte_memcpy(&vq->used->ring[0], &vq->async_descs_split[from + size],
+ rte_memcpy(&vq->used->ring[0], &async->descs_split[from + size],
(nr_copy - size) * sizeof(struct vring_used_elem));
}
- vq->last_async_desc_idx_split += nr_copy;
+ async->last_desc_idx_split += nr_copy;
vq->last_used_idx += nr_copy;
nr_left -= nr_copy;
} while (nr_left > 0);
@@ -1975,20 +1978,21 @@ static __rte_always_inline void
write_back_completed_descs_packed(struct vhost_virtqueue *vq,
uint16_t n_buffers)
{
+ struct vhost_async *async = vq->async;
uint16_t nr_left = n_buffers;
uint16_t from, to;
do {
- from = vq->last_async_buffer_idx_packed;
+ from = async->last_buffer_idx_packed;
to = (from + nr_left) % vq->size;
if (to > from) {
- vhost_update_used_packed(vq, vq->async_buffers_packed + from, to - from);
- vq->last_async_buffer_idx_packed += nr_left;
+ vhost_update_used_packed(vq, async->buffers_packed + from, to - from);
+ async->last_buffer_idx_packed += nr_left;
nr_left = 0;
} else {
- vhost_update_used_packed(vq, vq->async_buffers_packed + from,
+ vhost_update_used_packed(vq, async->buffers_packed + from,
vq->size - from);
- vq->last_async_buffer_idx_packed = 0;
+ async->last_buffer_idx_packed = 0;
nr_left -= vq->size - from;
}
} while (nr_left > 0);
@@ -1999,6 +2003,7 @@ vhost_poll_enqueue_completed(struct virtio_net *dev, uint16_t queue_id,
struct rte_mbuf **pkts, uint16_t count)
{
struct vhost_virtqueue *vq;
+ struct vhost_async *async;
struct async_inflight_info *pkts_info;
int32_t n_cpl;
uint16_t n_pkts_cpl = 0, n_pkts_put = 0, n_descs = 0, n_buffers = 0;
@@ -2006,15 +2011,16 @@ vhost_poll_enqueue_completed(struct virtio_net *dev, uint16_t queue_id,
uint16_t from, i;
vq = dev->virtqueue[queue_id];
- pkts_idx = vq->async_pkts_idx % vq->size;
- pkts_info = vq->async_pkts_info;
+ async = vq->async;
+ pkts_idx = async->pkts_idx % vq->size;
+ pkts_info = async->pkts_info;
vq_size = vq->size;
start_idx = virtio_dev_rx_async_get_info_idx(pkts_idx,
- vq_size, vq->async_pkts_inflight_n);
+ vq_size, async->pkts_inflight_n);
- if (count > vq->async_last_pkts_n) {
- n_cpl = vq->async_ops.check_completed_copies(dev->vid,
- queue_id, 0, count - vq->async_last_pkts_n);
+ if (count > async->last_pkts_n) {
+ n_cpl = async->ops.check_completed_copies(dev->vid,
+ queue_id, 0, count - async->last_pkts_n);
if (likely(n_cpl >= 0)) {
n_pkts_cpl = n_cpl;
} else {
@@ -2025,10 +2031,10 @@ vhost_poll_enqueue_completed(struct virtio_net *dev, uint16_t queue_id,
}
}
- n_pkts_cpl += vq->async_last_pkts_n;
+ n_pkts_cpl += async->last_pkts_n;
n_pkts_put = RTE_MIN(n_pkts_cpl, count);
if (unlikely(n_pkts_put == 0)) {
- vq->async_last_pkts_n = n_pkts_cpl;
+ async->last_pkts_n = n_pkts_cpl;
return 0;
}
@@ -2045,8 +2051,8 @@ vhost_poll_enqueue_completed(struct virtio_net *dev, uint16_t queue_id,
pkts[i] = pkts_info[from].mbuf;
}
}
- vq->async_last_pkts_n = n_pkts_cpl - n_pkts_put;
- vq->async_pkts_inflight_n -= n_pkts_put;
+ async->last_pkts_n = n_pkts_cpl - n_pkts_put;
+ async->pkts_inflight_n -= n_pkts_put;
if (likely(vq->enabled && vq->access_ok)) {
if (vq_is_packed(dev)) {
@@ -2062,11 +2068,11 @@ vhost_poll_enqueue_completed(struct virtio_net *dev, uint16_t queue_id,
}
} else {
if (vq_is_packed(dev)) {
- vq->last_async_buffer_idx_packed += n_buffers;
- if (vq->last_async_buffer_idx_packed >= vq->size)
- vq->last_async_buffer_idx_packed -= vq->size;
+ async->last_buffer_idx_packed += n_buffers;
+ if (async->last_buffer_idx_packed >= vq->size)
+ async->last_buffer_idx_packed -= vq->size;
} else {
- vq->last_async_desc_idx_split += n_descs;
+ async->last_desc_idx_split += n_descs;
}
}
@@ -2093,7 +2099,7 @@ rte_vhost_poll_enqueue_completed(int vid, uint16_t queue_id,
vq = dev->virtqueue[queue_id];
- if (unlikely(!vq->async_registered)) {
+ if (unlikely(!vq->async)) {
VHOST_LOG_DATA(ERR, "(%d) %s: async not registered for queue id %d.\n",
dev->vid, __func__, queue_id);
return 0;
@@ -2128,7 +2134,7 @@ rte_vhost_clear_queue_thread_unsafe(int vid, uint16_t queue_id,
vq = dev->virtqueue[queue_id];
- if (unlikely(!vq->async_registered)) {
+ if (unlikely(!vq->async)) {
VHOST_LOG_DATA(ERR, "(%d) %s: async not registered for queue id %d.\n",
dev->vid, __func__, queue_id);
return 0;
@@ -2157,7 +2163,7 @@ virtio_dev_rx_async_submit(struct virtio_net *dev, uint16_t queue_id,
rte_spinlock_lock(&vq->access_lock);
- if (unlikely(!vq->enabled || !vq->async_registered))
+ if (unlikely(!vq->enabled || !vq->async))
goto out_access_unlock;
if (dev->features & (1ULL << VIRTIO_F_IOMMU_PLATFORM))
--
2.31.1
^ permalink raw reply [flat|nested] 21+ messages in thread
* [dpdk-dev] [RFC 02/14] vhost: hide inflight async structure
2021-10-07 21:59 [dpdk-dev] [RFC 00/14] vhost: clean-up and simplify async implementation Maxime Coquelin
2021-10-07 22:00 ` [dpdk-dev] [RFC 01/14] vhost: move async data in a dedicated structure Maxime Coquelin
@ 2021-10-07 22:00 ` Maxime Coquelin
2021-10-07 22:00 ` [dpdk-dev] [RFC 03/14] vhost: simplify async IO vectors Maxime Coquelin
` (13 subsequent siblings)
15 siblings, 0 replies; 21+ messages in thread
From: Maxime Coquelin @ 2021-10-07 22:00 UTC (permalink / raw)
To: dev, chenbo.xia, jiayu.hu, yuanx.wang, wenwux.ma,
bruce.richardson, john.mcnamara
Cc: Maxime Coquelin
This patch moves async_inflight_info struct to internal
header since it should not be part of the API.
Signed-off-by: Maxime Coquelin <maxime.coquelin@redhat.com>
---
lib/vhost/rte_vhost_async.h | 9 ---------
lib/vhost/vhost.h | 9 +++++++++
2 files changed, 9 insertions(+), 9 deletions(-)
diff --git a/lib/vhost/rte_vhost_async.h b/lib/vhost/rte_vhost_async.h
index ad71555a7f..789b80f5a0 100644
--- a/lib/vhost/rte_vhost_async.h
+++ b/lib/vhost/rte_vhost_async.h
@@ -83,15 +83,6 @@ struct rte_vhost_async_channel_ops {
uint16_t max_packets);
};
-/**
- * inflight async packet information
- */
-struct async_inflight_info {
- struct rte_mbuf *mbuf;
- uint16_t descs; /* num of descs inflight */
- uint16_t nr_buffers; /* num of buffers inflight for packed ring */
-};
-
/**
* async channel features
*/
diff --git a/lib/vhost/vhost.h b/lib/vhost/vhost.h
index ba33c6a69d..9de87d20cc 100644
--- a/lib/vhost/vhost.h
+++ b/lib/vhost/vhost.h
@@ -119,6 +119,15 @@ struct vring_used_elem_packed {
uint32_t count;
};
+/**
+ * inflight async packet information
+ */
+struct async_inflight_info {
+ struct rte_mbuf *mbuf;
+ uint16_t descs; /* num of descs inflight */
+ uint16_t nr_buffers; /* num of buffers inflight for packed ring */
+};
+
struct vhost_async {
/* operation callbacks for DMA */
struct rte_vhost_async_channel_ops ops;
--
2.31.1
^ permalink raw reply [flat|nested] 21+ messages in thread
* [dpdk-dev] [RFC 03/14] vhost: simplify async IO vectors
2021-10-07 21:59 [dpdk-dev] [RFC 00/14] vhost: clean-up and simplify async implementation Maxime Coquelin
2021-10-07 22:00 ` [dpdk-dev] [RFC 01/14] vhost: move async data in a dedicated structure Maxime Coquelin
2021-10-07 22:00 ` [dpdk-dev] [RFC 02/14] vhost: hide inflight async structure Maxime Coquelin
@ 2021-10-07 22:00 ` Maxime Coquelin
2021-10-07 22:00 ` [dpdk-dev] [RFC 04/14] vhost: simplify async IO vectors iterators Maxime Coquelin
` (12 subsequent siblings)
15 siblings, 0 replies; 21+ messages in thread
From: Maxime Coquelin @ 2021-10-07 22:00 UTC (permalink / raw)
To: dev, chenbo.xia, jiayu.hu, yuanx.wang, wenwux.ma,
bruce.richardson, john.mcnamara
Cc: Maxime Coquelin
IO vectors implementation is unnecessarily complex, mixing
source and destinations vectors in the same array.
This patch declares two arrays, one for the source and one
for the destination. It also get rid off seg_awaits variable
in both packed and split implementation, which is the same
as iovec_idx.
Signed-off-by: Maxime Coquelin <maxime.coquelin@redhat.com>
---
lib/vhost/vhost.h | 5 +++--
lib/vhost/virtio_net.c | 28 +++++++++++-----------------
2 files changed, 14 insertions(+), 19 deletions(-)
diff --git a/lib/vhost/vhost.h b/lib/vhost/vhost.h
index 9de87d20cc..f2d9535174 100644
--- a/lib/vhost/vhost.h
+++ b/lib/vhost/vhost.h
@@ -49,7 +49,7 @@
#define MAX_PKT_BURST 32
#define VHOST_MAX_ASYNC_IT (MAX_PKT_BURST * 2)
-#define VHOST_MAX_ASYNC_VEC (BUF_VECTOR_MAX * 4)
+#define VHOST_MAX_ASYNC_VEC (BUF_VECTOR_MAX * 2)
#define PACKED_DESC_ENQUEUE_USED_FLAG(w) \
((w) ? (VRING_DESC_F_AVAIL | VRING_DESC_F_USED | VRING_DESC_F_WRITE) : \
@@ -133,7 +133,8 @@ struct vhost_async {
struct rte_vhost_async_channel_ops ops;
struct rte_vhost_iov_iter it_pool[VHOST_MAX_ASYNC_IT];
- struct iovec vec_pool[VHOST_MAX_ASYNC_VEC];
+ struct iovec src_iovec[VHOST_MAX_ASYNC_VEC];
+ struct iovec dst_iovec[VHOST_MAX_ASYNC_VEC];
/* data transfer status */
struct async_inflight_info *pkts_info;
diff --git a/lib/vhost/virtio_net.c b/lib/vhost/virtio_net.c
index a109c2a316..4e0e1584b8 100644
--- a/lib/vhost/virtio_net.c
+++ b/lib/vhost/virtio_net.c
@@ -1512,14 +1512,12 @@ virtio_dev_rx_async_submit_split(struct virtio_net *dev,
struct vhost_async *async = vq->async;
struct rte_vhost_iov_iter *it_pool = async->it_pool;
- struct iovec *vec_pool = async->vec_pool;
struct rte_vhost_async_desc tdes[MAX_PKT_BURST];
- struct iovec *src_iovec = vec_pool;
- struct iovec *dst_iovec = vec_pool + (VHOST_MAX_ASYNC_VEC >> 1);
+ struct iovec *src_iovec = async->src_iovec;
+ struct iovec *dst_iovec = async->dst_iovec;
struct async_inflight_info *pkts_info = async->pkts_info;
uint32_t n_pkts = 0, pkt_err = 0;
int32_t n_xfer;
- uint16_t segs_await = 0;
uint16_t iovec_idx = 0, it_idx = 0, slot_idx = 0;
/*
@@ -1562,7 +1560,6 @@ virtio_dev_rx_async_submit_split(struct virtio_net *dev,
pkts_info[slot_idx].mbuf = pkts[pkt_idx];
iovec_idx += it_pool[it_idx].nr_segs;
- segs_await += it_pool[it_idx].nr_segs;
it_idx += 2;
vq->last_avail_idx += num_buffers;
@@ -1573,8 +1570,7 @@ virtio_dev_rx_async_submit_split(struct virtio_net *dev,
* - unused async iov number is less than max vhost vector
*/
if (unlikely(pkt_burst_idx >= VHOST_ASYNC_BATCH_THRESHOLD ||
- ((VHOST_MAX_ASYNC_VEC >> 1) - segs_await <
- BUF_VECTOR_MAX))) {
+ (VHOST_MAX_ASYNC_VEC - iovec_idx < BUF_VECTOR_MAX))) {
n_xfer = async->ops.transfer_data(dev->vid,
queue_id, tdes, 0, pkt_burst_idx);
if (likely(n_xfer >= 0)) {
@@ -1588,7 +1584,6 @@ virtio_dev_rx_async_submit_split(struct virtio_net *dev,
iovec_idx = 0;
it_idx = 0;
- segs_await = 0;
if (unlikely(n_pkts < pkt_burst_idx)) {
/*
@@ -1745,8 +1740,11 @@ vhost_enqueue_async_packed(struct virtio_net *dev,
if (unlikely(++tries > max_tries))
return -1;
- if (unlikely(fill_vec_buf_packed(dev, vq, avail_idx, &desc_count, buf_vec, &nr_vec,
- &buf_id, &len, VHOST_ACCESS_RW) < 0))
+ if (unlikely(fill_vec_buf_packed(dev, vq,
+ avail_idx, &desc_count,
+ buf_vec, &nr_vec,
+ &buf_id, &len,
+ VHOST_ACCESS_RW) < 0))
return -1;
len = RTE_MIN(len, size);
@@ -1832,14 +1830,12 @@ virtio_dev_rx_async_submit_packed(struct virtio_net *dev,
struct vhost_async *async = vq->async;
struct rte_vhost_iov_iter *it_pool = async->it_pool;
- struct iovec *vec_pool = async->vec_pool;
struct rte_vhost_async_desc tdes[MAX_PKT_BURST];
- struct iovec *src_iovec = vec_pool;
- struct iovec *dst_iovec = vec_pool + (VHOST_MAX_ASYNC_VEC >> 1);
+ struct iovec *src_iovec = async->src_iovec;
+ struct iovec *dst_iovec = async->dst_iovec;
struct async_inflight_info *pkts_info = async->pkts_info;
uint32_t n_pkts = 0, pkt_err = 0;
uint16_t slot_idx = 0;
- uint16_t segs_await = 0;
uint16_t iovec_idx = 0, it_idx = 0;
do {
@@ -1861,7 +1857,6 @@ virtio_dev_rx_async_submit_packed(struct virtio_net *dev,
pkts_info[slot_idx].nr_buffers = num_buffers;
pkts_info[slot_idx].mbuf = pkts[pkt_idx];
iovec_idx += it_pool[it_idx].nr_segs;
- segs_await += it_pool[it_idx].nr_segs;
it_idx += 2;
pkt_idx++;
@@ -1874,7 +1869,7 @@ virtio_dev_rx_async_submit_packed(struct virtio_net *dev,
* - unused async iov number is less than max vhost vector
*/
if (unlikely(pkt_burst_idx >= VHOST_ASYNC_BATCH_THRESHOLD ||
- ((VHOST_MAX_ASYNC_VEC >> 1) - segs_await < BUF_VECTOR_MAX))) {
+ (VHOST_MAX_ASYNC_VEC - iovec_idx < BUF_VECTOR_MAX))) {
n_xfer = async->ops.transfer_data(dev->vid,
queue_id, tdes, 0, pkt_burst_idx);
if (likely(n_xfer >= 0)) {
@@ -1888,7 +1883,6 @@ virtio_dev_rx_async_submit_packed(struct virtio_net *dev,
iovec_idx = 0;
it_idx = 0;
- segs_await = 0;
if (unlikely(n_pkts < pkt_burst_idx)) {
/*
--
2.31.1
^ permalink raw reply [flat|nested] 21+ messages in thread
* [dpdk-dev] [RFC 04/14] vhost: simplify async IO vectors iterators
2021-10-07 21:59 [dpdk-dev] [RFC 00/14] vhost: clean-up and simplify async implementation Maxime Coquelin
` (2 preceding siblings ...)
2021-10-07 22:00 ` [dpdk-dev] [RFC 03/14] vhost: simplify async IO vectors Maxime Coquelin
@ 2021-10-07 22:00 ` Maxime Coquelin
2021-10-07 22:00 ` [dpdk-dev] [RFC 05/14] vhost: remove async batch threshold Maxime Coquelin
` (11 subsequent siblings)
15 siblings, 0 replies; 21+ messages in thread
From: Maxime Coquelin @ 2021-10-07 22:00 UTC (permalink / raw)
To: dev, chenbo.xia, jiayu.hu, yuanx.wang, wenwux.ma,
bruce.richardson, john.mcnamara
Cc: Maxime Coquelin
This patch splits the iterator arrays in two, one for
source and one for destination. The goal is make the code
easier to understand.
Signed-off-by: Maxime Coquelin <maxime.coquelin@redhat.com>
---
lib/vhost/vhost.h | 5 +++--
lib/vhost/virtio_net.c | 24 ++++++++++++------------
2 files changed, 15 insertions(+), 14 deletions(-)
diff --git a/lib/vhost/vhost.h b/lib/vhost/vhost.h
index f2d9535174..cb4c0f022a 100644
--- a/lib/vhost/vhost.h
+++ b/lib/vhost/vhost.h
@@ -48,7 +48,7 @@
#define MAX_PKT_BURST 32
-#define VHOST_MAX_ASYNC_IT (MAX_PKT_BURST * 2)
+#define VHOST_MAX_ASYNC_IT (MAX_PKT_BURST)
#define VHOST_MAX_ASYNC_VEC (BUF_VECTOR_MAX * 2)
#define PACKED_DESC_ENQUEUE_USED_FLAG(w) \
@@ -132,7 +132,8 @@ struct vhost_async {
/* operation callbacks for DMA */
struct rte_vhost_async_channel_ops ops;
- struct rte_vhost_iov_iter it_pool[VHOST_MAX_ASYNC_IT];
+ struct rte_vhost_iov_iter src_iov_iter[VHOST_MAX_ASYNC_IT];
+ struct rte_vhost_iov_iter dst_iov_iter[VHOST_MAX_ASYNC_IT];
struct iovec src_iovec[VHOST_MAX_ASYNC_VEC];
struct iovec dst_iovec[VHOST_MAX_ASYNC_VEC];
diff --git a/lib/vhost/virtio_net.c b/lib/vhost/virtio_net.c
index 4e0e1584b8..e2a9e138b7 100644
--- a/lib/vhost/virtio_net.c
+++ b/lib/vhost/virtio_net.c
@@ -1511,7 +1511,8 @@ virtio_dev_rx_async_submit_split(struct virtio_net *dev,
uint16_t avail_head;
struct vhost_async *async = vq->async;
- struct rte_vhost_iov_iter *it_pool = async->it_pool;
+ struct rte_vhost_iov_iter *src_iter = async->src_iov_iter;
+ struct rte_vhost_iov_iter *dst_iter = async->dst_iov_iter;
struct rte_vhost_async_desc tdes[MAX_PKT_BURST];
struct iovec *src_iovec = async->src_iovec;
struct iovec *dst_iovec = async->dst_iovec;
@@ -1547,20 +1548,19 @@ virtio_dev_rx_async_submit_split(struct virtio_net *dev,
if (async_mbuf_to_desc(dev, vq, pkts[pkt_idx], buf_vec, nr_vec, num_buffers,
&src_iovec[iovec_idx], &dst_iovec[iovec_idx],
- &it_pool[it_idx], &it_pool[it_idx + 1]) < 0) {
+ &src_iter[it_idx], &dst_iter[it_idx]) < 0) {
vq->shadow_used_idx -= num_buffers;
break;
}
- async_fill_desc(&tdes[pkt_burst_idx++], &it_pool[it_idx],
- &it_pool[it_idx + 1]);
+ async_fill_desc(&tdes[pkt_burst_idx++], &src_iter[it_idx], &dst_iter[it_idx]);
slot_idx = (async->pkts_idx + pkt_idx) & (vq->size - 1);
pkts_info[slot_idx].descs = num_buffers;
pkts_info[slot_idx].mbuf = pkts[pkt_idx];
- iovec_idx += it_pool[it_idx].nr_segs;
- it_idx += 2;
+ iovec_idx += src_iter[it_idx].nr_segs;
+ it_idx++;
vq->last_avail_idx += num_buffers;
@@ -1829,7 +1829,8 @@ virtio_dev_rx_async_submit_packed(struct virtio_net *dev,
uint16_t num_descs;
struct vhost_async *async = vq->async;
- struct rte_vhost_iov_iter *it_pool = async->it_pool;
+ struct rte_vhost_iov_iter *src_iter = async->src_iov_iter;
+ struct rte_vhost_iov_iter *dst_iter = async->dst_iov_iter;
struct rte_vhost_async_desc tdes[MAX_PKT_BURST];
struct iovec *src_iovec = async->src_iovec;
struct iovec *dst_iovec = async->dst_iovec;
@@ -1846,18 +1847,17 @@ virtio_dev_rx_async_submit_packed(struct virtio_net *dev,
if (unlikely(virtio_dev_rx_async_packed(dev, vq, pkts[pkt_idx],
&num_descs, &num_buffers,
&src_iovec[iovec_idx], &dst_iovec[iovec_idx],
- &it_pool[it_idx], &it_pool[it_idx + 1]) < 0))
+ &src_iter[it_idx], &dst_iter[it_idx]) < 0))
break;
slot_idx = (async->pkts_idx + pkt_idx) % vq->size;
- async_fill_desc(&tdes[pkt_burst_idx++], &it_pool[it_idx],
- &it_pool[it_idx + 1]);
+ async_fill_desc(&tdes[pkt_burst_idx++], &src_iter[it_idx], &dst_iter[it_idx]);
pkts_info[slot_idx].descs = num_descs;
pkts_info[slot_idx].nr_buffers = num_buffers;
pkts_info[slot_idx].mbuf = pkts[pkt_idx];
- iovec_idx += it_pool[it_idx].nr_segs;
- it_idx += 2;
+ iovec_idx += src_iter[it_idx].nr_segs;
+ it_idx++;
pkt_idx++;
remained--;
--
2.31.1
^ permalink raw reply [flat|nested] 21+ messages in thread
* [dpdk-dev] [RFC 05/14] vhost: remove async batch threshold
2021-10-07 21:59 [dpdk-dev] [RFC 00/14] vhost: clean-up and simplify async implementation Maxime Coquelin
` (3 preceding siblings ...)
2021-10-07 22:00 ` [dpdk-dev] [RFC 04/14] vhost: simplify async IO vectors iterators Maxime Coquelin
@ 2021-10-07 22:00 ` Maxime Coquelin
2021-10-07 22:00 ` [dpdk-dev] [RFC 06/14] vhost: introduce specific iovec structure Maxime Coquelin
` (10 subsequent siblings)
15 siblings, 0 replies; 21+ messages in thread
From: Maxime Coquelin @ 2021-10-07 22:00 UTC (permalink / raw)
To: dev, chenbo.xia, jiayu.hu, yuanx.wang, wenwux.ma,
bruce.richardson, john.mcnamara
Cc: Maxime Coquelin
Reaching the async batch threshold was one of the condition
to trigger the DMA transfer. However, this condition was
never met since the threshold value is 32, same as the
MAX_PKT_BURST value.
Signed-off-by: Maxime Coquelin <maxime.coquelin@redhat.com>
---
lib/vhost/virtio_net.c | 14 ++++----------
1 file changed, 4 insertions(+), 10 deletions(-)
diff --git a/lib/vhost/virtio_net.c b/lib/vhost/virtio_net.c
index e2a9e138b7..a939481eaf 100644
--- a/lib/vhost/virtio_net.c
+++ b/lib/vhost/virtio_net.c
@@ -25,8 +25,6 @@
#define MAX_BATCH_LEN 256
-#define VHOST_ASYNC_BATCH_THRESHOLD 32
-
static __rte_always_inline bool
rxvq_is_mergeable(struct virtio_net *dev)
{
@@ -1565,12 +1563,10 @@ virtio_dev_rx_async_submit_split(struct virtio_net *dev,
vq->last_avail_idx += num_buffers;
/*
- * conditions to trigger async device transfer:
- * - buffered packet number reaches transfer threshold
+ * condition to trigger async device transfer:
* - unused async iov number is less than max vhost vector
*/
- if (unlikely(pkt_burst_idx >= VHOST_ASYNC_BATCH_THRESHOLD ||
- (VHOST_MAX_ASYNC_VEC - iovec_idx < BUF_VECTOR_MAX))) {
+ if (unlikely(VHOST_MAX_ASYNC_VEC - iovec_idx < BUF_VECTOR_MAX)) {
n_xfer = async->ops.transfer_data(dev->vid,
queue_id, tdes, 0, pkt_burst_idx);
if (likely(n_xfer >= 0)) {
@@ -1864,12 +1860,10 @@ virtio_dev_rx_async_submit_packed(struct virtio_net *dev,
vq_inc_last_avail_packed(vq, num_descs);
/*
- * conditions to trigger async device transfer:
- * - buffered packet number reaches transfer threshold
+ * condition to trigger async device transfer:
* - unused async iov number is less than max vhost vector
*/
- if (unlikely(pkt_burst_idx >= VHOST_ASYNC_BATCH_THRESHOLD ||
- (VHOST_MAX_ASYNC_VEC - iovec_idx < BUF_VECTOR_MAX))) {
+ if (unlikely(VHOST_MAX_ASYNC_VEC - iovec_idx < BUF_VECTOR_MAX)) {
n_xfer = async->ops.transfer_data(dev->vid,
queue_id, tdes, 0, pkt_burst_idx);
if (likely(n_xfer >= 0)) {
--
2.31.1
^ permalink raw reply [flat|nested] 21+ messages in thread
* [dpdk-dev] [RFC 06/14] vhost: introduce specific iovec structure
2021-10-07 21:59 [dpdk-dev] [RFC 00/14] vhost: clean-up and simplify async implementation Maxime Coquelin
` (4 preceding siblings ...)
2021-10-07 22:00 ` [dpdk-dev] [RFC 05/14] vhost: remove async batch threshold Maxime Coquelin
@ 2021-10-07 22:00 ` Maxime Coquelin
2021-10-07 22:00 ` [dpdk-dev] [RFC 07/14] vhost: remove useless fields in async iterator struct Maxime Coquelin
` (9 subsequent siblings)
15 siblings, 0 replies; 21+ messages in thread
From: Maxime Coquelin @ 2021-10-07 22:00 UTC (permalink / raw)
To: dev, chenbo.xia, jiayu.hu, yuanx.wang, wenwux.ma,
bruce.richardson, john.mcnamara
Cc: Maxime Coquelin
This patch introduces rte_vhost_iovec struct that contains
both source and destination addresses since we always have
a 1:1 mapping between source and destination. While using
the standard iovec struct might have seemed better, having
to duplicate IO vectors and its iterators is memory
inefficient and make the implementation more complex.
Signed-off-by: Maxime Coquelin <maxime.coquelin@redhat.com>
---
examples/vhost/ioat.c | 24 ++++++-------
lib/vhost/rte_vhost_async.h | 19 +++++++----
lib/vhost/vhost.h | 6 ++--
lib/vhost/virtio_net.c | 68 ++++++++++++++-----------------------
4 files changed, 52 insertions(+), 65 deletions(-)
diff --git a/examples/vhost/ioat.c b/examples/vhost/ioat.c
index 457f8171f0..dcbcf65e4e 100644
--- a/examples/vhost/ioat.c
+++ b/examples/vhost/ioat.c
@@ -129,33 +129,31 @@ ioat_transfer_data_cb(int vid, uint16_t queue_id,
{
uint32_t i_desc;
uint16_t dev_id = dma_bind[vid].dmas[queue_id * 2 + VIRTIO_RXQ].dev_id;
- struct rte_vhost_iov_iter *src = NULL;
- struct rte_vhost_iov_iter *dst = NULL;
+ struct rte_vhost_iov_iter *iter = NULL;
unsigned long i_seg;
unsigned short mask = MAX_ENQUEUED_SIZE - 1;
unsigned short write = cb_tracker[dev_id].next_write;
if (!opaque_data) {
for (i_desc = 0; i_desc < count; i_desc++) {
- src = descs[i_desc].src;
- dst = descs[i_desc].dst;
+ iter = descs[i_desc].iter;
i_seg = 0;
- if (cb_tracker[dev_id].ioat_space < src->nr_segs)
+ if (cb_tracker[dev_id].ioat_space < iter->nr_segs)
break;
- while (i_seg < src->nr_segs) {
+ while (i_seg < iter->nr_segs) {
rte_ioat_enqueue_copy(dev_id,
- (uintptr_t)(src->iov[i_seg].iov_base)
- + src->offset,
- (uintptr_t)(dst->iov[i_seg].iov_base)
- + dst->offset,
- src->iov[i_seg].iov_len,
+ (uintptr_t)(iter->iov[i_seg].src_addr)
+ + iter->offset,
+ (uintptr_t)(iter->iov[i_seg].dst_addr)
+ + iter->offset,
+ iter->iov[i_seg].len,
0,
0);
i_seg++;
}
write &= mask;
- cb_tracker[dev_id].size_track[write] = src->nr_segs;
- cb_tracker[dev_id].ioat_space -= src->nr_segs;
+ cb_tracker[dev_id].size_track[write] = iter->nr_segs;
+ cb_tracker[dev_id].ioat_space -= iter->nr_segs;
write++;
}
} else {
diff --git a/lib/vhost/rte_vhost_async.h b/lib/vhost/rte_vhost_async.h
index 789b80f5a0..d7bb77bf90 100644
--- a/lib/vhost/rte_vhost_async.h
+++ b/lib/vhost/rte_vhost_async.h
@@ -7,6 +7,15 @@
#include "rte_vhost.h"
+/**
+ * iovec
+ */
+struct rte_vhost_iovec {
+ void *src_addr;
+ void *dst_addr;
+ size_t len;
+};
+
/**
* iovec iterator
*/
@@ -16,19 +25,17 @@ struct rte_vhost_iov_iter {
/** total bytes of data in this iterator */
size_t count;
/** pointer to the iovec array */
- struct iovec *iov;
+ struct rte_vhost_iovec *iov;
/** number of iovec in this iterator */
unsigned long nr_segs;
};
/**
- * dma transfer descriptor pair
+ * dma transfer descriptor
*/
struct rte_vhost_async_desc {
- /** source memory iov_iter */
- struct rte_vhost_iov_iter *src;
- /** destination memory iov_iter */
- struct rte_vhost_iov_iter *dst;
+ /* memory iov_iter */
+ struct rte_vhost_iov_iter *iter;
};
/**
diff --git a/lib/vhost/vhost.h b/lib/vhost/vhost.h
index cb4c0f022a..dae9a1ac2d 100644
--- a/lib/vhost/vhost.h
+++ b/lib/vhost/vhost.h
@@ -132,10 +132,8 @@ struct vhost_async {
/* operation callbacks for DMA */
struct rte_vhost_async_channel_ops ops;
- struct rte_vhost_iov_iter src_iov_iter[VHOST_MAX_ASYNC_IT];
- struct rte_vhost_iov_iter dst_iov_iter[VHOST_MAX_ASYNC_IT];
- struct iovec src_iovec[VHOST_MAX_ASYNC_VEC];
- struct iovec dst_iovec[VHOST_MAX_ASYNC_VEC];
+ struct rte_vhost_iov_iter iov_iter[VHOST_MAX_ASYNC_IT];
+ struct rte_vhost_iovec iovec[VHOST_MAX_ASYNC_VEC];
/* data transfer status */
struct async_inflight_info *pkts_info;
diff --git a/lib/vhost/virtio_net.c b/lib/vhost/virtio_net.c
index a939481eaf..7b1c52ea7d 100644
--- a/lib/vhost/virtio_net.c
+++ b/lib/vhost/virtio_net.c
@@ -925,15 +925,16 @@ copy_mbuf_to_desc(struct virtio_net *dev, struct vhost_virtqueue *vq,
}
static __rte_always_inline void
-async_fill_vec(struct iovec *v, void *base, size_t len)
+async_fill_vec(struct rte_vhost_iovec *v, void *src, void *dst, size_t len)
{
- v->iov_base = base;
- v->iov_len = len;
+ v->src_addr = src;
+ v->dst_addr = dst;
+ v->len = len;
}
static __rte_always_inline void
async_fill_iter(struct rte_vhost_iov_iter *it, size_t count,
- struct iovec *vec, unsigned long nr_seg)
+ struct rte_vhost_iovec *vec, unsigned long nr_seg)
{
it->offset = 0;
it->count = count;
@@ -948,20 +949,16 @@ async_fill_iter(struct rte_vhost_iov_iter *it, size_t count,
}
static __rte_always_inline void
-async_fill_desc(struct rte_vhost_async_desc *desc,
- struct rte_vhost_iov_iter *src, struct rte_vhost_iov_iter *dst)
+async_fill_desc(struct rte_vhost_async_desc *desc, struct rte_vhost_iov_iter *iter)
{
- desc->src = src;
- desc->dst = dst;
+ desc->iter = iter;
}
static __rte_always_inline int
async_mbuf_to_desc(struct virtio_net *dev, struct vhost_virtqueue *vq,
struct rte_mbuf *m, struct buf_vector *buf_vec,
uint16_t nr_vec, uint16_t num_buffers,
- struct iovec *src_iovec, struct iovec *dst_iovec,
- struct rte_vhost_iov_iter *src_it,
- struct rte_vhost_iov_iter *dst_it)
+ struct rte_vhost_iovec *iovec, struct rte_vhost_iov_iter *iter)
{
struct rte_mbuf *hdr_mbuf;
struct virtio_net_hdr_mrg_rxbuf tmp_hdr, *hdr = NULL;
@@ -1075,11 +1072,9 @@ async_mbuf_to_desc(struct virtio_net *dev, struct vhost_virtqueue *vq,
goto out;
}
- async_fill_vec(src_iovec + tvec_idx,
+ async_fill_vec(iovec + tvec_idx,
(void *)(uintptr_t)rte_pktmbuf_iova_offset(m,
- mbuf_offset), (size_t)mapped_len);
- async_fill_vec(dst_iovec + tvec_idx,
- hpa, (size_t)mapped_len);
+ mbuf_offset), hpa, (size_t)mapped_len);
tlen += (uint32_t)mapped_len;
cpy_len -= (uint32_t)mapped_len;
@@ -1091,8 +1086,7 @@ async_mbuf_to_desc(struct virtio_net *dev, struct vhost_virtqueue *vq,
}
}
- async_fill_iter(src_it, tlen, src_iovec, tvec_idx);
- async_fill_iter(dst_it, tlen, dst_iovec, tvec_idx);
+ async_fill_iter(iter, tlen, iovec, tvec_idx);
out:
return error;
}
@@ -1509,11 +1503,9 @@ virtio_dev_rx_async_submit_split(struct virtio_net *dev,
uint16_t avail_head;
struct vhost_async *async = vq->async;
- struct rte_vhost_iov_iter *src_iter = async->src_iov_iter;
- struct rte_vhost_iov_iter *dst_iter = async->dst_iov_iter;
+ struct rte_vhost_iov_iter *iter = async->iov_iter;
struct rte_vhost_async_desc tdes[MAX_PKT_BURST];
- struct iovec *src_iovec = async->src_iovec;
- struct iovec *dst_iovec = async->dst_iovec;
+ struct rte_vhost_iovec *iovec = async->iovec;
struct async_inflight_info *pkts_info = async->pkts_info;
uint32_t n_pkts = 0, pkt_err = 0;
int32_t n_xfer;
@@ -1545,19 +1537,18 @@ virtio_dev_rx_async_submit_split(struct virtio_net *dev,
vq->last_avail_idx + num_buffers);
if (async_mbuf_to_desc(dev, vq, pkts[pkt_idx], buf_vec, nr_vec, num_buffers,
- &src_iovec[iovec_idx], &dst_iovec[iovec_idx],
- &src_iter[it_idx], &dst_iter[it_idx]) < 0) {
+ &iovec[iovec_idx], &iter[it_idx]) < 0) {
vq->shadow_used_idx -= num_buffers;
break;
}
- async_fill_desc(&tdes[pkt_burst_idx++], &src_iter[it_idx], &dst_iter[it_idx]);
+ async_fill_desc(&tdes[pkt_burst_idx++], &iter[it_idx]);
slot_idx = (async->pkts_idx + pkt_idx) & (vq->size - 1);
pkts_info[slot_idx].descs = num_buffers;
pkts_info[slot_idx].mbuf = pkts[pkt_idx];
- iovec_idx += src_iter[it_idx].nr_segs;
+ iovec_idx += iter[it_idx].nr_segs;
it_idx++;
vq->last_avail_idx += num_buffers;
@@ -1707,9 +1698,8 @@ vhost_enqueue_async_packed(struct virtio_net *dev,
struct buf_vector *buf_vec,
uint16_t *nr_descs,
uint16_t *nr_buffers,
- struct iovec *src_iovec, struct iovec *dst_iovec,
- struct rte_vhost_iov_iter *src_it,
- struct rte_vhost_iov_iter *dst_it)
+ struct rte_vhost_iovec *iovec,
+ struct rte_vhost_iov_iter *iter)
{
uint16_t nr_vec = 0;
uint16_t avail_idx = vq->last_avail_idx;
@@ -1757,8 +1747,7 @@ vhost_enqueue_async_packed(struct virtio_net *dev,
}
if (unlikely(async_mbuf_to_desc(dev, vq, pkt, buf_vec, nr_vec,
- *nr_buffers, src_iovec, dst_iovec,
- src_it, dst_it) < 0))
+ *nr_buffers, iovec, iter) < 0))
return -1;
vhost_shadow_enqueue_packed(vq, buffer_len, buffer_buf_id, buffer_desc_count, *nr_buffers);
@@ -1769,14 +1758,12 @@ vhost_enqueue_async_packed(struct virtio_net *dev,
static __rte_always_inline int16_t
virtio_dev_rx_async_packed(struct virtio_net *dev, struct vhost_virtqueue *vq,
struct rte_mbuf *pkt, uint16_t *nr_descs, uint16_t *nr_buffers,
- struct iovec *src_iovec, struct iovec *dst_iovec,
- struct rte_vhost_iov_iter *src_it, struct rte_vhost_iov_iter *dst_it)
+ struct rte_vhost_iovec *iovec, struct rte_vhost_iov_iter *iter)
{
struct buf_vector buf_vec[BUF_VECTOR_MAX];
if (unlikely(vhost_enqueue_async_packed(dev, vq, pkt, buf_vec, nr_descs, nr_buffers,
- src_iovec, dst_iovec,
- src_it, dst_it) < 0)) {
+ iovec, iter) < 0)) {
VHOST_LOG_DATA(DEBUG, "(%d) failed to get enough desc from vring\n", dev->vid);
return -1;
}
@@ -1825,11 +1812,9 @@ virtio_dev_rx_async_submit_packed(struct virtio_net *dev,
uint16_t num_descs;
struct vhost_async *async = vq->async;
- struct rte_vhost_iov_iter *src_iter = async->src_iov_iter;
- struct rte_vhost_iov_iter *dst_iter = async->dst_iov_iter;
+ struct rte_vhost_iov_iter *iter = async->iov_iter;
struct rte_vhost_async_desc tdes[MAX_PKT_BURST];
- struct iovec *src_iovec = async->src_iovec;
- struct iovec *dst_iovec = async->dst_iovec;
+ struct rte_vhost_iovec *iovec = async->iovec;
struct async_inflight_info *pkts_info = async->pkts_info;
uint32_t n_pkts = 0, pkt_err = 0;
uint16_t slot_idx = 0;
@@ -1842,17 +1827,16 @@ virtio_dev_rx_async_submit_packed(struct virtio_net *dev,
num_descs = 0;
if (unlikely(virtio_dev_rx_async_packed(dev, vq, pkts[pkt_idx],
&num_descs, &num_buffers,
- &src_iovec[iovec_idx], &dst_iovec[iovec_idx],
- &src_iter[it_idx], &dst_iter[it_idx]) < 0))
+ &iovec[iovec_idx], &iter[it_idx]) < 0))
break;
slot_idx = (async->pkts_idx + pkt_idx) % vq->size;
- async_fill_desc(&tdes[pkt_burst_idx++], &src_iter[it_idx], &dst_iter[it_idx]);
+ async_fill_desc(&tdes[pkt_burst_idx++], &iter[it_idx]);
pkts_info[slot_idx].descs = num_descs;
pkts_info[slot_idx].nr_buffers = num_buffers;
pkts_info[slot_idx].mbuf = pkts[pkt_idx];
- iovec_idx += src_iter[it_idx].nr_segs;
+ iovec_idx += iter[it_idx].nr_segs;
it_idx++;
pkt_idx++;
--
2.31.1
^ permalink raw reply [flat|nested] 21+ messages in thread
* [dpdk-dev] [RFC 07/14] vhost: remove useless fields in async iterator struct
2021-10-07 21:59 [dpdk-dev] [RFC 00/14] vhost: clean-up and simplify async implementation Maxime Coquelin
` (5 preceding siblings ...)
2021-10-07 22:00 ` [dpdk-dev] [RFC 06/14] vhost: introduce specific iovec structure Maxime Coquelin
@ 2021-10-07 22:00 ` Maxime Coquelin
2021-10-07 22:00 ` [dpdk-dev] [RFC 08/14] vhost: improve IO vector logic Maxime Coquelin
` (8 subsequent siblings)
15 siblings, 0 replies; 21+ messages in thread
From: Maxime Coquelin @ 2021-10-07 22:00 UTC (permalink / raw)
To: dev, chenbo.xia, jiayu.hu, yuanx.wang, wenwux.ma,
bruce.richardson, john.mcnamara
Cc: Maxime Coquelin
Offset and count fields are unused and so can be removed.
The offset field was actually in the Vhost example, but
in a way that does not make sense.
Signed-off-by: Maxime Coquelin <maxime.coquelin@redhat.com>
---
examples/vhost/ioat.c | 6 ++----
lib/vhost/rte_vhost_async.h | 4 ----
lib/vhost/virtio_net.c | 19 ++++---------------
3 files changed, 6 insertions(+), 23 deletions(-)
diff --git a/examples/vhost/ioat.c b/examples/vhost/ioat.c
index dcbcf65e4e..a8c588deff 100644
--- a/examples/vhost/ioat.c
+++ b/examples/vhost/ioat.c
@@ -142,10 +142,8 @@ ioat_transfer_data_cb(int vid, uint16_t queue_id,
break;
while (i_seg < iter->nr_segs) {
rte_ioat_enqueue_copy(dev_id,
- (uintptr_t)(iter->iov[i_seg].src_addr)
- + iter->offset,
- (uintptr_t)(iter->iov[i_seg].dst_addr)
- + iter->offset,
+ (uintptr_t)(iter->iov[i_seg].src_addr),
+ (uintptr_t)(iter->iov[i_seg].dst_addr),
iter->iov[i_seg].len,
0,
0);
diff --git a/lib/vhost/rte_vhost_async.h b/lib/vhost/rte_vhost_async.h
index d7bb77bf90..4ea5cfab10 100644
--- a/lib/vhost/rte_vhost_async.h
+++ b/lib/vhost/rte_vhost_async.h
@@ -20,10 +20,6 @@ struct rte_vhost_iovec {
* iovec iterator
*/
struct rte_vhost_iov_iter {
- /** offset to the first byte of interesting data */
- size_t offset;
- /** total bytes of data in this iterator */
- size_t count;
/** pointer to the iovec array */
struct rte_vhost_iovec *iov;
/** number of iovec in this iterator */
diff --git a/lib/vhost/virtio_net.c b/lib/vhost/virtio_net.c
index 7b1c52ea7d..ae7dded979 100644
--- a/lib/vhost/virtio_net.c
+++ b/lib/vhost/virtio_net.c
@@ -933,19 +933,10 @@ async_fill_vec(struct rte_vhost_iovec *v, void *src, void *dst, size_t len)
}
static __rte_always_inline void
-async_fill_iter(struct rte_vhost_iov_iter *it, size_t count,
- struct rte_vhost_iovec *vec, unsigned long nr_seg)
+async_fill_iter(struct rte_vhost_iov_iter *it, struct rte_vhost_iovec *vec, unsigned long nr_seg)
{
- it->offset = 0;
- it->count = count;
-
- if (count) {
- it->iov = vec;
- it->nr_segs = nr_seg;
- } else {
- it->iov = 0;
- it->nr_segs = 0;
- }
+ it->iov = vec;
+ it->nr_segs = nr_seg;
}
static __rte_always_inline void
@@ -971,7 +962,6 @@ async_mbuf_to_desc(struct virtio_net *dev, struct vhost_virtqueue *vq,
uint32_t cpy_len, buf_len;
int error = 0;
- uint32_t tlen = 0;
int tvec_idx = 0;
void *hpa;
@@ -1076,7 +1066,6 @@ async_mbuf_to_desc(struct virtio_net *dev, struct vhost_virtqueue *vq,
(void *)(uintptr_t)rte_pktmbuf_iova_offset(m,
mbuf_offset), hpa, (size_t)mapped_len);
- tlen += (uint32_t)mapped_len;
cpy_len -= (uint32_t)mapped_len;
mbuf_avail -= (uint32_t)mapped_len;
mbuf_offset += (uint32_t)mapped_len;
@@ -1086,7 +1075,7 @@ async_mbuf_to_desc(struct virtio_net *dev, struct vhost_virtqueue *vq,
}
}
- async_fill_iter(iter, tlen, iovec, tvec_idx);
+ async_fill_iter(iter, iovec, tvec_idx);
out:
return error;
}
--
2.31.1
^ permalink raw reply [flat|nested] 21+ messages in thread
* [dpdk-dev] [RFC 08/14] vhost: improve IO vector logic
2021-10-07 21:59 [dpdk-dev] [RFC 00/14] vhost: clean-up and simplify async implementation Maxime Coquelin
` (6 preceding siblings ...)
2021-10-07 22:00 ` [dpdk-dev] [RFC 07/14] vhost: remove useless fields in async iterator struct Maxime Coquelin
@ 2021-10-07 22:00 ` Maxime Coquelin
2021-10-12 6:05 ` Hu, Jiayu
2021-10-07 22:00 ` [dpdk-dev] [RFC 09/14] vhost: remove notion of async descriptor Maxime Coquelin
` (7 subsequent siblings)
15 siblings, 1 reply; 21+ messages in thread
From: Maxime Coquelin @ 2021-10-07 22:00 UTC (permalink / raw)
To: dev, chenbo.xia, jiayu.hu, yuanx.wang, wenwux.ma,
bruce.richardson, john.mcnamara
Cc: Maxime Coquelin
IO vectors and their iterators arrays were part of the
async metadata but not their indexes.
In order to makes this more consistent, the patch adds the
indexes to the async metadata. Doing that, we can avoid
triggering DMA transfer within the loop as it IO vector
index overflow is now prevented in the async_mbuf_to_desc()
function.
Note that previous detection mechanism was broken
since the overflow already happened when detected, so OOB
memory access would already have happened.
With this changes done, virtio_dev_rx_async_submit_split()
and virtio_dev_rx_async_submit_packed() can be further
simplified.
Signed-off-by: Maxime Coquelin <maxime.coquelin@redhat.com>
---
lib/vhost/vhost.h | 2 +
lib/vhost/virtio_net.c | 296 +++++++++++++++++++----------------------
2 files changed, 136 insertions(+), 162 deletions(-)
diff --git a/lib/vhost/vhost.h b/lib/vhost/vhost.h
index dae9a1ac2d..812d4c55a5 100644
--- a/lib/vhost/vhost.h
+++ b/lib/vhost/vhost.h
@@ -134,6 +134,8 @@ struct vhost_async {
struct rte_vhost_iov_iter iov_iter[VHOST_MAX_ASYNC_IT];
struct rte_vhost_iovec iovec[VHOST_MAX_ASYNC_VEC];
+ uint16_t iter_idx;
+ uint16_t iovec_idx;
/* data transfer status */
struct async_inflight_info *pkts_info;
diff --git a/lib/vhost/virtio_net.c b/lib/vhost/virtio_net.c
index ae7dded979..5ce4c14a73 100644
--- a/lib/vhost/virtio_net.c
+++ b/lib/vhost/virtio_net.c
@@ -924,33 +924,91 @@ copy_mbuf_to_desc(struct virtio_net *dev, struct vhost_virtqueue *vq,
return error;
}
+static __rte_always_inline int
+async_iter_initialize(struct vhost_async *async)
+{
+ struct rte_vhost_iov_iter *iter;
+
+ if (unlikely(async->iter_idx >= VHOST_MAX_ASYNC_IT)) {
+ VHOST_LOG_DATA(ERR, "no more async iterators available\n");
+ return -1;
+ }
+
+ if (unlikely(async->iovec_idx >= VHOST_MAX_ASYNC_VEC)) {
+ VHOST_LOG_DATA(ERR, "no more async iovec available\n");
+ return -1;
+ }
+
+
+ iter = async->iov_iter + async->iter_idx;
+ iter->iov = async->iovec + async->iovec_idx;
+ iter->nr_segs = 0;
+
+ return 0;
+}
+
+static __rte_always_inline int
+async_iter_add_iovec(struct vhost_async *async, void *src, void *dst, size_t len)
+{
+ struct rte_vhost_iov_iter *iter;
+ struct rte_vhost_iovec *iovec;
+
+ if (unlikely(async->iovec_idx >= VHOST_MAX_ASYNC_VEC)) {
+ VHOST_LOG_DATA(ERR, "no more async iovec available\n");
+ return -1;
+ }
+
+ iter = async->iov_iter + async->iter_idx;
+ iovec = async->iovec + async->iovec_idx;
+
+ iovec->src_addr = src;
+ iovec->dst_addr = dst;
+ iovec->len = len;
+
+ iter->nr_segs++;
+
+ return 0;
+}
+
static __rte_always_inline void
-async_fill_vec(struct rte_vhost_iovec *v, void *src, void *dst, size_t len)
+async_iter_finalize(struct vhost_async *async)
{
- v->src_addr = src;
- v->dst_addr = dst;
- v->len = len;
+ async->iter_idx++;
}
static __rte_always_inline void
-async_fill_iter(struct rte_vhost_iov_iter *it, struct rte_vhost_iovec *vec, unsigned long nr_seg)
+async_iter_cancel(struct vhost_async *async)
{
- it->iov = vec;
- it->nr_segs = nr_seg;
+ struct rte_vhost_iov_iter *iter;
+
+ iter = async->iov_iter + async->iter_idx;
+ async->iovec_idx -= iter->nr_segs;
+ iter->nr_segs = 0;
+ iter->iov = NULL;
}
static __rte_always_inline void
-async_fill_desc(struct rte_vhost_async_desc *desc, struct rte_vhost_iov_iter *iter)
+async_iter_reset(struct vhost_async *async)
{
- desc->iter = iter;
+ async->iter_idx = 0;
+ async->iovec_idx = 0;
+}
+
+static __rte_always_inline void
+async_fill_descs(struct vhost_async *async, struct rte_vhost_async_desc *descs)
+{
+ int i;
+
+ for (i = 0; i < async->iter_idx; i++)
+ descs[i].iter = async->iov_iter + i;
}
static __rte_always_inline int
async_mbuf_to_desc(struct virtio_net *dev, struct vhost_virtqueue *vq,
struct rte_mbuf *m, struct buf_vector *buf_vec,
- uint16_t nr_vec, uint16_t num_buffers,
- struct rte_vhost_iovec *iovec, struct rte_vhost_iov_iter *iter)
+ uint16_t nr_vec, uint16_t num_buffers)
{
+ struct vhost_async *async = vq->async;
struct rte_mbuf *hdr_mbuf;
struct virtio_net_hdr_mrg_rxbuf tmp_hdr, *hdr = NULL;
uint64_t buf_addr, buf_iova;
@@ -960,24 +1018,18 @@ async_mbuf_to_desc(struct virtio_net *dev, struct vhost_virtqueue *vq,
uint32_t mbuf_offset, mbuf_avail;
uint32_t buf_offset, buf_avail;
uint32_t cpy_len, buf_len;
- int error = 0;
- int tvec_idx = 0;
void *hpa;
- if (unlikely(m == NULL)) {
- error = -1;
- goto out;
- }
+ if (unlikely(m == NULL))
+ return -1;
buf_addr = buf_vec[vec_idx].buf_addr;
buf_iova = buf_vec[vec_idx].buf_iova;
buf_len = buf_vec[vec_idx].buf_len;
- if (unlikely(buf_len < dev->vhost_hlen && nr_vec <= 1)) {
- error = -1;
- goto out;
- }
+ if (unlikely(buf_len < dev->vhost_hlen && nr_vec <= 1))
+ return -1;
hdr_mbuf = m;
hdr_addr = buf_addr;
@@ -1005,14 +1057,15 @@ async_mbuf_to_desc(struct virtio_net *dev, struct vhost_virtqueue *vq,
mbuf_avail = rte_pktmbuf_data_len(m);
mbuf_offset = 0;
+ if (async_iter_initialize(async))
+ return -1;
+
while (mbuf_avail != 0 || m->next != NULL) {
/* done with current buf, get the next one */
if (buf_avail == 0) {
vec_idx++;
- if (unlikely(vec_idx >= nr_vec)) {
- error = -1;
- goto out;
- }
+ if (unlikely(vec_idx >= nr_vec))
+ goto error;
buf_addr = buf_vec[vec_idx].buf_addr;
buf_iova = buf_vec[vec_idx].buf_iova;
@@ -1058,26 +1111,30 @@ async_mbuf_to_desc(struct virtio_net *dev, struct vhost_virtqueue *vq,
if (unlikely(!hpa)) {
VHOST_LOG_DATA(ERR, "(%d) %s: failed to get hpa.\n",
dev->vid, __func__);
- error = -1;
- goto out;
+ goto error;
}
- async_fill_vec(iovec + tvec_idx,
- (void *)(uintptr_t)rte_pktmbuf_iova_offset(m,
- mbuf_offset), hpa, (size_t)mapped_len);
+ if (unlikely(async_iter_add_iovec(async,
+ (void *)(uintptr_t)rte_pktmbuf_iova_offset(m,
+ mbuf_offset),
+ hpa, (size_t)mapped_len)))
+ goto error;
cpy_len -= (uint32_t)mapped_len;
mbuf_avail -= (uint32_t)mapped_len;
mbuf_offset += (uint32_t)mapped_len;
buf_avail -= (uint32_t)mapped_len;
buf_offset += (uint32_t)mapped_len;
- tvec_idx++;
}
}
- async_fill_iter(iter, iovec, tvec_idx);
-out:
- return error;
+ async_iter_finalize(async);
+
+ return 0;
+error:
+ async_iter_cancel(async);
+
+ return -1;
}
static __rte_always_inline int
@@ -1487,18 +1544,16 @@ virtio_dev_rx_async_submit_split(struct virtio_net *dev,
struct rte_mbuf **pkts, uint32_t count)
{
struct buf_vector buf_vec[BUF_VECTOR_MAX];
- uint32_t pkt_idx = 0, pkt_burst_idx = 0;
+ uint32_t pkt_idx = 0;
uint16_t num_buffers;
uint16_t avail_head;
struct vhost_async *async = vq->async;
- struct rte_vhost_iov_iter *iter = async->iov_iter;
- struct rte_vhost_async_desc tdes[MAX_PKT_BURST];
- struct rte_vhost_iovec *iovec = async->iovec;
+ struct rte_vhost_async_desc async_descs[MAX_PKT_BURST];
struct async_inflight_info *pkts_info = async->pkts_info;
- uint32_t n_pkts = 0, pkt_err = 0;
+ uint32_t pkt_err = 0;
int32_t n_xfer;
- uint16_t iovec_idx = 0, it_idx = 0, slot_idx = 0;
+ uint16_t slot_idx = 0;
/*
* The ordering between avail index and desc reads need to be enforced.
@@ -1507,95 +1562,53 @@ virtio_dev_rx_async_submit_split(struct virtio_net *dev,
rte_prefetch0(&vq->avail->ring[vq->last_avail_idx & (vq->size - 1)]);
+ async_iter_reset(async);
+
for (pkt_idx = 0; pkt_idx < count; pkt_idx++) {
uint32_t pkt_len = pkts[pkt_idx]->pkt_len + dev->vhost_hlen;
uint16_t nr_vec = 0;
- if (unlikely(reserve_avail_buf_split(dev, vq,
- pkt_len, buf_vec, &num_buffers,
- avail_head, &nr_vec) < 0)) {
- VHOST_LOG_DATA(DEBUG,
- "(%d) failed to get enough desc from vring\n",
- dev->vid);
+ if (unlikely(reserve_avail_buf_split(dev, vq, pkt_len, buf_vec,
+ &num_buffers, avail_head, &nr_vec) < 0)) {
+ VHOST_LOG_DATA(DEBUG, "(%d) failed to get enough desc from vring\n",
+ dev->vid);
vq->shadow_used_idx -= num_buffers;
break;
}
VHOST_LOG_DATA(DEBUG, "(%d) current index %d | end index %d\n",
- dev->vid, vq->last_avail_idx,
- vq->last_avail_idx + num_buffers);
+ dev->vid, vq->last_avail_idx, vq->last_avail_idx + num_buffers);
- if (async_mbuf_to_desc(dev, vq, pkts[pkt_idx], buf_vec, nr_vec, num_buffers,
- &iovec[iovec_idx], &iter[it_idx]) < 0) {
+ if (async_mbuf_to_desc(dev, vq, pkts[pkt_idx], buf_vec, nr_vec, num_buffers) < 0) {
vq->shadow_used_idx -= num_buffers;
break;
}
- async_fill_desc(&tdes[pkt_burst_idx++], &iter[it_idx]);
-
slot_idx = (async->pkts_idx + pkt_idx) & (vq->size - 1);
pkts_info[slot_idx].descs = num_buffers;
pkts_info[slot_idx].mbuf = pkts[pkt_idx];
- iovec_idx += iter[it_idx].nr_segs;
- it_idx++;
-
vq->last_avail_idx += num_buffers;
+ }
- /*
- * condition to trigger async device transfer:
- * - unused async iov number is less than max vhost vector
- */
- if (unlikely(VHOST_MAX_ASYNC_VEC - iovec_idx < BUF_VECTOR_MAX)) {
- n_xfer = async->ops.transfer_data(dev->vid,
- queue_id, tdes, 0, pkt_burst_idx);
- if (likely(n_xfer >= 0)) {
- n_pkts = n_xfer;
- } else {
- VHOST_LOG_DATA(ERR,
- "(%d) %s: failed to transfer data for queue id %d.\n",
- dev->vid, __func__, queue_id);
- n_pkts = 0;
- }
-
- iovec_idx = 0;
- it_idx = 0;
-
- if (unlikely(n_pkts < pkt_burst_idx)) {
- /*
- * log error packets number here and do actual
- * error processing when applications poll
- * completion
- */
- pkt_err = pkt_burst_idx - n_pkts;
- pkt_idx++;
- pkt_burst_idx = 0;
- break;
- }
+ if (unlikely(pkt_idx == 0))
+ return 0;
- pkt_burst_idx = 0;
- }
- }
+ async_fill_descs(async, async_descs);
- if (pkt_burst_idx) {
- n_xfer = async->ops.transfer_data(dev->vid, queue_id, tdes, 0, pkt_burst_idx);
- if (likely(n_xfer >= 0)) {
- n_pkts = n_xfer;
- } else {
- VHOST_LOG_DATA(ERR, "(%d) %s: failed to transfer data for queue id %d.\n",
+ n_xfer = async->ops.transfer_data(dev->vid, queue_id, async_descs, 0, pkt_idx);
+ if (unlikely(n_xfer < 0)) {
+ VHOST_LOG_DATA(ERR, "(%d) %s: failed to transfer data for queue id %d.\n",
dev->vid, __func__, queue_id);
- n_pkts = 0;
- }
-
- if (unlikely(n_pkts < pkt_burst_idx))
- pkt_err = pkt_burst_idx - n_pkts;
+ n_xfer = 0;
}
+ pkt_err = pkt_idx - n_xfer;
if (unlikely(pkt_err)) {
uint16_t num_descs = 0;
/* update number of completed packets */
- pkt_idx -= pkt_err;
+ pkt_idx = n_xfer;
/* calculate the sum of descriptors to revert */
while (pkt_err-- > 0) {
@@ -1686,9 +1699,7 @@ vhost_enqueue_async_packed(struct virtio_net *dev,
struct rte_mbuf *pkt,
struct buf_vector *buf_vec,
uint16_t *nr_descs,
- uint16_t *nr_buffers,
- struct rte_vhost_iovec *iovec,
- struct rte_vhost_iov_iter *iter)
+ uint16_t *nr_buffers)
{
uint16_t nr_vec = 0;
uint16_t avail_idx = vq->last_avail_idx;
@@ -1736,7 +1747,7 @@ vhost_enqueue_async_packed(struct virtio_net *dev,
}
if (unlikely(async_mbuf_to_desc(dev, vq, pkt, buf_vec, nr_vec,
- *nr_buffers, iovec, iter) < 0))
+ *nr_buffers) < 0))
return -1;
vhost_shadow_enqueue_packed(vq, buffer_len, buffer_buf_id, buffer_desc_count, *nr_buffers);
@@ -1746,13 +1757,12 @@ vhost_enqueue_async_packed(struct virtio_net *dev,
static __rte_always_inline int16_t
virtio_dev_rx_async_packed(struct virtio_net *dev, struct vhost_virtqueue *vq,
- struct rte_mbuf *pkt, uint16_t *nr_descs, uint16_t *nr_buffers,
- struct rte_vhost_iovec *iovec, struct rte_vhost_iov_iter *iter)
+ struct rte_mbuf *pkt, uint16_t *nr_descs, uint16_t *nr_buffers)
{
struct buf_vector buf_vec[BUF_VECTOR_MAX];
- if (unlikely(vhost_enqueue_async_packed(dev, vq, pkt, buf_vec, nr_descs, nr_buffers,
- iovec, iter) < 0)) {
+ if (unlikely(vhost_enqueue_async_packed(dev, vq, pkt, buf_vec,
+ nr_descs, nr_buffers) < 0)) {
VHOST_LOG_DATA(DEBUG, "(%d) failed to get enough desc from vring\n", dev->vid);
return -1;
}
@@ -1794,20 +1804,17 @@ virtio_dev_rx_async_submit_packed(struct virtio_net *dev,
struct vhost_virtqueue *vq, uint16_t queue_id,
struct rte_mbuf **pkts, uint32_t count)
{
- uint32_t pkt_idx = 0, pkt_burst_idx = 0;
+ uint32_t pkt_idx = 0;
uint32_t remained = count;
int32_t n_xfer;
uint16_t num_buffers;
uint16_t num_descs;
struct vhost_async *async = vq->async;
- struct rte_vhost_iov_iter *iter = async->iov_iter;
- struct rte_vhost_async_desc tdes[MAX_PKT_BURST];
- struct rte_vhost_iovec *iovec = async->iovec;
+ struct rte_vhost_async_desc async_descs[MAX_PKT_BURST];
struct async_inflight_info *pkts_info = async->pkts_info;
- uint32_t n_pkts = 0, pkt_err = 0;
+ uint32_t pkt_err = 0;
uint16_t slot_idx = 0;
- uint16_t iovec_idx = 0, it_idx = 0;
do {
rte_prefetch0(&vq->desc_packed[vq->last_avail_idx]);
@@ -1815,71 +1822,36 @@ virtio_dev_rx_async_submit_packed(struct virtio_net *dev,
num_buffers = 0;
num_descs = 0;
if (unlikely(virtio_dev_rx_async_packed(dev, vq, pkts[pkt_idx],
- &num_descs, &num_buffers,
- &iovec[iovec_idx], &iter[it_idx]) < 0))
+ &num_descs, &num_buffers) < 0))
break;
slot_idx = (async->pkts_idx + pkt_idx) % vq->size;
- async_fill_desc(&tdes[pkt_burst_idx++], &iter[it_idx]);
pkts_info[slot_idx].descs = num_descs;
pkts_info[slot_idx].nr_buffers = num_buffers;
pkts_info[slot_idx].mbuf = pkts[pkt_idx];
- iovec_idx += iter[it_idx].nr_segs;
- it_idx++;
pkt_idx++;
remained--;
vq_inc_last_avail_packed(vq, num_descs);
+ } while (pkt_idx < count);
- /*
- * condition to trigger async device transfer:
- * - unused async iov number is less than max vhost vector
- */
- if (unlikely(VHOST_MAX_ASYNC_VEC - iovec_idx < BUF_VECTOR_MAX)) {
- n_xfer = async->ops.transfer_data(dev->vid,
- queue_id, tdes, 0, pkt_burst_idx);
- if (likely(n_xfer >= 0)) {
- n_pkts = n_xfer;
- } else {
- VHOST_LOG_DATA(ERR,
- "(%d) %s: failed to transfer data for queue id %d.\n",
- dev->vid, __func__, queue_id);
- n_pkts = 0;
- }
-
- iovec_idx = 0;
- it_idx = 0;
-
- if (unlikely(n_pkts < pkt_burst_idx)) {
- /*
- * log error packets number here and do actual
- * error processing when applications poll
- * completion
- */
- pkt_err = pkt_burst_idx - n_pkts;
- pkt_burst_idx = 0;
- break;
- }
+ if (unlikely(pkt_idx == 0))
+ return 0;
- pkt_burst_idx = 0;
- }
- } while (pkt_idx < count);
+ async_fill_descs(async, async_descs);
- if (pkt_burst_idx) {
- n_xfer = async->ops.transfer_data(dev->vid, queue_id, tdes, 0, pkt_burst_idx);
- if (likely(n_xfer >= 0)) {
- n_pkts = n_xfer;
- } else {
- VHOST_LOG_DATA(ERR, "(%d) %s: failed to transfer data for queue id %d.\n",
+ n_xfer = async->ops.transfer_data(dev->vid, queue_id, async_descs, 0, pkt_idx);
+ if (unlikely(n_xfer < 0)) {
+ VHOST_LOG_DATA(ERR, "(%d) %s: failed to transfer data for queue id %d.\n",
dev->vid, __func__, queue_id);
- n_pkts = 0;
- }
-
- if (unlikely(n_pkts < pkt_burst_idx))
- pkt_err = pkt_burst_idx - n_pkts;
+ n_xfer = 0;
}
+ pkt_err = pkt_idx - n_xfer;
+
+ async_iter_reset(async);
+
if (unlikely(pkt_err))
dma_error_handler_packed(vq, slot_idx, pkt_err, &pkt_idx);
--
2.31.1
^ permalink raw reply [flat|nested] 21+ messages in thread
* [dpdk-dev] [RFC 09/14] vhost: remove notion of async descriptor
2021-10-07 21:59 [dpdk-dev] [RFC 00/14] vhost: clean-up and simplify async implementation Maxime Coquelin
` (7 preceding siblings ...)
2021-10-07 22:00 ` [dpdk-dev] [RFC 08/14] vhost: improve IO vector logic Maxime Coquelin
@ 2021-10-07 22:00 ` Maxime Coquelin
2021-10-07 22:00 ` [dpdk-dev] [RFC 10/14] vhost: simplify async enqueue completion Maxime Coquelin
` (6 subsequent siblings)
15 siblings, 0 replies; 21+ messages in thread
From: Maxime Coquelin @ 2021-10-07 22:00 UTC (permalink / raw)
To: dev, chenbo.xia, jiayu.hu, yuanx.wang, wenwux.ma,
bruce.richardson, john.mcnamara
Cc: Maxime Coquelin
Now that IO vectors iterator have been simplified, the
rte_vhost_async_desc struct only contains a pointer on
the iterator array stored in the async metadata.
This patch removes it, and pass directly the iterators
array pointer to the transfer_data callback. Doing that,
we avoid declaring the descriptor array in the stack, and
also avoid the cost of filling it.
Signed-off-by: Maxime Coquelin <maxime.coquelin@redhat.com>
---
examples/vhost/ioat.c | 10 +++++-----
examples/vhost/ioat.h | 2 +-
lib/vhost/rte_vhost_async.h | 16 ++++------------
lib/vhost/virtio_net.c | 19 ++-----------------
4 files changed, 12 insertions(+), 35 deletions(-)
diff --git a/examples/vhost/ioat.c b/examples/vhost/ioat.c
index a8c588deff..9aeeb12fd9 100644
--- a/examples/vhost/ioat.c
+++ b/examples/vhost/ioat.c
@@ -124,10 +124,10 @@ open_ioat(const char *value)
int32_t
ioat_transfer_data_cb(int vid, uint16_t queue_id,
- struct rte_vhost_async_desc *descs,
+ struct rte_vhost_iov_iter *iov_iter,
struct rte_vhost_async_status *opaque_data, uint16_t count)
{
- uint32_t i_desc;
+ uint32_t i_iter;
uint16_t dev_id = dma_bind[vid].dmas[queue_id * 2 + VIRTIO_RXQ].dev_id;
struct rte_vhost_iov_iter *iter = NULL;
unsigned long i_seg;
@@ -135,8 +135,8 @@ ioat_transfer_data_cb(int vid, uint16_t queue_id,
unsigned short write = cb_tracker[dev_id].next_write;
if (!opaque_data) {
- for (i_desc = 0; i_desc < count; i_desc++) {
- iter = descs[i_desc].iter;
+ for (i_iter = 0; i_iter < count; i_iter++) {
+ iter = iov_iter + i_iter;
i_seg = 0;
if (cb_tracker[dev_id].ioat_space < iter->nr_segs)
break;
@@ -161,7 +161,7 @@ ioat_transfer_data_cb(int vid, uint16_t queue_id,
/* ring the doorbell */
rte_ioat_perform_ops(dev_id);
cb_tracker[dev_id].next_write = write;
- return i_desc;
+ return i_iter;
}
int32_t
diff --git a/examples/vhost/ioat.h b/examples/vhost/ioat.h
index 62e163c585..a4f09ee39b 100644
--- a/examples/vhost/ioat.h
+++ b/examples/vhost/ioat.h
@@ -29,7 +29,7 @@ int open_ioat(const char *value);
int32_t
ioat_transfer_data_cb(int vid, uint16_t queue_id,
- struct rte_vhost_async_desc *descs,
+ struct rte_vhost_iov_iter *iov_iter,
struct rte_vhost_async_status *opaque_data, uint16_t count);
int32_t
diff --git a/lib/vhost/rte_vhost_async.h b/lib/vhost/rte_vhost_async.h
index 4ea5cfab10..a87ea6ba37 100644
--- a/lib/vhost/rte_vhost_async.h
+++ b/lib/vhost/rte_vhost_async.h
@@ -26,14 +26,6 @@ struct rte_vhost_iov_iter {
unsigned long nr_segs;
};
-/**
- * dma transfer descriptor
- */
-struct rte_vhost_async_desc {
- /* memory iov_iter */
- struct rte_vhost_iov_iter *iter;
-};
-
/**
* dma transfer status
*/
@@ -55,17 +47,17 @@ struct rte_vhost_async_channel_ops {
* id of vhost device to perform data copies
* @param queue_id
* queue id to perform data copies
- * @param descs
- * an array of DMA transfer memory descriptors
+ * @param iov_iter
+ * an array of IOV iterators
* @param opaque_data
* opaque data pair sending to DMA engine
* @param count
* number of elements in the "descs" array
* @return
- * number of descs processed, negative value means error
+ * number of IOV iterators processed, negative value means error
*/
int32_t (*transfer_data)(int vid, uint16_t queue_id,
- struct rte_vhost_async_desc *descs,
+ struct rte_vhost_iov_iter *iov_iter,
struct rte_vhost_async_status *opaque_data,
uint16_t count);
/**
diff --git a/lib/vhost/virtio_net.c b/lib/vhost/virtio_net.c
index 5ce4c14a73..b295dc1d39 100644
--- a/lib/vhost/virtio_net.c
+++ b/lib/vhost/virtio_net.c
@@ -994,15 +994,6 @@ async_iter_reset(struct vhost_async *async)
async->iovec_idx = 0;
}
-static __rte_always_inline void
-async_fill_descs(struct vhost_async *async, struct rte_vhost_async_desc *descs)
-{
- int i;
-
- for (i = 0; i < async->iter_idx; i++)
- descs[i].iter = async->iov_iter + i;
-}
-
static __rte_always_inline int
async_mbuf_to_desc(struct virtio_net *dev, struct vhost_virtqueue *vq,
struct rte_mbuf *m, struct buf_vector *buf_vec,
@@ -1549,7 +1540,6 @@ virtio_dev_rx_async_submit_split(struct virtio_net *dev,
uint16_t avail_head;
struct vhost_async *async = vq->async;
- struct rte_vhost_async_desc async_descs[MAX_PKT_BURST];
struct async_inflight_info *pkts_info = async->pkts_info;
uint32_t pkt_err = 0;
int32_t n_xfer;
@@ -1594,9 +1584,7 @@ virtio_dev_rx_async_submit_split(struct virtio_net *dev,
if (unlikely(pkt_idx == 0))
return 0;
- async_fill_descs(async, async_descs);
-
- n_xfer = async->ops.transfer_data(dev->vid, queue_id, async_descs, 0, pkt_idx);
+ n_xfer = async->ops.transfer_data(dev->vid, queue_id, async->iov_iter, 0, pkt_idx);
if (unlikely(n_xfer < 0)) {
VHOST_LOG_DATA(ERR, "(%d) %s: failed to transfer data for queue id %d.\n",
dev->vid, __func__, queue_id);
@@ -1811,7 +1799,6 @@ virtio_dev_rx_async_submit_packed(struct virtio_net *dev,
uint16_t num_descs;
struct vhost_async *async = vq->async;
- struct rte_vhost_async_desc async_descs[MAX_PKT_BURST];
struct async_inflight_info *pkts_info = async->pkts_info;
uint32_t pkt_err = 0;
uint16_t slot_idx = 0;
@@ -1839,9 +1826,7 @@ virtio_dev_rx_async_submit_packed(struct virtio_net *dev,
if (unlikely(pkt_idx == 0))
return 0;
- async_fill_descs(async, async_descs);
-
- n_xfer = async->ops.transfer_data(dev->vid, queue_id, async_descs, 0, pkt_idx);
+ n_xfer = async->ops.transfer_data(dev->vid, queue_id, async->iov_iter, 0, pkt_idx);
if (unlikely(n_xfer < 0)) {
VHOST_LOG_DATA(ERR, "(%d) %s: failed to transfer data for queue id %d.\n",
dev->vid, __func__, queue_id);
--
2.31.1
^ permalink raw reply [flat|nested] 21+ messages in thread
* [dpdk-dev] [RFC 10/14] vhost: simplify async enqueue completion
2021-10-07 21:59 [dpdk-dev] [RFC 00/14] vhost: clean-up and simplify async implementation Maxime Coquelin
` (8 preceding siblings ...)
2021-10-07 22:00 ` [dpdk-dev] [RFC 09/14] vhost: remove notion of async descriptor Maxime Coquelin
@ 2021-10-07 22:00 ` Maxime Coquelin
2021-10-07 22:00 ` [dpdk-dev] [RFC 11/14] vhost: simplify getting the first in-flight index Maxime Coquelin
` (5 subsequent siblings)
15 siblings, 0 replies; 21+ messages in thread
From: Maxime Coquelin @ 2021-10-07 22:00 UTC (permalink / raw)
To: dev, chenbo.xia, jiayu.hu, yuanx.wang, wenwux.ma,
bruce.richardson, john.mcnamara
Cc: Maxime Coquelin
vhost_poll_enqueue_completed() assumes some inflight
packets could have been completed in a previous call but
not returned to the application. But this is not the case,
since check_completed_copies callback is never called with
more than the current count as argument.
In other words, async->last_pkts_n is always 0. Removing it
greatly simplfies the function.
Signed-off-by: Maxime Coquelin <maxime.coquelin@redhat.com>
---
lib/vhost/vhost.h | 1 -
lib/vhost/virtio_net.c | 76 ++++++++++++++++--------------------------
2 files changed, 28 insertions(+), 49 deletions(-)
diff --git a/lib/vhost/vhost.h b/lib/vhost/vhost.h
index 812d4c55a5..1a2cd21a1d 100644
--- a/lib/vhost/vhost.h
+++ b/lib/vhost/vhost.h
@@ -141,7 +141,6 @@ struct vhost_async {
struct async_inflight_info *pkts_info;
uint16_t pkts_idx;
uint16_t pkts_inflight_n;
- uint16_t last_pkts_n;
union {
struct vring_used_elem *descs_split;
struct vring_used_elem_packed *buffers_packed;
diff --git a/lib/vhost/virtio_net.c b/lib/vhost/virtio_net.c
index b295dc1d39..c5651f1e0f 100644
--- a/lib/vhost/virtio_net.c
+++ b/lib/vhost/virtio_net.c
@@ -1618,7 +1618,11 @@ virtio_dev_rx_async_submit_split(struct virtio_net *dev,
vq->shadow_used_idx);
async->desc_idx_split += vq->shadow_used_idx;
+
async->pkts_idx += pkt_idx;
+ if (async->pkts_idx >= vq->size)
+ async->pkts_idx -= vq->size;
+
async->pkts_inflight_n += pkt_idx;
vq->shadow_used_idx = 0;
}
@@ -1920,68 +1924,44 @@ static __rte_always_inline uint16_t
vhost_poll_enqueue_completed(struct virtio_net *dev, uint16_t queue_id,
struct rte_mbuf **pkts, uint16_t count)
{
- struct vhost_virtqueue *vq;
- struct vhost_async *async;
- struct async_inflight_info *pkts_info;
+ struct vhost_virtqueue *vq = dev->virtqueue[queue_id];
+ struct vhost_async *async = vq->async;
+ struct async_inflight_info *pkts_info = async->pkts_info;
int32_t n_cpl;
- uint16_t n_pkts_cpl = 0, n_pkts_put = 0, n_descs = 0, n_buffers = 0;
- uint16_t start_idx, pkts_idx, vq_size;
- uint16_t from, i;
+ uint16_t n_descs = 0, n_buffers = 0;
+ uint16_t start_idx, from, i;
- vq = dev->virtqueue[queue_id];
- async = vq->async;
- pkts_idx = async->pkts_idx % vq->size;
- pkts_info = async->pkts_info;
- vq_size = vq->size;
- start_idx = virtio_dev_rx_async_get_info_idx(pkts_idx,
- vq_size, async->pkts_inflight_n);
-
- if (count > async->last_pkts_n) {
- n_cpl = async->ops.check_completed_copies(dev->vid,
- queue_id, 0, count - async->last_pkts_n);
- if (likely(n_cpl >= 0)) {
- n_pkts_cpl = n_cpl;
- } else {
- VHOST_LOG_DATA(ERR,
- "(%d) %s: failed to check completed copies for queue id %d.\n",
+ start_idx = virtio_dev_rx_async_get_info_idx(async->pkts_idx,
+ vq->size, async->pkts_inflight_n);
+
+ n_cpl = async->ops.check_completed_copies(dev->vid, queue_id, 0, count);
+ if (unlikely(n_cpl < 0)) {
+ VHOST_LOG_DATA(ERR, "(%d) %s: failed to check completed copies for queue id %d.\n",
dev->vid, __func__, queue_id);
- n_pkts_cpl = 0;
- }
+ return 0;
}
- n_pkts_cpl += async->last_pkts_n;
- n_pkts_put = RTE_MIN(n_pkts_cpl, count);
- if (unlikely(n_pkts_put == 0)) {
- async->last_pkts_n = n_pkts_cpl;
+ if (n_cpl == 0)
return 0;
- }
- if (vq_is_packed(dev)) {
- for (i = 0; i < n_pkts_put; i++) {
- from = (start_idx + i) % vq_size;
- n_buffers += pkts_info[from].nr_buffers;
- pkts[i] = pkts_info[from].mbuf;
- }
- } else {
- for (i = 0; i < n_pkts_put; i++) {
- from = (start_idx + i) & (vq_size - 1);
- n_descs += pkts_info[from].descs;
- pkts[i] = pkts_info[from].mbuf;
- }
+ for (i = 0; i < n_cpl; i++) {
+ from = (start_idx + i) % vq->size;
+ /* Only used with packed ring */
+ n_buffers += pkts_info[from].nr_buffers;
+ /* Only used with split ring */
+ n_descs += pkts_info[from].descs;
+ pkts[i] = pkts_info[from].mbuf;
}
- async->last_pkts_n = n_pkts_cpl - n_pkts_put;
- async->pkts_inflight_n -= n_pkts_put;
+
+ async->pkts_inflight_n -= n_cpl;
if (likely(vq->enabled && vq->access_ok)) {
if (vq_is_packed(dev)) {
write_back_completed_descs_packed(vq, n_buffers);
-
vhost_vring_call_packed(dev, vq);
} else {
write_back_completed_descs_split(vq, n_descs);
-
- __atomic_add_fetch(&vq->used->idx, n_descs,
- __ATOMIC_RELEASE);
+ __atomic_add_fetch(&vq->used->idx, n_descs, __ATOMIC_RELEASE);
vhost_vring_call_split(dev, vq);
}
} else {
@@ -1994,7 +1974,7 @@ vhost_poll_enqueue_completed(struct virtio_net *dev, uint16_t queue_id,
}
}
- return n_pkts_put;
+ return n_cpl;
}
uint16_t
--
2.31.1
^ permalink raw reply [flat|nested] 21+ messages in thread
* [dpdk-dev] [RFC 11/14] vhost: simplify getting the first in-flight index
2021-10-07 21:59 [dpdk-dev] [RFC 00/14] vhost: clean-up and simplify async implementation Maxime Coquelin
` (9 preceding siblings ...)
2021-10-07 22:00 ` [dpdk-dev] [RFC 10/14] vhost: simplify async enqueue completion Maxime Coquelin
@ 2021-10-07 22:00 ` Maxime Coquelin
2021-10-07 22:00 ` [dpdk-dev] [RFC 12/14] vhost: prepare async for mbuf to desc refactoring Maxime Coquelin
` (4 subsequent siblings)
15 siblings, 0 replies; 21+ messages in thread
From: Maxime Coquelin @ 2021-10-07 22:00 UTC (permalink / raw)
To: dev, chenbo.xia, jiayu.hu, yuanx.wang, wenwux.ma,
bruce.richardson, john.mcnamara
Cc: Maxime Coquelin
This patch reworks the function getting the index
for the first packet in-flight.
When this index turns out to be zero, let's use the simple
path. Doing that avoid having to do a modulo with the
virtqueue size.
The patch also rename the function for better clarifty,
and only pass the virtqueue metadata pointer, as all the
needed information are stored there.
Signed-off-by: Maxime Coquelin <maxime.coquelin@redhat.com>
---
lib/vhost/virtio_net.c | 16 +++++++++-------
1 file changed, 9 insertions(+), 7 deletions(-)
diff --git a/lib/vhost/virtio_net.c b/lib/vhost/virtio_net.c
index c5651f1e0f..2aa06a958c 100644
--- a/lib/vhost/virtio_net.c
+++ b/lib/vhost/virtio_net.c
@@ -1489,11 +1489,14 @@ rte_vhost_enqueue_burst(int vid, uint16_t queue_id,
}
static __rte_always_inline uint16_t
-virtio_dev_rx_async_get_info_idx(uint16_t pkts_idx,
- uint16_t vq_size, uint16_t n_inflight)
+async_get_first_inflight_pkt_idx(struct vhost_virtqueue *vq)
{
- return pkts_idx > n_inflight ? (pkts_idx - n_inflight) :
- (vq_size - n_inflight + pkts_idx) % vq_size;
+ struct vhost_async *async = vq->async;
+
+ if (async->pkts_idx >= async->pkts_inflight_n)
+ return async->pkts_idx - async->pkts_inflight_n;
+ else
+ return vq->size - async->pkts_inflight_n + async->pkts_idx;
}
static __rte_always_inline void
@@ -1931,9 +1934,6 @@ vhost_poll_enqueue_completed(struct virtio_net *dev, uint16_t queue_id,
uint16_t n_descs = 0, n_buffers = 0;
uint16_t start_idx, from, i;
- start_idx = virtio_dev_rx_async_get_info_idx(async->pkts_idx,
- vq->size, async->pkts_inflight_n);
-
n_cpl = async->ops.check_completed_copies(dev->vid, queue_id, 0, count);
if (unlikely(n_cpl < 0)) {
VHOST_LOG_DATA(ERR, "(%d) %s: failed to check completed copies for queue id %d.\n",
@@ -1944,6 +1944,8 @@ vhost_poll_enqueue_completed(struct virtio_net *dev, uint16_t queue_id,
if (n_cpl == 0)
return 0;
+ start_idx = async_get_first_inflight_pkt_idx(vq);
+
for (i = 0; i < n_cpl; i++) {
from = (start_idx + i) % vq->size;
/* Only used with packed ring */
--
2.31.1
^ permalink raw reply [flat|nested] 21+ messages in thread
* [dpdk-dev] [RFC 12/14] vhost: prepare async for mbuf to desc refactoring
2021-10-07 21:59 [dpdk-dev] [RFC 00/14] vhost: clean-up and simplify async implementation Maxime Coquelin
` (10 preceding siblings ...)
2021-10-07 22:00 ` [dpdk-dev] [RFC 11/14] vhost: simplify getting the first in-flight index Maxime Coquelin
@ 2021-10-07 22:00 ` Maxime Coquelin
2021-10-07 22:00 ` [dpdk-dev] [RFC 13/14] vhost: prepare sync " Maxime Coquelin
` (3 subsequent siblings)
15 siblings, 0 replies; 21+ messages in thread
From: Maxime Coquelin @ 2021-10-07 22:00 UTC (permalink / raw)
To: dev, chenbo.xia, jiayu.hu, yuanx.wang, wenwux.ma,
bruce.richardson, john.mcnamara
Cc: Maxime Coquelin
This patch extracts the IO vectors filling from
async_mbuf_to_desc() into a dedicated function as a
preliminary step of merging copy_mubf_to_desc() and
async_mbuf_to_desc().
Signed-off-by: Maxime Coquelin <maxime.coquelin@redhat.com>
---
lib/vhost/virtio_net.c | 205 ++++++++++++++++++++++-------------------
1 file changed, 111 insertions(+), 94 deletions(-)
diff --git a/lib/vhost/virtio_net.c b/lib/vhost/virtio_net.c
index 2aa06a958c..9d131e6df1 100644
--- a/lib/vhost/virtio_net.c
+++ b/lib/vhost/virtio_net.c
@@ -791,6 +791,109 @@ copy_vnet_hdr_to_desc(struct virtio_net *dev, struct vhost_virtqueue *vq,
}
}
+
+static __rte_always_inline int
+async_iter_initialize(struct vhost_async *async)
+{
+ struct rte_vhost_iov_iter *iter;
+
+ if (unlikely(async->iter_idx >= VHOST_MAX_ASYNC_IT)) {
+ VHOST_LOG_DATA(ERR, "no more async iterators available\n");
+ return -1;
+ }
+
+ if (unlikely(async->iovec_idx >= VHOST_MAX_ASYNC_VEC)) {
+ VHOST_LOG_DATA(ERR, "no more async iovec available\n");
+ return -1;
+ }
+
+
+ iter = async->iov_iter + async->iter_idx;
+ iter->iov = async->iovec + async->iovec_idx;
+ iter->nr_segs = 0;
+
+ return 0;
+}
+
+static __rte_always_inline int
+async_iter_add_iovec(struct vhost_async *async, void *src, void *dst, size_t len)
+{
+ struct rte_vhost_iov_iter *iter;
+ struct rte_vhost_iovec *iovec;
+
+ if (unlikely(async->iovec_idx >= VHOST_MAX_ASYNC_VEC)) {
+ VHOST_LOG_DATA(ERR, "no more async iovec available\n");
+ return -1;
+ }
+
+ iter = async->iov_iter + async->iter_idx;
+ iovec = async->iovec + async->iovec_idx;
+
+ iovec->src_addr = src;
+ iovec->dst_addr = dst;
+ iovec->len = len;
+
+ iter->nr_segs++;
+
+ return 0;
+}
+
+static __rte_always_inline void
+async_iter_finalize(struct vhost_async *async)
+{
+ async->iter_idx++;
+}
+
+static __rte_always_inline void
+async_iter_cancel(struct vhost_async *async)
+{
+ struct rte_vhost_iov_iter *iter;
+
+ iter = async->iov_iter + async->iter_idx;
+ async->iovec_idx -= iter->nr_segs;
+ iter->nr_segs = 0;
+ iter->iov = NULL;
+}
+
+static __rte_always_inline void
+async_iter_reset(struct vhost_async *async)
+{
+ async->iter_idx = 0;
+ async->iovec_idx = 0;
+}
+
+static __rte_always_inline int
+async_mbuf_to_desc_seg(struct virtio_net *dev, struct vhost_virtqueue *vq,
+ struct rte_mbuf *m, uint32_t mbuf_offset,
+ uint64_t buf_iova, uint32_t cpy_len)
+{
+ struct vhost_async *async = vq->async;
+ uint64_t mapped_len;
+ uint32_t buf_offset = 0;
+ void *hpa;
+
+ while (cpy_len) {
+ hpa = (void *)(uintptr_t)gpa_to_first_hpa(dev,
+ buf_iova + buf_offset, cpy_len, &mapped_len);
+ if (unlikely(!hpa)) {
+ VHOST_LOG_DATA(ERR, "(%d) %s: failed to get hpa.\n", dev->vid, __func__);
+ return -1;
+ }
+
+ if (unlikely(async_iter_add_iovec(async,
+ (void *)(uintptr_t)rte_pktmbuf_iova_offset(m,
+ mbuf_offset),
+ hpa, (size_t)mapped_len)))
+ return -1;
+
+ cpy_len -= (uint32_t)mapped_len;
+ mbuf_offset += (uint32_t)mapped_len;
+ buf_offset += (uint32_t)mapped_len;
+ }
+
+ return 0;
+}
+
static __rte_always_inline int
copy_mbuf_to_desc(struct virtio_net *dev, struct vhost_virtqueue *vq,
struct rte_mbuf *m, struct buf_vector *buf_vec,
@@ -924,76 +1027,6 @@ copy_mbuf_to_desc(struct virtio_net *dev, struct vhost_virtqueue *vq,
return error;
}
-static __rte_always_inline int
-async_iter_initialize(struct vhost_async *async)
-{
- struct rte_vhost_iov_iter *iter;
-
- if (unlikely(async->iter_idx >= VHOST_MAX_ASYNC_IT)) {
- VHOST_LOG_DATA(ERR, "no more async iterators available\n");
- return -1;
- }
-
- if (unlikely(async->iovec_idx >= VHOST_MAX_ASYNC_VEC)) {
- VHOST_LOG_DATA(ERR, "no more async iovec available\n");
- return -1;
- }
-
-
- iter = async->iov_iter + async->iter_idx;
- iter->iov = async->iovec + async->iovec_idx;
- iter->nr_segs = 0;
-
- return 0;
-}
-
-static __rte_always_inline int
-async_iter_add_iovec(struct vhost_async *async, void *src, void *dst, size_t len)
-{
- struct rte_vhost_iov_iter *iter;
- struct rte_vhost_iovec *iovec;
-
- if (unlikely(async->iovec_idx >= VHOST_MAX_ASYNC_VEC)) {
- VHOST_LOG_DATA(ERR, "no more async iovec available\n");
- return -1;
- }
-
- iter = async->iov_iter + async->iter_idx;
- iovec = async->iovec + async->iovec_idx;
-
- iovec->src_addr = src;
- iovec->dst_addr = dst;
- iovec->len = len;
-
- iter->nr_segs++;
-
- return 0;
-}
-
-static __rte_always_inline void
-async_iter_finalize(struct vhost_async *async)
-{
- async->iter_idx++;
-}
-
-static __rte_always_inline void
-async_iter_cancel(struct vhost_async *async)
-{
- struct rte_vhost_iov_iter *iter;
-
- iter = async->iov_iter + async->iter_idx;
- async->iovec_idx -= iter->nr_segs;
- iter->nr_segs = 0;
- iter->iov = NULL;
-}
-
-static __rte_always_inline void
-async_iter_reset(struct vhost_async *async)
-{
- async->iter_idx = 0;
- async->iovec_idx = 0;
-}
-
static __rte_always_inline int
async_mbuf_to_desc(struct virtio_net *dev, struct vhost_virtqueue *vq,
struct rte_mbuf *m, struct buf_vector *buf_vec,
@@ -1004,14 +1037,11 @@ async_mbuf_to_desc(struct virtio_net *dev, struct vhost_virtqueue *vq,
struct virtio_net_hdr_mrg_rxbuf tmp_hdr, *hdr = NULL;
uint64_t buf_addr, buf_iova;
uint64_t hdr_addr;
- uint64_t mapped_len;
uint32_t vec_idx = 0;
uint32_t mbuf_offset, mbuf_avail;
uint32_t buf_offset, buf_avail;
uint32_t cpy_len, buf_len;
- void *hpa;
-
if (unlikely(m == NULL))
return -1;
@@ -1095,28 +1125,15 @@ async_mbuf_to_desc(struct virtio_net *dev, struct vhost_virtqueue *vq,
cpy_len = RTE_MIN(buf_avail, mbuf_avail);
- while (unlikely(cpy_len)) {
- hpa = (void *)(uintptr_t)gpa_to_first_hpa(dev,
- buf_iova + buf_offset,
- cpy_len, &mapped_len);
- if (unlikely(!hpa)) {
- VHOST_LOG_DATA(ERR, "(%d) %s: failed to get hpa.\n",
- dev->vid, __func__);
- goto error;
- }
-
- if (unlikely(async_iter_add_iovec(async,
- (void *)(uintptr_t)rte_pktmbuf_iova_offset(m,
- mbuf_offset),
- hpa, (size_t)mapped_len)))
- goto error;
-
- cpy_len -= (uint32_t)mapped_len;
- mbuf_avail -= (uint32_t)mapped_len;
- mbuf_offset += (uint32_t)mapped_len;
- buf_avail -= (uint32_t)mapped_len;
- buf_offset += (uint32_t)mapped_len;
+ if (async_mbuf_to_desc_seg(dev, vq, m, mbuf_offset,
+ buf_iova + buf_offset, cpy_len) < 0) {
+ goto error;
}
+
+ mbuf_avail -= cpy_len;
+ mbuf_offset += cpy_len;
+ buf_avail -= cpy_len;
+ buf_offset += cpy_len;
}
async_iter_finalize(async);
--
2.31.1
^ permalink raw reply [flat|nested] 21+ messages in thread
* [dpdk-dev] [RFC 13/14] vhost: prepare sync for mbuf to desc refactoring
2021-10-07 21:59 [dpdk-dev] [RFC 00/14] vhost: clean-up and simplify async implementation Maxime Coquelin
` (11 preceding siblings ...)
2021-10-07 22:00 ` [dpdk-dev] [RFC 12/14] vhost: prepare async for mbuf to desc refactoring Maxime Coquelin
@ 2021-10-07 22:00 ` Maxime Coquelin
2021-10-07 22:00 ` [dpdk-dev] [RFC 14/14] vhost: merge sync and async mbuf to desc filling Maxime Coquelin
` (2 subsequent siblings)
15 siblings, 0 replies; 21+ messages in thread
From: Maxime Coquelin @ 2021-10-07 22:00 UTC (permalink / raw)
To: dev, chenbo.xia, jiayu.hu, yuanx.wang, wenwux.ma,
bruce.richardson, john.mcnamara
Cc: Maxime Coquelin
This patch extracts the descriptors buffers filling
from copy_mbuf_to_desc() into a dedicated function as a
preliminary step of merging copy_mubf_to_desc() and
async_mbuf_to_desc().
Signed-off-by: Maxime Coquelin <maxime.coquelin@redhat.com>
---
lib/vhost/virtio_net.c | 73 +++++++++++++++++++++---------------------
1 file changed, 36 insertions(+), 37 deletions(-)
diff --git a/lib/vhost/virtio_net.c b/lib/vhost/virtio_net.c
index 9d131e6df1..868ac77dff 100644
--- a/lib/vhost/virtio_net.c
+++ b/lib/vhost/virtio_net.c
@@ -894,6 +894,30 @@ async_mbuf_to_desc_seg(struct virtio_net *dev, struct vhost_virtqueue *vq,
return 0;
}
+static __rte_always_inline void
+sync_mbuf_to_desc_seg(struct virtio_net *dev, struct vhost_virtqueue *vq,
+ struct rte_mbuf *m, uint32_t mbuf_offset,
+ uint64_t buf_addr, uint64_t buf_iova, uint32_t cpy_len)
+{
+ struct batch_copy_elem *batch_copy = vq->batch_copy_elems;
+
+ if (likely(cpy_len > MAX_BATCH_LEN || vq->batch_copy_nb_elems >= vq->size)) {
+ rte_memcpy((void *)((uintptr_t)(buf_addr)),
+ rte_pktmbuf_mtod_offset(m, void *, mbuf_offset),
+ cpy_len);
+ vhost_log_cache_write_iova(dev, vq, buf_iova, cpy_len);
+ PRINT_PACKET(dev, (uintptr_t)(buf_addr), cpy_len, 0);
+ } else {
+ batch_copy[vq->batch_copy_nb_elems].dst =
+ (void *)((uintptr_t)(buf_addr));
+ batch_copy[vq->batch_copy_nb_elems].src =
+ rte_pktmbuf_mtod_offset(m, void *, mbuf_offset);
+ batch_copy[vq->batch_copy_nb_elems].log_addr = buf_iova;
+ batch_copy[vq->batch_copy_nb_elems].len = cpy_len;
+ vq->batch_copy_nb_elems++;
+ }
+}
+
static __rte_always_inline int
copy_mbuf_to_desc(struct virtio_net *dev, struct vhost_virtqueue *vq,
struct rte_mbuf *m, struct buf_vector *buf_vec,
@@ -906,23 +930,17 @@ copy_mbuf_to_desc(struct virtio_net *dev, struct vhost_virtqueue *vq,
uint32_t cpy_len;
uint64_t hdr_addr;
struct rte_mbuf *hdr_mbuf;
- struct batch_copy_elem *batch_copy = vq->batch_copy_elems;
struct virtio_net_hdr_mrg_rxbuf tmp_hdr, *hdr = NULL;
- int error = 0;
- if (unlikely(m == NULL)) {
- error = -1;
- goto out;
- }
+ if (unlikely(m == NULL))
+ return -1;
buf_addr = buf_vec[vec_idx].buf_addr;
buf_iova = buf_vec[vec_idx].buf_iova;
buf_len = buf_vec[vec_idx].buf_len;
- if (unlikely(buf_len < dev->vhost_hlen && nr_vec <= 1)) {
- error = -1;
- goto out;
- }
+ if (unlikely(buf_len < dev->vhost_hlen && nr_vec <= 1))
+ return -1;
hdr_mbuf = m;
hdr_addr = buf_addr;
@@ -953,10 +971,8 @@ copy_mbuf_to_desc(struct virtio_net *dev, struct vhost_virtqueue *vq,
/* done with current buf, get the next one */
if (buf_avail == 0) {
vec_idx++;
- if (unlikely(vec_idx >= nr_vec)) {
- error = -1;
- goto out;
- }
+ if (unlikely(vec_idx >= nr_vec))
+ goto error;
buf_addr = buf_vec[vec_idx].buf_addr;
buf_iova = buf_vec[vec_idx].buf_iova;
@@ -995,26 +1011,9 @@ copy_mbuf_to_desc(struct virtio_net *dev, struct vhost_virtqueue *vq,
cpy_len = RTE_MIN(buf_avail, mbuf_avail);
- if (likely(cpy_len > MAX_BATCH_LEN ||
- vq->batch_copy_nb_elems >= vq->size)) {
- rte_memcpy((void *)((uintptr_t)(buf_addr + buf_offset)),
- rte_pktmbuf_mtod_offset(m, void *, mbuf_offset),
- cpy_len);
- vhost_log_cache_write_iova(dev, vq,
- buf_iova + buf_offset,
- cpy_len);
- PRINT_PACKET(dev, (uintptr_t)(buf_addr + buf_offset),
- cpy_len, 0);
- } else {
- batch_copy[vq->batch_copy_nb_elems].dst =
- (void *)((uintptr_t)(buf_addr + buf_offset));
- batch_copy[vq->batch_copy_nb_elems].src =
- rte_pktmbuf_mtod_offset(m, void *, mbuf_offset);
- batch_copy[vq->batch_copy_nb_elems].log_addr =
- buf_iova + buf_offset;
- batch_copy[vq->batch_copy_nb_elems].len = cpy_len;
- vq->batch_copy_nb_elems++;
- }
+ sync_mbuf_to_desc_seg(dev, vq, m, mbuf_offset,
+ buf_addr + buf_offset,
+ buf_iova + buf_offset, cpy_len);
mbuf_avail -= cpy_len;
mbuf_offset += cpy_len;
@@ -1022,9 +1021,9 @@ copy_mbuf_to_desc(struct virtio_net *dev, struct vhost_virtqueue *vq,
buf_offset += cpy_len;
}
-out:
-
- return error;
+ return 0;
+error:
+ return -1;
}
static __rte_always_inline int
--
2.31.1
^ permalink raw reply [flat|nested] 21+ messages in thread
* [dpdk-dev] [RFC 14/14] vhost: merge sync and async mbuf to desc filling
2021-10-07 21:59 [dpdk-dev] [RFC 00/14] vhost: clean-up and simplify async implementation Maxime Coquelin
` (12 preceding siblings ...)
2021-10-07 22:00 ` [dpdk-dev] [RFC 13/14] vhost: prepare sync " Maxime Coquelin
@ 2021-10-07 22:00 ` Maxime Coquelin
2021-10-08 12:36 ` [dpdk-dev] [RFC 00/14] vhost: clean-up and simplify async implementation David Marchand
2021-10-12 6:24 ` Hu, Jiayu
15 siblings, 0 replies; 21+ messages in thread
From: Maxime Coquelin @ 2021-10-07 22:00 UTC (permalink / raw)
To: dev, chenbo.xia, jiayu.hu, yuanx.wang, wenwux.ma,
bruce.richardson, john.mcnamara
Cc: Maxime Coquelin
This patches merges copy_mbuf_to_desc() used by the sync
path with async_mbuf_to_desc() used by the async path.
Most of these complex functions are identical, so merging
them will make the maintenance easier.
In order not to degrade performance, the patch introduces
a boolean function parameter to specify whether it is called
in async context. This boolean is statically passed to this
always-inlined function, so the compiler will optimize this
out.
Signed-off-by: Maxime Coquelin <maxime.coquelin@redhat.com>
---
lib/vhost/virtio_net.c | 153 +++++++----------------------------------
1 file changed, 26 insertions(+), 127 deletions(-)
diff --git a/lib/vhost/virtio_net.c b/lib/vhost/virtio_net.c
index 868ac77dff..6ef2e1352d 100644
--- a/lib/vhost/virtio_net.c
+++ b/lib/vhost/virtio_net.c
@@ -919,9 +919,9 @@ sync_mbuf_to_desc_seg(struct virtio_net *dev, struct vhost_virtqueue *vq,
}
static __rte_always_inline int
-copy_mbuf_to_desc(struct virtio_net *dev, struct vhost_virtqueue *vq,
- struct rte_mbuf *m, struct buf_vector *buf_vec,
- uint16_t nr_vec, uint16_t num_buffers)
+mbuf_to_desc(struct virtio_net *dev, struct vhost_virtqueue *vq,
+ struct rte_mbuf *m, struct buf_vector *buf_vec,
+ uint16_t nr_vec, uint16_t num_buffers, bool is_async)
{
uint32_t vec_idx = 0;
uint32_t mbuf_offset, mbuf_avail;
@@ -931,115 +931,7 @@ copy_mbuf_to_desc(struct virtio_net *dev, struct vhost_virtqueue *vq,
uint64_t hdr_addr;
struct rte_mbuf *hdr_mbuf;
struct virtio_net_hdr_mrg_rxbuf tmp_hdr, *hdr = NULL;
-
- if (unlikely(m == NULL))
- return -1;
-
- buf_addr = buf_vec[vec_idx].buf_addr;
- buf_iova = buf_vec[vec_idx].buf_iova;
- buf_len = buf_vec[vec_idx].buf_len;
-
- if (unlikely(buf_len < dev->vhost_hlen && nr_vec <= 1))
- return -1;
-
- hdr_mbuf = m;
- hdr_addr = buf_addr;
- if (unlikely(buf_len < dev->vhost_hlen)) {
- memset(&tmp_hdr, 0, sizeof(struct virtio_net_hdr_mrg_rxbuf));
- hdr = &tmp_hdr;
- } else
- hdr = (struct virtio_net_hdr_mrg_rxbuf *)(uintptr_t)hdr_addr;
-
- VHOST_LOG_DATA(DEBUG, "(%d) RX: num merge buffers %d\n",
- dev->vid, num_buffers);
-
- if (unlikely(buf_len < dev->vhost_hlen)) {
- buf_offset = dev->vhost_hlen - buf_len;
- vec_idx++;
- buf_addr = buf_vec[vec_idx].buf_addr;
- buf_iova = buf_vec[vec_idx].buf_iova;
- buf_len = buf_vec[vec_idx].buf_len;
- buf_avail = buf_len - buf_offset;
- } else {
- buf_offset = dev->vhost_hlen;
- buf_avail = buf_len - dev->vhost_hlen;
- }
-
- mbuf_avail = rte_pktmbuf_data_len(m);
- mbuf_offset = 0;
- while (mbuf_avail != 0 || m->next != NULL) {
- /* done with current buf, get the next one */
- if (buf_avail == 0) {
- vec_idx++;
- if (unlikely(vec_idx >= nr_vec))
- goto error;
-
- buf_addr = buf_vec[vec_idx].buf_addr;
- buf_iova = buf_vec[vec_idx].buf_iova;
- buf_len = buf_vec[vec_idx].buf_len;
-
- buf_offset = 0;
- buf_avail = buf_len;
- }
-
- /* done with current mbuf, get the next one */
- if (mbuf_avail == 0) {
- m = m->next;
-
- mbuf_offset = 0;
- mbuf_avail = rte_pktmbuf_data_len(m);
- }
-
- if (hdr_addr) {
- virtio_enqueue_offload(hdr_mbuf, &hdr->hdr);
- if (rxvq_is_mergeable(dev))
- ASSIGN_UNLESS_EQUAL(hdr->num_buffers,
- num_buffers);
-
- if (unlikely(hdr == &tmp_hdr)) {
- copy_vnet_hdr_to_desc(dev, vq, buf_vec, hdr);
- } else {
- PRINT_PACKET(dev, (uintptr_t)hdr_addr,
- dev->vhost_hlen, 0);
- vhost_log_cache_write_iova(dev, vq,
- buf_vec[0].buf_iova,
- dev->vhost_hlen);
- }
-
- hdr_addr = 0;
- }
-
- cpy_len = RTE_MIN(buf_avail, mbuf_avail);
-
- sync_mbuf_to_desc_seg(dev, vq, m, mbuf_offset,
- buf_addr + buf_offset,
- buf_iova + buf_offset, cpy_len);
-
- mbuf_avail -= cpy_len;
- mbuf_offset += cpy_len;
- buf_avail -= cpy_len;
- buf_offset += cpy_len;
- }
-
- return 0;
-error:
- return -1;
-}
-
-static __rte_always_inline int
-async_mbuf_to_desc(struct virtio_net *dev, struct vhost_virtqueue *vq,
- struct rte_mbuf *m, struct buf_vector *buf_vec,
- uint16_t nr_vec, uint16_t num_buffers)
-{
struct vhost_async *async = vq->async;
- struct rte_mbuf *hdr_mbuf;
- struct virtio_net_hdr_mrg_rxbuf tmp_hdr, *hdr = NULL;
- uint64_t buf_addr, buf_iova;
- uint64_t hdr_addr;
- uint32_t vec_idx = 0;
- uint32_t mbuf_offset, mbuf_avail;
- uint32_t buf_offset, buf_avail;
- uint32_t cpy_len, buf_len;
if (unlikely(m == NULL))
return -1;
@@ -1077,8 +969,10 @@ async_mbuf_to_desc(struct virtio_net *dev, struct vhost_virtqueue *vq,
mbuf_avail = rte_pktmbuf_data_len(m);
mbuf_offset = 0;
- if (async_iter_initialize(async))
- return -1;
+ if (is_async) {
+ if (async_iter_initialize(async))
+ return -1;
+ }
while (mbuf_avail != 0 || m->next != NULL) {
/* done with current buf, get the next one */
@@ -1092,7 +986,7 @@ async_mbuf_to_desc(struct virtio_net *dev, struct vhost_virtqueue *vq,
buf_len = buf_vec[vec_idx].buf_len;
buf_offset = 0;
- buf_avail = buf_len;
+ buf_avail = buf_len;
}
/* done with current mbuf, get the next one */
@@ -1100,7 +994,7 @@ async_mbuf_to_desc(struct virtio_net *dev, struct vhost_virtqueue *vq,
m = m->next;
mbuf_offset = 0;
- mbuf_avail = rte_pktmbuf_data_len(m);
+ mbuf_avail = rte_pktmbuf_data_len(m);
}
if (hdr_addr) {
@@ -1124,9 +1018,14 @@ async_mbuf_to_desc(struct virtio_net *dev, struct vhost_virtqueue *vq,
cpy_len = RTE_MIN(buf_avail, mbuf_avail);
- if (async_mbuf_to_desc_seg(dev, vq, m, mbuf_offset,
- buf_iova + buf_offset, cpy_len) < 0) {
- goto error;
+ if (is_async) {
+ if (async_mbuf_to_desc_seg(dev, vq, m, mbuf_offset,
+ buf_iova + buf_offset, cpy_len) < 0)
+ goto error;
+ } else {
+ sync_mbuf_to_desc_seg(dev, vq, m, mbuf_offset,
+ buf_addr + buf_offset,
+ buf_iova + buf_offset, cpy_len);
}
mbuf_avail -= cpy_len;
@@ -1135,11 +1034,13 @@ async_mbuf_to_desc(struct virtio_net *dev, struct vhost_virtqueue *vq,
buf_offset += cpy_len;
}
- async_iter_finalize(async);
+ if (is_async)
+ async_iter_finalize(async);
return 0;
error:
- async_iter_cancel(async);
+ if (is_async)
+ async_iter_cancel(async);
return -1;
}
@@ -1198,7 +1099,7 @@ vhost_enqueue_single_packed(struct virtio_net *dev,
avail_idx -= vq->size;
}
- if (copy_mbuf_to_desc(dev, vq, pkt, buf_vec, nr_vec, num_buffers) < 0)
+ if (mbuf_to_desc(dev, vq, pkt, buf_vec, nr_vec, num_buffers, false) < 0)
return -1;
vhost_shadow_enqueue_single_packed(dev, vq, buffer_len, buffer_buf_id,
@@ -1242,9 +1143,8 @@ virtio_dev_rx_split(struct virtio_net *dev, struct vhost_virtqueue *vq,
dev->vid, vq->last_avail_idx,
vq->last_avail_idx + num_buffers);
- if (copy_mbuf_to_desc(dev, vq, pkts[pkt_idx],
- buf_vec, nr_vec,
- num_buffers) < 0) {
+ if (mbuf_to_desc(dev, vq, pkts[pkt_idx], buf_vec, nr_vec,
+ num_buffers, false) < 0) {
vq->shadow_used_idx -= num_buffers;
break;
}
@@ -1588,7 +1488,7 @@ virtio_dev_rx_async_submit_split(struct virtio_net *dev,
VHOST_LOG_DATA(DEBUG, "(%d) current index %d | end index %d\n",
dev->vid, vq->last_avail_idx, vq->last_avail_idx + num_buffers);
- if (async_mbuf_to_desc(dev, vq, pkts[pkt_idx], buf_vec, nr_vec, num_buffers) < 0) {
+ if (mbuf_to_desc(dev, vq, pkts[pkt_idx], buf_vec, nr_vec, num_buffers, true) < 0) {
vq->shadow_used_idx -= num_buffers;
break;
}
@@ -1757,8 +1657,7 @@ vhost_enqueue_async_packed(struct virtio_net *dev,
avail_idx -= vq->size;
}
- if (unlikely(async_mbuf_to_desc(dev, vq, pkt, buf_vec, nr_vec,
- *nr_buffers) < 0))
+ if (unlikely(mbuf_to_desc(dev, vq, pkt, buf_vec, nr_vec, *nr_buffers, true) < 0))
return -1;
vhost_shadow_enqueue_packed(vq, buffer_len, buffer_buf_id, buffer_desc_count, *nr_buffers);
--
2.31.1
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [dpdk-dev] [RFC 00/14] vhost: clean-up and simplify async implementation
2021-10-07 21:59 [dpdk-dev] [RFC 00/14] vhost: clean-up and simplify async implementation Maxime Coquelin
` (13 preceding siblings ...)
2021-10-07 22:00 ` [dpdk-dev] [RFC 14/14] vhost: merge sync and async mbuf to desc filling Maxime Coquelin
@ 2021-10-08 12:36 ` David Marchand
2021-10-12 6:24 ` Hu, Jiayu
15 siblings, 0 replies; 21+ messages in thread
From: David Marchand @ 2021-10-08 12:36 UTC (permalink / raw)
To: Maxime Coquelin
Cc: dev, Xia, Chenbo, Jiayu Hu, Wang, YuanX, Ma, WenwuX,
Bruce Richardson, Mcnamara, John
On Fri, Oct 8, 2021 at 12:13 AM Maxime Coquelin
<maxime.coquelin@redhat.com> wrote:
>
> This series aims at cleaning and simplifying the async
> enqueue path. I think it makes the code easier to
> understand, and is necessary before integrating new
> changes.
>
> I may have more reworks to propose in next revisions,
There is some consolidation that could be done in split/packed
sync/async rx helpers too.
Is this what you have in mind?
> but I wanted to share my current status early so that
> you have time to review/test it.
>
> It is only compile-tested, as I don't have HW with IOAT
> support to test it.
>
> Maxime Coquelin (14):
> vhost: move async data in a dedicated structure
> vhost: hide inflight async structure
> vhost: simplify async IO vectors
> vhost: simplify async IO vectors iterators
> vhost: remove async batch threshold
> vhost: introduce specific iovec structure
> vhost: remove useless fields in async iterator struct
> vhost: improve IO vector logic
> vhost: remove notion of async descriptor
> vhost: simplify async enqueue completion
> vhost: simplify getting the first in-flight index
> vhost: prepare async for mbuf to desc refactoring
> vhost: prepare sync for mbuf to desc refactoring
> vhost: merge sync and async mbuf to desc filling
>
> examples/vhost/ioat.c | 30 +-
> examples/vhost/ioat.h | 2 +-
> lib/vhost/rte_vhost_async.h | 42 +--
> lib/vhost/vhost.c | 129 +++----
> lib/vhost/vhost.h | 67 ++--
> lib/vhost/vhost_user.c | 4 +-
> lib/vhost/virtio_net.c | 697 ++++++++++++++----------------------
> 7 files changed, 379 insertions(+), 592 deletions(-)
Just had a quick look.
Nice to shrink vq memory footprint again.
And less code is always better.
+1 from me.
--
David Marchand
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [dpdk-dev] [RFC 08/14] vhost: improve IO vector logic
2021-10-07 22:00 ` [dpdk-dev] [RFC 08/14] vhost: improve IO vector logic Maxime Coquelin
@ 2021-10-12 6:05 ` Hu, Jiayu
2021-10-12 8:34 ` Maxime Coquelin
0 siblings, 1 reply; 21+ messages in thread
From: Hu, Jiayu @ 2021-10-12 6:05 UTC (permalink / raw)
To: Maxime Coquelin, dev, Xia, Chenbo, Wang, YuanX, Ma, WenwuX,
Richardson, Bruce, Mcnamara, John
Hi,
> -----Original Message-----
> From: Maxime Coquelin <maxime.coquelin@redhat.com>
> Sent: Friday, October 8, 2021 6:00 AM
> To: dev@dpdk.org; Xia, Chenbo <chenbo.xia@intel.com>; Hu, Jiayu
> <jiayu.hu@intel.com>; Wang, YuanX <yuanx.wang@intel.com>; Ma,
> WenwuX <wenwux.ma@intel.com>; Richardson, Bruce
> <bruce.richardson@intel.com>; Mcnamara, John
> <john.mcnamara@intel.com>
> Cc: Maxime Coquelin <maxime.coquelin@redhat.com>
> Subject: [RFC 08/14] vhost: improve IO vector logic
>
> IO vectors and their iterators arrays were part of the async metadata but not
> their indexes.
>
> In order to makes this more consistent, the patch adds the indexes to the
> async metadata. Doing that, we can avoid triggering DMA transfer within the
> loop as it IO vector index overflow is now prevented in the
> async_mbuf_to_desc() function.
>
> Note that previous detection mechanism was broken since the overflow
> already happened when detected, so OOB memory access would already
> have happened.
>
> With this changes done, virtio_dev_rx_async_submit_split()
> and virtio_dev_rx_async_submit_packed() can be further simplified.
>
> Signed-off-by: Maxime Coquelin <maxime.coquelin@redhat.com>
> ---
> lib/vhost/vhost.h | 2 +
> lib/vhost/virtio_net.c | 296 +++++++++++++++++++----------------------
> 2 files changed, 136 insertions(+), 162 deletions(-)
>
> diff --git a/lib/vhost/vhost.h b/lib/vhost/vhost.h index
> dae9a1ac2d..812d4c55a5 100644
> --- a/lib/vhost/vhost.h
> +++ b/lib/vhost/vhost.h
> @@ -134,6 +134,8 @@ struct vhost_async {
>
> struct rte_vhost_iov_iter iov_iter[VHOST_MAX_ASYNC_IT];
> struct rte_vhost_iovec iovec[VHOST_MAX_ASYNC_VEC];
> + uint16_t iter_idx;
> + uint16_t iovec_idx;
>
> /* data transfer status */
> struct async_inflight_info *pkts_info; diff --git a/lib/vhost/virtio_net.c
> b/lib/vhost/virtio_net.c index ae7dded979..5ce4c14a73 100644
> --- a/lib/vhost/virtio_net.c
> +++ b/lib/vhost/virtio_net.c
> @@ -924,33 +924,91 @@ copy_mbuf_to_desc(struct virtio_net *dev, struct
> vhost_virtqueue *vq,
> return error;
> }
>
> +static __rte_always_inline int
> +async_iter_initialize(struct vhost_async *async) {
> + struct rte_vhost_iov_iter *iter;
> +
> + if (unlikely(async->iter_idx >= VHOST_MAX_ASYNC_IT)) {
> + VHOST_LOG_DATA(ERR, "no more async iterators
> available\n");
> + return -1;
> + }
async->iter_idx will not exceed VHOST_MAX_ASYNC_IT, as virtio_dev_rx
makes sure the number of packets to enqueue is less than or equal to
MAX_PKT_BURST and it is the same as VHOST_MAX_ASYNC_IT.
> +
> + if (unlikely(async->iovec_idx >= VHOST_MAX_ASYNC_VEC)) {
> + VHOST_LOG_DATA(ERR, "no more async iovec available\n");
> + return -1;
> + }
> +
> +
> + iter = async->iov_iter + async->iter_idx;
> + iter->iov = async->iovec + async->iovec_idx;
> + iter->nr_segs = 0;
> +
> + return 0;
> +}
> +
> +static __rte_always_inline int
> +async_iter_add_iovec(struct vhost_async *async, void *src, void *dst,
> +size_t len) {
> + struct rte_vhost_iov_iter *iter;
> + struct rte_vhost_iovec *iovec;
> +
> + if (unlikely(async->iovec_idx >= VHOST_MAX_ASYNC_VEC)) {
> + VHOST_LOG_DATA(ERR, "no more async iovec available\n");
> + return -1;
> + }
> +
> + iter = async->iov_iter + async->iter_idx;
> + iovec = async->iovec + async->iovec_idx;
async->iovec_idx is never gotten increased.
Thanks,
Jiayu
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [dpdk-dev] [RFC 00/14] vhost: clean-up and simplify async implementation
2021-10-07 21:59 [dpdk-dev] [RFC 00/14] vhost: clean-up and simplify async implementation Maxime Coquelin
` (14 preceding siblings ...)
2021-10-08 12:36 ` [dpdk-dev] [RFC 00/14] vhost: clean-up and simplify async implementation David Marchand
@ 2021-10-12 6:24 ` Hu, Jiayu
15 siblings, 0 replies; 21+ messages in thread
From: Hu, Jiayu @ 2021-10-12 6:24 UTC (permalink / raw)
To: Maxime Coquelin, dev, Xia, Chenbo, Wang, YuanX, Ma, WenwuX,
Richardson, Bruce, Mcnamara, John
Hi Maxime,
> -----Original Message-----
> From: Maxime Coquelin <maxime.coquelin@redhat.com>
> Sent: Friday, October 8, 2021 6:00 AM
> To: dev@dpdk.org; Xia, Chenbo <chenbo.xia@intel.com>; Hu, Jiayu
> <jiayu.hu@intel.com>; Wang, YuanX <yuanx.wang@intel.com>; Ma,
> WenwuX <wenwux.ma@intel.com>; Richardson, Bruce
> <bruce.richardson@intel.com>; Mcnamara, John
> <john.mcnamara@intel.com>
> Cc: Maxime Coquelin <maxime.coquelin@redhat.com>
> Subject: [RFC 00/14] vhost: clean-up and simplify async implementation
>
> This series aims at cleaning and simplifying the async enqueue path. I think it
> makes the code easier to understand, and is necessary before integrating
> new changes.
I really appreciate those changes, and the code looks better, especially for
improving io vector logic, as the existed implementation has a potential risk
of OOB reads/writes.
About the fifth patch, removing async batch threshold, this change is OK for me,
as current vhost is packet level offloading and buffer level DMA batching copy
will be done inside the DMA callback, so no need to keep the batching threshold.
But when integrate DMA logic inside vhost, the async data path still needs to handle
buffer level batching, and some of deleted code may need to be recovered.
Thanks,
Jiayu
>
> I may have more reworks to propose in next revisions, but I wanted to share
> my current status early so that you have time to review/test it.
>
> It is only compile-tested, as I don't have HW with IOAT support to test it.
>
> Maxime Coquelin (14):
> vhost: move async data in a dedicated structure
> vhost: hide inflight async structure
> vhost: simplify async IO vectors
> vhost: simplify async IO vectors iterators
> vhost: remove async batch threshold
> vhost: introduce specific iovec structure
> vhost: remove useless fields in async iterator struct
> vhost: improve IO vector logic
> vhost: remove notion of async descriptor
> vhost: simplify async enqueue completion
> vhost: simplify getting the first in-flight index
> vhost: prepare async for mbuf to desc refactoring
> vhost: prepare sync for mbuf to desc refactoring
> vhost: merge sync and async mbuf to desc filling
>
> examples/vhost/ioat.c | 30 +-
> examples/vhost/ioat.h | 2 +-
> lib/vhost/rte_vhost_async.h | 42 +--
> lib/vhost/vhost.c | 129 +++----
> lib/vhost/vhost.h | 67 ++--
> lib/vhost/vhost_user.c | 4 +-
> lib/vhost/virtio_net.c | 697 ++++++++++++++----------------------
> 7 files changed, 379 insertions(+), 592 deletions(-)
>
> --
> 2.31.1
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [dpdk-dev] [RFC 08/14] vhost: improve IO vector logic
2021-10-12 6:05 ` Hu, Jiayu
@ 2021-10-12 8:34 ` Maxime Coquelin
0 siblings, 0 replies; 21+ messages in thread
From: Maxime Coquelin @ 2021-10-12 8:34 UTC (permalink / raw)
To: Hu, Jiayu, dev, Xia, Chenbo, Wang, YuanX, Ma, WenwuX, Richardson,
Bruce, Mcnamara, John
Hi,
On 10/12/21 08:05, Hu, Jiayu wrote:
> Hi,
>
>> -----Original Message-----
>> From: Maxime Coquelin <maxime.coquelin@redhat.com>
>> Sent: Friday, October 8, 2021 6:00 AM
>> To: dev@dpdk.org; Xia, Chenbo <chenbo.xia@intel.com>; Hu, Jiayu
>> <jiayu.hu@intel.com>; Wang, YuanX <yuanx.wang@intel.com>; Ma,
>> WenwuX <wenwux.ma@intel.com>; Richardson, Bruce
>> <bruce.richardson@intel.com>; Mcnamara, John
>> <john.mcnamara@intel.com>
>> Cc: Maxime Coquelin <maxime.coquelin@redhat.com>
>> Subject: [RFC 08/14] vhost: improve IO vector logic
>>
>> IO vectors and their iterators arrays were part of the async metadata but not
>> their indexes.
>>
>> In order to makes this more consistent, the patch adds the indexes to the
>> async metadata. Doing that, we can avoid triggering DMA transfer within the
>> loop as it IO vector index overflow is now prevented in the
>> async_mbuf_to_desc() function.
>>
>> Note that previous detection mechanism was broken since the overflow
>> already happened when detected, so OOB memory access would already
>> have happened.
>>
>> With this changes done, virtio_dev_rx_async_submit_split()
>> and virtio_dev_rx_async_submit_packed() can be further simplified.
>>
>> Signed-off-by: Maxime Coquelin <maxime.coquelin@redhat.com>
>> ---
>> lib/vhost/vhost.h | 2 +
>> lib/vhost/virtio_net.c | 296 +++++++++++++++++++----------------------
>> 2 files changed, 136 insertions(+), 162 deletions(-)
>>
>> diff --git a/lib/vhost/vhost.h b/lib/vhost/vhost.h index
>> dae9a1ac2d..812d4c55a5 100644
>> --- a/lib/vhost/vhost.h
>> +++ b/lib/vhost/vhost.h
>> @@ -134,6 +134,8 @@ struct vhost_async {
>>
>> struct rte_vhost_iov_iter iov_iter[VHOST_MAX_ASYNC_IT];
>> struct rte_vhost_iovec iovec[VHOST_MAX_ASYNC_VEC];
>> + uint16_t iter_idx;
>> + uint16_t iovec_idx;
>>
>> /* data transfer status */
>> struct async_inflight_info *pkts_info; diff --git a/lib/vhost/virtio_net.c
>> b/lib/vhost/virtio_net.c index ae7dded979..5ce4c14a73 100644
>> --- a/lib/vhost/virtio_net.c
>> +++ b/lib/vhost/virtio_net.c
>> @@ -924,33 +924,91 @@ copy_mbuf_to_desc(struct virtio_net *dev, struct
>> vhost_virtqueue *vq,
>> return error;
>> }
>>
>> +static __rte_always_inline int
>> +async_iter_initialize(struct vhost_async *async) {
>> + struct rte_vhost_iov_iter *iter;
>> +
>> + if (unlikely(async->iter_idx >= VHOST_MAX_ASYNC_IT)) {
>> + VHOST_LOG_DATA(ERR, "no more async iterators
>> available\n");
>> + return -1;
>> + }
>
> async->iter_idx will not exceed VHOST_MAX_ASYNC_IT, as virtio_dev_rx
> makes sure the number of packets to enqueue is less than or equal to
> MAX_PKT_BURST and it is the same as VHOST_MAX_ASYNC_IT.
Agree, this may not be necessary.
>> +
>> + if (unlikely(async->iovec_idx >= VHOST_MAX_ASYNC_VEC)) {
>> + VHOST_LOG_DATA(ERR, "no more async iovec available\n");
>> + return -1;
>> + }
>> +
>> +
>> + iter = async->iov_iter + async->iter_idx;
>> + iter->iov = async->iovec + async->iovec_idx;
>> + iter->nr_segs = 0;
>> +
>> + return 0;
>> +}
>> +
>> +static __rte_always_inline int
>> +async_iter_add_iovec(struct vhost_async *async, void *src, void *dst,
>> +size_t len) {
>> + struct rte_vhost_iov_iter *iter;
>> + struct rte_vhost_iovec *iovec;
>> +
>> + if (unlikely(async->iovec_idx >= VHOST_MAX_ASYNC_VEC)) {
>> + VHOST_LOG_DATA(ERR, "no more async iovec available\n");
>> + return -1;
>> + }
>> +
>> + iter = async->iov_iter + async->iter_idx;
>> + iovec = async->iovec + async->iovec_idx;
>
> async->iovec_idx is never gotten increased.
Good catch!
Thanks,
Maxime
> Thanks,
> Jiayu
>
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [dpdk-dev] [RFC 01/14] vhost: move async data in a dedicated structure
2021-10-07 22:00 ` [dpdk-dev] [RFC 01/14] vhost: move async data in a dedicated structure Maxime Coquelin
@ 2021-10-14 3:24 ` Hu, Jiayu
2021-10-14 8:54 ` Maxime Coquelin
0 siblings, 1 reply; 21+ messages in thread
From: Hu, Jiayu @ 2021-10-14 3:24 UTC (permalink / raw)
To: Maxime Coquelin, dev, Xia, Chenbo, Wang, YuanX, Ma, WenwuX,
Richardson, Bruce, Mcnamara, John
Hi Maxime,
> -----Original Message-----
> From: Maxime Coquelin <maxime.coquelin@redhat.com>
> Sent: Friday, October 8, 2021 6:00 AM
> To: dev@dpdk.org; Xia, Chenbo <chenbo.xia@intel.com>; Hu, Jiayu
> <jiayu.hu@intel.com>; Wang, YuanX <yuanx.wang@intel.com>; Ma,
> WenwuX <wenwux.ma@intel.com>; Richardson, Bruce
> <bruce.richardson@intel.com>; Mcnamara, John
> <john.mcnamara@intel.com>
> Cc: Maxime Coquelin <maxime.coquelin@redhat.com>
> Subject: [RFC 01/14] vhost: move async data in a dedicated structure
>
> This patch moves async-related metadata from vhost_virtqueue to a
> dedicated struct. It makes it clear which fields are async related, and also
> saves some memory when async feature is not in use.
>
> Signed-off-by: Maxime Coquelin <maxime.coquelin@redhat.com>
> ---
> lib/vhost/vhost.c | 129 ++++++++++++++++-------------------------
> lib/vhost/vhost.h | 53 ++++++++---------
> lib/vhost/vhost_user.c | 4 +-
> lib/vhost/virtio_net.c | 114 +++++++++++++++++++-----------------
> 4 files changed, 140 insertions(+), 160 deletions(-)
>
> diff --git a/lib/vhost/vhost.c b/lib/vhost/vhost.c index
> 9540522dac..58f72b633c 100644
> --- a/lib/vhost/vhost.c
> +++ b/lib/vhost/vhost.c
> @@ -340,19 +340,15 @@ cleanup_device(struct virtio_net *dev, int destroy)
> static void vhost_free_async_mem(struct vhost_virtqueue *vq) {
> - rte_free(vq->async_pkts_info);
> + rte_free(vq->async->pkts_info);
Apps may unregister async in vring_state_changed() explicitly when
vring is disabled. In this case, rte_vhost_async_channel_unregister()
will call vhost_free_async_mem() first, so that vq->async becomes NULL.
But after then device is destroyed, free_vq() calls vhost_free_async_mem()
again. "rte_free(vq->async->pkts_info)" will try to read a NULL pointer,
which will cause segment fault.
>
> - rte_free(vq->async_buffers_packed);
> - vq->async_buffers_packed = NULL;
> - rte_free(vq->async_descs_split);
> - vq->async_descs_split = NULL;
> + rte_free(vq->async->buffers_packed);
> + vq->async->buffers_packed = NULL;
> + rte_free(vq->async->descs_split);
> + vq->async->descs_split = NULL;
>
> - rte_free(vq->it_pool);
> - rte_free(vq->vec_pool);
> -
> - vq->async_pkts_info = NULL;
> - vq->it_pool = NULL;
> - vq->vec_pool = NULL;
> + rte_free(vq->async);
> + vq->async = NULL;
> }
>
> void
> @@ -1629,77 +1625,63 @@ async_channel_register(int vid, uint16_t
> queue_id, {
> struct virtio_net *dev = get_device(vid);
> struct vhost_virtqueue *vq = dev->virtqueue[queue_id];
> + struct vhost_async *async;
> + int node = vq->numa_node;
>
> - if (unlikely(vq->async_registered)) {
> + if (unlikely(vq->async)) {
> VHOST_LOG_CONFIG(ERR,
> - "async register failed: channel already registered "
> - "(vid %d, qid: %d)\n", vid, queue_id);
> + "async register failed: already registered
> (vid %d, qid: %d)\n",
> + vid, queue_id);
> return -1;
> }
>
> - vq->async_pkts_info = rte_malloc_socket(NULL,
> - vq->size * sizeof(struct async_inflight_info),
> - RTE_CACHE_LINE_SIZE, vq->numa_node);
> - if (!vq->async_pkts_info) {
> - vhost_free_async_mem(vq);
> - VHOST_LOG_CONFIG(ERR,
> - "async register failed: cannot allocate memory for
> async_pkts_info "
> - "(vid %d, qid: %d)\n", vid, queue_id);
> + async = rte_zmalloc_socket(NULL, sizeof(struct vhost_async), 0,
> node);
> + if (!async) {
> + VHOST_LOG_CONFIG(ERR, "failed to allocate async metadata
> (vid %d, qid: %d)\n",
> + vid, queue_id);
> return -1;
> }
>
> - vq->it_pool = rte_malloc_socket(NULL,
> - VHOST_MAX_ASYNC_IT * sizeof(struct
> rte_vhost_iov_iter),
> - RTE_CACHE_LINE_SIZE, vq->numa_node);
> - if (!vq->it_pool) {
> - vhost_free_async_mem(vq);
> - VHOST_LOG_CONFIG(ERR,
> - "async register failed: cannot allocate memory for
> it_pool "
> - "(vid %d, qid: %d)\n", vid, queue_id);
> - return -1;
> - }
> -
> - vq->vec_pool = rte_malloc_socket(NULL,
> - VHOST_MAX_ASYNC_VEC * sizeof(struct iovec),
> - RTE_CACHE_LINE_SIZE, vq->numa_node);
> - if (!vq->vec_pool) {
> - vhost_free_async_mem(vq);
> - VHOST_LOG_CONFIG(ERR,
> - "async register failed: cannot allocate memory for
> vec_pool "
> - "(vid %d, qid: %d)\n", vid, queue_id);
> - return -1;
> + async->pkts_info = rte_malloc_socket(NULL, vq->size * sizeof(struct
> async_inflight_info),
> + RTE_CACHE_LINE_SIZE, node);
> + if (async->pkts_info) {
It should be "if (!async->pkts_info)".
Thanks,
Jiayu
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [dpdk-dev] [RFC 01/14] vhost: move async data in a dedicated structure
2021-10-14 3:24 ` Hu, Jiayu
@ 2021-10-14 8:54 ` Maxime Coquelin
0 siblings, 0 replies; 21+ messages in thread
From: Maxime Coquelin @ 2021-10-14 8:54 UTC (permalink / raw)
To: Hu, Jiayu, dev, Xia, Chenbo, Wang, YuanX, Ma, WenwuX, Richardson,
Bruce, Mcnamara, John
On 10/14/21 05:24, Hu, Jiayu wrote:
> Hi Maxime,
>
>> -----Original Message-----
>> From: Maxime Coquelin <maxime.coquelin@redhat.com>
>> Sent: Friday, October 8, 2021 6:00 AM
>> To: dev@dpdk.org; Xia, Chenbo <chenbo.xia@intel.com>; Hu, Jiayu
>> <jiayu.hu@intel.com>; Wang, YuanX <yuanx.wang@intel.com>; Ma,
>> WenwuX <wenwux.ma@intel.com>; Richardson, Bruce
>> <bruce.richardson@intel.com>; Mcnamara, John
>> <john.mcnamara@intel.com>
>> Cc: Maxime Coquelin <maxime.coquelin@redhat.com>
>> Subject: [RFC 01/14] vhost: move async data in a dedicated structure
>>
>> This patch moves async-related metadata from vhost_virtqueue to a
>> dedicated struct. It makes it clear which fields are async related, and also
>> saves some memory when async feature is not in use.
>>
>> Signed-off-by: Maxime Coquelin <maxime.coquelin@redhat.com>
>> ---
>> lib/vhost/vhost.c | 129 ++++++++++++++++-------------------------
>> lib/vhost/vhost.h | 53 ++++++++---------
>> lib/vhost/vhost_user.c | 4 +-
>> lib/vhost/virtio_net.c | 114 +++++++++++++++++++-----------------
>> 4 files changed, 140 insertions(+), 160 deletions(-)
>>
>> diff --git a/lib/vhost/vhost.c b/lib/vhost/vhost.c index
>> 9540522dac..58f72b633c 100644
>> --- a/lib/vhost/vhost.c
>> +++ b/lib/vhost/vhost.c
>> @@ -340,19 +340,15 @@ cleanup_device(struct virtio_net *dev, int destroy)
>> static void vhost_free_async_mem(struct vhost_virtqueue *vq) {
>> - rte_free(vq->async_pkts_info);
>> + rte_free(vq->async->pkts_info);
>
> Apps may unregister async in vring_state_changed() explicitly when
> vring is disabled. In this case, rte_vhost_async_channel_unregister()
> will call vhost_free_async_mem() first, so that vq->async becomes NULL.
> But after then device is destroyed, free_vq() calls vhost_free_async_mem()
> again. "rte_free(vq->async->pkts_info)" will try to read a NULL pointer,
> which will cause segment fault.
Good catch, I'll add a check of vq->async before dereferencing it.
>>
>> - rte_free(vq->async_buffers_packed);
>> - vq->async_buffers_packed = NULL;
>> - rte_free(vq->async_descs_split);
>> - vq->async_descs_split = NULL;
>> + rte_free(vq->async->buffers_packed);
>> + vq->async->buffers_packed = NULL;
>> + rte_free(vq->async->descs_split);
>> + vq->async->descs_split = NULL;
>>
>> - rte_free(vq->it_pool);
>> - rte_free(vq->vec_pool);
>> -
>> - vq->async_pkts_info = NULL;
>> - vq->it_pool = NULL;
>> - vq->vec_pool = NULL;
>> + rte_free(vq->async);
>> + vq->async = NULL;
>> }
>>
>> void
>> @@ -1629,77 +1625,63 @@ async_channel_register(int vid, uint16_t
>> queue_id, {
>> struct virtio_net *dev = get_device(vid);
>> struct vhost_virtqueue *vq = dev->virtqueue[queue_id];
>> + struct vhost_async *async;
>> + int node = vq->numa_node;
>>
>> - if (unlikely(vq->async_registered)) {
>> + if (unlikely(vq->async)) {
>> VHOST_LOG_CONFIG(ERR,
>> - "async register failed: channel already registered "
>> - "(vid %d, qid: %d)\n", vid, queue_id);
>> + "async register failed: already registered
>> (vid %d, qid: %d)\n",
>> + vid, queue_id);
>> return -1;
>> }
>>
>> - vq->async_pkts_info = rte_malloc_socket(NULL,
>> - vq->size * sizeof(struct async_inflight_info),
>> - RTE_CACHE_LINE_SIZE, vq->numa_node);
>> - if (!vq->async_pkts_info) {
>> - vhost_free_async_mem(vq);
>> - VHOST_LOG_CONFIG(ERR,
>> - "async register failed: cannot allocate memory for
>> async_pkts_info "
>> - "(vid %d, qid: %d)\n", vid, queue_id);
>> + async = rte_zmalloc_socket(NULL, sizeof(struct vhost_async), 0,
>> node);
>> + if (!async) {
>> + VHOST_LOG_CONFIG(ERR, "failed to allocate async metadata
>> (vid %d, qid: %d)\n",
>> + vid, queue_id);
>> return -1;
>> }
>>
>> - vq->it_pool = rte_malloc_socket(NULL,
>> - VHOST_MAX_ASYNC_IT * sizeof(struct
>> rte_vhost_iov_iter),
>> - RTE_CACHE_LINE_SIZE, vq->numa_node);
>> - if (!vq->it_pool) {
>> - vhost_free_async_mem(vq);
>> - VHOST_LOG_CONFIG(ERR,
>> - "async register failed: cannot allocate memory for
>> it_pool "
>> - "(vid %d, qid: %d)\n", vid, queue_id);
>> - return -1;
>> - }
>> -
>> - vq->vec_pool = rte_malloc_socket(NULL,
>> - VHOST_MAX_ASYNC_VEC * sizeof(struct iovec),
>> - RTE_CACHE_LINE_SIZE, vq->numa_node);
>> - if (!vq->vec_pool) {
>> - vhost_free_async_mem(vq);
>> - VHOST_LOG_CONFIG(ERR,
>> - "async register failed: cannot allocate memory for
>> vec_pool "
>> - "(vid %d, qid: %d)\n", vid, queue_id);
>> - return -1;
>> + async->pkts_info = rte_malloc_socket(NULL, vq->size * sizeof(struct
>> async_inflight_info),
>> + RTE_CACHE_LINE_SIZE, node);
>> + if (async->pkts_info) {
>
> It should be "if (!async->pkts_info)".
Correct.
You can notice I didn't lie when I said it was only compile-tested in
the cover-letter! :)
Regards,
Maxime
> Thanks,
> Jiayu
>
^ permalink raw reply [flat|nested] 21+ messages in thread
end of thread, other threads:[~2021-10-14 8:54 UTC | newest]
Thread overview: 21+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-10-07 21:59 [dpdk-dev] [RFC 00/14] vhost: clean-up and simplify async implementation Maxime Coquelin
2021-10-07 22:00 ` [dpdk-dev] [RFC 01/14] vhost: move async data in a dedicated structure Maxime Coquelin
2021-10-14 3:24 ` Hu, Jiayu
2021-10-14 8:54 ` Maxime Coquelin
2021-10-07 22:00 ` [dpdk-dev] [RFC 02/14] vhost: hide inflight async structure Maxime Coquelin
2021-10-07 22:00 ` [dpdk-dev] [RFC 03/14] vhost: simplify async IO vectors Maxime Coquelin
2021-10-07 22:00 ` [dpdk-dev] [RFC 04/14] vhost: simplify async IO vectors iterators Maxime Coquelin
2021-10-07 22:00 ` [dpdk-dev] [RFC 05/14] vhost: remove async batch threshold Maxime Coquelin
2021-10-07 22:00 ` [dpdk-dev] [RFC 06/14] vhost: introduce specific iovec structure Maxime Coquelin
2021-10-07 22:00 ` [dpdk-dev] [RFC 07/14] vhost: remove useless fields in async iterator struct Maxime Coquelin
2021-10-07 22:00 ` [dpdk-dev] [RFC 08/14] vhost: improve IO vector logic Maxime Coquelin
2021-10-12 6:05 ` Hu, Jiayu
2021-10-12 8:34 ` Maxime Coquelin
2021-10-07 22:00 ` [dpdk-dev] [RFC 09/14] vhost: remove notion of async descriptor Maxime Coquelin
2021-10-07 22:00 ` [dpdk-dev] [RFC 10/14] vhost: simplify async enqueue completion Maxime Coquelin
2021-10-07 22:00 ` [dpdk-dev] [RFC 11/14] vhost: simplify getting the first in-flight index Maxime Coquelin
2021-10-07 22:00 ` [dpdk-dev] [RFC 12/14] vhost: prepare async for mbuf to desc refactoring Maxime Coquelin
2021-10-07 22:00 ` [dpdk-dev] [RFC 13/14] vhost: prepare sync " Maxime Coquelin
2021-10-07 22:00 ` [dpdk-dev] [RFC 14/14] vhost: merge sync and async mbuf to desc filling Maxime Coquelin
2021-10-08 12:36 ` [dpdk-dev] [RFC 00/14] vhost: clean-up and simplify async implementation David Marchand
2021-10-12 6:24 ` Hu, Jiayu
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).