Skip to content

Add minpoll/maxpoll for ntp client - #1601

Open
mattiaswal wants to merge 2 commits into
mainfrom
fix-ntp-test
Open

Add minpoll/maxpoll for ntp client#1601
mattiaswal wants to merge 2 commits into
mainfrom
fix-ntp-test

Conversation

@mattiaswal

@mattiaswal mattiaswal commented Sep 1, 2026

Copy link
Copy Markdown
Contributor

Also make the local timeserver more robust, changing from busybox ntpd to chronyd.
This for now only effects one test: client_stratum_selection

Description

Checklist

Tick relevant boxes, this PR is-a or has-a:

  • Bugfix
    • Regression tests
    • ChangeLog updates (for next release)
  • Feature
    • YANG model change => revision updated?
    • Regression tests added?
    • ChangeLog updates (for next release)
    • Documentation added?
  • Test changes
    • Checked in changed Readme.adoc (make test-spec)
    • Added new test to group Readme.adoc and yaml file
  • Code style update (formatting, renaming)
  • Refactoring (please detail in commit messages)
  • Build related changes
  • Documentation content changes
    • ChangeLog updated (for major changes)
  • Other (please describe):

New infix-system augment of the ietf-system NTP server list, written
to the generated chronyd sources.  Defaults match the daemon (6/10).

Signed-off-by: Mattias Walström <lazzer@gmail.com>
Replace BusyBox ntpd in the netns server helper with chronyd, serving
local stratum 5 with an honest root distance, and fail loudly if it
cannot start.  Wait for srv2 to converge on srv1 before configuring
the client, and use 16 s polls to cut worst-case runtime.

Signed-off-by: Mattias Walström <lazzer@gmail.com>
@mattiaswal
mattiaswal marked this pull request as ready for review September 2, 2026 12:03
@mattiaswal mattiaswal changed the title WiP: Try to fix NTP flakiness Add minpoll/maxpoll for ntp client Sep 2, 2026

@troglobit troglobit left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Simply adopting the Chrony naming somehow feels wrong. Can't put my finger on it but I was sort of expecting a CLI syntax more like:

set poll min A max B

We also need to gate these settings to client mode with a must or when expression.

@mattiaswal

Copy link
Copy Markdown
Contributor Author

Simply adopting the Chrony naming somehow feels wrong. Can't put my finger on it but I was sort of expecting a CLI syntax more like:

set poll min A max B

We also need to gate these settings to client mode with a must or when expression.

Simply adopting the Chrony naming somehow feels wrong. Can't put my finger on it but I was sort of expecting a CLI syntax more like:

set poll min A max B

We also need to gate these settings to client mode with a must or when expression.

It is already limited to client since it is in ietf-system.

@mattiaswal

Copy link
Copy Markdown
Contributor Author

Simply adopting the Chrony naming somehow feels wrong. Can't put my finger on it but I was sort of expecting a CLI syntax more like:

set poll min A max B

We also need to gate these settings to client mode with a must or when expression.

And changing from maxpoll/minpoll diverge from how ietf-ntp implements it:

 grouping common-attributes {
    description
      "NTP common attributes for configuration";
    leaf minpoll {
      type log2seconds;
      default "6";
      description
        "The minimum poll interval used in this association";
      reference
        "RFC 5905: Network Time Protocol Version 4: Protocol and
         Algorithms Specification, Section 7.2";
    }
    leaf maxpoll {
      type log2seconds;
      default "10";
      description
        "The maximum poll interval used in this association";
      reference
        "RFC 5905: Network Time Protocol Version 4: Protocol and
         Algorithms Specification, Section 7.2";
    }
    leaf port {
      if-feature "ntp-port";
      type inet:port-number {
        range "123 | 1024..max";
      }
      default "123";
      description

@mattiaswal mattiaswal added the ci:main Build default defconfig, not minimal label Sep 2, 2026
@troglobit

Copy link
Copy Markdown
Contributor

Ah, ietf-system, I missed that tiny detail, thanks! And if the standard model has minpoll/maxpoll that's fine too, always better to optimize for consistency over perfection.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ci:main Build default defconfig, not minimal

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants