[Git][NTPsec/ntpsec][master] 6 commits: First cut at fixing lockclock mode
Hal Murray (@hal.murray)
gitlab at mg.gitlab.com
Fri Mar 8 06:19:14 UTC 2024
Hal Murray pushed to branch master at NTPsec / ntpsec
Commits:
ab37f1dd by Hal Murray at 2024-03-06T23:51:13-08:00
First cut at fixing lockclock mode
The old Mills code implemented lockclock mode with a configure/build
time switch. We switched that to a runtime test: loop_data.lockclock
But that didn't get turned on early enough so ntpd's initialization
did things like try to restore the drift setting from the drift file
which would have trampled the code that was doing the real timekeeping.
This patch does:
1: Patch refclock_local to use ntp_gettime() rather than ntp_adjtime()
(The answer is the return code, not anything in the struct.)
2: Patch ntp_config to turn on loop_data.lockclock while parsing
the config file rather than waiting for somebody to open the
local driver so it can do it.
That pair lets us put a break at ntp_adjtime since nobody should call it.
Note that ntp_adjtime has a messy API. It is a combined reader and
writer depending on various bits in a field.
- - - - -
ee60b790 by Hal Murray at 2024-03-07T00:09:50-08:00
Second step on fixing lockclock mode
I think this is a bug in the Mills code.
The comment for local_clock in ntp_loopfilter.c says:
* Return codes:
* -1 update ignored: exceeds panic threshold
* 0 update ignored: popcorn or exceeds step threshold
* 1 clock was slewed
* 2 clock was stepped
When in lockclock mode it was returning 0. The caller was
doing nothing rather than switching sys_leap from
LEAP_NOTINSYNC to LEAP_NOWARNING. While stuck in
LEAP_NOTINSYNC, clients would ignore our responses.
The comment also says:
* If lockclock is on, the only thing this routine does is set the
* sys_rootdisp variable equal to the peer dispersion.
But the code doesn't do that.
- - - - -
25ef5c65 by Hal Murray at 2024-03-07T00:22:57-08:00
Drop LOOP_KERN_CLEAR -- not used
- - - - -
57a0e780 by Hal Murray at 2024-03-07T00:25:54-08:00
Use xmt_leap
Looks like an edit was missed when some code was merged from
the Mills code long long ago. It is setup by set_sys_leap
and only used in this one place so looks a bit silly when
it is never used.
- - - - -
6f4b00e2 by Hal Murray at 2024-03-07T00:48:32-08:00
Use set_sys_leap() in a few places were it was missed.
- - - - -
e8ba7e96 by Hal Murray at 2024-03-07T00:54:16-08:00
Drop unused CLK_HOLDOVER and keyword "holdover"
- - - - -
9 changed files:
- include/ntp.h
- include/ntp_refclock.h
- ntpd/keyword-gen.c
- ntpd/ntp_config.c
- ntpd/ntp_loopfilter.c
- ntpd/ntp_parser.y
- ntpd/ntp_proto.c
- ntpd/ntp_timer.c
- ntpd/refclock_local.c
Changes:
=====================================
include/ntp.h
=====================================
@@ -587,7 +587,7 @@ struct pkt {
* Configuration items for the loop filter
*/
#define LOOP_DRIFTINIT 1 /* iniitialize frequency */
-#define LOOP_KERN_CLEAR 2 /* set initial frequency offset */
+// #define LOOP_KERN_CLEAR 2 /* set initial frequency offset */
#define LOOP_MAX 3 /* set both step offsets */
#define LOOP_MAX_BACK 4 /* set bacward-step offset */
#define LOOP_MAX_FWD 5 /* set forward-step offset */
=====================================
include/ntp_refclock.h
=====================================
@@ -25,7 +25,7 @@
#define CLK_FLAG2 0x2
#define CLK_FLAG3 0x4
#define CLK_FLAG4 0x8
-#define CLK_HOLDOVER 0x10 /* no corresponding HAVE_ flag */
+// #define CLK_HOLDOVER 0x10 /* no corresponding HAVE_ flag */
#define CLK_HAVEFLAG1 0x10
#define CLK_HAVEFLAG2 0x20
=====================================
ntpd/keyword-gen.c
=====================================
@@ -37,7 +37,6 @@ struct key_tok ntp_keywords[] = {
{ "end", T_End, FOLLBY_TOKEN },
{ "filegen", T_Filegen, FOLLBY_TOKEN },
{ "fudge", T_Fudge, FOLLBY_STRING },
-{ "holdover", T_Holdover, FOLLBY_TOKEN },
{ "io", T_Io, FOLLBY_TOKEN },
{ "includefile", T_Includefile, FOLLBY_STRING },
{ "leapfile", T_Leapfile, FOLLBY_STRING },
=====================================
ntpd/ntp_config.c
=====================================
@@ -746,8 +746,18 @@ create_peer_node(
case T_Flag1:
my_node->clock_stat.haveflags |= CLK_HAVEFLAG1;
- if (option->value.i)
+ if (option->value.i) {
+#ifdef CLOCK_LOCAL
+ /* Ugly hack:
+ * Need to set loop_data.lockclock early */
+ char * name = my_node->addr->address;
+ if (strstr(name, "127.127.1") == name) {
+ msyslog(LOG_INFO, "INIT-server: Switching to lockclock mode");
+ loop_data.lockclock = true;
+ }
+#endif
my_node->clock_stat.flags |= CLK_FLAG1;
+ }
else
my_node->clock_stat.flags &= ~CLK_FLAG1;
break;
@@ -776,9 +786,6 @@ create_peer_node(
my_node->clock_stat.flags &= ~CLK_FLAG4;
break;
- case T_Holdover:
- my_node->clock_stat.flags |= CLK_HOLDOVER;
- break;
#endif /* REFCLOCK */
default:
@@ -2406,8 +2413,18 @@ config_fudge(
case T_Flag1:
clock_stat.haveflags |= CLK_HAVEFLAG1;
- if (curr_opt->value.i)
+ if (curr_opt->value.i) {
+#ifdef CLOCK_LOCAL
+ /* Ugly hack:
+ * Need to set loop_data.lockclock early */
+ char * name = addr_node->address;
+ if (strstr(name, "127.127.1") == name) {
+ msyslog(LOG_INFO, "INIT-fudge: Switching to lockclock mode");
+ loop_data.lockclock = true;
+ }
+#endif
clock_stat.flags |= CLK_FLAG1;
+ }
else
clock_stat.flags &= ~CLK_FLAG1;
break;
=====================================
ntpd/ntp_loopfilter.c
=====================================
@@ -439,7 +439,7 @@ local_clock(
record_loop_stats(fp_offset, loop_data.drift_comp,
clkstate.clock_jitter, loop_data.clock_stability,
clkstate.sys_poll);
- return (0);
+ return (1); /* very tiny slew */
}
int rval; /* return code */
@@ -1181,20 +1181,6 @@ loop_config(
loop_started = true;
break;
- case LOOP_KERN_CLEAR:
-#if 0 /* XXX: needs more review, and how can we get here? */
- if (!loop_data.lockclock && (clock_ctl.pll_control && clock_ctl.kern_enable)) {
- memset((char *)&ntv, 0, sizeof(ntv));
- ntv.modes = MOD_STATUS;
- ntv.status = STA_UNSYNC;
- ntp_adjtime_ns(&ntv);
- sync_status("kernel time sync disabled",
- pll_status,
- ntv.status);
- }
-#endif
- break;
-
/*
* Tinker command variables for Ulrich Windl. Very dangerous.
*/
=====================================
ntpd/ntp_parser.y
=====================================
@@ -95,7 +95,6 @@
%token <Integer> T_Floor
%token <Integer> T_Freq
%token <Integer> T_Fudge
-%token <Integer> T_Holdover
%token <Integer> T_Huffpuff
%token <Integer> T_Iburst
%token <Integer> T_Ignore
@@ -451,7 +450,6 @@ option_int_keyword
| T_Subtype
| T_Version
| T_Baud
- | T_Holdover
;
option_double
=====================================
ntpd/ntp_proto.c
=====================================
@@ -2330,7 +2330,7 @@ fast_xmit(
* So far, nobody cares.
* Note: There is significant NTPv1 traffic. See #707
*/
- xpkt.li_vn_mode = PKT_LI_VN_MODE(sys_vars.sys_leap,
+ xpkt.li_vn_mode = PKT_LI_VN_MODE(xmt_leap,
PKT_VERSION(rbufp->pkt.li_vn_mode), MODE_SERVER);
xpkt.stratum = STRATUM_TO_PKT(sys_vars.sys_stratum);
xpkt.ppoll = max(rbufp->pkt.ppoll, rstrct.ntp_minpoll);
@@ -2863,10 +2863,10 @@ init_proto(const bool verbose)
l_fp dummy;
/*
- * Fill in the sys_* stuff. Default is don't listen to
- * broadcasting, require authentication.
+ * Fill in the sys_* stuff.
+ * Default is require authentication.
*/
- sys_vars.sys_leap = LEAP_NOTINSYNC;
+ set_sys_leap(LEAP_NOTINSYNC);
sys_vars.sys_stratum = STRATUM_UNSPEC;
memcpy(&sys_vars.sys_refid, "INIT", REFIDLEN);
sys_vars.sys_peer = NULL;
=====================================
ntpd/ntp_timer.c
=====================================
@@ -226,7 +226,7 @@ timer(void)
if (sys_orphan < STRATUM_UNSPEC && sys_vars.sys_peer == NULL &&
current_time > orphwait) {
if (sys_vars.sys_leap == LEAP_NOTINSYNC) {
- sys_vars.sys_leap = LEAP_NOWARNING;
+ set_sys_leap(LEAP_NOWARNING);
}
sys_vars.sys_stratum = (uint8_t)sys_orphan;
if (sys_vars.sys_stratum > 1)
@@ -249,11 +249,11 @@ timer(void)
if (sys_vars.sys_leap != LEAP_NOTINSYNC) {
if (leapsec >= LSPROX_ANNOUNCE && leapdif) {
if (leapdif > 0)
- sys_vars.sys_leap = LEAP_ADDSECOND;
+ set_sys_leap(LEAP_ADDSECOND);
else
- sys_vars.sys_leap = LEAP_DELSECOND;
+ set_sys_leap(LEAP_DELSECOND);
} else {
- sys_vars.sys_leap = LEAP_NOWARNING;
+ set_sys_leap(LEAP_NOWARNING);
}
}
=====================================
ntpd/refclock_local.c
=====================================
@@ -44,9 +44,14 @@
* oscillator. In extreme cases, this can cause clients to exceed the
* 128-ms slew window and drop off the NTP subnet.
*
+ * In lockclock mode, ntpd will not call ntp_adjtime() since that
+ * might trample on whatever some other program is trying to do.
+ * NB: setting loop_data.lockclock here is too late.
+ * There is hack code in ntp_config.c to set it when parsing ntp.conf
+ *
* Fudge Factors
+ * flag1 set to 1 enables lockclock mode
*
- * None currently supported.
*/
/*
* Local interface definitions
@@ -109,6 +114,7 @@ local_start(
memcpy(&pp->refid, "LOCL", REFIDLEN);
peer->sstclktype = CTL_SST_TS_LOCAL;
poll_time = current_time;
+ /* This is too late. See comment way above. */
if (pp->sloppyclockflag & CLK_FLAG1)
loop_data.lockclock = true;
return true;
@@ -159,9 +165,10 @@ local_poll(
* the leap bits and quality indicators from the kernel.
*/
if (loop_data.lockclock) {
- struct timex ntv;
+ struct ntptimeval ntv;
memset(&ntv, 0, sizeof ntv);
- switch (ntp_adjtime(&ntv)) {
+ /* status info in return value */
+ switch (ntp_gettime(&ntv)) {
case TIME_OK:
pp->leap = LEAP_NOWARNING;
peer->stratum = pp->stratum;
View it on GitLab: https://gitlab.com/NTPsec/ntpsec/-/compare/6386e59bbd336b23affa784b00bc7ecf113411ea...e8ba7e96e8c56589d3681eba73a20ea890730b78
--
View it on GitLab: https://gitlab.com/NTPsec/ntpsec/-/compare/6386e59bbd336b23affa784b00bc7ecf113411ea...e8ba7e96e8c56589d3681eba73a20ea890730b78
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/20240308/a884d7fd/attachment-0001.htm>
More information about the vc
mailing list