From 5160f14fdcbf7335120dc10b09f95d7a881029bf Mon Sep 17 00:00:00 2001 From: Miroslav Lichvar Date: Tue, 14 Mar 2023 12:23:21 +0100 Subject: [PATCH] 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. --- conf.c | 9 ++- conf.h | 1 + doc/chrony.conf.adoc | 11 +++- ntp_io_linux.c | 97 ++++++++++++++++++++++++++------- sched.h | 1 + test/simulation/133-hwtimestamp | 31 ++++++++++- 6 files changed, 124 insertions(+), 26 deletions(-) diff --git a/conf.c b/conf.c index 54652a0..bce06fa 100644 --- 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 5ce6559..ca18abc 100644 --- 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 02e3bfa..49d5168 100644 --- 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 3f0620a..41d581b 100644 --- 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 f1f4eb9..90d757f 100644 --- 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 d3cce6d..f02a010 100755 --- 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