ntp: add maximum PHC poll interval

Specify maxpoll for HW timestamping (default minpoll + 1) to track the
PHC well even when there is little NTP traffic on the interface. After
each PHC reading schedule a timeout according to the maxpoll. Polling
between minpoll and maxpoll is still triggered by HW timestamps.

Wait for the first HW timestamp before adding the timeout to avoid
polling PHCs on interfaces that are enabled in the configuration but
not used for NTP. Add a new scheduling class to separate polling of
different PHCs to avoid too long intervals between processing I/O
events.

diff --git a/conf.c b/conf.c
index 54652a099d7954695c330eb0cb7c533c6efaf30c..bce06fa54046da41c9cad035ac72e7ac77d2654f 100644 (file)
--- a/conf.c
+++ b/conf.c
@@ -1430,8 +1430,8 @@ static void
 parse_hwtimestamp(char *line)
 {
   CNF_HwTsInterface *iface;
+  int n, maxpoll_set = 0;
   char *p, filter[5];
-  int n;
 
   if (!*line) {
     command_parse_error();
@@ -1461,6 +1461,10 @@ parse_hwtimestamp(char *line)
     } else if (!strcasecmp(p, "minpoll")) {
       if (sscanf(line, "%d%n", &iface->minpoll, &n) != 1)
         break;
+    } else if (!strcasecmp(p, "maxpoll")) {
+      if (sscanf(line, "%d%n", &iface->maxpoll, &n) != 1)
+        break;
+      maxpoll_set = 1;
     } else if (!strcasecmp(p, "minsamples")) {
       if (sscanf(line, "%d%n", &iface->min_samples, &n) != 1)
         break;
@@ -1496,6 +1500,9 @@ parse_hwtimestamp(char *line)
 
   if (*p)
     command_parse_error();
+
+  if (!maxpoll_set)
+    iface->maxpoll = iface->minpoll + 1;
 }
 
 /* ================================================== */

diff --git a/conf.h b/conf.h
index 5ce6559e54350a40446f3e6df0d768b55041ede3..ca18abc65bbbb31a19f8b3b0e877d239a0de6dce 100644 (file)
--- a/conf.h
+++ b/conf.h
@@ -144,6 +144,7 @@ typedef enum {
 typedef struct {
   char *name;
   int minpoll;
+  int maxpoll;
   int min_samples;
   int max_samples;
   int nocrossts;

diff --git a/doc/chrony.conf.adoc b/doc/chrony.conf.adoc
index 02e3bfa0a19968912c4a126f30639194bcfe5fff..49d5168ba57f565511051370820109c257790d17 100644 (file)
--- a/doc/chrony.conf.adoc
+++ b/doc/chrony.conf.adoc
@@ -2549,10 +2549,15 @@ The *hwtimestamp* directive has the following options:
 +
 *minpoll* _poll_:::
 This option specifies the minimum interval between readings of the NIC clock.
-It's defined as a power of two. It should correspond to the minimum polling
+It's defined as a power of 2. It should correspond to the minimum polling
 interval of all NTP sources and the minimum expected polling interval of NTP
-clients. The default value is 0 (1 second) and the minimum value is -6 (1/64th
-of a second).
+clients. The default value is 0 (1 second), the minimum value is -6 (1/64th
+of a second), and the maximum value is 20 (about 12 days).
+*maxpoll* _poll_:::
+This option specifies the maximum interval between readings of the NIC clock,
+as a power of 2. The default value is *minpoll* + 1, i.e. 1 (2 seconds) with
+the default *minpoll* of 0. The minimum and maximum values are the same as with
+the *minpoll* option.
 *minsamples* _samples_:::
 This option specifies the minimum number of readings kept for tracking of the
 NIC clock. The default value is 2.

diff --git a/ntp_io_linux.c b/ntp_io_linux.c
index 3f0620a582d1be6721ac8e54e498f4d4770c931a..41d581b3c12a5d16c374f8513c6bee74ef3b7215 100644 (file)
--- a/ntp_io_linux.c
+++ b/ntp_io_linux.c
@@ -64,13 +64,16 @@ struct Interface {
   double tx_comp;
   double rx_comp;
   HCL_Instance clock;
+  int maxpoll;
+  SCH_TimeoutID poll_timeout_id;
 };
 
 /* Number of PHC readings per HW clock sample */
 #define PHC_READINGS 25
 
-/* Minimum interval between PHC readings */
+/* Minimum and maximum interval between PHC readings */
 #define MIN_PHC_POLL -6
+#define MAX_PHC_POLL 20
 
 /* Maximum acceptable offset between SW/HW and daemon timestamp */
 #define MAX_TS_DELAY 1.0
@@ -110,13 +113,17 @@ static int dummy_rxts_socket;
 
 /* ================================================== */
 
+static void poll_phc(struct Interface *iface, struct timespec *now);
+
+/* ================================================== */
+
 static int
 add_interface(CNF_HwTsInterface *conf_iface)
 {
+  int sock_fd, if_index, minpoll, phc_fd, req_hwts_flags, rx_filter;
   struct ethtool_ts_info ts_info;
   struct hwtstamp_config ts_config;
   struct ifreq req;
-  int sock_fd, if_index, phc_fd, req_hwts_flags, rx_filter;
   unsigned int i;
   struct Interface *iface;
 
@@ -248,9 +255,15 @@ add_interface(CNF_HwTsInterface *conf_iface)
   iface->tx_comp = conf_iface->tx_comp;
   iface->rx_comp = conf_iface->rx_comp;
 
+  minpoll = CLAMP(MIN_PHC_POLL, conf_iface->minpoll, MAX_PHC_POLL);
   iface->clock = HCL_CreateInstance(conf_iface->min_samples, conf_iface->max_samples,
-                                    UTI_Log2ToDouble(MAX(conf_iface->minpoll, MIN_PHC_POLL)),
-                                    conf_iface->precision);
+                                    UTI_Log2ToDouble(minpoll), conf_iface->precision);
+
+  iface->maxpoll = CLAMP(minpoll, conf_iface->maxpoll, MAX_PHC_POLL);
+
+  /* Do not schedule the first poll timeout here!  The argument (interface) can
+     move until all interfaces are added.  Wait for the first HW timestamp. */
+  iface->poll_timeout_id = 0;
 
   LOG(LOGS_INFO, "Enabled HW timestamping %son %s",
       ts_config.rx_filter == HWTSTAMP_FILTER_NONE ? "(TX only) " : "", iface->name);
@@ -436,6 +449,7 @@ NIO_Linux_Finalise(void)
 
   for (i = 0; i < ARR_GetSize(interfaces); i++) {
     iface = ARR_GetElement(interfaces, i);
+    SCH_RemoveTimeout(iface->poll_timeout_id);
     HCL_DestroyInstance(iface->clock);
     close(iface->phc_fd);
   }
@@ -580,29 +594,70 @@ get_interface(int if_index)
 
 /* ================================================== */
 
+static void
+poll_timeout(void *arg)
+{
+  struct Interface *iface = arg;
+  struct timespec now;
+
+  iface->poll_timeout_id = 0;
+
+  SCH_GetLastEventTime(&now, NULL, NULL);
+  poll_phc(iface, &now);
+}
+
+/* ================================================== */
+
+static void
+poll_phc(struct Interface *iface, struct timespec *now)
+{
+  struct timespec sample_phc_ts, sample_sys_ts, sample_local_ts;
+  struct timespec phc_readings[PHC_READINGS][3];
+  double phc_err, local_err, interval;
+  int n_readings;
+
+  if (!HCL_NeedsNewSample(iface->clock, now))
+    return;
+
+  DEBUG_LOG("Polling PHC on %s%s",
+            iface->name, iface->poll_timeout_id != 0 ? " before timeout" : "");
+
+  n_readings = SYS_Linux_GetPHCReadings(iface->phc_fd, iface->phc_nocrossts,
+                                        &iface->phc_mode, PHC_READINGS, phc_readings);
+
+  /* Add timeout for the next poll in case no HW timestamp will be captured
+     between the minpoll and maxpoll.  Separate reading of different PHCs to
+     avoid long intervals between handling I/O events. */
+  SCH_RemoveTimeout(iface->poll_timeout_id);
+  interval = UTI_Log2ToDouble(iface->maxpoll);
+  iface->poll_timeout_id = SCH_AddTimeoutInClass(interval, interval /
+                                                   ARR_GetSize(interfaces) / 4, 0.1,
+                                                 SCH_PhcPollClass, poll_timeout, iface);
+
+  if (n_readings <= 0)
+    return;
+
+  if (!HCL_ProcessReadings(iface->clock, n_readings, phc_readings,
+                           &sample_phc_ts, &sample_sys_ts, &phc_err))
+    return;
+
+  LCL_CookTime(&sample_sys_ts, &sample_local_ts, &local_err);
+  HCL_AccumulateSample(iface->clock, &sample_phc_ts, &sample_local_ts, phc_err + local_err);
+
+  update_interface_speed(iface);
+}
+
+/* ================================================== */
+
 static void
 process_hw_timestamp(struct Interface *iface, struct timespec *hw_ts,
                      NTP_Local_Timestamp *local_ts, int rx_ntp_length, int family,
                      int l2_length)
 {
-  struct timespec sample_phc_ts, sample_sys_ts, sample_local_ts, ts;
-  struct timespec phc_readings[PHC_READINGS][3];
-  double rx_correction, ts_delay, phc_err, local_err;
-  int n_readings;
+  double rx_correction, ts_delay, local_err;
+  struct timespec ts;
 
-  if (HCL_NeedsNewSample(iface->clock, &local_ts->ts)) {
-    n_readings = SYS_Linux_GetPHCReadings(iface->phc_fd, iface->phc_nocrossts,
-                                          &iface->phc_mode, PHC_READINGS, phc_readings);
-    if (n_readings > 0 &&
-        HCL_ProcessReadings(iface->clock, n_readings, phc_readings,
-                             &sample_phc_ts, &sample_sys_ts, &phc_err)) {
-      LCL_CookTime(&sample_sys_ts, &sample_local_ts, &local_err);
-      HCL_AccumulateSample(iface->clock, &sample_phc_ts, &sample_local_ts,
-                           phc_err + local_err);
-
-      update_interface_speed(iface);
-    }
-  }
+  poll_phc(iface, &local_ts->ts);
 
   /* We need to transpose RX timestamps as hardware timestamps are normally
      preamble timestamps and RX timestamps in NTP are supposed to be trailer

diff --git a/sched.h b/sched.h
index f1f4eb94015dda91b6838cf667d1322d6ed4560a..90d757fad85ef6582e2d92e069e468285d478ad0 100644 (file)
--- a/sched.h
+++ b/sched.h
@@ -37,6 +37,7 @@ typedef enum {
   SCH_NtpClientClass,
   SCH_NtpPeerClass,
   SCH_NtpBroadcastClass,
+  SCH_PhcPollClass,
   SCH_NumberOfClasses /* needs to be last */
 } SCH_TimeoutClass;
 

diff --git a/test/simulation/133-hwtimestamp b/test/simulation/133-hwtimestamp
index d3cce6d7939517902f70c0ee19130d67354c91b5..f02a010cfe00359b75c4140262c836f2aaf810d3 100755 (executable)
--- a/test/simulation/133-hwtimestamp
+++ b/test/simulation/133-hwtimestamp
@@ -47,9 +47,10 @@ for client_conf in \
                check_log_messages "Received error.*message.*tss=KH" 195 200 || test_fail
                check_log_messages "Updated RX timestamp.*tss=1" 1 1 || test_fail
                check_log_messages "Updated RX timestamp.*tss=2" 195 200 || test_fail
+               check_log_messages "Polling PHC" 195 220 || test_fail
                if echo "$client_conf" | grep -q nocrossts; then
                        check_log_messages "update_tx_timestamp.*Updated" 180 200 || test_fail
-                       check_log_messages "update_tx_timestamp.*Unacceptable" 0 10 || test_fail
+                       check_log_messages "update_tx_timestamp.*Unacceptable" 0 13 || test_fail
                else
                        check_log_messages "update_tx_timestamp.*Updated" 50 140 || test_fail
                        check_log_messages "update_tx_timestamp.*Unacceptable" 50 140 || test_fail
@@ -57,4 +58,32 @@ for client_conf in \
        fi
 done
 
+server_conf+="
+server 192.168.123.2 minpoll 1 maxpoll 1 noselect"
+
+for maxpoll in -1 0 1; do
+       client_conf="hwtimestamp eth0 minpoll -1 maxpoll $maxpoll nocrossts"
+       run_test || test_fail
+       check_chronyd_exit || test_fail
+       check_source_selection || test_fail
+       check_sync || test_fail
+
+       if check_config_h 'FEAT_DEBUG 1'; then
+               case $maxpoll in
+                       -1)
+                               check_log_messages "Polling PHC on eth0$" 360 380 || test_fail
+                               check_log_messages "Polling PHC.*before" 3 25 || test_fail
+                               ;;
+                       0)
+                               check_log_messages "Polling PHC on eth0$" 8 45 || test_fail
+                               check_log_messages "Polling PHC.*before" 150 190 || test_fail
+                               ;;
+                       1)
+                               check_log_messages "Polling PHC on eth0$" 1 1 || test_fail
+                               check_log_messages "Polling PHC.*before" 194 199 || test_fail
+                               ;;
+               esac
+       fi
+done
+
 test_pass