From 07de9e16af8e520334766cd1b62346a24623d40e Mon Sep 17 00:00:00 2001
From: Matthew Dillon <dillon@dragonflybsd.org>
Date: Mon, 26 Jan 2004 20:58:11 +0000
Subject: [PATCH] Fix a serious bug in the NTPD loopfilter.  Basically what
 happens is that when the frequency error is low (e.g. like 15ppm), but an
 offset correction must be made (e.g. like 3 seconds), the rstclock() call
 loopfilter makes in the S_NSET state, after adjusting the system time, will
 reload a clock_offset value PRIOR to the system time adjustment actually
 having taken place, because rstclock() uses offset information prior to the
 time step, not after, and because the kernel may slew the time step over
 several seconds.

The result is that clock_offset will wind up being very high (e.g. like 3
seconds) as the loopfilter enters into the S_FREQ state.  This will cause
the loopfilter to make a wildly incorrect clock_frequency calculation and
in my tests it can take ntpd and the kernel upwards of one to two DAYS to
correct for the bad calculation.

The fix is to introduce an offset settling state, S_PFREQ, which resets
the offset calculations CLOCK_MINSEC seconds after the correction was made
in order to give the next state, S_FREQ, a good basis for calculating the
frequency drift.  clock_offset will be far more reasonable when the S_FREQ
state is entered, a good clock_frequency calculation will be made, and
ntpd will lock onto the frequency is less then half an hour (instead of
24+ hours).
---
 contrib/ntp/ntpd/ntp_loopfilter.c | 44 +++++++++++++++++++++++++++----
 1 file changed, 39 insertions(+), 5 deletions(-)

diff --git a/contrib/ntp/ntpd/ntp_loopfilter.c b/contrib/ntp/ntpd/ntp_loopfilter.c
index c8a86cf12b..fe15ea600b 100644
--- a/contrib/ntp/ntpd/ntp_loopfilter.c
+++ b/contrib/ntp/ntpd/ntp_loopfilter.c
@@ -74,9 +74,10 @@
 #define S_NSET	0		/* clock never set */
 #define S_FSET	1		/* frequency set from the drift file */
 #define S_TSET	2		/* time set */
-#define S_FREQ	3		/* frequency mode */
-#define S_SYNC	4		/* clock synchronized */
-#define S_SPIK	5		/* spike detected */
+#define S_PFREQ	3		/* frequency setup mode */
+#define S_FREQ	4		/* frequency mode */
+#define S_SYNC	5		/* clock synchronized */
+#define S_SPIK	6		/* spike detected */
 
 /*
  * Kernel PLL/PPS state machine. This is used with the kernel PLL
@@ -283,7 +284,7 @@ local_clock(
 		step_systime(fp_offset);
 		NLOG(NLOG_SYNCEVENT|NLOG_SYSEVENT)
 		    msyslog(LOG_NOTICE, "time set %.6f s", fp_offset);
-		rstclock(S_FREQ, peer->epoch, fp_offset);
+		rstclock(S_PFREQ, peer->epoch, fp_offset);
 		return (1);
 	}
 
@@ -336,6 +337,10 @@ local_clock(
 		sys_poll = peer->minpoll;
 	clock_frequency = flladj = plladj = 0;
 	mu = peer->epoch - last_time;
+#if 0
+	msyslog(LOG_ERR, "loopfilterX fp_offset %lg clock_offset %lg mu %lg clock_max %lg state %d",
+		fp_offset, clock_offset, mu, clock_max, state);
+#endif
 	if (fabs(fp_offset) > clock_max && clock_max > 0) {
 		switch (state) {
 
@@ -347,7 +352,7 @@ local_clock(
 		 * to S_FREQ state.
 		 */
 		case S_TSET:
-			state = S_FREQ;
+			state = S_PFREQ;
 			break;
 
 		/*
@@ -360,6 +365,15 @@ local_clock(
 				return (0);
 			state = S_SPIK;
 			return (0);
+		/*
+		 * We do not want to include the offset from a prior 
+		 * offset adjustment in our frequency calculation,
+		 * so supply some settling time.
+		 */
+		case S_PFREQ:
+			if (mu >= CLOCK_MINSEC)
+				rstclock(S_FREQ, peer->epoch, fp_offset);
+			return (0);
 
 		/*
 		 * In S_FREQ state we ignore outlyers. At the first
@@ -414,6 +428,15 @@ local_clock(
 		case S_FSET:
 			rstclock(S_TSET, peer->epoch, fp_offset);
 			break;
+		/*
+		 * We do not want to include the offset from a prior 
+		 * offset adjustment in our frequency calculation,
+		 * so supply some settling time.
+		 */
+		case S_PFREQ:
+			if (mu >= CLOCK_MINSEC)
+				rstclock(S_FREQ, peer->epoch, fp_offset);
+			return (0);
 
 		/*
 		 * In S_FREQ state ignore updates until the stepout
@@ -534,10 +557,18 @@ local_clock(
 				    1e6 + dtemp);
 				ntv.constant = sys_poll - 4;
 			}
+#if 0
+			msyslog(LOG_ERR, "loopfilter2 offset %d components: %lg %lg",
+				ntv.offset, clock_offset * 1e9, dtemp);
+#endif
 			if (clock_frequency != 0) {
 				ntv.modes |= MOD_FREQUENCY;
 				ntv.freq = (int32)((clock_frequency +
 				    drift_comp) * 65536e6);
+#if 0
+				msyslog(LOG_ERR, "loopfilter2 freq %d components: %lg %lg",
+					ntv.freq, clock_frequency, drift_comp);
+#endif
 			}
 			ntv.esterror = (u_int32)(sys_jitter * 1e6);
 			ntv.maxerror = (u_int32)((sys_rootdelay / 2 +
@@ -889,6 +920,9 @@ loop_config(
 				ntv.freq = (int32)(drift_comp *
 				    65536e6);
 			}
+			msyslog(LOG_ERR, "loopfilter1 offset %d freq %d (%lg)",
+				ntv.offset, ntv.freq, drift_comp
+			);
 			(void)ntp_adjtime(&ntv);
 		}
 #endif /* KERNEL_PLL */
-- 
2.32.0