Hi, Ferruh
> -----Original Message-----
> From: Ferruh Yigit <ferruh.yi...@intel.com>
> Sent: Thursday, April 9, 2020 14:56
> To: Slava Ovsiienko <viachesl...@mellanox.com>; dev@dpdk.org
> Cc: Thomas Monjalon <tho...@monjalon.net>;
> bernard.iremon...@intel.com
> Subject: Re: [PATCH v2 1/3] app/testpmd: add profiling flags set command
>
> 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?
OK, I'll update the documentation either and explain the new extended stats.
>
> >
> > Signed-off-by: Viacheslav Ovsiienko <viachesl...@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?
Agree, it would simplify, going to fix.
>
> <...>
>
> > @@ -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.
Yes, it is clearer, but longer to type 😊
If you insist on - I will extend, please, let me know your final opinion.
>
> And what about having another command to show existing value, right now it
> is 'set' only?
Do you mean it would be good to add "show fwdpro/fwdprofile" ?
With best regards, Slava
