From b2dac47c8267fb5a40320a72cd1143771fa411b8 Mon Sep 17 00:00:00 2001 From: Miroslav Lichvar Date: Mon, 15 May 2023 16:26:21 +0200 Subject: [PATCH] ntp: avoid unneccessary replacements on refresh command When the refresh command is issued, instead of trying to replace all NTP sources as if they were unreachable or falsetickers, keep using the current address if it is still returned by the resolver for the name. This avoids unnecessary loss of measurements and switching to potentially unreachable addresses. --- doc/chronyc.adoc | 15 ++++++++++----- ntp_sources.c | 24 +++++++++++++++++++++--- test/simulation/147-refresh | 31 +++++++++++++++++++++++++++++++ 3 files changed, 62 insertions(+), 8 deletions(-) create mode 100755 test/simulation/147-refresh diff --git a/doc/chronyc.adoc b/doc/chronyc.adoc index 2a37e5c..4823136 100644 --- a/doc/chronyc.adoc +++ b/doc/chronyc.adoc @@ -970,12 +970,17 @@ current set of sources. It is equivalent to the *polltarget* option in the [[refresh]]*refresh*:: The *refresh* command can be used to force *chronyd* to resolve the names of -configured sources to IP addresses again, e.g. after suspending and resuming -the machine in a different network. +configured NTP sources to IP addresses again and replace any addresses missing +in the list of resolved addresses. + -Sources that stop responding will be replaced with newly resolved addresses -automatically after 8 polling intervals, but this command can still be useful -to replace them immediately and not wait until they are marked as unreachable. +Sources that stop responding are replaced with newly resolved addresses +automatically after 8 polling intervals. This command can be used to replace +them immediately, e.g. after suspending and resuming the machine in a different +network. ++ +Note that with larger pools which do not have all their IPv4 or IPv6 addresses +included in a single DNS response (e.g. pool.ntp.org), this command will +replace the addresses even if they are still included in the pool. [[reload]]*reload* *sources*:: The *reload sources* command causes *chronyd* to re-read all _*.sources_ files diff --git a/ntp_sources.c b/ntp_sources.c index fa562a1..91267dc 100644 --- a/ntp_sources.c +++ b/ntp_sources.c @@ -96,6 +96,9 @@ struct UnresolvedSource { char *name; /* Flag indicating addresses should be used in a random order */ int random_order; + /* Flag indicating current address should be replaced only if it is + no longer returned by the resolver */ + int refreshment; /* Next unresolved source in the list */ struct UnresolvedSource *next; }; @@ -517,6 +520,19 @@ process_resolved_name(struct UnresolvedSource *us, IPAddr *ip_addrs, int n_addrs unsigned short first = 0; int i, j; + /* Keep using the current address if it is being refreshed and it is + still included in the resolved addresses */ + if (us->refreshment) { + assert(us->pool_id == INVALID_POOL); + + for (i = 0; i < n_addrs; i++) { + if (UTI_CompareIPs(&us->address.ip_addr, &ip_addrs[i], NULL) == 0) { + DEBUG_LOG("%s still fresh", UTI_IPToString(&us->address.ip_addr)); + return; + } + } + } + if (us->random_order) UTI_GetRandomBytes(&first, sizeof (first)); @@ -773,6 +789,7 @@ NSR_AddSourceByName(char *name, int port, int pool, NTP_Source_Type type, us = MallocNew(struct UnresolvedSource); us->name = Strdup(name); us->random_order = 0; + us->refreshment = 0; remote_addr.ip_addr.family = IPADDR_ID; remote_addr.ip_addr.addr.id = ++last_address_id; @@ -962,7 +979,7 @@ NSR_RemoveAllSources(void) /* ================================================== */ static void -resolve_source_replacement(SourceRecord *record) +resolve_source_replacement(SourceRecord *record, int refreshment) { struct UnresolvedSource *us; @@ -976,6 +993,7 @@ resolve_source_replacement(SourceRecord *record) stuck to a pair of addresses if the order doesn't change, or a group of IPv4/IPv6 addresses if the resolver prefers inaccessible IP family */ us->random_order = record->tentative; + us->refreshment = refreshment; us->pool_id = INVALID_POOL; us->address = *record->remote_addr; @@ -1015,7 +1033,7 @@ NSR_HandleBadSource(IPAddr *address) } last_replacement = now; - resolve_source_replacement(record); + resolve_source_replacement(record, 0); } /* ================================================== */ @@ -1031,7 +1049,7 @@ NSR_RefreshAddresses(void) if (!record->remote_addr) continue; - resolve_source_replacement(record); + resolve_source_replacement(record, 1); } } diff --git a/test/simulation/147-refresh b/test/simulation/147-refresh new file mode 100755 index 0000000..f83a9c6 --- /dev/null +++ b/test/simulation/147-refresh @@ -0,0 +1,31 @@ +#!/usr/bin/env bash + +. ./test.common + +test_start "address refreshment" + +limit=1000 +servers=5 +client_conf="logdir tmp +log measurements" +client_server_conf="server nodes-1-2.net1.clk maxpoll 6 +pool nodes-3-4-5.net1.clk maxpoll 6 maxsources 2" +client_chronyd_options="-d" +chronyc_conf="refresh" +chronyc_start=500 + +run_test || test_fail +check_chronyd_exit || test_fail +check_source_selection || test_fail +check_packet_interval || test_fail +check_sync || test_fail + +check_file_messages "20.*192.168.123.1" 0 0 measurements.log || test_fail +check_file_messages "20.*192.168.123.2" 15 17 measurements.log || test_fail +check_file_messages "20.*192.168.123.[345]" 31 33 measurements.log || test_fail +rm -f tmp/measurements.log +if check_config_h 'FEAT_DEBUG 1'; then + check_log_messages "resolved_name.*still fresh" 3 3 || test_fail +fi + +test_pass