[dpdk-dev] [PATCH 1/5] app/testpmd: print clock with CPU cycles per pkt

[dpdk-dev] [PATCH 1/5] app/testpmd: print clock with CPU cycles per pkt

[Dharmik Thakkar]

On aarch64 platforms, the cycles are counted using either a
low-resolution generic counter or a high-resolution PMU cycle counter.
Print the clock frequency along with CPU cycles/packet to identify which
cycle counter is being used.

Signed-off-by: Dharmik Thakkar <dharmik.thakkar@arm.com>
Reviewed-by: Honnappa Nagarahalli <honnappa.nagarahalli@arm.com>
Reviewed-by: Phil Yang <phil.yang@arm.com>
---
 app/test-pmd/testpmd.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/app/test-pmd/testpmd.c b/app/test-pmd/testpmd.c
index 99bacddbfdca..9a8cbbd6fc7c 100644
--- a/app/test-pmd/testpmd.c
+++ b/app/test-pmd/testpmd.c
@@ -1953,11 +1953,12 @@ fwd_stats_display(void)
 	       "%s\n",
 	       acc_stats_border, acc_stats_border);
 #ifdef RTE_TEST_PMD_RECORD_CORE_CYCLES
+#define CYC_PER_MHZ 1E6
 	if (total_recv > 0)
 		printf("\n  CPU cycles/packet=%u (total cycles="
-		       "%"PRIu64" / total RX packets=%"PRIu64")\n",
+		       "%"PRIu64" / total RX packets=%"PRIu64") at %lu MHz Clock\n",
 		       (unsigned int)(fwd_cycles / total_recv),
-		       fwd_cycles, total_recv);
+		       fwd_cycles, total_recv, (uint64_t)(rte_get_tsc_hz() / CYC_PER_MHZ));
 #endif
 }
 
-- 
2.20.1

[dpdk-dev] [PATCH 2/5] app/testpmd: print fractional part in CPU cycles

[Dharmik Thakkar]

Change printing of CPU cycles/packet to include fractional part for
accurateness.

Example:

Without patch:
CPU cycles/packet=14
(total cycles=4899533541 / total RX packets=343031966)

With patch:
CPU cycles/packet=14.28
(total cycles=4899533541 / total RX packets=343031966)

Signed-off-by: Dharmik Thakkar <dharmik.thakkar@arm.com>
Reviewed-by: Honnappa Nagarahalli <honnappa.nagarahalli@arm.com>
Reviewed-by: Phil Yang <phil.yang@arm.com>
---
 app/test-pmd/testpmd.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/app/test-pmd/testpmd.c b/app/test-pmd/testpmd.c
index 9a8cbbd6fc7c..9444a730a153 100644
--- a/app/test-pmd/testpmd.c
+++ b/app/test-pmd/testpmd.c
@@ -1955,9 +1955,9 @@ fwd_stats_display(void)
 #ifdef RTE_TEST_PMD_RECORD_CORE_CYCLES
 #define CYC_PER_MHZ 1E6
 	if (total_recv > 0)
-		printf("\n  CPU cycles/packet=%u (total cycles="
+		printf("\n  CPU cycles/packet=%.2f (total cycles="
 		       "%"PRIu64" / total RX packets=%"PRIu64") at %lu MHz Clock\n",
-		       (unsigned int)(fwd_cycles / total_recv),
+		       (double)(fwd_cycles / (double)total_recv),
 		       fwd_cycles, total_recv, (uint64_t)(rte_get_tsc_hz() / CYC_PER_MHZ));
 #endif
 }
-- 
2.20.1

[dpdk-dev] [PATCH 3/5] doc: add cycles per packet in testpmd user guide

[Dharmik Thakkar]

Update documentation for 'show fwd' testpmd runtime function to show
CPU cycles/packet example.

Signed-off-by: Dharmik Thakkar <dharmik.thakkar@arm.com>
Reviewed-by: Honnappa Nagarahalli <honnappa.nagarahalli@arm.com>
Reviewed-by: Phil Yang <phil.yang@arm.com>
---
 doc/guides/testpmd_app_ug/testpmd_funcs.rst | 51 +++++++++++++--------
 1 file changed, 31 insertions(+), 20 deletions(-)

diff --git a/doc/guides/testpmd_app_ug/testpmd_funcs.rst b/doc/guides/testpmd_app_ug/testpmd_funcs.rst
index a360ecccfd3f..2e565ccdbcd4 100644
--- a/doc/guides/testpmd_app_ug/testpmd_funcs.rst
+++ b/doc/guides/testpmd_app_ug/testpmd_funcs.rst
@@ -351,26 +351,37 @@ Example for the io forwarding engine, with some packet drops on the tx side::
 
    testpmd> show fwd stats all
 
-     ------- Forward Stats for RX Port= 0/Queue= 0 -> TX Port= 1/Queue= 0 -------
-     RX-packets: 274293770      TX-packets: 274293642      TX-dropped: 128
-
-     ------- Forward Stats for RX Port= 1/Queue= 0 -> TX Port= 0/Queue= 0 -------
-     RX-packets: 274301850      TX-packets: 274301850      TX-dropped: 0
-
-     ---------------------- Forward statistics for port 0  ----------------------
-     RX-packets: 274293802      RX-dropped: 0             RX-total: 274293802
-     TX-packets: 274301862      TX-dropped: 0             TX-total: 274301862
-     ----------------------------------------------------------------------------
-
-     ---------------------- Forward statistics for port 1  ----------------------
-     RX-packets: 274301894      RX-dropped: 0             RX-total: 274301894
-     TX-packets: 274293706      TX-dropped: 128           TX-total: 274293834
-     ----------------------------------------------------------------------------
-
-     +++++++++++++++ Accumulated forward statistics for all ports+++++++++++++++
-     RX-packets: 548595696      RX-dropped: 0             RX-total: 548595696
-     TX-packets: 548595568      TX-dropped: 128           TX-total: 548595696
-     ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
+     ------- Forward Stats for RX Port= 0/Queue= 0 -> TX Port= 1/Queue= 0 -------
+     RX-packets: 43536504       TX-packets: 43536488       TX-dropped: 0
+
+     ------- Forward Stats for RX Port= 1/Queue= 0 -> TX Port= 0/Queue= 0 -------
+     RX-packets: 149738504      TX-packets: 149738504      TX-dropped: 0
+
+     ------- Forward Stats for RX Port= 1/Queue= 1 -> TX Port= 0/Queue= 1 -------
+     RX-packets: 149753052      TX-packets: 149753052      TX-dropped: 0
+
+     ---------------------- Forward statistics for port 0  ----------------------
+     RX-packets: 43538881       RX-dropped: 72            RX-total: 43538953
+     TX-packets: 299491753      TX-dropped: 0             TX-total: 299491753
+     ----------------------------------------------------------------------------
+
+     ---------------------- Forward statistics for port 1  ----------------------
+     RX-packets: 299493085      RX-dropped: 8357          RX-total: 299501442
+     TX-packets: 43539683       TX-dropped: 0             TX-total: 43539683
+     ----------------------------------------------------------------------------
+
+     +++++++++++++++ Accumulated forward statistics for all ports+++++++++++++++
+     RX-packets: 343031966      RX-dropped: 8429          RX-total: 343040395
+     TX-packets: 343031436      TX-dropped: 0             TX-total: 343031436
+     ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
+
+     CPU cycles/packet=14.28 (total cycles=4899533541 / total RX packets=343031966) at 200 MHz
+
+.. note::
+
+   Measuring CPU cycles/packet requires enabling CONFIG_RTE_TEST_PMD_RECORD_CORE_TX_CYCLES configuration option.
+   On aarch64 platforms, by default, the cycles are counted using generic counter which runs at a lower frequency than the CPU clock.
+   To get the cycles/packet at CPU clock frequency, please scale the cycles/packet to CPU clock frequency. Alternatively, use the PMU based cycle counter.
 
 clear fwd
 ~~~~~~~~~
-- 
2.20.1

[dpdk-dev] [PATCH 4/5] doc: include config options in testpmd user guide

[Dharmik Thakkar]

Update testpmd documentation to include RECORD configuration options,
CONFIG_RTE_TEST_PMD_RECORD_CORE_CYCLES and
CONFIG_RTE_TEST_PMD_RECORD_BURST_STATS.

Signed-off-by: Dharmik Thakkar <dharmik.thakkar@arm.com>
Reviewed-by: Honnappa Nagarahalli <honnappa.nagarahalli@arm.com>
Reviewed-by: Phil Yang <phil.yang@arm.com>
---
 doc/guides/testpmd_app_ug/build_app.rst | 12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/doc/guides/testpmd_app_ug/build_app.rst b/doc/guides/testpmd_app_ug/build_app.rst
index d1ca9f3d19a9..caebb33849e3 100644
--- a/doc/guides/testpmd_app_ug/build_app.rst
+++ b/doc/guides/testpmd_app_ug/build_app.rst
@@ -21,6 +21,18 @@ The basic compilation steps are:
 
         export RTE_TARGET=x86_64-native-linux-gcc
 
+##.  If required, enable configuration options. For example:
+
+    .. code-block:: console
+
+        cd to the top-level DPDK directory
+        sed -i 's,\(CONFIG_RTE_TEST_PMD_RECORD_CORE_CYCLES\)=n,\1=y,' config/common_base
+        sed -i 's,\(CONFIG_RTE_TEST_PMD_RECORD_BURST_STATS\)=n,\1=y,' config/common_base
+
+    Enabling CONFIG_RTE_TEST_PMD_RECORD_CORE_CYCLES enables measurement of CPU cycles.
+
+    Enabling CONFIG_RTE_TEST_PMD_RECORD_BURST_STATS enables display of RX and TX bursts.
+
 #.  Build the application:
 
     .. code-block:: console
-- 
2.20.1

[dpdk-dev] [PATCH 5/5] doc: add aarch64 generic counter section

[Dharmik Thakkar]

Add a separate section for low-resolution generic counter
for ARM64 profiling methods.

Signed-off-by: Dharmik Thakkar <dharmik.thakkar@arm.com>
Reviewed-by: Honnappa Nagarahalli <honnappa.nagarahalli@arm.com>
Reviewed-by: Phil Yang <phil.yang@arm.com>
---
 doc/guides/prog_guide/profile_app.rst | 11 ++++++++---
 1 file changed, 8 insertions(+), 3 deletions(-)

diff --git a/doc/guides/prog_guide/profile_app.rst b/doc/guides/prog_guide/profile_app.rst
index a36ebef4db99..e5d0e9079e11 100644
--- a/doc/guides/prog_guide/profile_app.rst
+++ b/doc/guides/prog_guide/profile_app.rst
@@ -57,11 +57,16 @@ For more derails refer to the
 `ARM64 specific PMU events enumeration <http://infocenter.arm.com/help/index.jsp?topic=/com.arm.doc.100095_0002_04_en/way1382543438508.html>`_.
 
 
-High-resolution cycle counter
-~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+Low-resolution generic counter
+~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
 
 The default ``cntvct_el0`` based ``rte_rdtsc()`` provides a portable means to
-get a wall clock counter in user space. Typically it runs at <= 100MHz.
+get a wall clock counter in user space. Typically it runs at a lower clock frequency than the CPU clock frequency.
+Cycles counted using this method should be scaled to CPU clock frequency.
+
+
+High-resolution cycle counter
+~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
 
 The alternative method to enable ``rte_rdtsc()`` for a high resolution wall
 clock counter is through the ARMv8 PMU subsystem. The PMU cycle counter runs
-- 
2.20.1

Re: [dpdk-dev] [PATCH 1/5] app/testpmd: print clock with CPU cycles per pkt

[Lukasz Wojciechowski]

W dniu 06.05.2020 o 23:58, Dharmik Thakkar pisze:
> On aarch64 platforms, the cycles are counted using either a
> low-resolution generic counter or a high-resolution PMU cycle counter.
> Print the clock frequency along with CPU cycles/packet to identify which
> cycle counter is being used.
>
> Signed-off-by: Dharmik Thakkar <dharmik.thakkar@arm.com>
> Reviewed-by: Honnappa Nagarahalli <honnappa.nagarahalli@arm.com>
> Reviewed-by: Phil Yang <phil.yang@arm.com>
> ---
>   app/test-pmd/testpmd.c | 5 +++--
>   1 file changed, 3 insertions(+), 2 deletions(-)
>
> diff --git a/app/test-pmd/testpmd.c b/app/test-pmd/testpmd.c
> index 99bacddbfdca..9a8cbbd6fc7c 100644
> --- a/app/test-pmd/testpmd.c
> +++ b/app/test-pmd/testpmd.c
> @@ -1953,11 +1953,12 @@ fwd_stats_display(void)
>   	       "%s\n",
>   	       acc_stats_border, acc_stats_border);
>   #ifdef RTE_TEST_PMD_RECORD_CORE_CYCLES
> +#define CYC_PER_MHZ 1E6
>   	if (total_recv > 0)
>   		printf("\n  CPU cycles/packet=%u (total cycles="
> -		       "%"PRIu64" / total RX packets=%"PRIu64")\n",
> +		       "%"PRIu64" / total RX packets=%"PRIu64") at %lu MHz Clock\n",
Please use PRIu64 instead of %lu to keep printing uint64_t portable for 
different hardware, OS and compilers.
>   		       (unsigned int)(fwd_cycles / total_recv),
> -		       fwd_cycles, total_recv);
> +		       fwd_cycles, total_recv, (uint64_t)(rte_get_tsc_hz() / CYC_PER_MHZ));
>   #endif
>   }
>   

-- 

Lukasz Wojciechowski
Principal Software Engineer

Samsung R&D Institute Poland
Samsung Electronics
Office +48 22 377 88 25
l.wojciechow@partner.samsung.com

Re: [dpdk-dev] [PATCH 5/5] doc: add aarch64 generic counter section

[Jerin Jacob]

On Thu, May 7, 2020 at 3:29 AM Dharmik Thakkar <dharmik.thakkar@arm.com> wrote:
>
> Add a separate section for low-resolution generic counter
> for ARM64 profiling methods.
>
> Signed-off-by: Dharmik Thakkar <dharmik.thakkar@arm.com>
> Reviewed-by: Honnappa Nagarahalli <honnappa.nagarahalli@arm.com>
> Reviewed-by: Phil Yang <phil.yang@arm.com>

Acked-by: Jerin Jacob <jerinj@marvell.com>



> ---
>  doc/guides/prog_guide/profile_app.rst | 11 ++++++++---
>  1 file changed, 8 insertions(+), 3 deletions(-)
>
> diff --git a/doc/guides/prog_guide/profile_app.rst b/doc/guides/prog_guide/profile_app.rst
> index a36ebef4db99..e5d0e9079e11 100644
> --- a/doc/guides/prog_guide/profile_app.rst
> +++ b/doc/guides/prog_guide/profile_app.rst
> @@ -57,11 +57,16 @@ For more derails refer to the
>  `ARM64 specific PMU events enumeration <http://infocenter.arm.com/help/index.jsp?topic=/com.arm.doc.100095_0002_04_en/way1382543438508.html>`_.
>
>
> -High-resolution cycle counter
> -~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> +Low-resolution generic counter
> +~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>
>  The default ``cntvct_el0`` based ``rte_rdtsc()`` provides a portable means to
> -get a wall clock counter in user space. Typically it runs at <= 100MHz.
> +get a wall clock counter in user space. Typically it runs at a lower clock frequency than the CPU clock frequency.
> +Cycles counted using this method should be scaled to CPU clock frequency.
> +
> +
> +High-resolution cycle counter
> +~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>
>  The alternative method to enable ``rte_rdtsc()`` for a high resolution wall
>  clock counter is through the ARMv8 PMU subsystem. The PMU cycle counter runs
> --
> 2.20.1
>

Re: [dpdk-dev] [PATCH 3/5] doc: add cycles per packet in testpmd user guide

[Jerin Jacob]

On Thu, May 7, 2020 at 3:30 AM Dharmik Thakkar <dharmik.thakkar@arm.com> wrote:
>
> Update documentation for 'show fwd' testpmd runtime function to show
> CPU cycles/packet example.
>
> Signed-off-by: Dharmik Thakkar <dharmik.thakkar@arm.com>
> Reviewed-by: Honnappa Nagarahalli <honnappa.nagarahalli@arm.com>
> Reviewed-by: Phil Yang <phil.yang@arm.com>
> ---
>  doc/guides/testpmd_app_ug/testpmd_funcs.rst | 51 +++++++++++++--------
>  1 file changed, 31 insertions(+), 20 deletions(-)
>
> diff --git a/doc/guides/testpmd_app_ug/testpmd_funcs.rst b/doc/guides/testpmd_app_ug/testpmd_funcs.rst
> index a360ecccfd3f..2e565ccdbcd4 100644
> --- a/doc/guides/testpmd_app_ug/testpmd_funcs.rst
> +++ b/doc/guides/testpmd_app_ug/testpmd_funcs.rst
> @@ -351,26 +351,37 @@ Example for the io forwarding engine, with some packet drops on the tx side::
>
>     testpmd> show fwd stats all
>
> -     ------- Forward Stats for RX Port= 0/Queue= 0 -> TX Port= 1/Queue= 0 -------
> -     RX-packets: 274293770      TX-packets: 274293642      TX-dropped: 128
> -
> -     ------- Forward Stats for RX Port= 1/Queue= 0 -> TX Port= 0/Queue= 0 -------
> -     RX-packets: 274301850      TX-packets: 274301850      TX-dropped: 0
> -
> -     ---------------------- Forward statistics for port 0  ----------------------
> -     RX-packets: 274293802      RX-dropped: 0             RX-total: 274293802
> -     TX-packets: 274301862      TX-dropped: 0             TX-total: 274301862
> -     ----------------------------------------------------------------------------
> -
> -     ---------------------- Forward statistics for port 1  ----------------------
> -     RX-packets: 274301894      RX-dropped: 0             RX-total: 274301894
> -     TX-packets: 274293706      TX-dropped: 128           TX-total: 274293834
> -     ----------------------------------------------------------------------------
> -
> -     +++++++++++++++ Accumulated forward statistics for all ports+++++++++++++++
> -     RX-packets: 548595696      RX-dropped: 0             RX-total: 548595696
> -     TX-packets: 548595568      TX-dropped: 128           TX-total: 548595696
> -     ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
> +     ------- Forward Stats for RX Port= 0/Queue= 0 -> TX Port= 1/Queue= 0 -------
> +     RX-packets: 43536504       TX-packets: 43536488       TX-dropped: 0
> +
> +     ------- Forward Stats for RX Port= 1/Queue= 0 -> TX Port= 0/Queue= 0 -------
> +     RX-packets: 149738504      TX-packets: 149738504      TX-dropped: 0
> +
> +     ------- Forward Stats for RX Port= 1/Queue= 1 -> TX Port= 0/Queue= 1 -------
> +     RX-packets: 149753052      TX-packets: 149753052      TX-dropped: 0
> +
> +     ---------------------- Forward statistics for port 0  ----------------------
> +     RX-packets: 43538881       RX-dropped: 72            RX-total: 43538953
> +     TX-packets: 299491753      TX-dropped: 0             TX-total: 299491753
> +     ----------------------------------------------------------------------------
> +
> +     ---------------------- Forward statistics for port 1  ----------------------
> +     RX-packets: 299493085      RX-dropped: 8357          RX-total: 299501442
> +     TX-packets: 43539683       TX-dropped: 0             TX-total: 43539683
> +     ----------------------------------------------------------------------------
> +
> +     +++++++++++++++ Accumulated forward statistics for all ports+++++++++++++++
> +     RX-packets: 343031966      RX-dropped: 8429          RX-total: 343040395
> +     TX-packets: 343031436      TX-dropped: 0             TX-total: 343031436
> +     ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
> +
> +     CPU cycles/packet=14.28 (total cycles=4899533541 / total RX packets=343031966) at 200 MHz
> +
> +.. note::
> +
> +   Measuring CPU cycles/packet requires enabling CONFIG_RTE_TEST_PMD_RECORD_CORE_TX_CYCLES configuration option.

IMO, it a very useful feature for CI/CD and development as we don't
need to see any Traffic generator to see any regression.

I think, enabling CONFIG_RTE_TEST_PMD_RECORD_CORE_TX_CYCLES
configuration is overhead to use in all the environments.

How about adding a "new forward" engine with and remove
CONFIG_RTE_TEST_PMD_RECORD_CORE_TX_CYCLES.
The new forward engine will have
CONFIG_RTE_TEST_PMD_RECORD_CORE_TX_CYCLES code enabled.
So the application can use the forward engine dynamically. aka the
same build works for both cases.


> +   On aarch64 platforms, by default, the cycles are counted using generic counter which runs at a lower frequency than the CPU clock.
> +   To get the cycles/packet at CPU clock frequency, please scale the cycles/packet to CPU clock frequency. Alternatively, use the PMU based cycle counter.
>
>  clear fwd
>  ~~~~~~~~~
> --
> 2.20.1
>

Re: [dpdk-dev] [PATCH 2/5] app/testpmd: print fractional part in CPU cycles

[Ananyev, Konstantin]

> -----Original Message-----
> From: dev <dev-bounces@dpdk.org> On Behalf Of Dharmik Thakkar
> Sent: Wednesday, May 6, 2020 10:59 PM
> To: Lu, Wenzhuo <wenzhuo.lu@intel.com>; Wu, Jingjing <jingjing.wu@intel.com>; Iremonger, Bernard <bernard.iremonger@intel.com>
> Cc: dev@dpdk.org; nd@arm.com; Dharmik Thakkar <dharmik.thakkar@arm.com>
> Subject: [dpdk-dev] [PATCH 2/5] app/testpmd: print fractional part in CPU cycles
> 
> Change printing of CPU cycles/packet to include fractional part for
> accurateness.
> 
> Example:
> 
> Without patch:
> CPU cycles/packet=14
> (total cycles=4899533541 / total RX packets=343031966)
> 
> With patch:
> CPU cycles/packet=14.28
> (total cycles=4899533541 / total RX packets=343031966)
> 
> Signed-off-by: Dharmik Thakkar <dharmik.thakkar@arm.com>
> Reviewed-by: Honnappa Nagarahalli <honnappa.nagarahalli@arm.com>
> Reviewed-by: Phil Yang <phil.yang@arm.com>
> ---
>  app/test-pmd/testpmd.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/app/test-pmd/testpmd.c b/app/test-pmd/testpmd.c
> index 9a8cbbd6fc7c..9444a730a153 100644
> --- a/app/test-pmd/testpmd.c
> +++ b/app/test-pmd/testpmd.c
> @@ -1955,9 +1955,9 @@ fwd_stats_display(void)
>  #ifdef RTE_TEST_PMD_RECORD_CORE_CYCLES
>  #define CYC_PER_MHZ 1E6
>  	if (total_recv > 0)
> -		printf("\n  CPU cycles/packet=%u (total cycles="
> +		printf("\n  CPU cycles/packet=%.2f (total cycles="
>  		       "%"PRIu64" / total RX packets=%"PRIu64") at %lu MHz Clock\n",
> -		       (unsigned int)(fwd_cycles / total_recv),
> +		       (double)(fwd_cycles / (double)total_recv),

Probably safer long double - to avoid overflow. 

>  		       fwd_cycles, total_recv, (uint64_t)(rte_get_tsc_hz() / CYC_PER_MHZ));
>  #endif
>  }
> --
> 2.20.1

Re: [dpdk-dev] [PATCH 1/5] app/testpmd: print clock with CPU cycles per pkt

[Iremonger, Bernard]

Hi Dharmik,

> -----Original Message-----
> From: Dharmik Thakkar <dharmik.thakkar@arm.com>
> Sent: Wednesday, May 6, 2020 10:59 PM
> To: Lu, Wenzhuo <wenzhuo.lu@intel.com>; Wu, Jingjing
> <jingjing.wu@intel.com>; Iremonger, Bernard
> <bernard.iremonger@intel.com>
> Cc: dev@dpdk.org; nd@arm.com; Dharmik Thakkar
> <dharmik.thakkar@arm.com>
> Subject: [PATCH 1/5] app/testpmd: print clock with CPU cycles per pkt
> 
> On aarch64 platforms, the cycles are counted using either a low-resolution
> generic counter or a high-resolution PMU cycle counter.
> Print the clock frequency along with CPU cycles/packet to identify which
> cycle counter is being used.
> 
> Signed-off-by: Dharmik Thakkar <dharmik.thakkar@arm.com>
> Reviewed-by: Honnappa Nagarahalli <honnappa.nagarahalli@arm.com>
> Reviewed-by: Phil Yang <phil.yang@arm.com>
> ---
>  app/test-pmd/testpmd.c | 5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)
> 
> diff --git a/app/test-pmd/testpmd.c b/app/test-pmd/testpmd.c index
> 99bacddbfdca..9a8cbbd6fc7c 100644
> --- a/app/test-pmd/testpmd.c
> +++ b/app/test-pmd/testpmd.c
> @@ -1953,11 +1953,12 @@ fwd_stats_display(void)
>  	       "%s\n",
>  	       acc_stats_border, acc_stats_border);  #ifdef
> RTE_TEST_PMD_RECORD_CORE_CYCLES
> +#define CYC_PER_MHZ 1E6
>  	if (total_recv > 0)
>  		printf("\n  CPU cycles/packet=%u (total cycles="
> -		       "%"PRIu64" / total RX packets=%"PRIu64")\n",
> +		       "%"PRIu64" / total RX packets=%"PRIu64") at %lu MHz
> Clock\n",

Better to use =%"PRIu64" instead of %lu in previous line

>  		       (unsigned int)(fwd_cycles / total_recv),
> -		       fwd_cycles, total_recv);
> +		       fwd_cycles, total_recv, (uint64_t)(rte_get_tsc_hz() /
> +CYC_PER_MHZ));
>  #endif
>  }
> 
> --
> 2.20.1
Better to fix checkpatch the following checkpatch warning:

WARNING:LONG_LINE: line over 80 characters
#77: FILE: app/test-pmd/testpmd.c:1961:
+                      fwd_cycles, total_recv, (uint64_t)(rte_get_tsc_hz() / CYC_PER_MHZ));

Regards,

Bernard.

Re: [dpdk-dev] [PATCH 3/5] doc: add cycles per packet in testpmd user guide

[Iremonger, Bernard]

Hi Dharmik,


> -----Original Message-----
> From: Dharmik Thakkar <dharmik.thakkar@arm.com>
> Sent: Wednesday, May 6, 2020 10:59 PM
> To: Lu, Wenzhuo <wenzhuo.lu@intel.com>; Wu, Jingjing
> <jingjing.wu@intel.com>; Iremonger, Bernard
> <bernard.iremonger@intel.com>; Mcnamara, John
> <john.mcnamara@intel.com>; Kovacevic, Marko
> <marko.kovacevic@intel.com>
> Cc: dev@dpdk.org; nd@arm.com; Dharmik Thakkar
> <dharmik.thakkar@arm.com>
> Subject: [PATCH 3/5] doc: add cycles per packet in testpmd user guide
> 
> Update documentation for 'show fwd' testpmd runtime function to show
> CPU cycles/packet example.
> 
> Signed-off-by: Dharmik Thakkar <dharmik.thakkar@arm.com>
> Reviewed-by: Honnappa Nagarahalli <honnappa.nagarahalli@arm.com>
> Reviewed-by: Phil Yang <phil.yang@arm.com>
> ---
>  doc/guides/testpmd_app_ug/testpmd_funcs.rst | 51 +++++++++++++------
> --
>  1 file changed, 31 insertions(+), 20 deletions(-)
> 
> diff --git a/doc/guides/testpmd_app_ug/testpmd_funcs.rst
> b/doc/guides/testpmd_app_ug/testpmd_funcs.rst
> index a360ecccfd3f..2e565ccdbcd4 100644
> --- a/doc/guides/testpmd_app_ug/testpmd_funcs.rst
> +++ b/doc/guides/testpmd_app_ug/testpmd_funcs.rst
> @@ -351,26 +351,37 @@ Example for the io forwarding engine, with some
> packet drops on the tx side::
> 
>     testpmd> show fwd stats all
> 
> -     ------- Forward Stats for RX Port= 0/Queue= 0 -> TX Port= 1/Queue= 0 -----
> --
> -     RX-packets: 274293770      TX-packets: 274293642      TX-dropped: 128
> -
> -     ------- Forward Stats for RX Port= 1/Queue= 0 -> TX Port= 0/Queue= 0 -----
> --
> -     RX-packets: 274301850      TX-packets: 274301850      TX-dropped: 0
> -
> -     ---------------------- Forward statistics for port 0  ----------------------
> -     RX-packets: 274293802      RX-dropped: 0             RX-total: 274293802
> -     TX-packets: 274301862      TX-dropped: 0             TX-total: 274301862
> -     ----------------------------------------------------------------------------
> -
> -     ---------------------- Forward statistics for port 1  ----------------------
> -     RX-packets: 274301894      RX-dropped: 0             RX-total: 274301894
> -     TX-packets: 274293706      TX-dropped: 128           TX-total: 274293834
> -     ----------------------------------------------------------------------------
> -
> -     +++++++++++++++ Accumulated forward statistics for all
> ports+++++++++++++++
> -     RX-packets: 548595696      RX-dropped: 0             RX-total: 548595696
> -     TX-packets: 548595568      TX-dropped: 128           TX-total: 548595696
> -
> ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
> ++++++++++++++++++
> +     ------- Forward Stats for RX Port= 0/Queue= 0 -> TX Port= 1/Queue= 0 ----
> ---
> +     RX-packets: 43536504       TX-packets: 43536488       TX-dropped:
> + 0
> +
> +     ------- Forward Stats for RX Port= 1/Queue= 0 -> TX Port= 0/Queue= 0 ----
> ---
> +     RX-packets: 149738504      TX-packets: 149738504      TX-dropped:
> + 0
> +
> +     ------- Forward Stats for RX Port= 1/Queue= 1 -> TX Port= 0/Queue= 1 ----
> ---
> +     RX-packets: 149753052      TX-packets: 149753052      TX-dropped:
> + 0
> +
> +     ---------------------- Forward statistics for port 0  ----------------------
> +     RX-packets: 43538881       RX-dropped: 72            RX-total: 43538953
> +     TX-packets: 299491753      TX-dropped: 0             TX-total: 299491753
> +
> + ----------------------------------------------------------------------
> + ------
> +
> +     ---------------------- Forward statistics for port 1  ----------------------
> +     RX-packets: 299493085      RX-dropped: 8357          RX-total: 299501442
> +     TX-packets: 43539683       TX-dropped: 0             TX-total: 43539683
> +
> + ----------------------------------------------------------------------
> + ------
> +
> +     +++++++++++++++ Accumulated forward statistics for all
> ports+++++++++++++++
> +     RX-packets: 343031966      RX-dropped: 8429          RX-total: 343040395
> +     TX-packets: 343031436      TX-dropped: 0             TX-total: 343031436
> +
> +
> ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
> ++++++++++++
> + ++++++
> +
> +     CPU cycles/packet=14.28 (total cycles=4899533541 / total RX
> + packets=343031966) at 200 MHz
> +
> +.. note::
> +
> +   Measuring CPU cycles/packet requires enabling
> CONFIG_RTE_TEST_PMD_RECORD_CORE_TX_CYCLES configuration option.
> +   On aarch64 platforms, by default, the cycles are counted using generic
> counter which runs at a lower frequency than the CPU clock.
> +   To get the cycles/packet at CPU clock frequency, please scale the
> cycles/packet to CPU clock frequency. Alternatively, use the PMU based
> cycle counter.

It is preferred to use shorter lines in .rst file, preferably  <= 80, up to 100 is acceptable.

> 
>  clear fwd
>  ~~~~~~~~~
> --
> 2.20.1

Regards,

Bernard.

Re: [dpdk-dev] [PATCH 4/5] doc: include config options in testpmd user guide

[Iremonger, Bernard]

Hi Dharmik,

> -----Original Message-----
> From: Dharmik Thakkar <dharmik.thakkar@arm.com>
> Sent: Wednesday, May 6, 2020 10:59 PM
> To: Lu, Wenzhuo <wenzhuo.lu@intel.com>; Wu, Jingjing
> <jingjing.wu@intel.com>; Iremonger, Bernard
> <bernard.iremonger@intel.com>; Mcnamara, John
> <john.mcnamara@intel.com>; Kovacevic, Marko
> <marko.kovacevic@intel.com>
> Cc: dev@dpdk.org; nd@arm.com; Dharmik Thakkar
> <dharmik.thakkar@arm.com>
> Subject: [PATCH 4/5] doc: include config options in testpmd user guide
> 
> Update testpmd documentation to include RECORD configuration options,
> CONFIG_RTE_TEST_PMD_RECORD_CORE_CYCLES and
> CONFIG_RTE_TEST_PMD_RECORD_BURST_STATS.
> 
> Signed-off-by: Dharmik Thakkar <dharmik.thakkar@arm.com>
> Reviewed-by: Honnappa Nagarahalli <honnappa.nagarahalli@arm.com>
> Reviewed-by: Phil Yang <phil.yang@arm.com>
> ---
>  doc/guides/testpmd_app_ug/build_app.rst | 12 ++++++++++++
>  1 file changed, 12 insertions(+)
> 
> diff --git a/doc/guides/testpmd_app_ug/build_app.rst
> b/doc/guides/testpmd_app_ug/build_app.rst
> index d1ca9f3d19a9..caebb33849e3 100644
> --- a/doc/guides/testpmd_app_ug/build_app.rst
> +++ b/doc/guides/testpmd_app_ug/build_app.rst
> @@ -21,6 +21,18 @@ The basic compilation steps are:
> 
>          export RTE_TARGET=x86_64-native-linux-gcc
> 
> +##.  If required, enable configuration options. For example:

##. Should be #.

> +
> +    .. code-block:: console
> +
> +        cd to the top-level DPDK directory
> +        sed -i 's,\(CONFIG_RTE_TEST_PMD_RECORD_CORE_CYCLES\)=n,\1=y,'
> config/common_base
> +        sed -i 's,\(CONFIG_RTE_TEST_PMD_RECORD_BURST_STATS\)=n,\1=y,'
> + config/common_base
> +
> +    Enabling CONFIG_RTE_TEST_PMD_RECORD_CORE_CYCLES enables
> measurement of CPU cycles.
> +
> +    Enabling CONFIG_RTE_TEST_PMD_RECORD_BURST_STATS enables
> display of RX and TX bursts.
> +
>  #.  Build the application:
> 
>      .. code-block:: console
> --
> 2.20.1

Regards,

Bernard.

Re: [dpdk-dev] [PATCH 2/5] app/testpmd: print fractional part in CPU cycles

[Dharmik Thakkar]

Hi Konstantin,

> On May 7, 2020, at 4:50 AM, Ananyev, Konstantin <konstantin.ananyev@intel.com> wrote:
> 
> 
> 
>> -----Original Message-----
>> From: dev <dev-bounces@dpdk.org> On Behalf Of Dharmik Thakkar
>> Sent: Wednesday, May 6, 2020 10:59 PM
>> To: Lu, Wenzhuo <wenzhuo.lu@intel.com>; Wu, Jingjing <jingjing.wu@intel.com>; Iremonger, Bernard <bernard.iremonger@intel.com>
>> Cc: dev@dpdk.org; nd@arm.com; Dharmik Thakkar <dharmik.thakkar@arm.com>
>> Subject: [dpdk-dev] [PATCH 2/5] app/testpmd: print fractional part in CPU cycles
>> 
>> Change printing of CPU cycles/packet to include fractional part for
>> accurateness.
>> 
>> Example:
>> 
>> Without patch:
>> CPU cycles/packet=14
>> (total cycles=4899533541 / total RX packets=343031966)
>> 
>> With patch:
>> CPU cycles/packet=14.28
>> (total cycles=4899533541 / total RX packets=343031966)
>> 
>> Signed-off-by: Dharmik Thakkar <dharmik.thakkar@arm.com>
>> Reviewed-by: Honnappa Nagarahalli <honnappa.nagarahalli@arm.com>
>> Reviewed-by: Phil Yang <phil.yang@arm.com>
>> ---
>> app/test-pmd/testpmd.c | 4 ++--
>> 1 file changed, 2 insertions(+), 2 deletions(-)
>> 
>> diff --git a/app/test-pmd/testpmd.c b/app/test-pmd/testpmd.c
>> index 9a8cbbd6fc7c..9444a730a153 100644
>> --- a/app/test-pmd/testpmd.c
>> +++ b/app/test-pmd/testpmd.c
>> @@ -1955,9 +1955,9 @@ fwd_stats_display(void)
>> #ifdef RTE_TEST_PMD_RECORD_CORE_CYCLES
>> #define CYC_PER_MHZ 1E6
>> 	if (total_recv > 0)
>> -		printf("\n  CPU cycles/packet=%u (total cycles="
>> +		printf("\n  CPU cycles/packet=%.2f (total cycles="
>> 		       "%"PRIu64" / total RX packets=%"PRIu64") at %lu MHz Clock\n",
>> -		       (unsigned int)(fwd_cycles / total_recv),
>> +		       (double)(fwd_cycles / (double)total_recv),
> 
> Probably safer long double - to avoid overflow. 

Is it possible for a ‘double' to be less than 8 bytes?

> 
>> 		       fwd_cycles, total_recv, (uint64_t)(rte_get_tsc_hz() / CYC_PER_MHZ));
>> #endif
>> }
>> --
>> 2.20.1

Re: [dpdk-dev] [PATCH 3/5] doc: add cycles per packet in testpmd user guide

[Dharmik Thakkar]

Hi Jerin,

> On May 7, 2020, at 12:24 AM, Jerin Jacob <jerinjacobk@gmail.com> wrote:
> 
> On Thu, May 7, 2020 at 3:30 AM Dharmik Thakkar <dharmik.thakkar@arm.com> wrote:
>> 
>> Update documentation for 'show fwd' testpmd runtime function to show
>> CPU cycles/packet example.
>> 
>> Signed-off-by: Dharmik Thakkar <dharmik.thakkar@arm.com>
>> Reviewed-by: Honnappa Nagarahalli <honnappa.nagarahalli@arm.com>
>> Reviewed-by: Phil Yang <phil.yang@arm.com>
>> ---
>> doc/guides/testpmd_app_ug/testpmd_funcs.rst | 51 +++++++++++++--------
>> 1 file changed, 31 insertions(+), 20 deletions(-)
>> 
>> diff --git a/doc/guides/testpmd_app_ug/testpmd_funcs.rst b/doc/guides/testpmd_app_ug/testpmd_funcs.rst
>> index a360ecccfd3f..2e565ccdbcd4 100644
>> --- a/doc/guides/testpmd_app_ug/testpmd_funcs.rst
>> +++ b/doc/guides/testpmd_app_ug/testpmd_funcs.rst
>> @@ -351,26 +351,37 @@ Example for the io forwarding engine, with some packet drops on the tx side::
>> 
>>    testpmd> show fwd stats all
>> 
>> -     ------- Forward Stats for RX Port= 0/Queue= 0 -> TX Port= 1/Queue= 0 -------
>> -     RX-packets: 274293770      TX-packets: 274293642      TX-dropped: 128
>> -
>> -     ------- Forward Stats for RX Port= 1/Queue= 0 -> TX Port= 0/Queue= 0 -------
>> -     RX-packets: 274301850      TX-packets: 274301850      TX-dropped: 0
>> -
>> -     ---------------------- Forward statistics for port 0  ----------------------
>> -     RX-packets: 274293802      RX-dropped: 0             RX-total: 274293802
>> -     TX-packets: 274301862      TX-dropped: 0             TX-total: 274301862
>> -     ----------------------------------------------------------------------------
>> -
>> -     ---------------------- Forward statistics for port 1  ----------------------
>> -     RX-packets: 274301894      RX-dropped: 0             RX-total: 274301894
>> -     TX-packets: 274293706      TX-dropped: 128           TX-total: 274293834
>> -     ----------------------------------------------------------------------------
>> -
>> -     +++++++++++++++ Accumulated forward statistics for all ports+++++++++++++++
>> -     RX-packets: 548595696      RX-dropped: 0             RX-total: 548595696
>> -     TX-packets: 548595568      TX-dropped: 128           TX-total: 548595696
>> -     ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
>> +     ------- Forward Stats for RX Port= 0/Queue= 0 -> TX Port= 1/Queue= 0 -------
>> +     RX-packets: 43536504       TX-packets: 43536488       TX-dropped: 0
>> +
>> +     ------- Forward Stats for RX Port= 1/Queue= 0 -> TX Port= 0/Queue= 0 -------
>> +     RX-packets: 149738504      TX-packets: 149738504      TX-dropped: 0
>> +
>> +     ------- Forward Stats for RX Port= 1/Queue= 1 -> TX Port= 0/Queue= 1 -------
>> +     RX-packets: 149753052      TX-packets: 149753052      TX-dropped: 0
>> +
>> +     ---------------------- Forward statistics for port 0  ----------------------
>> +     RX-packets: 43538881       RX-dropped: 72            RX-total: 43538953
>> +     TX-packets: 299491753      TX-dropped: 0             TX-total: 299491753
>> +     ----------------------------------------------------------------------------
>> +
>> +     ---------------------- Forward statistics for port 1  ----------------------
>> +     RX-packets: 299493085      RX-dropped: 8357          RX-total: 299501442
>> +     TX-packets: 43539683       TX-dropped: 0             TX-total: 43539683
>> +     ----------------------------------------------------------------------------
>> +
>> +     +++++++++++++++ Accumulated forward statistics for all ports+++++++++++++++
>> +     RX-packets: 343031966      RX-dropped: 8429          RX-total: 343040395
>> +     TX-packets: 343031436      TX-dropped: 0             TX-total: 343031436
>> +     ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
>> +
>> +     CPU cycles/packet=14.28 (total cycles=4899533541 / total RX packets=343031966) at 200 MHz
>> +
>> +.. note::
>> +
>> +   Measuring CPU cycles/packet requires enabling CONFIG_RTE_TEST_PMD_RECORD_CORE_TX_CYCLES configuration option.
> 
> IMO, it a very useful feature for CI/CD and development as we don't
> need to see any Traffic generator to see any regression.
> 
> I think, enabling CONFIG_RTE_TEST_PMD_RECORD_CORE_TX_CYCLES
> configuration is overhead to use in all the environments.
> 
> How about adding a "new forward" engine with and remove
> CONFIG_RTE_TEST_PMD_RECORD_CORE_TX_CYCLES.
> The new forward engine will have
> CONFIG_RTE_TEST_PMD_RECORD_CORE_TX_CYCLES code enabled.
> So the application can use the forward engine dynamically. aka the
> same build works for both cases.

Yes, this can be done as a separate patch.
The idea of this patch is to update the documentation with the missing CPU cycles/packet information.

> 
> 
>> +   On aarch64 platforms, by default, the cycles are counted using generic counter which runs at a lower frequency than the CPU clock.
>> +   To get the cycles/packet at CPU clock frequency, please scale the cycles/packet to CPU clock frequency. Alternatively, use the PMU based cycle counter.
>> 
>> clear fwd
>> ~~~~~~~~~
>> --
>> 2.20.1

Re: [dpdk-dev] [PATCH 2/5] app/testpmd: print fractional part in CPU cycles

[Ananyev, Konstantin]

> Hi Konstantin,
> 
> > On May 7, 2020, at 4:50 AM, Ananyev, Konstantin <konstantin.ananyev@intel.com> wrote:
> >
> >
> >
> >> -----Original Message-----
> >> From: dev <dev-bounces@dpdk.org> On Behalf Of Dharmik Thakkar
> >> Sent: Wednesday, May 6, 2020 10:59 PM
> >> To: Lu, Wenzhuo <wenzhuo.lu@intel.com>; Wu, Jingjing <jingjing.wu@intel.com>; Iremonger, Bernard <bernard.iremonger@intel.com>
> >> Cc: dev@dpdk.org; nd@arm.com; Dharmik Thakkar <dharmik.thakkar@arm.com>
> >> Subject: [dpdk-dev] [PATCH 2/5] app/testpmd: print fractional part in CPU cycles
> >>
> >> Change printing of CPU cycles/packet to include fractional part for
> >> accurateness.
> >>
> >> Example:
> >>
> >> Without patch:
> >> CPU cycles/packet=14
> >> (total cycles=4899533541 / total RX packets=343031966)
> >>
> >> With patch:
> >> CPU cycles/packet=14.28
> >> (total cycles=4899533541 / total RX packets=343031966)
> >>
> >> Signed-off-by: Dharmik Thakkar <dharmik.thakkar@arm.com>
> >> Reviewed-by: Honnappa Nagarahalli <honnappa.nagarahalli@arm.com>
> >> Reviewed-by: Phil Yang <phil.yang@arm.com>
> >> ---
> >> app/test-pmd/testpmd.c | 4 ++--
> >> 1 file changed, 2 insertions(+), 2 deletions(-)
> >>
> >> diff --git a/app/test-pmd/testpmd.c b/app/test-pmd/testpmd.c
> >> index 9a8cbbd6fc7c..9444a730a153 100644
> >> --- a/app/test-pmd/testpmd.c
> >> +++ b/app/test-pmd/testpmd.c
> >> @@ -1955,9 +1955,9 @@ fwd_stats_display(void)
> >> #ifdef RTE_TEST_PMD_RECORD_CORE_CYCLES
> >> #define CYC_PER_MHZ 1E6
> >> 	if (total_recv > 0)
> >> -		printf("\n  CPU cycles/packet=%u (total cycles="
> >> +		printf("\n  CPU cycles/packet=%.2f (total cycles="
> >> 		       "%"PRIu64" / total RX packets=%"PRIu64") at %lu MHz Clock\n",
> >> -		       (unsigned int)(fwd_cycles / total_recv),
> >> +		       (double)(fwd_cycles / (double)total_recv),
> >
> > Probably safer long double - to avoid overflow.
> 
> Is it possible for a ‘double' to be less than 8 bytes?

That was my initial thought - that on some 32 bit systems it could be 4B.
Though it seems I was wrong, so feel free to ignore.
BTW, what for double conversion, why not just:
double)(fwd_cycles /total_recv
?



> 
> >
> >> 		       fwd_cycles, total_recv, (uint64_t)(rte_get_tsc_hz() / CYC_PER_MHZ));
> >> #endif
> >> }
> >> --
> >> 2.20.1

Re: [dpdk-dev] [PATCH 2/5] app/testpmd: print fractional part in CPU cycles

[Dharmik Thakkar]

> On May 8, 2020, at 12:17 PM, Ananyev, Konstantin <konstantin.ananyev@intel.com> wrote:
> 
>> Hi Konstantin,
>> 
>>> On May 7, 2020, at 4:50 AM, Ananyev, Konstantin <konstantin.ananyev@intel.com> wrote:
>>> 
>>> 
>>> 
>>>> -----Original Message-----
>>>> From: dev <dev-bounces@dpdk.org> On Behalf Of Dharmik Thakkar
>>>> Sent: Wednesday, May 6, 2020 10:59 PM
>>>> To: Lu, Wenzhuo <wenzhuo.lu@intel.com>; Wu, Jingjing <jingjing.wu@intel.com>; Iremonger, Bernard <bernard.iremonger@intel.com>
>>>> Cc: dev@dpdk.org; nd@arm.com; Dharmik Thakkar <dharmik.thakkar@arm.com>
>>>> Subject: [dpdk-dev] [PATCH 2/5] app/testpmd: print fractional part in CPU cycles
>>>> 
>>>> Change printing of CPU cycles/packet to include fractional part for
>>>> accurateness.
>>>> 
>>>> Example:
>>>> 
>>>> Without patch:
>>>> CPU cycles/packet=14
>>>> (total cycles=4899533541 / total RX packets=343031966)
>>>> 
>>>> With patch:
>>>> CPU cycles/packet=14.28
>>>> (total cycles=4899533541 / total RX packets=343031966)
>>>> 
>>>> Signed-off-by: Dharmik Thakkar <dharmik.thakkar@arm.com>
>>>> Reviewed-by: Honnappa Nagarahalli <honnappa.nagarahalli@arm.com>
>>>> Reviewed-by: Phil Yang <phil.yang@arm.com>
>>>> ---
>>>> app/test-pmd/testpmd.c | 4 ++--
>>>> 1 file changed, 2 insertions(+), 2 deletions(-)
>>>> 
>>>> diff --git a/app/test-pmd/testpmd.c b/app/test-pmd/testpmd.c
>>>> index 9a8cbbd6fc7c..9444a730a153 100644
>>>> --- a/app/test-pmd/testpmd.c
>>>> +++ b/app/test-pmd/testpmd.c
>>>> @@ -1955,9 +1955,9 @@ fwd_stats_display(void)
>>>> #ifdef RTE_TEST_PMD_RECORD_CORE_CYCLES
>>>> #define CYC_PER_MHZ 1E6
>>>> 	if (total_recv > 0)
>>>> -		printf("\n  CPU cycles/packet=%u (total cycles="
>>>> +		printf("\n  CPU cycles/packet=%.2f (total cycles="
>>>> 		       "%"PRIu64" / total RX packets=%"PRIu64") at %lu MHz Clock\n",
>>>> -		       (unsigned int)(fwd_cycles / total_recv),
>>>> +		       (double)(fwd_cycles / (double)total_recv),
>>> 
>>> Probably safer long double - to avoid overflow.
>> 
>> Is it possible for a ‘double' to be less than 8 bytes?
> 
> That was my initial thought - that on some 32 bit systems it could be 4B.
> Though it seems I was wrong, so feel free to ignore.
> BTW, what for double conversion, why not just:
> double)(fwd_cycles /total_recv
> ?

Without (double) total recv, I will always get the fractional part as .00.
For the above example, with (double)(fwd_cycles / total_recv), I see 14.00 instead of 14.28

> 
> 
> 
>> 
>>> 
>>>> 		       fwd_cycles, total_recv, (uint64_t)(rte_get_tsc_hz() / CYC_PER_MHZ));
>>>> #endif
>>>> }
>>>> --
>>>> 2.20.1

Re: [dpdk-dev] [PATCH 2/5] app/testpmd: print fractional part in CPU cycles

[Ananyev, Konstantin]

> -----Original Message-----
> From: Dharmik Thakkar <Dharmik.Thakkar@arm.com>
> Sent: Friday, May 8, 2020 6:37 PM
> To: Ananyev, Konstantin <konstantin.ananyev@intel.com>
> Cc: Lu, Wenzhuo <wenzhuo.lu@intel.com>; Wu, Jingjing <jingjing.wu@intel.com>; Iremonger, Bernard <bernard.iremonger@intel.com>;
> dev@dpdk.org; nd <nd@arm.com>
> Subject: Re: [dpdk-dev] [PATCH 2/5] app/testpmd: print fractional part in CPU cycles
> 
> 
> 
> > On May 8, 2020, at 12:17 PM, Ananyev, Konstantin <konstantin.ananyev@intel.com> wrote:
> >
> >> Hi Konstantin,
> >>
> >>> On May 7, 2020, at 4:50 AM, Ananyev, Konstantin <konstantin.ananyev@intel.com> wrote:
> >>>
> >>>
> >>>
> >>>> -----Original Message-----
> >>>> From: dev <dev-bounces@dpdk.org> On Behalf Of Dharmik Thakkar
> >>>> Sent: Wednesday, May 6, 2020 10:59 PM
> >>>> To: Lu, Wenzhuo <wenzhuo.lu@intel.com>; Wu, Jingjing <jingjing.wu@intel.com>; Iremonger, Bernard
> <bernard.iremonger@intel.com>
> >>>> Cc: dev@dpdk.org; nd@arm.com; Dharmik Thakkar <dharmik.thakkar@arm.com>
> >>>> Subject: [dpdk-dev] [PATCH 2/5] app/testpmd: print fractional part in CPU cycles
> >>>>
> >>>> Change printing of CPU cycles/packet to include fractional part for
> >>>> accurateness.
> >>>>
> >>>> Example:
> >>>>
> >>>> Without patch:
> >>>> CPU cycles/packet=14
> >>>> (total cycles=4899533541 / total RX packets=343031966)
> >>>>
> >>>> With patch:
> >>>> CPU cycles/packet=14.28
> >>>> (total cycles=4899533541 / total RX packets=343031966)
> >>>>
> >>>> Signed-off-by: Dharmik Thakkar <dharmik.thakkar@arm.com>
> >>>> Reviewed-by: Honnappa Nagarahalli <honnappa.nagarahalli@arm.com>
> >>>> Reviewed-by: Phil Yang <phil.yang@arm.com>
> >>>> ---
> >>>> app/test-pmd/testpmd.c | 4 ++--
> >>>> 1 file changed, 2 insertions(+), 2 deletions(-)
> >>>>
> >>>> diff --git a/app/test-pmd/testpmd.c b/app/test-pmd/testpmd.c
> >>>> index 9a8cbbd6fc7c..9444a730a153 100644
> >>>> --- a/app/test-pmd/testpmd.c
> >>>> +++ b/app/test-pmd/testpmd.c
> >>>> @@ -1955,9 +1955,9 @@ fwd_stats_display(void)
> >>>> #ifdef RTE_TEST_PMD_RECORD_CORE_CYCLES
> >>>> #define CYC_PER_MHZ 1E6
> >>>> 	if (total_recv > 0)
> >>>> -		printf("\n  CPU cycles/packet=%u (total cycles="
> >>>> +		printf("\n  CPU cycles/packet=%.2f (total cycles="
> >>>> 		       "%"PRIu64" / total RX packets=%"PRIu64") at %lu MHz Clock\n",
> >>>> -		       (unsigned int)(fwd_cycles / total_recv),
> >>>> +		       (double)(fwd_cycles / (double)total_recv),
> >>>
> >>> Probably safer long double - to avoid overflow.
> >>
> >> Is it possible for a ‘double' to be less than 8 bytes?
> >
> > That was my initial thought - that on some 32 bit systems it could be 4B.
> > Though it seems I was wrong, so feel free to ignore.
> > BTW, what for double conversion, why not just:
> > double)(fwd_cycles /total_recv
> > ?
> 
> Without (double) total recv, I will always get the fractional part as .00.
> For the above example, with (double)(fwd_cycles / total_recv), I see 14.00 instead of 14.28

AFAIK, that's because that way you told compiler to do integer division and then convert result to double.
While (double)fwd_cycles /total_recv, will tell compiler to explicitly convert fwd_cycles to double
and compiler will implicitly do the same for total_recv too, and result will be double too.


> 
> >
> >
> >
> >>
> >>>
> >>>> 		       fwd_cycles, total_recv, (uint64_t)(rte_get_tsc_hz() / CYC_PER_MHZ));
> >>>> #endif
> >>>> }
> >>>> --
> >>>> 2.20.1

[dpdk-dev] [PATCH v2 1/5] app/testpmd: print clock with CPU cycles per pkt

[Dharmik Thakkar]

On aarch64 platforms, the cycles are counted using either a
low-resolution generic counter or a high-resolution PMU cycle counter.
Print the clock frequency along with CPU cycles/packet to identify which
cycle counter is being used.

Signed-off-by: Dharmik Thakkar <dharmik.thakkar@arm.com>
Reviewed-by: Honnappa Nagarahalli <honnappa.nagarahalli@arm.com>
Reviewed-by: Phil Yang <phil.yang@arm.com>
---
v2:
 - Use %"PRIu64" instead of %lu
 - Fix checkpatch warning

---
 app/test-pmd/testpmd.c | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/app/test-pmd/testpmd.c b/app/test-pmd/testpmd.c
index 99bacddbfdca..806abb805879 100644
--- a/app/test-pmd/testpmd.c
+++ b/app/test-pmd/testpmd.c
@@ -1953,11 +1953,14 @@ fwd_stats_display(void)
 	       "%s\n",
 	       acc_stats_border, acc_stats_border);
 #ifdef RTE_TEST_PMD_RECORD_CORE_CYCLES
+#define CYC_PER_MHZ 1E6
 	if (total_recv > 0)
 		printf("\n  CPU cycles/packet=%u (total cycles="
-		       "%"PRIu64" / total RX packets=%"PRIu64")\n",
+		       "%"PRIu64" / total RX packets=%"PRIu64") at %"PRIu64
+		       " MHz Clock\n",
 		       (unsigned int)(fwd_cycles / total_recv),
-		       fwd_cycles, total_recv);
+		       fwd_cycles, total_recv,
+		       (uint64_t)(rte_get_tsc_hz() / CYC_PER_MHZ));
 #endif
 }
 
-- 
2.20.1

[dpdk-dev] [PATCH v2 2/5] app/testpmd: print fractional part in CPU cycles

[Dharmik Thakkar]

Change printing of CPU cycles/packet to include fractional part for
accurateness.

Example:

Without patch:
CPU cycles/packet=14
(total cycles=4899533541 / total RX packets=343031966)

With patch:
CPU cycles/packet=14.28
(total cycles=4899533541 / total RX packets=343031966)

Signed-off-by: Dharmik Thakkar <dharmik.thakkar@arm.com>
Reviewed-by: Honnappa Nagarahalli <honnappa.nagarahalli@arm.com>
Reviewed-by: Phil Yang <phil.yang@arm.com>
---
v2:
 - Change typecast to (double) fwd_cycles / total_recv

---
 app/test-pmd/testpmd.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/app/test-pmd/testpmd.c b/app/test-pmd/testpmd.c
index 806abb805879..a7ac6c66ce1c 100644
--- a/app/test-pmd/testpmd.c
+++ b/app/test-pmd/testpmd.c
@@ -1955,10 +1955,10 @@ fwd_stats_display(void)
 #ifdef RTE_TEST_PMD_RECORD_CORE_CYCLES
 #define CYC_PER_MHZ 1E6
 	if (total_recv > 0)
-		printf("\n  CPU cycles/packet=%u (total cycles="
+		printf("\n  CPU cycles/packet=%.2F (total cycles="
 		       "%"PRIu64" / total RX packets=%"PRIu64") at %"PRIu64
 		       " MHz Clock\n",
-		       (unsigned int)(fwd_cycles / total_recv),
+		       (double) fwd_cycles / total_recv,
 		       fwd_cycles, total_recv,
 		       (uint64_t)(rte_get_tsc_hz() / CYC_PER_MHZ));
 #endif
-- 
2.20.1

[dpdk-dev] [PATCH v2 3/5] doc: add cycles per packet in testpmd user guide

[Dharmik Thakkar]

Update documentation for 'show fwd' testpmd runtime function to show
CPU cycles/packet example.

Signed-off-by: Dharmik Thakkar <dharmik.thakkar@arm.com>
Reviewed-by: Honnappa Nagarahalli <honnappa.nagarahalli@arm.com>
Reviewed-by: Phil Yang <phil.yang@arm.com>
---
v2:
 - Use shorter lines, up to 100.
---
 doc/guides/testpmd_app_ug/testpmd_funcs.rst | 53 +++++++++++++--------
 1 file changed, 33 insertions(+), 20 deletions(-)

diff --git a/doc/guides/testpmd_app_ug/testpmd_funcs.rst b/doc/guides/testpmd_app_ug/testpmd_funcs.rst
index a360ecccfd3f..441ed41e3803 100644
--- a/doc/guides/testpmd_app_ug/testpmd_funcs.rst
+++ b/doc/guides/testpmd_app_ug/testpmd_funcs.rst
@@ -351,26 +351,39 @@ Example for the io forwarding engine, with some packet drops on the tx side::
 
    testpmd> show fwd stats all
 
-     ------- Forward Stats for RX Port= 0/Queue= 0 -> TX Port= 1/Queue= 0 -------
-     RX-packets: 274293770      TX-packets: 274293642      TX-dropped: 128
-
-     ------- Forward Stats for RX Port= 1/Queue= 0 -> TX Port= 0/Queue= 0 -------
-     RX-packets: 274301850      TX-packets: 274301850      TX-dropped: 0
-
-     ---------------------- Forward statistics for port 0  ----------------------
-     RX-packets: 274293802      RX-dropped: 0             RX-total: 274293802
-     TX-packets: 274301862      TX-dropped: 0             TX-total: 274301862
-     ----------------------------------------------------------------------------
-
-     ---------------------- Forward statistics for port 1  ----------------------
-     RX-packets: 274301894      RX-dropped: 0             RX-total: 274301894
-     TX-packets: 274293706      TX-dropped: 128           TX-total: 274293834
-     ----------------------------------------------------------------------------
-
-     +++++++++++++++ Accumulated forward statistics for all ports+++++++++++++++
-     RX-packets: 548595696      RX-dropped: 0             RX-total: 548595696
-     TX-packets: 548595568      TX-dropped: 128           TX-total: 548595696
-     ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
+     ------- Forward Stats for RX Port= 0/Queue= 0 -> TX Port= 1/Queue= 0 -------
+     RX-packets: 43536504       TX-packets: 43536488       TX-dropped: 0
+
+     ------- Forward Stats for RX Port= 1/Queue= 0 -> TX Port= 0/Queue= 0 -------
+     RX-packets: 149738504      TX-packets: 149738504      TX-dropped: 0
+
+     ------- Forward Stats for RX Port= 1/Queue= 1 -> TX Port= 0/Queue= 1 -------
+     RX-packets: 149753052      TX-packets: 149753052      TX-dropped: 0
+
+     ---------------------- Forward statistics for port 0  ----------------------
+     RX-packets: 43538881       RX-dropped: 72            RX-total: 43538953
+     TX-packets: 299491753      TX-dropped: 0             TX-total: 299491753
+     ----------------------------------------------------------------------------
+
+     ---------------------- Forward statistics for port 1  ----------------------
+     RX-packets: 299493085      RX-dropped: 8357          RX-total: 299501442
+     TX-packets: 43539683       TX-dropped: 0             TX-total: 43539683
+     ----------------------------------------------------------------------------
+
+     +++++++++++++++ Accumulated forward statistics for all ports+++++++++++++++
+     RX-packets: 343031966      RX-dropped: 8429          RX-total: 343040395
+     TX-packets: 343031436      TX-dropped: 0             TX-total: 343031436
+     ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
+
+     CPU cycles/packet=14.28 (total cycles=4899533541 / total RX packets=343031966) at 200 MHz Clock
+
+.. note::
+
+   Measuring CPU cycles/packet requires enabling CONFIG_RTE_TEST_PMD_RECORD_CORE_TX_CYCLES
+   configuration option. On aarch64 platforms, by default, the cycles are counted using
+   generic counter which runs at a lower frequency than the CPU clock. To get the cycles/packet
+   at CPU clock frequency, please scale the cycles/packet to CPU clock frequency.
+   Alternatively, use the PMU based cycle counter.
 
 clear fwd
 ~~~~~~~~~
-- 
2.20.1

[dpdk-dev] [PATCH v2 4/5] doc: include config options in testpmd user guide

[Dharmik Thakkar]

Update testpmd documentation to include RECORD configuration options,
CONFIG_RTE_TEST_PMD_RECORD_CORE_CYCLES and
CONFIG_RTE_TEST_PMD_RECORD_BURST_STATS.

Signed-off-by: Dharmik Thakkar <dharmik.thakkar@arm.com>
Reviewed-by: Honnappa Nagarahalli <honnappa.nagarahalli@arm.com>
Reviewed-by: Phil Yang <phil.yang@arm.com>
---
v2:
 - Remove extra '#'.
---
 doc/guides/testpmd_app_ug/build_app.rst | 12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/doc/guides/testpmd_app_ug/build_app.rst b/doc/guides/testpmd_app_ug/build_app.rst
index d1ca9f3d19a9..b93c82b8c115 100644
--- a/doc/guides/testpmd_app_ug/build_app.rst
+++ b/doc/guides/testpmd_app_ug/build_app.rst
@@ -21,6 +21,18 @@ The basic compilation steps are:
 
         export RTE_TARGET=x86_64-native-linux-gcc
 
+#.  If required, enable configuration options. For example:
+
+    .. code-block:: console
+
+        cd to the top-level DPDK directory
+        sed -i 's,\(CONFIG_RTE_TEST_PMD_RECORD_CORE_CYCLES\)=n,\1=y,' config/common_base
+        sed -i 's,\(CONFIG_RTE_TEST_PMD_RECORD_BURST_STATS\)=n,\1=y,' config/common_base
+
+    Enabling CONFIG_RTE_TEST_PMD_RECORD_CORE_CYCLES enables measurement of CPU cycles.
+
+    Enabling CONFIG_RTE_TEST_PMD_RECORD_BURST_STATS enables display of RX and TX bursts.
+
 #.  Build the application:
 
     .. code-block:: console
-- 
2.20.1

[dpdk-dev] [PATCH v2 5/5] doc: add aarch64 generic counter section

[Dharmik Thakkar]

Add a separate section for low-resolution generic counter
for ARM64 profiling methods.

Signed-off-by: Dharmik Thakkar <dharmik.thakkar@arm.com>
Reviewed-by: Honnappa Nagarahalli <honnappa.nagarahalli@arm.com>
Reviewed-by: Phil Yang <phil.yang@arm.com>
Acked-by: Jerin Jacob <jerinj@marvell.com>
---
v2:
 - Add Acked-by tag
---
 doc/guides/prog_guide/profile_app.rst | 11 ++++++++---
 1 file changed, 8 insertions(+), 3 deletions(-)

diff --git a/doc/guides/prog_guide/profile_app.rst b/doc/guides/prog_guide/profile_app.rst
index a36ebef4db99..e5d0e9079e11 100644
--- a/doc/guides/prog_guide/profile_app.rst
+++ b/doc/guides/prog_guide/profile_app.rst
@@ -57,11 +57,16 @@ For more derails refer to the
 `ARM64 specific PMU events enumeration <http://infocenter.arm.com/help/index.jsp?topic=/com.arm.doc.100095_0002_04_en/way1382543438508.html>`_.
 
 
-High-resolution cycle counter
-~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+Low-resolution generic counter
+~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
 
 The default ``cntvct_el0`` based ``rte_rdtsc()`` provides a portable means to
-get a wall clock counter in user space. Typically it runs at <= 100MHz.
+get a wall clock counter in user space. Typically it runs at a lower clock frequency than the CPU clock frequency.
+Cycles counted using this method should be scaled to CPU clock frequency.
+
+
+High-resolution cycle counter
+~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
 
 The alternative method to enable ``rte_rdtsc()`` for a high resolution wall
 clock counter is through the ARMv8 PMU subsystem. The PMU cycle counter runs
-- 
2.20.1

Re: [dpdk-dev] [PATCH v2 1/5] app/testpmd: print clock with CPU cycles per pkt

[Iremonger, Bernard]

> -----Original Message-----
> From: Dharmik Thakkar <dharmik.thakkar@arm.com>
> Sent: Friday, May 8, 2020 11:38 PM
> To: Lu, Wenzhuo <wenzhuo.lu@intel.com>; Wu, Jingjing
> <jingjing.wu@intel.com>; Iremonger, Bernard
> <bernard.iremonger@intel.com>
> Cc: dev@dpdk.org; nd@arm.com; Dharmik Thakkar
> <dharmik.thakkar@arm.com>
> Subject: [PATCH v2 1/5] app/testpmd: print clock with CPU cycles per pkt
> 
> On aarch64 platforms, the cycles are counted using either a low-resolution
> generic counter or a high-resolution PMU cycle counter.
> Print the clock frequency along with CPU cycles/packet to identify which
> cycle counter is being used.
> 
> Signed-off-by: Dharmik Thakkar <dharmik.thakkar@arm.com>
> Reviewed-by: Honnappa Nagarahalli <honnappa.nagarahalli@arm.com>
> Reviewed-by: Phil Yang <phil.yang@arm.com>

Acked-by: Bernard Iremonger <bernard.iremonger@intel.com>

Re: [dpdk-dev] [PATCH v2 2/5] app/testpmd: print fractional part in CPU cycles

[Iremonger, Bernard]

> -----Original Message-----
> From: Dharmik Thakkar <dharmik.thakkar@arm.com>
> Sent: Friday, May 8, 2020 11:38 PM
> To: Lu, Wenzhuo <wenzhuo.lu@intel.com>; Wu, Jingjing
> <jingjing.wu@intel.com>; Iremonger, Bernard
> <bernard.iremonger@intel.com>
> Cc: dev@dpdk.org; nd@arm.com; Dharmik Thakkar
> <dharmik.thakkar@arm.com>
> Subject: [PATCH v2 2/5] app/testpmd: print fractional part in CPU cycles
> 
> Change printing of CPU cycles/packet to include fractional part for
> accurateness.
> 
> Example:
> 
> Without patch:
> CPU cycles/packet=14
> (total cycles=4899533541 / total RX packets=343031966)
> 
> With patch:
> CPU cycles/packet=14.28
> (total cycles=4899533541 / total RX packets=343031966)
> 
> Signed-off-by: Dharmik Thakkar <dharmik.thakkar@arm.com>
> Reviewed-by: Honnappa Nagarahalli <honnappa.nagarahalli@arm.com>
> Reviewed-by: Phil Yang <phil.yang@arm.com>

Acked-by: Bernard Iremonger <bernard.iremonger@intel.com>

Re: [dpdk-dev] [PATCH v2 3/5] doc: add cycles per packet in testpmd user guide

[Iremonger, Bernard]

> -----Original Message-----
> From: Dharmik Thakkar <dharmik.thakkar@arm.com>
> Sent: Friday, May 8, 2020 11:38 PM
> To: Lu, Wenzhuo <wenzhuo.lu@intel.com>; Wu, Jingjing
> <jingjing.wu@intel.com>; Iremonger, Bernard
> <bernard.iremonger@intel.com>; Mcnamara, John
> <john.mcnamara@intel.com>; Kovacevic, Marko
> <marko.kovacevic@intel.com>
> Cc: dev@dpdk.org; nd@arm.com; Dharmik Thakkar
> <dharmik.thakkar@arm.com>
> Subject: [PATCH v2 3/5] doc: add cycles per packet in testpmd user guide
> 
> Update documentation for 'show fwd' testpmd runtime function to show
> CPU cycles/packet example.
> 
> Signed-off-by: Dharmik Thakkar <dharmik.thakkar@arm.com>
> Reviewed-by: Honnappa Nagarahalli <honnappa.nagarahalli@arm.com>
> Reviewed-by: Phil Yang <phil.yang@arm.com>

Acked-by: Bernard Iremonger <bernard.iremonger@intel.com>

Re: [dpdk-dev] [PATCH v2 4/5] doc: include config options in testpmd user guide

[Iremonger, Bernard]

> -----Original Message-----
> From: Dharmik Thakkar <dharmik.thakkar@arm.com>
> Sent: Friday, May 8, 2020 11:38 PM
> To: Lu, Wenzhuo <wenzhuo.lu@intel.com>; Wu, Jingjing
> <jingjing.wu@intel.com>; Iremonger, Bernard
> <bernard.iremonger@intel.com>; Mcnamara, John
> <john.mcnamara@intel.com>; Kovacevic, Marko
> <marko.kovacevic@intel.com>
> Cc: dev@dpdk.org; nd@arm.com; Dharmik Thakkar
> <dharmik.thakkar@arm.com>
> Subject: [PATCH v2 4/5] doc: include config options in testpmd user guide
> 
> Update testpmd documentation to include RECORD configuration options,
> CONFIG_RTE_TEST_PMD_RECORD_CORE_CYCLES and
> CONFIG_RTE_TEST_PMD_RECORD_BURST_STATS.
> 
> Signed-off-by: Dharmik Thakkar <dharmik.thakkar@arm.com>
> Reviewed-by: Honnappa Nagarahalli <honnappa.nagarahalli@arm.com>
> Reviewed-by: Phil Yang <phil.yang@arm.com>

Acked-by: Bernard Iremonger <bernard.iremonger@intel.com>

Re: [dpdk-dev] [PATCH v2 3/5] doc: add cycles per packet in testpmd user guide

[Ferruh Yigit]

On 5/8/2020 11:38 PM, Dharmik Thakkar wrote:
> Update documentation for 'show fwd' testpmd runtime function to show
> CPU cycles/packet example.
> 
> Signed-off-by: Dharmik Thakkar <dharmik.thakkar@arm.com>
> Reviewed-by: Honnappa Nagarahalli <honnappa.nagarahalli@arm.com>
> Reviewed-by: Phil Yang <phil.yang@arm.com>
> ---
> v2:
>  - Use shorter lines, up to 100.
> ---
>  doc/guides/testpmd_app_ug/testpmd_funcs.rst | 53 +++++++++++++--------
>  1 file changed, 33 insertions(+), 20 deletions(-)
> 
> diff --git a/doc/guides/testpmd_app_ug/testpmd_funcs.rst b/doc/guides/testpmd_app_ug/testpmd_funcs.rst
> index a360ecccfd3f..441ed41e3803 100644
> --- a/doc/guides/testpmd_app_ug/testpmd_funcs.rst
> +++ b/doc/guides/testpmd_app_ug/testpmd_funcs.rst
> @@ -351,26 +351,39 @@ Example for the io forwarding engine, with some packet drops on the tx side::
>  
>     testpmd> show fwd stats all
>  
> -     ------- Forward Stats for RX Port= 0/Queue= 0 -> TX Port= 1/Queue= 0 -------
> -     RX-packets: 274293770      TX-packets: 274293642      TX-dropped: 128
> -
> -     ------- Forward Stats for RX Port= 1/Queue= 0 -> TX Port= 0/Queue= 0 -------
> -     RX-packets: 274301850      TX-packets: 274301850      TX-dropped: 0
> -
> -     ---------------------- Forward statistics for port 0  ----------------------
> -     RX-packets: 274293802      RX-dropped: 0             RX-total: 274293802
> -     TX-packets: 274301862      TX-dropped: 0             TX-total: 274301862
> -     ----------------------------------------------------------------------------
> -
> -     ---------------------- Forward statistics for port 1  ----------------------
> -     RX-packets: 274301894      RX-dropped: 0             RX-total: 274301894
> -     TX-packets: 274293706      TX-dropped: 128           TX-total: 274293834
> -     ----------------------------------------------------------------------------
> -
> -     +++++++++++++++ Accumulated forward statistics for all ports+++++++++++++++
> -     RX-packets: 548595696      RX-dropped: 0             RX-total: 548595696
> -     TX-packets: 548595568      TX-dropped: 128           TX-total: 548595696
> -     ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
> +     ------- Forward Stats for RX Port= 0/Queue= 0 -> TX Port= 1/Queue= 0 -------
> +     RX-packets: 43536504       TX-packets: 43536488       TX-dropped: 0
> +
> +     ------- Forward Stats for RX Port= 1/Queue= 0 -> TX Port= 0/Queue= 0 -------
> +     RX-packets: 149738504      TX-packets: 149738504      TX-dropped: 0
> +
> +     ------- Forward Stats for RX Port= 1/Queue= 1 -> TX Port= 0/Queue= 1 -------
> +     RX-packets: 149753052      TX-packets: 149753052      TX-dropped: 0
> +
> +     ---------------------- Forward statistics for port 0  ----------------------
> +     RX-packets: 43538881       RX-dropped: 72            RX-total: 43538953
> +     TX-packets: 299491753      TX-dropped: 0             TX-total: 299491753
> +     ----------------------------------------------------------------------------
> +
> +     ---------------------- Forward statistics for port 1  ----------------------
> +     RX-packets: 299493085      RX-dropped: 8357          RX-total: 299501442
> +     TX-packets: 43539683       TX-dropped: 0             TX-total: 43539683
> +     ----------------------------------------------------------------------------
> +
> +     +++++++++++++++ Accumulated forward statistics for all ports+++++++++++++++
> +     RX-packets: 343031966      RX-dropped: 8429          RX-total: 343040395
> +     TX-packets: 343031436      TX-dropped: 0             TX-total: 343031436
> +     ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
> +
> +     CPU cycles/packet=14.28 (total cycles=4899533541 / total RX packets=343031966) at 200 MHz Clock

Hi Dharmik,

Overall it is OK to show "CPU cycles/packet", but my concern is numbers can be
misleading, because for example the numbers I am getting is like below:

CPU cycles/packet=4.38 (total cycles=32871036274 / total RX packets=7511734336)
at 2100 MHz Clock

> +
> +.. note::
> +
> +   Measuring CPU cycles/packet requires enabling CONFIG_RTE_TEST_PMD_RECORD_CORE_TX_CYCLES

s/TX_CYCLES/CYCLES

> +   configuration option. On aarch64 platforms, by default, the cycles are counted using
> +   generic counter which runs at a lower frequency than the CPU clock. To get the cycles/packet
> +   at CPU clock frequency, please scale the cycles/packet to CPU clock frequency.
> +   Alternatively, use the PMU based cycle counter.

Not sure the "aarch64 platforms" note suits to here, where documents the
command. And I can see the platform documentation already updated for this.

Combining above two comments, what do you think instead of replacing existing
stats, add a note saying enabling 'CONFIG_RTE_TEST_PMD_RECORD_CORE_CYCLES'
appends "CPU cycles/packet" and give above two samples, like:

"
Enabling RTE_TEST_PMD_RECORD_CORE_CYCLES appends "CPU cycles/packet" stats, like:

CPU cycles/packet=4.38 (total cycles=32871036274 / total RX packets=7511734336)
at 2100 MHz Clock
OR
CPU cycles/packet=14.28 (total cycles=4899533541 / total RX packets=343031966)
at 200 MHz Clock
"

Re: [dpdk-dev] [PATCH v2 1/5] app/testpmd: print clock with CPU cycles per pkt

[Ferruh Yigit]

On 5/8/2020 11:38 PM, Dharmik Thakkar wrote:
> On aarch64 platforms, the cycles are counted using either a
> low-resolution generic counter or a high-resolution PMU cycle counter.
> Print the clock frequency along with CPU cycles/packet to identify which
> cycle counter is being used.
> 
> Signed-off-by: Dharmik Thakkar <dharmik.thakkar@arm.com>
> Reviewed-by: Honnappa Nagarahalli <honnappa.nagarahalli@arm.com>
> Reviewed-by: Phil Yang <phil.yang@arm.com>

Since 3/5 is independent patch, to not cause that patch hold the patchset, I
will merge rest while we can keep discuss the 3/5.

Except 3/5
Series applied to dpdk-next-net/master, thanks.

Re: [dpdk-dev] [PATCH v2 3/5] doc: add cycles per packet in testpmd user guide

[Dharmik Thakkar]

Hi Ferruh,

> On May 15, 2020, at 7:15 AM, Ferruh Yigit <ferruh.yigit@intel.com> wrote:
> 
> On 5/8/2020 11:38 PM, Dharmik Thakkar wrote:
>> Update documentation for 'show fwd' testpmd runtime function to show
>> CPU cycles/packet example.
>> 
>> Signed-off-by: Dharmik Thakkar <dharmik.thakkar@arm.com>
>> Reviewed-by: Honnappa Nagarahalli <honnappa.nagarahalli@arm.com>
>> Reviewed-by: Phil Yang <phil.yang@arm.com>
>> ---
>> v2:
>> - Use shorter lines, up to 100.
>> ---
>> doc/guides/testpmd_app_ug/testpmd_funcs.rst | 53 +++++++++++++--------
>> 1 file changed, 33 insertions(+), 20 deletions(-)
>> 
>> diff --git a/doc/guides/testpmd_app_ug/testpmd_funcs.rst b/doc/guides/testpmd_app_ug/testpmd_funcs.rst
>> index a360ecccfd3f..441ed41e3803 100644
>> --- a/doc/guides/testpmd_app_ug/testpmd_funcs.rst
>> +++ b/doc/guides/testpmd_app_ug/testpmd_funcs.rst
>> @@ -351,26 +351,39 @@ Example for the io forwarding engine, with some packet drops on the tx side::
>> 
>>    testpmd> show fwd stats all
>> 
>> -     ------- Forward Stats for RX Port= 0/Queue= 0 -> TX Port= 1/Queue= 0 -------
>> -     RX-packets: 274293770      TX-packets: 274293642      TX-dropped: 128
>> -
>> -     ------- Forward Stats for RX Port= 1/Queue= 0 -> TX Port= 0/Queue= 0 -------
>> -     RX-packets: 274301850      TX-packets: 274301850      TX-dropped: 0
>> -
>> -     ---------------------- Forward statistics for port 0  ----------------------
>> -     RX-packets: 274293802      RX-dropped: 0             RX-total: 274293802
>> -     TX-packets: 274301862      TX-dropped: 0             TX-total: 274301862
>> -     ----------------------------------------------------------------------------
>> -
>> -     ---------------------- Forward statistics for port 1  ----------------------
>> -     RX-packets: 274301894      RX-dropped: 0             RX-total: 274301894
>> -     TX-packets: 274293706      TX-dropped: 128           TX-total: 274293834
>> -     ----------------------------------------------------------------------------
>> -
>> -     +++++++++++++++ Accumulated forward statistics for all ports+++++++++++++++
>> -     RX-packets: 548595696      RX-dropped: 0             RX-total: 548595696
>> -     TX-packets: 548595568      TX-dropped: 128           TX-total: 548595696
>> -     ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
>> +     ------- Forward Stats for RX Port= 0/Queue= 0 -> TX Port= 1/Queue= 0 -------
>> +     RX-packets: 43536504       TX-packets: 43536488       TX-dropped: 0
>> +
>> +     ------- Forward Stats for RX Port= 1/Queue= 0 -> TX Port= 0/Queue= 0 -------
>> +     RX-packets: 149738504      TX-packets: 149738504      TX-dropped: 0
>> +
>> +     ------- Forward Stats for RX Port= 1/Queue= 1 -> TX Port= 0/Queue= 1 -------
>> +     RX-packets: 149753052      TX-packets: 149753052      TX-dropped: 0
>> +
>> +     ---------------------- Forward statistics for port 0  ----------------------
>> +     RX-packets: 43538881       RX-dropped: 72            RX-total: 43538953
>> +     TX-packets: 299491753      TX-dropped: 0             TX-total: 299491753
>> +     ----------------------------------------------------------------------------
>> +
>> +     ---------------------- Forward statistics for port 1  ----------------------
>> +     RX-packets: 299493085      RX-dropped: 8357          RX-total: 299501442
>> +     TX-packets: 43539683       TX-dropped: 0             TX-total: 43539683
>> +     ----------------------------------------------------------------------------
>> +
>> +     +++++++++++++++ Accumulated forward statistics for all ports+++++++++++++++
>> +     RX-packets: 343031966      RX-dropped: 8429          RX-total: 343040395
>> +     TX-packets: 343031436      TX-dropped: 0             TX-total: 343031436
>> +     ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
>> +
>> +     CPU cycles/packet=14.28 (total cycles=4899533541 / total RX packets=343031966) at 200 MHz Clock
> 
> Hi Dharmik,
> 
> Overall it is OK to show "CPU cycles/packet", but my concern is numbers can be
> misleading, because for example the numbers I am getting is like below:
> 
> CPU cycles/packet=4.38 (total cycles=32871036274 / total RX packets=7511734336)
> at 2100 MHz Clock

CPU cycles/packet shown above is a part of the example. The numbers are as per the statistics shown in the example.

> 
>> +
>> +.. note::
>> +
>> +   Measuring CPU cycles/packet requires enabling CONFIG_RTE_TEST_PMD_RECORD_CORE_TX_CYCLES
> 
> s/TX_CYCLES/CYCLES

Sorry, didn’t understand this comment.

> 
>> +   configuration option. On aarch64 platforms, by default, the cycles are counted using
>> +   generic counter which runs at a lower frequency than the CPU clock. To get the cycles/packet
>> +   at CPU clock frequency, please scale the cycles/packet to CPU clock frequency.
>> +   Alternatively, use the PMU based cycle counter.
> 
> Not sure the "aarch64 platforms" note suits to here, where documents the
> command. And I can see the platform documentation already updated for this.

This additional comment about aarch64 platforms is to provide complete information to the users with regards to the config option
and offer a better understanding of the CPU cycles/packet numbers. IMO, without this note, CPU cycles/packet numbers can be misleading.

> 
> Combining above two comments, what do you think instead of replacing existing
> stats, add a note saying enabling 'CONFIG_RTE_TEST_PMD_RECORD_CORE_CYCLES'
> appends "CPU cycles/packet" and give above two samples, like:
> 
> "
> Enabling RTE_TEST_PMD_RECORD_CORE_CYCLES appends "CPU cycles/packet" stats, like:
> 
> CPU cycles/packet=4.38 (total cycles=32871036274 / total RX packets=7511734336)
> at 2100 MHz Clock
> OR
> CPU cycles/packet=14.28 (total cycles=4899533541 / total RX packets=343031966)
> at 200 MHz Clock
> "

Yes, this is also an option. IMO, numbers shown within CPU cycles/packet example should be consistent with the forward statistics numbers.

Re: [dpdk-dev] [PATCH v2 3/5] doc: add cycles per packet in testpmd user guide

[Ferruh Yigit]

On 5/15/2020 9:33 PM, Dharmik Thakkar wrote:
> Hi Ferruh,
> 
>> On May 15, 2020, at 7:15 AM, Ferruh Yigit <ferruh.yigit@intel.com> wrote:
>>
>> On 5/8/2020 11:38 PM, Dharmik Thakkar wrote:
>>> Update documentation for 'show fwd' testpmd runtime function to show
>>> CPU cycles/packet example.
>>>
>>> Signed-off-by: Dharmik Thakkar <dharmik.thakkar@arm.com>
>>> Reviewed-by: Honnappa Nagarahalli <honnappa.nagarahalli@arm.com>
>>> Reviewed-by: Phil Yang <phil.yang@arm.com>
>>> ---
>>> v2:
>>> - Use shorter lines, up to 100.
>>> ---
>>> doc/guides/testpmd_app_ug/testpmd_funcs.rst | 53 +++++++++++++--------
>>> 1 file changed, 33 insertions(+), 20 deletions(-)
>>>
>>> diff --git a/doc/guides/testpmd_app_ug/testpmd_funcs.rst b/doc/guides/testpmd_app_ug/testpmd_funcs.rst
>>> index a360ecccfd3f..441ed41e3803 100644
>>> --- a/doc/guides/testpmd_app_ug/testpmd_funcs.rst
>>> +++ b/doc/guides/testpmd_app_ug/testpmd_funcs.rst
>>> @@ -351,26 +351,39 @@ Example for the io forwarding engine, with some packet drops on the tx side::
>>>
>>>    testpmd> show fwd stats all
>>>
>>> -     ------- Forward Stats for RX Port= 0/Queue= 0 -> TX Port= 1/Queue= 0 -------
>>> -     RX-packets: 274293770      TX-packets: 274293642      TX-dropped: 128
>>> -
>>> -     ------- Forward Stats for RX Port= 1/Queue= 0 -> TX Port= 0/Queue= 0 -------
>>> -     RX-packets: 274301850      TX-packets: 274301850      TX-dropped: 0
>>> -
>>> -     ---------------------- Forward statistics for port 0  ----------------------
>>> -     RX-packets: 274293802      RX-dropped: 0             RX-total: 274293802
>>> -     TX-packets: 274301862      TX-dropped: 0             TX-total: 274301862
>>> -     ----------------------------------------------------------------------------
>>> -
>>> -     ---------------------- Forward statistics for port 1  ----------------------
>>> -     RX-packets: 274301894      RX-dropped: 0             RX-total: 274301894
>>> -     TX-packets: 274293706      TX-dropped: 128           TX-total: 274293834
>>> -     ----------------------------------------------------------------------------
>>> -
>>> -     +++++++++++++++ Accumulated forward statistics for all ports+++++++++++++++
>>> -     RX-packets: 548595696      RX-dropped: 0             RX-total: 548595696
>>> -     TX-packets: 548595568      TX-dropped: 128           TX-total: 548595696
>>> -     ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
>>> +     ------- Forward Stats for RX Port= 0/Queue= 0 -> TX Port= 1/Queue= 0 -------
>>> +     RX-packets: 43536504       TX-packets: 43536488       TX-dropped: 0
>>> +
>>> +     ------- Forward Stats for RX Port= 1/Queue= 0 -> TX Port= 0/Queue= 0 -------
>>> +     RX-packets: 149738504      TX-packets: 149738504      TX-dropped: 0
>>> +
>>> +     ------- Forward Stats for RX Port= 1/Queue= 1 -> TX Port= 0/Queue= 1 -------
>>> +     RX-packets: 149753052      TX-packets: 149753052      TX-dropped: 0
>>> +
>>> +     ---------------------- Forward statistics for port 0  ----------------------
>>> +     RX-packets: 43538881       RX-dropped: 72            RX-total: 43538953
>>> +     TX-packets: 299491753      TX-dropped: 0             TX-total: 299491753
>>> +     ----------------------------------------------------------------------------
>>> +
>>> +     ---------------------- Forward statistics for port 1  ----------------------
>>> +     RX-packets: 299493085      RX-dropped: 8357          RX-total: 299501442
>>> +     TX-packets: 43539683       TX-dropped: 0             TX-total: 43539683
>>> +     ----------------------------------------------------------------------------
>>> +
>>> +     +++++++++++++++ Accumulated forward statistics for all ports+++++++++++++++
>>> +     RX-packets: 343031966      RX-dropped: 8429          RX-total: 343040395
>>> +     TX-packets: 343031436      TX-dropped: 0             TX-total: 343031436
>>> +     ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
>>> +
>>> +     CPU cycles/packet=14.28 (total cycles=4899533541 / total RX packets=343031966) at 200 MHz Clock
>>
>> Hi Dharmik,
>>
>> Overall it is OK to show "CPU cycles/packet", but my concern is numbers can be
>> misleading, because for example the numbers I am getting is like below:
>>
>> CPU cycles/packet=4.38 (total cycles=32871036274 / total RX packets=7511734336)
>> at 2100 MHz Clock
> 
> CPU cycles/packet shown above is a part of the example. The numbers are as per the statistics shown in the example.

I know but now you are showing a performance number and it is specific to a
platform, my concern it may be misleading and create false expectations for others.

> 
>>
>>> +
>>> +.. note::
>>> +
>>> +   Measuring CPU cycles/packet requires enabling CONFIG_RTE_TEST_PMD_RECORD_CORE_TX_CYCLES
>>
>> s/TX_CYCLES/CYCLES
> 
> Sorry, didn’t understand this comment.

There is no 'CONFIG_RTE_TEST_PMD_RECORD_CORE_TX_CYCLES' config option.

> 
>>
>>> +   configuration option. On aarch64 platforms, by default, the cycles are counted using
>>> +   generic counter which runs at a lower frequency than the CPU clock. To get the cycles/packet
>>> +   at CPU clock frequency, please scale the cycles/packet to CPU clock frequency.
>>> +   Alternatively, use the PMU based cycle counter.
>>
>> Not sure the "aarch64 platforms" note suits to here, where documents the
>> command. And I can see the platform documentation already updated for this.
> 
> This additional comment about aarch64 platforms is to provide complete information to the users with regards to the config option
> and offer a better understanding of the CPU cycles/packet numbers. IMO, without this note, CPU cycles/packet numbers can be misleading.

This is testpmd documentation to describe "show fwd stats" command. We are
adding a note to say with a config option it may also show performance data,
which is OK. But starting to describe the platform differences for this config
option looks to much details for the scope of the document. For testpmd I would
prefer command usage independent from platforms, otherwise the documentation may
become too confusing/complex.
But agree to provide the information for the specific platform, which you are
already doing in other patch in the platform documentation.

> 
>>
>> Combining above two comments, what do you think instead of replacing existing
>> stats, add a note saying enabling 'CONFIG_RTE_TEST_PMD_RECORD_CORE_CYCLES'
>> appends "CPU cycles/packet" and give above two samples, like:
>>
>> "
>> Enabling RTE_TEST_PMD_RECORD_CORE_CYCLES appends "CPU cycles/packet" stats, like:
>>
>> CPU cycles/packet=4.38 (total cycles=32871036274 / total RX packets=7511734336)
>> at 2100 MHz Clock
>> OR
>> CPU cycles/packet=14.28 (total cycles=4899533541 / total RX packets=343031966)
>> at 200 MHz Clock
>> "
> 
> Yes, this is also an option. IMO, numbers shown within CPU cycles/packet example should be consistent with the forward statistics numbers.
> 

This part is just the sample of the "CPU cycles/packet" output, I think it
provides enough context to be understandable without matching Rx packet count in
above part of the stats.

Re: [dpdk-dev] [PATCH v2 3/5] doc: add cycles per packet in testpmd user guide

[Dharmik Thakkar]

> On May 18, 2020, at 4:47 AM, Ferruh Yigit <ferruh.yigit@intel.com> wrote:
> 
> On 5/15/2020 9:33 PM, Dharmik Thakkar wrote:
>> Hi Ferruh,
>> 
>>> On May 15, 2020, at 7:15 AM, Ferruh Yigit <ferruh.yigit@intel.com> wrote:
>>> 
>>> On 5/8/2020 11:38 PM, Dharmik Thakkar wrote:
>>>> Update documentation for 'show fwd' testpmd runtime function to show
>>>> CPU cycles/packet example.
>>>> 
>>>> Signed-off-by: Dharmik Thakkar <dharmik.thakkar@arm.com>
>>>> Reviewed-by: Honnappa Nagarahalli <honnappa.nagarahalli@arm.com>
>>>> Reviewed-by: Phil Yang <phil.yang@arm.com>
>>>> ---
>>>> v2:
>>>> - Use shorter lines, up to 100.
>>>> ---
>>>> doc/guides/testpmd_app_ug/testpmd_funcs.rst | 53 +++++++++++++--------
>>>> 1 file changed, 33 insertions(+), 20 deletions(-)
>>>> 
>>>> diff --git a/doc/guides/testpmd_app_ug/testpmd_funcs.rst b/doc/guides/testpmd_app_ug/testpmd_funcs.rst
>>>> index a360ecccfd3f..441ed41e3803 100644
>>>> --- a/doc/guides/testpmd_app_ug/testpmd_funcs.rst
>>>> +++ b/doc/guides/testpmd_app_ug/testpmd_funcs.rst
>>>> @@ -351,26 +351,39 @@ Example for the io forwarding engine, with some packet drops on the tx side::
>>>> 
>>>>   testpmd> show fwd stats all
>>>> 
>>>> -     ------- Forward Stats for RX Port= 0/Queue= 0 -> TX Port= 1/Queue= 0 -------
>>>> -     RX-packets: 274293770      TX-packets: 274293642      TX-dropped: 128
>>>> -
>>>> -     ------- Forward Stats for RX Port= 1/Queue= 0 -> TX Port= 0/Queue= 0 -------
>>>> -     RX-packets: 274301850      TX-packets: 274301850      TX-dropped: 0
>>>> -
>>>> -     ---------------------- Forward statistics for port 0  ----------------------
>>>> -     RX-packets: 274293802      RX-dropped: 0             RX-total: 274293802
>>>> -     TX-packets: 274301862      TX-dropped: 0             TX-total: 274301862
>>>> -     ----------------------------------------------------------------------------
>>>> -
>>>> -     ---------------------- Forward statistics for port 1  ----------------------
>>>> -     RX-packets: 274301894      RX-dropped: 0             RX-total: 274301894
>>>> -     TX-packets: 274293706      TX-dropped: 128           TX-total: 274293834
>>>> -     ----------------------------------------------------------------------------
>>>> -
>>>> -     +++++++++++++++ Accumulated forward statistics for all ports+++++++++++++++
>>>> -     RX-packets: 548595696      RX-dropped: 0             RX-total: 548595696
>>>> -     TX-packets: 548595568      TX-dropped: 128           TX-total: 548595696
>>>> -     ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
>>>> +     ------- Forward Stats for RX Port= 0/Queue= 0 -> TX Port= 1/Queue= 0 -------
>>>> +     RX-packets: 43536504       TX-packets: 43536488       TX-dropped: 0
>>>> +
>>>> +     ------- Forward Stats for RX Port= 1/Queue= 0 -> TX Port= 0/Queue= 0 -------
>>>> +     RX-packets: 149738504      TX-packets: 149738504      TX-dropped: 0
>>>> +
>>>> +     ------- Forward Stats for RX Port= 1/Queue= 1 -> TX Port= 0/Queue= 1 -------
>>>> +     RX-packets: 149753052      TX-packets: 149753052      TX-dropped: 0
>>>> +
>>>> +     ---------------------- Forward statistics for port 0  ----------------------
>>>> +     RX-packets: 43538881       RX-dropped: 72            RX-total: 43538953
>>>> +     TX-packets: 299491753      TX-dropped: 0             TX-total: 299491753
>>>> +     ----------------------------------------------------------------------------
>>>> +
>>>> +     ---------------------- Forward statistics for port 1  ----------------------
>>>> +     RX-packets: 299493085      RX-dropped: 8357          RX-total: 299501442
>>>> +     TX-packets: 43539683       TX-dropped: 0             TX-total: 43539683
>>>> +     ----------------------------------------------------------------------------
>>>> +
>>>> +     +++++++++++++++ Accumulated forward statistics for all ports+++++++++++++++
>>>> +     RX-packets: 343031966      RX-dropped: 8429          RX-total: 343040395
>>>> +     TX-packets: 343031436      TX-dropped: 0             TX-total: 343031436
>>>> +     ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
>>>> +
>>>> +     CPU cycles/packet=14.28 (total cycles=4899533541 / total RX packets=343031966) at 200 MHz Clock
>>> 
>>> Hi Dharmik,
>>> 
>>> Overall it is OK to show "CPU cycles/packet", but my concern is numbers can be
>>> misleading, because for example the numbers I am getting is like below:
>>> 
>>> CPU cycles/packet=4.38 (total cycles=32871036274 / total RX packets=7511734336)
>>> at 2100 MHz Clock
>> 
>> CPU cycles/packet shown above is a part of the example. The numbers are as per the statistics shown in the example.
> 
> I know but now you are showing a performance number and it is specific to a
> platform, my concern it may be misleading and create false expectations for others.

Ok, makes sense.

> 
>> 
>>> 
>>>> +
>>>> +.. note::
>>>> +
>>>> +   Measuring CPU cycles/packet requires enabling CONFIG_RTE_TEST_PMD_RECORD_CORE_TX_CYCLES
>>> 
>>> s/TX_CYCLES/CYCLES
>> 
>> Sorry, didn’t understand this comment.
> 
> There is no 'CONFIG_RTE_TEST_PMD_RECORD_CORE_TX_CYCLES' config option.

Got it, will update.

> 
>> 
>>> 
>>>> +   configuration option. On aarch64 platforms, by default, the cycles are counted using
>>>> +   generic counter which runs at a lower frequency than the CPU clock. To get the cycles/packet
>>>> +   at CPU clock frequency, please scale the cycles/packet to CPU clock frequency.
>>>> +   Alternatively, use the PMU based cycle counter.
>>> 
>>> Not sure the "aarch64 platforms" note suits to here, where documents the
>>> command. And I can see the platform documentation already updated for this.
>> 
>> This additional comment about aarch64 platforms is to provide complete information to the users with regards to the config option
>> and offer a better understanding of the CPU cycles/packet numbers. IMO, without this note, CPU cycles/packet numbers can be misleading.
> 
> This is testpmd documentation to describe "show fwd stats" command. We are
> adding a note to say with a config option it may also show performance data,
> which is OK. But starting to describe the platform differences for this config
> option looks to much details for the scope of the document. For testpmd I would
> prefer command usage independent from platforms, otherwise the documentation may
> become too confusing/complex.
> But agree to provide the information for the specific platform, which you are
> already doing in other patch in the platform documentation.

Alright, I will remove platform in the next version.

> 
>> 
>>> 
>>> Combining above two comments, what do you think instead of replacing existing
>>> stats, add a note saying enabling 'CONFIG_RTE_TEST_PMD_RECORD_CORE_CYCLES'
>>> appends "CPU cycles/packet" and give above two samples, like:
>>> 
>>> "
>>> Enabling RTE_TEST_PMD_RECORD_CORE_CYCLES appends "CPU cycles/packet" stats, like:
>>> 
>>> CPU cycles/packet=4.38 (total cycles=32871036274 / total RX packets=7511734336)
>>> at 2100 MHz Clock
>>> OR
>>> CPU cycles/packet=14.28 (total cycles=4899533541 / total RX packets=343031966)
>>> at 200 MHz Clock
>>> "
>> 
>> Yes, this is also an option. IMO, numbers shown within CPU cycles/packet example should be consistent with the forward statistics numbers.
>> 
> 
> This part is just the sample of the "CPU cycles/packet" output, I think it
> provides enough context to be understandable without matching Rx packet count in
> above part of the stats.

Instead of adding 2 samples, do you think using xxx is best? For example:
CPU cycles/packet=xx.dd (total cycles=xxxx / total RX packets=xxxx) at xxxx MHz clock

Re: [dpdk-dev] [PATCH v2 4/5] doc: include config options in testpmd user guide

[Thomas Monjalon]

> > Update testpmd documentation to include RECORD configuration options,
> > CONFIG_RTE_TEST_PMD_RECORD_CORE_CYCLES and
> > CONFIG_RTE_TEST_PMD_RECORD_BURST_STATS.
> > 
> > Signed-off-by: Dharmik Thakkar <dharmik.thakkar@arm.com>
> > Reviewed-by: Honnappa Nagarahalli <honnappa.nagarahalli@arm.com>
> > Reviewed-by: Phil Yang <phil.yang@arm.com>
> 
> Acked-by: Bernard Iremonger <bernard.iremonger@intel.com>

How these options are managed with meson?

Re: [dpdk-dev] [PATCH v2 4/5] doc: include config options in testpmd user guide

[Thomas Monjalon]

09/05/2020 00:38, Dharmik Thakkar:
> Update testpmd documentation to include RECORD configuration options,
> CONFIG_RTE_TEST_PMD_RECORD_CORE_CYCLES and
> CONFIG_RTE_TEST_PMD_RECORD_BURST_STATS.
> 
> Signed-off-by: Dharmik Thakkar <dharmik.thakkar@arm.com>
> Reviewed-by: Honnappa Nagarahalli <honnappa.nagarahalli@arm.com>
> Reviewed-by: Phil Yang <phil.yang@arm.com>
> ---
> +#.  If required, enable configuration options. For example:
> +
> +    .. code-block:: console
> +
> +        cd to the top-level DPDK directory
> +        sed -i 's,\(CONFIG_RTE_TEST_PMD_RECORD_CORE_CYCLES\)=n,\1=y,' config/common_base
> +        sed -i 's,\(CONFIG_RTE_TEST_PMD_RECORD_BURST_STATS\)=n,\1=y,' config/common_base

Changing source code is wrong.
Only the generated .config file should be changed.

Not even speaking about meson...

Why we don't have any review on documentation patches?

Patch dropped from next-net pulling.

Re: [dpdk-dev] [PATCH v2 3/5] doc: add cycles per packet in testpmd user guide

[Ferruh Yigit]

On 5/18/2020 10:49 PM, Dharmik Thakkar wrote:
> 
> 
>> On May 18, 2020, at 4:47 AM, Ferruh Yigit <ferruh.yigit@intel.com> wrote:
>>
>> On 5/15/2020 9:33 PM, Dharmik Thakkar wrote:
>>> Hi Ferruh,
>>>
>>>> On May 15, 2020, at 7:15 AM, Ferruh Yigit <ferruh.yigit@intel.com> wrote:
>>>>
>>>> On 5/8/2020 11:38 PM, Dharmik Thakkar wrote:
>>>>> Update documentation for 'show fwd' testpmd runtime function to show
>>>>> CPU cycles/packet example.
>>>>>
>>>>> Signed-off-by: Dharmik Thakkar <dharmik.thakkar@arm.com>
>>>>> Reviewed-by: Honnappa Nagarahalli <honnappa.nagarahalli@arm.com>
>>>>> Reviewed-by: Phil Yang <phil.yang@arm.com>
>>>>> ---
>>>>> v2:
>>>>> - Use shorter lines, up to 100.
>>>>> ---
>>>>> doc/guides/testpmd_app_ug/testpmd_funcs.rst | 53 +++++++++++++--------
>>>>> 1 file changed, 33 insertions(+), 20 deletions(-)
>>>>>
>>>>> diff --git a/doc/guides/testpmd_app_ug/testpmd_funcs.rst b/doc/guides/testpmd_app_ug/testpmd_funcs.rst
>>>>> index a360ecccfd3f..441ed41e3803 100644
>>>>> --- a/doc/guides/testpmd_app_ug/testpmd_funcs.rst
>>>>> +++ b/doc/guides/testpmd_app_ug/testpmd_funcs.rst
>>>>> @@ -351,26 +351,39 @@ Example for the io forwarding engine, with some packet drops on the tx side::
>>>>>
>>>>>   testpmd> show fwd stats all
>>>>>
>>>>> -     ------- Forward Stats for RX Port= 0/Queue= 0 -> TX Port= 1/Queue= 0 -------
>>>>> -     RX-packets: 274293770      TX-packets: 274293642      TX-dropped: 128
>>>>> -
>>>>> -     ------- Forward Stats for RX Port= 1/Queue= 0 -> TX Port= 0/Queue= 0 -------
>>>>> -     RX-packets: 274301850      TX-packets: 274301850      TX-dropped: 0
>>>>> -
>>>>> -     ---------------------- Forward statistics for port 0  ----------------------
>>>>> -     RX-packets: 274293802      RX-dropped: 0             RX-total: 274293802
>>>>> -     TX-packets: 274301862      TX-dropped: 0             TX-total: 274301862
>>>>> -     ----------------------------------------------------------------------------
>>>>> -
>>>>> -     ---------------------- Forward statistics for port 1  ----------------------
>>>>> -     RX-packets: 274301894      RX-dropped: 0             RX-total: 274301894
>>>>> -     TX-packets: 274293706      TX-dropped: 128           TX-total: 274293834
>>>>> -     ----------------------------------------------------------------------------
>>>>> -
>>>>> -     +++++++++++++++ Accumulated forward statistics for all ports+++++++++++++++
>>>>> -     RX-packets: 548595696      RX-dropped: 0             RX-total: 548595696
>>>>> -     TX-packets: 548595568      TX-dropped: 128           TX-total: 548595696
>>>>> -     ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
>>>>> +     ------- Forward Stats for RX Port= 0/Queue= 0 -> TX Port= 1/Queue= 0 -------
>>>>> +     RX-packets: 43536504       TX-packets: 43536488       TX-dropped: 0
>>>>> +
>>>>> +     ------- Forward Stats for RX Port= 1/Queue= 0 -> TX Port= 0/Queue= 0 -------
>>>>> +     RX-packets: 149738504      TX-packets: 149738504      TX-dropped: 0
>>>>> +
>>>>> +     ------- Forward Stats for RX Port= 1/Queue= 1 -> TX Port= 0/Queue= 1 -------
>>>>> +     RX-packets: 149753052      TX-packets: 149753052      TX-dropped: 0
>>>>> +
>>>>> +     ---------------------- Forward statistics for port 0  ----------------------
>>>>> +     RX-packets: 43538881       RX-dropped: 72            RX-total: 43538953
>>>>> +     TX-packets: 299491753      TX-dropped: 0             TX-total: 299491753
>>>>> +     ----------------------------------------------------------------------------
>>>>> +
>>>>> +     ---------------------- Forward statistics for port 1  ----------------------
>>>>> +     RX-packets: 299493085      RX-dropped: 8357          RX-total: 299501442
>>>>> +     TX-packets: 43539683       TX-dropped: 0             TX-total: 43539683
>>>>> +     ----------------------------------------------------------------------------
>>>>> +
>>>>> +     +++++++++++++++ Accumulated forward statistics for all ports+++++++++++++++
>>>>> +     RX-packets: 343031966      RX-dropped: 8429          RX-total: 343040395
>>>>> +     TX-packets: 343031436      TX-dropped: 0             TX-total: 343031436
>>>>> +     ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
>>>>> +
>>>>> +     CPU cycles/packet=14.28 (total cycles=4899533541 / total RX packets=343031966) at 200 MHz Clock
>>>>
>>>> Hi Dharmik,
>>>>
>>>> Overall it is OK to show "CPU cycles/packet", but my concern is numbers can be
>>>> misleading, because for example the numbers I am getting is like below:
>>>>
>>>> CPU cycles/packet=4.38 (total cycles=32871036274 / total RX packets=7511734336)
>>>> at 2100 MHz Clock
>>>
>>> CPU cycles/packet shown above is a part of the example. The numbers are as per the statistics shown in the example.
>>
>> I know but now you are showing a performance number and it is specific to a
>> platform, my concern it may be misleading and create false expectations for others.
> 
> Ok, makes sense.
> 
>>
>>>
>>>>
>>>>> +
>>>>> +.. note::
>>>>> +
>>>>> +   Measuring CPU cycles/packet requires enabling CONFIG_RTE_TEST_PMD_RECORD_CORE_TX_CYCLES
>>>>
>>>> s/TX_CYCLES/CYCLES
>>>
>>> Sorry, didn’t understand this comment.
>>
>> There is no 'CONFIG_RTE_TEST_PMD_RECORD_CORE_TX_CYCLES' config option.
> 
> Got it, will update.
> 
>>
>>>
>>>>
>>>>> +   configuration option. On aarch64 platforms, by default, the cycles are counted using
>>>>> +   generic counter which runs at a lower frequency than the CPU clock. To get the cycles/packet
>>>>> +   at CPU clock frequency, please scale the cycles/packet to CPU clock frequency.
>>>>> +   Alternatively, use the PMU based cycle counter.
>>>>
>>>> Not sure the "aarch64 platforms" note suits to here, where documents the
>>>> command. And I can see the platform documentation already updated for this.
>>>
>>> This additional comment about aarch64 platforms is to provide complete information to the users with regards to the config option
>>> and offer a better understanding of the CPU cycles/packet numbers. IMO, without this note, CPU cycles/packet numbers can be misleading.
>>
>> This is testpmd documentation to describe "show fwd stats" command. We are
>> adding a note to say with a config option it may also show performance data,
>> which is OK. But starting to describe the platform differences for this config
>> option looks to much details for the scope of the document. For testpmd I would
>> prefer command usage independent from platforms, otherwise the documentation may
>> become too confusing/complex.
>> But agree to provide the information for the specific platform, which you are
>> already doing in other patch in the platform documentation.
> 
> Alright, I will remove platform in the next version.
> 
>>
>>>
>>>>
>>>> Combining above two comments, what do you think instead of replacing existing
>>>> stats, add a note saying enabling 'CONFIG_RTE_TEST_PMD_RECORD_CORE_CYCLES'
>>>> appends "CPU cycles/packet" and give above two samples, like:
>>>>
>>>> "
>>>> Enabling RTE_TEST_PMD_RECORD_CORE_CYCLES appends "CPU cycles/packet" stats, like:
>>>>
>>>> CPU cycles/packet=4.38 (total cycles=32871036274 / total RX packets=7511734336)
>>>> at 2100 MHz Clock
>>>> OR
>>>> CPU cycles/packet=14.28 (total cycles=4899533541 / total RX packets=343031966)
>>>> at 200 MHz Clock
>>>> "
>>>
>>> Yes, this is also an option. IMO, numbers shown within CPU cycles/packet example should be consistent with the forward statistics numbers.
>>>
>>
>> This part is just the sample of the "CPU cycles/packet" output, I think it
>> provides enough context to be understandable without matching Rx packet count in
>> above part of the stats.
> 
> Instead of adding 2 samples, do you think using xxx is best? For example:
> CPU cycles/packet=xx.dd (total cycles=xxxx / total RX packets=xxxx) at xxxx MHz clock
> 

+1

Re: [dpdk-dev] [PATCH v2 4/5] doc: include config options in testpmd user guide

[Dharmik Thakkar]

> On May 19, 2020, at 2:42 AM, Thomas Monjalon <thomas@monjalon.net> wrote:
> 
>>> Update testpmd documentation to include RECORD configuration options,
>>> CONFIG_RTE_TEST_PMD_RECORD_CORE_CYCLES and
>>> CONFIG_RTE_TEST_PMD_RECORD_BURST_STATS.
>>> 
>>> Signed-off-by: Dharmik Thakkar <dharmik.thakkar@arm.com>
>>> Reviewed-by: Honnappa Nagarahalli <honnappa.nagarahalli@arm.com>
>>> Reviewed-by: Phil Yang <phil.yang@arm.com>
>> 
>> Acked-by: Bernard Iremonger <bernard.iremonger@intel.com>
> 
> How these options are managed with meson?
> 
> 

As per my understanding, currently, this is not implemented with meson.
With ‘make’, the configuration options are saved within ./build/include/rte_config.h ( which gets generated during make config …).
But this file (rte_config.h) does not get generated when using meson.

[dpdk-dev] [PATCH v3 1/2] doc: add cycles per packet in testpmd user guide

[Dharmik Thakkar]

Update documentation for 'show fwd' testpmd runtime function to show
CPU cycles/packet example.

Signed-off-by: Dharmik Thakkar <dharmik.thakkar@arm.com>
Reviewed-by: Honnappa Nagarahalli <honnappa.nagarahalli@arm.com>
Reviewed-by: Phil Yang <phil.yang@arm.com>
---
v3:
 - Keep existing stats and add a note providing CPU cycles/packet
   example (Ferruh)
 - Remove platform specific information (Ferruh)
 - Remove performance numbers from CPU cycles/packet example (Ferruh)
 - Fix typo in CONFIG_RTE_TEST_PMD_RECORD_CORE_CYCLES (Ferruh)

v2:
 - Use shorter lines, up to 100.
---
 doc/guides/testpmd_app_ug/testpmd_funcs.rst | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/doc/guides/testpmd_app_ug/testpmd_funcs.rst b/doc/guides/testpmd_app_ug/testpmd_funcs.rst
index 3ded179ee72a..a808b6a308f2 100644
--- a/doc/guides/testpmd_app_ug/testpmd_funcs.rst
+++ b/doc/guides/testpmd_app_ug/testpmd_funcs.rst
@@ -372,6 +372,12 @@ Example for the io forwarding engine, with some packet drops on the tx side::
      TX-packets: 548595568      TX-dropped: 128           TX-total: 548595696
      ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 
+.. note::
+
+   Enabling CONFIG_RTE_TEST_PMD_RECORD_CORE_CYCLES appends "CPU cycles/packet" stats, like:
+
+   CPU cycles/packet=xx.dd (total cycles=xxxx / total RX packets=xxxx) at xxx MHz clock
+
 clear fwd
 ~~~~~~~~~
 
-- 
2.17.1

[dpdk-dev] [PATCH v3 2/2] doc: include config options in testpmd user guide

[Dharmik Thakkar]

Update testpmd documentation to include RECORD configuration options,
CONFIG_RTE_TEST_PMD_RECORD_CORE_CYCLES and
CONFIG_RTE_TEST_PMD_RECORD_BURST_STATS.

Signed-off-by: Dharmik Thakkar <dharmik.thakkar@arm.com>
Reviewed-by: Honnappa Nagarahalli <honnappa.nagarahalli@arm.com>
Reviewed-by: Phil Yang <phil.yang@arm.com>
---
v3:
 - Replace config/common_base with build/.config (Thomas)

v2:
 - Remove extra '#'.
---
 doc/guides/testpmd_app_ug/build_app.rst | 12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/doc/guides/testpmd_app_ug/build_app.rst b/doc/guides/testpmd_app_ug/build_app.rst
index d1ca9f3d19a9..8c9aaa83187f 100644
--- a/doc/guides/testpmd_app_ug/build_app.rst
+++ b/doc/guides/testpmd_app_ug/build_app.rst
@@ -21,6 +21,18 @@ The basic compilation steps are:
 
         export RTE_TARGET=x86_64-native-linux-gcc
 
+#.  If required, enable configuration options. For example:
+
+    .. code-block:: console
+
+        cd to the top-level DPDK directory
+        sed -i 's,\(CONFIG_RTE_TEST_PMD_RECORD_CORE_CYCLES\)=n,\1=y,' build/.config
+        sed -i 's,\(CONFIG_RTE_TEST_PMD_RECORD_BURST_STATS\)=n,\1=y,' build/.config
+
+    Enabling CONFIG_RTE_TEST_PMD_RECORD_CORE_CYCLES enables measurement of CPU cycles.
+
+    Enabling CONFIG_RTE_TEST_PMD_RECORD_BURST_STATS enables display of RX and TX bursts.
+
 #.  Build the application:
 
     .. code-block:: console
-- 
2.17.1

Re: [dpdk-dev] [PATCH v2 4/5] doc: include config options in testpmd user guide

[Thomas Monjalon]

20/05/2020 00:58, Dharmik Thakkar:
> > On May 19, 2020, at 2:42 AM, Thomas Monjalon <thomas@monjalon.net> wrote:
> >>> Update testpmd documentation to include RECORD configuration options,
> >>> CONFIG_RTE_TEST_PMD_RECORD_CORE_CYCLES and
> >>> CONFIG_RTE_TEST_PMD_RECORD_BURST_STATS.
> >>> 
> >>> Signed-off-by: Dharmik Thakkar <dharmik.thakkar@arm.com>
> >>> Reviewed-by: Honnappa Nagarahalli <honnappa.nagarahalli@arm.com>
> >>> Reviewed-by: Phil Yang <phil.yang@arm.com>
> >> 
> >> Acked-by: Bernard Iremonger <bernard.iremonger@intel.com>
> > 
> > How these options are managed with meson?
> 
> As per my understanding, currently, this is not implemented with meson.
> With ‘make’, the configuration options are saved within ./build/include/rte_config.h ( which gets generated during make config …).
> But this file (rte_config.h) does not get generated when using meson.

That's also my understanding.
There is a gap which needs to be fixed, not sure what is the best approach.
Can it be made a runtime option?

Re: [dpdk-dev] [PATCH v3 1/2] doc: add cycles per packet in testpmd user guide

[Ferruh Yigit]

On 5/20/2020 4:20 AM, Dharmik Thakkar wrote:
> Update documentation for 'show fwd' testpmd runtime function to show
> CPU cycles/packet example.
> 
> Signed-off-by: Dharmik Thakkar <dharmik.thakkar@arm.com>
> Reviewed-by: Honnappa Nagarahalli <honnappa.nagarahalli@arm.com>
> Reviewed-by: Phil Yang <phil.yang@arm.com>

Reviewed-by: Ferruh Yigit <ferruh.yigit@intel.com>

Re: [dpdk-dev] [PATCH v2 4/5] doc: include config options in testpmd user guide

[Dharmik Thakkar]

> On May 20, 2020, at 2:53 AM, Thomas Monjalon <thomas@monjalon.net> wrote:
> 
> 20/05/2020 00:58, Dharmik Thakkar:
>>> On May 19, 2020, at 2:42 AM, Thomas Monjalon <thomas@monjalon.net> wrote:
>>>>> Update testpmd documentation to include RECORD configuration options,
>>>>> CONFIG_RTE_TEST_PMD_RECORD_CORE_CYCLES and
>>>>> CONFIG_RTE_TEST_PMD_RECORD_BURST_STATS.
>>>>> 
>>>>> Signed-off-by: Dharmik Thakkar <dharmik.thakkar@arm.com>
>>>>> Reviewed-by: Honnappa Nagarahalli <honnappa.nagarahalli@arm.com>
>>>>> Reviewed-by: Phil Yang <phil.yang@arm.com>
>>>> 
>>>> Acked-by: Bernard Iremonger <bernard.iremonger@intel.com>
>>> 
>>> How these options are managed with meson?
>> 
>> As per my understanding, currently, this is not implemented with meson.
>> With ‘make’, the configuration options are saved within ./build/include/rte_config.h ( which gets generated during make config …).
>> But this file (rte_config.h) does not get generated when using meson.
> 
> That's also my understanding.
> There is a gap which needs to be fixed, not sure what is the best approach.

IMO, for now, it is best to replicate what is being done for ‘make’.

> Can it be made a runtime option?

Yes, it can be made a runtime option but that will be a separate patch.

> 

Re: [dpdk-dev] [PATCH v3 1/2] doc: add cycles per packet in testpmd user guide

[Ferruh Yigit]

On 5/20/2020 3:49 PM, Ferruh Yigit wrote:
> On 5/20/2020 4:20 AM, Dharmik Thakkar wrote:
>> Update documentation for 'show fwd' testpmd runtime function to show
>> CPU cycles/packet example.
>>
>> Signed-off-by: Dharmik Thakkar <dharmik.thakkar@arm.com>
>> Reviewed-by: Honnappa Nagarahalli <honnappa.nagarahalli@arm.com>
>> Reviewed-by: Phil Yang <phil.yang@arm.com>
> 
> Reviewed-by: Ferruh Yigit <ferruh.yigit@intel.com>
> 

Applied to dpdk-next-net/master, thanks.

(note that not all patches in the patchset applied, only 1/2 applied)

Re: [dpdk-dev] [PATCH v3 2/2] doc: include config options in testpmd user guide

[Ferruh Yigit]

On 5/20/2020 4:20 AM, Dharmik Thakkar wrote:
> Update testpmd documentation to include RECORD configuration options,
> CONFIG_RTE_TEST_PMD_RECORD_CORE_CYCLES and
> CONFIG_RTE_TEST_PMD_RECORD_BURST_STATS.
> 
> Signed-off-by: Dharmik Thakkar <dharmik.thakkar@arm.com>
> Reviewed-by: Honnappa Nagarahalli <honnappa.nagarahalli@arm.com>
> Reviewed-by: Phil Yang <phil.yang@arm.com>
> ---
> v3:
>  - Replace config/common_base with build/.config (Thomas)
> 
> v2:
>  - Remove extra '#'.
> ---
>  doc/guides/testpmd_app_ug/build_app.rst | 12 ++++++++++++
>  1 file changed, 12 insertions(+)
> 
> diff --git a/doc/guides/testpmd_app_ug/build_app.rst b/doc/guides/testpmd_app_ug/build_app.rst
> index d1ca9f3d19a9..8c9aaa83187f 100644
> --- a/doc/guides/testpmd_app_ug/build_app.rst
> +++ b/doc/guides/testpmd_app_ug/build_app.rst
> @@ -21,6 +21,18 @@ The basic compilation steps are:
>  
>          export RTE_TARGET=x86_64-native-linux-gcc
>  
> +#.  If required, enable configuration options. For example:
> +
> +    .. code-block:: console
> +
> +        cd to the top-level DPDK directory
> +        sed -i 's,\(CONFIG_RTE_TEST_PMD_RECORD_CORE_CYCLES\)=n,\1=y,' build/.config
> +        sed -i 's,\(CONFIG_RTE_TEST_PMD_RECORD_BURST_STATS\)=n,\1=y,' build/.config
> +
> +    Enabling CONFIG_RTE_TEST_PMD_RECORD_CORE_CYCLES enables measurement of CPU cycles.
> +
> +    Enabling CONFIG_RTE_TEST_PMD_RECORD_BURST_STATS enables display of RX and TX bursts.
> +
>  #.  Build the application:
>  
>      .. code-block:: console
> 

Hi Dharmik,

This patch waiting to address the Thomas comment on document how to enable those
config options in meson. @Thomas, can you please correct if I got it wrong?

Re: [dpdk-dev] [PATCH v3 2/2] doc: include config options in testpmd user guide

[Thomas Monjalon]

17/06/2020 20:21, Ferruh Yigit:
> On 5/20/2020 4:20 AM, Dharmik Thakkar wrote:
> > Update testpmd documentation to include RECORD configuration options,
> > CONFIG_RTE_TEST_PMD_RECORD_CORE_CYCLES and
> > CONFIG_RTE_TEST_PMD_RECORD_BURST_STATS.
> > 
> > Signed-off-by: Dharmik Thakkar <dharmik.thakkar@arm.com>
> > Reviewed-by: Honnappa Nagarahalli <honnappa.nagarahalli@arm.com>
> > Reviewed-by: Phil Yang <phil.yang@arm.com>
> > ---
> > v3:
> >  - Replace config/common_base with build/.config (Thomas)
> > 
> > v2:
> >  - Remove extra '#'.
> > ---
> >  doc/guides/testpmd_app_ug/build_app.rst | 12 ++++++++++++
> >  1 file changed, 12 insertions(+)
> > 
> > diff --git a/doc/guides/testpmd_app_ug/build_app.rst b/doc/guides/testpmd_app_ug/build_app.rst
> > index d1ca9f3d19a9..8c9aaa83187f 100644
> > --- a/doc/guides/testpmd_app_ug/build_app.rst
> > +++ b/doc/guides/testpmd_app_ug/build_app.rst
> > @@ -21,6 +21,18 @@ The basic compilation steps are:
> >  
> >          export RTE_TARGET=x86_64-native-linux-gcc
> >  
> > +#.  If required, enable configuration options. For example:
> > +
> > +    .. code-block:: console
> > +
> > +        cd to the top-level DPDK directory
> > +        sed -i 's,\(CONFIG_RTE_TEST_PMD_RECORD_CORE_CYCLES\)=n,\1=y,' build/.config
> > +        sed -i 's,\(CONFIG_RTE_TEST_PMD_RECORD_BURST_STATS\)=n,\1=y,' build/.config
> > +
> > +    Enabling CONFIG_RTE_TEST_PMD_RECORD_CORE_CYCLES enables measurement of CPU cycles.
> > +
> > +    Enabling CONFIG_RTE_TEST_PMD_RECORD_BURST_STATS enables display of RX and TX bursts.
> > +
> >  #.  Build the application:
> >  
> >      .. code-block:: console
> > 
> 
> Hi Dharmik,
> 
> This patch waiting to address the Thomas comment on document how to enable those
> config options in meson. @Thomas, can you please correct if I got it wrong?

If the doc is only about make, it will be removed in 2 months.
So yes, I think it doesn't make sense to be merged as is.

Re: [dpdk-dev] [PATCH v3 2/2] doc: include config options in testpmd user guide

[Dharmik Thakkar]

+ Bruce

> On Jun 17, 2020, at 2:48 PM, Thomas Monjalon <thomas@monjalon.net> wrote:
> 
> 17/06/2020 20:21, Ferruh Yigit:
>> On 5/20/2020 4:20 AM, Dharmik Thakkar wrote:
>>> Update testpmd documentation to include RECORD configuration options,
>>> CONFIG_RTE_TEST_PMD_RECORD_CORE_CYCLES and
>>> CONFIG_RTE_TEST_PMD_RECORD_BURST_STATS.
>>> 
>>> Signed-off-by: Dharmik Thakkar <dharmik.thakkar@arm.com>
>>> Reviewed-by: Honnappa Nagarahalli <honnappa.nagarahalli@arm.com>
>>> Reviewed-by: Phil Yang <phil.yang@arm.com>
>>> ---
>>> v3:
>>> - Replace config/common_base with build/.config (Thomas)
>>> 
>>> v2:
>>> - Remove extra '#'.
>>> ---
>>> doc/guides/testpmd_app_ug/build_app.rst | 12 ++++++++++++
>>> 1 file changed, 12 insertions(+)
>>> 
>>> diff --git a/doc/guides/testpmd_app_ug/build_app.rst b/doc/guides/testpmd_app_ug/build_app.rst
>>> index d1ca9f3d19a9..8c9aaa83187f 100644
>>> --- a/doc/guides/testpmd_app_ug/build_app.rst
>>> +++ b/doc/guides/testpmd_app_ug/build_app.rst
>>> @@ -21,6 +21,18 @@ The basic compilation steps are:
>>> 
>>>         export RTE_TARGET=x86_64-native-linux-gcc
>>> 
>>> +#.  If required, enable configuration options. For example:
>>> +
>>> +    .. code-block:: console
>>> +
>>> +        cd to the top-level DPDK directory
>>> +        sed -i 's,\(CONFIG_RTE_TEST_PMD_RECORD_CORE_CYCLES\)=n,\1=y,' build/.config
>>> +        sed -i 's,\(CONFIG_RTE_TEST_PMD_RECORD_BURST_STATS\)=n,\1=y,' build/.config
>>> +
>>> +    Enabling CONFIG_RTE_TEST_PMD_RECORD_CORE_CYCLES enables measurement of CPU cycles.
>>> +
>>> +    Enabling CONFIG_RTE_TEST_PMD_RECORD_BURST_STATS enables display of RX and TX bursts.
>>> +
>>> #.  Build the application:
>>> 
>>>     .. code-block:: console
>>> 
>> 
>> Hi Dharmik,
>> 
>> This patch waiting to address the Thomas comment on document how to enable those
>> config options in meson. @Thomas, can you please correct if I got it wrong?
> 
> If the doc is only about make, it will be removed in 2 months.
> So yes, I think it doesn't make sense to be merged as is.
> 

Make sense.

With meson, I see multiple ways to enable configuration options:

1] macro in $(RTE_SDK)/config/rte_config.h
2] using dpdk_conf.set() in $(RTE_SDK)/config/meson.build

I’d appreciate some suggestions on the best way to enable these options.

> 
> 

Re: [dpdk-dev] [PATCH v3 2/2] doc: include config options in testpmd user guide

[Thomas Monjalon]

19/06/2020 17:38, Dharmik Thakkar:
> + Bruce
> 
> > On Jun 17, 2020, at 2:48 PM, Thomas Monjalon <thomas@monjalon.net> wrote:
> > 
> > 17/06/2020 20:21, Ferruh Yigit:
> >> On 5/20/2020 4:20 AM, Dharmik Thakkar wrote:
> >>> Update testpmd documentation to include RECORD configuration options,
> >>> CONFIG_RTE_TEST_PMD_RECORD_CORE_CYCLES and
> >>> CONFIG_RTE_TEST_PMD_RECORD_BURST_STATS.
> >>> 
> >>> Signed-off-by: Dharmik Thakkar <dharmik.thakkar@arm.com>
> >>> Reviewed-by: Honnappa Nagarahalli <honnappa.nagarahalli@arm.com>
> >>> Reviewed-by: Phil Yang <phil.yang@arm.com>
> >>> ---
> >>> v3:
> >>> - Replace config/common_base with build/.config (Thomas)
> >>> 
> >>> v2:
> >>> - Remove extra '#'.
> >>> ---
> >>> doc/guides/testpmd_app_ug/build_app.rst | 12 ++++++++++++
> >>> 1 file changed, 12 insertions(+)
> >>> 
> >>> diff --git a/doc/guides/testpmd_app_ug/build_app.rst b/doc/guides/testpmd_app_ug/build_app.rst
> >>> index d1ca9f3d19a9..8c9aaa83187f 100644
> >>> --- a/doc/guides/testpmd_app_ug/build_app.rst
> >>> +++ b/doc/guides/testpmd_app_ug/build_app.rst
> >>> @@ -21,6 +21,18 @@ The basic compilation steps are:
> >>> 
> >>>         export RTE_TARGET=x86_64-native-linux-gcc
> >>> 
> >>> +#.  If required, enable configuration options. For example:
> >>> +
> >>> +    .. code-block:: console
> >>> +
> >>> +        cd to the top-level DPDK directory
> >>> +        sed -i 's,\(CONFIG_RTE_TEST_PMD_RECORD_CORE_CYCLES\)=n,\1=y,' build/.config
> >>> +        sed -i 's,\(CONFIG_RTE_TEST_PMD_RECORD_BURST_STATS\)=n,\1=y,' build/.config
> >>> +
> >>> +    Enabling CONFIG_RTE_TEST_PMD_RECORD_CORE_CYCLES enables measurement of CPU cycles.
> >>> +
> >>> +    Enabling CONFIG_RTE_TEST_PMD_RECORD_BURST_STATS enables display of RX and TX bursts.
> >>> +
> >>> #.  Build the application:
> >>> 
> >>>     .. code-block:: console
> >>> 
> >> 
> >> Hi Dharmik,
> >> 
> >> This patch waiting to address the Thomas comment on document how to enable those
> >> config options in meson. @Thomas, can you please correct if I got it wrong?
> > 
> > If the doc is only about make, it will be removed in 2 months.
> > So yes, I think it doesn't make sense to be merged as is.
> > 
> 
> Make sense.
> 
> With meson, I see multiple ways to enable configuration options:
> 
> 1] macro in $(RTE_SDK)/config/rte_config.h
> 2] using dpdk_conf.set() in $(RTE_SDK)/config/meson.build
> 
> I’d appreciate some suggestions on the best way to enable these options.

Neither 1 nor 2 because both are source files.
Configuration must be done in the build directory.

We use "meson -D" to configure options listed in meson_options.txt
The real question is which option we accept to be added.

Re: [dpdk-dev] [PATCH v3 2/2] doc: include config options in testpmd user guide

[Dharmik Thakkar]

> On Jun 19, 2020, at 10:43 AM, Thomas Monjalon <thomas@monjalon.net> wrote:
> 
> 19/06/2020 17:38, Dharmik Thakkar:
>> + Bruce
>> 
>>> On Jun 17, 2020, at 2:48 PM, Thomas Monjalon <thomas@monjalon.net> wrote:
>>> 
>>> 17/06/2020 20:21, Ferruh Yigit:
>>>> On 5/20/2020 4:20 AM, Dharmik Thakkar wrote:
>>>>> Update testpmd documentation to include RECORD configuration options,
>>>>> CONFIG_RTE_TEST_PMD_RECORD_CORE_CYCLES and
>>>>> CONFIG_RTE_TEST_PMD_RECORD_BURST_STATS.
>>>>> 
>>>>> Signed-off-by: Dharmik Thakkar <dharmik.thakkar@arm.com>
>>>>> Reviewed-by: Honnappa Nagarahalli <honnappa.nagarahalli@arm.com>
>>>>> Reviewed-by: Phil Yang <phil.yang@arm.com>
>>>>> ---
>>>>> v3:
>>>>> - Replace config/common_base with build/.config (Thomas)
>>>>> 
>>>>> v2:
>>>>> - Remove extra '#'.
>>>>> ---
>>>>> doc/guides/testpmd_app_ug/build_app.rst | 12 ++++++++++++
>>>>> 1 file changed, 12 insertions(+)
>>>>> 
>>>>> diff --git a/doc/guides/testpmd_app_ug/build_app.rst b/doc/guides/testpmd_app_ug/build_app.rst
>>>>> index d1ca9f3d19a9..8c9aaa83187f 100644
>>>>> --- a/doc/guides/testpmd_app_ug/build_app.rst
>>>>> +++ b/doc/guides/testpmd_app_ug/build_app.rst
>>>>> @@ -21,6 +21,18 @@ The basic compilation steps are:
>>>>> 
>>>>>        export RTE_TARGET=x86_64-native-linux-gcc
>>>>> 
>>>>> +#.  If required, enable configuration options. For example:
>>>>> +
>>>>> +    .. code-block:: console
>>>>> +
>>>>> +        cd to the top-level DPDK directory
>>>>> +        sed -i 's,\(CONFIG_RTE_TEST_PMD_RECORD_CORE_CYCLES\)=n,\1=y,' build/.config
>>>>> +        sed -i 's,\(CONFIG_RTE_TEST_PMD_RECORD_BURST_STATS\)=n,\1=y,' build/.config
>>>>> +
>>>>> +    Enabling CONFIG_RTE_TEST_PMD_RECORD_CORE_CYCLES enables measurement of CPU cycles.
>>>>> +
>>>>> +    Enabling CONFIG_RTE_TEST_PMD_RECORD_BURST_STATS enables display of RX and TX bursts.
>>>>> +
>>>>> #.  Build the application:
>>>>> 
>>>>>    .. code-block:: console
>>>>> 
>>>> 
>>>> Hi Dharmik,
>>>> 
>>>> This patch waiting to address the Thomas comment on document how to enable those
>>>> config options in meson. @Thomas, can you please correct if I got it wrong?
>>> 
>>> If the doc is only about make, it will be removed in 2 months.
>>> So yes, I think it doesn't make sense to be merged as is.
>>> 
>> 
>> Make sense.
>> 
>> With meson, I see multiple ways to enable configuration options:
>> 
>> 1] macro in $(RTE_SDK)/config/rte_config.h
>> 2] using dpdk_conf.set() in $(RTE_SDK)/config/meson.build
>> 
>> I’d appreciate some suggestions on the best way to enable these options.
> 
> Neither 1 nor 2 because both are source files.
> Configuration must be done in the build directory.
> 
> We use "meson -D" to configure options listed in meson_options.txt
> The real question is which option we accept to be added.
> 
> 

Yes, correct. There are suggestions for making these as runtime configuration functions which can be enabled(or disabled) using ’set’ from within the application.
Do you think, till this change is implemented, these options can be included in meson_options.txt?

> 

Re: [dpdk-dev] [PATCH v3 2/2] doc: include config options in testpmd user guide

[Thomas Monjalon]

19/06/2020 18:13, Dharmik Thakkar:
> > On Jun 19, 2020, at 10:43 AM, Thomas Monjalon <thomas@monjalon.net> wrote:
> > 19/06/2020 17:38, Dharmik Thakkar:
> >>> On Jun 17, 2020, at 2:48 PM, Thomas Monjalon <thomas@monjalon.net> wrote:
> >>> 17/06/2020 20:21, Ferruh Yigit:
> >>>> On 5/20/2020 4:20 AM, Dharmik Thakkar wrote:
> >>>>> Update testpmd documentation to include RECORD configuration options,
> >>>>> CONFIG_RTE_TEST_PMD_RECORD_CORE_CYCLES and
> >>>>> CONFIG_RTE_TEST_PMD_RECORD_BURST_STATS.
> >>>>> 
> >>>>> Signed-off-by: Dharmik Thakkar <dharmik.thakkar@arm.com>
> >>>>> Reviewed-by: Honnappa Nagarahalli <honnappa.nagarahalli@arm.com>
> >>>>> Reviewed-by: Phil Yang <phil.yang@arm.com>
> >>>>> ---
> >>>>> v3:
> >>>>> - Replace config/common_base with build/.config (Thomas)
> >>>>> 
> >>>>> v2:
> >>>>> - Remove extra '#'.
> >>>>> ---
> >>>>> doc/guides/testpmd_app_ug/build_app.rst | 12 ++++++++++++
> >>>>> 1 file changed, 12 insertions(+)
> >>>>> 
> >>>>> diff --git a/doc/guides/testpmd_app_ug/build_app.rst b/doc/guides/testpmd_app_ug/build_app.rst
> >>>>> index d1ca9f3d19a9..8c9aaa83187f 100644
> >>>>> --- a/doc/guides/testpmd_app_ug/build_app.rst
> >>>>> +++ b/doc/guides/testpmd_app_ug/build_app.rst
> >>>>> @@ -21,6 +21,18 @@ The basic compilation steps are:
> >>>>> 
> >>>>>        export RTE_TARGET=x86_64-native-linux-gcc
> >>>>> 
> >>>>> +#.  If required, enable configuration options. For example:
> >>>>> +
> >>>>> +    .. code-block:: console
> >>>>> +
> >>>>> +        cd to the top-level DPDK directory
> >>>>> +        sed -i 's,\(CONFIG_RTE_TEST_PMD_RECORD_CORE_CYCLES\)=n,\1=y,' build/.config
> >>>>> +        sed -i 's,\(CONFIG_RTE_TEST_PMD_RECORD_BURST_STATS\)=n,\1=y,' build/.config
> >>>>> +
> >>>>> +    Enabling CONFIG_RTE_TEST_PMD_RECORD_CORE_CYCLES enables measurement of CPU cycles.
> >>>>> +
> >>>>> +    Enabling CONFIG_RTE_TEST_PMD_RECORD_BURST_STATS enables display of RX and TX bursts.
> >>>>> +
> >>>>> #.  Build the application:
> >>>>> 
> >>>>>    .. code-block:: console
> >>>>> 
> >>>> 
> >>>> Hi Dharmik,
> >>>> 
> >>>> This patch waiting to address the Thomas comment on document how to enable those
> >>>> config options in meson. @Thomas, can you please correct if I got it wrong?
> >>> 
> >>> If the doc is only about make, it will be removed in 2 months.
> >>> So yes, I think it doesn't make sense to be merged as is.
> >>> 
> >> 
> >> Make sense.
> >> 
> >> With meson, I see multiple ways to enable configuration options:
> >> 
> >> 1] macro in $(RTE_SDK)/config/rte_config.h
> >> 2] using dpdk_conf.set() in $(RTE_SDK)/config/meson.build
> >> 
> >> I’d appreciate some suggestions on the best way to enable these options.
> > 
> > Neither 1 nor 2 because both are source files.
> > Configuration must be done in the build directory.
> > 
> > We use "meson -D" to configure options listed in meson_options.txt
> > The real question is which option we accept to be added.
> > 
> > 
> 
> Yes, correct. There are suggestions for making these as runtime configuration functions which can be enabled(or disabled) using ’set’ from within the application.

I don't understand this proposal.
Is there a patch?

> Do you think, till this change is implemented, these options can be included in meson_options.txt?

In general, if you think there is a better alternative,
please propose a patch.

I have no opinion what is best to manage such option.

Re: [dpdk-dev] [PATCH v3 2/2] doc: include config options in testpmd user guide

[Dharmik Thakkar]

> On Jun 22, 2020, at 10:26 AM, Thomas Monjalon <thomas@monjalon.net> wrote:
> 
> 19/06/2020 18:13, Dharmik Thakkar:
>>> On Jun 19, 2020, at 10:43 AM, Thomas Monjalon <thomas@monjalon.net> wrote:
>>> 19/06/2020 17:38, Dharmik Thakkar:
>>>>> On Jun 17, 2020, at 2:48 PM, Thomas Monjalon <thomas@monjalon.net> wrote:
>>>>> 17/06/2020 20:21, Ferruh Yigit:
>>>>>> On 5/20/2020 4:20 AM, Dharmik Thakkar wrote:
>>>>>>> Update testpmd documentation to include RECORD configuration options,
>>>>>>> CONFIG_RTE_TEST_PMD_RECORD_CORE_CYCLES and
>>>>>>> CONFIG_RTE_TEST_PMD_RECORD_BURST_STATS.
>>>>>>> 
>>>>>>> Signed-off-by: Dharmik Thakkar <dharmik.thakkar@arm.com>
>>>>>>> Reviewed-by: Honnappa Nagarahalli <honnappa.nagarahalli@arm.com>
>>>>>>> Reviewed-by: Phil Yang <phil.yang@arm.com>
>>>>>>> ---
>>>>>>> v3:
>>>>>>> - Replace config/common_base with build/.config (Thomas)
>>>>>>> 
>>>>>>> v2:
>>>>>>> - Remove extra '#'.
>>>>>>> ---
>>>>>>> doc/guides/testpmd_app_ug/build_app.rst | 12 ++++++++++++
>>>>>>> 1 file changed, 12 insertions(+)
>>>>>>> 
>>>>>>> diff --git a/doc/guides/testpmd_app_ug/build_app.rst b/doc/guides/testpmd_app_ug/build_app.rst
>>>>>>> index d1ca9f3d19a9..8c9aaa83187f 100644
>>>>>>> --- a/doc/guides/testpmd_app_ug/build_app.rst
>>>>>>> +++ b/doc/guides/testpmd_app_ug/build_app.rst
>>>>>>> @@ -21,6 +21,18 @@ The basic compilation steps are:
>>>>>>> 
>>>>>>>       export RTE_TARGET=x86_64-native-linux-gcc
>>>>>>> 
>>>>>>> +#.  If required, enable configuration options. For example:
>>>>>>> +
>>>>>>> +    .. code-block:: console
>>>>>>> +
>>>>>>> +        cd to the top-level DPDK directory
>>>>>>> +        sed -i 's,\(CONFIG_RTE_TEST_PMD_RECORD_CORE_CYCLES\)=n,\1=y,' build/.config
>>>>>>> +        sed -i 's,\(CONFIG_RTE_TEST_PMD_RECORD_BURST_STATS\)=n,\1=y,' build/.config
>>>>>>> +
>>>>>>> +    Enabling CONFIG_RTE_TEST_PMD_RECORD_CORE_CYCLES enables measurement of CPU cycles.
>>>>>>> +
>>>>>>> +    Enabling CONFIG_RTE_TEST_PMD_RECORD_BURST_STATS enables display of RX and TX bursts.
>>>>>>> +
>>>>>>> #.  Build the application:
>>>>>>> 
>>>>>>>   .. code-block:: console
>>>>>>> 
>>>>>> 
>>>>>> Hi Dharmik,
>>>>>> 
>>>>>> This patch waiting to address the Thomas comment on document how to enable those
>>>>>> config options in meson. @Thomas, can you please correct if I got it wrong?
>>>>> 
>>>>> If the doc is only about make, it will be removed in 2 months.
>>>>> So yes, I think it doesn't make sense to be merged as is.
>>>>> 
>>>> 
>>>> Make sense.
>>>> 
>>>> With meson, I see multiple ways to enable configuration options:
>>>> 
>>>> 1] macro in $(RTE_SDK)/config/rte_config.h
>>>> 2] using dpdk_conf.set() in $(RTE_SDK)/config/meson.build
>>>> 
>>>> I’d appreciate some suggestions on the best way to enable these options.
>>> 
>>> Neither 1 nor 2 because both are source files.
>>> Configuration must be done in the build directory.
>>> 
>>> We use "meson -D" to configure options listed in meson_options.txt
>>> The real question is which option we accept to be added.
>>> 
>>> 
>> 
>> Yes, correct. There are suggestions for making these as runtime configuration functions which can be enabled(or disabled) using ’set’ from within the application.
> 
> I don't understand this proposal.
> Is there a patch?
> 
>> Do you think, till this change is implemented, these options can be included in meson_options.txt?
> 
> In general, if you think there is a better alternative,
> please propose a patch.

I am not sure if changing it to a runtime function is a better alternative.
I am expecting some performance degradation with the change since the macros will be replaced with ‘if’ statements.
I will hack up a patch to see the performance difference and propose if there is not much of a difference.

> 
> I have no opinion what is best to manage such option.

[dpdk-dev] [PATCH 0/6] app/testpmd: add runtime config

[Dharmik Thakkar]

Meson build system lacks support for
CONFIG_RTE_TEST_PMD_RECORD_CORE_CYCLES and
CONFIG_RTE_TEST_PMD_RECORD_BURST_STATS configuration options.

One solution is to add these options within meson_options.txt

Since adding these as runtime config causes no performance impact,
it makes sense to enable these as runtime config alongside other
available runtime options within testpmd.

Dharmik Thakkar (5):
  app/testpmd: add record-core-cycles runtime config
  doc: add record-core-cycles to testpmd funcs doc
  app/testpmd: add record-burst-stats runtime config
  doc: add record-burst-stats to testpmd funcs doc
  app/testpmd: enable empty polls in 5tswap

Phil Yang (1):
  app/testpmd: enable burst stats for noisy vnf mode

 app/test-pmd/5tswap.c                       | 25 ++----
 app/test-pmd/cmdline.c                      | 92 +++++++++++++++++++++
 app/test-pmd/config.c                       | 12 +++
 app/test-pmd/csumonly.c                     | 24 ++----
 app/test-pmd/flowgen.c                      | 21 ++---
 app/test-pmd/icmpecho.c                     | 24 ++----
 app/test-pmd/iofwd.c                        | 26 ++----
 app/test-pmd/macfwd.c                       | 25 ++----
 app/test-pmd/macswap.c                      | 24 ++----
 app/test-pmd/noisy_vnf.c                    |  4 +
 app/test-pmd/parameters.c                   | 11 ++-
 app/test-pmd/rxonly.c                       | 19 +----
 app/test-pmd/testpmd.c                      | 77 ++++++++---------
 app/test-pmd/testpmd.h                      | 38 +++++++--
 app/test-pmd/txonly.c                       | 20 +----
 doc/guides/testpmd_app_ug/run_app.rst       |  8 ++
 doc/guides/testpmd_app_ug/testpmd_funcs.rst | 30 +++++++
 17 files changed, 272 insertions(+), 208 deletions(-)

-- 
2.17.1

[dpdk-dev] [PATCH 1/6] app/testpmd: add record-core-cycles runtime config

[Dharmik Thakkar]

Convert CONFIG_RTE_TEST_PMD_RECORD_CORE_CYCLES to a
runtime configuration.

Signed-off-by: Dharmik Thakkar <dharmik.thakkar@arm.com>
Tested-by: Phil Yang <phil.yang@arm.com>
Reviewed-by: Ruifeng Wang <ruifeng.wang@arm.com>
Reviewed-by: Honnappa Nagarahalli <honnappa.nagarahalli@arm.com>
---
 app/test-pmd/5tswap.c     | 16 +++-----------
 app/test-pmd/cmdline.c    | 46 +++++++++++++++++++++++++++++++++++++++
 app/test-pmd/config.c     |  6 +++++
 app/test-pmd/csumonly.c   | 16 +++-----------
 app/test-pmd/flowgen.c    | 17 ++++-----------
 app/test-pmd/icmpecho.c   | 16 +++-----------
 app/test-pmd/iofwd.c      | 18 ++++-----------
 app/test-pmd/macfwd.c     | 17 ++++-----------
 app/test-pmd/macswap.c    | 16 +++-----------
 app/test-pmd/parameters.c |  6 ++++-
 app/test-pmd/rxonly.c     | 15 +++----------
 app/test-pmd/testpmd.c    | 44 ++++++++++++++++++-------------------
 app/test-pmd/testpmd.h    | 18 +++++++++++++--
 app/test-pmd/txonly.c     | 16 +++-----------
 14 files changed, 125 insertions(+), 142 deletions(-)

diff --git a/app/test-pmd/5tswap.c b/app/test-pmd/5tswap.c
index 972f6b93b31d..04f035bb450f 100644
--- a/app/test-pmd/5tswap.c
+++ b/app/test-pmd/5tswap.c
@@ -105,15 +105,9 @@ pkt_burst_5tuple_swap(struct fwd_stream *fs)
 		uint8_t *byte;
 	} h;
 
-#ifdef RTE_TEST_PMD_RECORD_CORE_CYCLES
-	uint64_t start_tsc;
-	uint64_t end_tsc;
-	uint64_t core_cycles;
-#endif
+	uint64_t start_tsc = 0;
 
-#ifdef RTE_TEST_PMD_RECORD_CORE_CYCLES
-	start_tsc = rte_rdtsc();
-#endif
+	get_start_cycles(&start_tsc);
 
 	/*
 	 * Receive a burst of packets and forward them.
@@ -193,11 +187,7 @@ pkt_burst_5tuple_swap(struct fwd_stream *fs)
 			rte_pktmbuf_free(pkts_burst[nb_tx]);
 		} while (++nb_tx < nb_rx);
 	}
-#ifdef RTE_TEST_PMD_RECORD_CORE_CYCLES
-	end_tsc = rte_rdtsc();
-	core_cycles = (end_tsc - start_tsc);
-	fs->core_cycles = (uint64_t) (fs->core_cycles + core_cycles);
-#endif
+	get_end_cycles(fs, start_tsc);
 }
 
 struct fwd_engine five_tuple_swap_fwd_engine = {
diff --git a/app/test-pmd/cmdline.c b/app/test-pmd/cmdline.c
index 39a1e772ed7c..1f30a9021500 100644
--- a/app/test-pmd/cmdline.c
+++ b/app/test-pmd/cmdline.c
@@ -529,6 +529,9 @@ static void cmd_help_long_parsed(void *parsed_result,
 			"    Set the option to hide the zero values"
 			" for xstats display.\n"
 
+			"set record-core-cycles on|off\n"
+			"    Set the option to enable measurement of CPU cycles.\n"
+
 			"set port (port_id) vf (vf_id) rx|tx on|off\n"
 			"    Enable/Disable a VF receive/tranmit from a port\n\n"
 
@@ -8335,6 +8338,48 @@ cmdline_parse_inst_t cmd_set_xstats_hide_zero = {
 	},
 };
 
+/* *** SET OPTION TO ENABLE MEASUREMENT OF CPU CYCLES *** */
+struct cmd_set_record_core_cycles_result {
+	cmdline_fixed_string_t keyword;
+	cmdline_fixed_string_t name;
+	cmdline_fixed_string_t on_off;
+};
+
+static void
+cmd_set_record_core_cycles_parsed(void *parsed_result,
+			__rte_unused struct cmdline *cl,
+			__rte_unused void *data)
+{
+	struct cmd_set_record_core_cycles_result *res;
+	uint16_t on_off = 0;
+
+	res = parsed_result;
+	on_off = !strcmp(res->on_off, "on") ? 1 : 0;
+	set_record_core_cycles(on_off);
+}
+
+cmdline_parse_token_string_t cmd_set_record_core_cycles_keyword =
+	TOKEN_STRING_INITIALIZER(struct cmd_set_record_core_cycles_result,
+				 keyword, "set");
+cmdline_parse_token_string_t cmd_set_record_core_cycles_name =
+	TOKEN_STRING_INITIALIZER(struct cmd_set_record_core_cycles_result,
+				 name, "record-core-cycles");
+cmdline_parse_token_string_t cmd_set_record_core_cycles_on_off =
+	TOKEN_STRING_INITIALIZER(struct cmd_set_record_core_cycles_result,
+				 on_off, "on#off");
+
+cmdline_parse_inst_t cmd_set_record_core_cycles = {
+	.f = cmd_set_record_core_cycles_parsed,
+	.data = NULL,
+	.help_str = "set record-core-cycles on|off",
+	.tokens = {
+		(void *)&cmd_set_record_core_cycles_keyword,
+		(void *)&cmd_set_record_core_cycles_name,
+		(void *)&cmd_set_record_core_cycles_on_off,
+		NULL,
+	},
+};
+
 /* *** CONFIGURE UNICAST HASH TABLE *** */
 struct cmd_set_uc_hash_table {
 	cmdline_fixed_string_t set;
@@ -19487,6 +19532,7 @@ cmdline_parse_ctx_t main_ctx[] = {
 	(cmdline_parse_inst_t *)&cmd_set_fwd_eth_peer,
 	(cmdline_parse_inst_t *)&cmd_set_qmap,
 	(cmdline_parse_inst_t *)&cmd_set_xstats_hide_zero,
+	(cmdline_parse_inst_t *)&cmd_set_record_core_cycles,
 	(cmdline_parse_inst_t *)&cmd_operate_port,
 	(cmdline_parse_inst_t *)&cmd_operate_specific_port,
 	(cmdline_parse_inst_t *)&cmd_operate_attach_port,
diff --git a/app/test-pmd/config.c b/app/test-pmd/config.c
index fcbe6b6f7440..d93699b34e3a 100644
--- a/app/test-pmd/config.c
+++ b/app/test-pmd/config.c
@@ -3679,6 +3679,12 @@ set_xstats_hide_zero(uint8_t on_off)
 	xstats_hide_zero = on_off;
 }
 
+void
+set_record_core_cycles(uint8_t on_off)
+{
+	record_core_cycles = on_off;
+}
+
 static inline void
 print_fdir_mask(struct rte_eth_fdir_masks *mask)
 {
diff --git a/app/test-pmd/csumonly.c b/app/test-pmd/csumonly.c
index 0a96b24eb27b..836bafa0f6df 100644
--- a/app/test-pmd/csumonly.c
+++ b/app/test-pmd/csumonly.c
@@ -789,15 +789,9 @@ pkt_burst_checksum_forward(struct fwd_stream *fs)
 	uint16_t nb_segments = 0;
 	int ret;
 
-#ifdef RTE_TEST_PMD_RECORD_CORE_CYCLES
-	uint64_t start_tsc;
-	uint64_t end_tsc;
-	uint64_t core_cycles;
-#endif
+	uint64_t start_tsc = 0;
 
-#ifdef RTE_TEST_PMD_RECORD_CORE_CYCLES
-	start_tsc = rte_rdtsc();
-#endif
+	get_start_cycles(&start_tsc);
 
 	/* receive a burst of packet */
 	nb_rx = rte_eth_rx_burst(fs->rx_port, fs->rx_queue, pkts_burst,
@@ -1098,11 +1092,7 @@ pkt_burst_checksum_forward(struct fwd_stream *fs)
 		} while (++nb_tx < nb_rx);
 	}
 
-#ifdef RTE_TEST_PMD_RECORD_CORE_CYCLES
-	end_tsc = rte_rdtsc();
-	core_cycles = (end_tsc - start_tsc);
-	fs->core_cycles = (uint64_t) (fs->core_cycles + core_cycles);
-#endif
+	get_end_cycles(fs, start_tsc);
 }
 
 struct fwd_engine csum_fwd_engine = {
diff --git a/app/test-pmd/flowgen.c b/app/test-pmd/flowgen.c
index 4bd351e6754a..e92a0eeca254 100644
--- a/app/test-pmd/flowgen.c
+++ b/app/test-pmd/flowgen.c
@@ -97,16 +97,10 @@ pkt_burst_flow_gen(struct fwd_stream *fs)
 	uint16_t i;
 	uint32_t retry;
 	uint64_t tx_offloads;
-#ifdef RTE_TEST_PMD_RECORD_CORE_CYCLES
-	uint64_t start_tsc;
-	uint64_t end_tsc;
-	uint64_t core_cycles;
-#endif
+	uint64_t start_tsc = 0;
 	static int next_flow = 0;
 
-#ifdef RTE_TEST_PMD_RECORD_CORE_CYCLES
-	start_tsc = rte_rdtsc();
-#endif
+	get_start_cycles(&start_tsc);
 
 	/* Receive a burst of packets and discard them. */
 	nb_rx = rte_eth_rx_burst(fs->rx_port, fs->rx_queue, pkts_burst,
@@ -207,11 +201,8 @@ pkt_burst_flow_gen(struct fwd_stream *fs)
 			rte_pktmbuf_free(pkts_burst[nb_tx]);
 		} while (++nb_tx < nb_pkt);
 	}
-#ifdef RTE_TEST_PMD_RECORD_CORE_CYCLES
-	end_tsc = rte_rdtsc();
-	core_cycles = (end_tsc - start_tsc);
-	fs->core_cycles = (uint64_t) (fs->core_cycles + core_cycles);
-#endif
+
+	get_end_cycles(fs, start_tsc);
 }
 
 struct fwd_engine flow_gen_engine = {
diff --git a/app/test-pmd/icmpecho.c b/app/test-pmd/icmpecho.c
index 78e3adf2c106..3f27e8c29d33 100644
--- a/app/test-pmd/icmpecho.c
+++ b/app/test-pmd/icmpecho.c
@@ -293,15 +293,9 @@ reply_to_icmp_echo_rqsts(struct fwd_stream *fs)
 	uint32_t cksum;
 	uint8_t  i;
 	int l2_len;
-#ifdef RTE_TEST_PMD_RECORD_CORE_CYCLES
-	uint64_t start_tsc;
-	uint64_t end_tsc;
-	uint64_t core_cycles;
-#endif
+	uint64_t start_tsc = 0;
 
-#ifdef RTE_TEST_PMD_RECORD_CORE_CYCLES
-	start_tsc = rte_rdtsc();
-#endif
+	get_start_cycles(&start_tsc);
 
 	/*
 	 * First, receive a burst of packets.
@@ -520,11 +514,7 @@ reply_to_icmp_echo_rqsts(struct fwd_stream *fs)
 		}
 	}
 
-#ifdef RTE_TEST_PMD_RECORD_CORE_CYCLES
-	end_tsc = rte_rdtsc();
-	core_cycles = (end_tsc - start_tsc);
-	fs->core_cycles = (uint64_t) (fs->core_cycles + core_cycles);
-#endif
+	get_end_cycles(fs, start_tsc);
 }
 
 struct fwd_engine icmp_echo_engine = {
diff --git a/app/test-pmd/iofwd.c b/app/test-pmd/iofwd.c
index 22b59bbdaf5a..456f7b5925ed 100644
--- a/app/test-pmd/iofwd.c
+++ b/app/test-pmd/iofwd.c
@@ -50,16 +50,9 @@ pkt_burst_io_forward(struct fwd_stream *fs)
 	uint16_t nb_rx;
 	uint16_t nb_tx;
 	uint32_t retry;
+	uint64_t start_tsc = 0;
 
-#ifdef RTE_TEST_PMD_RECORD_CORE_CYCLES
-	uint64_t start_tsc;
-	uint64_t end_tsc;
-	uint64_t core_cycles;
-#endif
-
-#ifdef RTE_TEST_PMD_RECORD_CORE_CYCLES
-	start_tsc = rte_rdtsc();
-#endif
+	get_start_cycles(&start_tsc);
 
 	/*
 	 * Receive a burst of packets and forward them.
@@ -96,11 +89,8 @@ pkt_burst_io_forward(struct fwd_stream *fs)
 			rte_pktmbuf_free(pkts_burst[nb_tx]);
 		} while (++nb_tx < nb_rx);
 	}
-#ifdef RTE_TEST_PMD_RECORD_CORE_CYCLES
-	end_tsc = rte_rdtsc();
-	core_cycles = (end_tsc - start_tsc);
-	fs->core_cycles = (uint64_t) (fs->core_cycles + core_cycles);
-#endif
+
+	get_end_cycles(fs, start_tsc);
 }
 
 struct fwd_engine io_fwd_engine = {
diff --git a/app/test-pmd/macfwd.c b/app/test-pmd/macfwd.c
index 0a89d94befe5..6a77568c7e60 100644
--- a/app/test-pmd/macfwd.c
+++ b/app/test-pmd/macfwd.c
@@ -56,15 +56,9 @@ pkt_burst_mac_forward(struct fwd_stream *fs)
 	uint16_t i;
 	uint64_t ol_flags = 0;
 	uint64_t tx_offloads;
-#ifdef RTE_TEST_PMD_RECORD_CORE_CYCLES
-	uint64_t start_tsc;
-	uint64_t end_tsc;
-	uint64_t core_cycles;
-#endif
+	uint64_t start_tsc = 0;
 
-#ifdef RTE_TEST_PMD_RECORD_CORE_CYCLES
-	start_tsc = rte_rdtsc();
-#endif
+	get_start_cycles(&start_tsc);
 
 	/*
 	 * Receive a burst of packets and forward them.
@@ -126,11 +120,8 @@ pkt_burst_mac_forward(struct fwd_stream *fs)
 			rte_pktmbuf_free(pkts_burst[nb_tx]);
 		} while (++nb_tx < nb_rx);
 	}
-#ifdef RTE_TEST_PMD_RECORD_CORE_CYCLES
-	end_tsc = rte_rdtsc();
-	core_cycles = (end_tsc - start_tsc);
-	fs->core_cycles = (uint64_t) (fs->core_cycles + core_cycles);
-#endif
+
+	get_end_cycles(fs, start_tsc);
 }
 
 struct fwd_engine mac_fwd_engine = {
diff --git a/app/test-pmd/macswap.c b/app/test-pmd/macswap.c
index fbe8cb39e5d2..aaef5b0047b1 100644
--- a/app/test-pmd/macswap.c
+++ b/app/test-pmd/macswap.c
@@ -57,15 +57,9 @@ pkt_burst_mac_swap(struct fwd_stream *fs)
 	uint16_t nb_rx;
 	uint16_t nb_tx;
 	uint32_t retry;
-#ifdef RTE_TEST_PMD_RECORD_CORE_CYCLES
-	uint64_t start_tsc;
-	uint64_t end_tsc;
-	uint64_t core_cycles;
-#endif
+	uint64_t start_tsc = 0;
 
-#ifdef RTE_TEST_PMD_RECORD_CORE_CYCLES
-	start_tsc = rte_rdtsc();
-#endif
+	get_start_cycles(&start_tsc);
 
 	/*
 	 * Receive a burst of packets and forward them.
@@ -105,11 +99,7 @@ pkt_burst_mac_swap(struct fwd_stream *fs)
 			rte_pktmbuf_free(pkts_burst[nb_tx]);
 		} while (++nb_tx < nb_rx);
 	}
-#ifdef RTE_TEST_PMD_RECORD_CORE_CYCLES
-	end_tsc = rte_rdtsc();
-	core_cycles = (end_tsc - start_tsc);
-	fs->core_cycles = (uint64_t) (fs->core_cycles + core_cycles);
-#endif
+	get_end_cycles(fs, start_tsc);
 }
 
 struct fwd_engine mac_swap_engine = {
diff --git a/app/test-pmd/parameters.c b/app/test-pmd/parameters.c
index 7cb0e3d6ec0a..76072ae81b93 100644
--- a/app/test-pmd/parameters.c
+++ b/app/test-pmd/parameters.c
@@ -70,7 +70,7 @@ usage(char* progname)
 	       "--rxpt= | --rxht= | --rxwt= | --rxfreet= | "
 	       "--txpt= | --txht= | --txwt= | --txfreet= | "
 	       "--txrst= | --tx-offloads= | | --rx-offloads= | "
-	       "--vxlan-gpe-port= ]\n",
+	       "--vxlan-gpe-port= | --record-core-cycles]\n",
 	       progname);
 #ifdef RTE_LIBRTE_CMDLINE
 	printf("  --interactive: run in interactive mode.\n");
@@ -216,6 +216,7 @@ usage(char* progname)
 	       "valid only with --mp-alloc=anon\n");
 	printf("  --rx-mq-mode=0xX: hexadecimal bitmask of RX mq mode can be "
 	       "enabled\n");
+	printf("  --record-core-cycles: enable measurement of CPU cycles.\n");
 }
 
 #ifdef RTE_LIBRTE_CMDLINE
@@ -677,6 +678,7 @@ launch_args_parse(int argc, char** argv)
 		{ "noisy-lkup-num-reads-writes", 1, 0, 0 },
 		{ "no-iova-contig",             0, 0, 0 },
 		{ "rx-mq-mode",                 1, 0, 0 },
+		{ "record-core-cycles",         0, 0, 0 },
 		{ 0, 0, 0, 0 },
 	};
 
@@ -1381,6 +1383,8 @@ launch_args_parse(int argc, char** argv)
 						 "rx-mq-mode must be >= 0 and <= %d\n",
 						 ETH_MQ_RX_VMDQ_DCB_RSS);
 			}
+			if (!strcmp(lgopts[opt_idx].name, "record-core-cycles"))
+				record_core_cycles = 1;
 			break;
 		case 'h':
 			usage(argv[0]);
diff --git a/app/test-pmd/rxonly.c b/app/test-pmd/rxonly.c
index 18d59a68d652..b6c8b8c7d5db 100644
--- a/app/test-pmd/rxonly.c
+++ b/app/test-pmd/rxonly.c
@@ -49,14 +49,9 @@ pkt_burst_receive(struct fwd_stream *fs)
 	struct rte_mbuf  *pkts_burst[MAX_PKT_BURST];
 	uint16_t nb_rx;
 	uint16_t i;
+	uint64_t start_tsc = 0;
 
-#ifdef RTE_TEST_PMD_RECORD_CORE_CYCLES
-	uint64_t start_tsc;
-	uint64_t end_tsc;
-	uint64_t core_cycles;
-
-	start_tsc = rte_rdtsc();
-#endif
+	get_start_cycles(&start_tsc);
 
 	/*
 	 * Receive a burst of packets.
@@ -73,11 +68,7 @@ pkt_burst_receive(struct fwd_stream *fs)
 	for (i = 0; i < nb_rx; i++)
 		rte_pktmbuf_free(pkts_burst[i]);
 
-#ifdef RTE_TEST_PMD_RECORD_CORE_CYCLES
-	end_tsc = rte_rdtsc();
-	core_cycles = (end_tsc - start_tsc);
-	fs->core_cycles = (uint64_t) (fs->core_cycles + core_cycles);
-#endif
+	get_end_cycles(fs, start_tsc);
 }
 
 struct fwd_engine rx_only_engine = {
diff --git a/app/test-pmd/testpmd.c b/app/test-pmd/testpmd.c
index 7842c3b78120..f327b16604d2 100644
--- a/app/test-pmd/testpmd.c
+++ b/app/test-pmd/testpmd.c
@@ -475,6 +475,11 @@ uint16_t nb_rx_queue_stats_mappings = 0;
  */
 uint8_t xstats_hide_zero;
 
+/*
+ * Measure of CPU cycles disabled by default
+ */
+uint8_t record_core_cycles;
+
 unsigned int num_sockets = 0;
 unsigned int socket_ids[RTE_MAX_NUMA_NODES];
 
@@ -1800,9 +1805,7 @@ fwd_stats_display(void)
 	uint64_t total_tx_dropped = 0;
 	uint64_t total_rx_nombuf = 0;
 	struct rte_eth_stats stats;
-#ifdef RTE_TEST_PMD_RECORD_CORE_CYCLES
 	uint64_t fwd_cycles = 0;
-#endif
 	uint64_t total_recv = 0;
 	uint64_t total_xmit = 0;
 	struct rte_port *port;
@@ -1830,9 +1833,8 @@ fwd_stats_display(void)
 		ports_stats[fs->rx_port].rx_bad_outer_l4_csum +=
 				fs->rx_bad_outer_l4_csum;
 
-#ifdef RTE_TEST_PMD_RECORD_CORE_CYCLES
-		fwd_cycles += fs->core_cycles;
-#endif
+		if (record_core_cycles)
+			fwd_cycles += fs->core_cycles;
 	}
 	for (i = 0; i < cur_fwd_config.nb_fwd_ports; i++) {
 		uint8_t j;
@@ -1961,24 +1963,24 @@ fwd_stats_display(void)
 	printf("  %s++++++++++++++++++++++++++++++++++++++++++++++"
 	       "%s\n",
 	       acc_stats_border, acc_stats_border);
-#ifdef RTE_TEST_PMD_RECORD_CORE_CYCLES
+	if (record_core_cycles) {
 #define CYC_PER_MHZ 1E6
-	if (total_recv > 0 || total_xmit > 0) {
-		uint64_t total_pkts = 0;
-		if (strcmp(cur_fwd_eng->fwd_mode_name, "txonly") == 0 ||
-		    strcmp(cur_fwd_eng->fwd_mode_name, "flowgen") == 0)
-			total_pkts = total_xmit;
-		else
-			total_pkts = total_recv;
+		if (total_recv > 0 || total_xmit > 0) {
+			uint64_t total_pkts = 0;
+			if (strcmp(cur_fwd_eng->fwd_mode_name, "txonly") == 0 ||
+			    strcmp(cur_fwd_eng->fwd_mode_name, "flowgen") == 0)
+				total_pkts = total_xmit;
+			else
+				total_pkts = total_recv;
 
-		printf("\n  CPU cycles/packet=%.2F (total cycles="
-		       "%"PRIu64" / total %s packets=%"PRIu64") at %"PRIu64
-		       " MHz Clock\n",
-		       (double) fwd_cycles / total_pkts,
-		       fwd_cycles, cur_fwd_eng->fwd_mode_name, total_pkts,
-		       (uint64_t)(rte_get_tsc_hz() / CYC_PER_MHZ));
+			printf("\n  CPU cycles/packet=%.2F (total cycles="
+			       "%"PRIu64" / total %s packets=%"PRIu64") at %"PRIu64
+			       " MHz Clock\n",
+			       (double) fwd_cycles / total_pkts,
+			       fwd_cycles, cur_fwd_eng->fwd_mode_name, total_pkts,
+			       (uint64_t)(rte_get_tsc_hz() / CYC_PER_MHZ));
+		}
 	}
-#endif
 }
 
 void
@@ -2006,9 +2008,7 @@ fwd_stats_reset(void)
 		memset(&fs->rx_burst_stats, 0, sizeof(fs->rx_burst_stats));
 		memset(&fs->tx_burst_stats, 0, sizeof(fs->tx_burst_stats));
 #endif
-#ifdef RTE_TEST_PMD_RECORD_CORE_CYCLES
 		fs->core_cycles = 0;
-#endif
 	}
 }
 
diff --git a/app/test-pmd/testpmd.h b/app/test-pmd/testpmd.h
index 25a12b14f29d..dcc37be02d59 100644
--- a/app/test-pmd/testpmd.h
+++ b/app/test-pmd/testpmd.h
@@ -129,9 +129,7 @@ struct fwd_stream {
 	uint64_t rx_bad_outer_l4_csum;
 	/**< received packets has bad outer l4 checksum */
 	unsigned int gro_times;	/**< GRO operation times */
-#ifdef RTE_TEST_PMD_RECORD_CORE_CYCLES
 	uint64_t     core_cycles; /**< used for RX and TX processing */
-#endif
 #ifdef RTE_TEST_PMD_RECORD_BURST_STATS
 	struct pkt_burst_stats rx_burst_stats;
 	struct pkt_burst_stats tx_burst_stats;
@@ -301,6 +299,7 @@ extern uint16_t nb_rx_queue_stats_mappings;
 extern uint8_t xstats_hide_zero; /**< Hide zero values for xstats display */
 
 /* globals used for configuration */
+extern uint8_t record_core_cycles; /**< Enables measurement of CPU cycles */
 extern uint16_t verbose_level; /**< Drives messages being displayed, if any. */
 extern int testpmd_logtype; /**< Log type for testpmd logs */
 extern uint8_t  interactive;
@@ -681,6 +680,20 @@ port_pci_reg_write(struct rte_port *port, uint32_t reg_off, uint32_t reg_v)
 #define port_id_pci_reg_write(pt_id, reg_off, reg_value) \
 	port_pci_reg_write(&ports[(pt_id)], (reg_off), (reg_value))
 
+static inline void
+get_start_cycles(uint64_t *start_tsc)
+{
+	if (record_core_cycles)
+		*start_tsc = rte_rdtsc();
+}
+
+static inline void
+get_end_cycles(struct fwd_stream *fs, uint64_t start_tsc)
+{
+	if (record_core_cycles)
+		fs->core_cycles += rte_rdtsc() - start_tsc;
+}
+
 /* Prototypes */
 unsigned int parse_item_list(char* str, const char* item_name,
 			unsigned int max_items,
@@ -772,6 +785,7 @@ void set_qmap(portid_t port_id, uint8_t is_rx, uint16_t queue_id, uint8_t map_va
 
 void set_xstats_hide_zero(uint8_t on_off);
 
+void set_record_core_cycles(uint8_t on_off);
 void set_verbose_level(uint16_t vb_level);
 void set_tx_pkt_segments(unsigned *seg_lengths, unsigned nb_segs);
 void show_tx_pkt_segments(void);
diff --git a/app/test-pmd/txonly.c b/app/test-pmd/txonly.c
index 97f4a452da12..00a3a6f90c7a 100644
--- a/app/test-pmd/txonly.c
+++ b/app/test-pmd/txonly.c
@@ -294,15 +294,9 @@ pkt_burst_transmit(struct fwd_stream *fs)
 	uint32_t retry;
 	uint64_t ol_flags = 0;
 	uint64_t tx_offloads;
-#ifdef RTE_TEST_PMD_RECORD_CORE_CYCLES
-	uint64_t start_tsc;
-	uint64_t end_tsc;
-	uint64_t core_cycles;
-#endif
+	uint64_t start_tsc = 0;
 
-#ifdef RTE_TEST_PMD_RECORD_CORE_CYCLES
-	start_tsc = rte_rdtsc();
-#endif
+	get_start_cycles(&start_tsc);
 
 	mbp = current_fwd_lcore()->mbp;
 	txp = &ports[fs->tx_port];
@@ -391,11 +385,7 @@ pkt_burst_transmit(struct fwd_stream *fs)
 		} while (++nb_tx < nb_pkt);
 	}
 
-#ifdef RTE_TEST_PMD_RECORD_CORE_CYCLES
-	end_tsc = rte_rdtsc();
-	core_cycles = (end_tsc - start_tsc);
-	fs->core_cycles = (uint64_t) (fs->core_cycles + core_cycles);
-#endif
+	get_end_cycles(fs, start_tsc);
 }
 
 static void
-- 
2.17.1

[dpdk-dev] [PATCH 2/6] doc: add record-core-cycles to testpmd funcs doc

[Dharmik Thakkar]

Update documentation to include description of
'record-core-cycles' runtime configuration.

Signed-off-by: Dharmik Thakkar <dharmik.thakkar@arm.com>
Reviewed-by: Phil Yang <phil.yang@arm.com>
Reviewed-by: Ruifeng Wang <ruifeng.wang@arm.com>
Reviewed-by: Honnappa Nagarahalli <honnappa.nagarahalli@arm.com>
---
 doc/guides/testpmd_app_ug/run_app.rst       |  6 ++++++
 doc/guides/testpmd_app_ug/testpmd_funcs.rst | 15 +++++++++++++++
 2 files changed, 21 insertions(+)

diff --git a/doc/guides/testpmd_app_ug/run_app.rst b/doc/guides/testpmd_app_ug/run_app.rst
index d1e4ee3e7a46..2b8e0ad69157 100644
--- a/doc/guides/testpmd_app_ug/run_app.rst
+++ b/doc/guides/testpmd_app_ug/run_app.rst
@@ -489,3 +489,9 @@ The command line options are:
     The default value is 0x7::
 
        ETH_MQ_RX_RSS_FLAG | ETH_MQ_RX_DCB_FLAG | ETH_MQ_RX_VMDQ_FLAG
+
+*   ``--record-core-cycles``
+
+    Enable measurement of CPU cycles per packet.
+
+
diff --git a/doc/guides/testpmd_app_ug/testpmd_funcs.rst b/doc/guides/testpmd_app_ug/testpmd_funcs.rst
index f8cfb02e38ab..980bf3340763 100644
--- a/doc/guides/testpmd_app_ug/testpmd_funcs.rst
+++ b/doc/guides/testpmd_app_ug/testpmd_funcs.rst
@@ -710,6 +710,21 @@ Set the forwarding ports hexadecimal mask::
 
 This is equivalent to the ``--portmask`` command-line option.
 
+set record-core-cycles
+~~~~~~~~~~~~~~~~~~~~~~
+
+Set the recording of CPU cycles::
+
+   testpmd> set record-core-cycles (on|off)
+
+Where:
+
+* ``on`` enables measurement of CPU cycles per packet.
+
+* ``off`` disables measurement of CPU cycles per packet.
+
+This is equivalent to the ``--record-core-cycles command-line`` option.
+
 set burst
 ~~~~~~~~~
 
-- 
2.17.1

[dpdk-dev] [PATCH 3/6] app/testpmd: add record-burst-stats runtime config

[Dharmik Thakkar]

Convert CONFIG_RTE_TEST_PMD_RECORD_BURST_STATS to a
runtime configuration.

Signed-off-by: Dharmik Thakkar <dharmik.thakkar@arm.com>
Tested-by: Phil Yang <phil.yang@arm.com>
Reviewed-by: Ruifeng Wang <ruifeng.wang@arm.com>
Reviewed-by: Honnappa Nagarahalli <honnappa.nagarahalli@arm.com>
---
 app/test-pmd/5tswap.c     |  8 ++-----
 app/test-pmd/cmdline.c    | 46 +++++++++++++++++++++++++++++++++++++++
 app/test-pmd/config.c     |  6 +++++
 app/test-pmd/csumonly.c   |  8 ++-----
 app/test-pmd/flowgen.c    |  4 +---
 app/test-pmd/icmpecho.c   |  8 ++-----
 app/test-pmd/iofwd.c      |  8 ++-----
 app/test-pmd/macfwd.c     |  8 ++-----
 app/test-pmd/macswap.c    |  8 ++-----
 app/test-pmd/parameters.c |  7 +++++-
 app/test-pmd/rxonly.c     |  4 +---
 app/test-pmd/testpmd.c    | 33 ++++++++++++++--------------
 app/test-pmd/testpmd.h    | 20 +++++++++++++----
 app/test-pmd/txonly.c     |  4 +---
 14 files changed, 106 insertions(+), 66 deletions(-)

diff --git a/app/test-pmd/5tswap.c b/app/test-pmd/5tswap.c
index 04f035bb450f..d9026ce86556 100644
--- a/app/test-pmd/5tswap.c
+++ b/app/test-pmd/5tswap.c
@@ -117,9 +117,7 @@ pkt_burst_5tuple_swap(struct fwd_stream *fs)
 	if (unlikely(nb_rx == 0))
 		return;
 
-#ifdef RTE_TEST_PMD_RECORD_BURST_STATS
-	fs->rx_burst_stats.pkt_burst_spread[nb_rx]++;
-#endif
+	inc_rx_burst_stats(fs, nb_rx);
 
 	fs->rx_packets += nb_rx;
 	txp = &ports[fs->tx_port];
@@ -178,9 +176,7 @@ pkt_burst_5tuple_swap(struct fwd_stream *fs)
 		}
 	}
 	fs->tx_packets += nb_tx;
-#ifdef RTE_TEST_PMD_RECORD_BURST_STATS
-	fs->tx_burst_stats.pkt_burst_spread[nb_tx]++;
-#endif
+	inc_tx_burst_stats(fs, nb_tx);
 	if (unlikely(nb_tx < nb_rx)) {
 		fs->fwd_dropped += (nb_rx - nb_tx);
 		do {
diff --git a/app/test-pmd/cmdline.c b/app/test-pmd/cmdline.c
index 1f30a9021500..6235fa0074d4 100644
--- a/app/test-pmd/cmdline.c
+++ b/app/test-pmd/cmdline.c
@@ -532,6 +532,9 @@ static void cmd_help_long_parsed(void *parsed_result,
 			"set record-core-cycles on|off\n"
 			"    Set the option to enable measurement of CPU cycles.\n"
 
+			"set record-burst-stats on|off\n"
+			"    Set the option to enable display of RX and TX bursts.\n"
+
 			"set port (port_id) vf (vf_id) rx|tx on|off\n"
 			"    Enable/Disable a VF receive/tranmit from a port\n\n"
 
@@ -8380,6 +8383,48 @@ cmdline_parse_inst_t cmd_set_record_core_cycles = {
 	},
 };
 
+/* *** SET OPTION TO ENABLE DISPLAY OF RX AND TX BURSTS *** */
+struct cmd_set_record_burst_stats_result {
+	cmdline_fixed_string_t keyword;
+	cmdline_fixed_string_t name;
+	cmdline_fixed_string_t on_off;
+};
+
+static void
+cmd_set_record_burst_stats_parsed(void *parsed_result,
+			__rte_unused struct cmdline *cl,
+			__rte_unused void *data)
+{
+	struct cmd_set_record_burst_stats_result *res;
+	uint16_t on_off = 0;
+
+	res = parsed_result;
+	on_off = !strcmp(res->on_off, "on") ? 1 : 0;
+	set_record_burst_stats(on_off);
+}
+
+cmdline_parse_token_string_t cmd_set_record_burst_stats_keyword =
+	TOKEN_STRING_INITIALIZER(struct cmd_set_record_burst_stats_result,
+				 keyword, "set");
+cmdline_parse_token_string_t cmd_set_record_burst_stats_name =
+	TOKEN_STRING_INITIALIZER(struct cmd_set_record_burst_stats_result,
+				 name, "record-burst-stats");
+cmdline_parse_token_string_t cmd_set_record_burst_stats_on_off =
+	TOKEN_STRING_INITIALIZER(struct cmd_set_record_burst_stats_result,
+				 on_off, "on#off");
+
+cmdline_parse_inst_t cmd_set_record_burst_stats = {
+	.f = cmd_set_record_burst_stats_parsed,
+	.data = NULL,
+	.help_str = "set record-burst-stats on|off",
+	.tokens = {
+		(void *)&cmd_set_record_burst_stats_keyword,
+		(void *)&cmd_set_record_burst_stats_name,
+		(void *)&cmd_set_record_burst_stats_on_off,
+		NULL,
+	},
+};
+
 /* *** CONFIGURE UNICAST HASH TABLE *** */
 struct cmd_set_uc_hash_table {
 	cmdline_fixed_string_t set;
@@ -19533,6 +19578,7 @@ cmdline_parse_ctx_t main_ctx[] = {
 	(cmdline_parse_inst_t *)&cmd_set_qmap,
 	(cmdline_parse_inst_t *)&cmd_set_xstats_hide_zero,
 	(cmdline_parse_inst_t *)&cmd_set_record_core_cycles,
+	(cmdline_parse_inst_t *)&cmd_set_record_burst_stats,
 	(cmdline_parse_inst_t *)&cmd_operate_port,
 	(cmdline_parse_inst_t *)&cmd_operate_specific_port,
 	(cmdline_parse_inst_t *)&cmd_operate_attach_port,
diff --git a/app/test-pmd/config.c b/app/test-pmd/config.c
index d93699b34e3a..873a29a123a5 100644
--- a/app/test-pmd/config.c
+++ b/app/test-pmd/config.c
@@ -3685,6 +3685,12 @@ set_record_core_cycles(uint8_t on_off)
 	record_core_cycles = on_off;
 }
 
+void
+set_record_burst_stats(uint8_t on_off)
+{
+	record_burst_stats = on_off;
+}
+
 static inline void
 print_fdir_mask(struct rte_eth_fdir_masks *mask)
 {
diff --git a/app/test-pmd/csumonly.c b/app/test-pmd/csumonly.c
index 836bafa0f6df..7ece398bd462 100644
--- a/app/test-pmd/csumonly.c
+++ b/app/test-pmd/csumonly.c
@@ -796,9 +796,7 @@ pkt_burst_checksum_forward(struct fwd_stream *fs)
 	/* receive a burst of packet */
 	nb_rx = rte_eth_rx_burst(fs->rx_port, fs->rx_queue, pkts_burst,
 				 nb_pkt_per_burst);
-#ifdef RTE_TEST_PMD_RECORD_BURST_STATS
-	fs->rx_burst_stats.pkt_burst_spread[nb_rx]++;
-#endif
+	inc_rx_burst_stats(fs, nb_rx);
 	if (unlikely(nb_rx == 0))
 		return;
 
@@ -1082,9 +1080,7 @@ pkt_burst_checksum_forward(struct fwd_stream *fs)
 	fs->rx_bad_l4_csum += rx_bad_l4_csum;
 	fs->rx_bad_outer_l4_csum += rx_bad_outer_l4_csum;
 
-#ifdef RTE_TEST_PMD_RECORD_BURST_STATS
-	fs->tx_burst_stats.pkt_burst_spread[nb_tx]++;
-#endif
+	inc_tx_burst_stats(fs, nb_tx);
 	if (unlikely(nb_tx < nb_rx)) {
 		fs->fwd_dropped += (nb_rx - nb_tx);
 		do {
diff --git a/app/test-pmd/flowgen.c b/app/test-pmd/flowgen.c
index e92a0eeca254..acf3e24605eb 100644
--- a/app/test-pmd/flowgen.c
+++ b/app/test-pmd/flowgen.c
@@ -188,9 +188,7 @@ pkt_burst_flow_gen(struct fwd_stream *fs)
 	}
 	fs->tx_packets += nb_tx;
 
-#ifdef RTE_TEST_PMD_RECORD_BURST_STATS
-	fs->tx_burst_stats.pkt_burst_spread[nb_tx]++;
-#endif
+	inc_tx_burst_stats(fs, nb_tx);
 	if (unlikely(nb_tx < nb_pkt)) {
 		/* Back out the flow counter. */
 		next_flow -= (nb_pkt - nb_tx);
diff --git a/app/test-pmd/icmpecho.c b/app/test-pmd/icmpecho.c
index 3f27e8c29d33..af6f7e7902e4 100644
--- a/app/test-pmd/icmpecho.c
+++ b/app/test-pmd/icmpecho.c
@@ -302,9 +302,7 @@ reply_to_icmp_echo_rqsts(struct fwd_stream *fs)
 	 */
 	nb_rx = rte_eth_rx_burst(fs->rx_port, fs->rx_queue, pkts_burst,
 				 nb_pkt_per_burst);
-#ifdef RTE_TEST_PMD_RECORD_BURST_STATS
-	fs->rx_burst_stats.pkt_burst_spread[nb_rx]++;
-#endif
+	inc_rx_burst_stats(fs, nb_rx);
 	if (unlikely(nb_rx == 0))
 		return;
 
@@ -503,9 +501,7 @@ reply_to_icmp_echo_rqsts(struct fwd_stream *fs)
 			}
 		}
 		fs->tx_packets += nb_tx;
-#ifdef RTE_TEST_PMD_RECORD_BURST_STATS
-		fs->tx_burst_stats.pkt_burst_spread[nb_tx]++;
-#endif
+		inc_tx_burst_stats(fs, nb_tx);
 		if (unlikely(nb_tx < nb_replies)) {
 			fs->fwd_dropped += (nb_replies - nb_tx);
 			do {
diff --git a/app/test-pmd/iofwd.c b/app/test-pmd/iofwd.c
index 456f7b5925ed..83d098adcbf5 100644
--- a/app/test-pmd/iofwd.c
+++ b/app/test-pmd/iofwd.c
@@ -59,9 +59,7 @@ pkt_burst_io_forward(struct fwd_stream *fs)
 	 */
 	nb_rx = rte_eth_rx_burst(fs->rx_port, fs->rx_queue,
 			pkts_burst, nb_pkt_per_burst);
-#ifdef RTE_TEST_PMD_RECORD_BURST_STATS
-	fs->rx_burst_stats.pkt_burst_spread[nb_rx]++;
-#endif
+	inc_rx_burst_stats(fs, nb_rx);
 	if (unlikely(nb_rx == 0))
 		return;
 	fs->rx_packets += nb_rx;
@@ -80,9 +78,7 @@ pkt_burst_io_forward(struct fwd_stream *fs)
 		}
 	}
 	fs->tx_packets += nb_tx;
-#ifdef RTE_TEST_PMD_RECORD_BURST_STATS
-	fs->tx_burst_stats.pkt_burst_spread[nb_tx]++;
-#endif
+	inc_tx_burst_stats(fs, nb_tx);
 	if (unlikely(nb_tx < nb_rx)) {
 		fs->fwd_dropped += (nb_rx - nb_tx);
 		do {
diff --git a/app/test-pmd/macfwd.c b/app/test-pmd/macfwd.c
index 6a77568c7e60..0568ea794d48 100644
--- a/app/test-pmd/macfwd.c
+++ b/app/test-pmd/macfwd.c
@@ -65,9 +65,7 @@ pkt_burst_mac_forward(struct fwd_stream *fs)
 	 */
 	nb_rx = rte_eth_rx_burst(fs->rx_port, fs->rx_queue, pkts_burst,
 				 nb_pkt_per_burst);
-#ifdef RTE_TEST_PMD_RECORD_BURST_STATS
-	fs->rx_burst_stats.pkt_burst_spread[nb_rx]++;
-#endif
+	inc_rx_burst_stats(fs, nb_rx);
 	if (unlikely(nb_rx == 0))
 		return;
 
@@ -111,9 +109,7 @@ pkt_burst_mac_forward(struct fwd_stream *fs)
 	}
 
 	fs->tx_packets += nb_tx;
-#ifdef RTE_TEST_PMD_RECORD_BURST_STATS
-	fs->tx_burst_stats.pkt_burst_spread[nb_tx]++;
-#endif
+	inc_tx_burst_stats(fs, nb_tx);
 	if (unlikely(nb_tx < nb_rx)) {
 		fs->fwd_dropped += (nb_rx - nb_tx);
 		do {
diff --git a/app/test-pmd/macswap.c b/app/test-pmd/macswap.c
index aaef5b0047b1..74e2dd838dd5 100644
--- a/app/test-pmd/macswap.c
+++ b/app/test-pmd/macswap.c
@@ -66,9 +66,7 @@ pkt_burst_mac_swap(struct fwd_stream *fs)
 	 */
 	nb_rx = rte_eth_rx_burst(fs->rx_port, fs->rx_queue, pkts_burst,
 				 nb_pkt_per_burst);
-#ifdef RTE_TEST_PMD_RECORD_BURST_STATS
-	fs->rx_burst_stats.pkt_burst_spread[nb_rx]++;
-#endif
+	inc_rx_burst_stats(fs, nb_rx);
 	if (unlikely(nb_rx == 0))
 		return;
 
@@ -90,9 +88,7 @@ pkt_burst_mac_swap(struct fwd_stream *fs)
 		}
 	}
 	fs->tx_packets += nb_tx;
-#ifdef RTE_TEST_PMD_RECORD_BURST_STATS
-	fs->tx_burst_stats.pkt_burst_spread[nb_tx]++;
-#endif
+	inc_tx_burst_stats(fs, nb_tx);
 	if (unlikely(nb_tx < nb_rx)) {
 		fs->fwd_dropped += (nb_rx - nb_tx);
 		do {
diff --git a/app/test-pmd/parameters.c b/app/test-pmd/parameters.c
index 76072ae81b93..784515314ebd 100644
--- a/app/test-pmd/parameters.c
+++ b/app/test-pmd/parameters.c
@@ -70,7 +70,8 @@ usage(char* progname)
 	       "--rxpt= | --rxht= | --rxwt= | --rxfreet= | "
 	       "--txpt= | --txht= | --txwt= | --txfreet= | "
 	       "--txrst= | --tx-offloads= | | --rx-offloads= | "
-	       "--vxlan-gpe-port= | --record-core-cycles]\n",
+	       "--vxlan-gpe-port= | --record-core-cycles | "
+	       "--record-burst-stats]\n",
 	       progname);
 #ifdef RTE_LIBRTE_CMDLINE
 	printf("  --interactive: run in interactive mode.\n");
@@ -217,6 +218,7 @@ usage(char* progname)
 	printf("  --rx-mq-mode=0xX: hexadecimal bitmask of RX mq mode can be "
 	       "enabled\n");
 	printf("  --record-core-cycles: enable measurement of CPU cycles.\n");
+	printf("  --record-burst-stats: enable display of RX and TX bursts.\n");
 }
 
 #ifdef RTE_LIBRTE_CMDLINE
@@ -679,6 +681,7 @@ launch_args_parse(int argc, char** argv)
 		{ "no-iova-contig",             0, 0, 0 },
 		{ "rx-mq-mode",                 1, 0, 0 },
 		{ "record-core-cycles",         0, 0, 0 },
+		{ "record-burst-stats",         0, 0, 0 },
 		{ 0, 0, 0, 0 },
 	};
 
@@ -1385,6 +1388,8 @@ launch_args_parse(int argc, char** argv)
 			}
 			if (!strcmp(lgopts[opt_idx].name, "record-core-cycles"))
 				record_core_cycles = 1;
+			if (!strcmp(lgopts[opt_idx].name, "record-burst-stats"))
+				record_burst_stats = 1;
 			break;
 		case 'h':
 			usage(argv[0]);
diff --git a/app/test-pmd/rxonly.c b/app/test-pmd/rxonly.c
index b6c8b8c7d5db..c78fc4609ae5 100644
--- a/app/test-pmd/rxonly.c
+++ b/app/test-pmd/rxonly.c
@@ -58,9 +58,7 @@ pkt_burst_receive(struct fwd_stream *fs)
 	 */
 	nb_rx = rte_eth_rx_burst(fs->rx_port, fs->rx_queue, pkts_burst,
 				 nb_pkt_per_burst);
-#ifdef RTE_TEST_PMD_RECORD_BURST_STATS
-	fs->rx_burst_stats.pkt_burst_spread[nb_rx]++;
-#endif
+	inc_rx_burst_stats(fs, nb_rx);
 	if (unlikely(nb_rx == 0))
 		return;
 
diff --git a/app/test-pmd/testpmd.c b/app/test-pmd/testpmd.c
index f327b16604d2..fb286b86e51b 100644
--- a/app/test-pmd/testpmd.c
+++ b/app/test-pmd/testpmd.c
@@ -480,6 +480,11 @@ uint8_t xstats_hide_zero;
  */
 uint8_t record_core_cycles;
 
+/*
+ * Display of RX and TX bursts disabled by default
+ */
+uint8_t record_burst_stats;
+
 unsigned int num_sockets = 0;
 unsigned int socket_ids[RTE_MAX_NUMA_NODES];
 
@@ -1684,7 +1689,6 @@ init_fwd_streams(void)
 	return 0;
 }
 
-#ifdef RTE_TEST_PMD_RECORD_BURST_STATS
 static void
 pkt_burst_stats_display(const char *rx_tx, struct pkt_burst_stats *pbs)
 {
@@ -1751,7 +1755,6 @@ pkt_burst_stats_display(const char *rx_tx, struct pkt_burst_stats *pbs)
 		sburstp += burst_percent[i];
 	}
 }
-#endif /* RTE_TEST_PMD_RECORD_BURST_STATS */
 
 static void
 fwd_stream_stats_display(streamid_t stream_id)
@@ -1782,10 +1785,10 @@ fwd_stream_stats_display(streamid_t stream_id)
 		printf("\n");
 	}
 
-#ifdef RTE_TEST_PMD_RECORD_BURST_STATS
-	pkt_burst_stats_display("RX", &fs->rx_burst_stats);
-	pkt_burst_stats_display("TX", &fs->tx_burst_stats);
-#endif
+	if (record_burst_stats) {
+		pkt_burst_stats_display("RX", &fs->rx_burst_stats);
+		pkt_burst_stats_display("TX", &fs->tx_burst_stats);
+	}
 }
 
 void
@@ -1915,14 +1918,14 @@ fwd_stats_display(void)
 			       stats.opackets + ports_stats[pt_id].tx_dropped);
 		}
 
-#ifdef RTE_TEST_PMD_RECORD_BURST_STATS
-		if (ports_stats[pt_id].rx_stream)
-			pkt_burst_stats_display("RX",
-				&ports_stats[pt_id].rx_stream->rx_burst_stats);
-		if (ports_stats[pt_id].tx_stream)
-			pkt_burst_stats_display("TX",
-				&ports_stats[pt_id].tx_stream->tx_burst_stats);
-#endif
+		if (record_burst_stats) {
+			if (ports_stats[pt_id].rx_stream)
+				pkt_burst_stats_display("RX",
+					&ports_stats[pt_id].rx_stream->rx_burst_stats);
+			if (ports_stats[pt_id].tx_stream)
+				pkt_burst_stats_display("TX",
+					&ports_stats[pt_id].tx_stream->tx_burst_stats);
+		}
 
 		if (port->rx_queue_stats_mapping_enabled) {
 			printf("\n");
@@ -2004,10 +2007,8 @@ fwd_stats_reset(void)
 		fs->rx_bad_l4_csum = 0;
 		fs->rx_bad_outer_l4_csum = 0;
 
-#ifdef RTE_TEST_PMD_RECORD_BURST_STATS
 		memset(&fs->rx_burst_stats, 0, sizeof(fs->rx_burst_stats));
 		memset(&fs->tx_burst_stats, 0, sizeof(fs->tx_burst_stats));
-#endif
 		fs->core_cycles = 0;
 	}
 }
diff --git a/app/test-pmd/testpmd.h b/app/test-pmd/testpmd.h
index dcc37be02d59..a8ae5cceb198 100644
--- a/app/test-pmd/testpmd.h
+++ b/app/test-pmd/testpmd.h
@@ -76,7 +76,6 @@ enum {
 	/**< allocate mempool natively, use rte_pktmbuf_pool_create_extbuf */
 };
 
-#ifdef RTE_TEST_PMD_RECORD_BURST_STATS
 /**
  * The data structure associated with RX and TX packet burst statistics
  * that are recorded for each forwarding stream.
@@ -84,7 +83,6 @@ enum {
 struct pkt_burst_stats {
 	unsigned int pkt_burst_spread[MAX_PKT_BURST];
 };
-#endif
 
 /** Information for a given RSS type. */
 struct rss_type_info {
@@ -130,10 +128,8 @@ struct fwd_stream {
 	/**< received packets has bad outer l4 checksum */
 	unsigned int gro_times;	/**< GRO operation times */
 	uint64_t     core_cycles; /**< used for RX and TX processing */
-#ifdef RTE_TEST_PMD_RECORD_BURST_STATS
 	struct pkt_burst_stats rx_burst_stats;
 	struct pkt_burst_stats tx_burst_stats;
-#endif
 };
 
 /** Descriptor for a single flow. */
@@ -300,6 +296,7 @@ extern uint8_t xstats_hide_zero; /**< Hide zero values for xstats display */
 
 /* globals used for configuration */
 extern uint8_t record_core_cycles; /**< Enables measurement of CPU cycles */
+extern uint8_t record_burst_stats; /**< Enables display of RX and TX bursts */
 extern uint16_t verbose_level; /**< Drives messages being displayed, if any. */
 extern int testpmd_logtype; /**< Log type for testpmd logs */
 extern uint8_t  interactive;
@@ -694,6 +691,20 @@ get_end_cycles(struct fwd_stream *fs, uint64_t start_tsc)
 		fs->core_cycles += rte_rdtsc() - start_tsc;
 }
 
+static inline void
+inc_rx_burst_stats(struct fwd_stream *fs, uint16_t nb_rx)
+{
+	if (record_burst_stats)
+		fs->rx_burst_stats.pkt_burst_spread[nb_rx]++;
+}
+
+static inline void
+inc_tx_burst_stats(struct fwd_stream *fs, uint16_t nb_tx)
+{
+	if (record_burst_stats)
+		fs->tx_burst_stats.pkt_burst_spread[nb_tx]++;
+}
+
 /* Prototypes */
 unsigned int parse_item_list(char* str, const char* item_name,
 			unsigned int max_items,
@@ -786,6 +797,7 @@ void set_qmap(portid_t port_id, uint8_t is_rx, uint16_t queue_id, uint8_t map_va
 void set_xstats_hide_zero(uint8_t on_off);
 
 void set_record_core_cycles(uint8_t on_off);
+void set_record_burst_stats(uint8_t on_off);
 void set_verbose_level(uint16_t vb_level);
 void set_tx_pkt_segments(unsigned *seg_lengths, unsigned nb_segs);
 void show_tx_pkt_segments(void);
diff --git a/app/test-pmd/txonly.c b/app/test-pmd/txonly.c
index 00a3a6f90c7a..7d4f105ccd68 100644
--- a/app/test-pmd/txonly.c
+++ b/app/test-pmd/txonly.c
@@ -369,9 +369,7 @@ pkt_burst_transmit(struct fwd_stream *fs)
 	if (txonly_multi_flow)
 		RTE_PER_LCORE(_ip_var) -= nb_pkt - nb_tx;
 
-#ifdef RTE_TEST_PMD_RECORD_BURST_STATS
-	fs->tx_burst_stats.pkt_burst_spread[nb_tx]++;
-#endif
+	inc_tx_burst_stats(fs, nb_tx);
 	if (unlikely(nb_tx < nb_pkt)) {
 		if (verbose_level > 0 && fs->fwd_dropped == 0)
 			printf("port %d tx_queue %d - drop "
-- 
2.17.1

[dpdk-dev] [PATCH 4/6] doc: add record-burst-stats to testpmd funcs doc

[Dharmik Thakkar]

Update documentation to include description of
'record-burst-stats' runtime configuration.

Signed-off-by: Dharmik Thakkar <dharmik.thakkar@arm.com>
Reviewed-by: Phil Yang <phil.yang@arm.com>
Reviewed-by: Ruifeng Wang <ruifeng.wang@arm.com>
Reviewed-by: Honnappa Nagarahalli <honnappa.nagarahalli@arm.com>
---
 doc/guides/testpmd_app_ug/run_app.rst       |  2 ++
 doc/guides/testpmd_app_ug/testpmd_funcs.rst | 15 +++++++++++++++
 2 files changed, 17 insertions(+)

diff --git a/doc/guides/testpmd_app_ug/run_app.rst b/doc/guides/testpmd_app_ug/run_app.rst
index 2b8e0ad69157..e2539f693f66 100644
--- a/doc/guides/testpmd_app_ug/run_app.rst
+++ b/doc/guides/testpmd_app_ug/run_app.rst
@@ -494,4 +494,6 @@ The command line options are:
 
     Enable measurement of CPU cycles per packet.
 
+*   ``--record-burst-stats``
 
+    Enable display of RX and TX burst stats.
diff --git a/doc/guides/testpmd_app_ug/testpmd_funcs.rst b/doc/guides/testpmd_app_ug/testpmd_funcs.rst
index 980bf3340763..2192a7e4720d 100644
--- a/doc/guides/testpmd_app_ug/testpmd_funcs.rst
+++ b/doc/guides/testpmd_app_ug/testpmd_funcs.rst
@@ -725,6 +725,21 @@ Where:
 
 This is equivalent to the ``--record-core-cycles command-line`` option.
 
+set record-burst-stats
+~~~~~~~~~~~~~~~~~~~~~~
+
+Set the displaying of RX and TX bursts::
+
+   testpmd> set record-burst-stats (on|off)
+
+Where:
+
+* ``on`` enables display of RX and TX bursts.
+
+* ``off`` disables display of RX and TX bursts.
+
+This is equivalent to the ``--record-burst-stats command-line`` option.
+
 set burst
 ~~~~~~~~~
 
-- 
2.17.1

[dpdk-dev] [PATCH 5/6] app/testpmd: enable burst stats for noisy vnf mode

[Dharmik Thakkar]

From: Phil Yang <phil.yang@arm.com>

Add burst stats for noisy vnf mode.

Fixes: 3c156061b938 ("app/testpmd: add noisy neighbour forwarding mode")
Cc: stable@dpdk.org
Cc: jfreimann@redhat.com

Signed-off-by: Dharmik Thakkar <dharmik.thakkar@arm.com>
Signed-off-by: Phil Yang <phil.yang@arm.com>
Reviewed-by: Honnappa Nagarahalli <honnappa.nagarahalli@arm.com>
Reviewed-by: Ruifeng Wang <ruifeng.wang@arm.com>
---
 app/test-pmd/noisy_vnf.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/app/test-pmd/noisy_vnf.c b/app/test-pmd/noisy_vnf.c
index 58c4ee925566..382a4c2aaef1 100644
--- a/app/test-pmd/noisy_vnf.c
+++ b/app/test-pmd/noisy_vnf.c
@@ -154,6 +154,7 @@ pkt_burst_noisy_vnf(struct fwd_stream *fs)
 
 	nb_rx = rte_eth_rx_burst(fs->rx_port, fs->rx_queue,
 			pkts_burst, nb_pkt_per_burst);
+	inc_rx_burst_stats(fs, nb_rx);
 	if (unlikely(nb_rx == 0))
 		goto flush;
 	fs->rx_packets += nb_rx;
@@ -164,6 +165,7 @@ pkt_burst_noisy_vnf(struct fwd_stream *fs)
 				pkts_burst, nb_rx);
 		if (unlikely(nb_tx < nb_rx) && fs->retry_enabled)
 			nb_tx += do_retry(nb_rx, nb_tx, pkts_burst, fs);
+		inc_tx_burst_stats(fs, nb_tx);
 		fs->tx_packets += nb_tx;
 		fs->fwd_dropped += drop_pkts(pkts_burst, nb_rx, nb_tx);
 		return;
@@ -187,6 +189,7 @@ pkt_burst_noisy_vnf(struct fwd_stream *fs)
 					nb_deqd);
 			if (unlikely(nb_tx < nb_rx) && fs->retry_enabled)
 				nb_tx += do_retry(nb_rx, nb_tx, tmp_pkts, fs);
+			inc_tx_burst_stats(fs, nb_tx);
 			fs->fwd_dropped += drop_pkts(tmp_pkts, nb_deqd, nb_tx);
 		}
 	}
@@ -211,6 +214,7 @@ pkt_burst_noisy_vnf(struct fwd_stream *fs)
 					 tmp_pkts, nb_deqd);
 		if (unlikely(sent < nb_deqd) && fs->retry_enabled)
 			nb_tx += do_retry(nb_rx, nb_tx, tmp_pkts, fs);
+		inc_tx_burst_stats(fs, nb_tx);
 		fs->fwd_dropped += drop_pkts(tmp_pkts, nb_deqd, sent);
 		ncf->prev_time = rte_get_timer_cycles();
 	}
-- 
2.17.1

[dpdk-dev] [PATCH 6/6] app/testpmd: enable empty polls in 5tswap

[Dharmik Thakkar]

Enable empty polls in burst stats within 5tswap.c

Signed-off-by: Dharmik Thakkar <dharmik.thakkar@arm.com>
Reviewed-by: Honnappa Nagarahalli <honnappa.nagarahalli@arm.com>
---
 app/test-pmd/5tswap.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/app/test-pmd/5tswap.c b/app/test-pmd/5tswap.c
index d9026ce86556..3cf1692eaba8 100644
--- a/app/test-pmd/5tswap.c
+++ b/app/test-pmd/5tswap.c
@@ -114,11 +114,10 @@ pkt_burst_5tuple_swap(struct fwd_stream *fs)
 	 */
 	nb_rx = rte_eth_rx_burst(fs->rx_port, fs->rx_queue, pkts_burst,
 				 nb_pkt_per_burst);
+	inc_rx_burst_stats(fs, nb_rx);
 	if (unlikely(nb_rx == 0))
 		return;
 
-	inc_rx_burst_stats(fs, nb_rx);
-
 	fs->rx_packets += nb_rx;
 	txp = &ports[fs->tx_port];
 	ol_flags = ol_flags_init(txp->dev_conf.txmode.offloads);
-- 
2.17.1

Re: [dpdk-dev] [PATCH 6/6] app/testpmd: enable empty polls in 5tswap

[Phil Yang]

> -----Original Message-----
> From: dev <dev-bounces@dpdk.org> On Behalf Of Dharmik Thakkar
> Sent: Wednesday, July 15, 2020 5:51 AM
> To: Wenzhuo Lu <wenzhuo.lu@intel.com>; Beilei Xing
> <beilei.xing@intel.com>; Bernard Iremonger <bernard.iremonger@intel.com>
> Cc: dev@dpdk.org; nd <nd@arm.com>; Dharmik Thakkar
> <Dharmik.Thakkar@arm.com>
> Subject: [dpdk-dev] [PATCH 6/6] app/testpmd: enable empty polls in 5tswap
> 
> Enable empty polls in burst stats within 5tswap.c
> 
> Signed-off-by: Dharmik Thakkar <dharmik.thakkar@arm.com>
> Reviewed-by: Honnappa Nagarahalli <honnappa.nagarahalli@arm.com>

Reviewed-by: Phil Yang <phil.yang@arm.com>

Re: [dpdk-dev] [PATCH 0/6] app/testpmd: add runtime config

[Ferruh Yigit]

On 7/14/2020 10:51 PM, Dharmik Thakkar wrote:
> Meson build system lacks support for
> CONFIG_RTE_TEST_PMD_RECORD_CORE_CYCLES and
> CONFIG_RTE_TEST_PMD_RECORD_BURST_STATS configuration options.
> 
> One solution is to add these options within meson_options.txt
> 
> Since adding these as runtime config causes no performance impact,

Hi Dharmik,

These are on the datapath, and even disable there will be additional checks,
isn't it expected to have some impact?
Did you do any measurements for it?

> it makes sense to enable these as runtime config alongside other
> available runtime options within testpmd.

If there is performance impact,
another option can be still using the compile time config with meson.
As Jerin suggested previously, or Ed is doing in his patch right now, something
like:


 #ifdef RTE_TEST_PMD_RECORD_CORE_CYCLES
 const int record_core_cycles = 1;
 #else
 const int record_core_cycles = 0;
 #endif

 static inline void
 get_start_cycles(uint64_t *start_tsc)
 {
 	if (record_core_cycles)
 		*start_tsc = rte_rdtsc();
 }


When meson compiled by default, compiler will optimize out the code during
compile time.

To enable it, can provide the config option with CFLAGS,
CFLAGS="-DRTE_TEST_PMD_RECORD_CORE_CYCLES" meson build

> 
> Dharmik Thakkar (5):
>   app/testpmd: add record-core-cycles runtime config
>   doc: add record-core-cycles to testpmd funcs doc
>   app/testpmd: add record-burst-stats runtime config
>   doc: add record-burst-stats to testpmd funcs doc
>   app/testpmd: enable empty polls in 5tswap
> 
> Phil Yang (1):
>   app/testpmd: enable burst stats for noisy vnf mode
> 
>  app/test-pmd/5tswap.c                       | 25 ++----
>  app/test-pmd/cmdline.c                      | 92 +++++++++++++++++++++
>  app/test-pmd/config.c                       | 12 +++
>  app/test-pmd/csumonly.c                     | 24 ++----
>  app/test-pmd/flowgen.c                      | 21 ++---
>  app/test-pmd/icmpecho.c                     | 24 ++----
>  app/test-pmd/iofwd.c                        | 26 ++----
>  app/test-pmd/macfwd.c                       | 25 ++----
>  app/test-pmd/macswap.c                      | 24 ++----
>  app/test-pmd/noisy_vnf.c                    |  4 +
>  app/test-pmd/parameters.c                   | 11 ++-
>  app/test-pmd/rxonly.c                       | 19 +----
>  app/test-pmd/testpmd.c                      | 77 ++++++++---------
>  app/test-pmd/testpmd.h                      | 38 +++++++--
>  app/test-pmd/txonly.c                       | 20 +----
>  doc/guides/testpmd_app_ug/run_app.rst       |  8 ++
>  doc/guides/testpmd_app_ug/testpmd_funcs.rst | 30 +++++++
>  17 files changed, 272 insertions(+), 208 deletions(-)
> 

Re: [dpdk-dev] [PATCH 0/6] app/testpmd: add runtime config

[Bruce Richardson]

On Wed, Aug 26, 2020 at 05:33:20PM +0100, Ferruh Yigit wrote:
> On 7/14/2020 10:51 PM, Dharmik Thakkar wrote:
> > Meson build system lacks support for
> > CONFIG_RTE_TEST_PMD_RECORD_CORE_CYCLES and
> > CONFIG_RTE_TEST_PMD_RECORD_BURST_STATS configuration options.
> > 
> > One solution is to add these options within meson_options.txt
> > 
> > Since adding these as runtime config causes no performance impact,
> 
> Hi Dharmik,
> 
> These are on the datapath, and even disable there will be additional
> checks, isn't it expected to have some impact?  Did you do any
> measurements for it?
> 
Branches that always predict the same way can be very cheap, and unless
proven to be a problem, I'd see no issue with having a few on the datapath
- especially if it just one or two per burst. If we start seeing a
significant number, or ones that occur for every packet, then we perhaps
need to be more cautious.

I also think that using lots of different CFLAGS for turning things on and
off is as bad - or even worse - than using the old build-time config
options, just this time we don't have a single list of them somewhere!
Therefore, I think we really need to start converting these to runtime
options where we can.

Regards,
/Bruce

Re: [dpdk-dev] [PATCH 0/6] app/testpmd: add runtime config

[Dharmik Thakkar]

> On Aug 26, 2020, at 11:41 AM, Bruce Richardson <bruce.richardson@intel.com> wrote:
> 
> On Wed, Aug 26, 2020 at 05:33:20PM +0100, Ferruh Yigit wrote:
>> On 7/14/2020 10:51 PM, Dharmik Thakkar wrote:
>>> Meson build system lacks support for
>>> CONFIG_RTE_TEST_PMD_RECORD_CORE_CYCLES and
>>> CONFIG_RTE_TEST_PMD_RECORD_BURST_STATS configuration options.
>>> 
>>> One solution is to add these options within meson_options.txt
>>> 
>>> Since adding these as runtime config causes no performance impact,
>> 
>> Hi Dharmik,
>> 
>> These are on the datapath, and even disable there will be additional
>> checks, isn't it expected to have some impact?  Did you do any
>> measurements for it?

Hi Ferruh,

In my measurements, I saw a maximum performance degradation of 0.3% for rx and tx throughput (pps)
with runtime option (both disabled and enabled cases) when compared to compile time option..
There was no difference in cycles per packet measurement.
I did these measurements with Mellanox-ConnectX-5 card on N1SDP server with ’set fwd mac retry’ option.

>> 
> Branches that always predict the same way can be very cheap, and unless
> proven to be a problem, I'd see no issue with having a few on the datapath
> - especially if it just one or two per burst. If we start seeing a
> significant number, or ones that occur for every packet, then we perhaps
> need to be more cautious.
> 
> I also think that using lots of different CFLAGS for turning things on and
> off is as bad - or even worse - than using the old build-time config
> options, just this time we don't have a single list of them somewhere!
> Therefore, I think we really need to start converting these to runtime
> options where we can.
> 
> Regards,
> /Bruce

Re: [dpdk-dev] [PATCH 0/6] app/testpmd: add runtime config

[Ferruh Yigit]

On 8/26/2020 5:41 PM, Bruce Richardson wrote:
> On Wed, Aug 26, 2020 at 05:33:20PM +0100, Ferruh Yigit wrote:
>> On 7/14/2020 10:51 PM, Dharmik Thakkar wrote:
>>> Meson build system lacks support for
>>> CONFIG_RTE_TEST_PMD_RECORD_CORE_CYCLES and
>>> CONFIG_RTE_TEST_PMD_RECORD_BURST_STATS configuration options.
>>>
>>> One solution is to add these options within meson_options.txt
>>>
>>> Since adding these as runtime config causes no performance impact,
>>
>> Hi Dharmik,
>>
>> These are on the datapath, and even disable there will be additional
>> checks, isn't it expected to have some impact?  Did you do any
>> measurements for it?
>>
> Branches that always predict the same way can be very cheap, and unless
> proven to be a problem, I'd see no issue with having a few on the datapath
> - especially if it just one or two per burst. If we start seeing a
> significant number, or ones that occur for every packet, then we perhaps
> need to be more cautious.
> 
> I also think that using lots of different CFLAGS for turning things on and
> off is as bad - or even worse - than using the old build-time config
> options, just this time we don't have a single list of them somewhere!
> Therefore, I think we really need to start converting these to runtime
> options where we can.

+1 using config options via 'CFLAGS' is worse. But if we want it to be compile
time option because of performance, this is an option.

Re: [dpdk-dev] [PATCH 0/6] app/testpmd: add runtime config

[Ferruh Yigit]

On 8/26/2020 6:07 PM, Dharmik Thakkar wrote:
> 
> 
>> On Aug 26, 2020, at 11:41 AM, Bruce Richardson <bruce.richardson@intel.com> wrote:
>>
>> On Wed, Aug 26, 2020 at 05:33:20PM +0100, Ferruh Yigit wrote:
>>> On 7/14/2020 10:51 PM, Dharmik Thakkar wrote:
>>>> Meson build system lacks support for
>>>> CONFIG_RTE_TEST_PMD_RECORD_CORE_CYCLES and
>>>> CONFIG_RTE_TEST_PMD_RECORD_BURST_STATS configuration options.
>>>>
>>>> One solution is to add these options within meson_options.txt
>>>>
>>>> Since adding these as runtime config causes no performance impact,
>>>
>>> Hi Dharmik,
>>>
>>> These are on the datapath, and even disable there will be additional
>>> checks, isn't it expected to have some impact?  Did you do any
>>> measurements for it?
> 
> Hi Ferruh,
> 
> In my measurements, I saw a maximum performance degradation of 0.3% for rx and tx throughput (pps)
> with runtime option (both disabled and enabled cases) when compared to compile time option..
> There was no difference in cycles per packet measurement.
> I did these measurements with Mellanox-ConnectX-5 card on N1SDP server with ’set fwd mac retry’ option.
>

Thanks Dharmik,

0.3% does not look a lot, I also don't get any recognizable drop when the stats
are disabled (there is a drop (up to %15) when stats are enabled but that is OK).
And there is a benefit to have the stats runtime configurable.

This minor drop for runtime configuration looks OK to me, but lets wait a little
more if there is any strong objection to it, we can proceed afterwards.

> 
>>>
>> Branches that always predict the same way can be very cheap, and unless
>> proven to be a problem, I'd see no issue with having a few on the datapath
>> - especially if it just one or two per burst. If we start seeing a
>> significant number, or ones that occur for every packet, then we perhaps
>> need to be more cautious.
>>
>> I also think that using lots of different CFLAGS for turning things on and
>> off is as bad - or even worse - than using the old build-time config
>> options, just this time we don't have a single list of them somewhere!
>> Therefore, I think we really need to start converting these to runtime
>> options where we can.
>>
>> Regards,
>> /Bruce
> 
> 
> 

Re: [dpdk-dev] [PATCH 0/6] app/testpmd: add runtime config

[Ferruh Yigit]

On 8/26/2020 11:06 PM, Ferruh Yigit wrote:
> On 8/26/2020 6:07 PM, Dharmik Thakkar wrote:
>>
>>
>>> On Aug 26, 2020, at 11:41 AM, Bruce Richardson <bruce.richardson@intel.com> wrote:
>>>
>>> On Wed, Aug 26, 2020 at 05:33:20PM +0100, Ferruh Yigit wrote:
>>>> On 7/14/2020 10:51 PM, Dharmik Thakkar wrote:
>>>>> Meson build system lacks support for
>>>>> CONFIG_RTE_TEST_PMD_RECORD_CORE_CYCLES and
>>>>> CONFIG_RTE_TEST_PMD_RECORD_BURST_STATS configuration options.
>>>>>
>>>>> One solution is to add these options within meson_options.txt
>>>>>
>>>>> Since adding these as runtime config causes no performance impact,
>>>>
>>>> Hi Dharmik,
>>>>
>>>> These are on the datapath, and even disable there will be additional
>>>> checks, isn't it expected to have some impact?  Did you do any
>>>> measurements for it?
>>
>> Hi Ferruh,
>>
>> In my measurements, I saw a maximum performance degradation of 0.3% for rx and tx throughput (pps)
>> with runtime option (both disabled and enabled cases) when compared to compile time option..
>> There was no difference in cycles per packet measurement.
>> I did these measurements with Mellanox-ConnectX-5 card on N1SDP server with ’set fwd mac retry’ option.
>>
> 
> Thanks Dharmik,
> 
> 0.3% does not look a lot, I also don't get any recognizable drop when the stats
> are disabled (there is a drop (up to %15) when stats are enabled but that is OK).
> And there is a benefit to have the stats runtime configurable.
> 
> This minor drop for runtime configuration looks OK to me, but lets wait a little
> more if there is any strong objection to it, we can proceed afterwards.

There seems no objection to switching runtime configuration.

For series,
Reviewed-by: Ferruh Yigit <ferruh.yigit@intel.com>

Series applied to dpdk-next-net/main, thanks.
(doc patches merged to the code while merging.)