diff --git a/src/confd/src/system.c b/src/confd/src/system.c index 3890685d2..3e4db6c8c 100644 --- a/src/confd/src/system.c +++ b/src/confd/src/system.c @@ -404,6 +404,17 @@ static int change_ntp_client(sr_session_ctx_t *session, struct lyd_node *config, fprintf(fp, " iburst"); if (srx_enabled(session, "%s/prefer", xpath) > 0) fprintf(fp, " prefer"); + + ptr = srx_get_str(session, "%s/infix-system:minpoll", xpath); + if (ptr) { + fprintf(fp, " minpoll %s", ptr); + free(ptr); + } + ptr = srx_get_str(session, "%s/infix-system:maxpoll", xpath); + if (ptr) { + fprintf(fp, " maxpoll %s", ptr); + free(ptr); + } } fprintf(fp, "\n"); fclose(fp); diff --git a/src/confd/yang/confd.inc b/src/confd/yang/confd.inc index 88cada9d8..86286ff30 100644 --- a/src/confd/yang/confd.inc +++ b/src/confd/yang/confd.inc @@ -43,7 +43,7 @@ MODULES=( "infix-firewall-icmp-types@2025-04-26.yang" "infix-meta@2025-12-10.yang" "infix-services@2026-06-17.yang" - "infix-system@2026-06-17.yang" + "infix-system@2026-09-01.yang" "ieee802-ethernet-interface@2025-09-10.yang" "ieee802-ethernet-phy-type@2025-09-10.yang" "infix-ethernet-interface@2026-05-21.yang" diff --git a/src/confd/yang/confd/infix-system.yang b/src/confd/yang/confd/infix-system.yang index b79a2fcad..37f8972a6 100644 --- a/src/confd/yang/confd/infix-system.yang +++ b/src/confd/yang/confd/infix-system.yang @@ -32,6 +32,10 @@ module infix-system { contact "kernelkit@googlegroups.com"; description "Infix augments and deviations to ietf-system."; + revision 2026-09-01 { + description "Add per-server minpoll/maxpoll to NTP client configuration."; + reference "internal"; + } revision 2026-06-17 { description "Add scheduled-reboot, triggered from a referenced schedule."; reference "internal"; @@ -370,6 +374,39 @@ module infix-system { } } + augment "/sys:system/sys:ntp/sys:server" { + description "Per-server polling interval limits for the NTP client."; + + leaf minpoll { + type int8 { + range "-6..24"; + } + default "6"; + units "log2 seconds"; + description + "Minimum poll interval used for this server, as log2 seconds."; + reference + "RFC 5905: Network Time Protocol Version 4: Protocol and + Algorithms Specification, Section 7.2"; + } + + leaf maxpoll { + must ". >= ../minpoll" { + error-message "maxpoll must be greater than or equal to minpoll"; + } + type int8 { + range "-6..24"; + } + default "10"; + units "log2 seconds"; + description + "Maximum poll interval used for this server, as log2 seconds."; + reference + "RFC 5905: Network Time Protocol Version 4: Protocol and + Algorithms Specification, Section 7.2"; + } + } + augment "/sys:system/sys:authentication/sys:user" { description "Augment of ietf-system to support setting login shell for users."; leaf shell { diff --git a/src/confd/yang/confd/infix-system@2026-06-17.yang b/src/confd/yang/confd/infix-system@2026-09-01.yang similarity index 100% rename from src/confd/yang/confd/infix-system@2026-06-17.yang rename to src/confd/yang/confd/infix-system@2026-09-01.yang diff --git a/test/.env b/test/.env index 857d9e128..16ef67f13 100644 --- a/test/.env +++ b/test/.env @@ -2,7 +2,7 @@ # shellcheck disable=SC2034,SC2154 # Current container image -INFIX_TEST=ghcr.io/kernelkit/infix-test:2.10 +INFIX_TEST=ghcr.io/kernelkit/infix-test:2.11 ixdir=$(readlink -f "$testdir/..") logdir=$(readlink -f "$testdir/.log") diff --git a/test/case/ntp/client_stratum_selection/test.adoc b/test/case/ntp/client_stratum_selection/test.adoc index 07a9ecc3a..28b10f815 100644 --- a/test/case/ntp/client_stratum_selection/test.adoc +++ b/test/case/ntp/client_stratum_selection/test.adoc @@ -10,8 +10,9 @@ stratum level. This test validates NTP clock selection algorithm by configuring a client to sync from two servers with different stratum levels: -- srv1: Test PC running BusyBox ntpd (stratum ~1 via -l flag) -- srv2: NTP server DUT syncing from srv1 (stratum ~2) +- srv1: Test PC running chronyd, serving its local clock at stratum 5 + with an honest root distance so clients tolerate startup transients +- srv2: NTP server DUT syncing from srv1 (stratum 6) - client: NTP client DUT syncing from both servers Both servers sync to the same time source (srv2 syncs from srv1), diff --git a/test/case/ntp/client_stratum_selection/test.py b/test/case/ntp/client_stratum_selection/test.py index f52be5461..7e1665a83 100755 --- a/test/case/ntp/client_stratum_selection/test.py +++ b/test/case/ntp/client_stratum_selection/test.py @@ -7,8 +7,9 @@ This test validates NTP clock selection algorithm by configuring a client to sync from two servers with different stratum levels: -- srv1: Test PC running BusyBox ntpd (stratum ~1 via -l flag) -- srv2: NTP server DUT syncing from srv1 (stratum ~2) +- srv1: Test PC running chronyd, serving its local clock at stratum 5 + with an honest root distance so clients tolerate startup transients +- srv2: NTP server DUT syncing from srv1 (stratum 6) - client: NTP client DUT syncing from both servers Both servers sync to the same time source (srv2 syncs from srv1), @@ -24,7 +25,7 @@ # Network configuration ips = { - "srv1": "192.168.1.1", # BusyBox ntpd on test PC + "srv1": "192.168.1.1", # chronyd on test PC "srv2": "192.168.1.2", # Infix NTP server "client": "192.168.1.3" # Infix NTP client } @@ -78,14 +79,37 @@ "unicast-configuration": [{ "address": ips["srv1"], # Sync from srv1 "type": "uc-server", - "iburst": True + "iburst": True, + # Poll every 16 s so sub-threshold + # offsets drain quickly (corrections + # are spread over ~3 poll intervals) + "minpoll": 4, + "maxpoll": 6 }] } } }) with test.step("Wait for srv2 to sync from srv1"): - until(lambda: ntp.server_has_associations(srv2), attempts=60) + # Wait for srv2 to select srv1 and converge on it, not + # just for the association to show up. If the client is + # configured while srv2 is still stepping/slewing, the + # client's early samples of the two servers disagree by + # seconds, chronyd flags srv1 as unstable ('~'), and it + # stays unselectable until the bad samples age out of + # the regression window (several 64 s polls). + # iburst + makestep converge in ~10-20 s; this budget + # is only consumed when something is actually broken + try: + until(lambda: ntp.server_source_synced(srv2, ips["srv1"]), + attempts=60) + except Exception: + print("DEBUG: srv2 did not converge on srv1. Associations:") + for assoc in ntp.server_get_associations(srv2): + print(f" {assoc.get('address')}: stratum={assoc.get('stratum')}, " + f"prefer={assoc.get('prefer')}, reach={assoc.get('reach')}, " + f"offset={assoc.get('offset')}") + raise with test.step("Configure client to sync from both servers"): client.put_config_dicts({ @@ -113,13 +137,19 @@ "udp": { "address": ips["srv1"] }, - "iburst": True + "iburst": True, + # Poll every 16 s so selection + # re-evaluates quickly after iburst + "infix-system:minpoll": 4, + "infix-system:maxpoll": 6 }, { "name": "srv2", "udp": { "address": ips["srv2"] }, - "iburst": True + "iburst": True, + "infix-system:minpoll": 4, + "infix-system:maxpoll": 6 }] } } @@ -127,7 +157,7 @@ }) with test.step("Wait for client to see both servers"): - until(lambda: ntp.number_of_sources(client) == 2, attempts=60) + until(lambda: ntp.number_of_sources(client) == 2, attempts=30) with test.step("Wait for srv2 stratum to stabilize"): # Ensure srv2 has synced with srv1 and is advertising @@ -148,7 +178,9 @@ def check_stratums(): return True return False - until(check_stratums, attempts=60) + # srv2 synced before the client was configured, so the + # client's iburst samples already carry stratum 6 + until(check_stratums, attempts=30) print(f"srv1 and srv2 stratums verified as different") with test.step("Verify client selects srv1 (lower stratum)"): @@ -159,7 +191,10 @@ def srv1_selected(): return None try: - selected = until(srv1_selected, attempts=120) + # Selection normally happens at the end of iburst; + # with minpoll 4 this covers two extra 16 s poll + # cycles plus slack + selected = until(srv1_selected, attempts=45) except Exception: # Timeout - print diagnostic info sources = ntp.get_sources(client) diff --git a/test/docker/Dockerfile b/test/docker/Dockerfile index c1750270c..1267d7c05 100644 --- a/test/docker/Dockerfile +++ b/test/docker/Dockerfile @@ -3,6 +3,7 @@ FROM alpine:3.18.0 # NOTE: please add packages alphabetically! RUN apk add --no-cache \ busybox-extras \ + chrony \ curl \ dhcp-server-vanilla \ dnsmasq \ diff --git a/test/infamy/ntp.py b/test/infamy/ntp.py index c05e6cd3d..311bf4bae 100644 --- a/test/infamy/ntp.py +++ b/test/infamy/ntp.py @@ -108,6 +108,45 @@ def server_has_associations(target): return False +def server_get_associations(target): + """Get list of NTP associations (ietf-ntp) from operational state.""" + try: + data = target.get_data("/ietf-ntp:ntp/associations") + if not data: + return [] + return data.get("ntp", {}).get("associations", {}).get("association", []) + except Exception: + return [] + + +def server_source_synced(target, address, max_offset_ms=50.0): + """Verify NTP server (ietf-ntp) has selected the given source and + converged on it, i.e., the estimated offset is small. + + Waiting only for the association to exist is not enough: the server + may still be stepping/slewing its clock, serving time that moves + around. Any client sampling both this server and its upstream + during that window collects inconsistent measurements and chronyd + flags the upstream as having too much variability, excluding it + from selection for several poll intervals. + """ + try: + for assoc in server_get_associations(target): + if assoc.get("address") != address: + continue + if not assoc.get("prefer"): + return False + + offset = assoc.get("offset") + if offset is None: + return False + return abs(float(offset)) <= max_offset_ms + + return False + except Exception: + return False + + def server_has_peer(target, peer_address): """Verify NTP server (ietf-ntp) has a peer association with given address.""" try: diff --git a/test/infamy/ntp_server.py b/test/infamy/ntp_server.py index baa01d85f..dbb2e9fdc 100644 --- a/test/infamy/ntp_server.py +++ b/test/infamy/ntp_server.py @@ -1,24 +1,68 @@ """Start NTP server in the background""" -import subprocess +import tempfile +import time + class Server: - def __init__(self, netns, iface="iface"): + """chronyd serving its local clock, never touching it (-x). + + stratum and distance control what is advertised to clients: the + stratum of the served time and the root distance (accuracy claim). + An honest, wide distance keeps clients from flagging the source as + unstable ('~') while their own clocks are still settling, which + BusyBox ntpd provoked by claiming near-zero dispersion. + """ + + def __init__(self, netns, iface="iface", stratum=5, distance=1.0): self.iface = iface self.process = None self.netns = netns + self.stratum = stratum + self.distance = distance + self.rundir = None + self.logfile = None def __enter__(self): self.start() + return self def __exit__(self, _, __, ___): self.stop() def start(self): - cmd=f"ntpd -w -n -l -I {self.iface}" - self.process = self.netns.popen(cmd.split(" "),stderr=subprocess.DEVNULL) + # Instances in different netns share the filesystem, so keep + # pidfile and command socket per-instance to avoid collisions + self.rundir = tempfile.TemporaryDirectory(prefix="chronyd-") + cmd = [ + "chronyd", "-d", "-x", "-u", "root", + f"local stratum {self.stratum} distance {self.distance}", + "allow", + f"pidfile {self.rundir.name}/chronyd.pid", + f"bindcmdaddress {self.rundir.name}/chronyd.sock", + ] + self.logfile = open(f"{self.rundir.name}/chronyd.log", "w") + self.process = self.netns.popen(cmd, stderr=self.logfile) + + # chronyd exits immediately on bad options or a missing binary + # behind an exec wrapper; fail loudly instead of serving nothing + time.sleep(1) + if self.process.poll() is not None: + with open(f"{self.rundir.name}/chronyd.log") as f: + log = f.read().strip() + code = self.process.returncode + self.stop() + raise RuntimeError( + f"chronyd failed to start (exit {code}): " + f"{log or 'no output; is chrony installed in the test environment?'}") def stop(self): if self.process: self.process.terminate() self.process.wait() self.process = None + if self.logfile: + self.logfile.close() + self.logfile = None + if self.rundir: + self.rundir.cleanup() + self.rundir = None