[dpdk-dev] [PATCH v1] app/testpmd: add bitrate stats option

[dpdk-dev] [PATCH v1] app/testpmd: add bitrate stats option

[Remy Horton]

Bit-rate collation should only be done by one core. This patch adds
an option to select which core performs the bit-rate calculation,
which is also disabled by default.

Fixes: 7e4441c8efb9 ("app/testpmd: add bitrate statistics calculation")

Signed-off-by: Remy Horton <remy.horton@intel.com>
---
 app/test-pmd/parameters.c | 15 +++++++++++++++
 app/test-pmd/testpmd.c    | 36 ++++++++++++++++++++++++++----------
 app/test-pmd/testpmd.h    |  5 +++++
 3 files changed, 46 insertions(+), 10 deletions(-)

diff --git a/app/test-pmd/parameters.c b/app/test-pmd/parameters.c
index 3f4d3a2..a6140b5 100644
--- a/app/test-pmd/parameters.c
+++ b/app/test-pmd/parameters.c
@@ -536,6 +536,9 @@ launch_args_parse(int argc, char** argv)
 #ifdef RTE_LIBRTE_LATENCY_STATS
 		{ "latencystats",               1, 0, 0 },
 #endif
+#ifdef RTE_LIBRTE_BITRATE
+		{ "bitratestats",               1, 0, 0 },
+#endif
 		{ "disable-crc-strip",          0, 0, 0 },
 		{ "enable-lro",                 0, 0, 0 },
 		{ "enable-rx-cksum",            0, 0, 0 },
@@ -793,6 +796,18 @@ launch_args_parse(int argc, char** argv)
 						 " must be >= 0\n", n);
 			}
 #endif
+#ifdef RTE_LIBRTE_BITRATE
+			if (!strcmp(lgopts[opt_idx].name, "bitratestats")) {
+				n = atoi(optarg);
+				if (n >= 0) {
+					bitrate_lcore_id = (lcoreid_t) n;
+					bitrate_enabled = 1;
+				} else
+					rte_exit(EXIT_FAILURE,
+						 "invalid lcore id %d for bitratestats"
+						 " must be >= 0\n", n);
+			}
+#endif
 			if (!strcmp(lgopts[opt_idx].name, "disable-crc-strip"))
 				rx_mode.hw_strip_crc = 0;
 			if (!strcmp(lgopts[opt_idx].name, "enable-lro"))
diff --git a/app/test-pmd/testpmd.c b/app/test-pmd/testpmd.c
index 3a57348..cfd5382 100644
--- a/app/test-pmd/testpmd.c
+++ b/app/test-pmd/testpmd.c
@@ -355,8 +355,12 @@ uint16_t nb_rx_queue_stats_mappings = 0;
 
 unsigned max_socket = 0;
 
+#ifdef RTE_LIBRTE_BITRATE
 /* Bitrate statistics */
 struct rte_stats_bitrates *bitrate_data;
+lcoreid_t bitrate_lcore_id;
+uint8_t bitrate_enabled;
+#endif
 
 /* Forward function declarations */
 static void map_port_queue_stats_mapping_registers(uint8_t pi, struct rte_port *port);
@@ -962,12 +966,18 @@ run_pkt_fwd_on_lcore(struct fwd_lcore *fc, packet_fwd_t pkt_fwd)
 		for (sm_id = 0; sm_id < nb_fs; sm_id++)
 			(*pkt_fwd)(fsm[sm_id]);
 #ifdef RTE_LIBRTE_BITRATE
-		tics_current = rte_rdtsc();
-		if (tics_current - tics_datum >= tics_per_1sec) {
-			/* Periodic bitrate calculation */
-			for (idx_port = 0; idx_port < cnt_ports; idx_port++)
-				rte_stats_bitrate_calc(bitrate_data, idx_port);
-			tics_datum = tics_current;
+		if (bitrate_enabled != 0 &&
+				bitrate_lcore_id == rte_lcore_id()) {
+			tics_current = rte_rdtsc();
+			if (tics_current - tics_datum >= tics_per_1sec) {
+				/* Periodic bitrate calculation */
+				for (idx_port = 0;
+						idx_port < cnt_ports;
+						idx_port++)
+					rte_stats_bitrate_calc(bitrate_data,
+						idx_port);
+				tics_datum = tics_current;
+			}
 		}
 #endif
 #ifdef RTE_LIBRTE_LATENCY_STATS
@@ -2238,6 +2248,9 @@ main(int argc, char** argv)
 		rte_panic("Empty set of forwarding logical cores - check the "
 			  "core mask supplied in the command parameters\n");
 
+	/* Bitrate stats disabled by default */
+	bitrate_enabled = 0;
+
 	argc -= diag;
 	argv += diag;
 	if (argc > 1)
@@ -2275,10 +2288,13 @@ main(int argc, char** argv)
 
 	/* Setup bitrate stats */
 #ifdef RTE_LIBRTE_BITRATE
-	bitrate_data = rte_stats_bitrate_create();
-	if (bitrate_data == NULL)
-		rte_exit(EXIT_FAILURE, "Could not allocate bitrate data.\n");
-	rte_stats_bitrate_reg(bitrate_data);
+	if (bitrate_enabled != 0) {
+		bitrate_data = rte_stats_bitrate_create();
+		if (bitrate_data == NULL)
+			rte_exit(EXIT_FAILURE,
+				"Could not allocate bitrate data.\n");
+		rte_stats_bitrate_reg(bitrate_data);
+	}
 #endif
 
 
diff --git a/app/test-pmd/testpmd.h b/app/test-pmd/testpmd.h
index a9ff07e..6443f7e 100644
--- a/app/test-pmd/testpmd.h
+++ b/app/test-pmd/testpmd.h
@@ -380,6 +380,11 @@ extern uint8_t latencystats_enabled;
 extern lcoreid_t latencystats_lcore_id;
 #endif
 
+#ifdef RTE_LIBRTE_BITRATE
+extern lcoreid_t bitrate_lcore_id;
+extern uint8_t bitrate_enabled;
+#endif
+
 extern struct rte_fdir_conf fdir_conf;
 
 /*
-- 
2.5.5

Re: [dpdk-dev] [PATCH v1] app/testpmd: add bitrate stats option

[Wu, Jingjing]

> -----Original Message-----
> From: Horton, Remy
> Sent: Wednesday, April 26, 2017 9:03 PM
> To: dev@dpdk.org
> Cc: Wu, Jingjing <jingjing.wu@intel.com>
> Subject: [PATCH v1] app/testpmd: add bitrate stats option
> 
> Bit-rate collation should only be done by one core. This patch adds an option to
> select which core performs the bit-rate calculation, which is also disabled by
> default.
> 
> Fixes: 7e4441c8efb9 ("app/testpmd: add bitrate statistics calculation")
> 
> Signed-off-by: Remy Horton <remy.horton@intel.com>
> ---
>  app/test-pmd/parameters.c | 15 +++++++++++++++
>  app/test-pmd/testpmd.c    | 36 ++++++++++++++++++++++++++----------
>  app/test-pmd/testpmd.h    |  5 +++++
>  3 files changed, 46 insertions(+), 10 deletions(-)
> 
> diff --git a/app/test-pmd/parameters.c b/app/test-pmd/parameters.c index
> 3f4d3a2..a6140b5 100644
> --- a/app/test-pmd/parameters.c
> +++ b/app/test-pmd/parameters.c
> @@ -536,6 +536,9 @@ launch_args_parse(int argc, char** argv)  #ifdef
> RTE_LIBRTE_LATENCY_STATS
>  		{ "latencystats",               1, 0, 0 },
>  #endif
> +#ifdef RTE_LIBRTE_BITRATE
> +		{ "bitratestats",               1, 0, 0 },
> +#endif
>  		{ "disable-crc-strip",          0, 0, 0 },
>  		{ "enable-lro",                 0, 0, 0 },
>  		{ "enable-rx-cksum",            0, 0, 0 },
> @@ -793,6 +796,18 @@ launch_args_parse(int argc, char** argv)
>  						 " must be >= 0\n", n);
>  			}
Please add description printing in function usage.


>  #ifdef RTE_LIBRTE_LATENCY_STATS
> @@ -2238,6 +2248,9 @@ main(int argc, char** argv)
>  		rte_panic("Empty set of forwarding logical cores - check the "
>  			  "core mask supplied in the command parameters\n");
> 
> +	/* Bitrate stats disabled by default */
> +	bitrate_enabled = 0;
You can assign it at the definition.
> +

Don't forget the testpmd doc to describe the parameters.

Thanks
Jingjing

[dpdk-dev] [PATCH v2] app/testpmd: add bitrate stats option

[Pablo de Lara]

From: Remy Horton <remy.horton@intel.com>

Bit-rate collation should only be done by one core. This patch adds
an option to select which core performs the bit-rate calculation,
which is also disabled by default.

Fixes: 7e4441c8efb9 ("app/testpmd: add bitrate statistics calculation")

Signed-off-by: Remy Horton <remy.horton@intel.com>
Acked-by: Pablo de Lara <pablo.de.lara.guarch@intel.com>
---

Changes in v2:
- Added parameter to documentation

 app/test-pmd/parameters.c             | 19 +++++++++++++++++-
 app/test-pmd/testpmd.c                | 36 +++++++++++++++++++++++++----------
 app/test-pmd/testpmd.h                |  5 +++++
 doc/guides/testpmd_app_ug/run_app.rst |  4 ++++
 4 files changed, 53 insertions(+), 11 deletions(-)

diff --git a/app/test-pmd/parameters.c b/app/test-pmd/parameters.c
index 3f4d3a2..a0300eb 100644
--- a/app/test-pmd/parameters.c
+++ b/app/test-pmd/parameters.c
@@ -201,7 +201,9 @@ usage(char* progname)
 	printf("  --disable-link-check: disable check on link status when "
 	       "starting/stopping ports.\n");
 	printf("  --no-lsc-interrupt: disable link status change interrupt.\n");
-	printf("  --no-rmv-interrupt: disable device removal interrupt.");
+	printf("  --no-rmv-interrupt: disable device removal interrupt.\n");
+	printf("  --bitrate-stats=N: set the logical core N to perform "
+		"bit-rate calculation.\n");
 }
 
 #ifdef RTE_LIBRTE_CMDLINE
@@ -536,6 +538,9 @@ launch_args_parse(int argc, char** argv)
 #ifdef RTE_LIBRTE_LATENCY_STATS
 		{ "latencystats",               1, 0, 0 },
 #endif
+#ifdef RTE_LIBRTE_BITRATE
+		{ "bitrate-stats",              1, 0, 0 },
+#endif
 		{ "disable-crc-strip",          0, 0, 0 },
 		{ "enable-lro",                 0, 0, 0 },
 		{ "enable-rx-cksum",            0, 0, 0 },
@@ -793,6 +798,18 @@ launch_args_parse(int argc, char** argv)
 						 " must be >= 0\n", n);
 			}
 #endif
+#ifdef RTE_LIBRTE_BITRATE
+			if (!strcmp(lgopts[opt_idx].name, "bitrate-stats")) {
+				n = atoi(optarg);
+				if (n >= 0) {
+					bitrate_lcore_id = (lcoreid_t) n;
+					bitrate_enabled = 1;
+				} else
+					rte_exit(EXIT_FAILURE,
+						 "invalid lcore id %d for bitrate stats"
+						 " must be >= 0\n", n);
+			}
+#endif
 			if (!strcmp(lgopts[opt_idx].name, "disable-crc-strip"))
 				rx_mode.hw_strip_crc = 0;
 			if (!strcmp(lgopts[opt_idx].name, "enable-lro"))
diff --git a/app/test-pmd/testpmd.c b/app/test-pmd/testpmd.c
index 3a57348..cfd5382 100644
--- a/app/test-pmd/testpmd.c
+++ b/app/test-pmd/testpmd.c
@@ -355,8 +355,12 @@ uint16_t nb_rx_queue_stats_mappings = 0;
 
 unsigned max_socket = 0;
 
+#ifdef RTE_LIBRTE_BITRATE
 /* Bitrate statistics */
 struct rte_stats_bitrates *bitrate_data;
+lcoreid_t bitrate_lcore_id;
+uint8_t bitrate_enabled;
+#endif
 
 /* Forward function declarations */
 static void map_port_queue_stats_mapping_registers(uint8_t pi, struct rte_port *port);
@@ -962,12 +966,18 @@ run_pkt_fwd_on_lcore(struct fwd_lcore *fc, packet_fwd_t pkt_fwd)
 		for (sm_id = 0; sm_id < nb_fs; sm_id++)
 			(*pkt_fwd)(fsm[sm_id]);
 #ifdef RTE_LIBRTE_BITRATE
-		tics_current = rte_rdtsc();
-		if (tics_current - tics_datum >= tics_per_1sec) {
-			/* Periodic bitrate calculation */
-			for (idx_port = 0; idx_port < cnt_ports; idx_port++)
-				rte_stats_bitrate_calc(bitrate_data, idx_port);
-			tics_datum = tics_current;
+		if (bitrate_enabled != 0 &&
+				bitrate_lcore_id == rte_lcore_id()) {
+			tics_current = rte_rdtsc();
+			if (tics_current - tics_datum >= tics_per_1sec) {
+				/* Periodic bitrate calculation */
+				for (idx_port = 0;
+						idx_port < cnt_ports;
+						idx_port++)
+					rte_stats_bitrate_calc(bitrate_data,
+						idx_port);
+				tics_datum = tics_current;
+			}
 		}
 #endif
 #ifdef RTE_LIBRTE_LATENCY_STATS
@@ -2238,6 +2248,9 @@ main(int argc, char** argv)
 		rte_panic("Empty set of forwarding logical cores - check the "
 			  "core mask supplied in the command parameters\n");
 
+	/* Bitrate stats disabled by default */
+	bitrate_enabled = 0;
+
 	argc -= diag;
 	argv += diag;
 	if (argc > 1)
@@ -2275,10 +2288,13 @@ main(int argc, char** argv)
 
 	/* Setup bitrate stats */
 #ifdef RTE_LIBRTE_BITRATE
-	bitrate_data = rte_stats_bitrate_create();
-	if (bitrate_data == NULL)
-		rte_exit(EXIT_FAILURE, "Could not allocate bitrate data.\n");
-	rte_stats_bitrate_reg(bitrate_data);
+	if (bitrate_enabled != 0) {
+		bitrate_data = rte_stats_bitrate_create();
+		if (bitrate_data == NULL)
+			rte_exit(EXIT_FAILURE,
+				"Could not allocate bitrate data.\n");
+		rte_stats_bitrate_reg(bitrate_data);
+	}
 #endif
 
 
diff --git a/app/test-pmd/testpmd.h b/app/test-pmd/testpmd.h
index a9ff07e..6443f7e 100644
--- a/app/test-pmd/testpmd.h
+++ b/app/test-pmd/testpmd.h
@@ -380,6 +380,11 @@ extern uint8_t latencystats_enabled;
 extern lcoreid_t latencystats_lcore_id;
 #endif
 
+#ifdef RTE_LIBRTE_BITRATE
+extern lcoreid_t bitrate_lcore_id;
+extern uint8_t bitrate_enabled;
+#endif
+
 extern struct rte_fdir_conf fdir_conf;
 
 /*
diff --git a/doc/guides/testpmd_app_ug/run_app.rst b/doc/guides/testpmd_app_ug/run_app.rst
index df5a0ee..b9be1e6 100644
--- a/doc/guides/testpmd_app_ug/run_app.rst
+++ b/doc/guides/testpmd_app_ug/run_app.rst
@@ -465,3 +465,7 @@ The commandline options are:
 *   ``--disable-link-check``
 
     Disable check on link status when starting/stopping ports.
+
+*  ``--bitrate-stats=N``
+
+    Set the logical core N to perform bitrate calculation.
-- 
2.7.4

Re: [dpdk-dev] [PATCH v2] app/testpmd: add bitrate stats option

[Thomas Monjalon]

28/04/2017 13:00, Pablo de Lara:
> From: Remy Horton <remy.horton@intel.com>
> 
> Bit-rate collation should only be done by one core. This patch adds
> an option to select which core performs the bit-rate calculation,
> which is also disabled by default.
> 
> Fixes: 7e4441c8efb9 ("app/testpmd: add bitrate statistics calculation")
> 
> Signed-off-by: Remy Horton <remy.horton@intel.com>
> Acked-by: Pablo de Lara <pablo.de.lara.guarch@intel.com>

Applied, thanks

Re: [dpdk-dev] [PATCH v2] app/testpmd: add bitrate stats option

[Patil, Harish]

>From: Remy Horton <remy.horton@intel.com>
>
>Bit-rate collation should only be done by one core. This patch adds
>an option to select which core performs the bit-rate calculation,
>which is also disabled by default.
>
>Fixes: 7e4441c8efb9 ("app/testpmd: add bitrate statistics calculation")
>
>Signed-off-by: Remy Horton <remy.horton@intel.com>
>Acked-by: Pablo de Lara <pablo.de.lara.guarch@intel.com>
>---
>
>Changes in v2:
>- Added parameter to documentation
>
> app/test-pmd/parameters.c             | 19 +++++++++++++++++-
> app/test-pmd/testpmd.c                | 36
>+++++++++++++++++++++++++----------
> app/test-pmd/testpmd.h                |  5 +++++
> doc/guides/testpmd_app_ug/run_app.rst |  4 ++++
> 4 files changed, 53 insertions(+), 11 deletions(-)
>
>diff --git a/app/test-pmd/parameters.c b/app/test-pmd/parameters.c
>index 3f4d3a2..a0300eb 100644
>--- a/app/test-pmd/parameters.c
>+++ b/app/test-pmd/parameters.c
>@@ -201,7 +201,9 @@ usage(char* progname)
> 	printf("  --disable-link-check: disable check on link status when "
> 	       "starting/stopping ports.\n");
> 	printf("  --no-lsc-interrupt: disable link status change interrupt.\n");
>-	printf("  --no-rmv-interrupt: disable device removal interrupt.");
>+	printf("  --no-rmv-interrupt: disable device removal interrupt.\n");
>+	printf("  --bitrate-stats=N: set the logical core N to perform "
>+		"bit-rate calculation.\n");
> }
> 
> #ifdef RTE_LIBRTE_CMDLINE
>@@ -536,6 +538,9 @@ launch_args_parse(int argc, char** argv)
> #ifdef RTE_LIBRTE_LATENCY_STATS
> 		{ "latencystats",               1, 0, 0 },
> #endif
>+#ifdef RTE_LIBRTE_BITRATE
>+		{ "bitrate-stats",              1, 0, 0 },
>+#endif
> 		{ "disable-crc-strip",          0, 0, 0 },
> 		{ "enable-lro",                 0, 0, 0 },
> 		{ "enable-rx-cksum",            0, 0, 0 },
>@@ -793,6 +798,18 @@ launch_args_parse(int argc, char** argv)
> 						 " must be >= 0\n", n);
> 			}
> #endif
>+#ifdef RTE_LIBRTE_BITRATE
>+			if (!strcmp(lgopts[opt_idx].name, "bitrate-stats")) {
>+				n = atoi(optarg);
>+				if (n >= 0) {
>+					bitrate_lcore_id = (lcoreid_t) n;
>+					bitrate_enabled = 1;
>+				} else
>+					rte_exit(EXIT_FAILURE,
>+						 "invalid lcore id %d for bitrate stats"
>+						 " must be >= 0\n", n);
>+			}
>+#endif
> 			if (!strcmp(lgopts[opt_idx].name, "disable-crc-strip"))
> 				rx_mode.hw_strip_crc = 0;
> 			if (!strcmp(lgopts[opt_idx].name, "enable-lro"))
>diff --git a/app/test-pmd/testpmd.c b/app/test-pmd/testpmd.c
>index 3a57348..cfd5382 100644
>--- a/app/test-pmd/testpmd.c
>+++ b/app/test-pmd/testpmd.c
>@@ -355,8 +355,12 @@ uint16_t nb_rx_queue_stats_mappings = 0;
> 
> unsigned max_socket = 0;
> 
>+#ifdef RTE_LIBRTE_BITRATE
> /* Bitrate statistics */
> struct rte_stats_bitrates *bitrate_data;
>+lcoreid_t bitrate_lcore_id;
>+uint8_t bitrate_enabled;
>+#endif
> 
> /* Forward function declarations */
> static void map_port_queue_stats_mapping_registers(uint8_t pi, struct
>rte_port *port);
>@@ -962,12 +966,18 @@ run_pkt_fwd_on_lcore(struct fwd_lcore *fc,
>packet_fwd_t pkt_fwd)
> 		for (sm_id = 0; sm_id < nb_fs; sm_id++)
> 			(*pkt_fwd)(fsm[sm_id]);
> #ifdef RTE_LIBRTE_BITRATE
>-		tics_current = rte_rdtsc();
>-		if (tics_current - tics_datum >= tics_per_1sec) {
>-			/* Periodic bitrate calculation */
>-			for (idx_port = 0; idx_port < cnt_ports; idx_port++)
>-				rte_stats_bitrate_calc(bitrate_data, idx_port);
>-			tics_datum = tics_current;
>+		if (bitrate_enabled != 0 &&
>+				bitrate_lcore_id == rte_lcore_id()) {
>+			tics_current = rte_rdtsc();
>+			if (tics_current - tics_datum >= tics_per_1sec) {
>+				/* Periodic bitrate calculation */
>+				for (idx_port = 0;
>+						idx_port < cnt_ports;
>+						idx_port++)
>+					rte_stats_bitrate_calc(bitrate_data,
>+						idx_port);
>+				tics_datum = tics_current;
>+			}
> 		}
> #endif
> #ifdef RTE_LIBRTE_LATENCY_STATS
>@@ -2238,6 +2248,9 @@ main(int argc, char** argv)
> 		rte_panic("Empty set of forwarding logical cores - check the "
> 			  "core mask supplied in the command parameters\n");
> 
>+	/* Bitrate stats disabled by default */
>+	bitrate_enabled = 0;
>+
> 	argc -= diag;
> 	argv += diag;
> 	if (argc > 1)
>@@ -2275,10 +2288,13 @@ main(int argc, char** argv)
> 
> 	/* Setup bitrate stats */
> #ifdef RTE_LIBRTE_BITRATE
>-	bitrate_data = rte_stats_bitrate_create();
>-	if (bitrate_data == NULL)
>-		rte_exit(EXIT_FAILURE, "Could not allocate bitrate data.\n");
>-	rte_stats_bitrate_reg(bitrate_data);
>+	if (bitrate_enabled != 0) {
>+		bitrate_data = rte_stats_bitrate_create();
>+		if (bitrate_data == NULL)
>+			rte_exit(EXIT_FAILURE,
>+				"Could not allocate bitrate data.\n");
>+		rte_stats_bitrate_reg(bitrate_data);
>+	}
> #endif
> 
> 
>diff --git a/app/test-pmd/testpmd.h b/app/test-pmd/testpmd.h
>index a9ff07e..6443f7e 100644
>--- a/app/test-pmd/testpmd.h
>+++ b/app/test-pmd/testpmd.h
>@@ -380,6 +380,11 @@ extern uint8_t latencystats_enabled;
> extern lcoreid_t latencystats_lcore_id;
> #endif
> 
>+#ifdef RTE_LIBRTE_BITRATE
>+extern lcoreid_t bitrate_lcore_id;
>+extern uint8_t bitrate_enabled;
>+#endif
>+
> extern struct rte_fdir_conf fdir_conf;
> 
> /*
>diff --git a/doc/guides/testpmd_app_ug/run_app.rst
>b/doc/guides/testpmd_app_ug/run_app.rst
>index df5a0ee..b9be1e6 100644
>--- a/doc/guides/testpmd_app_ug/run_app.rst
>+++ b/doc/guides/testpmd_app_ug/run_app.rst
>@@ -465,3 +465,7 @@ The commandline options are:
> *   ``--disable-link-check``
> 
>     Disable check on link status when starting/stopping ports.
>+
>+*  ``--bitrate-stats=N``
>+
>+    Set the logical core N to perform bitrate calculation.
>-- 
>2.7.4
>
>

Hi Remy,
Have a small suggestion here.
Since testpmd uses new libraries of librte_latencystats and
librte_bitratestats it hurts packet processing performance.
Many users who use testpmd to do the initial performance benchmarks may
not be aware of such a feature is default enabled.
So can we disable this feature by default in the config?
·         CONFIG_RTE_LIBRTE_BITRATE=n
·         CONFIG_RTE_LIBRTE_LATENCY_STATS=n
Only those folks interested in latency/jitter measurements can recompile
with those configs enabled.
Thanks.

Re: [dpdk-dev] [PATCH v2] app/testpmd: add bitrate stats option

[Thomas Monjalon]

01/05/2017 22:07, Patil, Harish:
> Hi Remy,
> Have a small suggestion here.
> Since testpmd uses new libraries of librte_latencystats and
> librte_bitratestats it hurts packet processing performance.
> Many users who use testpmd to do the initial performance benchmarks may
> not be aware of such a feature is default enabled.

Yes, the default config of testpmd must give good performance.

> So can we disable this feature by default in the config?
> ·         CONFIG_RTE_LIBRTE_BITRATE=n
> ·         CONFIG_RTE_LIBRTE_LATENCY_STATS=n
> Only those folks interested in latency/jitter measurements can recompile
> with those configs enabled.

I disagree about compile-time options.
It should be a run-time option of testpmd.

Please Remy (or others),
disable the metrics in the default configuration of testpmd,
before the 17.05 release.
You have few days, it is urgent.

Re: [dpdk-dev] [PATCH v2] app/testpmd: add bitrate stats option

[De Lara Guarch, Pablo]

> -----Original Message-----
> From: Thomas Monjalon [mailto:thomas@monjalon.net]
> Sent: Monday, May 01, 2017 9:22 PM
> To: Patil, Harish; Horton, Remy
> Cc: dev@dpdk.org; De Lara Guarch, Pablo; Wu, Jingjing
> Subject: Re: [dpdk-dev] [PATCH v2] app/testpmd: add bitrate stats option
> 
> 01/05/2017 22:07, Patil, Harish:
> > Hi Remy,
> > Have a small suggestion here.
> > Since testpmd uses new libraries of librte_latencystats and
> > librte_bitratestats it hurts packet processing performance.
> > Many users who use testpmd to do the initial performance benchmarks
> may
> > not be aware of such a feature is default enabled.
> 
> Yes, the default config of testpmd must give good performance.
> 
> > So can we disable this feature by default in the config?
> > *         CONFIG_RTE_LIBRTE_BITRATE=n
> > *         CONFIG_RTE_LIBRTE_LATENCY_STATS=n
> > Only those folks interested in latency/jitter measurements can recompile
> > with those configs enabled.
> 
> I disagree about compile-time options.
> It should be a run-time option of testpmd.
> 
> Please Remy (or others),
> disable the metrics in the default configuration of testpmd,
> before the 17.05 release.
> You have few days, it is urgent.

Bitrate stats are disabled by default, in testpmd.
I assume that the code that you want to avoid is:

                for (sm_id = 0; sm_id < nb_fs; sm_id++)
                        (*pkt_fwd)(fsm[sm_id]);
#ifdef RTE_LIBRTE_BITRATE
                if (bitrate_enabled != 0 &&
                                bitrate_lcore_id == rte_lcore_id()) {
                        tics_current = rte_rdtsc();
                        if (tics_current - tics_datum >= tics_per_1sec) {

Unless --bitrate-stats is used, bitrate_enabled = 0, so all this code won't be run.

I can send a patch to check the latencystats_enabled flag first, following the approach above, here:

#ifdef RTE_LIBRTE_LATENCY_STATS
                if (latencystats_lcore_id == rte_lcore_id())


Thanks,
Pablo