RE: [PATCH v7 4/5] vhost: support async dequeue for split ring

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

Hi Xuan,

> -----Original Message-----
> From: Ding, Xuan <xuan.ding@intel.com>
> Sent: Monday, May 16, 2022 10:43 AM
> To: maxime.coquelin@redhat.com; Xia, Chenbo <chenbo.xia@intel.com>
> Cc: dev@dpdk.org; Hu, Jiayu <jiayu.hu@intel.com>; Jiang, Cheng1
> <cheng1.jiang@intel.com>; Pai G, Sunil <sunil.pai.g@intel.com>;
> liangma@liangbit.com; Ding, Xuan <xuan.ding@intel.com>; Wang, YuanX
> <yuanx.wang@intel.com>
> Subject: [PATCH v7 4/5] vhost: support async dequeue for split ring
> 
> From: Xuan Ding <xuan.ding@intel.com>
> 
> This patch implements asynchronous dequeue data path for vhost split ring,
> a new API rte_vhost_async_try_dequeue_burst() is introduced.
> 
> Signed-off-by: Xuan Ding <xuan.ding@intel.com>
> Signed-off-by: Yuan Wang <yuanx.wang@intel.com>
> Tested-by: Yvonne Yang <yvonnex.yang@intel.com>
> Reviewed-by: Maxime Coquelin <maxime.coquelin@redhat.com>
> ---
>  doc/guides/prog_guide/vhost_lib.rst    |   7 +
>  doc/guides/rel_notes/release_22_07.rst |   5 +
>  lib/vhost/rte_vhost_async.h            |  37 +++
>  lib/vhost/version.map                  |   2 +-
>  lib/vhost/virtio_net.c                 | 337 +++++++++++++++++++++++++
>  5 files changed, 387 insertions(+), 1 deletion(-)
> 
> diff --git a/doc/guides/prog_guide/vhost_lib.rst
> b/doc/guides/prog_guide/vhost_lib.rst
> index f287b76ebf..09c1c24b48 100644
> --- a/doc/guides/prog_guide/vhost_lib.rst
> +++ b/doc/guides/prog_guide/vhost_lib.rst
> @@ -282,6 +282,13 @@ The following is an overview of some key Vhost API
> functions:
>    Clear inflight packets which are submitted to DMA engine in vhost async
> data
>    path. Completed packets are returned to applications through ``pkts``.
> 
> +* ``rte_vhost_async_try_dequeue_burst(int vid, uint16_t queue_id,
> +  struct rte_mempool *mbuf_pool, struct rte_mbuf **pkts, uint16_t count,
> +	int *nr_inflight, uint16_t dma_id, uint16_t vchan_id)``

In dmadev library, dma_id is int16_t, rather than uint16_t.

> +
> +  Receives (dequeues) ``count`` packets from guest to host in async
> + data path,  and stored them at ``pkts``.
> +
>  Vhost-user Implementations
>  --------------------------
> 
> diff --git a/doc/guides/rel_notes/release_22_07.rst
> b/doc/guides/rel_notes/release_22_07.rst
> index 88b1e478d4..564d88623e 100644
> --- a/doc/guides/rel_notes/release_22_07.rst
> +++ b/doc/guides/rel_notes/release_22_07.rst
> @@ -70,6 +70,11 @@ New Features
>    Added an API which can get the number of inflight packets in
>    vhost async data path without using lock.
> 
> +* **Added vhost async dequeue API to receive pkts from guest.**
> +
> +  Added vhost async dequeue API which can leverage DMA devices to
> + accelerate receiving pkts from guest.
> +
>  Removed Items
>  -------------
> 
> diff --git a/lib/vhost/rte_vhost_async.h b/lib/vhost/rte_vhost_async.h index
> 70234debf9..2789492e38 100644
> --- a/lib/vhost/rte_vhost_async.h
> +++ b/lib/vhost/rte_vhost_async.h
> @@ -204,6 +204,43 @@ uint16_t rte_vhost_clear_queue_thread_unsafe(int
> vid, uint16_t queue_id,  __rte_experimental  int
> rte_vhost_async_dma_configure(int16_t dma_id, uint16_t vchan_id);
> 
> +/**
> + * @warning
> + * @b EXPERIMENTAL: this API may change, or be removed, without prior
> +notice
> + *
> + * This function tries to receive packets from the guest with
> +offloading
> + * copies to the async channel. The packets that are transfer completed

In rte_vhost_async.h, DMA vChannel is referred as async channel or async copy
engine. But I think it's better to replace all with DMA vChannel to be consistent
with DPDK.

> + * are returned in "pkts". The other packets that their copies are
> +submitted to
> + * the async channel but not completed are called "in-flight packets".
> + * This function will not return in-flight packets until their copies
> +are
> + * completed by the async channel.
> + *
> + * @param vid
> + *  ID of vhost device to dequeue data
> + * @param queue_id
> + *  ID of virtqueue to dequeue data
> + * @param mbuf_pool
> + *  Mbuf_pool where host mbuf is allocated
> + * @param pkts
> + *  Blank array to keep successfully dequeued packets
> + * @param count
> + *  Size of the packet array
> + * @param nr_inflight
> + *  >= 0: The amount of in-flight packets

Better to add more descriptions about the meaning of "nr_inflight".

Thanks,
Jiayu