RE: [RFC 1/1] vhost: integrate dmadev in asynchronous datapath

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

Hi Maxime,

> -----Original Message-----
> From: Maxime Coquelin <maxime.coquelin@redhat.com>
> Sent: Monday, January 3, 2022 6:26 PM
> To: Hu, Jiayu <jiayu.hu@intel.com>; dev@dpdk.org
> Cc: i.maximets@ovn.org; Xia, Chenbo <chenbo.xia@intel.com>; Richardson,
> Bruce <bruce.richardson@intel.com>; Van Haaren, Harry
> <harry.van.haaren@intel.com>; Mcnamara, John
> <john.mcnamara@intel.com>; Pai G, Sunil <sunil.pai.g@intel.com>
> Subject: Re: [RFC 1/1] vhost: integrate dmadev in asynchronous datapath
> 
> Hi Jiayu,
> 
> On 12/28/21 02:15, Hu, Jiayu wrote:
> > Hi Maxime,
> >
> > Thanks for your comments, and some replies are inline.
> >
> > Thanks,
> > Jiayu
> >
> >> -----Original Message-----
> >> From: Maxime Coquelin <maxime.coquelin@redhat.com>
> >> Sent: Friday, December 24, 2021 6:40 PM
> >> To: Hu, Jiayu <jiayu.hu@intel.com>; dev@dpdk.org
> >> Cc: i.maximets@ovn.org; Xia, Chenbo <chenbo.xia@intel.com>;
> >> Richardson, Bruce <bruce.richardson@intel.com>; Van Haaren, Harry
> >> <harry.van.haaren@intel.com>; Mcnamara, John
> >> <john.mcnamara@intel.com>; Pai G, Sunil <sunil.pai.g@intel.com>
> >> Subject: Re: [RFC 1/1] vhost: integrate dmadev in asynchronous
> >> datapath
> >>
> >> Hi Jiayu,
> >>
> >> This is a first review, I need to spend more time on the series to
> >> understand it well. Do you have a prototype of the OVS part, so that
> >> it helps us to grasp how the full integration would look like?
> >
> > I think OVS patch will be sent soon. And we will send the deq side
> implementation too.
> >
> >>
> >> On 11/22/21 11:54, Jiayu Hu wrote:
> >>> Since dmadev is introduced in 21.11, to avoid the overhead of vhost
> >>> DMA abstraction layer and simplify application logics, this patch
> >>> integrates dmadev in asynchronous data path.
> >>>
> >>> Signed-off-by: Jiayu Hu <jiayu.hu@intel.com>
> >>> Signed-off-by: Sunil Pai G <sunil.pai.g@intel.com>
> >>> ---
> >>>    doc/guides/prog_guide/vhost_lib.rst |  63 ++++----
> >>>    examples/vhost/ioat.c               | 218 ----------------------------
> >>>    examples/vhost/ioat.h               |  63 --------
> >>>    examples/vhost/main.c               | 144 +++++++++++++++---
> >>>    examples/vhost/main.h               |  12 ++
> >>>    examples/vhost/meson.build          |   6 +-
> >>>    lib/vhost/meson.build               |   3 +-
> >>>    lib/vhost/rte_vhost_async.h         |  73 +++-------
> >>>    lib/vhost/vhost.c                   |  37 ++---
> >>>    lib/vhost/vhost.h                   |  45 +++++-
> >>>    lib/vhost/virtio_net.c              | 198 ++++++++++++++++++++-----
> >>>    11 files changed, 410 insertions(+), 452 deletions(-)
> >>>    delete mode 100644 examples/vhost/ioat.c
> >>>    delete mode 100644 examples/vhost/ioat.h
> >>>
> 
> ...
> 
> >>> diff --git a/lib/vhost/vhost.c b/lib/vhost/vhost.c index
> >>> 13a9bb9dd1..595cf63b8d 100644
> >>> --- a/lib/vhost/vhost.c
> >>> +++ b/lib/vhost/vhost.c
> >>> @@ -344,6 +344,7 @@ vhost_free_async_mem(struct vhost_virtqueue
> *vq)
> >>>    		return;
> >>>
> >>>    	rte_free(vq->async->pkts_info);
> >>> +	rte_free(vq->async->pkts_cmpl_flag);
> >>>
> >>>    	rte_free(vq->async->buffers_packed);
> >>>    	vq->async->buffers_packed = NULL; @@ -1626,8 +1627,7 @@
> >>> rte_vhost_extern_callback_register(int vid,
> >>>    }
> >>>
> >>> diff --git a/lib/vhost/vhost.h b/lib/vhost/vhost.h index
> >>> 7085e0885c..974e495b56 100644
> >>> --- a/lib/vhost/vhost.h
> >>> +++ b/lib/vhost/vhost.h
> >>> @@ -51,6 +51,11 @@
> >>>    #define VHOST_MAX_ASYNC_IT (MAX_PKT_BURST)
> >>>    #define VHOST_MAX_ASYNC_VEC 2048
> >>>
> >>> +/* DMA device copy operation tracking ring size. */ #define
> >>> +VHOST_ASYNC_DMA_TRACK_RING_SIZE (uint32_t)4096
> >>
> >> How is this value chosen? Is that specific to your hardware?
> >
> > Yes. But in fact, this value should be equal to or greater than vchan
> > desc number, and it should be dynamic. In addition, the context
> > tracking array " dma_copy_track" should be per-vchan basis, rather
> > than per-device, although existed DMA devices only supports 1 vchan at
> most.
> >
> > I have reworked this part which can be configured by users dynamically.
> 
> Wouldn't it be better to use the max_desc value from from struct
> rte_dma_info?

Yes, you are right. I will use this structure in the next version.

Thanks,
Jiayu