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

Adding packet receipt timestamp collection into RecvMeta for linux #2151

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Adding packet receipt timestamp collection into RecvMeta for linux #2151

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 5 additions & 1 deletion quinn-udp/src/cmsg/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -83,7 +83,11 @@ impl<M: MsgHdr> Drop for Encoder<'_, M> {
/// `cmsg` must refer to a native cmsg containing a payload of type `T`
pub(crate) unsafe fn decode<T: Copy, C: CMsgHdr>(cmsg: &impl CMsgHdr) -> T {
assert!(mem::align_of::<T>() <= mem::align_of::<C>());
debug_assert_eq!(cmsg.len(), C::cmsg_len(mem::size_of::<T>()));
debug_assert_eq!(
cmsg.len(),
C::cmsg_len(mem::size_of::<T>()),
"cmsg truncated -- you might need to raise the CMSG_LEN constant for this platform"
);
ptr::read(cmsg.cmsg_data() as *const T)
}

Expand Down
2 changes: 2 additions & 0 deletions quinn-udp/src/fallback.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -72,6 +72,8 @@ impl UdpSocketState {
addr: addr.as_socket().unwrap(),
ecn: None,
dst_ip: None,
#[cfg(not(wasm_browser))]
timestamp: None,
};
Ok(1)
}
Expand Down
9 changes: 9 additions & 0 deletions quinn-udp/src/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -115,6 +115,13 @@ pub struct RecvMeta {
/// Populated on platforms: Windows, Linux, Android (API level > 25),
/// FreeBSD, OpenBSD, NetBSD, macOS, and iOS.
pub dst_ip: Option<IpAddr>,
/// A timestamp for when the given packet was received by the operating system.
/// Controlled by the `set_recv_timestamping` option on the source socket where available.
///
/// Populated on platforms with varying clock sources, as follows:
/// - Linux: `CLOCK_REALTIME` (see `SO_TIMESTAMPNS` in `man 7 socket` for more information)
Copy link
Collaborator

@mxinden mxinden Feb 19, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

To me the main benefit of quinn-udp is its single interface abstraction over the various concrete OS-specific UDP mechanisms. Thus I would like to strive for a single interface to the user, abstracting over many potentially different OS-specific implementations. Strive, as in not always achievable.

I am fine with RecvMeta::timestamp only being available on certain OSs, e.g. via #[cfg(. Bugs on the user side will be detected at compile-time.

What I would like to prevent is the value of timestamp to have different meanings across OSs, but no difference in its type signature. Bugs on the user side will only be detected at runtime.

If I understand the above correctly, the Duration embedded in RecvMeta::timestamp should be interpreted as the time since UNIX_EPOCH on Linux. What would we expose on other OSs in the future, also Duration? What is the interpretation of that Duration? Can we find a single abstraction?

Copy link
Contributor Author

@recatek recatek Feb 19, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

It's unfortunately unavoidable that it will have different meanings across operating systems. It can even have different meanings within the same OS depending on how you configure the socket (assuming future support) -- FreeBSD can be configured to report timestamps in CLOCK_REALTIME, CLOCK_MONOTONIC, or CLOCK_BOOTTIME (and potentially others). MacOS has similar support as well. Some of these sources aren't trivially convertible to CLOCK_REALTIME/SystemTime.

That was the idea behind the original RecvTime enum I made to differentiate the clock sources, since we do (usually) know what clock source it's from based on the control message we took the timestamp from. The idea would be that that RecvTime enum could expand to something like:

pub enum RecvTime {
    Realtime(SystemTime),
    Monotonic(MonotonicTime),
    Boottime(BootTime),
}

...and so on. Or could be simplified and use Duration for each, but still be differentiated by the enum discriminant.

However, that makes it a bit of a pain to work with between quinn-proto and quinn-udp since I'd need to replicate that structure in both, and have conversion functions between them. After discussing it with @Ralith on Discord we agreed that going back to a Duration would be the much simpler plan. The user has to configure whether to receive timestamps, and according to what clock, on the socket, and so the user should know based on the OS and their own configuration what time clock source they're getting.

#[cfg(not(wasm_browser))]
pub timestamp: Option<Duration>,
}

impl Default for RecvMeta {
Expand All @@ -126,6 +133,8 @@ impl Default for RecvMeta {
stride: 0,
ecn: None,
dst_ip: None,
#[cfg(not(wasm_browser))]
timestamp: None,
}
}
}
Expand Down
32 changes: 31 additions & 1 deletion quinn-udp/src/unix.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,9 @@ use std::{
time::Instant,
};

#[cfg(target_os = "linux")]
use std::time::Duration;

use socket2::SockRef;

use super::{
Expand Down Expand Up @@ -96,6 +99,11 @@ impl UdpSocketState {
unsafe { libc::CMSG_SPACE(mem::size_of::<libc::in6_pktinfo>() as _) as usize };
}

if cfg!(target_os = "linux") {
cmsg_platform_space +=
unsafe { libc::CMSG_SPACE(mem::size_of::<libc::timeval>() as _) as usize };
}

assert!(
CMSG_LEN
>= unsafe { libc::CMSG_SPACE(mem::size_of::<libc::c_int>() as _) as usize }
Expand Down Expand Up @@ -257,6 +265,18 @@ impl UdpSocketState {
self.may_fragment
}

/// Sets the socket to receive packet receipt timestamps from the operating system.
/// These can be accessed via [`RecvMeta::timestamp`] on packets when enabled.
#[cfg(target_os = "linux")]
pub fn set_recv_timestamping(&self, sock: UdpSockRef<'_>, enabled: bool) -> io::Result<()> {
let enabled = match enabled {
true => OPTION_ON,
false => OPTION_OFF,
};

set_socket_option(&*sock.0, libc::SOL_SOCKET, libc::SO_TIMESTAMPNS, enabled)
}

/// Returns true if we previously got an EINVAL error from `sendmsg` syscall.
fn sendmsg_einval(&self) -> bool {
self.sendmsg_einval.load(Ordering::Relaxed)
Expand Down Expand Up @@ -543,7 +563,7 @@ fn recv(io: SockRef<'_>, bufs: &mut [IoSliceMut<'_>], meta: &mut [RecvMeta]) ->
Ok(1)
}

const CMSG_LEN: usize = 88;
const CMSG_LEN: usize = 96;

fn prepare_msg(
transmit: &Transmit<'_>,
Expand Down Expand Up @@ -681,6 +701,8 @@ fn decode_recv(
let mut dst_ip = None;
#[allow(unused_mut)] // only mutable on Linux
let mut stride = len;
#[allow(unused_mut)] // only mutable on Linux
let mut timestamp = None;

let cmsg_iter = unsafe { cmsg::Iter::new(hdr) };
for cmsg in cmsg_iter {
Expand Down Expand Up @@ -725,6 +747,11 @@ fn decode_recv(
(libc::SOL_UDP, gro::UDP_GRO) => unsafe {
stride = cmsg::decode::<libc::c_int, libc::cmsghdr>(cmsg) as usize;
},
#[cfg(target_os = "linux")]
(libc::SOL_SOCKET, libc::SCM_TIMESTAMPNS) => {
let tv = unsafe { cmsg::decode::<libc::timespec, libc::cmsghdr>(cmsg) };
timestamp = Some(Duration::new(tv.tv_sec as u64, tv.tv_nsec as u32));
}
_ => {}
}
}
Expand Down Expand Up @@ -759,6 +786,7 @@ fn decode_recv(
addr,
ecn: EcnCodepoint::from_bits(ecn_bits),
dst_ip,
timestamp,
}
}

Expand Down Expand Up @@ -907,6 +935,8 @@ fn set_socket_option(
}
}

#[allow(dead_code)]
const OPTION_OFF: libc::c_int = 0;
const OPTION_ON: libc::c_int = 1;

#[cfg(not(any(target_os = "linux", target_os = "android")))]
Expand Down
1 change: 1 addition & 0 deletions quinn-udp/src/windows.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -268,6 +268,7 @@ impl UdpSocketState {
addr: addr.unwrap(),
ecn: EcnCodepoint::from_bits(ecn_bits as u8),
dst_ip,
timestamp: None,
};
Ok(1)
}
Expand Down
68 changes: 68 additions & 0 deletions quinn-udp/tests/tests.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,8 @@ use std::{
net::{IpAddr, Ipv4Addr, Ipv6Addr, UdpSocket},
slice,
};
#[cfg(target_os = "linux")]
use std::{mem::MaybeUninit, time::Duration};

use quinn_udp::{EcnCodepoint, RecvMeta, Transmit, UdpSocketState};
use socket2::Socket;
Expand Down Expand Up @@ -186,6 +188,72 @@ fn gso() {
);
}

#[test]
#[cfg(target_os = "linux")]
fn receive_timestamp() {
let send = UdpSocket::bind((Ipv6Addr::LOCALHOST, 0))
.or_else(|_| UdpSocket::bind((Ipv4Addr::LOCALHOST, 0)))
.unwrap();
let recv = UdpSocket::bind((Ipv6Addr::LOCALHOST, 0))
.or_else(|_| UdpSocket::bind((Ipv4Addr::LOCALHOST, 0)))
.unwrap();
let dst_addr = recv.local_addr().unwrap();

let send_state = UdpSocketState::new((&send).into()).unwrap();
let recv_state = UdpSocketState::new((&recv).into()).unwrap();

recv_state
.set_recv_timestamping((&recv).into(), true)
.expect("failed to set sockopt -- unsupported?");

// Reverse non-blocking flag set by `UdpSocketState` to make the test non-racy
recv.set_nonblocking(false).unwrap();

let mut buf = [0; u16::MAX as usize];
let mut meta = RecvMeta::default();

let mut time_start = MaybeUninit::<libc::timespec>::uninit();
let mut time_end = MaybeUninit::<libc::timespec>::uninit();

// Use the actual CLOCK_REALTIME clock source in linux, rather than relying on SystemTime
let errno = unsafe { libc::clock_gettime(libc::CLOCK_REALTIME, time_start.as_mut_ptr()) };
assert_eq!(errno, 0, "failed to read CLOCK_REALTIME");
let time_start = unsafe { time_start.assume_init() };
let time_start = Duration::new(time_start.tv_sec as u64, time_start.tv_nsec as u32);

send_state
.try_send(
(&send).into(),
&Transmit {
destination: dst_addr,
ecn: None,
contents: b"hello",
segment_size: None,
src_ip: None,
},
)
.unwrap();

recv_state
.recv(
(&recv).into(),
&mut [IoSliceMut::new(&mut buf)],
slice::from_mut(&mut meta),
)
.unwrap();

let errno = unsafe { libc::clock_gettime(libc::CLOCK_REALTIME, time_end.as_mut_ptr()) };
assert_eq!(errno, 0, "failed to read CLOCK_REALTIME");
let time_end = unsafe { time_end.assume_init() };
let time_end = Duration::new(time_end.tv_sec as u64, time_end.tv_nsec as u32);

// Note: there's a very slim chance that the clock could jump (via ntp, etc.) and throw off
// these two checks, but it's still better to validate the timestamp result with that risk
let timestamp = meta.timestamp.unwrap();
assert!(time_start <= timestamp);
assert!(timestamp <= time_end);
}

fn test_send_recv(send: &Socket, recv: &Socket, transmit: Transmit) {
let send_state = UdpSocketState::new(send.into()).unwrap();
let recv_state = UdpSocketState::new(recv.into()).unwrap();
Expand Down