ntp: rework filter option to count missing samples

Instead of waiting for the sample filter to accumulate the specified
number of samples and then deciding if the result is acceptable, count
missing samples and get the result after the specified number of polls.

This should work better when samples are dropped at a high rate. The
source and clock update interval will be stable as long as at least
one sample can be collected.

diff --git a/doc/chrony.conf.adoc b/doc/chrony.conf.adoc
index 5eb6cee72c715c12ad12b2961dfaf79922613139..e60e98389cc7c8562d178ce89cc3caefd5c5bf5a 100644 (file)
--- a/doc/chrony.conf.adoc
+++ b/doc/chrony.conf.adoc
@@ -176,11 +176,12 @@ Set the minimum number of samples kept for this source. This overrides the
 *maxsamples* _samples_:::
 Set the maximum number of samples kept for this source. This overrides the
 <<maxsamples,*maxsamples*>> directive.
-*filter* _samples_:::
+*filter* _polls_:::
 This option enables a median filter to reduce noise in NTP measurements. The
-filter will reduce the specified number of samples to a single sample. It is
-intended to be used with very short polling intervals in local networks where
-it is acceptable to generate a lot of NTP traffic.
+filter will process samples collected in the specified number of polls
+into a single sample. It is intended to be used with very short polling
+intervals in local networks where it is acceptable to generate a lot of NTP
+traffic.
 *offline*:::
 If the server will not be reachable when *chronyd* is started, the *offline*
 option can be specified. *chronyd* will not try to poll the server until it is

diff --git a/ntp_core.c b/ntp_core.c
index 6e4745b75cf20d4ec46c28196ed6b29288e4632d..95d7ba237b857c1fb38dc59bfdf52b62e17d2728 100644 (file)
--- a/ntp_core.c
+++ b/ntp_core.c
@@ -198,6 +198,7 @@ struct NCR_Instance_Record {
 
   /* Optional median filter for NTP measurements */
   SPF_Instance filter;
+  int filter_count;
 
   int burst_good_samples_to_go;
   int burst_total_samples_to_go;
@@ -318,6 +319,7 @@ static void transmit_timeout(void *arg);
 static double get_transmit_delay(NCR_Instance inst, int on_tx, double last_tx);
 static double get_separation(int poll);
 static int parse_packet(NTP_Packet *packet, int length, NTP_PacketInfo *info);
+static void process_sample(NCR_Instance inst, NTP_Sample *sample);
 static void set_connectivity(NCR_Instance inst, SRC_Connectivity connectivity);
 
 /* ================================================== */
@@ -641,8 +643,7 @@ NCR_CreateInstance(NTP_Remote_Address *remote_addr, NTP_Source_Type type,
                                          params->min_delay, params->asymmetry);
 
   if (params->filter_length >= 1)
-    result->filter = SPF_CreateInstance(params->filter_length, params->filter_length,
-                                        NTP_MAX_DISPERSION, 0.0);
+    result->filter = SPF_CreateInstance(1, params->filter_length, NTP_MAX_DISPERSION, 0.0);
   else
     result->filter = NULL;
 
@@ -734,6 +735,7 @@ NCR_ResetInstance(NCR_Instance instance)
 
   if (instance->filter)
     SPF_DropSamples(instance->filter);
+  instance->filter_count = 0;
 }
 
 /* ================================================== */
@@ -1357,6 +1359,9 @@ transmit_timeout(void *arg)
     }
 
     SRC_UpdateReachability(inst->source, 0);
+
+    /* Count missing samples for the sample filter */
+    process_sample(inst, NULL);
   }
 
   /* With auto_offline take the source offline if sending failed */
@@ -1630,33 +1635,29 @@ check_sync_loop(NCR_Instance inst, NTP_Packet *message, NTP_Local_Address *local
 static void
 process_sample(NCR_Instance inst, NTP_Sample *sample)
 {
-  double estimated_offset, error_in_estimate, filtered_sample_ago;
+  double estimated_offset, error_in_estimate;
   NTP_Sample filtered_sample;
-  int filtered_samples;
 
-  /* Accumulate the sample to the median filter if it is enabled.  When the
-     filter produces a result, check if it is not too old, i.e. the filter did
-     not miss too many samples due to missing responses or failing tests. */
+  /* Accumulate the sample to the median filter if enabled and wait for
+     the configured number of samples before processing (NULL indicates
+     a missing sample) */
   if (inst->filter) {
-    SPF_AccumulateSample(inst->filter, sample);
+    if (sample)
+      SPF_AccumulateSample(inst->filter, sample);
 
-    filtered_samples = SPF_GetNumberOfSamples(inst->filter);
+    if (++inst->filter_count < SPF_GetMaxSamples(inst->filter))
+      return;
 
     if (!SPF_GetFilteredSample(inst->filter, &filtered_sample))
       return;
 
-    filtered_sample_ago = UTI_DiffTimespecsToDouble(&sample->time, &filtered_sample.time);
-
-    if (filtered_sample_ago > SOURCE_REACH_BITS / 2 * filtered_samples *
-                              UTI_Log2ToDouble(inst->local_poll)) {
-      DEBUG_LOG("filtered sample dropped ago=%f poll=%d", filtered_sample_ago,
-                inst->local_poll);
-      return;
-    }
-
     sample = &filtered_sample;
+    inst->filter_count = 0;
   }
 
+  if (!sample)
+    return;
+
   /* Get the estimated offset predicted from previous samples.  The
      convention here is that positive means local clock FAST of
      reference, i.e. backwards to the way that 'offset' is defined. */
@@ -2097,6 +2098,9 @@ process_response(NCR_Instance inst, NTP_Local_Address *local_addr,
     } else {
       /* Slowly increase the polling interval if we can't get a good response */
       adjust_poll(inst, testD ? 0.02 : 0.1);
+
+      /* Count missing samples for the sample filter */
+      process_sample(inst, NULL);
     }
 
     /* If in client mode, no more packets are expected to be coming from the