Re: [PATCH 4/4] examples/dma: add minimal copy size parameter

[fengchengwen <fengchengwen@huawei.com>]

Hi Bruce, already fix in v2, Thanks.

On 2022/4/11 17:27, Bruce Richardson wrote:
> On Mon, Apr 11, 2022 at 10:56:34AM +0800, Chengwen Feng wrote:
>> This patch adds minimal copy size parameter(-m/--min-copy-size), so
>> when do copy by CPU or DMA, the real copy size will be the maximum of
>> mbuf's data_len and this parameter.
>>
>> This parameter was designed to compare the performance between CPU copy
>> and DMA copy. User could send small packets with a high rate to drive
>> the performance test.
>>
>> Signed-off-by: Chengwen Feng <fengchengwen@huawei.com>
> 
> Hi,
> 
> few comments inline below.
> 
> /Bruce
> 
>> ---
>>  examples/dma/dmafwd.c | 27 ++++++++++++++++++++++++---
>>  1 file changed, 24 insertions(+), 3 deletions(-)
>>
>> diff --git a/examples/dma/dmafwd.c b/examples/dma/dmafwd.c
>> index 6b1b777cb8..83094ba378 100644
>> --- a/examples/dma/dmafwd.c
>> +++ b/examples/dma/dmafwd.c
>> @@ -25,6 +25,7 @@
>>  #define CMD_LINE_OPT_RING_SIZE "ring-size"
>>  #define CMD_LINE_OPT_BATCH_SIZE "dma-batch-size"
>>  #define CMD_LINE_OPT_FRAME_SIZE "max-frame-size"
>> +#define CMD_LINE_OPT_COPY_SIZE	"min-copy-size"
> 
> While I'm not sure this strictly belongs in an example app to show use of
> dmadev, I can see the value of it. However, I suggest we need to make it
> clearer that it's not directly relevant to the normal use of the app. I
> suggest making the parameter "force-min-copy-size" to make it clearer that
> it's an explicit override.
> 
>>  #define CMD_LINE_OPT_STATS_INTERVAL "stats-interval"
>>  
>>  /* configurable number of RX/TX ring descriptors */
>> @@ -119,6 +120,7 @@ static volatile bool force_quit;
>>  
>>  static uint32_t dma_batch_sz = MAX_PKT_BURST;
>>  static uint32_t max_frame_size;

...

>> @@ -576,6 +585,7 @@ dma_usage(const char *prgname)
>>  	printf("%s [EAL options] -- -p PORTMASK [-q NQ]\n"
>>  		"  -b --dma-batch-size: number of requests per DMA batch\n"
>>  		"  -f --max-frame-size: max frame size\n"
>> +		"  -m --min-copy-size: minimum copy length\n"
> 
> The help text needs to be expanded, again to make clear that this is for
> perf comparison and the like. Something like "Force a minimum copy length,
> 	even for smaller packets"
> 
> 
> .
>