[dpdk-dev] [RFC] rte_timer: add rte_timer_next_ticks

Carrillo, Erik G erik.g.carrillo at intel.com
Wed Jun 12 20:19:09 CEST 2019


Hi Stephen,

This looks like a useful addition.  Some comments in-line:

> -----Original Message-----
> From: Stephen Hemminger <stephen at networkplumber.org>
> Sent: Monday, June 10, 2019 5:45 PM
> To: rsanford at akamai.com; Carrillo, Erik G <erik.g.carrillo at intel.com>
> Cc: dev at dpdk.org; Stephen Hemminger <stephen at 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 at 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



More information about the dev mailing list