[Git][NTPsec/ntpsec][master] 7 commits: ntptime: ntp_gettime()-returned tv_frac_sec is STA_NANO-sensitive
Hal Murray (@hal.murray)
gitlab at mg.gitlab.com
Mon Nov 13 07:37:02 UTC 2023
Hal Murray pushed to branch master at NTPsec / ntpsec
Commits:
2cae932e by Maciej S. Szmigiero at 2023-11-01T21:41:20+01:00
ntptime: ntp_gettime()-returned tv_frac_sec is STA_NANO-sensitive
However, the code always prints it as microseconds - add a FIXME reminder
to eventually fix this display.
- - - - -
962ef918 by Maciej S. Szmigiero at 2023-11-01T22:13:07+01:00
ntpfrob: adjtimex()-returned values are STA_NANO-sensitive
"Time offset" and "PPS jitter" values can be either in microseconds or in
nanoseconds - mark them accordingly.
Also add a FIXME reminder to eventually fix time.tv_usec display, since
the code currently blindly assumes it is in nanoseconds.
- - - - -
13dea8ba by Maciej S. Szmigiero at 2023-11-01T22:36:54+01:00
ntp_control: the value for v_kli type is in p_li member not p_uint
Otherwise we might not load the whole value.
- - - - -
c7bb643e by Maciej S. Szmigiero at 2023-11-01T22:38:54+01:00
ntp_control: fix kerninfo STA_NANO-sensitive and non-sensitive stats
Some of these stats are STA_NANO-sensitive ("KNUToMS") but some are always
in microseconds ("KUToMS") - treat them accordingly when converting to the
milliseconds that the NTP protocol uses.
In addition to the above, there was a typo in the code - some of the
variables had a "KToMS" flag, but the "v_kli" kernel variable handler only
tested for "ToMS" flag (a different one).
- - - - -
45fa52ac by Maciej S. Szmigiero at 2023-11-01T22:44:40+01:00
clockwork: use bool type for initial_call flag
It's better to use a bool flag since signed integer overflow is an
undefined operation - even though is unlikely to happen in this particular
case.
- - - - -
a610310d by Maciej S. Szmigiero at 2023-11-01T22:52:51+01:00
clockwork: try to set MOD_NANO if requested but not set yet
Otherwise, if the STA_NANO flag was unset in the kernel then the very first
call with MOD_NANO set will have "offset" wrongly divided by 1000.
- - - - -
89c1a8e5 by Maciej S. Szmigiero at 2023-11-02T10:43:47+01:00
clockwork: move STA_NANO-related "constant" adjustment to ntp_adjtime_ns()
We are already adjusting other STA_NANO-related parameters here, adjust
"constant" here too.
Looking at the relevant kernel code it is also clear that "constant" cannot
go below zero only after having 4 added in the microseconds mode, so we
need to allow the input parameter to go as low as -4 in this mode.
Otherwise initial setting of zero "constant" will set it to 4 instead.
- - - - -
6 changed files:
- include/ntpd.h
- libntp/clockwork.c
- ntpd/ntp_control.c
- ntpd/ntp_loopfilter.c
- ntpfrob/dump.c
- ntptime/ntptime.c
Changes:
=====================================
include/ntpd.h
=====================================
@@ -77,8 +77,9 @@ struct ctl_var {
#define N_CLOCK 0x1000 /* Need to read kernel Clock info */
/* Conversions for ntp_adjtime's timex */
-#define KToMS 0x10000 /* nano vs old micro */
+#define KNUToMS 0x10000 /* nano vs old micro */
#define K_16 0x20000 /* 16 bit scaling */
+#define KUToMS 0x40000 /* always micro */
#define RO (CAN_READ)
#define RW (CAN_READ|CAN_WRITE)
=====================================
libntp/clockwork.c
=====================================
@@ -41,12 +41,25 @@ int ntp_adjtime_ns(struct timex *ntx)
{
#ifdef STA_NANO
static bool nanoseconds = false;
- static int callcount = 0;
- if (callcount++ == 0){
+ static bool initial_call = true;
+ if (initial_call)
+ {
struct timex ztx;
memset(&ztx, '\0', sizeof(ztx));
ntp_adjtime(&ztx);
nanoseconds = (STA_NANO & ztx.status) != 0;
+ initial_call = false;
+ }
+#endif
+
+#ifdef STA_NANO
+ if (!nanoseconds && (ntx->modes & MOD_NANO))
+ {
+ struct timex ztx;
+ memset(&ztx, '\0', sizeof(ztx));
+ ztx.modes = MOD_NANO;
+ ntp_adjtime(&ztx);
+ nanoseconds = (STA_NANO & ztx.status) != 0;
}
#endif
@@ -54,6 +67,21 @@ int ntp_adjtime_ns(struct timex *ntx)
if (!nanoseconds)
#endif
ntx->offset /= 1000;
+
+#ifdef MOD_TAI
+ if (!(ntx->modes & MOD_TAI))
+#endif
+ {
+ long kernel_constant_adj = nanoseconds ? 0 : 4;
+
+ ntx->constant -= kernel_constant_adj;
+ if (ntx->constant < -kernel_constant_adj)
+ {
+ ntx->constant = 0;
+ }
+ }
+
+
int errval = ntp_adjtime(ntx);
#ifdef STA_NANO
nanoseconds = (STA_NANO & ntx->status) != 0;
=====================================
ntpd/ntp_control.c
=====================================
@@ -365,17 +365,17 @@ static const struct var sys_var[] = {
Var_uli("authcmacfails", RO, authcmacfail),
/* kerninfo: Kernel timekeeping info */
- Var_kli("koffset", RO|N_CLOCK|KToMS, ntx.offset),
+ Var_kli("koffset", RO|N_CLOCK|KNUToMS, ntx.offset),
Var_kli("kfreq", RO|N_CLOCK|K_16, ntx.freq),
- Var_kli("kmaxerr", RO|N_CLOCK|KToMS, ntx.maxerror),
- Var_kli("kesterr", RO|N_CLOCK|KToMS, ntx.esterror),
+ Var_kli("kmaxerr", RO|N_CLOCK|KUToMS, ntx.maxerror),
+ Var_kli("kesterr", RO|N_CLOCK|KUToMS, ntx.esterror),
Var_int("kstflags", RO|N_CLOCK, ntx.status), // turn to text
Var_li("ktimeconst", RO|N_CLOCK, ntx.constant),
- Var_kli("kprecis", RO|N_CLOCK|KToMS, ntx.precision),
+ Var_kli("kprecis", RO|N_CLOCK|KUToMS, ntx.precision),
Var_kli("kfreqtol", RO|N_CLOCK|K_16, ntx.tolerance), // Not in man page
Var_kli("kppsfreq", RO|N_CLOCK|K_16, ntx.ppsfreq),
Var_kli("kppsstab", RO|N_CLOCK|K_16, ntx.stabil),
- Var_kli("kppsjitter", RO|N_CLOCK|KToMS, ntx.jitter),
+ Var_kli("kppsjitter", RO|N_CLOCK|KNUToMS, ntx.jitter),
Var_int("kppscalibdur", RO|N_CLOCK, ntx.shift), // 1<<shift
Var_li("kppscalibs", RO|N_CLOCK, ntx.calcnt),
Var_li("kppscaliberrs", RO|N_CLOCK, ntx.errcnt),
@@ -1440,17 +1440,21 @@ ctl_putsys(const struct var * v) {
case v_kli:
if (v->flags&K_16) {
/* value is scaled by 16 bits */
- temp_d = FP_UNSCALE(*v->p_uint);
+ temp_d = FP_UNSCALE(*v->p_li);
} else {
- temp_d = (double)*v->p_uint;
+ temp_d = (double)*v->p_li;
};
- if (v->flags&ToMS) {
+ if (v->flags & (KNUToMS | KUToMS)) {
/* value is in nanoseconds or microseconds */
# ifdef STA_NANO
- temp_d *= 1E-9;
-# else
- temp_d *= 1E-6;
+ if ((v->flags & KNUToMS) && (ntx.status & STA_NANO)) {
+ temp_d *= 1E-9;
+ } else
# endif
+ {
+ temp_d *= 1E-6;
+ }
+
temp_d *= MS_PER_S;
}
ctl_putdbl(v->name, temp_d);
=====================================
ntpd/ntp_loopfilter.c
=====================================
@@ -745,14 +745,7 @@ local_clock(
dtemp = .5;
}
ntv.offset = (long)(clock_offset * NS_PER_S + dtemp);
-#ifdef STA_NANO
ntv.constant = clkstate.sys_poll;
-#else /* STA_NANO */
- ntv.constant = clkstate.sys_poll - 4;
-#endif /* STA_NANO */
- if (ntv.constant < 0)
- ntv.constant = 0;
-
ntv.esterror = (long)(clkstate.clock_jitter * US_PER_S);
ntv.maxerror = (long)((sys_vars.sys_rootdelay / 2 +
sys_vars.sys_rootdisp) * US_PER_S);
@@ -1059,7 +1052,7 @@ start_kern_loop(void)
ntv.status = STA_PLL;
ntv.maxerror = sys_maxdisp;
ntv.esterror = sys_maxdisp;
- ntv.constant = clkstate.sys_poll; /* why is it that here constant is unconditionally set to sys_poll, whereas elsewhere is is modified depending on nanosecond vs. microsecond kernel? */
+ ntv.constant = clkstate.sys_poll;
if ((ntp_adj_ret = ntp_adjtime_ns(&ntv)) != 0) {
ntp_adjtime_error_handler(__func__, &ntv, ntp_adj_ret, errno, false, false, __LINE__ - 1);
}
=====================================
ntpfrob/dump.c
=====================================
@@ -19,7 +19,7 @@ const char * verbose[] =
"The system clock is not synchronized to a reliable server.",
"-30-" };
const char * adjkey[] =
-{ "time offset (ns)",
+{ "time offset (us/ns)",
"TAI offset",
"frequency offset",
"error max (us)",
@@ -30,7 +30,7 @@ const char * adjkey[] =
"clock tolerance",
"tick (us)",
"PPS frequency",
- "PPS jitter (ns)",
+ "PPS jitter (us/ns)",
"PPS interval (s)",
"PPS stability",
"PPS jitter limit exceed",
@@ -99,6 +99,7 @@ do_dump(const iomode mode, const int force)
end = 18;
}
+ /* FIXME: this should probably check for STA_NANO when printing tv_usec */
if (mode == json) {
printf("{\"time\":%ld.%09ld, \"verbose\": \"%s\"", txc.time.tv_sec,
txc.time.tv_usec, verbose[v]);
=====================================
ntptime/ntptime.c
=====================================
@@ -255,6 +255,7 @@ main(
if (pll_control < 0) {
break;
}
+ /* FIXME: this should probably check for STA_NANO */
times[c] = ntv.time.tv_frac_sec;
}
#ifdef SIGSYS
View it on GitLab: https://gitlab.com/NTPsec/ntpsec/-/compare/92d84ebd8ba28112c69b2484938524a300a72586...89c1a8e5abec2aa10a794061b49a0637aee6c37d
--
View it on GitLab: https://gitlab.com/NTPsec/ntpsec/-/compare/92d84ebd8ba28112c69b2484938524a300a72586...89c1a8e5abec2aa10a794061b49a0637aee6c37d
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/20231113/f2251fdd/attachment-0001.htm>
More information about the vc
mailing list