ntp bug fix
cover ntp bug fix
I have been cooking up a patch series to keep the core time in TAI instead of UTC. In the course of that work I noticed a bug in the current NTP code. During a leap second insertion, the kernel will advertise the wrong TAI offset.
There is a nice explanation of this issue in the NIST leap second list, which I quote below. With reference to that, the Linux kernel handles the UTC time value like in the first case, but sets the TAI offset as in the second case, resulting in a discontinuity in the apparent TAI time.
* Excerpt from ftp://time.nist.gov/pub/leap-seconds.list
If your system keeps time as the number of seconds since some epoch (e.g., NTP timestamps), then the algorithm for assigning a UTC time stamp to an event that happens during a positive leap second is not well defined. The official name of that leap second is 23:59:60, but there is no way of representing that time in these systems.
Many systems of this type effectively stop the system clock for one second during the leap second and use a time that is equivalent to 23:59:59 UTC twice. For these systems, the corresponding TAI timestamp would be obtained by advancing to the next entry in the following table when the time equivalent to 23:59:59 UTC is used for the second time. Thus the leap second which occurred on 30 June 1972 at 23:59:59 UTC would have TAI timestamps computed as follows:
30 June 1972 23:59:59 (2287785599, first time): TAI= UTC + 10 seconds 30 June 1972 23:59:60 (2287785599,second time): TAI= UTC + 11 seconds 1 July 1972 00:00:00 (2287785600) TAI= UTC + 11 seconds
If your system realizes the leap second by repeating 00:00:00 UTC twice (this is possible but not usual), then the advance to the next entry in the table must occur the second time that a time equivlent to 00:00:00 UTC is used. Thus, using the same example as above:
30 June 1972 23:59:59 (2287785599): TAI= UTC + 10 seconds 30 June 1972 23:59:60 (2287785600, first time): TAI= UTC + 10 seconds 1 July 1972 00:00:00 (2287785600,second time): TAI= UTC + 11 seconds
in both cases the use of timestamps based on TAI produces a smooth time scale with no discontinuity in the time interval.
Richard Cochran (1):
ntp: advertise correct TAI offset during leap second
kernel/time/ntp.c | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-)
Signature · 1 line
1.7.2.5
PATCH 1/1 ntp: advertise correct TAI offset during leap second
When repeating a UTC time value during a leap second (when the UTC time should be 23:59:60), the TAI timescale should not stop. The kernel NTP code increments the TAI offset one second too late. This patch fixes the issue by incrementing the offset during the leap second itself.
--- kernel/time/ntp.c | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-)
kernel/time/ntp.c +1 -1
@@ -412,6 +412,7 @@ int second_overflow(unsigned long secs)if (secs %86400 ==0 ) { leap =-1 ; time_state =TIME_OOP ; + time_tai++;printk (KERN_NOTICE "Clock: inserting leap second 23:59:60 UTC\n" ); }
@@ -426,7 +427,6 @@ int second_overflow(unsigned long secs) }break ;case TIME_OOP : - time_tai++; time_state =TIME_WAIT ;break ;
Signature · 1 line
1.7.2.5
Re: [PATCH 1/1] ntp: advertise correct TAI offset during leap second
Quoted 6 lines · Richard Cochran
When repeating a UTC time value during a leap second (when the UTC time should be 23:59:60), the TAI timescale should not stop. The kernel NTP code increments the TAI offset one second too late. This patch fixes the issue by incrementing the offset during the leap second itself.
Signed-off-by Richard Cochran<[email protected]>
This looks good to me. Although, have you actually tested against an ntp client that sets the tai offset to make sure you're not duplicating any ADJ_TAI adjustment it might make?
thanks
-john
Re: [PATCH 1/1] ntp: advertise correct TAI offset during leap second
Quoted 11 lines · John Stultz
Quoted 6 lines · Richard Cochran
When repeating a UTC time value during a leap second (when the UTC time should be 23:59:60), the TAI timescale should not stop. The kernel NTP code increments the TAI offset one second too late. This patch fixes the issue by incrementing the offset during the leap second itself.
Signed-off-by Richard Cochran<[email protected]>This looks good to me. Although, have you actually tested against an ntp client that sets the tai offset to make sure you're not duplicating any
ADJ_TAIadjustment it might make?
No, I cooked up my own test program that uses the adjtimex interface directly. I really am not very familiar with the ntp.org software.
Wait a minute. If user space manages this variable, then shouldn't the kernel leave it alone?
This David Mills paper [1] gives a leap second example that does it the "other" way from Linux (see Figure 4), repeating the new epoch rather than the leap second. It may well be that ntp.org servers do behave that way. However, the NIST file claims that this way is unusual.
So, you have a good question. But, if ntp.org uses the NIST second method, shouldn't Linux do the same?
Thanks,
Richard
1. http://www.eecis.udel.edu/~mills/database/papers/time.pdf
Re: [PATCH 1/1] ntp: advertise correct TAI offset during leap second
Quoted 8 lines · Richard Cochran
Quoted 7 lines · John Stultz
Quoted 6 lines · Richard Cochran
When repeating a UTC time value during a leap second (when the UTC time should be 23:59:60), the TAI timescale should not stop. The kernel NTP code increments the TAI offset one second too late. This patch fixes the issue by incrementing the offset during the leap second itself.
Signed-off-by Richard Cochran<[email protected]>
Quoted 8 lines
Quoted 3 lines
This looks good to me. Although, have you actually tested against an ntp client that sets the tai offset to make sure you're not duplicating any
ADJ_TAIadjustment it might make?No, I cooked up my own test program that uses the adjtimex interface directly. I really am not very familiar with the ntp.org software.
Wait a minute. If user space manages this variable, then shouldn't the kernel leave it alone?
Right. That's why I'm asking. I actually haven't spent much time looking at how the tai value provided via adjtimex is handled, and I want to make sure its ok if we modify it from the kernel.
Quoted 9 lines
This David Mills paper [1] gives a leap second example that does it the "other" way from Linux (see Figure 4), repeating the new epoch rather than the leap second. It may well be that ntp.org servers do behave that way. However, the NIST file claims that this way is unusual.
So, you have a good question. But, if ntp.org uses the NIST second method, shouldn't Linux do the same?
Not sure I'm following here. In Linux 23:59:60 is represented as 23:59:59 + TIME_OOP. Could you expand on what in particular is inconsistent here?
thanks
-john
Re: [PATCH 1/1] ntp: advertise correct TAI offset during leap second
Quoted 8 lines · John Stultz
Quoted 3 lines · Richard Cochran
Wait a minute. If user space manages this variable, then shouldn't the kernel leave it alone?
Right. That's why I'm asking. I actually haven't spent much time looking at how the tai value provided via adjtimex is handled, and I want to make sure its ok if we modify it from the kernel.
We *are* already modifying it in kernel, but at the wrong time.
I don't know either what ntpd does, but I will find out.
[ But if ntpd just uses adjtimex() to use the kernel as storage for
variables, then that is really stupid. ]
Quoted 6 lines
Quoted 6 lines
This David Mills paper [1] gives a leap second example that does it the "other" way from Linux (see Figure 4), repeating the new epoch rather than the leap second. It may well be that ntp.org servers do behave that way. However, the NIST file claims that this way is unusual.
Quoted 6 lines
Quoted 3 lines
So, you have a good question. But, if ntp.org uses the NIST second method, shouldn't Linux do the same?
Not sure I'm following here. In Linux 23:59:60 is represented as 23:59:59 +
TIME_OOP. Could you expand on what in particular is inconsistent here?
I am talking about the numerical values of time_t. The NIST note explains it well. There are two ways to insert a leap second, and neither way is specified by any standard, or at least I think that is what the note is saying.
So you can have either sequences [1] or [2] during a leap second, but not [3], which is what you are doing. The Mills paper has a table that looks like [2], but the NIST note says that is not usual. So, I wonder what ntpd broadcasts during a leap second.
NTP OFF TAI UTC STATUS
[1] ----------------------------------------------------
2287785598 10 78796808 78796798 ins
2287785599 10 78796809 78796799 ins
2287785599 11 78796810 78796799 oop
2287785600 11 78796811 78796800 wait
2287785601 11 78796812 78796801 wait
[2] ----------------------------------------------------
2287785598 10 78796808 78796798 ins
2287785599 10 78796809 78796799 ins
2287785600 10 78796810 78796800 oop
2287785600 11 78796811 78796800 wait
2287785601 11 78796812 78796801 wait
[3] ----------------------------------------------------
2287785598 10 78796808 78796798 ins
2287785599 10 78796809 78796799 ins
2287785599 10 78796809 * 78796799 oop
2287785600 11 78796811 * 78796800 wait
2287785601 11 78796812 78796801 wait* Notice apparent jump in TAI value.
Thanks,
Richard
Re: [PATCH 1/1] ntp: advertise correct TAI offset during leap second
Quoted 10 lines · Richard Cochran
Quoted 8 lines · John Stultz
Quoted 3 lines · Richard Cochran
Wait a minute. If user space manages this variable, then shouldn't the kernel leave it alone?
Right. That's why I'm asking. I actually haven't spent much time looking at how the tai value provided via adjtimex is handled, and I want to make sure its ok if we modify it from the kernel.
Quoted 6 lines
We *are* already modifying it in kernel, but at the wrong time.
I don't know either what ntpd does, but I will find out.
[ But if ntpd just uses
adjtimex()to use the kernel as storage for
variables, then that is really stupid. ]
I took a look at ntp-4.2.6p5, and ntpd unconditionally sets the TAI offset using MOD_TAI when it thinks the offset has changed.
But I think it won't hurt if the kernel changes the offset automatically, since ntpd will just try and set the same value again.
Thanks,
Richard
Re: [PATCH 1/1] ntp: advertise correct TAI offset during leap second
Quoted 12 lines · Richard Cochran
Quoted 11 lines · Richard Cochran
Quoted 6 lines · John Stultz
Quoted 2 lines · Richard Cochran
Wait a minute. If user space manages this variable, then shouldn't the kernel leave it alone?
Right. That's why I'm asking. I actually haven't spent much time looking at how the tai value provided via adjtimex is handled, and I want to make sure its ok if we modify it from the kernel.
We *are* already modifying it in kernel, but at the wrong time.
I don't know either what ntpd does, but I will find out.
Quoted 7 lines
Quoted 2 lines
[ But if ntpd just uses
adjtimex()to use the kernel as storage for
variables, then that is really stupid. ]I took a look at ntp-4.2.6p5, and ntpd unconditionally sets the TAI offset using
MOD_TAIwhen it thinks the offset has changed.But I think it won't hurt if the kernel changes the offset automatically, since ntpd will just try and set the same value again.
Assuming that it doesn't happen right before the tick that adjusts the leapsecond. :) But that's a pre-existing bug and not a reason to block your patch that improves things.
Thanks for the extra due-diligence here!
thanks
-john
Re: [PATCH 1/1] ntp: advertise correct TAI offset during leap second
In reply to Richard CochranQuoted 4 lines · Richard Cochran
When repeating a UTC time value during a leap second (when the UTC time should be 23:59:60), the TAI timescale should not stop. The kernel NTP code increments the TAI offset one second too late. This patch fixes the issue by incrementing the offset during the leap second itself.
Got a chance to look at this closer and I agree its the right fix. So its queued. thanks
-john
Quoted 25 lines
Signed-off-by Richard Cochran<[email protected]>--- kernel/time/ntp.c | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-)kernel/time/ntp.c +1
-1@@ -412,6 +412,7 @@ int second_overflow(unsigned long secs)if (secs %86400 ==0 ) { leap =-1 ; time_state =TIME_OOP ; + time_tai++;printk (KERN_NOTICE "Clock: inserting leap second 23:59:60 UTC\n" ); }@@ -426,7 +427,6 @@ int second_overflow(unsigned long secs) }break ;case TIME_OOP : - time_tai++; time_state =TIME_WAIT ;break ;