Hi Robert,
Thanks for the review and suggestions. I’m out of the office on bonding leave
for the next few weeks, but I’ll update the patch to address your points below
when I return.
Thanks,
Erik
> On Mar 20, 2019, at 8:53 AM, Sanford, Robert <rsanf...@akamai.com> wrote:
>
> Hi Erik,
>
> I have a few questions and comments on this patch series.
>
> 1. Don't you think we need new tests (in test/test/) to verify the
> secondary-process APIs?
> 2. I suggest we define default_data_id as const, and explicitly set it to 0.
> 3. The outer for-loop in rte_timer_alt_manage() touches beyond the end of
> poll_lcores[]. I suggest a change like this:
>
> - for (i = 0, poll_lcore = poll_lcores[i]; i < nb_poll_lcores;
> - poll_lcore = poll_lcores[++i]) {
> + for (i = 0; I < nb_poll_lcores; i++) {
> + poll_lcore = poll_lcores[i];
>
> 4. Same problem (as #3) in the for-loop in rte_timer_stop_all(), in patch v4
> 2/2.
> 5. There seems to be no difference between "typedef void
> (*rte_timer_cb_t)(struct rte_timer *, void *)" and "typedef void
> (*rte_timer_stop_all_cb_t)(struct rte_timer *tim, void *arg)", why add
> rte_timer_stop_all_cb_t?
> 6. Can you provide a use case or code snippet that shows how we will use
> rte_timer_alt_manage()?
> 7. Why not make the argument to rte_timer_alt_manage_cb_t a "struct rte_timer
> *", instead of a "void *", since we pass a pointer-to-timer when we invoke
> the function?
>
> --
> Regards,
> Robert Sanford
>
>
> On 3/6/19, 12:20 PM, "Erik Gabriel Carrillo" <erik.g.carri...@intel.com>
> wrote:
>
> Currently, the timer library uses a per-process table of structures to
> manage skiplists of timers presumably because timers contain arbitrary
> function pointers whose value may not resolve properly in other
> processes.
>
> However, if the same callback is used handle all timers, and that
> callback is only invoked in one process, then it woud be safe to allow
> the data structures to be allocated in shared memory, and to allow
> secondary processes to modify the timer lists. This would let timers be
> used in more multi-process scenarios.
>
> The library's global variables are wrapped with a struct, and an array
> of these structures is created in shared memory. The original APIs
> are updated to reference the zeroth entry in the array. This maintains
> the original behavior for both primary and secondary processes since
> the set intersection of their coremasks should be empty [1]. New APIs
> are introduced to enable the allocation/deallocation of other entries
> in the array.
>
> New variants of the APIs used to start and stop timers are introduced;
> they allow a caller to specify which array entry should be used to
> locate the timer list to insert into or delete from.
>
> Finally, a new variant of rte_timer_manage() is introduced, which
> allows a caller to specify which array entry should be used to locate
> the timer lists to process; it can also process multiple timer lists per
> invocation.
>
> [1]
> https://doc.dpdk.org/guides/prog_guide/multi_proc_support.html#multi-process-limitations
>
> Signed-off-by: Erik Gabriel Carrillo <erik.g.carri...@intel.com>
> ---
> lib/librte_timer/Makefile | 1 +
> lib/librte_timer/rte_timer.c | 519 ++++++++++++++++++++++++++++++---
> lib/librte_timer/rte_timer.h | 226 +++++++++++++-
> lib/librte_timer/rte_timer_version.map | 22 ++
> 4 files changed, 723 insertions(+), 45 deletions(-)
>
> diff --git a/lib/librte_timer/Makefile b/lib/librte_timer/Makefile
> index 4ebd528..8ec63f4 100644
> --- a/lib/librte_timer/Makefile
> +++ b/lib/librte_timer/Makefile
> @@ -6,6 +6,7 @@ include $(RTE_SDK)/mk/rte.vars.mk
> # library name
> LIB = librte_timer.a
>
> +CFLAGS += -DALLOW_EXPERIMENTAL_API
> CFLAGS += $(WERROR_FLAGS) -I$(SRCDIR) -O3
> LDLIBS += -lrte_eal
>
> diff --git a/lib/librte_timer/rte_timer.c b/lib/librte_timer/rte_timer.c
> index 30c7b0a..2bd49d0 100644
> --- a/lib/librte_timer/rte_timer.c
> +++ b/lib/librte_timer/rte_timer.c
> @@ -5,6 +5,7 @@
> #include <string.h>
> #include <stdio.h>
> #include <stdint.h>
> +#include <stdbool.h>
> #include <inttypes.h>
> #include <assert.h>
> #include <sys/queue.h>
> @@ -21,11 +22,15 @@
> #include <rte_spinlock.h>
> #include <rte_random.h>
> #include <rte_pause.h>
> +#include <rte_memzone.h>
> +#include <rte_malloc.h>
> +#include <rte_compat.h>
>
> #include "rte_timer.h"
>
> -LIST_HEAD(rte_timer_list, rte_timer);
> -
> +/**
> + * Per-lcore info for timers.
> + */
> struct priv_timer {
> struct rte_timer pending_head; /**< dummy timer instance to head up list
> */
> rte_spinlock_t list_lock; /**< lock to protect list access */
> @@ -48,25 +53,84 @@ struct priv_timer {
> #endif
> } __rte_cache_aligned;
>
> -/** per-lcore private info for timers */
> -static struct priv_timer priv_timer[RTE_MAX_LCORE];
> +#define FL_ALLOCATED (1 << 0)
> +struct rte_timer_data {
> + struct priv_timer priv_timer[RTE_MAX_LCORE];
> + uint8_t internal_flags;
> +};
> +
> +#define RTE_MAX_DATA_ELS 64
> +static struct rte_timer_data *rte_timer_data_arr;
> +static uint32_t default_data_id;
> +static uint32_t rte_timer_subsystem_initialized;
> +
> +/* For maintaining older interfaces for a period */
> +static struct rte_timer_data default_timer_data;
>
> /* when debug is enabled, store some statistics */
> #ifdef RTE_LIBRTE_TIMER_DEBUG
> -#define __TIMER_STAT_ADD(name, n) do { \
> +#define __TIMER_STAT_ADD(priv_timer, name, n) do { \
> unsigned __lcore_id = rte_lcore_id(); \
> if (__lcore_id < RTE_MAX_LCORE) \
> priv_timer[__lcore_id].stats.name += (n); \
> } while(0)
> #else
> -#define __TIMER_STAT_ADD(name, n) do {} while(0)
> +#define __TIMER_STAT_ADD(priv_timer, name, n) do {} while (0)
> #endif
>
> -/* Init the timer library. */
> +static inline int
> +timer_data_valid(uint32_t id)
> +{
> + return !!(rte_timer_data_arr[id].internal_flags & FL_ALLOCATED);
> +}
> +
> +/* validate ID and retrieve timer data pointer, or return error value */
> +#define TIMER_DATA_VALID_GET_OR_ERR_RET(id, timer_data, retval) do { \
> + if (id >= RTE_MAX_DATA_ELS || !timer_data_valid(id)) \
> + return retval; \
> + timer_data = &rte_timer_data_arr[id]; \
> +} while (0)
> +
> +int __rte_experimental
> +rte_timer_data_alloc(uint32_t *id_ptr)
> +{
> + int i;
> + struct rte_timer_data *data;
> +
> + if (!rte_timer_subsystem_initialized)
> + return -ENOMEM;
> +
> + for (i = 0; i < RTE_MAX_DATA_ELS; i++) {
> + data = &rte_timer_data_arr[i];
> + if (!(data->internal_flags & FL_ALLOCATED)) {
> + data->internal_flags |= FL_ALLOCATED;
> +
> + if (id_ptr)
> + *id_ptr = i;
> +
> + return 0;
> + }
> + }
> +
> + return -ENOSPC;
> +}
> +
> +int __rte_experimental
> +rte_timer_data_dealloc(uint32_t id)
> +{
> + struct rte_timer_data *timer_data;
> + TIMER_DATA_VALID_GET_OR_ERR_RET(id, timer_data, -EINVAL);
> +
> + timer_data->internal_flags &= ~(FL_ALLOCATED);
> +
> + return 0;
> +}
> +
> void
> -rte_timer_subsystem_init(void)
> +rte_timer_subsystem_init_v20(void)
> {
> unsigned lcore_id;
> + struct priv_timer *priv_timer = default_timer_data.priv_timer;
>
> /* since priv_timer is static, it's zeroed by default, so only init some
> * fields.
> @@ -76,6 +140,76 @@ rte_timer_subsystem_init(void)
> priv_timer[lcore_id].prev_lcore = lcore_id;
> }
> }
> +VERSION_SYMBOL(rte_timer_subsystem_init, _v20, 2.0);
> +
> +/* Init the timer library. Allocate an array of timer data structs in shared
> + * memory, and allocate the zeroth entry for use with original timer
> + * APIs. Since the intersection of the sets of lcore ids in primary and
> + * secondary processes should be empty, the zeroth entry can be shared by
> + * multiple processes.
> + */
> +int
> +rte_timer_subsystem_init_v1905(void)
> +{
> + const struct rte_memzone *mz;
> + struct rte_timer_data *data;
> + int i, lcore_id;
> + static const char *mz_name = "rte_timer_mz";
> +
> + if (rte_timer_subsystem_initialized)
> + return -EALREADY;
> +
> + if (rte_eal_process_type() != RTE_PROC_PRIMARY) {
> + mz = rte_memzone_lookup(mz_name);
> + if (mz == NULL)
> + return -EEXIST;
> +
> + rte_timer_data_arr = mz->addr;
> +
> + rte_timer_data_arr[default_data_id].internal_flags |=
> + FL_ALLOCATED;
> +
> + rte_timer_subsystem_initialized = 1;
> +
> + return 0;
> + }
> +
> + mz = rte_memzone_reserve_aligned(mz_name,
> + RTE_MAX_DATA_ELS * sizeof(*rte_timer_data_arr),
> + SOCKET_ID_ANY, 0, RTE_CACHE_LINE_SIZE);
> + if (mz == NULL)
> + return -ENOMEM;
> +
> + rte_timer_data_arr = mz->addr;
> +
> + for (i = 0; i < RTE_MAX_DATA_ELS; i++) {
> + data = &rte_timer_data_arr[i];
> +
> + for (lcore_id = 0; lcore_id < RTE_MAX_LCORE; lcore_id++) {
> + rte_spinlock_init(
> + &data->priv_timer[lcore_id].list_lock);
> + data->priv_timer[lcore_id].prev_lcore = lcore_id;
> + }
> + }
> +
> + rte_timer_data_arr[default_data_id].internal_flags |= FL_ALLOCATED;
> +
> + rte_timer_subsystem_initialized = 1;
> +
> + return 0;
> +}
> +MAP_STATIC_SYMBOL(int rte_timer_subsystem_init(void),
> + rte_timer_subsystem_init_v1905);
> +BIND_DEFAULT_SYMBOL(rte_timer_subsystem_init, _v1905, 19.05);
> +
> +void __rte_experimental
> +rte_timer_subsystem_finalize(void)
> +{
> + if (rte_timer_data_arr)
> + rte_free(rte_timer_data_arr);
> +
> + rte_timer_subsystem_initialized = 0;
> +}
>
> /* Initialize the timer handle tim for use */
> void
> @@ -95,7 +229,8 @@ rte_timer_init(struct rte_timer *tim)
> */
> static int
> timer_set_config_state(struct rte_timer *tim,
> - union rte_timer_status *ret_prev_status)
> + union rte_timer_status *ret_prev_status,
> + struct priv_timer *priv_timer)
> {
> union rte_timer_status prev_status, status;
> int success = 0;
> @@ -207,7 +342,7 @@ timer_get_skiplist_level(unsigned curr_depth)
> */
> static void
> timer_get_prev_entries(uint64_t time_val, unsigned tim_lcore,
> - struct rte_timer **prev)
> + struct rte_timer **prev, struct priv_timer *priv_timer)
> {
> unsigned lvl = priv_timer[tim_lcore].curr_skiplist_depth;
> prev[lvl] = &priv_timer[tim_lcore].pending_head;
> @@ -226,13 +361,15 @@ timer_get_prev_entries(uint64_t time_val, unsigned
> tim_lcore,
> */
> static void
> timer_get_prev_entries_for_node(struct rte_timer *tim, unsigned tim_lcore,
> - struct rte_timer **prev)
> + struct rte_timer **prev,
> + struct priv_timer *priv_timer)
> {
> int i;
> +
> /* to get a specific entry in the list, look for just lower than the time
> * values, and then increment on each level individually if necessary
> */
> - timer_get_prev_entries(tim->expire - 1, tim_lcore, prev);
> + timer_get_prev_entries(tim->expire - 1, tim_lcore, prev, priv_timer);
> for (i = priv_timer[tim_lcore].curr_skiplist_depth - 1; i >= 0; i--) {
> while (prev[i]->sl_next[i] != NULL &&
> prev[i]->sl_next[i] != tim &&
> @@ -247,14 +384,15 @@ timer_get_prev_entries_for_node(struct rte_timer *tim,
> unsigned tim_lcore,
> * timer must not be in a list
> */
> static void
> -timer_add(struct rte_timer *tim, unsigned int tim_lcore)
> +timer_add(struct rte_timer *tim, unsigned int tim_lcore,
> + struct priv_timer *priv_timer)
> {
> unsigned lvl;
> struct rte_timer *prev[MAX_SKIPLIST_DEPTH+1];
>
> /* find where exactly this element goes in the list of elements
> * for each depth. */
> - timer_get_prev_entries(tim->expire, tim_lcore, prev);
> + timer_get_prev_entries(tim->expire, tim_lcore, prev, priv_timer);
>
> /* now assign it a new level and add at that level */
> const unsigned tim_level = timer_get_skiplist_level(
> @@ -284,7 +422,7 @@ timer_add(struct rte_timer *tim, unsigned int tim_lcore)
> */
> static void
> timer_del(struct rte_timer *tim, union rte_timer_status prev_status,
> - int local_is_locked)
> + int local_is_locked, struct priv_timer *priv_timer)
> {
> unsigned lcore_id = rte_lcore_id();
> unsigned prev_owner = prev_status.owner;
> @@ -304,7 +442,7 @@ timer_del(struct rte_timer *tim, union rte_timer_status
> prev_status,
> ((tim->sl_next[0] == NULL) ? 0 : tim->sl_next[0]->expire);
>
> /* adjust pointers from previous entries to point past this */
> - timer_get_prev_entries_for_node(tim, prev_owner, prev);
> + timer_get_prev_entries_for_node(tim, prev_owner, prev, priv_timer);
> for (i = priv_timer[prev_owner].curr_skiplist_depth - 1; i >= 0; i--) {
> if (prev[i]->sl_next[i] == tim)
> prev[i]->sl_next[i] = tim->sl_next[i];
> @@ -326,11 +464,13 @@ static int
> __rte_timer_reset(struct rte_timer *tim, uint64_t expire,
> uint64_t period, unsigned tim_lcore,
> rte_timer_cb_t fct, void *arg,
> - int local_is_locked)
> + int local_is_locked,
> + struct rte_timer_data *timer_data)
> {
> union rte_timer_status prev_status, status;
> int ret;
> unsigned lcore_id = rte_lcore_id();
> + struct priv_timer *priv_timer = timer_data->priv_timer;
>
> /* round robin for tim_lcore */
> if (tim_lcore == (unsigned)LCORE_ID_ANY) {
> @@ -348,11 +488,11 @@ __rte_timer_reset(struct rte_timer *tim, uint64_t
> expire,
>
> /* wait that the timer is in correct status before update,
> * and mark it as being configured */
> - ret = timer_set_config_state(tim, &prev_status);
> + ret = timer_set_config_state(tim, &prev_status, priv_timer);
> if (ret < 0)
> return -1;
>
> - __TIMER_STAT_ADD(reset, 1);
> + __TIMER_STAT_ADD(priv_timer, reset, 1);
> if (prev_status.state == RTE_TIMER_RUNNING &&
> lcore_id < RTE_MAX_LCORE) {
> priv_timer[lcore_id].updated = 1;
> @@ -360,8 +500,8 @@ __rte_timer_reset(struct rte_timer *tim, uint64_t expire,
>
> /* remove it from list */
> if (prev_status.state == RTE_TIMER_PENDING) {
> - timer_del(tim, prev_status, local_is_locked);
> - __TIMER_STAT_ADD(pending, -1);
> + timer_del(tim, prev_status, local_is_locked, priv_timer);
> + __TIMER_STAT_ADD(priv_timer, pending, -1);
> }
>
> tim->period = period;
> @@ -376,8 +516,8 @@ __rte_timer_reset(struct rte_timer *tim, uint64_t expire,
> if (tim_lcore != lcore_id || !local_is_locked)
> rte_spinlock_lock(&priv_timer[tim_lcore].list_lock);
>
> - __TIMER_STAT_ADD(pending, 1);
> - timer_add(tim, tim_lcore);
> + __TIMER_STAT_ADD(priv_timer, pending, 1);
> + timer_add(tim, tim_lcore, priv_timer);
>
> /* update state: as we are in CONFIG state, only us can modify
> * the state so we don't need to use cmpset() here */
> @@ -394,9 +534,9 @@ __rte_timer_reset(struct rte_timer *tim, uint64_t expire,
>
> /* Reset and start the timer associated with the timer handle tim */
> int
> -rte_timer_reset(struct rte_timer *tim, uint64_t ticks,
> - enum rte_timer_type type, unsigned tim_lcore,
> - rte_timer_cb_t fct, void *arg)
> +rte_timer_reset_v20(struct rte_timer *tim, uint64_t ticks,
> + enum rte_timer_type type, unsigned int tim_lcore,
> + rte_timer_cb_t fct, void *arg)
> {
> uint64_t cur_time = rte_get_timer_cycles();
> uint64_t period;
> @@ -412,7 +552,48 @@ rte_timer_reset(struct rte_timer *tim, uint64_t ticks,
> period = 0;
>
> return __rte_timer_reset(tim, cur_time + ticks, period, tim_lcore,
> - fct, arg, 0);
> + fct, arg, 0, &default_timer_data);
> +}
> +VERSION_SYMBOL(rte_timer_reset, _v20, 2.0);
> +
> +int
> +rte_timer_reset_v1905(struct rte_timer *tim, uint64_t ticks,
> + enum rte_timer_type type, unsigned int tim_lcore,
> + rte_timer_cb_t fct, void *arg)
> +{
> + return rte_timer_alt_reset(default_data_id, tim, ticks, type,
> + tim_lcore, fct, arg);
> +}
> +MAP_STATIC_SYMBOL(int rte_timer_reset(struct rte_timer *tim, uint64_t ticks,
> + enum rte_timer_type type,
> + unsigned int tim_lcore,
> + rte_timer_cb_t fct, void *arg),
> + rte_timer_reset_v1905);
> +BIND_DEFAULT_SYMBOL(rte_timer_reset, _v1905, 19.05);
> +
> +int __rte_experimental
> +rte_timer_alt_reset(uint32_t timer_data_id, struct rte_timer *tim,
> + uint64_t ticks, enum rte_timer_type type,
> + unsigned int tim_lcore, rte_timer_cb_t fct, void *arg)
> +{
> + uint64_t cur_time = rte_get_timer_cycles();
> + uint64_t period;
> + struct rte_timer_data *timer_data;
> +
> + TIMER_DATA_VALID_GET_OR_ERR_RET(timer_data_id, timer_data, -EINVAL);
> +
> + if (unlikely((tim_lcore != (unsigned int)LCORE_ID_ANY) &&
> + !(rte_lcore_is_enabled(tim_lcore) ||
> + rte_lcore_has_role(tim_lcore, ROLE_SERVICE))))
> + return -1;
> +
> + if (type == PERIODICAL)
> + period = ticks;
> + else
> + period = 0;
> +
> + return __rte_timer_reset(tim, cur_time + ticks, period, tim_lcore,
> + fct, arg, 0, timer_data);
> }
>
> /* loop until rte_timer_reset() succeed */
> @@ -426,21 +607,22 @@ rte_timer_reset_sync(struct rte_timer *tim, uint64_t
> ticks,
> rte_pause();
> }
>
> -/* Stop the timer associated with the timer handle tim */
> -int
> -rte_timer_stop(struct rte_timer *tim)
> +static int
> +__rte_timer_stop(struct rte_timer *tim, int local_is_locked,
> + struct rte_timer_data *timer_data)
> {
> union rte_timer_status prev_status, status;
> unsigned lcore_id = rte_lcore_id();
> int ret;
> + struct priv_timer *priv_timer = timer_data->priv_timer;
>
> /* wait that the timer is in correct status before update,
> * and mark it as being configured */
> - ret = timer_set_config_state(tim, &prev_status);
> + ret = timer_set_config_state(tim, &prev_status, priv_timer);
> if (ret < 0)
> return -1;
>
> - __TIMER_STAT_ADD(stop, 1);
> + __TIMER_STAT_ADD(priv_timer, stop, 1);
> if (prev_status.state == RTE_TIMER_RUNNING &&
> lcore_id < RTE_MAX_LCORE) {
> priv_timer[lcore_id].updated = 1;
> @@ -448,8 +630,8 @@ rte_timer_stop(struct rte_timer *tim)
>
> /* remove it from list */
> if (prev_status.state == RTE_TIMER_PENDING) {
> - timer_del(tim, prev_status, 0);
> - __TIMER_STAT_ADD(pending, -1);
> + timer_del(tim, prev_status, local_is_locked, priv_timer);
> + __TIMER_STAT_ADD(priv_timer, pending, -1);
> }
>
> /* mark timer as stopped */
> @@ -461,6 +643,33 @@ rte_timer_stop(struct rte_timer *tim)
> return 0;
> }
>
> +/* Stop the timer associated with the timer handle tim */
> +int
> +rte_timer_stop_v20(struct rte_timer *tim)
> +{
> + return __rte_timer_stop(tim, 0, &default_timer_data);
> +}
> +VERSION_SYMBOL(rte_timer_stop, _v20, 2.0);
> +
> +int
> +rte_timer_stop_v1905(struct rte_timer *tim)
> +{
> + return rte_timer_alt_stop(default_data_id, tim);
> +}
> +MAP_STATIC_SYMBOL(int rte_timer_stop(struct rte_timer *tim),
> + rte_timer_stop_v1905);
> +BIND_DEFAULT_SYMBOL(rte_timer_stop, _v1905, 19.05);
> +
> +int __rte_experimental
> +rte_timer_alt_stop(uint32_t timer_data_id, struct rte_timer *tim)
> +{
> + struct rte_timer_data *timer_data;
> +
> + TIMER_DATA_VALID_GET_OR_ERR_RET(timer_data_id, timer_data, -EINVAL);
> +
> + return __rte_timer_stop(tim, 0, timer_data);
> +}
> +
> /* loop until rte_timer_stop() succeed */
> void
> rte_timer_stop_sync(struct rte_timer *tim)
> @@ -477,7 +686,8 @@ rte_timer_pending(struct rte_timer *tim)
> }
>
> /* must be called periodically, run all timer that expired */
> -void rte_timer_manage(void)
> +static void
> +__rte_timer_manage(struct rte_timer_data *timer_data)
> {
> union rte_timer_status status;
> struct rte_timer *tim, *next_tim;
> @@ -486,11 +696,12 @@ void rte_timer_manage(void)
> struct rte_timer *prev[MAX_SKIPLIST_DEPTH + 1];
> uint64_t cur_time;
> int i, ret;
> + struct priv_timer *priv_timer = timer_data->priv_timer;
>
> /* timer manager only runs on EAL thread with valid lcore_id */
> assert(lcore_id < RTE_MAX_LCORE);
>
> - __TIMER_STAT_ADD(manage, 1);
> + __TIMER_STAT_ADD(priv_timer, manage, 1);
> /* optimize for the case where per-cpu list is empty */
> if (priv_timer[lcore_id].pending_head.sl_next[0] == NULL)
> return;
> @@ -518,7 +729,7 @@ void rte_timer_manage(void)
> tim = priv_timer[lcore_id].pending_head.sl_next[0];
>
> /* break the existing list at current time point */
> - timer_get_prev_entries(cur_time, lcore_id, prev);
> + timer_get_prev_entries(cur_time, lcore_id, prev, priv_timer);
> for (i = priv_timer[lcore_id].curr_skiplist_depth -1; i >= 0; i--) {
> if (prev[i] == &priv_timer[lcore_id].pending_head)
> continue;
> @@ -563,7 +774,7 @@ void rte_timer_manage(void)
> /* execute callback function with list unlocked */
> tim->f(tim, tim->arg);
>
> - __TIMER_STAT_ADD(pending, -1);
> + __TIMER_STAT_ADD(priv_timer, pending, -1);
> /* the timer was stopped or reloaded by the callback
> * function, we have nothing to do here */
> if (priv_timer[lcore_id].updated == 1)
> @@ -580,24 +791,222 @@ void rte_timer_manage(void)
> /* keep it in list and mark timer as pending */
> rte_spinlock_lock(&priv_timer[lcore_id].list_lock);
> status.state = RTE_TIMER_PENDING;
> - __TIMER_STAT_ADD(pending, 1);
> + __TIMER_STAT_ADD(priv_timer, pending, 1);
> status.owner = (int16_t)lcore_id;
> rte_wmb();
> tim->status.u32 = status.u32;
> __rte_timer_reset(tim, tim->expire + tim->period,
> - tim->period, lcore_id, tim->f, tim->arg, 1);
> + tim->period, lcore_id, tim->f, tim->arg, 1,
> + timer_data);
> rte_spinlock_unlock(&priv_timer[lcore_id].list_lock);
> }
> }
> priv_timer[lcore_id].running_tim = NULL;
> }
>
> +void
> +rte_timer_manage_v20(void)
> +{
> + __rte_timer_manage(&default_timer_data);
> +}
> +VERSION_SYMBOL(rte_timer_manage, _v20, 2.0);
> +
> +int
> +rte_timer_manage_v1905(void)
> +{
> + struct rte_timer_data *timer_data;
> +
> + TIMER_DATA_VALID_GET_OR_ERR_RET(default_data_id, timer_data, -EINVAL);
> +
> + __rte_timer_manage(timer_data);
> +
> + return 0;
> +}
> +MAP_STATIC_SYMBOL(int rte_timer_manage(void), rte_timer_manage_v1905);
> +BIND_DEFAULT_SYMBOL(rte_timer_manage, _v1905, 19.05);
> +
> +int __rte_experimental
> +rte_timer_alt_manage(uint32_t timer_data_id,
> + unsigned int *poll_lcores,
> + int nb_poll_lcores,
> + rte_timer_alt_manage_cb_t f)
> +{
> + union rte_timer_status status;
> + struct rte_timer *tim, *next_tim, **pprev;
> + struct rte_timer *run_first_tims[RTE_MAX_LCORE];
> + unsigned int runlist_lcore_ids[RTE_MAX_LCORE];
> + unsigned int this_lcore = rte_lcore_id();
> + struct rte_timer *prev[MAX_SKIPLIST_DEPTH + 1];
> + uint64_t cur_time;
> + int i, j, ret;
> + int nb_runlists = 0;
> + struct rte_timer_data *data;
> + struct priv_timer *privp;
> + uint32_t poll_lcore;
> +
> + TIMER_DATA_VALID_GET_OR_ERR_RET(timer_data_id, data, -EINVAL);
> +
> + /* timer manager only runs on EAL thread with valid lcore_id */
> + assert(this_lcore < RTE_MAX_LCORE);
> +
> + __TIMER_STAT_ADD(data->priv_timer, manage, 1);
> +
> + if (poll_lcores == NULL) {
> + poll_lcores = (unsigned int []){rte_lcore_id()};
> + nb_poll_lcores = 1;
> + }
> +
> + for (i = 0, poll_lcore = poll_lcores[i]; i < nb_poll_lcores;
> + poll_lcore = poll_lcores[++i]) {
> + privp = &data->priv_timer[poll_lcore];
> +
> + /* optimize for the case where per-cpu list is empty */
> + if (privp->pending_head.sl_next[0] == NULL)
> + continue;
> + cur_time = rte_get_timer_cycles();
> +
> +#ifdef RTE_ARCH_64
> + /* on 64-bit the value cached in the pending_head.expired will
> + * be updated atomically, so we can consult that for a quick
> + * check here outside the lock
> + */
> + if (likely(privp->pending_head.expire > cur_time))
> + continue;
> +#endif
> +
> + /* browse ordered list, add expired timers in 'expired' list */
> + rte_spinlock_lock(&privp->list_lock);
> +
> + /* if nothing to do just unlock and return */
> + if (privp->pending_head.sl_next[0] == NULL ||
> + privp->pending_head.sl_next[0]->expire > cur_time) {
> + rte_spinlock_unlock(&privp->list_lock);
> + continue;
> + }
> +
> + /* save start of list of expired timers */
> + tim = privp->pending_head.sl_next[0];
> +
> + /* break the existing list at current time point */
> + timer_get_prev_entries(cur_time, poll_lcore, prev,
> + data->priv_timer);
> + for (j = privp->curr_skiplist_depth - 1; j >= 0; j--) {
> + if (prev[j] == &privp->pending_head)
> + continue;
> + privp->pending_head.sl_next[j] =
> + prev[j]->sl_next[j];
> + if (prev[j]->sl_next[j] == NULL)
> + privp->curr_skiplist_depth--;
> +
> + prev[j]->sl_next[j] = NULL;
> + }
> +
> + /* transition run-list from PENDING to RUNNING */
> + run_first_tims[nb_runlists] = tim;
> + runlist_lcore_ids[nb_runlists] = poll_lcore;
> + pprev = &run_first_tims[nb_runlists];
> + nb_runlists++;
> +
> + for ( ; tim != NULL; tim = next_tim) {
> + next_tim = tim->sl_next[0];
> +
> + ret = timer_set_running_state(tim);
> + if (likely(ret == 0)) {
> + pprev = &tim->sl_next[0];
> + } else {
> + /* another core is trying to re-config this one,
> + * remove it from local expired list
> + */
> + *pprev = next_tim;
> + }
> + }
> +
> + /* update the next to expire timer value */
> + privp->pending_head.expire =
> + (privp->pending_head.sl_next[0] == NULL) ? 0 :
> + privp->pending_head.sl_next[0]->expire;
> +
> + rte_spinlock_unlock(&privp->list_lock);
> + }
> +
> + /* Now process the run lists */
> + while (1) {
> + bool done = true;
> + uint64_t min_expire = UINT64_MAX;
> + int min_idx = 0;
> +
> + /* Find the next oldest timer to process */
> + for (i = 0; i < nb_runlists; i++) {
> + tim = run_first_tims[i];
> +
> + if (tim != NULL && tim->expire < min_expire) {
> + min_expire = tim->expire;
> + min_idx = i;
> + done = false;
> + }
> + }
> +
> + if (done)
> + break;
> +
> + tim = run_first_tims[min_idx];
> + privp = &data->priv_timer[runlist_lcore_ids[min_idx]];
> +
> + /* Move down the runlist from which we picked a timer to
> + * execute
> + */
> + run_first_tims[min_idx] = run_first_tims[min_idx]->sl_next[0];
> +
> + privp->updated = 0;
> + privp->running_tim = tim;
> +
> + /* Call the provided callback function */
> + f(tim);
> +
> + __TIMER_STAT_ADD(privp, pending, -1);
> +
> + /* the timer was stopped or reloaded by the callback
> + * function, we have nothing to do here
> + */
> + if (privp->updated == 1)
> + continue;
> +
> + if (tim->period == 0) {
> + /* remove from done list and mark timer as stopped */
> + status.state = RTE_TIMER_STOP;
> + status.owner = RTE_TIMER_NO_OWNER;
> + rte_wmb();
> + tim->status.u32 = status.u32;
> + } else {
> + /* keep it in list and mark timer as pending */
> + rte_spinlock_lock(
> + &data->priv_timer[this_lcore].list_lock);
> + status.state = RTE_TIMER_PENDING;
> + __TIMER_STAT_ADD(data->priv_timer, pending, 1);
> + status.owner = (int16_t)this_lcore;
> + rte_wmb();
> + tim->status.u32 = status.u32;
> + __rte_timer_reset(tim, tim->expire + tim->period,
> + tim->period, this_lcore, tim->f, tim->arg, 1,
> + data);
> + rte_spinlock_unlock(
> + &data->priv_timer[this_lcore].list_lock);
> + }
> +
> + privp->running_tim = NULL;
> + }
> +
> + return 0;
> +}
> +
> /* dump statistics about timers */
> -void rte_timer_dump_stats(FILE *f)
> +static void
> +__rte_timer_dump_stats(struct rte_timer_data *timer_data __rte_unused, FILE
> *f)
> {
> #ifdef RTE_LIBRTE_TIMER_DEBUG
> struct rte_timer_debug_stats sum;
> unsigned lcore_id;
> + struct priv_timer *priv_timer = timer_data->priv_timer;
>
> memset(&sum, 0, sizeof(sum));
> for (lcore_id = 0; lcore_id < RTE_MAX_LCORE; lcore_id++) {
> @@ -615,3 +1024,31 @@ void rte_timer_dump_stats(FILE *f)
> fprintf(f, "No timer statistics, RTE_LIBRTE_TIMER_DEBUG is disabled\n");
> #endif
> }
> +
> +void
> +rte_timer_dump_stats_v20(FILE *f)
> +{
> + __rte_timer_dump_stats(&default_timer_data, f);
> +}
> +VERSION_SYMBOL(rte_timer_dump_stats, _v20, 2.0);
> +
> +int
> +rte_timer_dump_stats_v1905(FILE *f)
> +{
> + return rte_timer_alt_dump_stats(default_data_id, f);
> +}
> +MAP_STATIC_SYMBOL(int rte_timer_dump_stats(FILE *f),
> + rte_timer_dump_stats_v1905);
> +BIND_DEFAULT_SYMBOL(rte_timer_dump_stats, _v1905, 19.05);
> +
> +int __rte_experimental
> +rte_timer_alt_dump_stats(uint32_t timer_data_id __rte_unused, FILE *f)
> +{
> + struct rte_timer_data *timer_data;
> +
> + TIMER_DATA_VALID_GET_OR_ERR_RET(timer_data_id, timer_data, -EINVAL);
> +
> + __rte_timer_dump_stats(timer_data, f);
> +
> + return 0;
> +}
> diff --git a/lib/librte_timer/rte_timer.h b/lib/librte_timer/rte_timer.h
> index 9b95cd2..bee1676 100644
> --- a/lib/librte_timer/rte_timer.h
> +++ b/lib/librte_timer/rte_timer.h
> @@ -39,6 +39,7 @@
> #include <stddef.h>
> #include <rte_common.h>
> #include <rte_config.h>
> +#include <rte_spinlock.h>
>
> #ifdef __cplusplus
> extern "C" {
> @@ -132,12 +133,68 @@ struct rte_timer
> #endif
>
> /**
> + * @warning
> + * @b EXPERIMENTAL: this API may change without prior notice
> + *
> + * Allocate a timer data instance in shared memory to track a set of pending
> + * timer lists.
> + *
> + * @param id_ptr
> + * Pointer to variable into which to write the identifier of the allocated
> + * timer data instance.
> + *
> + * @return
> + * - 0: Success
> + * - -ENOSPC: maximum number of timer data instances already allocated
> + */
> +int __rte_experimental rte_timer_data_alloc(uint32_t *id_ptr);
> +
> +/**
> + * @warning
> + * @b EXPERIMENTAL: this API may change without prior notice
> + *
> + * Deallocate a timer data instance.
> + *
> + * @param id
> + * Identifier of the timer data instance to deallocate.
> + *
> + * @return
> + * - 0: Success
> + * - -EINVAL: invalid timer data instance identifier
> + */
> +int __rte_experimental rte_timer_data_dealloc(uint32_t id);
> +
> +/**
> * Initialize the timer library.
> *
> * Initializes internal variables (list, locks and so on) for the RTE
> * timer library.
> */
> -void rte_timer_subsystem_init(void);
> +void rte_timer_subsystem_init_v20(void);
> +
> +/**
> + * Initialize the timer library.
> + *
> + * Initializes internal variables (list, locks and so on) for the RTE
> + * timer library.
> + *
> + * @return
> + * - 0: Success
> + * - -EEXIST: Returned in secondary process when primary process has not
> + * yet initialized the timer subsystem
> + * - -ENOMEM: Unable to allocate memory needed to initialize timer
> + * subsystem
> + */
> +int rte_timer_subsystem_init_v1905(void);
> +int rte_timer_subsystem_init(void);
> +
> +/**
> + * @warning
> + * @b EXPERIMENTAL: this API may change without prior notice
> + *
> + * Free timer subsystem resources.
> + */
> +void __rte_experimental rte_timer_subsystem_finalize(void);
>
> /**
> * Initialize a timer handle.
> @@ -193,6 +250,12 @@ void rte_timer_init(struct rte_timer *tim);
> * - 0: Success; the timer is scheduled.
> * - (-1): Timer is in the RUNNING or CONFIG state.
> */
> +int rte_timer_reset_v20(struct rte_timer *tim, uint64_t ticks,
> + enum rte_timer_type type, unsigned int tim_lcore,
> + rte_timer_cb_t fct, void *arg);
> +int rte_timer_reset_v1905(struct rte_timer *tim, uint64_t ticks,
> + enum rte_timer_type type, unsigned int tim_lcore,
> + rte_timer_cb_t fct, void *arg);
> int rte_timer_reset(struct rte_timer *tim, uint64_t ticks,
> enum rte_timer_type type, unsigned tim_lcore,
> rte_timer_cb_t fct, void *arg);
> @@ -252,9 +315,10 @@ rte_timer_reset_sync(struct rte_timer *tim, uint64_t
> ticks,
> * - 0: Success; the timer is stopped.
> * - (-1): The timer is in the RUNNING or CONFIG state.
> */
> +int rte_timer_stop_v20(struct rte_timer *tim);
> +int rte_timer_stop_v1905(struct rte_timer *tim);
> int rte_timer_stop(struct rte_timer *tim);
>
> -
> /**
> * Loop until rte_timer_stop() succeeds.
> *
> @@ -292,7 +356,25 @@ int rte_timer_pending(struct rte_timer *tim);
> * function. However, the more often the function is called, the more
> * CPU resources it will use.
> */
> -void rte_timer_manage(void);
> +void rte_timer_manage_v20(void);
> +
> +/**
> + * Manage the timer list and execute callback functions.
> + *
> + * This function must be called periodically from EAL lcores
> + * main_loop(). It browses the list of pending timers and runs all
> + * timers that are expired.
> + *
> + * The precision of the timer depends on the call frequency of this
> + * function. However, the more often the function is called, the more
> + * CPU resources it will use.
> + *
> + * @return
> + * - 0: Success
> + * - -EINVAL: timer subsystem not yet initialized
> + */
> +int rte_timer_manage_v1905(void);
> +int rte_timer_manage(void);
>
> /**
> * Dump statistics about timers.
> @@ -300,7 +382,143 @@ void rte_timer_manage(void);
> * @param f
> * A pointer to a file for output
> */
> -void rte_timer_dump_stats(FILE *f);
> +void rte_timer_dump_stats_v20(FILE *f);
> +
> +/**
> + * Dump statistics about timers.
> + *
> + * @param f
> + * A pointer to a file for output
> + * @return
> + * - 0: Success
> + * - -EINVAL: timer subsystem not yet initialized
> + */
> +int rte_timer_dump_stats_v1905(FILE *f);
> +int rte_timer_dump_stats(FILE *f);
> +
> +/**
> + * @warning
> + * @b EXPERIMENTAL: this API may change without prior notice
> + *
> + * This function is the same as rte_timer_reset(), except that it allows a
> + * caller to specify the rte_timer_data instance containing the list to which
> + * the timer should be added.
> + *
> + * @see rte_timer_reset()
> + *
> + * @param timer_data_id
> + * An identifier indicating which instance of timer data should be used for
> + * this operation.
> + * @param tim
> + * The timer handle.
> + * @param ticks
> + * The number of cycles (see rte_get_hpet_hz()) before the callback
> + * function is called.
> + * @param type
> + * The type can be either:
> + * - PERIODICAL: The timer is automatically reloaded after execution
> + * (returns to the PENDING state)
> + * - SINGLE: The timer is one-shot, that is, the timer goes to a
> + * STOPPED state after execution.
> + * @param tim_lcore
> + * The ID of the lcore where the timer callback function has to be
> + * executed. If tim_lcore is LCORE_ID_ANY, the timer library will
> + * launch it on a different core for each call (round-robin).
> + * @param fct
> + * The callback function of the timer. This parameter can be NULL if (and
> + * only if) rte_timer_alt_manage() will be used to manage this timer.
> + * @param arg
> + * The user argument of the callback function.
> + * @return
> + * - 0: Success; the timer is scheduled.
> + * - (-1): Timer is in the RUNNING or CONFIG state.
> + * - -EINVAL: invalid timer_data_id
> + */
> +int __rte_experimental
> +rte_timer_alt_reset(uint32_t timer_data_id, struct rte_timer *tim,
> + uint64_t ticks, enum rte_timer_type type,
> + unsigned int tim_lcore, rte_timer_cb_t fct, void *arg);
> +
> +/**
> + * @warning
> + * @b EXPERIMENTAL: this API may change without prior notice
> + *
> + * This function is the same as rte_timer_stop(), except that it allows a
> + * caller to specify the rte_timer_data instance containing the list from
> which
> + * this timer should be removed.
> + *
> + * @see rte_timer_stop()
> + *
> + * @param timer_data_id
> + * An identifier indicating which instance of timer data should be used for
> + * this operation.
> + * @param tim
> + * The timer handle.
> + * @return
> + * - 0: Success; the timer is stopped.
> + * - (-1): The timer is in the RUNNING or CONFIG state.
> + * - -EINVAL: invalid timer_data_id
> + */
> +int __rte_experimental
> +rte_timer_alt_stop(uint32_t timer_data_id, struct rte_timer *tim);
> +
> +/**
> + * Callback function type for rte_timer_alt_manage().
> + */
> +typedef void (*rte_timer_alt_manage_cb_t)(void *);
> +
> +/**
> + * @warning
> + * @b EXPERIMENTAL: this API may change without prior notice
> + *
> + * Manage a set of timer lists and execute the specified callback function
> for
> + * all expired timers. This function is similar to rte_timer_manage(), except
> + * that it allows a caller to specify the timer_data instance that should
> + * be operated on, as well as a set of lcore IDs identifying which timer
> lists
> + * should be processed. Callback functions of individual timers are ignored.
> + *
> + * @see rte_timer_manage()
> + *
> + * @param timer_data_id
> + * An identifier indicating which instance of timer data should be used for
> + * this operation.
> + * @param poll_lcores
> + * An array of lcore ids identifying the timer lists that should be
> processed.
> + * NULL is allowed - if NULL, the timer list corresponding to the lcore
> + * calling this routine is processed (same as rte_timer_manage()).
> + * @param n_poll_lcores
> + * The size of the poll_lcores array. If 'poll_lcores' is NULL, this
> parameter
> + * is ignored.
> + * @param f
> + * The callback function which should be called for all expired timers.
> + * @return
> + * - 0: success
> + * - -EINVAL: invalid timer_data_id
> + */
> +int __rte_experimental
> +rte_timer_alt_manage(uint32_t timer_data_id, unsigned int *poll_lcores,
> + int n_poll_lcores, rte_timer_alt_manage_cb_t f);
> +
> +/**
> + * @warning
> + * @b EXPERIMENTAL: this API may change without prior notice
> + *
> + * This function is the same as rte_timer_dump_stats(), except that it allows
> + * the caller to specify the rte_timer_data instance that should be used.
> + *
> + * @see rte_timer_dump_stats()
> + *
> + * @param timer_data_id
> + * An identifier indicating which instance of timer data should be used for
> + * this operation.
> + * @param f
> + * A pointer to a file for output
> + * @return
> + * - 0: success
> + * - -EINVAL: invalid timer_data_id
> + */
> +int __rte_experimental
> +rte_timer_alt_dump_stats(uint32_t timer_data_id, FILE *f);
>
> #ifdef __cplusplus
> }
> diff --git a/lib/librte_timer/rte_timer_version.map
> b/lib/librte_timer/rte_timer_version.map
> index 9b2e4b8..c2e5836 100644
> --- a/lib/librte_timer/rte_timer_version.map
> +++ b/lib/librte_timer/rte_timer_version.map
> @@ -13,3 +13,25 @@ DPDK_2.0 {
>
> local: *;
> };
> +
> +DPDK_19.05 {
> + global:
> +
> + rte_timer_dump_stats;
> + rte_timer_manage;
> + rte_timer_reset;
> + rte_timer_stop;
> + rte_timer_subsystem_init;
> +} DPDK_2.0;
> +
> +EXPERIMENTAL {
> + global:
> +
> + rte_timer_alt_dump_stats;
> + rte_timer_alt_manage;
> + rte_timer_alt_reset;
> + rte_timer_alt_stop;
> + rte_timer_data_alloc;
> + rte_timer_data_dealloc;
> + rte_timer_subsystem_finalize;
> +};
> --
> 2.6.4
>
>
