Re: [dpdk-dev] [RFC 08/14] vhost: improve IO vector logic

["Hu, Jiayu" <jiayu.hu@intel.com>]

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