Fix PTP_SYS_OFFSET_EXTENDED returning erroneous post timestamp #1
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
agagniere
force-pushed
the
fix-ts_window
branch
from
ae8b974 to
76fec33
Compare
ts_window_adjust and utc_tai_offset are u32 timespec64's members are s64 and long ktime_t is s64 s64 -> %lli long -> %li u32 -> %u u16 -> %hu u8 -> %hhu 1UL -> %lu
For some reason the driver may be unable to estimate PCI timing. In that situation (which happens 100% of the time on the various hardware I tried) the ts_window_adjust would be assinged to (u32)((U64_MAX >> 5) * 3) = 4294967293 because of this PTP_SYS_OFFSET_EXTENDED's post_ts is substracted 4.3s, making it smaller than pre_ts.
The delay was initialized to U64_MAX, while it was a s64, so its initial value was interpreted as -1 Then when it was compared to positive delays, the lowest value was always -1 Then when it was divided by 10 and cast into an u32, that u32 ended up with a value of U32_MAX - 2
agagniere
force-pushed
the
fix-ts_window
branch
from
76fec33 to
127b8a6
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Context
The value stored in the read-write attribute
ts_window_adjustis a number of nanoseconds subtracted to thepost_tstimestamp of the PHC readings of the ioctlPTP_SYS_OFFSET_EXTENDED, to compensate for PCI delays.Its initial value is set by estimating PCI delay.
Bug
The PCI delay estimation starts with the value$4294967293$
U64_MAXand makes 3 measurements, taking the minimum value.However because the delay was stored in a
s64,U64_MAXwas interpreted as -1, which compared as smaller than any positive values measured.Then, that delay is divided by 10 and placed in
ts_window_adjust, which is au32.So
ts_window_adjustends up with(u32)((U64_MAX >> 5) * 3)inside, which isSymptom
The consequence was that the$4.29$ seconds
post_tsofPTP_SYS_OFFSET_EXTENDEDwas substractedAs a consequence chrony rejected all readings from the PHC
Difficulty to diagnose
Using
catto read the attribute value showed-3because the format flags%dwas used instead of%u, resulting in a re-interpret cast.Fixes
U32_MAXas initial value for PCI delays: no one is expecting anioreadto take more than