SHM refclock age control of local time stamp correctness

[satinowl]

https://github.com/ntp-project/ntp/blob/9c75327c3796ff59ac648478cd4da8b205bceb77/ntpd/refclock_shm.c#L606C8-L606C8

/* check 1: age control of local time stamp */
tt = shm_stat.tvc.tv_sec - shm_stat.tvr.tv_sec;
if (tt < 0 || tt > up->max_delay) {
    DPRINTF(1, ("%s:SHM stale/bad receive time, delay=%llds\n",
            refnumtoa(&peer->srcadr), (long long)tt));
    up->bad++;
    msyslog (LOG_ERR, "SHM: stale/bad receive time, delay=%llds",
         (long long)tt);
    return;
}

Here we substract received time stamp from current local time stamp (tt variable). If received time stamp is greater (which means that we should move system clock forward), we get negative value in tt. Further, we check if tt is negative ( tt < 0) and refuse to do such time adjustment. Is it correct behaviour? Error message for such case claims that received time stamp is stale, i.e. the received time is older than current time, which conflicts with actual actions in code. It is a common case that hardware without power support on local RTC clock will get in such scenario, that received time stamp from some external time source is newer than local time stamp. The reverse case seems to be unrealistic. And it is protected by tt > up->max_delay check, as far as i see.

[hart-NTP]

satinowl, please open a report at https://bugs.ntp.org/

It would be helpful to provide a bit more context of the actual problem you're trying to fix. It sounds like you're using ntpd on a device without a battery to maintain the clock. The ntpd code in github is woefully out of date, but more recent releases include a "basedate" functionality which may be helpful, which sets the clock at startup to the build date or the basedate from ntp.conf.

We are progressing recently in getting the github code up to date and tracking our changes.

[hart-NTP]

satinowl, I haven't seen a new bug report filed on this issue. If you're having problems signing up for an account at https://bugs.ntp.org/ shoot me an email at hart@ntp.org and I'll take care of it.

[chausner]

I think you misunderstood the meaning of receiveTimeStamp. It denotes the time (based on the local computer clock) at which the SHM data was updated, while clockTimeStamp is the reference time, i.e. the time received from the reference clock. shm_stat.tvc is the time (again based on the local computer clock) at which the SHM data was read by ntp. So the implementation appears correct to me.

In the condition

if (tt < 0 || tt > up->max_delay) {

tt < 0 should normally never occur because the time when ntpd reads the SHM data can never be earlier than the time when it was written. tt > up->max_delay should also not occur because the SHM driver reads the memory once every second and resets valid to zero every time.

[hart-NTP]

Thanks for the analysis @chausner

I agree the check looks correct. The only case where I can see the 0 <= tt < maxdelay test triggering is if the clock had been stepped, in which case ignoring the sample is the right thing to do.