On 6/23/2023 9:00 AM, Slava Ovsiienko wrote:
> Hi, Ferruh
>
>> -----Original Message-----
>> From: Ferruh Yigit <ferruh.yi...@amd.com>
>> Sent: Wednesday, June 21, 2023 2:16 PM
>> To: Slava Ovsiienko <viachesl...@nvidia.com>; dev@dpdk.org; Aman Singh
>> <aman.deep.si...@intel.com>
>> Cc: Jerin Jacob Kollanukkaran <jer...@marvell.com>
>> Subject: Re: [PATCH v2 1/5] app/testpmd: add trace save command
>>
>> On 6/13/2023 5:58 PM, Viacheslav Ovsiienko wrote:
>>> The "save_trace" CLI command is added to trigger saving the trace
>>> dumps to the trace directory.
>>>
>>
>> Hi Viacheslav,
>>
>> Trace is already saved when dpdk application terminated, I guess this is to
>> save the trace before exiting the application, what is the use case for
>> this, can
>> you please detail in the commit log.
>
> OK, will update the commit log. The command "save_trace" is useful in some
> dynamic debug scenarios to save the trace without restarting the entire
> application.
>
>>
>> And what happens if this is called multiple times, or what happens on the
>> application exit, will it overwrite the file or fail?
> It overwrites.
>
>> Again please explain in the commit log.
> Sure, will do.
>
>>
>>> Signed-off-by: Viacheslav Ovsiienko <viachesl...@nvidia.com>
>>> ---
>>> app/test-pmd/cmdline.c | 38 ++++++++++++++++++++++++++++++++++++++
>>> 1 file changed, 38 insertions(+)
>>>
>>
>> Can you please update documentation too?
>>
>>> diff --git a/app/test-pmd/cmdline.c b/app/test-pmd/cmdline.c index
>>> a15a442a06..db71ce2028 100644
>>> --- a/app/test-pmd/cmdline.c
>>> +++ b/app/test-pmd/cmdline.c
>>> @@ -39,6 +39,7 @@
>>> #include <rte_gro.h>
>>> #endif
>>> #include <rte_mbuf_dyn.h>
>>> +#include <rte_trace.h>
>>>
>>> #include <cmdline_rdline.h>
>>> #include <cmdline_parse.h>
>>> @@ -12745,6 +12746,40 @@ static cmdline_parse_inst_t
>> cmd_config_tx_affinity_map = {
>>> },
>>> };
>>>
>>> +#ifndef RTE_EXEC_ENV_WINDOWS
>>> +/* *** SAVE_TRACE *** */
>>> +
>>> +struct cmd_save_trace_result {
>>> + cmdline_fixed_string_t save;
>>> +};
>>> +
>>> +static void cmd_save_trace_parsed(__rte_unused void *parsed_result,
>>> + __rte_unused struct cmdline *cl,
>>> + __rte_unused void *data)
>>> +{
>>> + int rc;
>>> +
>>> + rc = rte_trace_save();
>>> + if (rc)
>>> + printf("Save trace failed with error: %d\n", rc);
>>> + else
>>> + printf("Trace saved successfully\n"); }
>>> +
>>> +static cmdline_parse_token_string_t cmd_save_trace_save =
>>> + TOKEN_STRING_INITIALIZER(struct cmd_save_trace_result, save,
>>> +"save_trace");
>>> +
>>
>> We have dump_* commands, what do you think to have 'dump_trace'
>> command for this?
> It was initially (in v1) with "dump_trace" command.
> And there is the comment by Jerin:
> https://inbox.dpdk.org/dev/calbae1of79a_jhnft3kx--enhud-h5rzl02tmqbsmow721d...@mail.gmail.com/#t
>
> So, I have changed to "save_trace". I have no strong opinion about command
> name, any allowing trace save is OK for me.
>
Ah, I missed that.
@Jerin,
I just saw your comment, agree more exact action can be 'save' but
'dump' also describes enough.
Since there are existing 'dump_*' commands, it makes command more
intuitive and easy to remember.
As an active user of testpmd myself, I am finding it hard to
remember/find the command I need as number of commands increased. That
is why I am paying extra attention to have more hierarchical, consistent
and intuitive commands.
For me "dump_trace" works better in that manner, what do you think, do
you have strong opinion on 'save_trace'?
