diff --git a/aya/src/bpf.rs b/aya/src/bpf.rs index 413fec61..08b10818 100644 --- a/aya/src/bpf.rs +++ b/aya/src/bpf.rs @@ -854,7 +854,7 @@ impl Bpf { self.maps.iter().map(|(name, map)| (name.as_str(), map)) } - /// An iterator mutably referencing all the maps. + /// An iterator over all the maps that allows modification. /// /// # Examples /// ```no_run diff --git a/aya/src/maps/mod.rs b/aya/src/maps/mod.rs index 5660d768..31bc0bc9 100644 --- a/aya/src/maps/mod.rs +++ b/aya/src/maps/mod.rs @@ -38,6 +38,7 @@ //! implement the [Pod] trait. use std::{ borrow::BorrowMut, + collections::HashSet, ffi::CString, fmt, io, marker::PhantomData, @@ -175,6 +176,10 @@ pub enum MapError { /// The map type map_type: u32, }, + + /// Unable to parse map name from kernel info + #[error("Unable to parse map name")] + ParseNameError, } /// A map file descriptor. @@ -309,7 +314,7 @@ impl Map { } /// Removes the pinned map from a BPF filesystem. - pub fn unpin(&mut self) -> Result<(), io::Error> { + pub fn unpin(&mut self) -> Result<(), PinError> { match self { Self::Array(map) => map.unpin(), Self::PerCpuArray(map) => map.unpin(), @@ -357,7 +362,7 @@ macro_rules! impl_map_pin { } /// Removes the pinned map from a BPF filesystem. - pub fn unpin(mut self) -> Result<(), io::Error> { + pub fn unpin(mut self) -> Result<(), PinError> { let data = self.inner.borrow_mut(); data.unpin() } @@ -504,7 +509,7 @@ pub struct MapData { pub(crate) obj: obj::Map, pub(crate) fd: RawFd, pub(crate) name: String, - pub(crate) paths: Vec, + pub(crate) paths: HashSet, } impl MapData { @@ -539,7 +544,7 @@ impl MapData { obj, fd, name: name.to_string(), - paths: Vec::new(), + paths: HashSet::new(), }) } @@ -570,7 +575,7 @@ impl MapData { obj, fd: fd.into_raw_fd(), name: name.to_string(), - paths: vec![path], + paths: HashSet::from([path]), }), Err(_) => { let mut map = Self::create(obj, name, btf_fd)?; @@ -610,9 +615,9 @@ impl MapData { obj: parse_map_info(info, PinningType::ByName), fd: fd.into_raw_fd(), name: std::str::from_utf8(name_bytes) - .expect("unable to parse map name from bpf_map_info") + .map_err(|_| MapError::ParseNameError)? .to_string(), - paths: vec![path.to_path_buf()], + paths: HashSet::from([path.to_path_buf()]), }) } @@ -629,9 +634,9 @@ impl MapData { obj: parse_map_info(info, PinningType::None), fd: fd.into_raw_fd(), name: std::str::from_utf8(name_bytes) - .expect("unable to parse map name from bpf_map_info") + .map_err(|_| MapError::ParseNameError)? .to_string(), - paths: Vec::new(), + paths: HashSet::new(), }) } @@ -655,25 +660,42 @@ impl MapData { error, } })?; - debug!("Attempting to pin map {name} at {path_string:?}"); + debug!( + "Attempting to pin map {name} at {}", + path.as_ref().display() + ); bpf_pin_object(*fd, &path_string).map_err(|(_, io_error)| SyscallError { call: "BPF_OBJ_PIN", io_error, })?; - paths.push(path.as_ref().to_path_buf()); + paths.insert(path.as_ref().to_path_buf()); Ok(()) } - pub(crate) fn unpin(&mut self) -> Result<(), io::Error> { + pub(crate) fn unpin(&mut self) -> Result<(), PinError> { let Self { fd: _, paths, obj: _, - name: _, + name, } = self; - for path in paths.drain(..) { - std::fs::remove_file(path)?; + + let mut errors: Vec<(PathBuf, io::Error)> = Vec::new(); + for path in paths.drain() { + std::fs::remove_file(&path).unwrap_or_else(|e| { + errors.push((path, e)); + }); } + + if !errors.is_empty() { + let (paths, errors) = errors.into_iter().unzip(); + return Err(PinError::UnpinError { + name: name.to_string(), + paths, + errors, + }); + }; + Ok(()) } diff --git a/aya/src/pin.rs b/aya/src/pin.rs index 743df95f..fdee66e9 100644 --- a/aya/src/pin.rs +++ b/aya/src/pin.rs @@ -31,4 +31,17 @@ pub enum PinError { /// An error ocurred making a syscall. #[error(transparent)] SyscallError(#[from] SyscallError), + + /// An error occured unpinning an object. + #[error("Failed to remove pins {paths:?} for object {name}")] + UnpinError { + /// Object name. + name: String, + + /// The paths. + paths: Vec, + + /// The source error + errors: Vec, + }, } diff --git a/aya/src/programs/mod.rs b/aya/src/programs/mod.rs index a4f68258..6fb3600c 100644 --- a/aya/src/programs/mod.rs +++ b/aya/src/programs/mod.rs @@ -786,9 +786,13 @@ macro_rules! impl_program_pin{ } /// Removes the pinned link from the filesystem. - pub fn unpin(mut self) -> Result<(), io::Error> { + pub fn unpin(mut self) -> Result<(), PinError> { if let Some(path) = self.data.path.take() { - std::fs::remove_file(path)?; + std::fs::remove_file(&path).map_err(|e| PinError::UnpinError{ + name: self.data.name.clone().unwrap(), + paths: vec![path], + errors: vec![e] + })?; } Ok(()) } diff --git a/aya/src/util.rs b/aya/src/util.rs index ef244333..a9242173 100644 --- a/aya/src/util.rs +++ b/aya/src/util.rs @@ -338,11 +338,8 @@ pub(crate) fn bytes_of_slice(val: &[T]) -> &[u8] { } pub(crate) fn bytes_of_c_char(val: &[std::os::raw::c_char]) -> &[u8] { - let length = val - .iter() - .rposition(|ch| *ch != 0) - .map(|pos| pos + 1) - .unwrap_or(0); + // length is the end of the C-style string (nul byte). + let length = val.iter().position(|ch| *ch == 0).unwrap_or(0); // The name field is defined as [std::os::raw::c_char; 16]. c_char may be signed or // unsigned depending on the platform; that's why we're using from_raw_parts here diff --git a/test/integration-test/src/tests/load.rs b/test/integration-test/src/tests/load.rs index 693b80d5..6b267707 100644 --- a/test/integration-test/src/tests/load.rs +++ b/test/integration-test/src/tests/load.rs @@ -91,11 +91,11 @@ fn pin_lifecycle_multiple_btf_maps() { let mut map_pin_by_name: Array<_, u64> = bpf.take_map("map_pin_by_name").unwrap().try_into().unwrap(); - let prog: &mut TracePoint = bpf.program_mut("bpf_prog").unwrap().try_into().unwrap(); + let prog: &mut UProbe = bpf.program_mut("bpf_prog").unwrap().try_into().unwrap(); prog.load().unwrap(); - prog.attach("sched", "sched_switch").unwrap(); - - thread::sleep(time::Duration::from_secs(3)); + prog.attach(Some("trigger_bpf_program"), 0, "/proc/self/exe", None) + .unwrap(); + trigger_bpf_program(); let key = 0; let val_1 = map_1.get(&key, 0).unwrap(); diff --git a/test/integration-test/src/tests/rbpf.rs b/test/integration-test/src/tests/rbpf.rs index c823251b..a2d742ba 100644 --- a/test/integration-test/src/tests/rbpf.rs +++ b/test/integration-test/src/tests/rbpf.rs @@ -44,7 +44,7 @@ fn use_map_with_rbpf() { ); // Initialize maps: - // - fd: 0xCAFE00 or 0xCAFE01 (the 0xCAFE00 part is used to distinguish fds from indices), + // - fd: Bitwise OR of the map_id with 0xCAFE00 (used to distinguish fds from indices), // - Note that rbpf does not convert fds into real pointers, // so we keeps the pointers to our maps in MULTIMAP_MAPS, to be used in helpers. let mut maps = HashMap::new(); @@ -105,13 +105,10 @@ fn use_map_with_rbpf() { .expect("Helper failed"); assert_eq!(vm.execute_program().unwrap(), 0); - assert_eq!(map_instances[0][0], 24); - assert_eq!(map_instances[1][0], 42); - assert_eq!(map_instances[2][0], 44); + assert_eq!(map_instances, vec![vec![24], vec![42], vec![44]]); unsafe { - MULTIMAP_MAPS[0] = null_mut(); - MULTIMAP_MAPS[1] = null_mut(); + MULTIMAP_MAPS.iter_mut().for_each(|v| *v = null_mut()); } }