Re: [EXTERNAL] Re: [EXT] Re: [PATCH v2] app/dma-perf: support bi-directional transfer

[fengchengwen <fengchengwen@huawei.com>]

Hi Amit,

On 2024/3/1 18:59, Amit Prakash Shukla wrote:
> Hi Chengwen,
> 
> Please find my reply in-line.
> 
> Thanks,
> Amit Shukla
> 
>> Hi Amit,
>>
>> On 2024/3/1 16:31, Amit Prakash Shukla wrote:
>>> Hi Chengwen,
>>>
>>> If I'm not wrong, your concern was about config file additions and not
>>> about the test as such. If the config file is getting complicated and
>>> there are better alternatives, we can minimize the config file changes
>>> with this patch and just provide minimum functionality as required and
>>> leave it open for future changes. For now, I can document the existing
>>> behavior in documentation as "Constraints". Similar approach is
>>> followed in other application such as ipsec-secgw
>>> https://urldefense.proofpoint.com/v2/url?u=https-3A__doc.dpdk.org_guid
>>> es_sample-5Fapp-5Fug_ipsec-5Fsecgw.html-
>> 23constraints&d=DwICaQ&c=nKjWe
>>>
>> c2b6R0mOyPaz7xtfQ&r=ALGdXl3fZgFGR69VnJLdSnADun7zLaXG1p5Rs7pXihE
>> &m=UXlZ
>>>
>> 1CWj8uotMMmYQ4e7wtBXj4geBwcMUirlqFw0pZzSlOIIAVjWaPgcaXtni370&
>> s=haaehrX
>>> QSEG6EFRW8w2sHKUTU75aJX7ML8vM-0mJsAI&e=
>>
>> Yes, I prefer enable different test just by modify configuration file, and then
>> limit the number of entries at the same time.
>>
>> This commit is bi-direction transfer, it is fixed, maybe later we should test 3/4
>> for mem2dev while 1/4 for dev2mem.
> 
> Agreed. We will add this later after the base functionality is merged. I will send next version with constraints listed. Can I assume next version is good for merge?

I suggest do it all at once in [1].

[1] https://patches.dpdk.org/project/dpdk/cover/cover.1709210551.git.gmuthukrishn@marvell.com/

Thanks

> 
>>
>> sometime we may need evaluate performance of one dma channel for
>> mem2mem, while another channel for mem2dev, we can't do this in current
>> implement (because vchan_dev is for all DMA channel).
> 
> We are okay with extending it later. As you said, we are still deciding how the configuration file should look like.
> 
>>
>> So I prefer restrict DMA non-mem2mem's config (include
>> dir/type/coreid/pfid/vfid/raddr) as the dma device's private configuration.
>>
>> Thanks
>>
>>>
>>> Constraints:
>>> 1. vchan_dev config will be same for all the configured DMA devices.
>>> 2. Alternate DMA device will do dev2mem and mem2dev implicitly.
>>> Example:
>>> xfer_mode=1
>>> vchan_dev=raddr=0x200000000,coreid=1,pfid=2,vfid=3
>>> lcore_dma=lcore10@0000:00:04.2, lcore11@0000:00:04.3,
>>> lcore12@0000:00:04.4, lcore13@0000:00:04.5
>>>
>>> lcore10@0000:00:04.2, lcore12@0000:00:04.4 will do dev2mem and
>> lcore11@0000:00:04.3, lcore13@0000:00:04.5 will do mem2dev.
>>>
>>> Thanks,
>>> Amit Shukla
>>>
>>>> -----Original Message-----
>>>> From: fengchengwen <fengchengwen@huawei.com>
>>>> Sent: Friday, March 1, 2024 7:16 AM
>>>> To: Amit Prakash Shukla <amitprakashs@marvell.com>; Cheng Jiang
>>>> <honest.jiang@foxmail.com>; Gowrishankar Muthukrishnan
>>>> <gmuthukrishn@marvell.com>
>>>> Cc: dev@dpdk.org; Jerin Jacob <jerinj@marvell.com>; Anoob Joseph
>>>> <anoobj@marvell.com>; Kevin Laatz <kevin.laatz@intel.com>; Bruce
>>>> Richardson <bruce.richardson@intel.com>; Pavan Nikhilesh Bhagavatula
>>>> <pbhagavatula@marvell.com>
>>>> Subject: [EXTERNAL] Re: [EXT] Re: [PATCH v2] app/dma-perf: support
>>>> bi- directional transfer
>>>>
>>>> Prioritize security for external emails: Confirm sender and content
>>>> safety before clicking links or opening attachments
>>>>
>>>> ---------------------------------------------------------------------
>>>> -
>>>> Hi Amit,
>>>>
>>>> I think this commit will complicated the test, plus futer we may add
>>>> more test (e.g. fill)
>>>>
>>>> I agree Bruce's advise in the [1], let also support "lcore_dma0/1/2",
>>>>
>>>> User could provide dma info by two way:
>>>> 1) lcore_dma=, which seperate each dma with ", ", but a maximum of a
>>>> certain number is allowed.
>>>> 2) lcore_dma0/1/2/..., each dma device take one line
>>>>
>>>> [1] https://urldefense.proofpoint.com/v2/url?u=https-
>>>> 3A__patchwork.dpdk.org_project_dpdk_patch_20231206112952.1588-
>>>> 2D1-2Dvipin.varghese-
>>>>
>> 40amd.com_&d=DwICaQ&c=nKjWec2b6R0mOyPaz7xtfQ&r=ALGdXl3fZgFGR6
>>>> 9VnJLdSnADun7zLaXG1p5Rs7pXihE&m=OwrvdPIi-
>>>>
>> TQ2UEH3cztfXDzT8YkOB099Pl1mfUzGaq9td0fEWrRBLQQBzAFkjQSU&s=kKin
>>>> YsGoNyTxuLEyPJ0LppT17Yq64CvFBtJMirGEISI&e=
>>>>
>>>> Thanks
>>>>
>>>> On 2024/2/29 22:03, Amit Prakash Shukla wrote:
>>>>> Hi Chengwen,
>>>>>
>>>>> I liked your suggestion and tried making changes, but encountered
>>>>> parsing
>>>> issue for CFG files with line greater than CFG_VALUE_LEN=256(current
>>>> value set).
>>>>>
>>>>> There is a discussion on the similar lines in another patch set:
>>>> https://urldefense.proofpoint.com/v2/url?u=https-
>>>> 3A__patchwork.dpdk.org_project_dpdk_patch_20231206112952.1588-
>>>> 2D1-2Dvipin.varghese-
>>>>
>> 40amd.com_&d=DwICaQ&c=nKjWec2b6R0mOyPaz7xtfQ&r=ALGdXl3fZgFGR6
>>>> 9VnJLdSnADun7zLaXG1p5Rs7pXihE&m=OwrvdPIi-
>>>>
>> TQ2UEH3cztfXDzT8YkOB099Pl1mfUzGaq9td0fEWrRBLQQBzAFkjQSU&s=kKin
>>>> YsGoNyTxuLEyPJ0LppT17Yq64CvFBtJMirGEISI&e= .
>>>>>
>>>>> I believe this patch can be taken as-is and we can come up with the
>>>>> solution
>>>> when we can increase the CFG_VALUE_LEN as changing CFG_VALUE_LEN in
>>>> this release is causing ABI breakage.
>>>>>
>>>>> Thanks,
>>>>> Amit Shukla
>>>>>
> 
> <snip>
>