netdev - RE: [RFC PATCH net-next] ptp: Introduce PTP_SYS_OFFSET_EXTENDED

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-Id: <20250803235142.57900-1-julien@synclab.org>
Date: Sun,  3 Aug 2025 16:51:42 -0700
From: Julien Ridoux <julien@...clab.org>
To: mlichvar@...hat.com
Cc: akiyano@...zon.com,
	aliguori@...zon.com,
	alisaidi@...zon.com,
	amitbern@...zon.com,
	andrew@...n.ch,
	benh@...zon.com,
	darinzon@...zon.com,
	davem@...emloft.net,
	dwmw2@...radead.org,
	dwmw@...zon.com,
	edumazet@...gle.com,
	evgenys@...zon.com,
	evostrov@...zon.com,
	joshlev@...zon.com,
	kuba@...nel.org,
	matua@...zon.com,
	msw@...zon.com,
	nafea@...zon.com,
	ndagan@...zon.com,
	netanel@...zon.com,
	netdev@...r.kernel.org,
	ofirt@...zon.com,
	pabeni@...hat.com,
	richardcochran@...il.com,
	ridouxj@...zon.com,
	saeedb@...zon.com,
	tglx@...utronix.de,
	zorik@...zon.com
Subject: RE: [RFC PATCH net-next] ptp: Introduce PTP_SYS_OFFSET_EXTENDED_TRUSTED ioctl

> On 7/28/25, 7:28 AM, "Miroslav Lichvar" <mlichvar@...hat.com <mailto:mlichvar@...hat.com>> wrote:
>
> On Thu, Jul 24, 2025 at 02:56:56PM +0300, David Arinzon wrote:
> > 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.

> > 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.

> > - 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.