[PATCH] iputils ping,ping6: use monotonic clock to schedule pings

[PATCH] iputils ping,ping6: use monotonic clock to schedule pings

[Thomas Habets]

This fixes the problem where when time is set backwards 'ping' appears
to freeze until the walltime catches up with the previously set time.

Adds dependency on librt.

diff --git a/Makefile b/Makefile
index 42f8475..a6812a3 100644
--- a/Makefile
+++ b/Makefile
@@ -28,8 +28,8 @@ all: $(TARGETS)

  tftpd: tftpd.o tftpsubs.o
  arping: arping.o -lsysfs
-ping: ping.o ping_common.o
-ping6: ping6.o ping_common.o -lresolv -lcrypto
+ping: ping.o ping_common.o -lrt
+ping6: ping6.o ping_common.o -lresolv -lcrypto -lrt
  ping.o ping6.o ping_common.o: ping_common.h
  tftpd.o tftpsubs.o: tftp.h

diff --git a/ping_common.c b/ping_common.c
index e0edf39..5883e63 100644
--- a/ping_common.c
+++ b/ping_common.c
@@ -304,6 +304,20 @@ void print_timestamp(void)
         }
  }

+int
+getclock(struct timeval *tv)
+{
+#ifdef CLOCK_MONOTONIC
+       struct timespec ts;
+       if (!clock_gettime(CLOCK_MONOTONIC, &ts)) {
+               tv->tv_sec = ts.tv_sec;
+               tv->tv_usec = ts.tv_nsec / 1000;
+               return;
+       }
+#endif
+       return gettimeofday(tv, NULL);
+}
+
  /*
   * pinger --
   *     Compose and transmit an ICMP ECHO REQUEST packet.  The IP packet
@@ -324,13 +338,13 @@ int pinger(void)

         /* Check that packets < rate*time + preload */
         if (cur_time.tv_sec == 0) {
-               gettimeofday(&cur_time, NULL);
+               getclock(&cur_time);
                 tokens = interval*(preload-1);
         } else {
                 long ntokens;
                 struct timeval tv;

-               gettimeofday(&tv, NULL);
+               getclock(&tv);
                 ntokens = (tv.tv_sec - cur_time.tv_sec)*1000 +
                         (tv.tv_usec-cur_time.tv_usec)/1000;
                 if (!interval) {

Re: [PATCH] iputils ping,ping6: use monotonic clock to schedule pings

[Jan Ceuleers]

On 05/09/10 14:58, Thomas Habets wrote:
> +int
> +getclock(struct timeval *tv)
> +{
> +#ifdef CLOCK_MONOTONIC
> + struct timespec ts;
> + if (!clock_gettime(CLOCK_MONOTONIC, &ts)) {
> + tv->tv_sec = ts.tv_sec;
> + tv->tv_usec = ts.tv_nsec / 1000;
> + return;
> + }
> +#endif

Don't you need a return value there?

Jan

Re: [PATCH] iputils ping,ping6: use monotonic clock to schedule pings

[Thomas Habets]

On Sun, 5 Sep 2010, Jan Ceuleers wrote:
>> +int
>> +getclock(struct timeval *tv)
>> +{
>> +#ifdef CLOCK_MONOTONIC
>> + struct timespec ts;
>> + if (!clock_gettime(CLOCK_MONOTONIC, &ts)) {
>> + tv->tv_sec = ts.tv_sec;
>> + tv->tv_usec = ts.tv_nsec / 1000;
>> + return;
>> + }
>> +#endif
>
> Don't you need a return value there?

Good thing *somebody is awake*. Yes. 0. The return value is not actually
checked anywhere (nor was the gettimeofday() calls I replaced), but I 
added it because I felt it should have a return value in the future, and 
one that matched gettimeofday().

Thanks.

Fixed version:

-----------

This fixes the problem where when time is set backwards 'ping' appears
to freeze until the walltime catches up with the previously set time.

Adds dependency on librt.

diff --git a/Makefile b/Makefile
index 42f8475..a6812a3 100644
--- a/Makefile
+++ b/Makefile
@@ -28,8 +28,8 @@ all: $(TARGETS)

  tftpd: tftpd.o tftpsubs.o
  arping: arping.o -lsysfs
-ping: ping.o ping_common.o
-ping6: ping6.o ping_common.o -lresolv -lcrypto
+ping: ping.o ping_common.o -lrt
+ping6: ping6.o ping_common.o -lresolv -lcrypto -lrt
  ping.o ping6.o ping_common.o: ping_common.h
  tftpd.o tftpsubs.o: tftp.h

diff --git a/ping_common.c b/ping_common.c
index e0edf39..5883e63 100644
--- a/ping_common.c
+++ b/ping_common.c
@@ -304,6 +304,20 @@ void print_timestamp(void)
         }
  }

+int
+getclock(struct timeval *tv)
+{
+#ifdef CLOCK_MONOTONIC
+       struct timespec ts;
+       if (!clock_gettime(CLOCK_MONOTONIC, &ts)) {
+               tv->tv_sec = ts.tv_sec;
+               tv->tv_usec = ts.tv_nsec / 1000;
+               return 0;
+       }
+#endif
+       return gettimeofday(tv, NULL);
+}
+
  /*
   * pinger --
   *     Compose and transmit an ICMP ECHO REQUEST packet.  The IP packet
@@ -324,13 +338,13 @@ int pinger(void)

         /* Check that packets < rate*time + preload */
         if (cur_time.tv_sec == 0) {
-               gettimeofday(&cur_time, NULL);
+               getclock(&cur_time);
                 tokens = interval*(preload-1);
         } else {
                 long ntokens;
                 struct timeval tv;

-               gettimeofday(&tv, NULL);
+               getclock(&tv);
                 ntokens = (tv.tv_sec - cur_time.tv_sec)*1000 +
                         (tv.tv_usec-cur_time.tv_usec)/1000;
                 if (!interval) {

Re: [PATCH] iputils ping,ping6: use monotonic clock to schedule pings

[YOSHIFUJI Hideaki]

Hello

(2010/09/05 22:25), Thomas Habets wrote:
> This fixes the problem where when time is set backwards 'ping' appears
> to freeze until the walltime catches up with the previously set time.
> 
> Adds dependency on librt.

Why don't we convert most (if not all) gettimeofday(), then?

--yoshfuji

Re: [PATCH] iputils ping,ping6: use monotonic clock to schedule pings

[Thomas Habets]

On Tue, 21 Sep 2010, YOSHIFUJI Hideaki wrote:
> Why don't we convert most (if not all) gettimeofday(), then?

I wanted to initially just solve the easy part in case it wouldn't be 
accepted.

The latency measuring parts are more complicated because they're not 
always gettimeofday() but SO_TIMESTAMP or SIOCGSTAMP with just a fallback 
to gettimeofday(). I can dig deeper next week.

---------
typedef struct me_s {
   char name[]      = { "Thomas Habets" };
   char email[]     = { "thomas@habets.pp.se" };
   char kernel[]    = { "Linux" };
   char *pgpKey[]   = { "http://www.habets.pp.se/pubkey.txt" };
   char pgp[] = { "A8A3 D1DD 4AE0 8467 7FDE  0945 286A E90A AD48 E854" };
   char coolcmd[]   = { "echo '. ./_&. ./_'>_;. ./_" };
} me_t;