From 02272cce0539a62d2e4b4c0fc30f38a267636b90 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. 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 ++- xtask/public-api/aya.txt | 46 ++++++++++++++++- 6 files changed, 123 insertions(+), 27 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(); diff --git a/xtask/public-api/aya.txt b/xtask/public-api/aya.txt index 862214985..ae2c73b6f 100644 --- a/xtask/public-api/aya.txt +++ b/xtask/public-api/aya.txt @@ -6582,7 +6582,7 @@ pub fn aya::programs::ProgramInfo::fd(&self) -> core::result::Result>(path: P) -> core::result::Result pub fn aya::programs::ProgramInfo::gpl_compatible(&self) -> bool pub fn aya::programs::ProgramInfo::id(&self) -> u32 -pub fn aya::programs::ProgramInfo::loaded_at(&self) -> std::time::SystemTime +pub fn aya::programs::ProgramInfo::loaded_at(&self) -> aya::time::SinceBoot pub fn aya::programs::ProgramInfo::map_ids(&self) -> core::result::Result, aya::programs::ProgramError> pub fn aya::programs::ProgramInfo::memory_locked(&self) -> core::result::Result pub fn aya::programs::ProgramInfo::name(&self) -> &[u8] @@ -7294,6 +7294,50 @@ pub type aya::programs::xdp::XdpLink::Id = aya::programs::xdp::XdpLinkId pub fn aya::programs::xdp::XdpLink::detach(self) -> core::result::Result<(), aya::programs::ProgramError> pub fn aya::programs::xdp::XdpLink::id(&self) -> Self::Id pub fn aya::programs::loaded_programs() -> impl core::iter::traits::iterator::Iterator> +pub mod aya::time +pub struct aya::time::SinceBoot(_) +impl aya::time::SinceBoot +pub fn aya::time::SinceBoot::into_system(self) -> std::time::SystemTime +pub fn aya::time::SinceBoot::now() -> Self +impl core::clone::Clone for aya::time::SinceBoot +pub fn aya::time::SinceBoot::clone(&self) -> aya::time::SinceBoot +impl core::cmp::Eq for aya::time::SinceBoot +impl core::cmp::Ord for aya::time::SinceBoot +pub fn aya::time::SinceBoot::cmp(&self, other: &aya::time::SinceBoot) -> core::cmp::Ordering +impl core::cmp::PartialEq for aya::time::SinceBoot +pub fn aya::time::SinceBoot::eq(&self, other: &aya::time::SinceBoot) -> bool +impl core::cmp::PartialOrd for aya::time::SinceBoot +pub fn aya::time::SinceBoot::partial_cmp(&self, other: &aya::time::SinceBoot) -> core::option::Option +impl core::fmt::Debug for aya::time::SinceBoot +pub fn aya::time::SinceBoot::fmt(&self, f: &mut core::fmt::Formatter<'_>) -> core::fmt::Result +impl core::marker::Copy for aya::time::SinceBoot +impl core::marker::StructuralEq for aya::time::SinceBoot +impl core::marker::StructuralPartialEq for aya::time::SinceBoot +impl core::marker::Send for aya::time::SinceBoot +impl core::marker::Sync for aya::time::SinceBoot +impl core::marker::Unpin for aya::time::SinceBoot +impl core::panic::unwind_safe::RefUnwindSafe for aya::time::SinceBoot +impl core::panic::unwind_safe::UnwindSafe for aya::time::SinceBoot +impl core::convert::Into for aya::time::SinceBoot where U: core::convert::From +pub fn aya::time::SinceBoot::into(self) -> U +impl core::convert::TryFrom for aya::time::SinceBoot where U: core::convert::Into +pub type aya::time::SinceBoot::Error = core::convert::Infallible +pub fn aya::time::SinceBoot::try_from(value: U) -> core::result::Result>::Error> +impl core::convert::TryInto for aya::time::SinceBoot where U: core::convert::TryFrom +pub type aya::time::SinceBoot::Error = >::Error +pub fn aya::time::SinceBoot::try_into(self) -> core::result::Result>::Error> +impl alloc::borrow::ToOwned for aya::time::SinceBoot where T: core::clone::Clone +pub type aya::time::SinceBoot::Owned = T +pub fn aya::time::SinceBoot::clone_into(&self, target: &mut T) +pub fn aya::time::SinceBoot::to_owned(&self) -> T +impl core::any::Any for aya::time::SinceBoot where T: 'static + core::marker::Sized +pub fn aya::time::SinceBoot::type_id(&self) -> core::any::TypeId +impl core::borrow::Borrow for aya::time::SinceBoot where T: core::marker::Sized +pub fn aya::time::SinceBoot::borrow(&self) -> &T +impl core::borrow::BorrowMut for aya::time::SinceBoot where T: core::marker::Sized +pub fn aya::time::SinceBoot::borrow_mut(&mut self) -> &mut T +impl core::convert::From for aya::time::SinceBoot +pub fn aya::time::SinceBoot::from(t: T) -> T pub mod aya::util pub struct aya::util::KernelVersion impl aya::util::KernelVersion