p2p/discover: emphasize warning, add 10 min cooldown

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
 			}
 		}