On Fri, 29 Mar 2024, Andy Fiddaman wrote:
>
> On Thu, 28 Mar 2024, Miroslav Lichvar wrote:
>
> > On Mon, Mar 25, 2024 at 07:10:31PM +0000, Andy Fiddaman wrote:
> > > 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
> >
> > The distance option is already described as a threshold, so I think a
> > different name might work better. How about trigger, start or activate?
>
> I like 'activate', that's much clearer.
>
> > Let's think a bit about possible future options. We might need a
> > timeout-based activation, e.g. wait for 10 minutes before activating
> > local. It could be a second parameter of this option or a separate
> > option. What naming would be most consistent?
>
> If such things were required, I think it would probably be better
> to group them with 'activate'; as in having the timer as an extra
> parameter. It's hard to predict what might be needed in the future
> though. The same goes for the potential padding of the local
> control message.
>
> I've attached the latest patch.
>
> I'm happy to go back and do something here if you'd like, but also
> please feel free to adjust what I've done here to better fit the
> goals of the project.
The last patch was missing a couple of things, and did not preserve support
for older clients that might be using REQ_LOCAL2. I've updated the patch
to address those.
Andy
From 9083a01a09d2cbae80669feccb0e2a29e200e980 Mon Sep 17 00:00:00 2001
From: Andy Fiddaman <illu...@fiddaman.net>
Date: Mon, 25 Mar 2024 19:05:52 +0000
Subject: [PATCH] Add "local activate" option
---
candm.h | 4 +++-
client.c | 7 ++++---
cmdmon.c | 12 ++++++++----
cmdparse.c | 6 +++++-
cmdparse.h | 2 +-
conf.c | 6 ++++--
conf.h | 2 +-
doc/chrony.conf.adoc | 21 ++++++++++++++-------
pktlength.c | 3 ++-
reference.c | 21 +++++++++++++++------
reference.h | 2 +-
11 files changed, 58 insertions(+), 28 deletions(-)
diff --git a/candm.h b/candm.h
index 0894ad5..b4e41f1 100644
--- a/candm.h
+++ b/candm.h
@@ -111,7 +111,8 @@
#define REQ_DOFFSET2 71
#define REQ_MODIFY_SELECTOPTS 72
#define REQ_MODIFY_OFFSET 73
-#define N_REQUEST_TYPES 74
+#define REQ_LOCAL3 74
+#define N_REQUEST_TYPES 75
/* Structure used to exchange timespecs independent of time_t size */
typedef struct {
@@ -237,6 +238,7 @@ typedef struct {
int32_t stratum;
Float distance;
int32_t orphan;
+ Float activate;
int32_t EOR;
} REQ_Local;
diff --git a/client.c b/client.c
index 0231b9e..a1e213f 100644
--- a/client.c
+++ b/client.c
@@ -755,22 +755,23 @@ 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, activate = 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, &activate)) {
on_off = 1;
} else {
LOG(LOGS_ERR, "Invalid syntax for local command");
return 0;
}
- msg->command = htons(REQ_LOCAL2);
+ msg->command = htons(REQ_LOCAL3);
msg->data.local.on_off = htonl(on_off);
msg->data.local.stratum = htonl(stratum);
msg->data.local.distance = UTI_FloatHostToNetwork(distance);
msg->data.local.orphan = htonl(orphan);
+ msg->data.local.activate = UTI_FloatHostToNetwork(activate);
return 1;
}
diff --git a/cmdmon.c b/cmdmon.c
index 716775f..8d7d7e5 100644
--- a/cmdmon.c
+++ b/cmdmon.c
@@ -146,6 +146,7 @@ static const char permissions[] = {
PERMIT_AUTH, /* DOFFSET2 */
PERMIT_AUTH, /* MODIFY_SELECTOPTS */
PERMIT_AUTH, /* MODIFY_OFFSET */
+ PERMIT_AUTH, /* LOCAL3 */
};
/* ================================================== */
@@ -526,12 +527,14 @@ handle_settime(CMD_Request *rx_message, CMD_Reply
*tx_message)
/* ================================================== */
static void
-handle_local(CMD_Request *rx_message, CMD_Reply *tx_message)
+handle_local(CMD_Request *rx_message, CMD_Reply *tx_message, uint16_t cmd)
{
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),
+ cmd == REQ_LOCAL2 ? 0 :
+ UTI_FloatNetworkToHost(rx_message->data.local.activate));
} else {
REF_DisableLocal();
}
@@ -1648,9 +1651,10 @@ read_from_cmd_socket(int sock_fd, int event, void
*anything)
case REQ_SETTIME:
handle_settime(&rx_message, &tx_message);
break;
-
+
case REQ_LOCAL2:
- handle_local(&rx_message, &tx_message);
+ case REQ_LOCAL3:
+ handle_local(&rx_message, &tx_message, rx_command);
break;
case REQ_MANUAL:
diff --git a/cmdparse.c b/cmdparse.c
index 0a80fc0..77447dc 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
*activate)
{
int n;
char *cmd;
*stratum = 10;
*distance = 1.0;
+ *activate = 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, "activate")) {
+ if (sscanf(line, "%lf%n", activate, &n) != 1)
+ return 0;
} else {
return 0;
}
diff --git a/cmdparse.h b/cmdparse.h
index 095a8e2..f0bc7a4 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 *activate);
/* Remove extra white-space and comments */
extern void CPS_NormalizeLine(char *line);
diff --git a/conf.c b/conf.c
index 8849bdc..d73c6dd 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_activate;
/* 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_activate))
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
*activate)
{
if (enable_local) {
*stratum = local_stratum;
*orphan = local_orphan;
*distance = local_distance;
+ *activate = local_activate;
return 1;
} else {
return 0;
diff --git a/conf.h b/conf.h
index 4c0a787..2128a3e 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 *activate);
extern void CNF_SetupAccessRestrictions(void);
diff --git a/doc/chrony.conf.adoc b/doc/chrony.conf.adoc
index bd296bc..2c29ae7 100644
--- a/doc/chrony.conf.adoc
+++ b/doc/chrony.conf.adoc
@@ -1663,11 +1663,11 @@ be very close to real time. Stratum 2 computers are
those which have a stratum
of 10 indicates that the clock is so many hops away from a reference clock that
its time is fairly unreliable.
*distance* _distance_:::
-This option sets the threshold for the root distance which will activate the
local
-reference. If *chronyd* was synchronised to some source, the local reference
-will not be activated until its root distance reaches the specified value (the
-rate at which the distance is increasing depends on how well the clock was
-tracking the source). The default value is 1 second.
+This option sets the threshold for the root distance which will activate the
+local reference. If *chronyd* was synchronised to some source, the local
+reference will not be activated until its root distance reaches the specified
+value (the rate at which the distance is increasing depends on how well the
+clock was tracking the source). The default value is 1 second.
+
The current root distance can be calculated from root delay and root dispersion
(reported by the <<chronyc.adoc#tracking,*tracking*>> command in *chronyc*) as:
@@ -1675,6 +1675,14 @@ The current root distance can be calculated from root
delay and root dispersion
----
distance = delay / 2 + dispersion
----
+*activate* _distance_:::
+This option sets an activating root distance for the local reference. The
+local reference will not be used until the root distance drops below the
+configured value for the first time. This can be used to prevent the local
+reference from being activated on a server which has never been synchronised
+with an upstream server. The default value of 0.0 causes no activating
+distance to be used, such that the local reference is always eligible for
+activation.
*orphan*:::
This option enables a special '`orphan`' mode, where sources with stratum equal
to the local _stratum_ are assumed to not serve real time. They are ignored
@@ -1692,12 +1700,11 @@ smallest reference ID will take over when its local
reference mode activates
+
The *orphan* mode is compatible with the *ntpd*'s orphan mode (enabled by the
*tos orphan* command).
-{blank}::
+
An example of the directive is:
+
----
-local stratum 10 orphan distance 0.1
+local stratum 10 orphan distance 0.1 activate 0.5
----
[[ntpsigndsocket]]*ntpsigndsocket* _directory_::
diff --git a/pktlength.c b/pktlength.c
index 3cca306..8961f4a 100644
--- a/pktlength.c
+++ b/pktlength.c
@@ -111,7 +111,7 @@ static const struct request_length request_lengths[] = {
REQ_LENGTH_ENTRY(null, null), /* REFRESH */
REQ_LENGTH_ENTRY(null, server_stats), /* SERVER_STATS */
{ 0, 0 }, /* CLIENT_ACCESSES_BY_INDEX2 -
not supported */
- REQ_LENGTH_ENTRY(local, null), /* LOCAL2 */
+ { offsetof(CMD_Request, data.local.activate), 0 }, /* LOCAL2 */
REQ_LENGTH_ENTRY(ntp_data, ntp_data), /* NTP_DATA */
{ 0, 0 }, /* ADD_SERVER2 */
{ 0, 0 }, /* ADD_PEER2 */
@@ -131,6 +131,7 @@ static const struct request_length request_lengths[] = {
REQ_LENGTH_ENTRY(doffset, null), /* DOFFSET2 */
REQ_LENGTH_ENTRY(modify_select_opts, null), /* MODIFY_SELECTOPTS */
REQ_LENGTH_ENTRY(modify_offset, null), /* MODIFY_OFFSET */
+ REQ_LENGTH_ENTRY(local, null), /* LOCAL3 */
};
static const uint16_t reply_lengths[] = {
diff --git a/reference.c b/reference.c
index 1ac6cb9..39ef95e 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_activate_ok;
+static double local_activate;
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_activate_ok = 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_activate);
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,15 @@ REF_GetReferenceParams
dispersion = 0.0;
}
+ distance = our_root_delay / 2 + dispersion;
+
+ if (local_activate == 0.0 || (are_we_synchronised && distance <
local_activate))
+ local_activate_ok = 1;
+
/* 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 +1166,7 @@ REF_GetReferenceParams
*root_delay = our_root_delay;
*root_dispersion = dispersion;
- } else if (enable_local_stratum) {
+ } else if (enable_local_stratum && local_activate_ok) {
*is_synchronised = 0;
@@ -1258,12 +1266,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 activate)
{
enable_local_stratum = 1;
local_stratum = CLAMP(1, stratum, NTP_MAX_STRATUM - 1);
local_distance = distance;
local_orphan = !!orphan;
+ local_activate = activate;
LOG(LOGS_INFO, "%s local reference mode", "Enabled");
}
diff --git a/reference.h b/reference.h
index 73454d4..2eddcae 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
activate);
extern void REF_DisableLocal(void);
/* Check if either of the current raw and cooked time, and optionally a
--
2.42.0
