<div dir="ltr"><div><div>The patch looks good. I'm a bit concerned that clock_gettime is linux specific. Portable replacements *may* need to be made for BSD, however I'm not too worried about that now, so I'll likely merge the patch over the next few days.<br>
<br></div>1.12 will continue to be maintained and enhanced as necessary.<br></div>2.0 patch will be appreciated too. Note that (I *think*) 1.12 uses poll here 2.0 uses ppoll (it may make a minor difference in the patch).<br>
<br></div><div class="gmail_extra"><br><br><div class="gmail_quote">On Fri, Jun 13, 2014 at 10:36 AM, Markus Pargmann <span dir="ltr"><<a href="mailto:mpa@pengutronix.de" target="_blank">mpa@pengutronix.de</a>></span> wrote:<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">Currently the system time is used to calculate the timeout in<br>
milliseconds for the poll function. This can lead to bugs as soon as the<br>
time of the system changes through NTP. For example starting radvd<br>
before NTP can lead to a 44 year jump, resulting in a poll timeout of<br>
10^6 seconds.<br>
<br>
This patch replaces all gettimeofday() usage by the better clock_gettime<br>
call using a monotonic clock which avoids those situations.<br>
<br>
Signed-off-by: Markus Pargmann <<a href="mailto:mpa@pengutronix.de">mpa@pengutronix.de</a>><br>
---<br>
<br>
This is the patch for 1.12, in case there is some stable release in the future.<br>
I will send a patch for 2.0 later.<br>
<br>
Regards,<br>
<br>
Markus<br>
<br>
<br>
includes.h | 1 +<br>
process.c | 12 ++++++------<br>
radvd.c | 8 ++++----<br>
radvd.h | 12 ++++++------<br>
send.c | 24 ++++++------------------<br>
timer.c | 35 +++++++++++++++++++----------------<br>
6 files changed, 42 insertions(+), 50 deletions(-)<br>
<br>
diff --git a/includes.h b/includes.h<br>
index 2111e6651d31..d621b5ee585e 100644<br>
--- a/includes.h<br>
+++ b/includes.h<br>
@@ -21,6 +21,7 @@<br>
#include <stdio.h><br>
#include <stdarg.h><br>
#include <stdlib.h><br>
+#include <stdint.h><br>
#include <time.h><br>
#include <syslog.h><br>
#include <unistd.h><br>
diff --git a/process.c b/process.c<br>
index 53a1150c2091..91011d036528 100644<br>
--- a/process.c<br>
+++ b/process.c<br>
@@ -119,7 +119,7 @@ static void process_rs(struct Interface *iface, unsigned char *msg, int len, str<br>
{<br>
double delay;<br>
double next;<br>
- struct timeval tv;<br>
+ struct timespec ts;<br>
uint8_t *opt_str;<br>
<br>
/* validation */<br>
@@ -154,22 +154,22 @@ static void process_rs(struct Interface *iface, unsigned char *msg, int len, str<br>
opt_str += optlen;<br>
}<br>
<br>
- gettimeofday(&tv, NULL);<br>
+ clock_gettime(CLOCK_MONOTONIC, &ts);<br>
<br>
delay = MAX_RA_DELAY_TIME * rand() / (RAND_MAX + 1.0);<br>
<br>
if (iface->UnicastOnly) {<br>
send_ra_forall(iface, &addr->sin6_addr);<br>
- } else if (timevaldiff(&tv, &iface->last_multicast) / 1000.0 < iface->MinDelayBetweenRAs) {<br>
+ } else if (timespecdiff(&ts, &iface->last_multicast) / 1000.0 < iface->MinDelayBetweenRAs) {<br>
/* last RA was sent only a few moments ago, don't send another immediately. */<br>
next =<br>
- iface->MinDelayBetweenRAs - (tv.tv_sec + tv.tv_usec / 1000000.0) + (iface->last_multicast.tv_sec + iface->last_multicast.tv_usec / 1000000.0) + delay / 1000.0;<br>
- iface->next_multicast = next_timeval(next);<br>
+ iface->MinDelayBetweenRAs - (ts.tv_sec + ts.tv_nsec / 1000000000.0) + (iface->last_multicast.tv_sec + iface->last_multicast.tv_nsec / 1000000000.0) + delay / 1000.0;<br>
+ iface->next_multicast = next_timespec(next);<br>
} else {<br>
/* no RA sent in a while, send a multicast reply */<br>
send_ra_forall(iface, NULL);<br>
next = rand_between(iface->MinRtrAdvInterval, iface->MaxRtrAdvInterval);<br>
- iface->next_multicast = next_timeval(next);<br>
+ iface->next_multicast = next_timespec(next);<br>
}<br>
}<br>
<br>
diff --git a/radvd.c b/radvd.c<br>
index ab275efee40f..fcea72a2eea0 100644<br>
--- a/radvd.c<br>
+++ b/radvd.c<br>
@@ -470,7 +470,7 @@ void timer_handler(void *data)<br>
next = min(MAX_INITIAL_RTR_ADVERT_INTERVAL, next);<br>
}<br>
<br>
- iface->next_multicast = next_timeval(next);<br>
+ iface->next_multicast = next_timespec(next);<br>
}<br>
<br>
void config_interface(void)<br>
@@ -499,12 +499,12 @@ void kickoff_adverts(void)<br>
for (iface = IfaceList; iface; iface = iface->next) {<br>
double next;<br>
<br>
- gettimeofday(&iface->last_ra_time, NULL);<br>
+ clock_gettime(CLOCK_MONOTONIC, &iface->last_ra_time);<br>
<br>
if (iface->UnicastOnly)<br>
continue;<br>
<br>
- gettimeofday(&iface->last_multicast, NULL);<br>
+ clock_gettime(CLOCK_MONOTONIC, &iface->last_multicast);<br>
<br>
/* TODO: AdvSendAdvert is being checked in send_ra now so it can be removed here. */<br>
if (!iface->AdvSendAdvert)<br>
@@ -516,7 +516,7 @@ void kickoff_adverts(void)<br>
iface->init_racount++;<br>
<br>
next = min(MAX_INITIAL_RTR_ADVERT_INTERVAL, iface->MaxRtrAdvInterval);<br>
- iface->next_multicast = next_timeval(next);<br>
+ iface->next_multicast = next_timespec(next);<br>
}<br>
}<br>
}<br>
diff --git a/radvd.h b/radvd.h<br>
index d3b6957cff24..8db4b9dbe1c6 100644<br>
--- a/radvd.h<br>
+++ b/radvd.h<br>
@@ -50,7 +50,7 @@ struct Interface {<br>
<br>
int cease_adv;<br>
<br>
- struct timeval last_ra_time;<br>
+ struct timespec last_ra_time;<br>
<br>
int IgnoreIfMissing;<br>
int AdvSendAdvert;<br>
@@ -87,8 +87,8 @@ struct Interface {<br>
struct AdvRDNSS *AdvRDNSSList;<br>
struct AdvDNSSL *AdvDNSSLList;<br>
struct Clients *ClientList;<br>
- struct timeval last_multicast;<br>
- struct timeval next_multicast;<br>
+ struct timespec last_multicast;<br>
+ struct timespec next_multicast;<br>
<br>
/* Info whether this interface has failed in the past (and may need to be reinitialized) */<br>
int HasFailed;<br>
@@ -212,9 +212,9 @@ void reload_config(void);<br>
void reset_prefix_lifetimes(void);<br>
<br>
/* timer.c */<br>
-struct timeval next_timeval(double next);<br>
-int timevaldiff(struct timeval const *a, struct timeval const *b);<br>
-int next_time_msec(struct Interface const *iface);<br>
+struct timespec next_timespec(double next);<br>
+int64_t timespecdiff(struct timespec const *a, struct timespec const *b);<br>
+uint64_t next_time_msec(struct Interface const *iface);<br>
int expired(struct Interface const *iface);<br>
<br>
/* device.c */<br>
diff --git a/send.c b/send.c<br>
index fb14184d9d84..eaa9506bf6a0 100644<br>
--- a/send.c<br>
+++ b/send.c<br>
@@ -76,18 +76,6 @@ static void send_ra_inc_len(size_t * len, int add)<br>
}<br>
}<br>
<br>
-static time_t time_diff_secs(const struct timeval *time_x, const struct timeval *time_y)<br>
-{<br>
- time_t secs_diff;<br>
-<br>
- secs_diff = time_x->tv_sec - time_y->tv_sec;<br>
- if ((time_x->tv_usec - time_y->tv_usec) >= 500000)<br>
- secs_diff++;<br>
-<br>
- return secs_diff;<br>
-<br>
-}<br>
-<br>
static void decrement_lifetime(const time_t secs, uint32_t * lifetime)<br>
{<br>
<br>
@@ -118,8 +106,8 @@ int send_ra(struct Interface *iface, struct in6_addr *dest)<br>
struct AdvDNSSL *dnssl;<br>
struct AdvLowpanCo *lowpanco;<br>
struct AdvAbro *abroo;<br>
- struct timeval time_now;<br>
- time_t secs_since_last_ra;<br>
+ struct timespec time_now;<br>
+ int64_t secs_since_last_ra;<br>
char addr_str[INET6_ADDRSTRLEN];<br>
<br>
unsigned char buff[MSG_SIZE_SEND];<br>
@@ -167,14 +155,14 @@ int send_ra(struct Interface *iface, struct in6_addr *dest)<br>
<br>
if (dest == NULL) {<br>
dest = (struct in6_addr *)all_hosts_addr;<br>
- gettimeofday(&iface->last_multicast, NULL);<br>
+ clock_gettime(CLOCK_MONOTONIC, &iface->last_multicast);<br>
}<br>
<br>
- gettimeofday(&time_now, NULL);<br>
- secs_since_last_ra = time_diff_secs(&time_now, &iface->last_ra_time);<br>
+ clock_gettime(CLOCK_MONOTONIC, &time_now);<br>
+ secs_since_last_ra = timespecdiff(&time_now, &iface->last_ra_time) / 1000;<br>
if (secs_since_last_ra < 0) {<br>
secs_since_last_ra = 0;<br>
- flog(LOG_WARNING, "gettimeofday() went backwards!");<br>
+ flog(LOG_WARNING, "clock_gettime(CLOCK_MONOTONIC) went backwards!");<br>
}<br>
iface->last_ra_time = time_now;<br>
<br>
diff --git a/timer.c b/timer.c<br>
index ede0c36de499..622034290601 100644<br>
--- a/timer.c<br>
+++ b/timer.c<br>
@@ -16,29 +16,32 @@<br>
#include "config.h"<br>
#include "radvd.h"<br>
<br>
-struct timeval next_timeval(double next)<br>
+struct timespec next_timespec(double next)<br>
{<br>
- struct timeval tv;<br>
- gettimeofday(&tv, NULL);<br>
- tv.tv_sec += (int)next;<br>
- tv.tv_usec += 1000000 * (next - (int)next);<br>
- return tv;<br>
+ struct timespec ts;<br>
+<br>
+ clock_gettime(CLOCK_MONOTONIC, &ts);<br>
+ ts.tv_sec += (int)next;<br>
+ ts.tv_nsec += 1000000000ULL * (next - (int)next);<br>
+ return ts;<br>
}<br>
<br>
-int timevaldiff(struct timeval const *a, struct timeval const *b)<br>
+int64_t timespecdiff(struct timespec const *a, struct timespec const *b)<br>
{<br>
- int msec;<br>
- msec = (a->tv_sec - b->tv_sec) * 1000;<br>
- msec += (a->tv_usec - b->tv_usec) / 1000;<br>
+ int64_t msec;<br>
+ msec = ((int64_t)a->tv_sec - b->tv_sec) * 1000;<br>
+ msec += ((int64_t)a->tv_nsec - b->tv_nsec) / (1000 * 1000);<br>
return msec;<br>
}<br>
<br>
/* Returns when the next time should expire in milliseconds. */<br>
-int next_time_msec(struct Interface const *iface)<br>
+uint64_t next_time_msec(struct Interface const *iface)<br>
{<br>
- struct timeval tv;<br>
- int retval;<br>
- gettimeofday(&tv, NULL);<br>
- retval = timevaldiff(&iface->next_multicast, &tv);<br>
- return retval >= 1 ? retval : 1;<br>
+ struct timespec ts;<br>
+ int64_t diff_ms;<br>
+ clock_gettime(CLOCK_MONOTONIC, &ts);<br>
+ diff_ms = timespecdiff(&iface->next_multicast, &ts);<br>
+ if (diff_ms <= 0)<br>
+ return 0;<br>
+ return (uint64_t)diff_ms;<br>
}<br>
<span class="HOEnZb"><font color="#888888">--<br>
2.0.0.rc2<br>
<br>
</font></span></blockquote></div><br></div>