On 7/28/25, 7:28 AM, "Miroslav Lichvar" <mlichvar@redhat.com <mailto:mlichvar@redhat.com>> wrote:

On Thu, Jul 24, 2025 at 02:56:56PM +0300, David Arinzon wrote:

quoted

The proposed PTP_SYS_OFFSET_EXTENDED_TRUSTED ioctl offers the
same timestamps as the PTP_SYS_OFFSET_EXTENDED ioctl, but extends
it with a measurement of the PHC device clock accuracy and the
synchronization status. This supports two objectives.

I have a slight issue with the naming of this new ioctl. TRUSTED
implies to me the other supported ioctls are not to be trusted
for some reason, but that's not the case, right? It's just more
information provided, i.e. it's extended once again. Would
PTP_SYS_OFFSET_EXTENDED3 or PTP_SYS_OFFSET_EXTENDED_ATTRS not work
better?

That's a fair call. The ioctl can be renamed to PTP_SYS_OFFSET_EXTENDED_ATTRS

It would be nice to have a new variant of the PTP_SYS_OFFSET_PRECISE
ioctl for the ptp_kvm driver to pass the system clock maxerror and
status.

Yes, let's add it to the patch.

quoted

The proposed PTP_SYS_OFFSET_EXTENDED_TRUSTED ioctl fulfills both
objectives by extending each PHC timestamp with two quality
indicators:

- error_bound: a device-calculated value (in nanoseconds)
reflecting the maximum possible deviation of the timestamp from
the true time, based on internal clock state.

Is this value expected to be changing so rapidly that it needs to be
provided with each sample of the ioctl? I'd expect a maximum value
from all samples to be sufficient (together with the worst status).

Yes, the error_bound value may be continuously changing.

It is possible for a device to implement a looser bound on the clock error,
similar to your suggestion (a worse case scenario over a "long" period of time)
for example.

But we want to offer the option to provide a tighter bound, consistent with
every clock read. Under normal conditions the error_bound reflects errors
accumulated upstream, and the maximum error the clock can gain between update.
A possible analogy: this is similar to the dispersion reported by `chronyc tracing`,
whose value is continuously changing.

quoted

- clock_status: a qualitative state of the clock, with defined
values including:
1. Unknown: the clock's synchronization status cannot be
reliably determined.
2. Initializing: the clock is acquiring synchronization.
3. Synchronized: The clock is actively being synchronized and
maintained accurately by the device.
4. FreeRunning: the clock is drifting and not being
synchronized and updated by the device.
5. Unreliable: the clock is known to be unreliable, the
error_bound value cannot be trusted.

I'd expect a holdover status to be included here.

There is no technical blocker to adding a holdover status to the list, letting
the underlying implementation decide if it needs to use this status or not.

I am, however, not convinced this is desirable. Although holdover is well
defined for timing equipment in a lab, in our experience, this introduces some
confusion and a false sense of security to the applications relying on accurate
time.

Holdover is a free running clock benefitting from some past history to make
corrections, and I see the risk of a semantic that is implementation
dependent. I would prefer that a consumer of this API decides whether to "trust"
the clock based on the clock entering the FreeRunning state, and the value of
the error_bound reported. In a sense, we have an opportunity to not offer a
footgun to the consumer of this API.

But again, adding a holdover value here does not force its use, and it can be
added now (or later) when the need arises.