[v2,1/3] app/testpmd: add profiling flags set command
Checks
Commit Message
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'.
Signed-off-by: Viacheslav Ovsiienko <viacheslavo@mellanox.com>
---
app/test-pmd/cmdline.c | 15 +++++++++++++++
app/test-pmd/config.c | 10 ++++++++++
app/test-pmd/testpmd.c | 3 +++
app/test-pmd/testpmd.h | 8 ++++++++
doc/guides/testpmd_app_ug/testpmd_funcs.rst | 22 ++++++++++++++++++++++
5 files changed, 58 insertions(+)
Comments
19/03/2020 14:50, Viacheslav Ovsiienko:
> +set fwdprof
> +~~~~~~~~~~~
> +
> +Set the flags controlling the datapath profiling statistics::
> +
> + testpmd> set fwdprof (flags)
> +
> +This command is available only if ``CONFIG_RTE_TEST_PMD_RECORD_CORE_CYCLES`` is
> +configured to Y, enabling the profiling code generation.
> +
> +The bitmask flag controls the gathering profiling statistics for datapath:
> +
> +* ``bit 0`` enables gathering the profiling data for the entire
> + forwarding routine, counts the ticks spent in the forwarding routine,
> + is set by default.
> +* ``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.
> +* ``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.
As commented in the cover letter, please replace flags with Rx/Tx text tokens.
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?
Hi, Ferruh
> -----Original Message-----
> From: Ferruh Yigit <ferruh.yigit@intel.com>
> Sent: Thursday, April 9, 2020 14:56
> To: Slava Ovsiienko <viacheslavo@mellanox.com>; dev@dpdk.org
> Cc: Thomas Monjalon <thomas@monjalon.net>;
> bernard.iremonger@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 <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?
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
13/04/2020 09:56, Slava Ovsiienko:
> From: Ferruh Yigit <ferruh.yigit@intel.com>
> > > +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.
I agree with Ferruh in general and especially about "fwdprof" not nice to read.
What about fwd-profiling?
I would be also OK with fwd-prof.
Is it common to use hyphen sign in testpmd commands?
On 4/13/2020 8:56 AM, Slava Ovsiienko wrote:
> Hi, Ferruh
>
>> -----Original Message-----
>> From: Ferruh Yigit <ferruh.yigit@intel.com>
>> Sent: Thursday, April 9, 2020 14:56
>> To: Slava Ovsiienko <viacheslavo@mellanox.com>; dev@dpdk.org
>> Cc: Thomas Monjalon <thomas@monjalon.net>;
>> bernard.iremonger@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 <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?
> 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.
In the scope of this patch it may look OK, but if you look all testpmd commands
without knowing the code, 'fwdprof' may not be clear what it is. I am for making
it more clear.
>
>>
>> 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" ?
Yes.
@@ -263,6 +263,9 @@ static void cmd_help_long_parsed(void *parsed_result,
"set verbose (level)\n"
" Set the debug verbosity level X.\n\n"
+ "set fwdprof (flags)\n"
+ " Set the flags to profile the forwarding.\n\n"
+
"set log global|(type) (level)\n"
" Set the log level.\n\n"
@@ -3743,20 +3746,32 @@ static void cmd_set_parsed(void *parsed_result,
set_nb_pkt_per_burst(res->value);
else if (!strcmp(res->what, "verbose"))
set_verbose_level(res->value);
+#ifdef RTE_TEST_PMD_RECORD_CORE_CYCLES
+ else if (!strcmp(res->what, "fwdprof"))
+ set_fwdprof_flags(res->value);
+#endif
}
cmdline_parse_token_string_t cmd_set_set =
TOKEN_STRING_INITIALIZER(struct cmd_set_result, set, "set");
cmdline_parse_token_string_t cmd_set_what =
TOKEN_STRING_INITIALIZER(struct cmd_set_result, what,
+#ifndef RTE_TEST_PMD_RECORD_CORE_CYCLES
"nbport#nbcore#burst#verbose");
+#else
+ "nbport#nbcore#burst#verbose#fwdprof");
+#endif
cmdline_parse_token_num_t cmd_set_value =
TOKEN_NUM_INITIALIZER(struct cmd_set_result, value, UINT16);
cmdline_parse_inst_t cmd_set_numbers = {
.f = cmd_set_parsed,
.data = NULL,
+#ifndef RTE_TEST_PMD_RECORD_CORE_CYCLES
.help_str = "set nbport|nbcore|burst|verbose <value>",
+#else
+ .help_str = "set nbport|nbcore|burst|verbose|fwdprof <value>",
+#endif
.tokens = {
(void *)&cmd_set_set,
(void *)&cmd_set_what,
@@ -3151,6 +3151,16 @@ struct igb_ring_desc_16_bytes {
configure_rxtx_dump_callbacks(verbose_level);
}
+#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);
+ fwdprof_flags = pf_flags;
+}
+#endif
+
void
vlan_extend_set(portid_t port_id, int on)
{
@@ -81,6 +81,9 @@
#define EXTBUF_ZONE_SIZE RTE_PGSIZE_2M
uint16_t verbose_level = 0; /**< Silent by default. */
+#ifdef RTE_TEST_PMD_RECORD_CORE_CYCLES
+uint16_t fwdprof_flags = RECORD_CORE_CYCLES_FWD; /**< Fwd only by default. */
+#endif
int testpmd_logtype; /**< Log type for testpmd logs */
/* use master core for command line ? */
@@ -321,8 +321,15 @@ struct queue_stats_mappings {
extern uint8_t xstats_hide_zero; /**< Hide zero values for xstats display */
+#ifdef RTE_TEST_PMD_RECORD_CORE_CYCLES
+#define RECORD_CORE_CYCLES_FWD (1<<0)
+#define RECORD_CORE_CYCLES_RX (1<<1)
+#define RECORD_CORE_CYCLES_TX (1<<2)
+#endif
+
/* globals used for configuration */
extern uint16_t verbose_level; /**< Drives messages being displayed, if any. */
+extern uint16_t fwdprof_flags; /**< Controls the profiling statistics. */
extern int testpmd_logtype; /**< Log type for testpmd logs */
extern uint8_t interactive;
extern uint8_t auto_start;
@@ -787,6 +794,7 @@ void vlan_tpid_set(portid_t port_id, enum rte_vlan_type vlan_type,
void set_xstats_hide_zero(uint8_t on_off);
void set_verbose_level(uint16_t vb_level);
+void set_fwdprof_flags(uint16_t pf_flags);
void set_tx_pkt_segments(unsigned *seg_lengths, unsigned nb_segs);
void show_tx_pkt_segments(void);
void set_tx_pkt_split(const char *name);
@@ -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)
+
+This command is available only if ``CONFIG_RTE_TEST_PMD_RECORD_CORE_CYCLES`` is
+configured to Y, enabling the profiling code generation.
+
+The bitmask flag controls the gathering profiling statistics for datapath:
+
+* ``bit 0`` enables gathering the profiling data for the entire
+ forwarding routine, counts the ticks spent in the forwarding routine,
+ is set by default.
+* ``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.
+* ``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.
+
set nbport
~~~~~~~~~~