feat/DHCP NTP Servers - Enhances time sync with DHCP NTP and custom servers #625

IDisposable · 2025-06-19T01:37:13Z

Extends time synchronization capabilities by:

Adding sup 10000 port for obtaining NTP server addresses from DHCP leases.
Enabling the use of user-defined NTP and HTTP servers for time synchronization.
Introduces a configurable time sync ordering, allowing users to specify the preferred order of time sources.

Eliminates (mostly) duplicate code

Since the ordering may have been previously defaulted and saved as "ntp,http", but that was being ignored and fallback-defaults were being used, in Ordering, `ntp` means use the fallback NTP servers, and `http` means use the fallback HTTP URLs. Thus `ntp_user_provided` and `http_user_provided` are the user specified static lists.

IDisposable · 2025-06-19T01:40:59Z

Fleshing out the device-side support for the configuration options added in #361

Still need the configuration UI to be finished, but this respects all the settings allowed in the config file https://github.com/jetkvm/kvm/pull/361/files#diff-5850db209b26ffd9fd3630b5255df6e30442d6cdfd7bcb6174c60924608dcc45R47-R50

IDisposable · 2025-06-19T01:44:36Z

internal/timesync/http.go

-	chunkSize := 4
-	httpUrls := t.httpUrls
+func (t *TimeSync) queryAllHttpTime(httpUrls []string) (now *time.Time) {
+	chunkSize := int(t.networkConfig.TimeSyncParallel.ValueOr(4))


Honors the setting of the number of parallel time-sync requests (defaults to 4)

IDisposable · 2025-06-19T01:45:05Z

internal/network/netif.go

@@ -89,7 +91,7 @@ func NewNetworkInterfaceState(opts *NetworkInterfaceOptions) (*NetworkInterfaceS
 				opts.Logger.Error().Err(err).Msg("failed to update network state")
 				return
 			}
-
+			_ = s.updateNtpServersFromLease(lease)


Every time we get a new DHCP lease, we need to copy over the NTP servers (if any) to the NetworkInterfaceState for access inside the time-sync logic.

IDisposable · 2025-06-19T01:53:44Z

internal/confparser/confparser_test.go

@@ -43,9 +43,11 @@ type testNetworkConfig struct {
 	LLDPTxTLVs              []string    `json:"lldp_tx_tlvs,omitempty" one_of:"chassis,port,system,vlan" default:"chassis,port,system,vlan"`
 	MDNSMode                null.String `json:"mdns_mode,omitempty" one_of:"disabled,auto,ipv4_only,ipv6_only" default:"auto"`
 	TimeSyncMode            null.String `json:"time_sync_mode,omitempty" one_of:"ntp_only,ntp_and_http,http_only,custom" default:"ntp_and_http"`
-	TimeSyncOrdering        []string    `json:"time_sync_ordering,omitempty" one_of:"http,ntp,ntp_dhcp,ntp_user_provided,ntp_fallback" default:"ntp,http"`
+	TimeSyncOrdering        []string    `json:"time_sync_ordering,omitempty" one_of:"http,ntp,ntp_dhcp,ntp_user_provided,http_user_provided" default:"ntp,http"`


Since anyone who ran a version that included the previous code would have gotten the defaults written to their device's config file, we should behave the same way as before until they change the setting (there's still no UI for that, but that's a different PR).

So, the old behavior was to try NTP on the default servers, then HTTP on the default servers. This means that we should interpret the saved config line for ntp,http as if they requested the default servers.

Thus ntp means the prior default NTP servers and we need ntp_user_provided to specify user-supplied values.
The same logic applies to the HTTP URLs, so http refers to the prior default list of HTTP URLs.

Ideally, we probably would like the default ordering to be ntp_dhcp. then ntp, then http but that ship has sailed for anyone running 0.4.x. We might want to change the defaults here for future users.

Thus we added the http_user_provided option and deleted the redundant ntp_fallback option here to match

IDisposable · 2025-06-19T01:56:11Z

internal/confparser/confparser_test.go

 	TimeSyncDisableFallback null.Bool   `json:"time_sync_disable_fallback,omitempty" default:"false"`
 	TimeSyncParallel        null.Int    `json:"time_sync_parallel,omitempty" default:"4"`
+	TimeSyncNTPServers      []string    `json:"time_sync_ntp_servers,omitempty" validate_type:"ipv4_or_ipv6" required_if:"TimeSyncOrdering=ntp_user_provided"`


These are where the user's user-provided NTP Server and HTTP URL values can be store. The required_if is likely not completely correct but changing the confparser seems way out of scope here. It will do the right thing iff the user selected only ntp_user_provided in the TimeSyncOrdering but not if they have it included in the string somewhere. I think @ym might know how to express that (includes not equals), but not there now.

IDisposable · 2025-06-19T02:00:35Z

internal/timesync/ntp.go

@@ -46,6 +47,10 @@ type ntpResult struct {

 func (t *TimeSync) queryMultipleNTP(servers []string, timeout time.Duration) (now *time.Time, offset *time.Duration) {
 	results := make(chan *ntpResult, len(servers))
+
+	_, cancel := context.WithTimeout(context.Background(), timeout)


Add logic to cancel all outstanding NTP queries when we process a valid reply or end this chunk. This parallels logic in the HTTP path, and might not be desired.

IDisposable · 2025-06-19T02:05:22Z

internal/timesync/ntp.go

@@ -66,15 +71,25 @@ func (t *TimeSync) queryMultipleNTP(servers []string, timeout time.Duration) (no
 				return
 			}

+			if response.IsKissOfDeath() {


We really need to make sure the NTP server we were talking to didn't tell us to go away. https://support.ntp.org/Dev/KoDResponses

IDisposable · 2025-06-19T02:06:32Z

internal/timesync/timesync.go

-		rtcDevicePath: rtcDevice,
-		rtcLock:       &sync.Mutex{},
-		preCheckFunc:  opts.PreCheckFunc,
-		ntpServers:    defaultNTPServers,


We don't need these two ntpServers and httpUrls in the TimeSync struct as they're available to the time-sync code.

IDisposable · 2025-06-19T02:11:38Z

internal/timesync/timesync.go

 func (t *TimeSync) getSyncMode() SyncMode {
 	syncMode := SyncMode{
+		Ntp:             true,
+		Http:            true,
+		Ordering:        []string{"ntp_dhcp", "ntp", "http"},


This default ordering is a lie, unless there was no stored config, because that will overwrite this defaulting... but just in case, use the DHCP provided NTP servers, then the NTP default servers, then the default HTTP URLs

IDisposable · 2025-06-19T02:12:20Z

internal/timesync/timesync.go

-	if syncMode.Http && now == nil {
-		now = t.queryAllHttpTime()
+Orders:
+	for _, mode := range syncMode.Ordering {


Do the options in the order they specified.

IDisposable · 2025-06-19T02:12:40Z

internal/timesync/timesync.go

+		case "ntp_user_provided":
+			if syncMode.Ntp {
+				t.l.Info().Msg("using NTP custom servers")
+				now, offset = t.queryNetworkTime(t.networkConfig.TimeSyncNTPServers)


NTP servers from the config (will be empty until the UI is coded)

IDisposable · 2025-06-19T02:12:52Z

internal/timesync/timesync.go

+		case "ntp_dhcp":
+			if syncMode.Ntp {
+				t.l.Info().Msg("using NTP servers from DHCP")
+				now, offset = t.queryNetworkTime(t.dhcpNtpAddresses)


NTP servers from the DHCP lease

IDisposable · 2025-06-19T02:13:22Z

internal/timesync/timesync.go

+		case "ntp":
+			if syncMode.Ntp && syncMode.NtpUseFallback {
+				t.l.Info().Msg("using NTP fallback")
+				now, offset = t.queryNetworkTime(defaultNTPServers)


NTP servers from the code-defined defaults (fallbacks)

IDisposable · 2025-06-19T02:13:37Z

internal/timesync/timesync.go

+		case "http":
+			if syncMode.Http && syncMode.HttpUseFallback {
+				t.l.Info().Msg("using HTTP fallback")
+				now = t.queryAllHttpTime(defaultHTTPUrls)


HTTP URLs from the code-defined defaults (fallbacks)

IDisposable · 2025-06-19T02:14:10Z

internal/timesync/timesync.go

+		case "http_user_provided":
+			if syncMode.Http {
+				t.l.Info().Msg("using HTTP custom URLs")
+				now = t.queryAllHttpTime(t.networkConfig.TimeSyncHTTPUrls)


HTTP URLs from the config (will be empty until the UI is coded)

IDisposable · 2025-06-19T02:14:46Z

internal/timesync/timesync.go

+				}
+			}
+		case "http_user_provided":
+			if syncMode.Http {


Note, that http means the prior HTTP URL default list.

IDisposable · 2025-06-19T02:15:08Z

internal/timesync/timesync.go

+					break Orders
+				}
+			}
+		case "ntp":


Note that ntp means the prior default NTP server list.

IDisposable · 2025-06-19T02:15:59Z

network.go

@@ -19,8 +19,17 @@ func networkStateChanged() {
 	// do not block the main thread
 	go waitCtrlAndRequestDisplayUpdate(true)

+	if timeSync != nil {


These nil checks are necessary because we can hit networkStateChanged before we've even finished setting up.

IDisposable · 2025-06-19T02:20:09Z

This is step one of two to enable all the rest of the NTP configurable behavior that was started in #361. Hopefully this is helpful.

IDisposable · 2025-06-19T15:26:34Z

If this is merged first, PR #609 will need to be adjusted (or just closed)

IDisposable added 2 commits June 18, 2025 12:38

Ensure the mDNS mode is set every time network state changes

466bf40

Eliminates (mostly) duplicate code

IDisposable commented Jun 19, 2025

View reviewed changes

IDisposable force-pushed the feat/dhcp-ntp-servers branch from 873fc91 to bdd8281 Compare June 19, 2025 01:49

IDisposable commented Jun 19, 2025

View reviewed changes

IDisposable force-pushed the feat/dhcp-ntp-servers branch from bdd8281 to 57e9a65 Compare June 19, 2025 01:57

IDisposable commented Jun 19, 2025

View reviewed changes

Add support for using DHCP-provided NTP server

926fcf2

IDisposable force-pushed the feat/dhcp-ntp-servers branch from 57e9a65 to 926fcf2 Compare June 19, 2025 02:10

IDisposable commented Jun 19, 2025

View reviewed changes

IDisposable mentioned this pull request Jun 19, 2025

fix/timesync: Make timesync work with fallbacks and repolling #609

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

feat/DHCP NTP Servers - Enhances time sync with DHCP NTP and custom servers #625

feat/DHCP NTP Servers - Enhances time sync with DHCP NTP and custom servers #625

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

feat/DHCP NTP Servers - Enhances time sync with DHCP NTP and custom servers #625

Are you sure you want to change the base?

feat/DHCP NTP Servers - Enhances time sync with DHCP NTP and custom servers #625

Conversation

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!