From ff86f1385c1c45ed8dadcf160f2d4ae67b39ec13 Mon Sep 17 00:00:00 2001 From: Tamir Duberstein Date: Thu, 6 Jul 2023 16:40:26 -0400 Subject: [PATCH 1/4] Remove dependency on bpftool in integration tests --- aya/src/programs/mod.rs | 6 ++++-- test/integration-test/src/tests/load.rs | 16 +++------------- test/run.sh | 2 +- xtask/src/run.rs | 9 +++++++-- 4 files changed, 15 insertions(+), 18 deletions(-) diff --git a/aya/src/programs/mod.rs b/aya/src/programs/mod.rs index d903cca7..561b3a1a 100644 --- a/aya/src/programs/mod.rs +++ b/aya/src/programs/mod.rs @@ -999,12 +999,14 @@ impl Iterator for ProgramsIter { io_error, }) .and_then(|fd| { - bpf_prog_get_info_by_fd(fd) + let info = bpf_prog_get_info_by_fd(fd) .map_err(|io_error| ProgramError::SyscallError { call: "bpf_prog_get_info_by_fd".to_owned(), io_error, }) - .map(ProgramInfo) + .map(ProgramInfo); + unsafe { libc::close(fd) }; + info }), ) } diff --git a/test/integration-test/src/tests/load.rs b/test/integration-test/src/tests/load.rs index 9aa21bb5..f78f3f73 100644 --- a/test/integration-test/src/tests/load.rs +++ b/test/integration-test/src/tests/load.rs @@ -1,11 +1,11 @@ -use std::{convert::TryInto, process::Command, thread, time}; +use std::{convert::TryInto as _, thread, time}; use aya::{ include_bytes_aligned, maps::Array, programs::{ links::{FdLink, PinnedLink}, - KProbe, TracePoint, Xdp, XdpFlags, + loaded_programs, KProbe, TracePoint, Xdp, XdpFlags, }, Bpf, }; @@ -58,20 +58,10 @@ fn multiple_btf_maps() { assert_eq!(val_2, 42); } -fn is_loaded(name: &str) -> bool { - let output = Command::new("bpftool").args(["prog"]).output(); - let output = match output { - Err(e) => panic!("Failed to run 'bpftool prog': {e}"), - Ok(out) => out, - }; - let stdout = String::from_utf8(output.stdout).unwrap(); - stdout.contains(name) -} - macro_rules! assert_loaded { ($name:literal, $loaded:expr) => { for i in 0..(MAX_RETRIES + 1) { - let state = is_loaded($name); + let state = loaded_programs().any(|prog| prog.unwrap().name() == $name.as_bytes()); if state == $loaded { break; } diff --git a/test/run.sh b/test/run.sh index 46caf224..67056c83 100755 --- a/test/run.sh +++ b/test/run.sh @@ -193,7 +193,7 @@ EOF exec_vm sudo dnf config-manager --set-enabled updates-testing exec_vm sudo dnf config-manager --set-enabled updates-testing-modular echo "Installing dependencies" - exec_vm sudo dnf install -qy bpftool llvm llvm-devel clang clang-devel zlib-devel + exec_vm sudo dnf install -qy llvm llvm-devel clang clang-devel zlib-devel exec_vm 'curl --proto '=https' --tlsv1.2 -sSf https://sh.rustup.rs | sh -s -- \ -y --profile minimal --default-toolchain nightly --component rust-src --component clippy' exec_vm 'echo source ~/.cargo/env >> ~/.bashrc' diff --git a/xtask/src/run.rs b/xtask/src/run.rs index c433c3f7..eb48f7ad 100644 --- a/xtask/src/run.rs +++ b/xtask/src/run.rs @@ -36,8 +36,13 @@ fn build(opts: &Options) -> Result<(), anyhow::Error> { .args(&args) .status() .expect("failed to build userspace"); - assert!(status.success()); - Ok(()) + match status.code() { + Some(code) => match code { + 0 => Ok(()), + code => Err(anyhow::anyhow!("exited with status code: {code}")), + }, + None => Err(anyhow::anyhow!("process terminated by signal")), + } } /// Build and run the project From 5a2906a6c9dac53f1dfe922349295b772f1bad32 Mon Sep 17 00:00:00 2001 From: Tamir Duberstein Date: Fri, 7 Jul 2023 11:14:51 -0400 Subject: [PATCH 2/4] test,xtask: Replace lazy_static with OnceCell This doesn't affect MSRV because this is testing code. --- test/integration-test/Cargo.toml | 1 - test/integration-test/src/tests/mod.rs | 11 +++++------ xtask/Cargo.toml | 1 - xtask/src/build_ebpf.rs | 8 ++++---- xtask/src/utils.rs | 26 ++++++++++++-------------- 5 files changed, 21 insertions(+), 26 deletions(-) diff --git a/test/integration-test/Cargo.toml b/test/integration-test/Cargo.toml index 24975368..174d66fb 100644 --- a/test/integration-test/Cargo.toml +++ b/test/integration-test/Cargo.toml @@ -14,7 +14,6 @@ env_logger = "0.10" futures-core = "0.3" inventory = "0.3" integration-test-macros = { path = "../integration-test-macros" } -lazy_static = "1" libc = { version = "0.2.105" } log = "0.4" object = { version = "0.31", default-features = false, features = ["std", "read_core", "elf"] } diff --git a/test/integration-test/src/tests/mod.rs b/test/integration-test/src/tests/mod.rs index 93e6aaa3..50f92820 100644 --- a/test/integration-test/src/tests/mod.rs +++ b/test/integration-test/src/tests/mod.rs @@ -1,8 +1,7 @@ use anyhow::bail; -use lazy_static::lazy_static; use libc::{uname, utsname}; use regex::Regex; -use std::{ffi::CStr, mem}; +use std::{cell::OnceCell, ffi::CStr, mem}; pub mod bpf_probe_read; pub mod btf_relocations; @@ -22,15 +21,15 @@ pub struct IntegrationTest { } pub(crate) fn kernel_version() -> anyhow::Result<(u8, u8, u8)> { - lazy_static! { - static ref RE: Regex = Regex::new(r"^([0-9]+)\.([0-9]+)\.([0-9]+)").unwrap(); - } + static mut RE: OnceCell = OnceCell::new(); + let re = + unsafe { &mut RE }.get_or_init(|| Regex::new(r"^([0-9]+)\.([0-9]+)\.([0-9]+)").unwrap()); let mut data: utsname = unsafe { mem::zeroed() }; let ret = unsafe { uname(&mut data) }; assert!(ret >= 0, "libc::uname failed."); let release_cstr = unsafe { CStr::from_ptr(data.release.as_ptr()) }; let release = release_cstr.to_string_lossy(); - if let Some(caps) = RE.captures(&release) { + if let Some(caps) = re.captures(&release) { let major = caps.get(1).unwrap().as_str().parse().unwrap(); let minor = caps.get(2).unwrap().as_str().parse().unwrap(); let patch = caps.get(3).unwrap().as_str().parse().unwrap(); diff --git a/xtask/Cargo.toml b/xtask/Cargo.toml index 671a987c..592534e6 100644 --- a/xtask/Cargo.toml +++ b/xtask/Cargo.toml @@ -12,5 +12,4 @@ syn = "2" quote = "1" proc-macro2 = "1" indoc = "2.0" -lazy_static = "1" serde_json = "1" diff --git a/xtask/src/build_ebpf.rs b/xtask/src/build_ebpf.rs index cf2b50f8..465980c5 100644 --- a/xtask/src/build_ebpf.rs +++ b/xtask/src/build_ebpf.rs @@ -7,7 +7,7 @@ use std::{ use anyhow::{bail, Context}; use clap::Parser; -use crate::utils::WORKSPACE_ROOT; +use crate::utils::workspace_root; #[derive(Debug, Copy, Clone)] pub enum Architecture { @@ -52,7 +52,7 @@ pub fn build_ebpf(opts: BuildEbpfOptions) -> anyhow::Result<()> { } fn build_rust_ebpf(opts: &BuildEbpfOptions) -> anyhow::Result<()> { - let mut dir = PathBuf::from(WORKSPACE_ROOT.to_string()); + let mut dir = PathBuf::from(workspace_root()); dir.push("test/integration-ebpf"); let target = format!("--target={}", opts.target); @@ -88,10 +88,10 @@ fn get_libbpf_headers>(libbpf_dir: P, include_path: P) -> anyhow: } fn build_c_ebpf(opts: &BuildEbpfOptions) -> anyhow::Result<()> { - let mut src = PathBuf::from(WORKSPACE_ROOT.to_string()); + let mut src = PathBuf::from(workspace_root()); src.push("test/integration-ebpf/src/bpf"); - let mut out_path = PathBuf::from(WORKSPACE_ROOT.to_string()); + let mut out_path = PathBuf::from(workspace_root()); out_path.push("target"); out_path.push(opts.target.to_string()); out_path.push("release"); diff --git a/xtask/src/utils.rs b/xtask/src/utils.rs index fcfb918c..e1e3eb2a 100644 --- a/xtask/src/utils.rs +++ b/xtask/src/utils.rs @@ -1,17 +1,15 @@ -use lazy_static::lazy_static; use serde_json::Value; -use std::process::Command; +use std::{cell::OnceCell, process::Command}; -lazy_static! { - pub static ref WORKSPACE_ROOT: String = workspace_root(); -} - -fn workspace_root() -> String { - let output = Command::new("cargo").arg("metadata").output().unwrap(); - if !output.status.success() { - panic!("unable to run cargo metadata") - } - let stdout = String::from_utf8(output.stdout).unwrap(); - let v: Value = serde_json::from_str(&stdout).unwrap(); - v["workspace_root"].as_str().unwrap().to_string() +pub fn workspace_root() -> &'static str { + static mut WORKSPACE_ROOT: OnceCell = OnceCell::new(); + unsafe { &mut WORKSPACE_ROOT }.get_or_init(|| { + let output = Command::new("cargo").arg("metadata").output().unwrap(); + if !output.status.success() { + panic!("unable to run cargo metadata") + } + let stdout = String::from_utf8(output.stdout).unwrap(); + let v: Value = serde_json::from_str(&stdout).unwrap(); + v["workspace_root"].as_str().unwrap().to_string() + }) } From 7067db450abfa9ea60756702d1248cdb88e843d7 Mon Sep 17 00:00:00 2001 From: Tamir Duberstein Date: Fri, 7 Jul 2023 11:18:51 -0400 Subject: [PATCH 3/4] xtask: Avoid lossy string conversion --- xtask/src/build_ebpf.rs | 23 +++++++++++++++-------- 1 file changed, 15 insertions(+), 8 deletions(-) diff --git a/xtask/src/build_ebpf.rs b/xtask/src/build_ebpf.rs index 465980c5..3a100fae 100644 --- a/xtask/src/build_ebpf.rs +++ b/xtask/src/build_ebpf.rs @@ -1,5 +1,8 @@ use std::{ - env, fs, + borrow::Cow, + env, + ffi::{OsStr, OsString}, + fs, path::{Path, PathBuf}, process::Command, }; @@ -77,9 +80,12 @@ fn build_rust_ebpf(opts: &BuildEbpfOptions) -> anyhow::Result<()> { fn get_libbpf_headers>(libbpf_dir: P, include_path: P) -> anyhow::Result<()> { let dir = include_path.as_ref(); fs::create_dir_all(dir)?; + let mut includedir = OsString::new(); + includedir.push("INCLUDEDIR="); + includedir.push(dir.as_os_str()); let status = Command::new("make") .current_dir(libbpf_dir.as_ref().join("src")) - .arg(format!("INCLUDEDIR={}", dir.as_os_str().to_string_lossy())) + .arg(includedir) .arg("install_headers") .status() .expect("failed to build get libbpf headers"); @@ -119,17 +125,18 @@ fn compile_with_clang>( out: P, include_path: P, ) -> anyhow::Result<()> { - let clang = match env::var("CLANG") { - Ok(val) => val, - Err(_) => String::from("/usr/bin/clang"), + let clang: Cow<'_, _> = match env::var_os("CLANG") { + Some(val) => val.into(), + None => OsStr::new("/usr/bin/clang").into(), }; - let arch = match std::env::consts::ARCH { + let arch = match env::consts::ARCH { "x86_64" => "x86", "aarch64" => "arm64", - _ => std::env::consts::ARCH, + arch => arch, }; let mut cmd = Command::new(clang); - cmd.arg(format!("-I{}", include_path.as_ref().to_string_lossy())) + cmd.arg("-I") + .arg(include_path.as_ref()) .arg("-g") .arg("-O2") .arg("-target") From 25859e4e334e7f97ba7777f318ae9cde551f9647 Mon Sep 17 00:00:00 2001 From: Tamir Duberstein Date: Fri, 7 Jul 2023 11:24:22 -0400 Subject: [PATCH 4/4] xtask: emit clang command on failure --- xtask/src/build_ebpf.rs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/xtask/src/build_ebpf.rs b/xtask/src/build_ebpf.rs index 3a100fae..6f389ed4 100644 --- a/xtask/src/build_ebpf.rs +++ b/xtask/src/build_ebpf.rs @@ -135,7 +135,8 @@ fn compile_with_clang>( arch => arch, }; let mut cmd = Command::new(clang); - cmd.arg("-I") + cmd.arg("-v") + .arg("-I") .arg(include_path.as_ref()) .arg("-g") .arg("-O2")