[Fwd: Fwd: [PATCH] ntpd not adjusting clock after a step backward in time]
Denys Vlasenko
vda.linux at googlemail.com
Wed Jan 8 16:17:22 UTC 2014
On Wed, Jan 8, 2014 at 1:14 PM, John Spencer
<maillist-busybox at barfooze.de> wrote:
> forwarding this on behalf of Maxim Naumov who registered to the ML but is
> unable to send anything to the list despite having received the "welcome"
> mail. denys, maybe you can find out what's wrong with his registration ?
I'm looking into it... First, I need to reset admin password on the list :(
> ---------- Forwarded message ----------
> From: Maxim Naumov <muxx.dev at gmail.com>
> Date: 20 December 2013 13:29
> Subject: [PATCH] ntpd not adjusting clock after a step backward in time
> To: busybox at busybox.net
>
>
> hi everyone
>
> I found a problem where busybox ntpd will ignore datapoints after a step
> backwards is performed.
>
> to reproduce:
> 1. set system clock 1 year (or other large amount) ahead (in the future)
> 2. start ntpd -p <server>
>
> the system clock will be set to NTP time correctly, but kernel clock
> frequency will not be updated for the duration of the jump, i.e. if the
> system clock was 1 year ahead, ntpd will not correct for 1 year.
>
> this is because immediately after the step, set_new_values() resets
> G.last_update_recv_time with the original receive time of the packet (which
> is in future). therefore successive datapoints are ignored as too old.
>
> the fix is not to use recv_time for set_new_values() but to use
> G.last_update_recv_time already updated by step_time() called previously.
>
> here's the patch:
>
> ---
> diff --git a/networking/ntpd.c b/networking/ntpd.c
> index ed83415..32d0dcb 100644
> --- a/networking/ntpd.c
> +++ b/networking/ntpd.c
> @@ -1447,12 +1447,14 @@ update_local_clock(peer_t *p)
>
> #if USING_INITIAL_FREQ_ESTIMATION
> if (G.discipline_state == STATE_NSET) {
> - set_new_values(STATE_FREQ, /*offset:*/ 0,
> recv_time);
> + /* G.last_update_recv_time already updated */
> + set_new_values(STATE_FREQ, /*offset:*/ 0,
> G.last_update_recv_time);
No, G.last_update_recv_time is *not* already updated here.
Its _previous_ value is just corrected by step_time(),
but here we want to record a _new_ one.
> return 1; /* "ok to increase poll interval" */
> }
> #endif
> abs_offset = offset = 0;
> - set_new_values(STATE_SYNC, offset, recv_time);
> + /* G.last_update_recv_time already updated */
> + set_new_values(STATE_SYNC, offset, G.last_update_recv_time);
>
> } else { /* abs_offset <= STEP_THRESHOLD */
I'd rather do
recv_time += offset;
right after step_time(offset) is called. This way. code is more obvious.
Can you test that it works?
More information about the busybox
mailing list