[radvd-devel-l] [PATCH for 1.12] time: Use clock_gettime and monotonic clock
Reuben Hawkins
reubenhwk at gmail.com
Fri Jun 13 11:54:33 EDT 2014
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.
1.12 will continue to be maintained and enhanced as necessary.
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).
On Fri, Jun 13, 2014 at 10:36 AM, Markus Pargmann <mpa at pengutronix.de>
wrote:
> Currently the system time is used to calculate the timeout in
> milliseconds for the poll function. This can lead to bugs as soon as the
> time of the system changes through NTP. For example starting radvd
> before NTP can lead to a 44 year jump, resulting in a poll timeout of
> 10^6 seconds.
>
> This patch replaces all gettimeofday() usage by the better clock_gettime
> call using a monotonic clock which avoids those situations.
>
> Signed-off-by: Markus Pargmann <mpa at pengutronix.de>
> ---
>
> This is the patch for 1.12, in case there is some stable release in the
> future.
> I will send a patch for 2.0 later.
>
> Regards,
>
> Markus
>
>
> includes.h | 1 +
> process.c | 12 ++++++------
> radvd.c | 8 ++++----
> radvd.h | 12 ++++++------
> send.c | 24 ++++++------------------
> timer.c | 35 +++++++++++++++++++----------------
> 6 files changed, 42 insertions(+), 50 deletions(-)
>
> diff --git a/includes.h b/includes.h
> index 2111e6651d31..d621b5ee585e 100644
> --- a/includes.h
> +++ b/includes.h
> @@ -21,6 +21,7 @@
> #include <stdio.h>
> #include <stdarg.h>
> #include <stdlib.h>
> +#include <stdint.h>
> #include <time.h>
> #include <syslog.h>
> #include <unistd.h>
> diff --git a/process.c b/process.c
> index 53a1150c2091..91011d036528 100644
> --- a/process.c
> +++ b/process.c
> @@ -119,7 +119,7 @@ static void process_rs(struct Interface *iface,
> unsigned char *msg, int len, str
> {
> double delay;
> double next;
> - struct timeval tv;
> + struct timespec ts;
> uint8_t *opt_str;
>
> /* validation */
> @@ -154,22 +154,22 @@ static void process_rs(struct Interface *iface,
> unsigned char *msg, int len, str
> opt_str += optlen;
> }
>
> - gettimeofday(&tv, NULL);
> + clock_gettime(CLOCK_MONOTONIC, &ts);
>
> delay = MAX_RA_DELAY_TIME * rand() / (RAND_MAX + 1.0);
>
> if (iface->UnicastOnly) {
> send_ra_forall(iface, &addr->sin6_addr);
> - } else if (timevaldiff(&tv, &iface->last_multicast) / 1000.0 <
> iface->MinDelayBetweenRAs) {
> + } else if (timespecdiff(&ts, &iface->last_multicast) / 1000.0 <
> iface->MinDelayBetweenRAs) {
> /* last RA was sent only a few moments ago, don't send
> another immediately. */
> next =
> - 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;
> - iface->next_multicast = next_timeval(next);
> + 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;
> + iface->next_multicast = next_timespec(next);
> } else {
> /* no RA sent in a while, send a multicast reply */
> send_ra_forall(iface, NULL);
> next = rand_between(iface->MinRtrAdvInterval,
> iface->MaxRtrAdvInterval);
> - iface->next_multicast = next_timeval(next);
> + iface->next_multicast = next_timespec(next);
> }
> }
>
> diff --git a/radvd.c b/radvd.c
> index ab275efee40f..fcea72a2eea0 100644
> --- a/radvd.c
> +++ b/radvd.c
> @@ -470,7 +470,7 @@ void timer_handler(void *data)
> next = min(MAX_INITIAL_RTR_ADVERT_INTERVAL, next);
> }
>
> - iface->next_multicast = next_timeval(next);
> + iface->next_multicast = next_timespec(next);
> }
>
> void config_interface(void)
> @@ -499,12 +499,12 @@ void kickoff_adverts(void)
> for (iface = IfaceList; iface; iface = iface->next) {
> double next;
>
> - gettimeofday(&iface->last_ra_time, NULL);
> + clock_gettime(CLOCK_MONOTONIC, &iface->last_ra_time);
>
> if (iface->UnicastOnly)
> continue;
>
> - gettimeofday(&iface->last_multicast, NULL);
> + clock_gettime(CLOCK_MONOTONIC, &iface->last_multicast);
>
> /* TODO: AdvSendAdvert is being checked in send_ra now so
> it can be removed here. */
> if (!iface->AdvSendAdvert)
> @@ -516,7 +516,7 @@ void kickoff_adverts(void)
> iface->init_racount++;
>
> next = min(MAX_INITIAL_RTR_ADVERT_INTERVAL,
> iface->MaxRtrAdvInterval);
> - iface->next_multicast = next_timeval(next);
> + iface->next_multicast = next_timespec(next);
> }
> }
> }
> diff --git a/radvd.h b/radvd.h
> index d3b6957cff24..8db4b9dbe1c6 100644
> --- a/radvd.h
> +++ b/radvd.h
> @@ -50,7 +50,7 @@ struct Interface {
>
> int cease_adv;
>
> - struct timeval last_ra_time;
> + struct timespec last_ra_time;
>
> int IgnoreIfMissing;
> int AdvSendAdvert;
> @@ -87,8 +87,8 @@ struct Interface {
> struct AdvRDNSS *AdvRDNSSList;
> struct AdvDNSSL *AdvDNSSLList;
> struct Clients *ClientList;
> - struct timeval last_multicast;
> - struct timeval next_multicast;
> + struct timespec last_multicast;
> + struct timespec next_multicast;
>
> /* Info whether this interface has failed in the past (and may
> need to be reinitialized) */
> int HasFailed;
> @@ -212,9 +212,9 @@ void reload_config(void);
> void reset_prefix_lifetimes(void);
>
> /* timer.c */
> -struct timeval next_timeval(double next);
> -int timevaldiff(struct timeval const *a, struct timeval const *b);
> -int next_time_msec(struct Interface const *iface);
> +struct timespec next_timespec(double next);
> +int64_t timespecdiff(struct timespec const *a, struct timespec const *b);
> +uint64_t next_time_msec(struct Interface const *iface);
> int expired(struct Interface const *iface);
>
> /* device.c */
> diff --git a/send.c b/send.c
> index fb14184d9d84..eaa9506bf6a0 100644
> --- a/send.c
> +++ b/send.c
> @@ -76,18 +76,6 @@ static void send_ra_inc_len(size_t * len, int add)
> }
> }
>
> -static time_t time_diff_secs(const struct timeval *time_x, const struct
> timeval *time_y)
> -{
> - time_t secs_diff;
> -
> - secs_diff = time_x->tv_sec - time_y->tv_sec;
> - if ((time_x->tv_usec - time_y->tv_usec) >= 500000)
> - secs_diff++;
> -
> - return secs_diff;
> -
> -}
> -
> static void decrement_lifetime(const time_t secs, uint32_t * lifetime)
> {
>
> @@ -118,8 +106,8 @@ int send_ra(struct Interface *iface, struct in6_addr
> *dest)
> struct AdvDNSSL *dnssl;
> struct AdvLowpanCo *lowpanco;
> struct AdvAbro *abroo;
> - struct timeval time_now;
> - time_t secs_since_last_ra;
> + struct timespec time_now;
> + int64_t secs_since_last_ra;
> char addr_str[INET6_ADDRSTRLEN];
>
> unsigned char buff[MSG_SIZE_SEND];
> @@ -167,14 +155,14 @@ int send_ra(struct Interface *iface, struct in6_addr
> *dest)
>
> if (dest == NULL) {
> dest = (struct in6_addr *)all_hosts_addr;
> - gettimeofday(&iface->last_multicast, NULL);
> + clock_gettime(CLOCK_MONOTONIC, &iface->last_multicast);
> }
>
> - gettimeofday(&time_now, NULL);
> - secs_since_last_ra = time_diff_secs(&time_now,
> &iface->last_ra_time);
> + clock_gettime(CLOCK_MONOTONIC, &time_now);
> + secs_since_last_ra = timespecdiff(&time_now, &iface->last_ra_time)
> / 1000;
> if (secs_since_last_ra < 0) {
> secs_since_last_ra = 0;
> - flog(LOG_WARNING, "gettimeofday() went backwards!");
> + flog(LOG_WARNING, "clock_gettime(CLOCK_MONOTONIC) went
> backwards!");
> }
> iface->last_ra_time = time_now;
>
> diff --git a/timer.c b/timer.c
> index ede0c36de499..622034290601 100644
> --- a/timer.c
> +++ b/timer.c
> @@ -16,29 +16,32 @@
> #include "config.h"
> #include "radvd.h"
>
> -struct timeval next_timeval(double next)
> +struct timespec next_timespec(double next)
> {
> - struct timeval tv;
> - gettimeofday(&tv, NULL);
> - tv.tv_sec += (int)next;
> - tv.tv_usec += 1000000 * (next - (int)next);
> - return tv;
> + struct timespec ts;
> +
> + clock_gettime(CLOCK_MONOTONIC, &ts);
> + ts.tv_sec += (int)next;
> + ts.tv_nsec += 1000000000ULL * (next - (int)next);
> + return ts;
> }
>
> -int timevaldiff(struct timeval const *a, struct timeval const *b)
> +int64_t timespecdiff(struct timespec const *a, struct timespec const *b)
> {
> - int msec;
> - msec = (a->tv_sec - b->tv_sec) * 1000;
> - msec += (a->tv_usec - b->tv_usec) / 1000;
> + int64_t msec;
> + msec = ((int64_t)a->tv_sec - b->tv_sec) * 1000;
> + msec += ((int64_t)a->tv_nsec - b->tv_nsec) / (1000 * 1000);
> return msec;
> }
>
> /* Returns when the next time should expire in milliseconds. */
> -int next_time_msec(struct Interface const *iface)
> +uint64_t next_time_msec(struct Interface const *iface)
> {
> - struct timeval tv;
> - int retval;
> - gettimeofday(&tv, NULL);
> - retval = timevaldiff(&iface->next_multicast, &tv);
> - return retval >= 1 ? retval : 1;
> + struct timespec ts;
> + int64_t diff_ms;
> + clock_gettime(CLOCK_MONOTONIC, &ts);
> + diff_ms = timespecdiff(&iface->next_multicast, &ts);
> + if (diff_ms <= 0)
> + return 0;
> + return (uint64_t)diff_ms;
> }
> --
> 2.0.0.rc2
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.litech.org/pipermail/radvd-devel-l/attachments/20140613/c47f55f1/attachment.html>
More information about the radvd-devel-l
mailing list