ntp: add maxdelayquant option

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.

Makefile.in

@@ -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)
 

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;
 

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;

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);

cmdparse.c

@@ -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;

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
 

doc/chrony.conf.adoc

@@ -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,
+. Results of the *maxdelay*, *maxdelayratio*, and *maxdelaydevratio* (or
-  against defined parameters, and a test for synchronisation loop (1=pass,
+  *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]

doc/chronyc.adoc

@@ -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*:::

ntp_core.c

@@ -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) {

srcparams.h

@@ -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;

test/simulation/110-chronyc

@@ -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" \

test/simulation/118-maxdelay

@@ -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