Re: [chrony-dev] SOCK refclock system time resolution

[Josef 'Jeff' Sipek <jeffpc@xxxxxxxxxxxxxx>]

On Wed, Sep 13, 2023 at 14:02:48 +0200, Miroslav Lichvar wrote:
> On Wed, Sep 13, 2023 at 07:13:04AM -0400, Josef 'Jeff' Sipek wrote:
> > Just to make sure I understand, the "proper" way is to adjust the offset to
> > compensate for any rounding?  IOW, something like what I had in my program
> > before I messed with chrony's code:
> > 
> > 	uint64_t sys; /* system clock unix time in ns */
> > 	uint64_t ref; /* reference clock unix time in ns */
> > 
> > 	...
> > 
> > 	/* align sys time to microseconds, adjust ref time to maintain offset */
> > 	ref -= (sys % 1000);
> > 	sys -= (sys % 1000);
> 
> This shouldn't be necessary. Just pass the difference as seconds in
> double. You can verify the raw offset reported in the refclocks log is
> matching the values you are feeding into SOCK.

Hmm...

If my code simply truncates the ns timestamp when generating the timeval,
the sent over timeval has [0,999] ns error:

	msg->tv.tv_sec = sys / 1000000000;
	msg->tv.tv_usec = (sys / 1000) % 1000000;
	msg->offset = ((int64_t) (ref - sys)) / 1e9;

Chrony can't undo this since it doesn't have the information.  In chrony's
refclock_sock.c:

	... receive sample ...
	UTI_TimevalToTimespec(&sample.tv, &sys_ts);
	UTI_NormaliseTimespec(&sys_ts);
	UTI_AddDoubleToTimespec(&sys_ts, sample.offset, &ref_ts);
	RCL_AddSample(instance, &sys_ts, &ref_ts, sample.leap);

UTI_TimevalToTimespec simply multiplies the us by 1000 to get ns which will
be off by [0,999] ns from sys in my code.  Because ref_ts is calculated as
sys_ts+offset, ref_ts will have the same [0,999] ns error.  Finally,
RCL_AddSample does ref_ts-sys_ts (via UTI_DiffTimespecsToDouble and later on
by open-coded offset calculation) which makes the error cancel out.

However, I also see that cooked_time (i.e., sys_ts+correction) is fed into
SPF_AccumulateSample as sample->time.  Later on, combine_selected_samples
makes use of ->time to calculate various things.  So, it looks like any
error due to ns->us truncation will affect the math here.  At a glance, I
can't tell if the effect is so small that it can be ignored or if there is
any benefit to avoiding the [0,999] ns error.

> > 	/* send message to chrony */
> > 	msg->tv.tv_sec = sys / 1000000000;
> > 	msg->tv.tv_usec = (sys / 1000) % 1000000;
> > 	msg->offset = (ref - sys) / 1e9;
> 
> I think the difference should be casted to int64_t before going to
> double.

You're right.  I simplified the code for the email and accidentally dropped
the intermediate conversion to int64_t.

Thanks,

Jeff.

-- 
To unsubscribe email chrony-dev-request@xxxxxxxxxxxxxxxxxxxx with "unsubscribe" in the subject.
For help email chrony-dev-request@xxxxxxxxxxxxxxxxxxxx with "help" in the subject.
Trouble?  Email listmaster@xxxxxxxxxxxxxxxxxxxx.