* Re: [dpdk-dev] [RFC] rte_timer: add rte_timer_next_ticks
2019-06-10 22:44 [dpdk-dev] [RFC] rte_timer: add rte_timer_next_ticks Stephen Hemminger
@ 2019-06-11 13:37 ` Sanford, Robert
2019-06-11 14:27 ` Stephen Hemminger
2019-06-12 18:19 ` Carrillo, Erik G
2019-12-17 0:55 ` [dpdk-dev] [PATCH v1] " Stephen Hemminger
2 siblings, 1 reply; 8+ messages in thread
From: Sanford, Robert @ 2019-06-11 13:37 UTC (permalink / raw)
To: Stephen Hemminger, erik.g.carrillo; +Cc: dev
Hi Stephen,
The code seems fine. My only comment is that there is not a blank line before the new code, in both the .c and .h.
--
Regards,
Robert
On 6/10/19, 6:45 PM, "Stephen Hemminger" <stephen@networkplumber.org> wrote:
It is useful to know when the next timer will expire when
using rte_epoll_wait (or sleep when idle). This experimental
API provides a hook to query the number of ticks remaining.
Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
---
lib/librte_timer/rte_timer.c | 21 +++++++++++++++++++++
lib/librte_timer/rte_timer.h | 14 ++++++++++++++
lib/librte_timer/rte_timer_version.map | 1 +
3 files changed, 36 insertions(+)
diff --git a/lib/librte_timer/rte_timer.c b/lib/librte_timer/rte_timer.c
index dd795392244c..e9bd3c845470 100644
--- a/lib/librte_timer/rte_timer.c
+++ b/lib/librte_timer/rte_timer.c
@@ -1032,6 +1032,27 @@ rte_timer_stop_all(uint32_t timer_data_id, unsigned int *walk_lcores,
return 0;
}
+int64_t __rte_experimental
+rte_timer_next_ticks(void)
+{
+ struct priv_timer *priv_timer = default_timer_data.priv_timer;
+ unsigned int lcore_id = rte_lcore_id();
+ uint64_t cur_time = rte_get_timer_cycles();
+ const struct rte_timer *tm;
+ int64_t left = -1;
+
+ rte_spinlock_lock(&priv_timer[lcore_id].list_lock);
+ tm = priv_timer[lcore_id].pending_head.sl_next[0];
+ if (tm) {
+ left = tm->expire - cur_time;
+ if (left < 0)
+ left = 0;
+ }
+ rte_spinlock_unlock(&priv_timer[lcore_id].list_lock);
+
+ return left;
+}
+
/* dump statistics about timers */
static void
__rte_timer_dump_stats(struct rte_timer_data *timer_data __rte_unused, FILE *f)
diff --git a/lib/librte_timer/rte_timer.h b/lib/librte_timer/rte_timer.h
index 2196934b2e29..1c1d3cfd6d4f 100644
--- a/lib/librte_timer/rte_timer.h
+++ b/lib/librte_timer/rte_timer.h
@@ -338,6 +338,20 @@ void rte_timer_stop_sync(struct rte_timer *tim);
*/
int rte_timer_pending(struct rte_timer *tim);
+/**
+ * Time until the next timer
+ *
+ * This function gives the interval until the next timer
+ * will be active.
+ *
+ * @return
+ * 0: A timer is already pending
+ * -1: No timer is pending
+ * otherwise ticks until the next timer.
+ */
+int64_t __rte_experimental
+rte_timer_next_ticks(void);
+
/**
* Manage the timer list and execute callback functions.
*
diff --git a/lib/librte_timer/rte_timer_version.map b/lib/librte_timer/rte_timer_version.map
index 72f75c818134..d64400bcc7bd 100644
--- a/lib/librte_timer/rte_timer_version.map
+++ b/lib/librte_timer/rte_timer_version.map
@@ -33,6 +33,7 @@ EXPERIMENTAL {
rte_timer_alt_stop;
rte_timer_data_alloc;
rte_timer_data_dealloc;
+ rte_timer_next_ticks;
rte_timer_stop_all;
rte_timer_subsystem_finalize;
};
--
2.20.1
^ permalink raw reply related [flat|nested] 8+ messages in thread* Re: [dpdk-dev] [RFC] rte_timer: add rte_timer_next_ticks
2019-06-11 13:37 ` Sanford, Robert
@ 2019-06-11 14:27 ` Stephen Hemminger
2019-06-11 14:45 ` Sanford, Robert
0 siblings, 1 reply; 8+ messages in thread
From: Stephen Hemminger @ 2019-06-11 14:27 UTC (permalink / raw)
To: Sanford, Robert; +Cc: erik.g.carrillo, dev
On Tue, 11 Jun 2019 13:37:33 +0000
"Sanford, Robert" <rsanford@akamai.com> wrote:
> Hi Stephen,
>
> The code seems fine. My only comment is that there is not a blank line before the new code, in both the .c and .h.
>
> --
> Regards
Where, I see the blank line between functions and checkpatch is happy.
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [dpdk-dev] [RFC] rte_timer: add rte_timer_next_ticks
2019-06-11 14:27 ` Stephen Hemminger
@ 2019-06-11 14:45 ` Sanford, Robert
0 siblings, 0 replies; 8+ messages in thread
From: Sanford, Robert @ 2019-06-11 14:45 UTC (permalink / raw)
To: Stephen Hemminger; +Cc: erik.g.carrillo, dev
You are correct. I just looked at it in Patchwork. Sorry about that. (I should learn never to trust Outlook for Mac.)
On 6/11/19, 10:27 AM, "Stephen Hemminger" <stephen@networkplumber.org> wrote:
On Tue, 11 Jun 2019 13:37:33 +0000
"Sanford, Robert" <rsanford@akamai.com> wrote:
> Hi Stephen,
>
> The code seems fine. My only comment is that there is not a blank line before the new code, in both the .c and .h.
>
> --
> Regards
Where, I see the blank line between functions and checkpatch is happy.
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [dpdk-dev] [RFC] rte_timer: add rte_timer_next_ticks
2019-06-10 22:44 [dpdk-dev] [RFC] rte_timer: add rte_timer_next_ticks Stephen Hemminger
2019-06-11 13:37 ` Sanford, Robert
@ 2019-06-12 18:19 ` Carrillo, Erik G
2019-12-17 0:55 ` [dpdk-dev] [PATCH v1] " Stephen Hemminger
2 siblings, 0 replies; 8+ messages in thread
From: Carrillo, Erik G @ 2019-06-12 18:19 UTC (permalink / raw)
To: Stephen Hemminger, rsanford; +Cc: dev
Hi Stephen,
This looks like a useful addition. Some comments in-line:
> -----Original Message-----
> From: Stephen Hemminger <stephen@networkplumber.org>
> Sent: Monday, June 10, 2019 5:45 PM
> To: rsanford@akamai.com; Carrillo, Erik G <erik.g.carrillo@intel.com>
> Cc: dev@dpdk.org; Stephen Hemminger <stephen@networkplumber.org>
> Subject: [RFC] rte_timer: add rte_timer_next_ticks
>
> It is useful to know when the next timer will expire when using
> rte_epoll_wait (or sleep when idle). This experimental API provides a hook
> to query the number of ticks remaining.
>
> Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
> ---
> lib/librte_timer/rte_timer.c | 21 +++++++++++++++++++++
> lib/librte_timer/rte_timer.h | 14 ++++++++++++++
> lib/librte_timer/rte_timer_version.map | 1 +
> 3 files changed, 36 insertions(+)
>
> diff --git a/lib/librte_timer/rte_timer.c b/lib/librte_timer/rte_timer.c index
> dd795392244c..e9bd3c845470 100644
> --- a/lib/librte_timer/rte_timer.c
> +++ b/lib/librte_timer/rte_timer.c
> @@ -1032,6 +1032,27 @@ rte_timer_stop_all(uint32_t timer_data_id,
> unsigned int *walk_lcores,
> return 0;
> }
>
> +int64_t __rte_experimental
> +rte_timer_next_ticks(void)
> +{
> + struct priv_timer *priv_timer = default_timer_data.priv_timer;
The timer data instance used above is used by the legacy API. We should do something like the following instead:
--- a/lib/librte_timer/rte_timer.c
+++ b/lib/librte_timer/rte_timer.c
@@ -1035,15 +1035,24 @@ rte_timer_stop_all(uint32_t timer_data_id, unsigned int *walk_lcores,
int64_t __rte_experimental
rte_timer_next_ticks(void)
{
- struct priv_timer *priv_timer = default_timer_data.priv_timer;
+ struct priv_timer *priv_timer;
+ struct rte_timer_data *timer_data;
unsigned int lcore_id = rte_lcore_id();
uint64_t cur_time = rte_get_timer_cycles();
const struct rte_timer *tm;
int64_t left = -1;
+ TIMER_DATA_VALID_GET_OR_ERR_RET(default_data_id, timer_data, -EINVAL);
+ priv_timer = timer_data->priv_timer;
This will select the correct instance that was allocated in shared memory.
> + unsigned int lcore_id = rte_lcore_id();
> + uint64_t cur_time = rte_get_timer_cycles();
> + const struct rte_timer *tm;
> + int64_t left = -1;
> +
> + rte_spinlock_lock(&priv_timer[lcore_id].list_lock);
> + tm = priv_timer[lcore_id].pending_head.sl_next[0];
> + if (tm) {
> + left = tm->expire - cur_time;
> + if (left < 0)
> + left = 0;
> + }
> + rte_spinlock_unlock(&priv_timer[lcore_id].list_lock);
> +
> + return left;
> +}
> +
> /* dump statistics about timers */
> static void
> __rte_timer_dump_stats(struct rte_timer_data *timer_data __rte_unused,
> FILE *f) diff --git a/lib/librte_timer/rte_timer.h b/lib/librte_timer/rte_timer.h
> index 2196934b2e29..1c1d3cfd6d4f 100644
> --- a/lib/librte_timer/rte_timer.h
> +++ b/lib/librte_timer/rte_timer.h
> @@ -338,6 +338,20 @@ void rte_timer_stop_sync(struct rte_timer *tim);
> */
> int rte_timer_pending(struct rte_timer *tim);
>
> +/**
This comment is missing the "@warning" and "@b EXPERIMENTAL" lines.
> + * Time until the next timer
> + *
> + * This function gives the interval until the next timer
s/interval/interval in ticks/ ?
> + * will be active.
> + *
> + * @return
> + * 0: A timer is already pending
I would say "The next timer has already expired" or "A timer is already running" here... the timer library describes timers that have been started but have not yet expired as "pending".
> + * -1: No timer is pending
> + * otherwise ticks until the next timer.
> + */
> +int64_t __rte_experimental
> +rte_timer_next_ticks(void);
> +
> /**
> * Manage the timer list and execute callback functions.
> *
> diff --git a/lib/librte_timer/rte_timer_version.map
> b/lib/librte_timer/rte_timer_version.map
> index 72f75c818134..d64400bcc7bd 100644
> --- a/lib/librte_timer/rte_timer_version.map
> +++ b/lib/librte_timer/rte_timer_version.map
> @@ -33,6 +33,7 @@ EXPERIMENTAL {
> rte_timer_alt_stop;
> rte_timer_data_alloc;
> rte_timer_data_dealloc;
> + rte_timer_next_ticks;
> rte_timer_stop_all;
> rte_timer_subsystem_finalize;
> };
> --
> 2.20.1
Thanks,
Erik
^ permalink raw reply [flat|nested] 8+ messages in thread* [dpdk-dev] [PATCH v1] rte_timer: add rte_timer_next_ticks
2019-06-10 22:44 [dpdk-dev] [RFC] rte_timer: add rte_timer_next_ticks Stephen Hemminger
2019-06-11 13:37 ` Sanford, Robert
2019-06-12 18:19 ` Carrillo, Erik G
@ 2019-12-17 0:55 ` Stephen Hemminger
2019-12-20 22:43 ` Carrillo, Erik G
2 siblings, 1 reply; 8+ messages in thread
From: Stephen Hemminger @ 2019-12-17 0:55 UTC (permalink / raw)
To: dev; +Cc: erik.g.carrillo, Stephen Hemminger
It is useful to know when the next timer will expire when
using rte_epoll_wait (or sleep when idle). This experimental
API provides a hook to query the number of ticks remaining.
Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
---
v1 - incorporate feedback from old RFC
lib/librte_timer/rte_timer.c | 27 ++++++++++++++++++++++++++
lib/librte_timer/rte_timer.h | 16 +++++++++++++++
lib/librte_timer/rte_timer_version.map | 1 +
3 files changed, 44 insertions(+)
diff --git a/lib/librte_timer/rte_timer.c b/lib/librte_timer/rte_timer.c
index ca88454ff68a..a1ed186cf591 100644
--- a/lib/librte_timer/rte_timer.c
+++ b/lib/librte_timer/rte_timer.c
@@ -978,6 +978,33 @@ rte_timer_stop_all(uint32_t timer_data_id, unsigned int *walk_lcores,
return 0;
}
+int64_t
+rte_timer_next_ticks(void)
+{
+ unsigned int lcore_id = rte_lcore_id();
+ struct rte_timer_data *timer_data;
+ struct priv_timer *priv_timer;
+ const struct rte_timer *tm;
+ uint64_t cur_time;
+ int64_t left = -ENOENT;
+
+ TIMER_DATA_VALID_GET_OR_ERR_RET(default_data_id, timer_data, -EINVAL);
+
+ priv_timer = timer_data->priv_timer;
+ cur_time = rte_get_timer_cycles();
+
+ rte_spinlock_lock(&priv_timer[lcore_id].list_lock);
+ tm = priv_timer[lcore_id].pending_head.sl_next[0];
+ if (tm) {
+ left = tm->expire - cur_time;
+ if (left < 0)
+ left = 0;
+ }
+ rte_spinlock_unlock(&priv_timer[lcore_id].list_lock);
+
+ return left;
+}
+
/* dump statistics about timers */
static void
__rte_timer_dump_stats(struct rte_timer_data *timer_data __rte_unused, FILE *f)
diff --git a/lib/librte_timer/rte_timer.h b/lib/librte_timer/rte_timer.h
index 9dc5fc309249..c5031d0448ba 100644
--- a/lib/librte_timer/rte_timer.h
+++ b/lib/librte_timer/rte_timer.h
@@ -331,6 +331,22 @@ void rte_timer_stop_sync(struct rte_timer *tim);
*/
int rte_timer_pending(struct rte_timer *tim);
+/**
+ * @warning
+ * @b EXPERIMENTAL: this API may change without prior notice
+ *
+ * Time until the next timer
+ * This function gives the ticks until the next timer will be active.
+ *
+ * @return
+ * - -EINVAL: invalid timer data instance identifier
+ * - -ENOENT: no timer pending
+ * - 0: a timer is pending and will run at next rte_timer_manage()
+ * - >0: ticks until the next timer is ready
+ */
+__rte_experimental
+int64_t rte_timer_next_ticks(void);
+
/**
* Manage the timer list and execute callback functions.
*
diff --git a/lib/librte_timer/rte_timer_version.map b/lib/librte_timer/rte_timer_version.map
index 2a59d3f081c4..4471cef92be5 100644
--- a/lib/librte_timer/rte_timer_version.map
+++ b/lib/librte_timer/rte_timer_version.map
@@ -23,6 +23,7 @@ EXPERIMENTAL {
rte_timer_alt_stop;
rte_timer_data_alloc;
rte_timer_data_dealloc;
+ rte_timer_next_ticks;
rte_timer_stop_all;
rte_timer_subsystem_finalize;
};
--
2.20.1
^ permalink raw reply related [flat|nested] 8+ messages in thread* Re: [dpdk-dev] [PATCH v1] rte_timer: add rte_timer_next_ticks
2019-12-17 0:55 ` [dpdk-dev] [PATCH v1] " Stephen Hemminger
@ 2019-12-20 22:43 ` Carrillo, Erik G
2020-01-20 0:38 ` Thomas Monjalon
0 siblings, 1 reply; 8+ messages in thread
From: Carrillo, Erik G @ 2019-12-20 22:43 UTC (permalink / raw)
To: Stephen Hemminger, dev
Hi Stephen,
I added a comment in-line. With that change, it looks good to me:
Acked-by: Erik Gabriel Carrillo <erik.g.carrillo@intel.com>
Regards,
Erik
> -----Original Message-----
> From: Stephen Hemminger <stephen@networkplumber.org>
> Sent: Monday, December 16, 2019 6:55 PM
> To: dev@dpdk.org
> Cc: Carrillo, Erik G <erik.g.carrillo@intel.com>; Stephen Hemminger
> <stephen@networkplumber.org>
> Subject: [PATCH v1] rte_timer: add rte_timer_next_ticks
>
> It is useful to know when the next timer will expire when using
> rte_epoll_wait (or sleep when idle). This experimental API provides a hook
> to query the number of ticks remaining.
>
> Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
> ---
> v1 - incorporate feedback from old RFC
>
> lib/librte_timer/rte_timer.c | 27 ++++++++++++++++++++++++++
> lib/librte_timer/rte_timer.h | 16 +++++++++++++++
> lib/librte_timer/rte_timer_version.map | 1 +
> 3 files changed, 44 insertions(+)
>
> diff --git a/lib/librte_timer/rte_timer.c b/lib/librte_timer/rte_timer.c index
> ca88454ff68a..a1ed186cf591 100644
> --- a/lib/librte_timer/rte_timer.c
> +++ b/lib/librte_timer/rte_timer.c
> @@ -978,6 +978,33 @@ rte_timer_stop_all(uint32_t timer_data_id,
> unsigned int *walk_lcores,
> return 0;
> }
>
> +int64_t
> +rte_timer_next_ticks(void)
> +{
> + unsigned int lcore_id = rte_lcore_id();
> + struct rte_timer_data *timer_data;
> + struct priv_timer *priv_timer;
> + const struct rte_timer *tm;
> + uint64_t cur_time;
> + int64_t left = -ENOENT;
> +
> + TIMER_DATA_VALID_GET_OR_ERR_RET(default_data_id,
> timer_data, -EINVAL);
> +
> + priv_timer = timer_data->priv_timer;
> + cur_time = rte_get_timer_cycles();
> +
> + rte_spinlock_lock(&priv_timer[lcore_id].list_lock);
> + tm = priv_timer[lcore_id].pending_head.sl_next[0];
> + if (tm) {
> + left = tm->expire - cur_time;
> + if (left < 0)
> + left = 0;
> + }
> + rte_spinlock_unlock(&priv_timer[lcore_id].list_lock);
> +
> + return left;
> +}
> +
> /* dump statistics about timers */
> static void
> __rte_timer_dump_stats(struct rte_timer_data *timer_data __rte_unused,
> FILE *f) diff --git a/lib/librte_timer/rte_timer.h b/lib/librte_timer/rte_timer.h
> index 9dc5fc309249..c5031d0448ba 100644
> --- a/lib/librte_timer/rte_timer.h
> +++ b/lib/librte_timer/rte_timer.h
> @@ -331,6 +331,22 @@ void rte_timer_stop_sync(struct rte_timer *tim);
> */
> int rte_timer_pending(struct rte_timer *tim);
>
> +/**
> + * @warning
> + * @b EXPERIMENTAL: this API may change without prior notice
> + *
> + * Time until the next timer
s/next timer/next timer on the current lcore
> + * This function gives the ticks until the next timer will be active.
> + *
> + * @return
> + * - -EINVAL: invalid timer data instance identifier
> + * - -ENOENT: no timer pending
> + * - 0: a timer is pending and will run at next rte_timer_manage()
> + * - >0: ticks until the next timer is ready
> + */
> +__rte_experimental
> +int64_t rte_timer_next_ticks(void);
> +
> /**
> * Manage the timer list and execute callback functions.
> *
> diff --git a/lib/librte_timer/rte_timer_version.map
> b/lib/librte_timer/rte_timer_version.map
> index 2a59d3f081c4..4471cef92be5 100644
> --- a/lib/librte_timer/rte_timer_version.map
> +++ b/lib/librte_timer/rte_timer_version.map
> @@ -23,6 +23,7 @@ EXPERIMENTAL {
> rte_timer_alt_stop;
> rte_timer_data_alloc;
> rte_timer_data_dealloc;
> + rte_timer_next_ticks;
> rte_timer_stop_all;
> rte_timer_subsystem_finalize;
> };
> --
> 2.20.1
^ permalink raw reply [flat|nested] 8+ messages in thread* Re: [dpdk-dev] [PATCH v1] rte_timer: add rte_timer_next_ticks
2019-12-20 22:43 ` Carrillo, Erik G
@ 2020-01-20 0:38 ` Thomas Monjalon
0 siblings, 0 replies; 8+ messages in thread
From: Thomas Monjalon @ 2020-01-20 0:38 UTC (permalink / raw)
To: Stephen Hemminger; +Cc: dev, Carrillo, Erik G
20/12/2019 23:43, Carrillo, Erik G:
> Hi Stephen,
>
> I added a comment in-line. With that change, it looks good to me:
>
> Acked-by: Erik Gabriel Carrillo <erik.g.carrillo@intel.com>
Applied with below change, thanks.
> From: Stephen Hemminger <stephen@networkplumber.org>
>
> > It is useful to know when the next timer will expire when using
> > rte_epoll_wait (or sleep when idle). This experimental API provides a hook
> > to query the number of ticks remaining.
> >
> > Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
> > ---
> > + * Time until the next timer
>
> s/next timer/next timer on the current lcore
^ permalink raw reply [flat|nested] 8+ messages in thread