Skip to content

Commit

Permalink
aya: use montonic SinceBoot for loaded_at
Browse files Browse the repository at this point in the history
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`.
  • Loading branch information
ajwerner committed Sep 28, 2023
1 parent 373fb7b commit e888291
Show file tree
Hide file tree
Showing 5 changed files with 78 additions and 26 deletions.
1 change: 1 addition & 0 deletions aya/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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::*;
Expand Down
9 changes: 5 additions & 4 deletions aya/src/programs/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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,
};
Expand Down Expand Up @@ -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.
Expand Down
20 changes: 0 additions & 20 deletions aya/src/programs/utils.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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::{
Expand Down Expand Up @@ -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::<libc::timespec>() };
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<u32, ProgramError> {
let info = File::open(format!("/proc/self/fdinfo/{}", fd.as_raw_fd()))?;
Expand Down
68 changes: 68 additions & 0 deletions aya/src/time.rs
Original file line number Diff line number Diff line change
@@ -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::<libc::timespec>() };
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
);
}
}
6 changes: 4 additions & 2 deletions test/integration-test/src/tests/load.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ use aya::{
links::{FdLink, PinnedLink},
loaded_links, loaded_programs, KProbe, TracePoint, UProbe, Xdp, XdpFlags,
},
time::SinceBoot,
util::KernelVersion,
Bpf,
};
Expand Down Expand Up @@ -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();
Expand Down

0 comments on commit e888291

Please sign in to comment.