From e888291ea6f837ae7466b0282e478f927219d0f5 Mon Sep 17 00:00:00 2001 From: Andrew Werner Date: Thu, 28 Sep 2023 11:38:34 -0400 Subject: [PATCH] aya: use montonic SinceBoot for loaded_at This commit was written to deflake test_loaded_at can. The flakiness is due to the lack of monotonicity in SystemTime clock calculations. See https://github.com/aya-rs/aya/actions/runs/6340369670/job/17221591723. This PR addresses that problem by introducing a wrapper type to encapsulate the monotonically increasing duration since boot, `SinceBoot`. This type has a method `into_system_time()` to convert it into a `SystemTime`. --- aya/src/lib.rs | 1 + aya/src/programs/mod.rs | 9 ++-- aya/src/programs/utils.rs | 20 -------- aya/src/time.rs | 68 +++++++++++++++++++++++++ test/integration-test/src/tests/load.rs | 6 ++- 5 files changed, 78 insertions(+), 26 deletions(-) create mode 100644 aya/src/time.rs diff --git a/aya/src/lib.rs b/aya/src/lib.rs index 743ea26d8..c61b3254a 100644 --- a/aya/src/lib.rs +++ b/aya/src/lib.rs @@ -87,6 +87,7 @@ pub mod pin; pub mod programs; pub use programs::loaded_programs; mod sys; +pub mod time; pub mod util; pub use bpf::*; diff --git a/aya/src/programs/mod.rs b/aya/src/programs/mod.rs index 64da0fdff..fbdc5486a 100644 --- a/aya/src/programs/mod.rs +++ b/aya/src/programs/mod.rs @@ -71,7 +71,6 @@ use std::{ os::fd::{AsFd, AsRawFd, BorrowedFd, OwnedFd}, path::{Path, PathBuf}, sync::Arc, - time::{Duration, SystemTime}, }; pub use cgroup_device::CgroupDevice; @@ -110,13 +109,14 @@ use crate::{ maps::MapError, obj::{self, btf::BtfError, VerifierLog}, pin::PinError, - programs::utils::{boot_time, get_fdinfo}, + programs::utils::get_fdinfo, sys::{ bpf_btf_get_fd_by_id, bpf_get_object, bpf_link_get_fd_by_id, bpf_link_get_info_by_fd, bpf_load_program, bpf_pin_object, bpf_prog_get_fd_by_id, bpf_prog_get_info_by_fd, bpf_prog_query, iter_link_ids, iter_prog_ids, retry_with_verifier_logs, BpfLoadProgramAttrs, SyscallError, }, + time::SinceBoot, util::KernelVersion, VerifierLogLevel, }; @@ -1086,8 +1086,9 @@ impl ProgramInfo { } /// The time the program was loaded. - pub fn loaded_at(&self) -> SystemTime { - boot_time() + Duration::from_nanos(self.0.load_time) + pub fn loaded_at(&self) -> SinceBoot { + // See https://github.com/torvalds/linux/blob/633b47cb/include/linux/bpf.h#L1418. + SinceBoot::from_nanos(self.0.load_time) } /// Returns a file descriptor referencing the program. diff --git a/aya/src/programs/utils.rs b/aya/src/programs/utils.rs index 0930e62b1..a3b197cc3 100644 --- a/aya/src/programs/utils.rs +++ b/aya/src/programs/utils.rs @@ -5,7 +5,6 @@ use std::{ io::{self, BufRead, BufReader}, os::fd::{AsFd as _, AsRawFd as _, BorrowedFd}, path::Path, - time::{Duration, SystemTime, UNIX_EPOCH}, }; use crate::{ @@ -58,25 +57,6 @@ pub(crate) fn find_tracefs_path() -> Result<&'static Path, ProgramError> { .ok_or_else(|| io::Error::new(io::ErrorKind::Other, "tracefs not found").into()) } -/// The time at which the system is booted. -pub(crate) fn boot_time() -> SystemTime { - let get_time = |clock_id| { - let mut time = unsafe { std::mem::zeroed::() }; - assert_eq!( - unsafe { libc::clock_gettime(clock_id, &mut time) }, - 0, - "clock_gettime({}, _)", - clock_id - ); - let libc::timespec { tv_sec, tv_nsec } = time; - - Duration::new(tv_sec as u64, tv_nsec as u32) - }; - let since_boot = get_time(libc::CLOCK_BOOTTIME); - let since_epoch = get_time(libc::CLOCK_REALTIME); - UNIX_EPOCH + since_epoch - since_boot -} - /// Get the specified information from a file descriptor's fdinfo. pub(crate) fn get_fdinfo(fd: BorrowedFd<'_>, key: &str) -> Result { let info = File::open(format!("/proc/self/fdinfo/{}", fd.as_raw_fd()))?; diff --git a/aya/src/time.rs b/aya/src/time.rs new file mode 100644 index 000000000..fed5b7936 --- /dev/null +++ b/aya/src/time.rs @@ -0,0 +1,68 @@ +//! Utilities for working with time. + +use std::time::{Duration, SystemTime, UNIX_EPOCH}; + +/// A timestamp relative to the system boot time. +#[derive(Debug, Clone, Copy, PartialEq, Eq, PartialOrd, Ord)] +pub struct SinceBoot(Duration); + +impl SinceBoot { + /// The current SinceBoot. + /// + /// Note that these calls will be monotonic. + pub fn now() -> Self { + Self(get_time(libc::CLOCK_BOOTTIME)) + } + + /// Converts the timestamp to a `SystemTime`. + /// + /// Note that this will not be robust to changes in the system clock, and thus these + /// times should not be used for comparisons. + pub fn into_system(self) -> SystemTime { + let Self(since_boot) = self; + boot_time() + since_boot + } + + pub(crate) fn from_nanos(nanos: u64) -> Self { + Self(Duration::from_nanos(nanos)) + } +} + +fn boot_time() -> SystemTime { + let since_boot = get_time(libc::CLOCK_BOOTTIME); + let since_epoch = get_time(libc::CLOCK_REALTIME); + UNIX_EPOCH + since_epoch - since_boot +} + +fn get_time(clock_id: libc::clockid_t) -> Duration { + let mut time = unsafe { std::mem::zeroed::() }; + assert_eq!( + unsafe { libc::clock_gettime(clock_id, &mut time) }, + 0, + "clock_gettime({}, _)", + clock_id + ); + let libc::timespec { tv_sec, tv_nsec } = time; + Duration::new(tv_sec as u64, tv_nsec as u32) +} + +#[cfg(test)] +mod test { + use super::*; + + #[test] + fn test_since_boot_now_into_system_near_system_time() { + let since_boot = SinceBoot::now().into_system(); + let system_time = SystemTime::now(); + let delta = system_time + .duration_since(since_boot) + .unwrap_or_else(|err| err.duration()); + const MAX_DELTA: Duration = Duration::from_micros(10); + assert!( + delta <= MAX_DELTA, + "delta {delta:?} > {MAX_DELTA:?}: since_boot: {:?}, system_time: {:?}", + since_boot, + system_time + ); + } +} diff --git a/test/integration-test/src/tests/load.rs b/test/integration-test/src/tests/load.rs index 795f396d7..622f6a0d3 100644 --- a/test/integration-test/src/tests/load.rs +++ b/test/integration-test/src/tests/load.rs @@ -10,6 +10,7 @@ use aya::{ links::{FdLink, PinnedLink}, loaded_links, loaded_programs, KProbe, TracePoint, UProbe, Xdp, XdpFlags, }, + time::SinceBoot, util::KernelVersion, Bpf, }; @@ -154,9 +155,10 @@ fn unload_xdp() { fn test_loaded_at() { let mut bpf = Bpf::load(crate::TEST).unwrap(); let prog: &mut Xdp = bpf.program_mut("pass").unwrap().try_into().unwrap(); - let t1 = SystemTime::now(); + + let t1 = SinceBoot::now(); prog.load().unwrap(); - let t2 = SystemTime::now(); + let t2 = SinceBoot::now(); assert_loaded("pass"); let loaded_at = prog.info().unwrap().loaded_at();