changeset 4924:7c1a365b1e2d draft

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.
author Roy Marples <roy@marples.name>
date Tue, 07 Jan 2020 14:15:14 +0000
parents 4fcca755943e
children 74b140568feb
files src/dhcp.c src/dhcp6.c src/eloop.c src/eloop.h src/ipv6.c src/ipv6nd.c
diffstat 6 files changed, 184 insertions(+), 152 deletions(-) [+]
line wrap: on
line diff
--- a/src/dhcp.c	Wed Jan 01 11:18:49 2020 +0000
+++ b/src/dhcp.c	Tue Jan 07 14:15:14 2020 +0000
@@ -798,13 +798,14 @@
 
 	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);
--- a/src/dhcp6.c	Wed Jan 01 11:18:49 2020 +0000
+++ b/src/dhcp6.c	Tue Jan 07 14:15:14 2020 +0000
@@ -401,7 +401,7 @@
 	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 @@
 		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 @@
 
 	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))
--- a/src/eloop.c	Wed Jan 01 11:18:49 2020 +0000
+++ b/src/eloop.c	Tue Jan 07 14:15:14 2020 +0000
@@ -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_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 @@
 	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 @@
 	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 @@
 #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 @@
  */
 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 @@
 		}
 	}
 
-	clock_gettime(CLOCK_MONOTONIC, &t->created);
+	eloop_reduce_timers(eloop);
+
 	t->seconds = seconds;
 	t->nseconds = nseconds;
 	t->callback = callback;
@@ -561,22 +632,8 @@
 	/* 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 @@
     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 @@
 
 	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 @@
 eloop_new(void)
 {
 	struct eloop *eloop;
-	struct timespec now;
-
-	/* Check we have a working monotonic clock. */
-	if (clock_gettime(CLOCK_MONOTONIC, &now) == -1)
-		return NULL;
 
 	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 (eloop == NULL)
+		return NULL;
+
+	/* Check we have a working monotonic clock. */
+	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;
+
 #if defined(HAVE_KQUEUE) || defined(HAVE_EPOLL)
-		if (eloop_open(eloop) == -1) {
-			eloop_free(eloop);
-			return NULL;
-		}
+	if (eloop_open(eloop) == -1) {
+		eloop_free(eloop);
+		return NULL;
+	}
 #endif
-	}
 
 	return eloop;
 }
@@ -935,7 +1000,7 @@
 	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 @@
 			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;
+		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;
+		}
 
-			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 (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;
--- a/src/eloop.h	Wed Jan 01 11:18:49 2020 +0000
+++ b/src/eloop.h	Tue Jan 07 14:15:14 2020 +0000
@@ -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 *);
--- a/src/ipv6.c	Wed Jan 01 11:18:49 2020 +0000
+++ b/src/ipv6.c	Tue Jan 07 14:15:14 2020 +0000
@@ -666,28 +666,26 @@
 	    (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;
 		}
 	}
 
--- a/src/ipv6nd.c	Wed Jan 01 11:18:49 2020 +0000
+++ b/src/ipv6nd.c	Tue Jan 07 14:15:14 2020 +0000
@@ -1594,7 +1594,8 @@
 {
 	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 @@
 			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 @@
 				}
 			} 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 @@
 				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 @@
 				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 @@
 			}
 
 			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;
 		}