patches for DPDK stable branches
 help / color / mirror / Atom feed
* [PATCH] examples/link_status_interrupt: fix stats refresh rate
@ 2022-05-30  9:37 Raja Zidane
  2022-06-26 21:22 ` Thomas Monjalon
  2022-07-07  8:22 ` [PATCH v2] " Omar Awaysa
  0 siblings, 2 replies; 4+ messages in thread
From: Raja Zidane @ 2022-05-30  9:37 UTC (permalink / raw)
  To: dev; +Cc: matan, stable

TIMER_MILLISECOND is defined as the number of cpu cycles per millisecond,
current definition is correct for cores with frequency of 2GHZ, for cores
with different frequency, it caused different periods between refresh,
(i.e. the definition is about 14ms on ARM cores).

Use dpdk API to get CPU frequency, to define TIMER_MILLISECOND.

Fixes: af75078fece3 ("first public release")
Cc: stable@dpdk.org

Signed-off-by: Raja Zidane <rzidane@nvidia.com>
Acked-by: Matan Azrad <matan@nvidia.com>
---
 examples/link_status_interrupt/main.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/examples/link_status_interrupt/main.c b/examples/link_status_interrupt/main.c
index 551f0524da..eb7a74d37c 100644
--- a/examples/link_status_interrupt/main.c
+++ b/examples/link_status_interrupt/main.c
@@ -101,9 +101,10 @@ struct lsi_port_statistics {
 struct lsi_port_statistics port_statistics[RTE_MAX_ETHPORTS];
 
 /* A tsc-based timer responsible for triggering statistics printout */
-#define TIMER_MILLISECOND 2000000ULL /* around 1ms at 2 Ghz */
+#define TIMER_MILLISECOND (rte_get_tsc_hz() / 1000)
 #define MAX_TIMER_PERIOD 86400 /* 1 day max */
-static int64_t timer_period = 10 * TIMER_MILLISECOND * 1000; /* default period is 10 seconds */
+#define DEFAULT_TIMER_PERIOD 10UL /* default period is 10 seconds */
+static int64_t timer_period;
 
 /* Print out statistics on packets dropped */
 static void
@@ -371,6 +372,7 @@ lsi_parse_args(int argc, char **argv)
 	};
 
 	argvopt = argv;
+	timer_period = DEFAULT_TIMER_PERIOD;
 
 	while ((opt = getopt_long(argc, argvopt, "p:q:T:",
 				  lgopts, &option_index)) != EOF) {
-- 
2.21.0


^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH] examples/link_status_interrupt: fix stats refresh rate
  2022-05-30  9:37 [PATCH] examples/link_status_interrupt: fix stats refresh rate Raja Zidane
@ 2022-06-26 21:22 ` Thomas Monjalon
  2022-07-07  8:22 ` [PATCH v2] " Omar Awaysa
  1 sibling, 0 replies; 4+ messages in thread
From: Thomas Monjalon @ 2022-06-26 21:22 UTC (permalink / raw)
  To: Raja Zidane; +Cc: dev, matan, stable

30/05/2022 11:37, Raja Zidane:
> TIMER_MILLISECOND is defined as the number of cpu cycles per millisecond,
> current definition is correct for cores with frequency of 2GHZ, for cores
> with different frequency, it caused different periods between refresh,
> (i.e. the definition is about 14ms on ARM cores).
> 
> Use dpdk API to get CPU frequency, to define TIMER_MILLISECOND.
> 
> Fixes: af75078fece3 ("first public release")
> Cc: stable@dpdk.org
> 
> Signed-off-by: Raja Zidane <rzidane@nvidia.com>
> Acked-by: Matan Azrad <matan@nvidia.com>
> ---
> --- a/examples/link_status_interrupt/main.c
> +++ b/examples/link_status_interrupt/main.c
>  /* A tsc-based timer responsible for triggering statistics printout */
> -#define TIMER_MILLISECOND 2000000ULL /* around 1ms at 2 Ghz */
> +#define TIMER_MILLISECOND (rte_get_tsc_hz() / 1000)

It is preferred to use rte_get_timer_hz().

>  #define MAX_TIMER_PERIOD 86400 /* 1 day max */
> -static int64_t timer_period = 10 * TIMER_MILLISECOND * 1000; /* default period is 10 seconds */
> +#define DEFAULT_TIMER_PERIOD 10UL /* default period is 10 seconds */
> +static int64_t timer_period;
[...]
> +	timer_period = DEFAULT_TIMER_PERIOD;

After a quick look, it seems we are missing the operation
	* TIMER_MILLISECOND * 1000
Isn't it?



^ permalink raw reply	[flat|nested] 4+ messages in thread

* [PATCH v2] examples/link_status_interrupt: fix stats refresh rate
  2022-05-30  9:37 [PATCH] examples/link_status_interrupt: fix stats refresh rate Raja Zidane
  2022-06-26 21:22 ` Thomas Monjalon
@ 2022-07-07  8:22 ` Omar Awaysa
  2022-07-08 14:47   ` Thomas Monjalon
  1 sibling, 1 reply; 4+ messages in thread
From: Omar Awaysa @ 2022-07-07  8:22 UTC (permalink / raw)
  To: dev; +Cc: thomas, rasland, Raja Zidane, stable

From: Raja Zidane <rzidane@nvidia.com>

TIMER_MILLISECOND is defined as the number of cpu cycles per millisecond,
current definition is correct for cores with frequency of 2GHZ, for cores
with different frequency, it caused different periods between refresh,
(i.e. the definition is about 14ms on ARM cores).

Use dpdk API to get CPU frequency, to define TIMER_MILLISECOND.

Fixes: af75078fece3 ("first public release")
Cc: stable@dpdk.org

Signed-off-by: Omar Awaysa <omara@nvidia.com>
---
v2: use rte_get_timer_hz instead of rte_get_tsc_hz
    update initial timer value to be in seconds
---
---
 examples/link_status_interrupt/main.c | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/examples/link_status_interrupt/main.c b/examples/link_status_interrupt/main.c
index 551f0524da..79dd7461fd 100644
--- a/examples/link_status_interrupt/main.c
+++ b/examples/link_status_interrupt/main.c
@@ -101,9 +101,10 @@ struct lsi_port_statistics {
 struct lsi_port_statistics port_statistics[RTE_MAX_ETHPORTS];
 
 /* A tsc-based timer responsible for triggering statistics printout */
-#define TIMER_MILLISECOND 2000000ULL /* around 1ms at 2 Ghz */
+#define TIMER_MILLISECOND (rte_get_timer_hz() / 1000)
 #define MAX_TIMER_PERIOD 86400 /* 1 day max */
-static int64_t timer_period = 10 * TIMER_MILLISECOND * 1000; /* default period is 10 seconds */
+#define DEFAULT_TIMER_PERIOD 10UL /* default period is 10 seconds */
+static int64_t timer_period;
 
 /* Print out statistics on packets dropped */
 static void
@@ -370,6 +371,8 @@ lsi_parse_args(int argc, char **argv)
 		{NULL, 0, 0, 0}
 	};
 
+        timer_period = DEFAULT_TIMER_PERIOD * TIMER_MILLISECOND * 1000;
+
 	argvopt = argv;
 
 	while ((opt = getopt_long(argc, argvopt, "p:q:T:",
-- 
2.17.1


^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH v2] examples/link_status_interrupt: fix stats refresh rate
  2022-07-07  8:22 ` [PATCH v2] " Omar Awaysa
@ 2022-07-08 14:47   ` Thomas Monjalon
  0 siblings, 0 replies; 4+ messages in thread
From: Thomas Monjalon @ 2022-07-08 14:47 UTC (permalink / raw)
  To: Omar Awaysa; +Cc: dev, rasland, Raja Zidane, stable

07/07/2022 10:22, Omar Awaysa:
> From: Raja Zidane <rzidane@nvidia.com>
> 
> TIMER_MILLISECOND is defined as the number of cpu cycles per millisecond,
> current definition is correct for cores with frequency of 2GHZ, for cores
> with different frequency, it caused different periods between refresh,
> (i.e. the definition is about 14ms on ARM cores).
> 
> Use dpdk API to get CPU frequency, to define TIMER_MILLISECOND.
> 
> Fixes: af75078fece3 ("first public release")
> Cc: stable@dpdk.org
> 
> Signed-off-by: Omar Awaysa <omara@nvidia.com>

Applied (with indent fixed), thanks.




^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2022-07-08 14:47 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-05-30  9:37 [PATCH] examples/link_status_interrupt: fix stats refresh rate Raja Zidane
2022-06-26 21:22 ` Thomas Monjalon
2022-07-07  8:22 ` [PATCH v2] " Omar Awaysa
2022-07-08 14:47   ` Thomas Monjalon

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).