Hi, On 10/12/21 08:05, Hu, Jiayu wrote:
Hi,-----Original Message----- From: Maxime Coquelin <maxime.coque...@redhat.com> Sent: Friday, October 8, 2021 6:00 AM To: dev@dpdk.org; Xia, Chenbo <chenbo....@intel.com>; Hu, Jiayu <jiayu...@intel.com>; Wang, YuanX <yuanx.w...@intel.com>; Ma, WenwuX <wenwux...@intel.com>; Richardson, Bruce <bruce.richard...@intel.com>; Mcnamara, John <john.mcnam...@intel.com> Cc: Maxime Coquelin <maxime.coque...@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.coque...@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
