[PATCH v4] app/testpmd: add trace dump command

[Ferruh Yigit]

On 6/27/2023 12:39 PM, Slava Ovsiienko wrote:
>> -----Original Message-----
>> From: Ferruh Yigit <ferruh.yigit at amd.com>
>> Sent: Tuesday, June 27, 2023 2:35 PM
>> To: Slava Ovsiienko <viacheslavo at nvidia.com>; dev at dpdk.org
>> Cc: jerinj at marvell.com
>> Subject: Re: [PATCH v4] app/testpmd: add trace dump command
>>
>> On 6/26/2023 12:57 PM, Viacheslav Ovsiienko wrote:
>>> The "dump_trace" CLI command is added to trigger saving the trace
>>> dumps to the trace directory.
>>>
>>> The tracing data are saved according to the EAL configuration
>>> (explicit --trace-dir EAL command line parameter alters the target
>>> folder to save). The result dump folder gets the name like
>>> rte-YYYY-MM-DD-xx-HH-MM-SS format.
>>>
>>> This command is useful to get the trace date without exiting testpmd
>>> application and to get the multiple dumps to observe the situation in
>>> dynamics.
>>>
>>> Signed-off-by: Viacheslav Ovsiienko <viacheslavo at nvidia.com>
>>>
>>
>> Acked-by: Ferruh Yigit <ferruh.yigit at amd.com>
>>
>>> --
>>>
>>> v1:
>>> https://inbox.dpdk.org/dev/20230609152847.32496-2-viacheslavo@nvidia.c
>>> om
>>> v2: https://inbox.dpdk.org/dev/20230613165845.19109-2-
>> viacheslavo at nvidia.com
>>>     - changed to save_trace command
>>>     - Windows compilation check added
>>>
>>> v3: https://inbox.dpdk.org/dev/20230626110734.14126-1-
>> viacheslavo at nvidia.com
>>>     - reverted to "dump_trace" command
>>>
>>> v4: - added missed header file include
>>>     - missed #ifdef added for Windows compilation (no trace support
>>>       for Windows)
>>> ---
>>>  app/test-pmd/cmdline.c | 8 ++++++++
>>>  1 file changed, 8 insertions(+)
>>>
>>
>> Can you please update 'doc/guides/testpmd_app_ug/testpmd_funcs.rst' for
>> new command?
> Sure.
> 
>>
>> It looks like dump_* commands missed in the help output,
>> 'cmd_help_long_parsed()', can you please append this new one end of
>> "display" section, we can complete the missing ones later?
>>
>> <...>
>>
>>> @@ -8371,10 +8372,17 @@ static void cmd_dump_parsed(void
>> *parsed_result,
>>>  		rte_lcore_dump(stdout);
>>>  	else if (!strcmp(res->dump, "dump_log_types"))
>>>  		rte_log_dump(stdout);
>>> +#ifndef RTE_EXEC_ENV_WINDOWS
>>> +	else if (!strcmp(res->dump, "dump_trace"))
>>> +		rte_trace_save();
>>> +#endif
>>>  }
>>>
>>>  static cmdline_parse_token_string_t cmd_dump_dump =
>>>  	TOKEN_STRING_INITIALIZER(struct cmd_dump_result, dump,
>>> +#ifndef RTE_EXEC_ENV_WINDOWS
>>> +		"dump_trace#"
>>> +#endif
>>>
>>
>> Why not add "dump_trace#" as last item, to keep same order with
>> 'cmd_dump_parsed()'?
> 
> This would require modify the preceding command undef #ifndef and #else:
> #ifndef RTE_EXEC_ENV_WINDOWS
> "dump_log_types#
> "dump_trace");
> #else
> "dump_log_types");
> #endif
> 
> If you think order is more important - please,  let me know, I'll update
> 

Lets move it just before 'dump_log_types', in both instance :)