Re: [NTP 8/9] convert to the NTP4 reference model

Re: [NTP 8/9] convert to the NTP4 reference model

[linux]

>>> For the ntp ignorami amongst us...  what is a "nanokernel reference
>>> implementation" and why do we want one?
> 
> It's the behaviour the current ntp daemon expects, the ntp documentation 
> has more information and a link to the package (e.g. under Debian at 
> /usr/share/doc/ntp-doc/html/kern.html).
>
> So...  the current kernel is behaving in a manner other than that which the
> NTP daemon expects?  Does this cause any problems?

To explain more clearly...

The original NTP kernel interface was defined in units of microseconds.
That's what Linux implements.  As computers have gotten faster and can
now split microseconds easily, a new kernel interface using nanosecond
units was defined ("the nanokernel", confusing as that name is to OS
hackers), and there's an STA_NANO bit in the adjtimex() status field to
tell the application which units it's using.

The current ntpd supports both, but Linux loses some possible timing
resolution because of quantization effects, and the ntpd hackers would
really like to be able to drop the backwards compatibility code.

Ulrich Windl has been maintaining a patch set to do the conversion for
years, but it's hard to keep in sync.

Re: [NTP 8/9] convert to the NTP4 reference model

[Roman Zippel]

Hi,

On Thu, 10 Aug 2006, linux@horizon.com wrote:

> The original NTP kernel interface was defined in units of microseconds.
> That's what Linux implements.  As computers have gotten faster and can
> now split microseconds easily, a new kernel interface using nanosecond
> units was defined ("the nanokernel", confusing as that name is to OS
> hackers), and there's an STA_NANO bit in the adjtimex() status field to
> tell the application which units it's using.

BTW this patch didn't add the STA_NANO bits yet, as that requires changes 
to <sys/timex.h> and a recompiled ntpd, so the values are still in usec.
It's not difficult to change/add, but one has to be a little careful about 
compatibility.

> The current ntpd supports both, but Linux loses some possible timing
> resolution because of quantization effects, and the ntpd hackers would
> really like to be able to drop the backwards compatibility code.
> 
> Ulrich Windl has been maintaining a patch set to do the conversion for
> years, but it's hard to keep in sync.

I think you refer to the PPSKit? Well, the first step to integrate it 
properly is done. Ulrich took the reference code rather unmodified, which 
has a rather strong emphasis on weird portability (one only has to look at 
l_fp.h to know what I mean), which is not really acceptable in the kernel. 
Its ioctl API probably should be converted to sysfs, so there is still 
some way to go.

bye, Roman

Re: [NTP 8/9] convert to the NTP4 reference model

[Roman Zippel]

Hi,

On Thu, 10 Aug 2006, Andrew Morton wrote:

> > It's the behaviour the current ntp daemon expects, the ntp documentation 
> > has more information and a link to the package (e.g. under Debian at 
> > /usr/share/doc/ntp-doc/html/kern.html).
> > 
> 
> So...  the current kernel is behaving in a manner other than that which the
> NTP daemon expects?  Does this cause any problems?

It's not drastically different, so for normal internet usage there is no 
big difference.
http://www.ntp.org/ntpfaq/NTP-s-compat.htm has a bit more information on 
this topic.

> I'm trying to work out what reason we might have for merging this patch.

It also allows us to sanely readd the PPS bits, where the changes were 
more significant.

bye, Roman

Re: [NTP 8/9] convert to the NTP4 reference model

[Andrew Morton]

On Thu, 10 Aug 2006 21:14:00 +0200 (CEST)
Roman Zippel <zippel@linux-m68k.org> wrote:

> Hi,
> 
> On Thu, 10 Aug 2006, Andrew Morton wrote:
> 
> > > This converts the kernel ntp model into a model which matches the
> > > nanokernel reference implementations.
> > 
> > For the ntp ignorami amongst us...  what is a "nanokernel reference
> > implementation" and why do we want one?
> 
> It's the behaviour the current ntp daemon expects, the ntp documentation 
> has more information and a link to the package (e.g. under Debian at 
> /usr/share/doc/ntp-doc/html/kern.html).
> 

So...  the current kernel is behaving in a manner other than that which the
NTP daemon expects?  Does this cause any problems?

I'm trying to work out what reason we might have for merging this patch.

Re: [NTP 8/9] convert to the NTP4 reference model

[Roman Zippel]

Hi,

On Thu, 10 Aug 2006, Andrew Morton wrote:

> > This converts the kernel ntp model into a model which matches the
> > nanokernel reference implementations.
> 
> For the ntp ignorami amongst us...  what is a "nanokernel reference
> implementation" and why do we want one?

It's the behaviour the current ntp daemon expects, the ntp documentation 
has more information and a link to the package (e.g. under Debian at 
/usr/share/doc/ntp-doc/html/kern.html).

bye, Roman

Re: [NTP 8/9] convert to the NTP4 reference model

[Andrew Morton]

On Thu, 10 Aug 2006 02:01:54 +0200
zippel@linux-m68k.org wrote:

> This converts the kernel ntp model into a model which matches the
> nanokernel reference implementations.

For the ntp ignorami amongst us...  what is a "nanokernel reference
implementation" and why do we want one?

Thanks.

[NTP 8/9] convert to the NTP4 reference model

[zippel]

[-- Attachment #1: 0008-NTP-convert-to-the-NTP4-reference-model.txt --]
[-- Type: text/plain, Size: 4974 bytes --]

This converts the kernel ntp model into a model which matches the
nanokernel reference implementations. The previous patches already
increased the resolution and precision of the computations, so that this
conversion becomes quite simple.

Signed-off-by: Roman Zippel <zippel@linux-m68k.org>
---
 include/linux/timex.h |   11 ++++------
 kernel/time/ntp.c     |   51 ++++++++++++++++++--------------------------------
 2 files changed, 24 insertions(+), 38 deletions(-)

Index: linux-2.6-mm/include/linux/timex.h
===================================================================
--- linux-2.6-mm.orig/include/linux/timex.h
+++ linux-2.6-mm/include/linux/timex.h
@@ -70,10 +70,9 @@
  * zero to MAXTC, the PLL will converge in 15 minutes to 16 hours,
  * respectively.
  */
-#define SHIFT_KG 6		/* phase factor (shift) */
-#define SHIFT_KF 16		/* PLL frequency factor (shift) */
-#define SHIFT_KH 2		/* FLL frequency factor (shift) */
-#define MAXTC 6			/* maximum time constant (shift) */
+#define SHIFT_PLL	4	/* PLL frequency factor (shift) */
+#define SHIFT_FLL	2	/* FLL frequency factor (shift) */
+#define MAXTC		10	/* maximum time constant (shift) */
 
 /*
  * The SHIFT_SCALE define establishes the decimal point of the time_phase
@@ -98,8 +97,8 @@
 #define MAXPHASE 512000L        /* max phase error (us) */
 #define MAXFREQ (512L << SHIFT_USEC)  /* max frequency error (ppm) */
 #define MAXFREQ_NSEC (512000L << SHIFT_NSEC) /* max frequency error (ppb) */
-#define MINSEC 16L              /* min interval between updates (s) */
-#define MAXSEC 1200L            /* max interval between updates (s) */
+#define MINSEC 256		/* min interval between updates (s) */
+#define MAXSEC 2048		/* max interval between updates (s) */
 #define	NTP_PHASE_LIMIT	(MAXPHASE << 5)	/* beyond max. dispersion */
 
 /*
Index: linux-2.6-mm/kernel/time/ntp.c
===================================================================
--- linux-2.6-mm.orig/kernel/time/ntp.c
+++ linux-2.6-mm/kernel/time/ntp.c
@@ -151,18 +151,11 @@ void second_overflow(void)
 	}
 
 	/*
-	 * Compute the phase adjustment for the next second. In PLL mode, the
-	 * offset is reduced by a fixed factor times the time constant. In FLL
-	 * mode the offset is used directly. In either mode, the maximum phase
-	 * adjustment for each second is clamped so as to spread the adjustment
-	 * over not more than the number of seconds between updates.
+	 * Compute the phase adjustment for the next second. The offset is
+	 * reduced by a fixed factor times the time constant.
 	 */
 	tick_length = tick_length_base;
-	time_adj = time_offset;
-	if (!(time_status & STA_FLL))
-		time_adj = shift_right(time_adj, SHIFT_KG + time_constant);
-	time_adj = min(time_adj, -((MAXPHASE / HZ) << SHIFT_UPDATE) / MINSEC);
-	time_adj = max(time_adj, ((MAXPHASE / HZ) << SHIFT_UPDATE) / MINSEC);
+	time_adj = shift_right(time_offset, SHIFT_PLL + time_constant);
 	time_offset -= time_adj;
 	tick_length += (s64)time_adj << (TICK_LENGTH_SHIFT - SHIFT_UPDATE);
 
@@ -206,7 +199,7 @@ void __attribute__ ((weak)) notify_arch_
 int do_adjtimex(struct timex *txc)
 {
 	long ltemp, mtemp, save_adjust;
-	s64 freq_adj;
+	s64 freq_adj, temp64;
 	int result;
 
 	/* In order to modify anything, you gotta be super-user! */
@@ -276,7 +269,7 @@ int do_adjtimex(struct timex *txc)
 		    result = -EINVAL;
 		    goto leave;
 		}
-		time_constant = txc->constant;
+		time_constant = min(txc->constant + 4, (long)MAXTC);
 	    }
 
 	    if (txc->modes & ADJ_OFFSET) {	/* values checked earlier */
@@ -304,26 +297,20 @@ int do_adjtimex(struct timex *txc)
 		        time_reftime = xtime.tv_sec;
 		    mtemp = xtime.tv_sec - time_reftime;
 		    time_reftime = xtime.tv_sec;
-		    freq_adj = 0;
-		    if (time_status & STA_FLL) {
-		        if (mtemp >= MINSEC) {
-			    freq_adj = (s64)time_offset << (SHIFT_NSEC - SHIFT_KH);
-			    if (time_offset < 0) {
-				freq_adj = -freq_adj;
-				do_div(freq_adj, mtemp);
-				freq_adj = -freq_adj;
-			    } else
-				do_div(freq_adj, mtemp);
-			} else /* calibration interval too short (p. 12) */
-				result = TIME_ERROR;
-		    } else {	/* PLL mode */
-		        if (mtemp < MAXSEC) {
-			    freq_adj = (s64)ltemp * mtemp;
-			    freq_adj = shift_right(freq_adj,(time_constant +
-						       time_constant +
-						       SHIFT_KF - SHIFT_NSEC));
-			} else /* calibration interval too long (p. 12) */
-				result = TIME_ERROR;
+
+		    freq_adj = (s64)time_offset * mtemp;
+		    freq_adj = shift_right(freq_adj, time_constant * 2 +
+					   (SHIFT_PLL + 2) * 2 - SHIFT_NSEC);
+		    if (mtemp >= MINSEC && (time_status & STA_FLL || mtemp > MAXSEC)) {
+			temp64 = (s64)time_offset << (SHIFT_NSEC - SHIFT_FLL);
+			if (time_offset < 0) {
+			    temp64 = -temp64;
+			    do_div(temp64, mtemp);
+			    freq_adj -= temp64;
+			} else {
+			    do_div(temp64, mtemp);
+			    freq_adj += temp64;
+			}
 		    }
 		    freq_adj += time_freq;
 		    freq_adj = min(freq_adj, (s64)MAXFREQ_NSEC);

--