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

[Jacob Keller <jacob.e.keller@intel.com>]

On 7/16/2026 1:09 AM, Arthur Kiyanovski wrote:
> On 2026-07-14 17:45:34-07:00, Jacob Keller wrote:
>> 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.

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

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

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