Skip to content

feat: implement kernel receive timestamps on Linux/Android#2634

Open
syszery wants to merge 1 commit into
quinn-rs:mainfrom
syszery:feat/recv-timestamp
Open

feat: implement kernel receive timestamps on Linux/Android#2634
syszery wants to merge 1 commit into
quinn-rs:mainfrom
syszery:feat/recv-timestamp

Conversation

@syszery

@syszery syszery commented May 7, 2026

Copy link
Copy Markdown
Contributor

Closes #2574.

Add Linux/Android support for kernel receive timestamps via SO_TIMESTAMPNS.

  • Enable SO_TIMESTAMPNS on socket setup
  • Parse SCM_TIMESTAMPNS ancillary messages
  • Expose timestamp via RecvMeta::timestamp (Option<u64>, nanoseconds since Unix epoch)
  • Increase CMSG_LEN from 88 to 96 bytes on Linux/Android

Tested on Linux x86_64. All tests pass locally.

@thomaseizinger thomaseizinger left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Thanks! Looks clean.

Two comments.

Comment thread quinn-udp/src/lib.rs Outdated
Comment thread quinn-udp/src/unix.rs Outdated

@thomaseizinger thomaseizinger left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

LGTM

@djc let me know if you are happy with the change to core::time::Duration.

Commits should be squashed into one before merging. Can you do that please @syszery ? Don't worry about attributing to me for the last one.

@syszery

syszery commented May 14, 2026

Copy link
Copy Markdown
Contributor Author

Hi @thomaseizinger, thank you for the feedback and the improvements! I reviewed the changes, and I noticed that we now convert with Duration::from_secs. But I think we would need from_nanos here because the current implementation in the decoder uses nanoseconds.

However, to make the intent clearer and the code safer, I suggest to make the following change to decode directly into a Duration. This way, we avoid this entirely.

#[cfg(any(target_os = "linux", target_os = "android"))]
(libc::SOL_SOCKET, libc::SCM_TIMESTAMPNS) => {
    let ts = unsafe { cmsg::decode::<libc::timespec, libc::cmsghdr>(cmsg) };
    
    // Handle potential negative values on 32-bit/64-bit systems
    let secs = u64::try_from(ts.tv_sec).unwrap_or(0);
    let nsecs = u32::try_from(ts.tv_nsec).unwrap_or(0);

    self.timestamp = Some(core::time::Duration::new(secs, nsecs));
}

@djc djc left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Looks okay to me modulo a few nits. Thanks!

Comment thread quinn-udp/src/unix.rs
Comment thread quinn-udp/src/unix.rs Outdated
Comment thread quinn-udp/tests/tests.rs Outdated
@syszery syszery force-pushed the feat/recv-timestamp branch from e2e6dd0 to 75fde81 Compare May 14, 2026 20:29
@syszery

syszery commented May 14, 2026

Copy link
Copy Markdown
Contributor Author

I've addressed all the feedback and squashed the history.

  • Moved the Duration imports to the top level of the corresponding modules (unix.rs and tests.rs).
  • Converted libc::timespec directly into a Duration in ControlMetadata::decode.
  • Kept the crate::log:: prefix in unix.rs as it is the prevailing style across the rest of this file.

@thomaseizinger thomaseizinger left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

One more nit, otherwise LGTM

Comment thread quinn-udp/src/lib.rs Outdated
Enable SO_TIMESTAMPNS on Linux and Android, parse SCM_TIMESTAMPNS
ancillary messages, and expose timestamps via RecvMeta::timestamp.
@syszery syszery force-pushed the feat/recv-timestamp branch from 75fde81 to db6b218 Compare May 14, 2026 22:05
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.

SO_TIMESTAMPNS

3 participants