[Git][NTPsec/ntpsec][master] Eliminates inappropriate CLOCK_MONOTONIC from ntpfrob.

[Eric S. Raymond]

Eric S. Raymond pushed to branch master at NTPsec / ntpsec


Commits:
d9b515b3 by Fred Wright at 2017-11-16T13:45:45-08:00
Eliminates inappropriate CLOCK_MONOTONIC from ntpfrob.

There are at least three reasons why it's inappropriate to use
CLOCK_MONOTONIC in the resolution measurement:

1) It's ostensibly trying to measure the resolution of CLOCK_REALTIME,
which is the clock actually managed by NTP, but is relying on the
assumption that CLOCK_REALTIME and CLOCK_MONOTONIC have the same
resolution.  Although this may be true in some cases, it's not
guaranteed.  Note that the clock_getres() function, which is the
intended method for obtaining the resolution, takes a clock_id
argument precisely because the clocks may have different resolutions.
However, actually using clock_getres() is problematic, since correct
implementations seem to be the exception rather than the rule, hence
the need for this code.

2) The stated purpose of using CLOCK_MONOTONIC is to decouple the
result from NTP adjustments.  But, although CLOCK_MONOTONIC is
woefully underspecified, it's most commonly implemented as a variant
of CLOCK_REALTIME that excludes step adjustments while including all
slewing adjustments.  Thus, it's not really decoupled from NTP
adjustments at all.  In fact, if one assumes that step adjustments are
fairly rare, then CLOCK_MONOTONIC is almost equivalent to
CLOCK_REALTIME, except for the differing epoch.

3) The only non-process clock guaranteed to be provided by
clock_gettime() is CLOCK_REALTIME.  Any use of CLOCK_MONOTONIC needs
to include fallback to handle its absence, but the current code
doesn't correctly handle failure of clock_gettime() at all.  Fixing
this with a proper fallback would only make sense if it weren't for #1
and #2 above.

The bottom line is that it's best to use CLOCK_REALTIME for the
resolution measurement, while being prepared for possible step
adjustments.  Given that this is just a special-purpose test tool, and
that step adjustments are rare, it should be sufficient to rely on
user retry for this.

It should be noted that this code has various other issues as well,
but this change only adddresses the CLOCK_MONOTONIC issue.

TESTED:
Observed plausible results on OSX 10.9, Ubuntu 14.04, CentOS 7,
and Fedora 25.

- - - - -


1 changed file:

- ntpfrob/precision.c


Changes:

=====================================
ntpfrob/precision.c
=====================================
--- a/ntpfrob/precision.c
+++ b/ntpfrob/precision.c
@@ -75,10 +75,10 @@ default_get_resolution(void)
 	long val;
 	int minsteps = MINLOOPS;	/* need at least this many steps */
 
-	clock_gettime(CLOCK_MONOTONIC, &tp);
+	clock_gettime(CLOCK_REALTIME, &tp);
 	last = tp.tv_nsec;
 	for (i = - --minsteps; i< MAXLOOPS; i++) {
-		clock_gettime(CLOCK_MONOTONIC, &tp);
+		clock_gettime(CLOCK_REALTIME, &tp);
 		diff = tp.tv_nsec - last;
 		if (diff < 0) diff += DNSECS;
 		if (diff > MINSTEP) if (minsteps-- <= 0) break;
@@ -111,7 +111,7 @@ default_get_resolution(void)
 
 /*
  * This routine calculates the differences between successive calls to
- * clock_gettime(MONOTONIC). If a difference is less than zero, the ns field
+ * clock_gettime(REALTIME). If a difference is less than zero, the ns field
  * has rolled over to the next second, so we add a second in ns. If
  * the difference is greater than zero and less than MINSTEP, the
  * clock has been advanced by a small amount to avoid standing still.
@@ -133,10 +133,10 @@ default_get_precision(void)
 
 	nsec = 0;
 	val = MAXSTEP;
-	clock_gettime(CLOCK_MONOTONIC, &tp);
+	clock_gettime(CLOCK_REALTIME, &tp);
 	last = tp.tv_nsec;
 	for (i = 0; i < MINLOOPS && nsec < HUSECS * 1024;) {
-	    clock_gettime(CLOCK_MONOTONIC, &tp);
+	    clock_gettime(CLOCK_REALTIME, &tp);
 		diff = tp.tv_nsec - last;
 		last = tp.tv_nsec;
 		if (diff < 0)



View it on GitLab: https://gitlab.com/NTPsec/ntpsec/commit/d9b515b39279fde347e5fbfd50f468f30c960c82

---
View it on GitLab: https://gitlab.com/NTPsec/ntpsec/commit/d9b515b39279fde347e5fbfd50f468f30c960c82
You're receiving this email because of your account on gitlab.com.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.ntpsec.org/pipermail/vc/attachments/20171120/cc338d82/attachment.html>