-
Notifications
You must be signed in to change notification settings - Fork 694
Support linux TIMESTAMPING #1420
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.
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -562,6 +562,14 @@ pub enum ControlMessageOwned { | |
/// [Further reading](https://www.kernel.org/doc/html/latest/networking/timestamping.html) | ||
#[cfg(all(target_os = "linux"))] | ||
ScmTimestampns(TimeSpec), | ||
/// Configurable nanoseconds resolution timestamps | ||
/// | ||
/// Three `TimeSpec`s are returned in the control message. At least one field is non-zero at | ||
/// any time. Most timestamps are passed in ts[0]. Hardware timestamps are passed in ts[2]. | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. So what is ts[1] for? Also, the second line of a multiline doc comment like this should be blank. That way, rustdoc will interpret the first line as a summary and the rest as extended details. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I don't think it is necessary to specify every nuances in this short description. User of this feature needs to read the original linux documentation to have a reasonable idea in how to set and what to set in order to get either software and hardware timestamp (in hardware case, user also need to initiate in different way according to different hardware), and to know the historical reason for There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. ts[1] is deprecated? In that case, there isn't any reason to expose it to the user at all. You should remove it in There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is a pretty highly specific interface. It is more likely the user has learned the original linux interface first and try to use it through |
||
/// | ||
/// [Further reading](https://www.kernel.org/doc/html/latest/networking/timestamping.html) | ||
#[cfg(all(target_os = "linux"))] | ||
ScmTimestamping([TimeSpec; 3]), | ||
#[cfg(any( | ||
target_os = "android", | ||
target_os = "ios", | ||
|
@@ -658,6 +666,12 @@ impl ControlMessageOwned { | |
let ts: libc::timespec = ptr::read_unaligned(p as *const _); | ||
ControlMessageOwned::ScmTimestampns(TimeSpec::from(ts)) | ||
} | ||
#[cfg(all(target_os = "linux"))] | ||
(libc::SOL_SOCKET, libc::SCM_TIMESTAMPING) => { | ||
let tss: [libc::timespec; 3] = ptr::read_unaligned(p as *const _); | ||
let tss2 = [TimeSpec::from(tss[0]), TimeSpec::from(tss[1]), TimeSpec::from(tss[2])]; | ||
ControlMessageOwned::ScmTimestamping(tss2) | ||
} | ||
#[cfg(any( | ||
target_os = "android", | ||
target_os = "freebsd", | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could you please explain what the wrapped type is? Why are there three timespecs?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
https://www.kernel.org/doc/html/latest/networking/timestamping.html#scm-timestamping-records
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A link to an external website is not sufficient documentation. Why is the wrapped value an array? What's special about "3"?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The link was specifically for your question. Have you checked it out?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No; I'm asking for Nix's own documentation to describe why the value is a 3-member array. You can leave the external link in there, similar to how many Nix functions have man page links, but Nix's own documentation should have at least basic information about what the type is.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ok. Added some documentation.