Thread (22 messages) 22 messages, 5 authors, 9h ago

Re: [PATCH v4 net-next 1/7] ptp: Add ioctls for PHC timestamps with quality attributes

From: Jacob Keller <jacob.e.keller@intel.com>
Date: 2026-07-16 18:15:01
Also in: linux-doc, linux-kselftest

On 7/16/2026 1:09 AM, Arthur Kiyanovski wrote:
On 2026-07-14 17:45:34-07:00, Jacob Keller wrote:
quoted
On 7/13/2026 7:03 PM, Arthur Kiyanovski wrote:
I'm also wondering if this can expose device-known error bounds on
timestamps even for devices which are operated as synchronized by ptp4l..
The intent is for these attributes to represent device-
known clock quality information. The mechanism is
deliberately synchronization-agnostic: drivers may report
any attributes they can meaningfully vouch for, regardless
of how the clock is being disciplined. Thus a device that
genuinely knows a hardware error bound could report it even
if the PHC is being adjusted by ptp4l.

In practice, however, when a PHC is disciplined entirely
from userspace the driver often has little or no visibility
into synchronization quality. The valid bitmask is designed
for exactly this case: a driver advertises only the
attributes it can populate, so anything it cannot determine
is simply reported as unavailable.
Right.
quoted
quoted
Timescale definitions use a Continuity/Discipline framework to describe
timeline properties and steering behavior consistently across all
entries.

This implementation is based on the original RFC and the UAPI design
discussion linked below.
Not a dig against this patch set, nor a request that you work to
implement anything else, but I am beginning to wonder if/when it would
make sense to transition from ioctl-based implementation to genetlink or
something. We did something similar for ethtool ioctls a few years ago.
I know the maintainer for PTP has some distaste for netlink and prefers
the simplicity of the ioctls.. but I think we're moving past where the
ioctls are "simple". Now that we have ynl tools, it has gotten easier to
implement properly. It makes extending the API much easier for the
future vs the array of ioctls we now carry for legacy implementations.
That's an interesting direction for future PTP UAPI
evolution, and ynl does make that path easier than it used
to be. For this series I've kept to the existing PTP
userspace API model and extended the current timestamping
interfaces in a backwards-compatible way; a move to a
netlink family would be a broader subsystem effort and feels
separate from this work.
Absolutely. I don't think that should change this patch series. Its just
a thought that we might want to tackle this at some point as the ioctl
interface is clearly reaching its limits.
quoted
Do you have any thought on how ptp4l synchronizing the clock should
impact the clock status here? Is this intended purely for device/drivers
which have their own synchronization and not for ones which expose a
clock that is synchronized by userspace? Would it make sense to have a
mode that is something like "this clock has been modified by userspace"
after any call to the .adjtime or .adjfreq is made?
Regarding a "modified by userspace" mode, I wasn't
planning to add one. Whether adjtime() or adjfreq() was
invoked does not by itself describe the current
synchronization state or quality of the clock. A clock
disciplined from userspace may still be highly accurate. I'd
prefer to keep clock_status focused on clock quality
information that the driver can directly determine rather
than on how the clock is being controlled.
Makes sense. Leave it up to userspace to coordinate and combine relevant
data from the device/driver and the daemons together. Ok.
quoted
quoted
@@ -106,7 +350,11 @@ struct ptp_clock_caps {
 	/* Whether the clock supports adjust phase */
 	int adjust_phase;
 	int max_phase_adj; /* Maximum phase adjustment in nanoseconds. */
-	int rsv[11];       /* Reserved for future use. */
+	/* Whether the clock supports extended timestamps with attributes */
+	int extended_attrs;
+	/* Whether the clock supports precise cross-timestamps with attributes */
+	int precise_attrs;
+	int rsv[9];       /* Reserved for future use. */
I do kind of wish we had opted for bit flags here given the number of
ints being used as booleans.. :( A lot of wasted reserved space.
Agreed — a flags field would likely have scaled better. I
followed the existing ptp_clock_caps convention (one int per
capability) to stay consistent with the current UAPI
structure rather than mix two styles within the same struct.
Yep, I agree that it is best to stick to the same pattern.
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help