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.

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

complete SO_TIMESTAMPNS to SO_TIMESTAMP fallback #375

Merged
merged 1 commit into from
Jan 26, 2025

Conversation

auerswal
Copy link
Collaborator

Commit e866063 added a fallback from setting the socket option SO_TIMESTAMPNS to setting the socket option SO_TIMESTAMP if the nanosecond timestamp option could not be set. But it did not add code to also look for the control message related to SO_TIMESTAMP. Thus microsecond timestamps were requested, but not read.

This commit adds the missing code to read microsecond timestamp control messages.

The problem was reported in GitHub issue #374 by @payload.

Commit e866063 added a
fallback from setting the socket option SO_TIMESTAMPNS to
setting the socket option SO_TIMESTAMP if the nanosecond
timestamp option could not be set.  But it did not add
code to also look for the control message related to
SO_TIMESTAMP.  Thus microsecond timestamps were requested,
but not read.

This commit adds the missing code to read microsecond
timestamp control messages.

The problem was reported in GitHub issue schweikert#374 by @payload.
@coveralls
Copy link

Coverage Status

coverage: 87.679% (+0.08%) from 87.602%
when pulling fdd5dee on auerswal:issue374
into cb83286 on schweikert:develop.

@gsnw-sebast
Copy link
Collaborator

Not sure, but the actual ticket had the problem, the binary was built with HAVE_SO_TIMESTAMPNS but the system itself could not use SO_TIMESTAMPNS. So not sure about the implications of timeval_ns() if it is still present in SCM_TIMESTAMP.

@auerswal
Copy link
Collaborator Author

According to man 7 socket, the two socket options SO_TIMESTAMP and SO_TIMESTAMPNS result in the creation of different control messages:

  • Setting SO_TIMESTAMP results in control messages of type SCM_TIMESTAMP with the data in struct timeval format for microsecond resolution.
  • Setting SO_TIMESTAMPNS results in control messages of type SCM_TIMESTAMPNS with the data in struct timespec format for nanosecond resolution.

Since only one of the two socket options is set for each socket, only one type of control messages is created for packets received on the respective socket.

According to the online man page, "A socket cannot mix SO_TIMESTAMP and SO_TIMESTAMPNS: the two modes are mutually exclusive."

I have tested this on GNU/Linux, where both SO_TIMESTAMPNS and SO_TIMESTAMP work, by adding debugging output to the CMSG related code and changing the preference of the two socket options:

  • When SO_TIMESTAMPNS is tried first, this succeeds, and only SCM_TIMESTAMPNS type control messages are created.
  • When SO_TIMESTAMP is tried first, this succeeds, and only SCM_TIMESTAMP type control messages are created.

I have then changed the code to prefer SO_TIMESTAMP for IPv4 and SO_TIMESTAMPNS for IPv6 and used a dual-stack target host: fping -mAne target.example. This showed SCM_TIMESTAMP for IPv4 and SCM_TIMESTAMPNS for IPv6, and similar elapsed times for both address families, as expected.

As such I think that the code in this pull request works as intended and is required to make use of the fallback to SO_TIMESTAMP.

Copy link
Collaborator

@gsnw-sebast gsnw-sebast left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for your detailed reply. I have tested it and it looks good

@auerswal auerswal merged commit 511aa37 into schweikert:develop Jan 26, 2025
6 of 9 checks passed
@auerswal auerswal deleted the issue374 branch January 26, 2025 20:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

When falling back to SO_TIMESTAMP reply timestamp is probably not working
3 participants