[RFC PATCH net-next] ptp: Introduce PTP_SYS_OFFSET_EXTENDED_TRUSTED ioctl

[RFC PATCH net-next] ptp: Introduce PTP_SYS_OFFSET_EXTENDED_TRUSTED ioctl

[David Arinzon]

This patch introduces a new ioctl command,
PTP_SYS_OFFSET_EXTENDED_TRUSTED, to the Linux PTP subsystem.

The existing PTP_SYS_OFFSET_EXTENDED ioctl enables user-space
applications to correlate timestamps between system and PTP clocks
using raw measurements. These timestamps are used, for example,
by applications such as phc2sys to synchronize the system clock
from a PHC device.

The existing PTP APIs lack information about the synchronization
status and the quality of time offered by the PHC device. This
limitation is commonly circumvented by user-space tools such as
ptp4l. These tools implement the synchronization logic and can
export their measurement of clock accuracy and report on
synchronization status, and possible failures.

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.

The first objective focuses on the use case where the PHC device
is fully managed. The ENA driver, for example, exposes a PHC
device, whose synchronization status and quality is maintained
without any user-space application. This new ioctl reports on the
clock accuracy and status of the PHC device to user-space
applications, where ptp4l and similar are not available.

The second objective is to provide user-space applications with a
complete view of the current time and associated quality of the
PHC device in one-single call. This objective benefits the
consumers of time offered by the PHC device, independent from how
it is kept in sync (using ptp4l or being a managed device).

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.

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

Note that the value taken by the clock status aligns with the
definition of the ptp_vmclock device and has the same semantic.
The status value are meant to be vendor-agnostic, and generic
enough to be mapped to specific protocols, including the
clockAccuracy and clockClass concepts defined within the IEEE
1588 standard.

This ioctl enables applications to directly obtain clock quality
metrics from the device, reducing the need for indirect inference
and simplifying clock monitoring.

Signed-off-by: Amit Bernstein <amitbern@amazon.com>
Signed-off-by: David Arinzon <darinzon@amazon.com>
---
 drivers/ptp/ptp_chardev.c        | 61 ++++++++++++++++++++++++++
 include/linux/ptp_clock_kernel.h | 19 +++++++++
 include/uapi/linux/ptp_clock.h   | 73 ++++++++++++++++++++++++++++++++
 3 files changed, 153 insertions(+)

diff --git a/drivers/ptp/ptp_chardev.c b/drivers/ptp/ptp_chardev.c
index 4ca5a464..12fef95c 100644
--- a/drivers/ptp/ptp_chardev.c
+++ b/drivers/ptp/ptp_chardev.c
@@ -365,6 +365,64 @@ static long ptp_sys_offset_extended(struct ptp_clock *ptp, void __user *arg)
 	return copy_to_user(arg, extoff, sizeof(*extoff)) ? -EFAULT : 0;
 }
 
+static long ptp_sys_offset_extended_trusted(struct ptp_clock *ptp,
+					    void __user *arg)
+{
+	struct ptp_sys_offset_extended_trusted *extofftrst __free(kfree) = NULL;
+	struct ptp_system_timestamp sts;
+	struct ptp_clock_attributes att;
+
+	if (!ptp->info->gettimextrusted64)
+		return -EOPNOTSUPP;
+
+	extofftrst = memdup_user(arg, sizeof(*extofftrst));
+	if (IS_ERR(extofftrst))
+		return PTR_ERR(extofftrst);
+
+	if (extofftrst->n_samples > PTP_MAX_SAMPLES ||
+	    extofftrst->rsv[0] ||
+	    extofftrst->rsv[1])
+		return -EINVAL;
+
+	switch (extofftrst->clockid) {
+	case CLOCK_REALTIME:
+	case CLOCK_MONOTONIC:
+	case CLOCK_MONOTONIC_RAW:
+		break;
+	case CLOCK_AUX ... CLOCK_AUX_LAST:
+		if (IS_ENABLED(CONFIG_POSIX_AUX_CLOCKS))
+			break;
+		fallthrough;
+	default:
+		return -EINVAL;
+	}
+
+	sts.clockid = extofftrst->clockid;
+	for (unsigned int i = 0; i < extofftrst->n_samples; i++) {
+		struct timespec64 ts;
+		int err;
+
+		err = ptp->info->gettimextrusted64(ptp->info, &ts, &sts, &att);
+		if (err)
+			return err;
+
+		/* Filter out disabled or unavailable clocks */
+		if (sts.pre_ts.tv_sec < 0 || sts.post_ts.tv_sec < 0)
+			return -EINVAL;
+
+		extofftrst->ts[i][0].pct.sec = sts.pre_ts.tv_sec;
+		extofftrst->ts[i][0].pct.nsec = sts.pre_ts.tv_nsec;
+		extofftrst->ts[i][1].pct.sec = ts.tv_sec;
+		extofftrst->ts[i][1].pct.nsec = ts.tv_nsec;
+		extofftrst->ts[i][1].att.error_bound = att.error_bound;
+		extofftrst->ts[i][1].att.clock_status = att.clock_status;
+		extofftrst->ts[i][2].pct.sec = sts.post_ts.tv_sec;
+		extofftrst->ts[i][2].pct.nsec = sts.post_ts.tv_nsec;
+	}
+
+	return copy_to_user(arg, extofftrst, sizeof(*extofftrst)) ? -EFAULT : 0;
+}
+
 static long ptp_sys_offset(struct ptp_clock *ptp, void __user *arg)
 {
 	struct ptp_sys_offset *sysoff __free(kfree) = NULL;
@@ -503,6 +561,9 @@ long ptp_ioctl(struct posix_clock_context *pccontext, unsigned int cmd,
 	case PTP_SYS_OFFSET_EXTENDED2:
 		return ptp_sys_offset_extended(ptp, argptr);
 
+	case PTP_SYS_OFFSET_EXTENDED_TRUSTED:
+		return ptp_sys_offset_extended_trusted(ptp, argptr);
+
 	case PTP_SYS_OFFSET:
 	case PTP_SYS_OFFSET2:
 		return ptp_sys_offset(ptp, argptr);
diff --git a/include/linux/ptp_clock_kernel.h b/include/linux/ptp_clock_kernel.h
index 3d089bd4..cd74e32f 100644
--- a/include/linux/ptp_clock_kernel.h
+++ b/include/linux/ptp_clock_kernel.h
@@ -120,6 +120,21 @@ struct ptp_system_timestamp {
  *               reading the lowest bits of the PHC timestamp and the second
  *               reading immediately follows that.
  *
+ * @gettimextrusted64:  Reads the current time from the hardware clock and
+ *                      optionally also the system clock with additional data on
+ *                      hardware clock accuracy and reliability.
+ *                      parameter ts: Holds the PHC timestamp.
+ *                      parameter sts: If not NULL, it holds a pair of
+ *                      timestamps from the system clock. The first reading is
+ *                      made right before reading the lowest bits of the PHC
+ *                      timestamp and the second reading immediately follows
+ *                      that.
+ *                      parameter error_bound: If not NULL, it holds the maximum
+ *                      error bound for the PHC timestamp in nanoseconds.
+ *                      parameter clock_status: If not NULL, it holds
+ *                      qualitative clock states indicating its synchronization
+ *                      status and reliability.
+ *
  * @getcrosststamp:  Reads the current time from the hardware clock and
  *                   system clock simultaneously.
  *                   parameter cts: Contains timestamp (device,system) pair,
@@ -200,6 +215,10 @@ struct ptp_clock_info {
 	int (*gettime64)(struct ptp_clock_info *ptp, struct timespec64 *ts);
 	int (*gettimex64)(struct ptp_clock_info *ptp, struct timespec64 *ts,
 			  struct ptp_system_timestamp *sts);
+	int (*gettimextrusted64)(struct ptp_clock_info *ptp,
+				 struct timespec64 *ts,
+				 struct ptp_system_timestamp *sts,
+				 struct ptp_clock_attributes *att);
 	int (*getcrosststamp)(struct ptp_clock_info *ptp,
 			      struct system_device_crosststamp *cts);
 	int (*settime64)(struct ptp_clock_info *p, const struct timespec64 *ts);
diff --git a/include/uapi/linux/ptp_clock.h b/include/uapi/linux/ptp_clock.h
index 18eefa6d..bb22ed61 100644
--- a/include/uapi/linux/ptp_clock.h
+++ b/include/uapi/linux/ptp_clock.h
@@ -76,6 +76,26 @@
  */
 #define PTP_PEROUT_V1_VALID_FLAGS	(0)
 
+/*
+ * Clock status values for struct ptp_clock_attributes.clock_status
+ */
+enum ptp_clock_status {
+	/* clock synchronization status cannot be reliably determined */
+	PTP_CLOCK_STATUS_UNKNOWN      = 0,
+
+	/* clock is acquiring synchronization */
+	PTP_CLOCK_STATUS_INITIALIZING = 1,
+
+	/* clock is synchronized and maintained accurately by the device */
+	PTP_CLOCK_STATUS_SYNCED       = 2,
+
+	/* clock is drifting and not being synchronized by the device */
+	PTP_CLOCK_STATUS_FREE_RUNNING = 3,
+
+	/* clock is unreliable, the error_bound value cannot be trusted */
+	PTP_CLOCK_STATUS_UNRELIABLE   = 4
+};
+
 /*
  * struct ptp_clock_time - represents a time value
  *
@@ -91,6 +111,33 @@ struct ptp_clock_time {
 	__u32 reserved;
 };
 
+/*
+ * struct ptp_clock_attributes - describes additional data for a PTP clock
+ *                               timestamp
+ *
+ * @error_bound:   The maximum possible error (in nanoseconds) associated with
+ *                 the reported timestamp, this value quantifies the inaccuracy
+ *                 of the clock at the time of reading.
+ * @clock_status:  Qualitative state of the clock (enum ptp_clock_status)
+ * @rsv:           Reserved for future use, should be set to zero.
+ */
+struct ptp_clock_attributes {
+	__u32 error_bound;
+	__u8 clock_status;
+	__u8 rsv[3];
+};
+
+/*
+ * struct ptp_clock_time_trusted - PTP timestamp with its associated attributes
+ *
+ * @pct: The PTP clock timestamp value.
+ * @att: PTP clock attributes about the timestamp
+ */
+struct ptp_clock_time_trusted {
+	struct ptp_clock_time pct;
+	struct ptp_clock_attributes att;
+};
+
 struct ptp_clock_caps {
 	int max_adj;   /* Maximum frequency adjustment in parts per billon. */
 	int n_alarm;   /* Number of programmable alarms. */
@@ -177,6 +224,30 @@ struct ptp_sys_offset_extended {
 	struct ptp_clock_time ts[PTP_MAX_SAMPLES][3];
 };
 
+/*
+ * ptp_sys_offset_extended_trusted - data structure for IOCTL operation
+ *				     PTP_SYS_OFFSET_EXTENDED_TRUSTED
+ *
+ * @n_samples:	Desired number of measurements.
+ * @clockid:	clockid of a clock-base used for pre/post timestamps.
+ * @rsv:	Reserved for future use.
+ * @ts:		Array of samples in the form [pre-TS, PHC, post-TS].
+ *		Each sample consists of timestamp in the form [sec, nsec],
+ *		while the PHC sample also includes clock attributes in the form
+ *		[error_bound, clock_status],
+ *
+ * Starting from kernel 6.12 and onwards, the first word of the reserved-field
+ * is used for @clockid. That's backward compatible since previous kernel
+ * expect all three reserved words (@rsv[3]) to be 0 while the clockid (first
+ * word in the new structure) for CLOCK_REALTIME is '0'.
+ */
+struct ptp_sys_offset_extended_trusted {
+	unsigned int n_samples;
+	__kernel_clockid_t clockid;
+	unsigned int rsv[2];
+	struct ptp_clock_time_trusted ts[PTP_MAX_SAMPLES][3];
+};
+
 struct ptp_sys_offset_precise {
 	struct ptp_clock_time device;
 	struct ptp_clock_time sys_realtime;
@@ -231,6 +302,8 @@ struct ptp_pin_desc {
 	_IOWR(PTP_CLK_MAGIC, 8, struct ptp_sys_offset_precise)
 #define PTP_SYS_OFFSET_EXTENDED \
 	_IOWR(PTP_CLK_MAGIC, 9, struct ptp_sys_offset_extended)
+#define PTP_SYS_OFFSET_EXTENDED_TRUSTED \
+	_IOWR(PTP_CLK_MAGIC, 10, struct ptp_sys_offset_extended_trusted)
 
 #define PTP_CLOCK_GETCAPS2  _IOR(PTP_CLK_MAGIC, 10, struct ptp_clock_caps)
 #define PTP_EXTTS_REQUEST2  _IOW(PTP_CLK_MAGIC, 11, struct ptp_extts_request)
-- 
2.47.1

Re: [RFC PATCH net-next] ptp: Introduce PTP_SYS_OFFSET_EXTENDED_TRUSTED ioctl

[David Woodhouse]

[-- Attachment #1: Type: text/plain, Size: 1280 bytes --]

On Thu, 2025-07-24 at 14:56 +0300, David Arinzon wrote:
> 
> 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.
> 
> - 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.

Should this also include a 'type' field matching vmclock & virtio-rtc?
Especially if it's a smeared type, that is a (contra-)indication of the
quality of the clock.

Julien, I assume you're happy that this is *maxerror* and we completely
ignore esterror which should never exist anyway?

[-- Attachment #2: smime.p7s --]
[-- Type: application/pkcs7-signature, Size: 5069 bytes --]

Re: [RFC PATCH net-next] ptp: Introduce PTP_SYS_OFFSET_EXTENDED_TRUSTED ioctl

[Richard Cochran]

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

> The first objective focuses on the use case where the PHC device
> is fully managed. The ENA driver, for example, exposes a PHC
> device, whose synchronization status and quality is maintained
> without any user-space application. This new ioctl reports on the
> clock accuracy and status of the PHC device to user-space
> applications, where ptp4l and similar are not available.

This seems pretty rare.  I can't think of any other hardware that
makes claims about synchronization quality.

So this ioctl would be just for one device.

Thanks,
Richard

Re: [RFC PATCH net-next] ptp: Introduce PTP_SYS_OFFSET_EXTENDED_TRUSTED ioctl

[David Woodhouse]

[-- Attachment #1: Type: text/plain, Size: 850 bytes --]

On Thu, 2025-07-24 at 21:09 -0700, Richard Cochran wrote:
> On Thu, Jul 24, 2025 at 02:56:56PM +0300, David Arinzon wrote:
> 
> > The first objective focuses on the use case where the PHC device
> > is fully managed. The ENA driver, for example, exposes a PHC
> > device, whose synchronization status and quality is maintained
> > without any user-space application. This new ioctl reports on the
> > clock accuracy and status of the PHC device to user-space
> > applications, where ptp4l and similar are not available.
> 
> This seems pretty rare.  I can't think of any other hardware that
> makes claims about synchronization quality.
> 
> So this ioctl would be just for one device.

The vmclock enlightenment also exposes the same information.

David, your RFC should probably have included that implementation
shouldn't it? 

[-- Attachment #2: smime.p7s --]
[-- Type: application/pkcs7-signature, Size: 5069 bytes --]

Re: [RFC PATCH net-next] ptp: Introduce PTP_SYS_OFFSET_EXTENDED_TRUSTED ioctl

[Miroslav Lichvar]

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?

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.

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

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

-- 
Miroslav Lichvar

Re: [RFC PATCH net-next] ptp: Introduce PTP_SYS_OFFSET_EXTENDED_TRUSTED ioctl

[Ridoux, Julien]

> On 7/24/25, 7:09 AM, "David Woodhouse" <dwmw2@infradead.org <mailto:dwmw2@infradead.org>> wrote:
> 
> On Thu, 2025-07-24 at 14:56 +0300, David Arinzon wrote:
> > 
> > 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.
> > 
> > - 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.
> 
> 
> Should this also include a 'type' field matching vmclock & virtio-rtc?
> Especially if it's a smeared type, that is a (contra-)indication of the
> quality of the clock.

That is a good suggestion. Let's add it.

 
> Julien, I assume you're happy that this is *maxerror* and we completely
> ignore esterror which should never exist anyway?

Absolutely, and you are right, having *maxerror* only makes me happy :-)

Re: [RFC PATCH net-next] ptp: Introduce PTP_SYS_OFFSET_EXTENDED_TRUSTED ioctl

[Richard Cochran]

On Fri, Jul 25, 2025 at 11:25:24AM +0200, David Woodhouse wrote:
> The vmclock enlightenment also exposes the same information.
> 
> David, your RFC should probably have included that implementation
> shouldn't it? 

Yes, a patch series with the new ioctl and two drivers that implement
it would be more compelling.

Thanks,
Richard

RE: [RFC PATCH net-next] ptp: Introduce PTP_SYS_OFFSET_EXTENDED_TRUSTED ioctl

[julien]

On 7/28/25, 8:55 PM, "Richard Cochran" <richardcochran@gmail.com <mailto:richardcochran@gmail.com>> wrote:

> On Fri, Jul 25, 2025 at 11:25:24AM +0200, David Woodhouse wrote:
> > The vmclock enlightenment also exposes the same information.
> > 
> > David, your RFC should probably have included that implementation
> > shouldn't it? 
> 
> Yes, a patch series with the new ioctl and two drivers that implement
> it would be more compelling.

We can include the driver side of this RFC for the ENA and vmclock. 

RE: [RFC PATCH net-next] ptp: Introduce PTP_SYS_OFFSET_EXTENDED_TRUSTED ioctl

[Julien Ridoux]

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

Re: [RFC PATCH net-next] ptp: Introduce PTP_SYS_OFFSET_EXTENDED_TRUSTED ioctl

[David Woodhouse]

[-- Attachment #1: Type: text/plain, Size: 1816 bytes --]

On Sun, 2025-08-03 at 16:51 -0700, Julien Ridoux wrote:
> > 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:
> > > 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

While we're talking about extending the userspace API... I think I'd
also like a way to use the actual hardware counter (TSC, arch timer) as
the reference clockid instead of CLOCK_MONOTONIC etc. 

In a hosting environment, I barely even care about calibrating the host
kernel's timekeeping against the external time reference. What I *do*
care about calibrating is the hardware counter, so that it can be
correctly advertised to guests through a vmclock device.

Using vmclock and simply advertising the relationship between the
hardware counter and precision time, makes *so* much more sense than
having hundreds of virtual guests on the same machine all performing
the *same* calibration of precisely the same underlying counter, each
of them potentially experiencing latency of measurements due to steal
time while they do so.

[-- Attachment #2: smime.p7s --]
[-- Type: application/pkcs7-signature, Size: 5069 bytes --]

Re: [RFC PATCH net-next] ptp: Introduce PTP_SYS_OFFSET_EXTENDED_TRUSTED ioctl

[Maciek Machnikowski]

On 7/29/2025 5:55 AM, Richard Cochran wrote:
> On Fri, Jul 25, 2025 at 11:25:24AM +0200, David Woodhouse wrote:
>> The vmclock enlightenment also exposes the same information.
>>
>> David, your RFC should probably have included that implementation
>> shouldn't it? 
> 
> Yes, a patch series with the new ioctl and two drivers that implement
> it would be more compelling.
> 
> Thanks,
> Richard
> 
> 
> 
Wouldn't it be more useful if it came with the API to set these values?
This way a clock could be controlled by ptp4l and the set the clock
quality parameters and other apps could get it from there.

Thanks,
Maciek