Re: ntpd: go into unsynced mode

2020-08-30 Thread Otto Moerbeek
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

2020-08-25 Thread Otto Moerbeek
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

2020-08-25 Thread Matthias Schmidt
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

2020-08-22 Thread Otto Moerbeek
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);
}
}