Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

proposal: x/sys/unix: add methods to convert between time.Time and unix.PtpClockTime on Linux #70032

Open
yarikk opened this issue Oct 24, 2024 · 6 comments · May be fixed by golang/sys#230
Open

proposal: x/sys/unix: add methods to convert between time.Time and unix.PtpClockTime on Linux #70032

yarikk opened this issue Oct 24, 2024 · 6 comments · May be fixed by golang/sys#230
Labels
Proposal
Milestone
Proposal

Comments

@yarikk
Copy link

yarikk commented Oct 24, 2024 •
edited
Loading

Proposal Details

Recently we have exposed the syscalls and types related to PTP (IEEE 1588) on Linux in the unix package.
Amongs those is a struct representing time in PTP calls, unix.PtpClockTime. Semantically it is very similar to Unix time, differing in underlying type for the nanoseconds field and having a placeholder for future extentions.

The proposal is to add the convenience methods to carry conversion between unix.PtpClockTime and time.Time:

package unix

func TimeToPtpClockTime(t time.Time) PtpClockTime
func (t *PtpClockTime) Time() time.Time
func (t *PtpClockTime) Unix() (sec int64, nsec int64)

This shall allow to use the maths of the standard time package while avoiding
the boilerplate: write ptpt.Time() instead of time.Unix(ptpt.Sec, int64(ptpt.Nsec)),
and unix.TimeToPtpClockTime(t) instead of unix.PtpClockTime{Sec: t.Unix(), Nsec: uint32(t.Nanosecond())}.

As an example of what the boilerplate looks like in real-world code, please see facebook/time#418.

golang/sys#230 implements the proposal.

Thank you for consideration.
@gopherbot gopherbot added this to the Proposal milestone Oct 24, 2024
@yarikk yarikk linked a pull request Oct 24, 2024 that will close this issue
Open
@gopherbot
Copy link
Contributor

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

@seankhliao seankhliao changed the title proposal: golang.org/x/sys/unix: add methods to covert between time.Time and unix.PtpClockTime on Linux proposal: x/sys/unix: add methods to covert between time.Time and unix.PtpClockTime on Linux Oct 24, 2024
@ianlancetaylor ianlancetaylor moved this to Incoming in Proposals Oct 24, 2024
@yarikk yarikk mentioned this issue Oct 25, 2024
Closed
facebook-github-bot pushed a commit to facebook/time that referenced this issue Nov 18, 2024
@yarikk @facebook-github-bot
drop PtpClockTime.Time() (#418)
ccc405e 
Summary:
Pull Request resolved: #418

Avoid the non-upstreamed unix.PtpClockTime.Time().

The related [golang/go#70032](golang/go#70032) is still pending decision. In case of positive outcome we can unland this.

Reviewed By: leoleovich

Differential Revision: D64901381

fbshipit-source-id: b06c389dff7f0720391bc8dac81e6a346a111a58
@ianlancetaylor ianlancetaylor changed the title proposal: x/sys/unix: add methods to covert between time.Time and unix.PtpClockTime on Linux proposal: x/sys/unix: add methods to convert between time.Time and unix.PtpClockTime on Linux Feb 12, 2025
@rsc rsc moved this from Incoming to Active in Proposals Feb 13, 2025
@rsc
Copy link
Contributor

rsc commented Feb 13, 2025

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

@aclements
Copy link
Member

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.

@aclements
Copy link
Member

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

@aclements
Copy link
Member

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.

@yarikk
Copy link
Author

yarikk commented Mar 7, 2025 •
edited
Loading

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
Proposal
Projects
Proposals
Status: Active
Milestone
Proposal
Development

Successfully merging a pull request may close this issue.

unix: add unix.TimeToPtpClockTime on Linux yarikk/go-sys
4 participants
@rsc @aclements @gopherbot @yarikk