Re: ntpd: go into unsynced mode
On Sat, Aug 22, 2020 at 03:51:48PM +0200, Otto Moerbeek wrote: > Hi, > > At the moment ntpd never goes into unsynced mode if network > connectivity is lost. The code to do that is only triggered when a > pakcet is received, which does not happen. > > This diff fixes that by going into unsynced mode if no time data was > processed for a while. > > An earlier version of this diff was tested by naddy@. Compared to that > version, the needed period of inactivity is now three times as large > and I set scale to 1, so recovery goes faster. > > Please test and review, anyone wants to ok? -Otto > > Index: ntp.c > === > RCS file: /cvs/src/usr.sbin/ntpd/ntp.c,v > retrieving revision 1.165 > diff -u -p -r1.165 ntp.c > --- ntp.c 22 Jun 2020 06:11:34 - 1.165 > +++ ntp.c 22 Aug 2020 13:48:34 - > @@ -89,6 +89,7 @@ ntp_main(struct ntpd_conf *nconf, struct > struct stat stb; > struct ctl_conn *cc; > time_t nextaction, last_sensor_scan = 0, now; > + time_t last_action = 0, interval; > void*newp; > > if (socketpair(AF_UNIX, SOCK_STREAM | SOCK_CLOEXEC, PF_UNSPEC, > @@ -402,6 +403,7 @@ ntp_main(struct ntpd_conf *nconf, struct > for (; nfds > 0 && j < idx_clients; j++) { > if (pfd[j].revents & (POLLIN|POLLERR)) { > nfds--; > + last_action = now; > if (client_dispatch(idx2peer[j - idx_peers], > conf->settime, conf->automatic) == -1) { > log_warn("pipe write error (settime)"); > @@ -417,8 +419,24 @@ ntp_main(struct ntpd_conf *nconf, struct > for (s = TAILQ_FIRST(>ntp_sensors); s != NULL; > s = next_s) { > next_s = TAILQ_NEXT(s, entry); > - if (s->next <= getmonotime()) > + if (s->next <= now) { > + last_action = now; > sensor_query(s); > + } > + } > + > + /* > + * Compute maximum of scale_interval(INTERVAL_QUERY_NORMAL), > + * if we did not process a time message for three times that > + * interval, stop advertising we're synced. > + */ > + interval = INTERVAL_QUERY_NORMAL * conf->scale; > + interval += MAXIMUM(5, interval / 10) - 1; > + if (conf->status.synced && last_action + 3 * interval < now) { > + log_info("clock is now unsynced"); > + conf->status.synced = 0; > + conf->scale = 1; > + priv_dns(IMSG_UNSYNCED, NULL, 0); > } > } > >
Re: ntpd: go into unsynced mode
On Tue, Aug 25, 2020 at 07:05:31PM +0200, Matthias Schmidt wrote: > Hi Otto, > > * Otto Moerbeek wrote: > > Hi, > > > > At the moment ntpd never goes into unsynced mode if network > > connectivity is lost. The code to do that is only triggered when a > > pakcet is received, which does not happen. > > > > This diff fixes that by going into unsynced mode if no time data was > > processed for a while. > > > > An earlier version of this diff was tested by naddy@. Compared to that > > version, the needed period of inactivity is now three times as large > > and I set scale to 1, so recovery goes faster. > > > > Please test and review, > > I have your diff running on my Laptop which sometimes not connected to a > network so it should be a good test case. > > I haven't noticed any difference to before, so I count that as a good > sign :) I spotted only one thing: While "ntpctl -s a" says that the > clock is unsynced I see no message from ntpd in the logs. Not sure if > that's on purpose or not, I just noticed it. Thanks for testing. There should be "clock is now unsynced" and "clock is now synced" messages in /var/log/daemon... here they do appear. -Otto
Re: ntpd: go into unsynced mode
Hi Otto, * Otto Moerbeek wrote: > Hi, > > At the moment ntpd never goes into unsynced mode if network > connectivity is lost. The code to do that is only triggered when a > pakcet is received, which does not happen. > > This diff fixes that by going into unsynced mode if no time data was > processed for a while. > > An earlier version of this diff was tested by naddy@. Compared to that > version, the needed period of inactivity is now three times as large > and I set scale to 1, so recovery goes faster. > > Please test and review, I have your diff running on my Laptop which sometimes not connected to a network so it should be a good test case. I haven't noticed any difference to before, so I count that as a good sign :) I spotted only one thing: While "ntpctl -s a" says that the clock is unsynced I see no message from ntpd in the logs. Not sure if that's on purpose or not, I just noticed it. Cheers Matthias
ntpd: go into unsynced mode
Hi, At the moment ntpd never goes into unsynced mode if network connectivity is lost. The code to do that is only triggered when a pakcet is received, which does not happen. This diff fixes that by going into unsynced mode if no time data was processed for a while. An earlier version of this diff was tested by naddy@. Compared to that version, the needed period of inactivity is now three times as large and I set scale to 1, so recovery goes faster. Please test and review, -Otto Index: ntp.c === RCS file: /cvs/src/usr.sbin/ntpd/ntp.c,v retrieving revision 1.165 diff -u -p -r1.165 ntp.c --- ntp.c 22 Jun 2020 06:11:34 - 1.165 +++ ntp.c 22 Aug 2020 13:48:34 - @@ -89,6 +89,7 @@ ntp_main(struct ntpd_conf *nconf, struct struct stat stb; struct ctl_conn *cc; time_t nextaction, last_sensor_scan = 0, now; + time_t last_action = 0, interval; void*newp; if (socketpair(AF_UNIX, SOCK_STREAM | SOCK_CLOEXEC, PF_UNSPEC, @@ -402,6 +403,7 @@ ntp_main(struct ntpd_conf *nconf, struct for (; nfds > 0 && j < idx_clients; j++) { if (pfd[j].revents & (POLLIN|POLLERR)) { nfds--; + last_action = now; if (client_dispatch(idx2peer[j - idx_peers], conf->settime, conf->automatic) == -1) { log_warn("pipe write error (settime)"); @@ -417,8 +419,24 @@ ntp_main(struct ntpd_conf *nconf, struct for (s = TAILQ_FIRST(>ntp_sensors); s != NULL; s = next_s) { next_s = TAILQ_NEXT(s, entry); - if (s->next <= getmonotime()) + if (s->next <= now) { + last_action = now; sensor_query(s); + } + } + + /* +* Compute maximum of scale_interval(INTERVAL_QUERY_NORMAL), +* if we did not process a time message for three times that +* interval, stop advertising we're synced. +*/ + interval = INTERVAL_QUERY_NORMAL * conf->scale; + interval += MAXIMUM(5, interval / 10) - 1; + if (conf->status.synced && last_action + 3 * interval < now) { + log_info("clock is now unsynced"); + conf->status.synced = 0; + conf->scale = 1; + priv_dns(IMSG_UNSYNCED, NULL, 0); } }