]> git.ipfire.org Git - thirdparty/chrony.git/commitdiff
ntp: add maxdelayquant option
authorMiroslav Lichvar <mlichvar@redhat.com>
Thu, 21 Jul 2022 13:16:47 +0000 (15:16 +0200)
committerMiroslav Lichvar <mlichvar@redhat.com>
Thu, 21 Jul 2022 14:05:48 +0000 (16:05 +0200)
Add a new test for maximum delay using a long-term estimate of a
p-quantile of the peer delay. If enabled, it replaces the
maxdelaydevratio test. It's main advantage is that it is not sensitive
to outliers corrupting the minimum delay.

As it can take a large number of samples for the estimate to reach the
expected value and adapt to a new value after a network change, the
option is recommended only for local networks with very short polling
intervals.

12 files changed:
Makefile.in
candm.h
client.c
cmdmon.c
cmdparse.c
configure
doc/chrony.conf.adoc
doc/chronyc.adoc
ntp_core.c
srcparams.h
test/simulation/110-chronyc
test/simulation/118-maxdelay

index 9a51df20074a4782da0a68d70ff457b48d855513..ef100a4b5c9b427422a7b5065dd5a00247eba775 100644 (file)
@@ -35,7 +35,7 @@ LDFLAGS = @LDFLAGS@
 
 EXTRA_OBJS = @EXTRA_OBJS@
 
-OBJS = array.o cmdparse.o conf.o local.o logging.o main.o memory.o \
+OBJS = array.o cmdparse.o conf.o local.o logging.o main.o memory.o quantiles.o \
        reference.o regress.o rtc.o samplefilt.o sched.o socket.o sources.o sourcestats.o \
        stubs.o smooth.o sys.o sys_null.o tempcomp.o util.o $(EXTRA_OBJS)
 
diff --git a/candm.h b/candm.h
index dd9941361861665a4179094a3d3482cb58b4aa35..a0e14b41067f476694201e36e25c82104a343623 100644 (file)
--- a/candm.h
+++ b/candm.h
@@ -296,7 +296,8 @@ typedef struct {
   uint32_t flags;
   int32_t filter_length;
   uint32_t cert_set;
-  uint32_t reserved[2];
+  Float max_delay_quant;
+  uint32_t reserved[1];
   int32_t EOR;
 } REQ_NTP_Source;
 
index 6f78cb1fb0a339464651d6f9a7f3e845d0653caa..60819c295eb2dd7c2fe0dbabe830b43d92020b7d 100644 (file)
--- a/client.c
+++ b/client.c
@@ -952,6 +952,8 @@ process_cmd_add_source(CMD_Request *msg, char *line)
           (data.params.sel_options & SRC_SELECT_REQUIRE ? REQ_ADDSRC_REQUIRE : 0));
       msg->data.ntp_source.filter_length = htonl(data.params.filter_length);
       msg->data.ntp_source.cert_set = htonl(data.params.cert_set);
+      msg->data.ntp_source.max_delay_quant =
+        UTI_FloatHostToNetwork(data.params.max_delay_quant);
       memset(msg->data.ntp_source.reserved, 0, sizeof (msg->data.ntp_source.reserved));
 
       result = 1;
index 89f5b95d68a1c0468a006a45d5137d2d3d9d8232..ec0c71f5240072413d3d831a950b8b6afb44154c 100644 (file)
--- a/cmdmon.c
+++ b/cmdmon.c
@@ -757,6 +757,8 @@ handle_add_source(CMD_Request *rx_message, CMD_Reply *tx_message)
     UTI_FloatNetworkToHost(rx_message->data.ntp_source.max_delay_ratio);
   params.max_delay_dev_ratio =
     UTI_FloatNetworkToHost(rx_message->data.ntp_source.max_delay_dev_ratio);
+  params.max_delay_quant =
+    UTI_FloatNetworkToHost(rx_message->data.ntp_source.max_delay_quant);
   params.min_delay = UTI_FloatNetworkToHost(rx_message->data.ntp_source.min_delay);
   params.asymmetry = UTI_FloatNetworkToHost(rx_message->data.ntp_source.asymmetry);
   params.offset = UTI_FloatNetworkToHost(rx_message->data.ntp_source.offset);
index d8ab61ec12969351d898018fc0bc825b387ec7a2..1a9e210b3d129ed2a8b1e396ff168eb342e76b9d 100644 (file)
@@ -72,6 +72,7 @@ CPS_ParseNTPSourceAdd(char *line, CPS_NTP_Source *src)
   src->params.max_delay = SRC_DEFAULT_MAXDELAY;
   src->params.max_delay_ratio = SRC_DEFAULT_MAXDELAYRATIO;
   src->params.max_delay_dev_ratio = SRC_DEFAULT_MAXDELAYDEVRATIO;
+  src->params.max_delay_quant = 0.0;
   src->params.min_delay = 0.0;
   src->params.asymmetry = SRC_DEFAULT_ASYMMETRY;
   src->params.offset = 0.0;
@@ -140,6 +141,9 @@ CPS_ParseNTPSourceAdd(char *line, CPS_NTP_Source *src)
     } else if (!strcasecmp(cmd, "maxdelaydevratio")) {
       if (sscanf(line, "%lf%n", &src->params.max_delay_dev_ratio, &n) != 1)
         return 0;
+    } else if (!strcasecmp(cmd, "maxdelayquant")) {
+      if (sscanf(line, "%lf%n", &src->params.max_delay_quant, &n) != 1)
+        return 0;
     } else if (!strcasecmp(cmd, "maxpoll")) {
       if (sscanf(line, "%d%n", &src->params.maxpoll, &n) != 1)
         return 0;
index 70a0a32ae74cf1965e513928b38ebbd4afc055a9..73d186735676c42e55ebdb7a837b1fc6880182bb 100755 (executable)
--- a/configure
+++ b/configure
@@ -737,7 +737,7 @@ if [ $feat_timestamping = "1" ] && [ $try_timestamping = "1" ] &&
                       &val, sizeof (val));'
 then
   add_def HAVE_LINUX_TIMESTAMPING
-  EXTRA_OBJECTS="$EXTRA_OBJECTS hwclock.o ntp_io_linux.o quantiles.o"
+  EXTRA_OBJECTS="$EXTRA_OBJECTS hwclock.o ntp_io_linux.o"
 
   if test_code 'other timestamping options' \
     'sys/types.h sys/socket.h linux/net_tstamp.h' '' '' '
@@ -830,7 +830,7 @@ if [ $feat_refclock = "1" ] && [ $feat_phc = "1" ] && [ $try_phc = "1" ] && \
     'ioctl(1, PTP_CLOCK_GETCAPS + PTP_SYS_OFFSET, 0);'
 then
   grep 'HAVE_LINUX_TIMESTAMPING' config.h > /dev/null ||
-    EXTRA_OBJECTS="$EXTRA_OBJECTS hwclock.o quantiles.o"
+    EXTRA_OBJECTS="$EXTRA_OBJECTS hwclock.o"
   add_def FEAT_PHC
 fi
 
index 7b43fffc1de72df9651e5c10ea3471a372a60a2d..a6e15207553e5a67c20e4ca13211fac4ca9fc09a 100644 (file)
@@ -151,6 +151,20 @@ If a measurement has a ratio of the increase in the round-trip delay from the
 minimum delay amongst the previous measurements to the standard deviation of
 the previous measurements that is greater than the specified ratio, it will be
 rejected. The default is 10.0.
+*maxdelayquant* _p_:::
+This option disables the *maxdelaydevratio* test and specifies the maximum
+acceptable delay as a quantile of the round-trip delay instead of a function of
+the minimum delay amongst the buffered measurements. If a measurement has a
+round-trip delay that is greater than a long-term estimate of the _p_-quantile,
+it will be rejected.
++
+The specified _p_ value should be between 0.05 and 0.95. For example,
+*maxdelayquant 0.2* would indicate that only measurements with the lowest 20
+percent of round-trip delays should be accepted. Note that it can take many
+measurements for the estimated quantile to reach the expected value. This
+option is intended for synchronisation in mostly static local networks with
+very short polling intervals and possibly combined with the *filter* option.
+By default, this test is disabled in favour of the *maxdelaydevratio* test.
 *mindelay* _delay_:::
 This option specifies a fixed minimum round-trip delay to be used instead of
 the minimum amongst the previous measurements. This can be useful in networks
@@ -2070,8 +2084,8 @@ from the example line above):
 . Stratum of remote computer. [2]
 . RFC 5905 tests 1 through 3 (1=pass, 0=fail) [111]
 . RFC 5905 tests 5 through 7 (1=pass, 0=fail) [111]
-. Tests for maximum delay, maximum delay ratio and maximum delay dev ratio,
-  against defined parameters, and a test for synchronisation loop (1=pass,
+. Results of the *maxdelay*, *maxdelayratio*, and *maxdelaydevratio* (or
+  *maxdelayquant*) tests, and a test for synchronisation loop (1=pass,
   0=fail). The first test from these four also checks the server precision,
   response time, and whether an interleaved response is acceptable for
   synchronisation. [1111]
index 2ec5445da53b55c55ac3d1a638b1bfb970c62977..fd393a4674f8d43c8af7d80fc2baaff73e0d273e 100644 (file)
@@ -699,7 +699,8 @@ packets sent to the source is more variable than the delay of packets sent
 from the source back.
 *NTP tests*:::
 Results of RFC 5905 tests 1 through 3, 5 through 7, and tests for maximum
-delay, delay ratio, delay dev ratio, and synchronisation loop.
+delay, delay ratio, delay dev ratio (or delay quantile), and synchronisation
+loop.
 *Interleaved*:::
 This shows if the response was in the interleaved mode.
 *Authenticated*:::
index 9aa7d7e259758dd8b1436d684527825ea33bb359..141fe4f0b7a7be41feb38de32604e12178e9d3e1 100644 (file)
@@ -35,6 +35,7 @@
 #include "ntp_ext.h"
 #include "ntp_io.h"
 #include "memory.h"
+#include "quantiles.h"
 #include "sched.h"
 #include "reference.h"
 #include "local.h"
@@ -196,6 +197,9 @@ struct NCR_Instance_Record {
 
   SRC_Instance source;
 
+  /* Optional long-term quantile estimate of peer delay */
+  QNT_Instance delay_quant;
+
   /* Optional median filter for NTP measurements */
   SPF_Instance filter;
   int filter_count;
@@ -266,6 +270,10 @@ static ARR_Instance broadcasts;
 #define MAX_MAXDELAYRATIO 1.0e6
 #define MAX_MAXDELAYDEVRATIO 1.0e6
 
+/* Parameters for the peer delay quantile */
+#define DELAY_QUANT_Q 100
+#define DELAY_QUANT_REPEAT 7
+
 /* Minimum and maximum allowed poll interval */
 #define MIN_POLL -7
 #define MAX_POLL 24
@@ -642,6 +650,14 @@ NCR_CreateInstance(NTP_Remote_Address *remote_addr, NTP_Source_Type type,
                                          params->min_samples, params->max_samples,
                                          params->min_delay, params->asymmetry);
 
+  if (params->max_delay_quant > 0.0) {
+    int k = round(CLAMP(0.05, params->max_delay_quant, 0.95) * DELAY_QUANT_Q);
+    result->delay_quant = QNT_CreateInstance(k, k, DELAY_QUANT_Q, DELAY_QUANT_REPEAT,
+                                             LCL_GetSysPrecisionAsQuantum() / 2.0);
+  } else {
+    result->delay_quant = NULL;
+  }
+
   if (params->filter_length >= 1)
     result->filter = SPF_CreateInstance(1, params->filter_length, NTP_MAX_DISPERSION, 0.0);
   else
@@ -677,6 +693,8 @@ NCR_DestroyInstance(NCR_Instance instance)
   if (instance->mode == MODE_ACTIVE)
     NIO_CloseServerSocket(instance->local_addr.sock_fd);
 
+  if (instance->delay_quant)
+    QNT_DestroyInstance(instance->delay_quant);
   if (instance->filter)
     SPF_DestroyInstance(instance->filter);
 
@@ -733,6 +751,8 @@ NCR_ResetInstance(NCR_Instance instance)
   UTI_ZeroNtp64(&instance->init_remote_ntp_tx);
   zero_local_timestamp(&instance->init_local_rx);
 
+  if (instance->delay_quant)
+    QNT_Reset(instance->delay_quant);
   if (instance->filter)
     SPF_DropSamples(instance->filter);
   instance->filter_count = 0;
@@ -1550,6 +1570,22 @@ check_delay_ratio(NCR_Instance inst, SST_Stats stats,
 
 /* ================================================== */
 
+static int
+check_delay_quant(NCR_Instance inst, double delay)
+{
+  double quant;
+
+  quant = QNT_GetQuantile(inst->delay_quant, QNT_GetMinK(inst->delay_quant));
+
+  if (delay <= quant)
+    return 1;
+
+  DEBUG_LOG("maxdelayquant: delay=%e quant=%e", delay, quant);
+  return 0;
+}
+
+/* ================================================== */
+
 static int
 check_delay_dev_ratio(NCR_Instance inst, SST_Stats stats,
                       struct timespec *sample_time, double offset, double delay)
@@ -1935,12 +1971,18 @@ process_response(NCR_Instance inst, NTP_Local_Address *local_addr,
        administrator-defined value */
     testB = check_delay_ratio(inst, stats, &sample.time, sample.peer_delay);
 
-    /* Test C requires that the ratio of the increase in delay from the minimum
+    /* Test C either requires that the delay is less than an estimate of an
+       administrator-defined quantile, or (if the quantile is not specified)
+       it requires that the ratio of the increase in delay from the minimum
        one in the stats data register to the standard deviation of the offsets
        in the register is less than an administrator-defined value or the
        difference between measured offset and predicted offset is larger than
        the increase in delay */
-    testC = check_delay_dev_ratio(inst, stats, &sample.time, sample.offset, sample.peer_delay);
+    if (inst->delay_quant)
+      testC = check_delay_quant(inst, sample.peer_delay);
+    else
+      testC = check_delay_dev_ratio(inst, stats, &sample.time, sample.offset,
+                                    sample.peer_delay);
 
     /* Test D requires that the source is not synchronised to us and is not us
        to prevent a synchronisation loop */
@@ -2073,6 +2115,9 @@ process_response(NCR_Instance inst, NTP_Local_Address *local_addr,
       }
 
       SRC_UpdateStatus(inst->source, MAX(inst->remote_stratum, inst->min_stratum), pkt_leap);
+
+      if (inst->delay_quant)
+        QNT_Accumulate(inst->delay_quant, sample.peer_delay);
     }
 
     if (good_packet) {
index f81114602b37a7978e1173c7152c93f067343ac4..31baed77d6cf2dc98ab3ebcb1c5ad845f1ccce16 100644 (file)
@@ -61,6 +61,7 @@ typedef struct {
   double max_delay;
   double max_delay_ratio;
   double max_delay_dev_ratio;
+  double max_delay_quant;
   double min_delay;
   double asymmetry;
   double offset;
index 6693d4de089df3bc7bd7902d5aa92b02d70ec724..9ddf95e38d13c14b7918c1195cb136104c45b40c 100755 (executable)
@@ -102,7 +102,7 @@ limit=1
 for chronyc_conf in \
        "accheck 1.2.3.4" \
        "add peer 10.0.0.0 minpoll 2 maxpoll 6" \
-       "add server 10.0.0.0 minpoll 6 maxpoll 10 iburst burst key 1 certset 2 maxdelay 1e-3 maxdelayratio 10.0 maxdelaydevratio 10.0 mindelay 1e-4 asymmetry 0.5 offset 1e-5 minsamples 6 maxsamples 6 filter 3 offline auto_offline prefer noselect trust require xleave polltarget 20 port 123 presend 7 minstratum 3 version 4 nts ntsport 4460 copy extfield F323" \
+       "add server 10.0.0.0 minpoll 6 maxpoll 10 iburst burst key 1 certset 2 maxdelay 1e-3 maxdelayratio 10.0 maxdelaydevratio 10.0 maxdelayquant 0.5 mindelay 1e-4 asymmetry 0.5 offset 1e-5 minsamples 6 maxsamples 6 filter 3 offline auto_offline prefer noselect trust require xleave polltarget 20 port 123 presend 7 minstratum 3 version 4 nts ntsport 4460 copy extfield F323" \
        "add server node1.net1.clk" \
        "allow 1.2.3.4" \
        "allow 1.2" \
index bb8b47c52ee4f20fed43732d1aa397bbc7ad59bc..117b170b874b80159cd2aa17f8bfd9bfc24d7275 100755 (executable)
@@ -25,4 +25,18 @@ for client_server_options in "maxpoll 6 maxdelay 2e-5"; do
        check_sync && test_fail
 done
 
+min_sync_time=10
+client_conf="
+logdir tmp
+log rawmeasurements"
+client_server_options="minpoll 2 maxpoll 2 maxdelayquant 0.1"
+
+run_test || test_fail
+check_chronyd_exit || test_fail
+check_packet_interval || test_fail
+check_sync || test_fail
+
+check_file_messages "20.*123\.1.* 111 111 1111" 200 500 measurements.log || test_fail
+check_file_messages "20.*123\.1.* 111 111 1101" 2000 2300 measurements.log || test_fail
+
 test_pass