Change https://go.dev/cl/621498 mentions this issue: unix: add unix.TimeToPtpClockTime on Linux

This proposal has been added to the active column of the proposals project
and will now be reviewed at the weekly proposal review meetings.
— rsc for the proposal review group

I'm probably missing some background on this. If you convert to a time.Time to do time math and then back to a unix.PtpClockTime, what exactly is the point of unix.PtpClockTime? It seems like any additional information it could carry would be lost.

I'd still like to understand if there's any danger of lossiness in the conversion to/from time.Time, but otherwise this makes sense. It basically parallels the APIs for Timespec and Timeval (which maybe we should expand with Time methods and TimeToTimeval).

I'd still like to understand if there's any danger of lossiness in the conversion to/from time.Time

Bump. It seems like there's no lossiness today, but I don't really understand what the plan is with the "reserved" bits.

The reserved bits MUST be set to zero in all PTP-related IOCTLs. In fact, the v2 of each of those calls were created to reject non-zero uses of the reserved bytes. In other words, nothing today should be setting those bytes to anything but zero.

I hope this clears it up.

Thanks.

I think we should generally clean up these APIs. Right now we have:

// type Timespec
func NsecToTimespec(nsec int64) Timespec
func TimeToTimespec(t time.Time) (Timespec, error)
func (ts *Timespec) Nano() int64
func (ts *Timespec) Unix() (sec int64, nsec int64)
// type Timeval
func NsecToTimeval(nsec int64) Timeval
func (tv *Timeval) Nano() int64
func (tv *Timeval) Unix() (sec int64, nsec int64)
// type PtpClockTime
// ... nothing ...

It seems clear that we should at least complete the product of these types and operations:

// type Timeval
func TimeToTimeval(t time.Time) (Timeval, error)
// type PtpClockTime
func NsecToPtpClockTime(nsec int64) PtpClockTime
func TimeToPtpClockTime(t time.Time) (PtpClockTime, error)
func (t *PtpClockTime) Nano() int64
func (t *PtpClockTime) Unix() (sec int64, nsec int64)

We could also add a Time() time.Time method like originally proposed, but we don't currently have that on any of the time types. Especially with the Unix method, I'm not sure it's particularly necessary. If t is any of these time types, you can just say time.Unix(t.Unix()). That has the benefit of making it fairly clear what is and isn't getting converted (e.g., definitely no timezone).

@aclements That works, too – if you are happy with that, I can update the proposal.

We also want to be very clear that it's only a storage format conversion – there's nothing done to the underlying values here, e.g. no implicit adjustments for the UTC-TAI offset or any other leap second manipulations.

Indeed, it would be convenient to track possible offsets (e.g. PHC can be in TAI, per local policy) as the underlying time's Location, but time.Unix() always assumes Location as time.Local (which is itself may be a bug – shouldn't it be time.UTC?), and there's no way to override that. But that's a problem outside of the scope of this proposal.

Your mention of TAI worries me. Do PTP clock times follow TAI or UTC?

Do PTP clock times imply a location (e.g., UTC)? If so, maybe it should have a method that goes straight to time.Time with the appropriate Location, rather than going through time.Unix.

PTP clock can be either, TAI or UTC, it is deployment-specific. There is no location information currently tracked in PtpClockTime; the convertion to time.Time would happen just like it is for time.Unix, assuming the default location. It's on the user to track and apply any coversion logic between TAI and UTC, whatever local considerations are, e.g. apply leap second smearing. The proposal deliberately leaves that out of scope.

Thanks for clarifying that.

Given that PTP can be TAI, I actually think we should not have a func (t *PtpClockTime) Unix() (sec int64, nsec int64) method. If your local PTP is in TAI, then this will not in fact return Unix time, and providing this method makes it easy to do the wrong thing.

Could we use take a PtpClockTime in conjunction with a PtpSysOffsetPrecise or PtpSysOffsetExtended (or PtpSysOffset? I don't really understand the difference between all these) to create a time.Time? If I understand this correctly, that would seem to be an API that encourages doing the right thing in that conversion.

This is the best reference I've found on these APIs: https://github.com/torvalds/linux/blob/master/include/uapi/linux/ptp_clock.h