Re: [chrony-dev] [PATCH] local threshold option

[Andy Fiddaman <illumos@xxxxxxxxxxxx>]

On Thu, 21 Mar 2024, Miroslav Lichvar wrote:

> > The first, "local oncesynced" prevents the local reference being activated
> > unless time was synchronised at some point in the past.
>
> This makes sense to me, although I think it might be better to
> generalize it as an activating root distance, which would completement
> the distance option (threshold).

How does the attached patch look? This adds `local threshold` as you
suggested, that defaults to 0 which means it is not used. If it's set,
then the local reference is not activated until the root distance drops
below the value once.

In our configuration, we can then use something like:

    local stratum 10 distance 1.0 threshold 1.0

Thanks,

Andy

From d094cb53ab1632fc741cb60cf92520e9d3c84880 Mon Sep 17 00:00:00 2001
From: Andy Fiddaman <illumos@xxxxxxxxxxxx>
Date: Mon, 25 Mar 2024 19:05:52 +0000
Subject: [PATCH] Add "local threshold" option

---
 candm.h              |  1 +
 client.c             |  5 +++--
 cmdmon.c             |  3 ++-
 cmdparse.c           |  6 +++++-
 cmdparse.h           |  2 +-
 conf.c               |  6 ++++--
 conf.h               |  2 +-
 doc/chrony.conf.adoc |  5 +++++
 reference.c          | 22 ++++++++++++++++------
 reference.h          |  2 +-
 10 files changed, 39 insertions(+), 15 deletions(-)

diff --git a/candm.h b/candm.h
index 0894ad5..64e3200 100644
--- a/candm.h
+++ b/candm.h
@@ -237,6 +237,7 @@ typedef struct {
   int32_t stratum;
   Float distance;
   int32_t orphan;
+  Float threshold;
   int32_t EOR;
 } REQ_Local;
 
diff --git a/client.c b/client.c
index 0231b9e..0674263 100644
--- a/client.c
+++ b/client.c
@@ -755,11 +755,11 @@ static int
 process_cmd_local(CMD_Request *msg, char *line)
 {
   int on_off, stratum = 0, orphan = 0;
-  double distance = 0.0;
+  double distance = 0.0, threshold = 0.0;
 
   if (!strcmp(line, "off")) {
     on_off = 0;
-  } else if (CPS_ParseLocal(line, &stratum, &orphan, &distance)) {
+  } else if (CPS_ParseLocal(line, &stratum, &orphan, &distance, &threshold)) {
     on_off = 1;
   } else {
     LOG(LOGS_ERR, "Invalid syntax for local command");
@@ -771,6 +771,7 @@ process_cmd_local(CMD_Request *msg, char *line)
   msg->data.local.stratum = htonl(stratum);
   msg->data.local.distance = UTI_FloatHostToNetwork(distance);
   msg->data.local.orphan = htonl(orphan);
+  msg->data.local.threshold = UTI_FloatHostToNetwork(threshold);
 
   return 1;
 }
diff --git a/cmdmon.c b/cmdmon.c
index 716775f..464aeb5 100644
--- a/cmdmon.c
+++ b/cmdmon.c
@@ -531,7 +531,8 @@ handle_local(CMD_Request *rx_message, CMD_Reply *tx_message)
   if (ntohl(rx_message->data.local.on_off)) {
     REF_EnableLocal(ntohl(rx_message->data.local.stratum),
                     UTI_FloatNetworkToHost(rx_message->data.local.distance),
-                    ntohl(rx_message->data.local.orphan));
+                    ntohl(rx_message->data.local.orphan),
+                    UTI_FloatNetworkToHost(rx_message->data.local.threshold));
   } else {
     REF_DisableLocal();
   }
diff --git a/cmdparse.c b/cmdparse.c
index 0a80fc0..d811948 100644
--- a/cmdparse.c
+++ b/cmdparse.c
@@ -296,13 +296,14 @@ CPS_ParseAllowDeny(char *line, int *all, IPAddr *ip, int *subnet_bits)
 /* ================================================== */
 
 int
-CPS_ParseLocal(char *line, int *stratum, int *orphan, double *distance)
+CPS_ParseLocal(char *line, int *stratum, int *orphan, double *distance, double *threshold)
 {
   int n;
   char *cmd;
 
   *stratum = 10;
   *distance = 1.0;
+  *threshold = 0.0;
   *orphan = 0;
 
   while (*line) {
@@ -319,6 +320,9 @@ CPS_ParseLocal(char *line, int *stratum, int *orphan, double *distance)
     } else if (!strcasecmp(cmd, "distance")) {
       if (sscanf(line, "%lf%n", distance, &n) != 1)
         return 0;
+    } else if (!strcasecmp(cmd, "threshold")) {
+      if (sscanf(line, "%lf%n", threshold, &n) != 1)
+        return 0;
     } else {
       return 0;
     }
diff --git a/cmdparse.h b/cmdparse.h
index 095a8e2..3d6bbe0 100644
--- a/cmdparse.h
+++ b/cmdparse.h
@@ -47,7 +47,7 @@ extern int CPS_GetSelectOption(char *option);
 extern int CPS_ParseAllowDeny(char *line, int *all, IPAddr *ip, int *subnet_bits);
 
 /* Parse a command to enable local reference */
-extern int CPS_ParseLocal(char *line, int *stratum, int *orphan, double *distance);
+extern int CPS_ParseLocal(char *line, int *stratum, int *orphan, double *distance, double *threshold);
 
 /* Remove extra white-space and comments */
 extern void CPS_NormalizeLine(char *line);
diff --git a/conf.c b/conf.c
index 8849bdc..7af484b 100644
--- a/conf.c
+++ b/conf.c
@@ -129,6 +129,7 @@ static int enable_local=0;
 static int local_stratum;
 static int local_orphan;
 static double local_distance;
+static double local_threshold;
 
 /* Threshold (in seconds) - if absolute value of initial error is less
    than this, slew instead of stepping */
@@ -1066,7 +1067,7 @@ parse_log(char *line)
 static void
 parse_local(char *line)
 {
-  if (!CPS_ParseLocal(line, &local_stratum, &local_orphan, &local_distance))
+  if (!CPS_ParseLocal(line, &local_stratum, &local_orphan, &local_distance, &local_threshold))
     command_parse_error();
   enable_local = 1;
 }
@@ -2166,12 +2167,13 @@ CNF_GetCommandPort(void) {
 /* ================================================== */
 
 int
-CNF_AllowLocalReference(int *stratum, int *orphan, double *distance)
+CNF_AllowLocalReference(int *stratum, int *orphan, double *distance, double *threshold)
 {
   if (enable_local) {
     *stratum = local_stratum;
     *orphan = local_orphan;
     *distance = local_distance;
+    *threshold = local_threshold;
     return 1;
   } else {
     return 0;
diff --git a/conf.h b/conf.h
index 4c0a787..f083e62 100644
--- a/conf.h
+++ b/conf.h
@@ -108,7 +108,7 @@ extern double CNF_GetReselectDistance(void);
 extern double CNF_GetStratumWeight(void);
 extern double CNF_GetCombineLimit(void);
 
-extern int CNF_AllowLocalReference(int *stratum, int *orphan, double *distance);
+extern int CNF_AllowLocalReference(int *stratum, int *orphan, double *distance, double *threshold);
 
 extern void CNF_SetupAccessRestrictions(void);
 
diff --git a/doc/chrony.conf.adoc b/doc/chrony.conf.adoc
index bd296bc..f5a4210 100644
--- a/doc/chrony.conf.adoc
+++ b/doc/chrony.conf.adoc
@@ -1699,6 +1699,11 @@ An example of the directive is:
 ----
 local stratum 10 orphan distance 0.1
 ----
+*threshold* _distance_:::
+This option sets an activating threshold for the local reference. The local
+reference will not be used until the root distance drops below the configured
+threshold once. This can be used to prevent the local reference being activated
+on a server which has never been synchronised with an upstream server.
 
 [[ntpsigndsocket]]*ntpsigndsocket* _directory_::
 This directive specifies the location of the Samba *ntp_signd* socket when it
diff --git a/reference.c b/reference.c
index 1ac6cb9..574d162 100644
--- a/reference.c
+++ b/reference.c
@@ -54,6 +54,8 @@ static int enable_local_stratum;
 static int local_stratum;
 static int local_orphan;
 static double local_distance;
+static int local_threshold_met;
+static double local_threshold;
 static struct timespec local_ref_time;
 static NTP_Leap our_leap_status;
 static int our_leap_sec;
@@ -207,6 +209,7 @@ REF_Initialise(void)
   our_frequency_sd = 0.0;
   our_offset_sd = 0.0;
   drift_file_age = 0.0;
+  local_threshold_met = 0;
 
   /* Now see if we can get the drift file opened */
   drift_file = CNF_GetDriftFile();
@@ -245,7 +248,8 @@ REF_Initialise(void)
 
   correction_time_ratio = CNF_GetCorrectionTimeRatio();
 
-  enable_local_stratum = CNF_AllowLocalReference(&local_stratum, &local_orphan, &local_distance);
+  enable_local_stratum = CNF_AllowLocalReference(&local_stratum, &local_orphan,
+                                                 &local_distance, &local_threshold);
   UTI_ZeroTimespec(&local_ref_time);
 
   leap_when = 0;
@@ -1132,7 +1136,7 @@ REF_GetReferenceParams
  double *root_dispersion
 )
 {
-  double dispersion, delta;
+  double dispersion, delta, distance;
 
   assert(initialised);
 
@@ -1142,11 +1146,12 @@ REF_GetReferenceParams
     dispersion = 0.0;
   }
 
+  distance = our_root_delay / 2 + dispersion;
+
   /* Local reference is active when enabled and the clock is not synchronised
      or the root distance exceeds the threshold */
-
   if (are_we_synchronised &&
-      !(enable_local_stratum && our_root_delay / 2 + dispersion > local_distance)) {
+      !(enable_local_stratum && distance > local_distance)) {
 
     *is_synchronised = 1;
 
@@ -1158,7 +1163,11 @@ REF_GetReferenceParams
     *root_delay = our_root_delay;
     *root_dispersion = dispersion;
 
-  } else if (enable_local_stratum) {
+    if (distance > 0 && distance < local_threshold)
+      local_threshold_met = 1;
+
+  } else if (enable_local_stratum &&
+             (local_threshold == 0 || local_threshold_met)) {
 
     *is_synchronised = 0;
 
@@ -1258,12 +1267,13 @@ REF_ModifyMakestep(int limit, double threshold)
 /* ================================================== */
 
 void
-REF_EnableLocal(int stratum, double distance, int orphan)
+REF_EnableLocal(int stratum, double distance, int orphan, double threshold)
 {
   enable_local_stratum = 1;
   local_stratum = CLAMP(1, stratum, NTP_MAX_STRATUM - 1);
   local_distance = distance;
   local_orphan = !!orphan;
+  local_threshold = threshold;
   LOG(LOGS_INFO, "%s local reference mode", "Enabled");
 }
 
diff --git a/reference.h b/reference.h
index 73454d4..4c0a7ca 100644
--- a/reference.h
+++ b/reference.h
@@ -185,7 +185,7 @@ extern void REF_ModifyMaxupdateskew(double new_max_update_skew);
 /* Modify makestep settings */
 extern void REF_ModifyMakestep(int limit, double threshold);
 
-extern void REF_EnableLocal(int stratum, double distance, int orphan);
+extern void REF_EnableLocal(int stratum, double distance, int orphan, double threshold);
 extern void REF_DisableLocal(void);
 
 /* Check if either of the current raw and cooked time, and optionally a
-- 
2.42.0