From ac954f48bd9f9897f3118782ff2e68eeb3e1ed4e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?P=C3=A9ter=20Szil=C3=A1gyi?= Date: Fri, 19 Feb 2016 16:18:55 +0200 Subject: [PATCH] p2p/discover: emphasize warning, add 10 min cooldown --- p2p/discover/ntp.go | 32 ++++++++++++++++++++++++++++++-- p2p/discover/udp.go | 25 +++++++++---------------- 2 files changed, 39 insertions(+), 18 deletions(-) diff --git a/p2p/discover/ntp.go b/p2p/discover/ntp.go index a87ee1fbe..c1a4b3af1 100644 --- a/p2p/discover/ntp.go +++ b/p2p/discover/ntp.go @@ -20,13 +20,20 @@ package discover import ( + "fmt" "net" "sort" + "strings" "time" + + "github.com/ethereum/go-ethereum/logger" + "github.com/ethereum/go-ethereum/logger/glog" ) -// ntpPool is the NTP server to query for the current time -const ntpPool = "pool.ntp.org" +const ( + ntpPool = "pool.ntp.org" // ntpPool is the NTP server to query for the current time + ntpChecks = 3 // Number of measurements to do against the NTP server +) // durationSlice attaches the methods of sort.Interface to []time.Duration, // sorting in increasing order. @@ -36,6 +43,27 @@ func (s durationSlice) Len() int { return len(s) } func (s durationSlice) Less(i, j int) bool { return s[i] < s[j] } func (s durationSlice) Swap(i, j int) { s[i], s[j] = s[j], s[i] } +// checkClockDrift queries an NTP server for clock drifts and warns the user if +// one large enough is detected. +func checkClockDrift() { + drift, err := sntpDrift(ntpChecks) + if err != nil { + return + } + if drift < -driftThreshold || drift > driftThreshold { + warning := fmt.Sprintf("System clock seems off by %v, which can prevent network connectivity", drift) + howtofix := fmt.Sprintf("Please enable network time synchronisation in system settings") + separator := strings.Repeat("-", len(warning)) + + glog.V(logger.Warn).Info(separator) + glog.V(logger.Warn).Info(warning) + glog.V(logger.Warn).Info(howtofix) + glog.V(logger.Warn).Info(separator) + } else { + glog.V(logger.Debug).Infof("Sanity NTP check reported %v drift, all ok", drift) + } +} + // sntpDrift does a naive time resolution against an NTP server and returns the // measured drift. This method uses the simple version of NTP. It's not precise // but should be fine for these purposes. diff --git a/p2p/discover/udp.go b/p2p/discover/udp.go index fa4bc07dc..03fa0c348 100644 --- a/p2p/discover/udp.go +++ b/p2p/discover/udp.go @@ -52,8 +52,9 @@ const ( sendTimeout = 500 * time.Millisecond expiration = 20 * time.Second - ntpThreshold = 32 // Continuous timeouts after which to check NTP - driftThreshold = 10 * time.Second // Allowed clock drift before warning user + ntpFailureThreshold = 32 // Continuous timeouts after which to check NTP + ntpWarningCooldown = 10 * time.Minute // Minimum amount of time to pass before repeating NTP warning + driftThreshold = 10 * time.Second // Allowed clock drift before warning user ) // RPC packet types @@ -327,6 +328,7 @@ func (t *udp) loop() { timeout = time.NewTimer(0) nextTimeout *pending // head of plist when timeout was last reset contTimeouts = 0 // number of continuous timeouts to do NTP checks + ntpWarnTime = time.Unix(0, 0) ) <-timeout.C // ignore first timeout defer timeout.Stop() @@ -400,20 +402,11 @@ func (t *udp) loop() { } } // If we've accumulated too many timeouts, do an NTP time sync check - if contTimeouts > ntpThreshold { - go func() { - drift, err := sntpDrift(3) - switch { - case err != nil: - glog.V(logger.Warn).Infof("No UDP connectivity, maybe blocked by firewall? (%v)", err) - - case drift < -driftThreshold || drift > driftThreshold: - glog.V(logger.Warn).Infof("System clock seems off by %v, which can prevent network connectivity", drift) - - default: - glog.V(logger.Debug).Infof("Sanity NTP check reported %v drift, all ok", drift) - } - }() + if contTimeouts > ntpFailureThreshold { + if time.Since(ntpWarnTime) >= ntpWarningCooldown { + ntpWarnTime = time.Now() + go checkClockDrift() + } contTimeouts = 0 } }