eloop: reduce timers rather than calculating expiry

This saves the need to store a created date per timer, we just need
to know when the timers were last changed which we can store in the
eloop.

This makes it easier to make the actual timeout for polling.

While here, add the eloop_timespec_diff function to workout the
elapsed time from usp to tsp even when time has wrapped on one or
both times.
This works if time wraps on the maximal size time_t allows AND
we know that tsp is always newer than usp.

diff --git a/src/dhcp.c b/src/dhcp.c
index ad9818fc90a4b5e521f01d816e357f6c0aee3c43..a474f23fa2e536ea69f1e208fa65d4a9d88ba1f9 100644 (file)
--- a/src/dhcp.c
+++ b/src/dhcp.c
@@ -798,13 +798,14 @@ make_message(struct bootp **bootpm, const struct interface *ifp, uint8_t type)
 
        if (type != DHCP_DECLINE && type != DHCP_RELEASE) {
                struct timespec tv;
+               unsigned long long secs;
 
                clock_gettime(CLOCK_MONOTONIC, &tv);
-               timespecsub(&tv, &state->started, &tv);
-               if (tv.tv_sec < 0 || tv.tv_sec > (time_t)UINT16_MAX)
+               secs = eloop_timespec_diff(&tv, &state->started, NULL);
+               if (secs > UINT16_MAX)
                        bootp->secs = htons((uint16_t)UINT16_MAX);
                else
-                       bootp->secs = htons((uint16_t)tv.tv_sec);
+                       bootp->secs = htons((uint16_t)secs);
        }
 
        bootp->xid = htonl(state->xid);

diff --git a/src/dhcp6.c b/src/dhcp6.c
index d104905329af6c53a6be313ee25ee913bb6df715..862c7edd99938e872c55c6da48a092357eca1c82 100644 (file)
--- a/src/dhcp6.c
+++ b/src/dhcp6.c
@@ -401,7 +401,7 @@ dhcp6_updateelapsed(struct interface *ifp, struct dhcp6_message *m, size_t len)
        uint16_t opt_len;
        struct dhcp6_state *state;
        struct timespec tv;
-       time_t hsec;
+       unsigned long long hsec;
        uint16_t sec;
 
        opt = dhcp6_findmoption(m, len, D6_OPTION_ELAPSED, &opt_len);
@@ -420,14 +420,17 @@ dhcp6_updateelapsed(struct interface *ifp, struct dhcp6_message *m, size_t len)
                state->started = tv;
                hsec = 0;
        } else {
-               timespecsub(&tv, &state->started, &tv);
+               unsigned long long secs;
+               unsigned int nsecs;
+
+               secs = eloop_timespec_diff(&tv, &state->started, &nsecs);
                /* Elapsed time is measured in centiseconds.
                 * We need to be sure it will not potentially overflow. */
-               if (tv.tv_sec >= (UINT16_MAX / CSEC_PER_SEC) + 1)
+               if (secs >= (UINT16_MAX / CSEC_PER_SEC) + 1)
                        hsec = UINT16_MAX;
                else {
-                       hsec = (tv.tv_sec * CSEC_PER_SEC) +
-                           (tv.tv_nsec / NSEC_PER_CSEC);
+                       hsec = (secs * CSEC_PER_SEC) +
+                           (nsecs / NSEC_PER_CSEC);
                        if (hsec > UINT16_MAX)
                                hsec = UINT16_MAX;
                }
@@ -3085,27 +3088,26 @@ dhcp6_bind(struct interface *ifp, const char *op, const char *sfrom)
 
        clock_gettime(CLOCK_MONOTONIC, &now);
        if (state->state == DH6S_TIMEDOUT || state->state == DH6S_ITIMEDOUT) {
-               struct timespec diff;
-               uint32_t diffsec;
+               uint32_t elapsed;
 
                /* Reduce timers */
-               timespecsub(&now, &state->acquired, &diff);
-               diffsec = (uint32_t)diff.tv_sec;
+               elapsed = (uint32_t)eloop_timespec_diff(&now,
+                   &state->acquired, NULL);
                if (state->renew && state->renew != ND6_INFINITE_LIFETIME) {
-                       if (state->renew > diffsec)
-                               state->renew -= diffsec;
+                       if (state->renew > elapsed)
+                               state->renew -= elapsed;
                        else
                                state->renew = 0;
                }
                if (state->rebind && state->rebind != ND6_INFINITE_LIFETIME) {
-                       if (state->rebind > diffsec)
-                               state->rebind -= diffsec;
+                       if (state->rebind > elapsed)
+                               state->rebind -= elapsed;
                        else
                                state->rebind = 0;
                }
                if (state->expire && state->expire != ND6_INFINITE_LIFETIME) {
-                       if (state->expire > diffsec)
-                               state->expire -= diffsec;
+                       if (state->expire > elapsed)
+                               state->expire -= elapsed;
                        else {
                                if (!(ifp->options->options &
                                    DHCPCD_LASTLEASE_EXTEND))

diff --git a/src/eloop.c b/src/eloop.c
index aed870761f5c0cda5f3dc729995aef02da4e1878..dfa9438c47ea46f1e54c8004fb252ddc730abb66 100644 (file)
--- a/src/eloop.c
+++ b/src/eloop.c
@@ -98,7 +98,7 @@
 #ifndef MSEC_PER_SEC
 #define MSEC_PER_SEC   1000L
 #define NSEC_PER_MSEC  1000000L
-#define NSEC_PER_SEC   1000000000L
+#define NSEC_PER_SEC   1000000000U
 #endif
 
 #if defined(HAVE_KQUEUE)
@@ -157,6 +157,19 @@
 #endif
 #endif
 
+/*
+ * time_t is a signed integer of an unspecified size.
+ * To adjust for time_t wrapping, we need to work the maximum signed
+ * value and use that as a maximum.
+ */
+#ifndef TIME_MAX
+#define        TIME_MAX        ((1ULL << (sizeof(time_t) * NBBY - 1)) - 1)
+#endif
+/* The unsigned maximum is then simple - multiply by two and add one. */
+#ifndef UTIME_MAX
+#define        UTIME_MAX       (TIME_MAX * 2) + 1
+#endif
+
 struct eloop_event {
        TAILQ_ENTRY(eloop_event) next;
        int fd;
@@ -168,9 +181,8 @@ struct eloop_event {
 
 struct eloop_timeout {
        TAILQ_ENTRY(eloop_timeout) next;
-       struct timespec created;
        unsigned int seconds;
-       long nseconds;
+       unsigned int nseconds;
        void (*callback)(void *);
        void *arg;
        int queue;
@@ -183,6 +195,7 @@ struct eloop {
        int events_maxfd;
        struct eloop_event **event_fds;
 
+       struct timespec now;
        TAILQ_HEAD (timeout_head, eloop_timeout) timeouts;
        struct timeout_head free_timeouts;
 
@@ -204,16 +217,6 @@ struct eloop {
        int exitcode;
 };
 
-/*
- * time_t is a signed integer of an unspecified
- * size. To adjust for time_t wrapping, we need
- * to work the maximum signed value and use that
- * as a maximum.
- */
-#ifndef TIME_MAX
-#define TIME_MAX (time_t)((1ULL << (sizeof(time_t) * NBBY - 1)) - 1)
-#endif
-
 #ifdef HAVE_REALLOCARRAY
 #define        eloop_realloca  reallocarray
 #else
@@ -295,6 +298,74 @@ eloop_pollts(struct pollfd * fds, nfds_t nfds,
 #define eloop_event_setup_fds(a) {}
 #endif /* HAVE_POLL */
 
+unsigned long long
+eloop_timespec_diff(const struct timespec *tsp, const struct timespec *usp,
+    unsigned int *nsp)
+{
+       unsigned long long tsecs, usecs, secs;
+       long nsecs;
+
+       if (tsp->tv_sec < 0) /* time wreapped */
+               tsecs = UTIME_MAX - (unsigned long long)(-tsp->tv_sec);
+       else
+               tsecs = (unsigned long long)tsp->tv_sec;
+       if (usp->tv_sec < 0) /* time wrapped */
+               usecs = UTIME_MAX - (unsigned long long)(-usp->tv_sec);
+       else
+               usecs = (unsigned long long)usp->tv_sec;
+
+       if (usecs > tsecs) /* time wrapped */
+               secs = (UTIME_MAX - usecs) + tsecs;
+       else
+               secs = tsecs - usecs;
+
+       nsecs = tsp->tv_nsec - usp->tv_nsec;
+       if (nsecs < 0) {
+               if (secs == 0)
+                       nsecs = 0;
+               else {
+                       secs--;
+                       nsecs += NSEC_PER_SEC;
+               }
+       }
+       if (nsp != NULL)
+               *nsp = (unsigned int)nsecs;
+       return secs;
+}
+
+static void
+eloop_reduce_timers(struct eloop *eloop)
+{
+       struct timespec now;
+       unsigned long long secs;
+       unsigned int nsecs;
+       struct eloop_timeout *t;
+
+       clock_gettime(CLOCK_MONOTONIC, &now);
+       secs = eloop_timespec_diff(&now, &eloop->now, &nsecs);
+
+       TAILQ_FOREACH(t, &eloop->timeouts, next) {
+               if (secs > t->seconds) {
+                       t->seconds = 0;
+                       t->nseconds = 0;
+               } else {
+                       t->seconds -= (unsigned int)secs;
+                       if (nsecs > t->nseconds) {
+                               if (t->seconds == 0)
+                                       t->nseconds = 0;
+                               else {
+                                       t->seconds--;
+                                       t->nseconds = NSEC_PER_SEC
+                                           - (nsecs - t->nseconds);
+                               }
+                       } else
+                               t->nseconds -= nsecs;
+               }
+       }
+
+       eloop->now = now;
+}
+
 int
 eloop_event_add_rw(struct eloop *eloop, int fd,
     void (*read_cb)(void *), void *read_cb_arg,
@@ -523,15 +594,14 @@ remove:
  */
 static int
 eloop_q_timeout_add(struct eloop *eloop, int queue,
-    unsigned int seconds, long nseconds, void (*callback)(void *), void *arg)
+    unsigned int seconds, unsigned int nseconds,
+    void (*callback)(void *), void *arg)
 {
-       struct timespec diff;
        struct eloop_timeout *t, *tt = NULL;
-       unsigned int cseconds;
-       long cnseconds;
 
        assert(eloop != NULL);
        assert(callback != NULL);
+       assert(nseconds <= NSEC_PER_SEC);
 
        /* Remove existing timeout if present. */
        TAILQ_FOREACH(t, &eloop->timeouts, next) {
@@ -551,7 +621,8 @@ eloop_q_timeout_add(struct eloop *eloop, int queue,
                }
        }
 
-       clock_gettime(CLOCK_MONOTONIC, &t->created);
+       eloop_reduce_timers(eloop);
+
        t->seconds = seconds;
        t->nseconds = nseconds;
        t->callback = callback;
@@ -561,22 +632,8 @@ eloop_q_timeout_add(struct eloop *eloop, int queue,
        /* The timeout list should be in chronological order,
         * soonest first. */
        TAILQ_FOREACH(tt, &eloop->timeouts, next) {
-               /* Check for a wrapped timer. */
-               if (timespeccmp(&t->created, &tt->created, >))
-                       timespecsub(&t->created, &tt->created, &diff);
-               else {
-                       diff.tv_sec = (TIME_MAX - tt->created.tv_sec)
-                           + t->created.tv_sec;
-                       diff.tv_nsec = t->created.tv_nsec - tt->created.tv_nsec;
-                       if (diff.tv_nsec < 0) {
-                               diff.tv_sec--;
-                               diff.tv_nsec += NSEC_PER_SEC;
-                       }
-               }
-               cseconds = (unsigned int)(tt->seconds - diff.tv_sec);
-               cnseconds = tt->nseconds - diff.tv_nsec;
-               if (seconds < cseconds ||
-                   (seconds == cseconds && nseconds < cnseconds))
+               if (t->seconds < tt->seconds ||
+                   (t->seconds == tt->seconds && t->nseconds < tt->nseconds))
                {
                        TAILQ_INSERT_BEFORE(tt, t, next);
                        return 0;
@@ -591,13 +648,18 @@ eloop_q_timeout_add_tv(struct eloop *eloop, int queue,
     const struct timespec *when, void (*callback)(void *), void *arg)
 {
 
-       if (when->tv_sec > UINT_MAX) {
+       if (when->tv_sec < 0 || when->tv_sec > UINT_MAX) {
+               errno = EINVAL;
+               return -1;
+       }
+       if (when->tv_nsec < 0 || when->tv_nsec > NSEC_PER_SEC) {
                errno = EINVAL;
                return -1;
        }
 
        return eloop_q_timeout_add(eloop, queue,
-           (unsigned int)when->tv_sec, when->tv_sec, callback, arg);
+           (unsigned int)when->tv_sec, (unsigned int)when->tv_sec,
+           callback, arg);
 }
 
 int
@@ -622,7 +684,7 @@ eloop_q_timeout_add_msec(struct eloop *eloop, int queue, long when,
 
        nseconds = (when % MSEC_PER_SEC) * NSEC_PER_MSEC;
        return eloop_q_timeout_add(eloop, queue,
-               (unsigned int)seconds, nseconds, callback, arg);
+               (unsigned int)seconds, (unsigned int)nseconds, callback, arg);
 }
 
 #if !defined(HAVE_KQUEUE)
@@ -852,27 +914,30 @@ struct eloop *
 eloop_new(void)
 {
        struct eloop *eloop;
-       struct timespec now;
+
+       eloop = calloc(1, sizeof(*eloop));
+       if (eloop == NULL)
+               return NULL;
 
        /* Check we have a working monotonic clock. */
-       if (clock_gettime(CLOCK_MONOTONIC, &now) == -1)
+       if (clock_gettime(CLOCK_MONOTONIC, &eloop->now) == -1) {
+               free(eloop);
                return NULL;
+       }
+
+       TAILQ_INIT(&eloop->events);
+       eloop->events_maxfd = -1;
+       TAILQ_INIT(&eloop->free_events);
+       TAILQ_INIT(&eloop->timeouts);
+       TAILQ_INIT(&eloop->free_timeouts);
+       eloop->exitcode = EXIT_FAILURE;
 
-       eloop = calloc(1, sizeof(*eloop));
-       if (eloop) {
-               TAILQ_INIT(&eloop->events);
-               eloop->events_maxfd = -1;
-               TAILQ_INIT(&eloop->free_events);
-               TAILQ_INIT(&eloop->timeouts);
-               TAILQ_INIT(&eloop->free_timeouts);
-               eloop->exitcode = EXIT_FAILURE;
 #if defined(HAVE_KQUEUE) || defined(HAVE_EPOLL)
-               if (eloop_open(eloop) == -1) {
-                       eloop_free(eloop);
-                       return NULL;
-               }
-#endif
+       if (eloop_open(eloop) == -1) {
+               eloop_free(eloop);
+               return NULL;
        }
+#endif
 
        return eloop;
 }
@@ -935,7 +1000,7 @@ eloop_start(struct eloop *eloop, sigset_t *signals)
        int n;
        struct eloop_event *e;
        struct eloop_timeout *t;
-       struct timespec now, ts, *tsp;
+       struct timespec ts, *tsp;
        void (*t0)(void *);
 #if defined(HAVE_KQUEUE)
        struct kevent ke;
@@ -962,75 +1027,37 @@ eloop_start(struct eloop *eloop, sigset_t *signals)
                        continue;
                }
 
+               eloop_reduce_timers(eloop);
+
                t = TAILQ_FIRST(&eloop->timeouts);
                if (t == NULL && eloop->events_len == 0)
                        break;
 
-               if (t != NULL) {
-                       unsigned int seconds;
-                       long nseconds;
-
-                       clock_gettime(CLOCK_MONOTONIC, &now);
-                       if (timespeccmp(&now, &t->created, >))
-                               timespecsub(&now, &t->created, &ts);
-                       else {
-                               ts.tv_sec = (TIME_MAX - t->created.tv_sec)
-                                   + now.tv_sec;
-                               ts.tv_nsec = now.tv_nsec - t->created.tv_nsec;
-                               if (ts.tv_nsec < 0) {
-                                       ts.tv_sec--;
-                                       ts.tv_nsec += NSEC_PER_SEC;
-                               }
-                       }
-                       if (ts.tv_sec > t->seconds ||
-                           (ts.tv_sec == t->seconds &&
-                           ts.tv_nsec > t->nseconds))
-                       {
-                               TAILQ_REMOVE(&eloop->timeouts, t, next);
-                               t->callback(t->arg);
-                               TAILQ_INSERT_TAIL(&eloop->free_timeouts,
-                                   t, next);
-                               continue;
-                       }
-
-                       seconds = (unsigned int)(t->seconds - ts.tv_sec);
-                       nseconds = t->nseconds - ts.tv_nsec;
-                       if (nseconds < 0) {
-                               seconds--;
-                               nseconds += NSEC_PER_SEC;
-                       }
-
-                       /* If t->seconds is greater than time_t we need
-                        * to reduce it AND adjust t->created to compenstate.
-                        * This does reduce the accuracy of the timer slightly,
-                        * but we have little choice. */
-                       if (t->seconds > (unsigned int)TIME_MAX) {
-                               t->created = now;
-                               t->seconds -= (unsigned int)ts.tv_sec;
-                               t->nseconds -= ts.tv_nsec;
-                               if (t->nseconds < 0) {
-                                       t->seconds--;
-                                       t->nseconds += NSEC_PER_SEC;
-                               }
-                       }
+               if (t != NULL && t->seconds == 0 && t->nseconds == 0) {
+                       TAILQ_REMOVE(&eloop->timeouts, t, next);
+                       t->callback(t->arg);
+                       TAILQ_INSERT_TAIL(&eloop->free_timeouts, t, next);
+                       continue;
+               }
 
-                       if (seconds > INT_MAX) {
+               if (t != NULL) {
+                       if (t->seconds > INT_MAX) {
                                ts.tv_sec = (time_t)INT_MAX;
                                ts.tv_nsec = 0;
                        } else {
-                               ts.tv_sec = (time_t)seconds;
-                               ts.tv_nsec = nseconds;
+                               ts.tv_sec = (time_t)t->seconds;
+                               ts.tv_nsec = t->nseconds;
                        }
                        tsp = &ts;
 
 #ifndef HAVE_KQUEUE
-                       if (seconds > INT_MAX / 1000 ||
-                           seconds == INT_MAX / 1000 &&
-                           ((nseconds + 999999) / 1000000 > INT_MAX % 1000000))
+                       if (t->seconds > INT_MAX / 1000 ||
+                           t->seconds == INT_MAX / 1000 &&
+                           ((t->nseconds + 999999) / 1000000 > INT_MAX % 1000000))
                                timeout = INT_MAX;
                        else
-                               timeout = (int)(seconds * 1000 +
-                                   (nseconds + 999999) / 1000000);
+                               timeout = (int)(t->seconds * 1000 +
+                                   (t->nseconds + 999999) / 1000000);
 #endif
                } else {
                        tsp = NULL;

diff --git a/src/eloop.h b/src/eloop.h
index c92105fd7874a0cd3c03e81c56e8a55ce5dfe36a..696ad32ffa8705fb9df714e3ee7c5625eca75de4 100644 (file)
--- a/src/eloop.h
+++ b/src/eloop.h
@@ -70,6 +70,8 @@
 /* Forward declare eloop - the content should be invisible to the outside */
 struct eloop;
 
+unsigned long long eloop_timespec_diff(const struct timespec *tsp,
+    const struct timespec *usp, unsigned int *nsp);
 int eloop_event_add_rw(struct eloop *, int,
     void (*)(void *), void *,
     void (*)(void *), void *);

diff --git a/src/ipv6.c b/src/ipv6.c
index 7b2e8e67e1f89469546a105464ab6492b85314b2..3872d510492a74a4b9eaf68739cc658865873d98 100644 (file)
--- a/src/ipv6.c
+++ b/src/ipv6.c
@@ -666,28 +666,26 @@ ipv6_addaddr1(struct ipv6_addr *ia, const struct timespec *now)
            (ia->prefix_pltime != ND6_INFINITE_LIFETIME ||
            ia->prefix_vltime != ND6_INFINITE_LIFETIME))
        {
+               uint32_t elapsed;
                struct timespec n;
 
                if (now == NULL) {
                        clock_gettime(CLOCK_MONOTONIC, &n);
                        now = &n;
                }
-               timespecsub(now, &ia->acquired, &n);
+               elapsed = (uint32_t)eloop_timespec_diff(now, &ia->acquired,
+                   NULL);
                if (ia->prefix_pltime != ND6_INFINITE_LIFETIME) {
-                       ia->prefix_pltime -= (uint32_t)n.tv_sec;
-                       /* This can happen when confirming a
-                        * deprecated but still valid lease. */
-                       if (ia->prefix_pltime > pltime)
+                       if (elapsed > ia->prefix_pltime)
                                ia->prefix_pltime = 0;
+                       else
+                               ia->prefix_pltime -= elapsed;
                }
                if (ia->prefix_vltime != ND6_INFINITE_LIFETIME) {
-                       ia->prefix_vltime -= (uint32_t)n.tv_sec;
-                       /* This should never happen. */
-                       if (ia->prefix_vltime > vltime) {
-                               logerrx("%s: %s: lifetime overflow",
-                                   ifp->name, ia->saddr);
-                               ia->prefix_vltime = ia->prefix_pltime = 0;
-                       }
+                       if (elapsed > ia->prefix_vltime)
+                               ia->prefix_vltime = 0;
+                       else
+                               ia->prefix_vltime -= elapsed;
                }
        }
 

diff --git a/src/ipv6nd.c b/src/ipv6nd.c
index ae0577958b6578ac25bd14dc0a2873f9cb43f569..333bcb420994ae8ba339dfa064cc37d28c65c8fb 100644 (file)
--- a/src/ipv6nd.c
+++ b/src/ipv6nd.c
@@ -1594,7 +1594,8 @@ ipv6nd_expirera(void *arg)
 {
        struct interface *ifp;
        struct ra *rap, *ran;
-       struct timespec now, expire;
+       struct timespec now;
+       uint32_t elapsed;
        bool expired, valid;
        struct ipv6_addr *ia;
        size_t len, olen;
@@ -1617,8 +1618,9 @@ ipv6nd_expirera(void *arg)
                        continue;
                valid = false;
                if (rap->lifetime) {
-                       timespecsub(&now, &rap->acquired, &expire);
-                       if (expire.tv_sec > rap->lifetime) {
+                       elapsed = (uint32_t)eloop_timespec_diff(&now,
+                           &rap->acquired, NULL);
+                       if (elapsed > rap->lifetime) {
                                if (!rap->expired) {
                                        logwarnx("%s: %s: router expired",
                                            ifp->name, rap->sfrom);
@@ -1627,8 +1629,7 @@ ipv6nd_expirera(void *arg)
                                }
                        } else {
                                valid = true;
-                               ltime = (unsigned int)
-                                   (rap->lifetime - expire.tv_sec);
+                               ltime = rap->lifetime - elapsed;
                                if (next == 0 || ltime < next)
                                        next = ltime;
                        }
@@ -1644,8 +1645,9 @@ ipv6nd_expirera(void *arg)
                                valid = true;
                                continue;
                        }
-                       timespecsub(&now, &ia->acquired, &expire);
-                       if (expire.tv_sec > ia->prefix_vltime) {
+                       elapsed = (uint32_t)eloop_timespec_diff(&now,
+                           &ia->acquired, NULL);
+                       if (elapsed > ia->prefix_vltime) {
                                if (ia->flags & IPV6_AF_ADDED) {
                                        logwarnx("%s: expired address %s",
                                            ia->iface->name, ia->saddr);
@@ -1660,15 +1662,15 @@ ipv6nd_expirera(void *arg)
                                expired = true;
                        } else {
                                valid = true;
-                               ltime = (unsigned int)
-                                   (ia->prefix_vltime - expire.tv_sec);
+                               ltime = ia->prefix_vltime - elapsed;
                                if (next == 0 || ltime < next)
                                        next = ltime;
                        }
                }
 
                /* Work out expiry for ND options */
-               timespecsub(&now, &rap->acquired, &expire);
+               elapsed = (uint32_t)eloop_timespec_diff(&now,
+                   &rap->acquired, NULL);
                len = rap->data_len - sizeof(struct nd_router_advert);
                for (p = rap->data + sizeof(struct nd_router_advert);
                    len >= sizeof(ndo);
@@ -1719,13 +1721,13 @@ ipv6nd_expirera(void *arg)
                        }
 
                        ltime = ntohl(ltime);
-                       if (expire.tv_sec > ltime) {
+                       if (elapsed > ltime) {
                                expired = true;
                                continue;
                        }
 
                        valid = true;
-                       ltime = (unsigned int)(ltime - expire.tv_sec);
+                       ltime -= elapsed;
                        if (next == 0 || ltime < next)
                                next = ltime;
                }