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

[Maxime Coquelin <maxime.coquelin@redhat.com>]

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?