From 78f20f7b3e3c424319a02f12f8d5fc16c90d0932 Mon Sep 17 00:00:00 2001 From: Miroslav Lichvar Date: Mon, 27 Jun 2016 14:38:51 +0200 Subject: [PATCH 01/11] conf: fix parsing of refclock directive Don't accept refclock directive which has as the last argument an option that requires a value. --- conf.c | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/conf.c b/conf.c index 94f4ee1..8470b7e 100644 --- a/conf.c +++ b/conf.c @@ -696,9 +696,9 @@ parse_refclock(char *line) line = CPS_SplitWord(line); param = Strdup(p); - while (*line) { - cmd = line; + for (cmd = line; *cmd; line += n, cmd = line) { line = CPS_SplitWord(line); + if (!strcasecmp(cmd, "refid")) { if (sscanf(line, "%4s%n", (char *)ref, &n) != 1) break; @@ -756,10 +756,9 @@ parse_refclock(char *line) other_parse_error("Invalid refclock option"); return; } - line += n; } - if (*line) { + if (*cmd) { command_parse_error(); return; } From 12befc2afd43b41c72e982e6b24966965697fcf1 Mon Sep 17 00:00:00 2001 From: Miroslav Lichvar Date: Mon, 22 Aug 2016 13:14:45 +0200 Subject: [PATCH 02/11] ntp: fix processing of kernel timestamps on non-Linux systems When the SO_TIMESTAMP socket option was enabled, the expected type of control messages containing timestamps was SO_TIMESTAMP instead of SCM_TIMESTAMP. This worked on Linux, where the two values are equal, but not on the other supported systems. The timestamps were ignored and this probably worsened the accuracy and stability of the synchronisation. --- ntp_io.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ntp_io.c b/ntp_io.c index cf087de..88fc76e 100644 --- a/ntp_io.c +++ b/ntp_io.c @@ -556,7 +556,7 @@ read_from_socket(void *anything) #endif #ifdef SO_TIMESTAMP - if (cmsg->cmsg_level == SOL_SOCKET && cmsg->cmsg_type == SO_TIMESTAMP) { + if (cmsg->cmsg_level == SOL_SOCKET && cmsg->cmsg_type == SCM_TIMESTAMP) { struct timeval tv; memcpy(&tv, CMSG_DATA(cmsg), sizeof(tv)); From 9603f0552a8d78ddd71e643e2a0c3361074bcb27 Mon Sep 17 00:00:00 2001 From: Miroslav Lichvar Date: Tue, 6 Sep 2016 15:42:00 +0200 Subject: [PATCH 03/11] client: fix printing of negative poll in sources report again This was broken in commit 3f51805e6214cad5cb9a863491316937541601ec. --- client.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/client.c b/client.c index b00fc77..477f548 100644 --- a/client.c +++ b/client.c @@ -2007,7 +2007,7 @@ process_cmd_sources(char *line) print_report("%c%c %-27s %2d %2d %3o %I %+S[%+S] +/- %S\n", mode_ch, state_ch, name, ntohs(reply.data.source_data.stratum), - ntohs(reply.data.source_data.poll), + (int16_t)ntohs(reply.data.source_data.poll), ntohs(reply.data.source_data.reachability), (unsigned long)ntohl(reply.data.source_data.since_sample), UTI_FloatNetworkToHost(reply.data.source_data.latest_meas), From 1d9d19d76ba7952a86b8266e2cfdced114fca883 Mon Sep 17 00:00:00 2001 From: Miroslav Lichvar Date: Tue, 6 Sep 2016 15:47:40 +0200 Subject: [PATCH 04/11] client: flush stdout after printing prompt Apparently fgets() doesn't flush stdout in some libc implementations. --- client.c | 1 + 1 file changed, 1 insertion(+) diff --git a/client.c b/client.c index 477f548..314b3a3 100644 --- a/client.c +++ b/client.c @@ -125,6 +125,7 @@ read_line(void) return( line ); #else printf("%s", prompt); + fflush(stdout); #endif } if (fgets(line, sizeof(line), stdin)) { From 37d1467368e63e65823465aa5eedaa76aba3bece Mon Sep 17 00:00:00 2001 From: Miroslav Lichvar Date: Mon, 12 Sep 2016 12:23:09 +0200 Subject: [PATCH 05/11] smooth: fix selection of 1st stage direction When the smoothing process is updated with extremely small (e.g. sub-nanosecond) values, both directions may give a negative length of the 1st or 3rd stage due to numerical errors and the selection will fail an in assertion. Rework the code to select the direction which gives a smaller error. --- smooth.c | 43 +++++++++++++++++++++++++++++++------------ 1 file changed, 31 insertions(+), 12 deletions(-) diff --git a/smooth.c b/smooth.c index ae64ca3..a558d55 100644 --- a/smooth.c +++ b/smooth.c @@ -137,7 +137,7 @@ get_smoothing(struct timeval *now, double *poffset, double *pfreq, static void update_stages(void) { - double s1, s2, s, l1, l2, l3, lc, f, f2; + double s1, s2, s, l1, l2, l3, lc, f, f2, l1t[2], l3t[2], err[2]; int i, dir; /* Prepare the three stages so that the integral of the frequency offset @@ -146,22 +146,41 @@ update_stages(void) s1 = smooth_offset / max_wander; s2 = smooth_freq * smooth_freq / (2.0 * max_wander * max_wander); - l1 = l2 = l3 = 0.0; - /* Calculate the lengths of the 1st and 3rd stage assuming there is no - frequency limit. If length of the 1st stage comes out negative, switch - its direction. */ - for (dir = -1; dir <= 1; dir += 2) { + frequency limit. The direction of the 1st stage is selected so that + the lengths will not be negative. With extremely small offsets both + directions may give a negative length due to numerical errors, so select + the one which gives a smaller error. */ + + for (i = 0, dir = -1; i <= 1; i++, dir += 2) { + err[i] = 0.0; s = dir * s1 + s2; - if (s >= 0.0) { - l3 = sqrt(s); - l1 = l3 - dir * smooth_freq / max_wander; - if (l1 >= 0.0) - break; + + if (s < 0.0) { + err[i] += -s; + s = 0.0; + } + + l3t[i] = sqrt(s); + l1t[i] = l3t[i] - dir * smooth_freq / max_wander; + + if (l1t[i] < 0.0) { + err[i] += l1t[i] * l1t[i]; + l1t[i] = 0.0; } } - assert(dir <= 1 && l1 >= 0.0 && l3 >= 0.0); + if (err[0] < err[1]) { + l1 = l1t[0]; + l3 = l3t[0]; + dir = -1; + } else { + l1 = l1t[1]; + l3 = l3t[1]; + dir = 1; + } + + l2 = 0.0; /* If the limit was reached, shorten 1st+3rd stages and set a 2nd stage */ f = dir * smooth_freq + l1 * max_wander - max_freq; From b443ec5ea585ecc6e9b1c644d8e32400e96f7eba Mon Sep 17 00:00:00 2001 From: Miroslav Lichvar Date: Mon, 12 Sep 2016 12:55:57 +0200 Subject: [PATCH 06/11] test: add smooth unit test --- test/unit/smooth.c | 63 ++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 63 insertions(+) create mode 100644 test/unit/smooth.c diff --git a/test/unit/smooth.c b/test/unit/smooth.c new file mode 100644 index 0000000..153a3e2 --- /dev/null +++ b/test/unit/smooth.c @@ -0,0 +1,63 @@ +/* + ********************************************************************** + * Copyright (C) Miroslav Lichvar 2016 + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of version 2 of the GNU General Public License as + * published by the Free Software Foundation. + * + * This program is distributed in the hope that it will be useful, but + * WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * General Public License for more details. + * + * You should have received a copy of the GNU General Public License along + * with this program; if not, write to the Free Software Foundation, Inc., + * 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA. + * + ********************************************************************** + */ + +#include +#include "test.h" + +void +test_unit(void) +{ + int i, j; + struct timeval tv; + double offset, freq, wander; + char conf[] = "smoothtime 300 0.01"; + + CNF_Initialise(0); + CNF_ParseLine(NULL, 1, conf); + + LCL_Initialise(); + SMT_Initialise(); + locked = 0; + + for (i = 0; i < 500; i++) { + tv.tv_sec = tv.tv_usec = 0; + SMT_Reset(&tv); + + DEBUG_LOG(0, "iteration %d", i); + + offset = (random() % 1000000 - 500000) / 1.0e6; + freq = (random() % 1000000 - 500000) / 1.0e9; + update_smoothing(&tv, offset, freq); + + for (j = 0; j < 10000; j++) { + update_smoothing(&tv, 0.0, 0.0); + UTI_AddDoubleToTimeval(&tv, 16.0, &tv); + get_smoothing(&tv, &offset, &freq, &wander); + } + + TEST_CHECK(fabs(offset) < 1e-12); + TEST_CHECK(fabs(freq) < 1e-12); + TEST_CHECK(fabs(wander) < 1e-12); + } + + SMT_Finalise(); + LCL_Finalise(); + CNF_Finalise(); +} From 0a848e2528aaef0b3347de0b49ce50da8dc1c9a4 Mon Sep 17 00:00:00 2001 From: Miroslav Lichvar Date: Thu, 6 Oct 2016 15:21:43 +0200 Subject: [PATCH 07/11] refclock: require new samples to have newer timestamp If all or most SHM/SOCK samples collected in a polling interval had the same local timestamp, the dispersion could end up as nan, which could trigger an assert failure later in the code. Before accumulating a refclock sample, check if the timestamp is newer than the previous one. --- refclock.c | 16 +++++++++++----- 1 file changed, 11 insertions(+), 5 deletions(-) diff --git a/refclock.c b/refclock.c index 7ee1468..4439d9c 100644 --- a/refclock.c +++ b/refclock.c @@ -505,16 +505,22 @@ RCL_AddPulse(RCL_Instance instance, struct timeval *pulse_time, double second) static int valid_sample_time(RCL_Instance instance, struct timeval *tv) { - struct timeval raw_time; - double diff; + struct timeval raw_time, last_sample_time; + double diff, last_offset, last_dispersion; LCL_ReadRawTime(&raw_time); UTI_DiffTimevalsToDouble(&diff, &raw_time, tv); - if (diff < 0.0 || diff > UTI_Log2ToDouble(instance->poll + 1)) { - DEBUG_LOG(LOGF_Refclock, "%s refclock sample not valid age=%.6f tv=%s", - UTI_RefidToString(instance->ref_id), diff, UTI_TimevalToString(tv)); + + if (diff < 0.0 || diff > UTI_Log2ToDouble(instance->poll + 1) || + (filter_get_last_sample(&instance->filter, &last_sample_time, + &last_offset, &last_dispersion) && + UTI_CompareTimevals(&last_sample_time, tv) >= 0)) { + DEBUG_LOG(LOGF_Refclock, "%s refclock sample not valid age=%.6f ts=%s lastts=%s", + UTI_RefidToString(instance->ref_id), diff, + UTI_TimevalToString(tv), UTI_TimevalToString(&last_sample_time)); return 0; } + return 1; } From 2b5c86b9a3e044f0cb1a1d231f7dc8b4ae7f242b Mon Sep 17 00:00:00 2001 From: Miroslav Lichvar Date: Fri, 7 Oct 2016 10:59:45 +0200 Subject: [PATCH 08/11] refclock: fix check for old samples The fix in commit 0a848e2528aaef0b3347de0b49ce50da8dc1c9a4 was incorrect. --- refclock.c | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/refclock.c b/refclock.c index 4439d9c..85e1899 100644 --- a/refclock.c +++ b/refclock.c @@ -92,7 +92,7 @@ static ARR_Instance refclocks; static LOG_FileID logfileid; -static int valid_sample_time(RCL_Instance instance, struct timeval *tv); +static int valid_sample_time(RCL_Instance instance, struct timeval *raw, struct timeval *cooked); static int pps_stratum(RCL_Instance instance, struct timeval *tv); static void poll_timeout(void *arg); static void slew_samples(struct timeval *raw, struct timeval *cooked, double dfreq, @@ -372,7 +372,7 @@ RCL_AddSample(RCL_Instance instance, struct timeval *sample_time, double offset, /* Make sure the timestamp and offset provided by the driver are sane */ if (!UTI_IsTimeOffsetSane(sample_time, offset) || - !valid_sample_time(instance, sample_time)) + !valid_sample_time(instance, sample_time, &cooked_time)) return 0; switch (leap) { @@ -412,7 +412,7 @@ RCL_AddPulse(RCL_Instance instance, struct timeval *pulse_time, double second) dispersion += instance->precision; if (!UTI_IsTimeOffsetSane(pulse_time, 0.0) || - !valid_sample_time(instance, pulse_time)) + !valid_sample_time(instance, pulse_time, &cooked_time)) return 0; rate = instance->pps_rate; @@ -503,21 +503,21 @@ RCL_AddPulse(RCL_Instance instance, struct timeval *pulse_time, double second) } static int -valid_sample_time(RCL_Instance instance, struct timeval *tv) +valid_sample_time(RCL_Instance instance, struct timeval *raw, struct timeval *cooked) { struct timeval raw_time, last_sample_time; double diff, last_offset, last_dispersion; LCL_ReadRawTime(&raw_time); - UTI_DiffTimevalsToDouble(&diff, &raw_time, tv); + UTI_DiffTimevalsToDouble(&diff, &raw_time, raw); if (diff < 0.0 || diff > UTI_Log2ToDouble(instance->poll + 1) || (filter_get_last_sample(&instance->filter, &last_sample_time, &last_offset, &last_dispersion) && - UTI_CompareTimevals(&last_sample_time, tv) >= 0)) { - DEBUG_LOG(LOGF_Refclock, "%s refclock sample not valid age=%.6f ts=%s lastts=%s", + UTI_CompareTimevals(&last_sample_time, cooked) >= 0)) { + DEBUG_LOG(LOGF_Refclock, "%s refclock sample not valid age=%.6f raw=%s cooked=%s", UTI_RefidToString(instance->ref_id), diff, - UTI_TimevalToString(tv), UTI_TimevalToString(&last_sample_time)); + UTI_TimevalToString(raw), UTI_TimevalToString(cooked)); return 0; } From b819c7fe552bb2e12aa90d417400740ee010530c Mon Sep 17 00:00:00 2001 From: Miroslav Lichvar Date: Mon, 21 Nov 2016 11:20:57 +0100 Subject: [PATCH 09/11] refclock: don't compare sample time with samples from previous poll This is an improvement of commit 0a848e2528aaef0b3347de0b49ce50da8dc1c9a4. --- refclock.c | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/refclock.c b/refclock.c index 85e1899..862a624 100644 --- a/refclock.c +++ b/refclock.c @@ -106,6 +106,7 @@ static void filter_reset(struct MedianFilter *filter); static double filter_get_avg_sample_dispersion(struct MedianFilter *filter); static void filter_add_sample(struct MedianFilter *filter, struct timeval *sample_time, double offset, double dispersion); static int filter_get_last_sample(struct MedianFilter *filter, struct timeval *sample_time, double *offset, double *dispersion); +static int filter_get_samples(struct MedianFilter *filter); static int filter_select_samples(struct MedianFilter *filter); static int filter_get_sample(struct MedianFilter *filter, struct timeval *sample_time, double *offset, double *dispersion); static void filter_slew_samples(struct MedianFilter *filter, struct timeval *when, double dfreq, double doffset); @@ -512,7 +513,8 @@ valid_sample_time(RCL_Instance instance, struct timeval *raw, struct timeval *co UTI_DiffTimevalsToDouble(&diff, &raw_time, raw); if (diff < 0.0 || diff > UTI_Log2ToDouble(instance->poll + 1) || - (filter_get_last_sample(&instance->filter, &last_sample_time, + (filter_get_samples(&instance->filter) > 0 && + filter_get_last_sample(&instance->filter, &last_sample_time, &last_offset, &last_dispersion) && UTI_CompareTimevals(&last_sample_time, cooked) >= 0)) { DEBUG_LOG(LOGF_Refclock, "%s refclock sample not valid age=%.6f raw=%s cooked=%s", @@ -725,6 +727,12 @@ filter_get_last_sample(struct MedianFilter *filter, struct timeval *sample_time, return 1; } +static int +filter_get_samples(struct MedianFilter *filter) +{ + return filter->used; +} + static const struct FilterSample *tmp_sorted_array; static int From 85fbfd9b150e899bde793c8a679798cfb1120985 Mon Sep 17 00:00:00 2001 From: Miroslav Lichvar Date: Fri, 11 Nov 2016 14:04:12 +0100 Subject: [PATCH 10/11] sources: add new status for sources that overlap trusted sources Sources that overlap trusted sources should be displayed in the chronyc sources report with the '-' symbol and they shouldn't trigger a replacement. --- sources.c | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/sources.c b/sources.c index 79f72cf..550d109 100644 --- a/sources.c +++ b/sources.c @@ -74,6 +74,7 @@ typedef enum { SRC_WAITS_STATS, /* Others have bad stats, selection postponed */ SRC_STALE, /* Has older samples than others */ SRC_ORPHAN, /* Has stratum equal or larger than orphan stratum */ + SRC_UNTRUSTED, /* Overlaps trusted sources */ SRC_FALSETICKER, /* Doesn't agree with others */ SRC_JITTERY, /* Scatter worse than other's dispersion (not used) */ SRC_WAITS_SOURCES, /* Not enough sources, selection postponed */ @@ -890,6 +891,9 @@ SRC_SelectSource(SRC_Instance updated_inst) if (sources[i]->sel_options & SRC_SELECT_REQUIRE) sel_req_source = 0; + } else if (sources[i]->sel_info.lo_limit <= best_lo && + sources[i]->sel_info.hi_limit >= best_hi) { + sources[i]->status = SRC_UNTRUSTED; } else { sources[i]->status = SRC_FALSETICKER; } @@ -1318,6 +1322,7 @@ SRC_ReportSource(int index, RPT_SourceReport *report, struct timeval *now) case SRC_JITTERY: report->state = RPT_JITTERY; break; + case SRC_UNTRUSTED: case SRC_WAITS_SOURCES: case SRC_NONPREFERRED: case SRC_WAITS_UPDATE: From db286ca6ead28d59d9ae3f41afa5946b16ed2b30 Mon Sep 17 00:00:00 2001 From: Miroslav Lichvar Date: Mon, 21 Nov 2016 11:58:26 +0100 Subject: [PATCH 11/11] doc: update NEWS --- NEWS | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/NEWS b/NEWS index a293541..4f5c218 100644 --- a/NEWS +++ b/NEWS @@ -1,3 +1,13 @@ +New in version 2.4.1 +==================== + +Bug fixes +--------- +* Fix processing of kernel timestamps on non-Linux systems +* Fix crash with smoothtime directive +* Fix validation of refclock sample times +* Fix parsing of refclock directive + New in version 2.4 ==================