Re: [dpdk-dev] [PATCH v2 1/3] app/testpmd: add profiling flags set command

[Ferruh Yigit <ferruh.yigit@intel.com>]

On 3/19/2020 1:50 PM, Viacheslav Ovsiienko wrote:
> This commit is preparation step before adding the separated profiling
> of Rx and Tx burst routines. The new testpmd command is introduced:
> 
>   set fwdprof (flags)
> 
> This command controls which profiling statistics is being gathered
> in runtime:
> 
> - bit 0 - enables profiling the entire forward routine, counts the ticks
>           spent in the forwarding routine, is set by default. Provides the
> 	  same data as previous implementation.
> 
> - bit 1 - enables gathering the profiling data for the transmit datapath,
>           counts the ticks spent within rte_eth_tx_burst() routine,
>           is cleared by default, extends the existing statistics.
> 
> - bit 2 - enables gathering the profiling data for the receive datapath,
>           counts the ticks spent within rte_eth_rx_burst() routine,
>           is cleared by default, extends the existing statistics.
> 
> The new counters for the cycles spent into rx/tx burst
> routines are introduced. The feature is engaged only if
> CONFIG_RTE_TEST_PMD_RECORD_CORE_CYCLES configured to 'Y'.

Hi Slava,

Thanks for improving the testpmd performance measuring, unfortunately these
features are not documented at all, unless I miss it, and now you are improving
it but still there is no documentation.

Would you mind adding a section for performance measures, document how to enable
and use it, and how to read the output?

> 
> Signed-off-by: Viacheslav Ovsiienko <viacheslavo@mellanox.com>

<...>

> +#ifdef RTE_TEST_PMD_RECORD_CORE_CYCLES
> +void
> +set_fwdprof_flags(uint16_t pf_flags)
> +{
> +	printf("Change forward profiling flags from %u to %u\n",
> +	       (unsigned int) fwdprof_flags, (unsigned int) pf_flags);

To reduce the 'RTE_TEST_PMD_RECORD_CORE_CYCLES' define usage, what do you think
have it only in this function, if it is defined work as expected and if not
print an error and don't update anything?

<...>

> @@ -647,6 +647,28 @@ Regexes can also be used for type. To change log level of user1, user2 and user3
>  
>  	testpmd> set log user[1-3] (level)
>  
> +set fwdprof
> +~~~~~~~~~~~
> +
> +Set the flags controlling the datapath profiling statistics::
> +
> +   testpmd> set fwdprof (flags)

What do you think a little longer command 'fwdprofile", which is more clear I think.

And what about having another command to show existing value, right now it is
'set' only?