From db975e977813ed6961963f7052ae53bc6df69309 Mon Sep 17 00:00:00 2001 From: Dave Tucker Date: Mon, 31 Jul 2023 11:05:36 +0100 Subject: [PATCH] aya: Don't store bpf_fd in MapData This is only used in create and therefore can be passed as a parameter. Signed-off-by: Dave Tucker --- aya/src/bpf.rs | 7 ++--- aya/src/maps/bloom_filter.rs | 9 ------ aya/src/maps/hash_map/hash_map.rs | 22 ------------- aya/src/maps/lpm_trie.rs | 12 -------- aya/src/maps/mod.rs | 51 +++++++++++++++---------------- aya/src/sys/bpf.rs | 3 +- xtask/public-api/aya.txt | 2 +- 7 files changed, 29 insertions(+), 77 deletions(-) diff --git a/aya/src/bpf.rs b/aya/src/bpf.rs index 6b63c103..ee074dba 100644 --- a/aya/src/bpf.rs +++ b/aya/src/bpf.rs @@ -4,7 +4,7 @@ use std::{ ffi::CString, fs, io, os::{ - fd::{OwnedFd, RawFd}, + fd::{AsFd as _, OwnedFd, RawFd}, raw::c_int, }, path::{Path, PathBuf}, @@ -479,7 +479,6 @@ impl<'a> BpfLoader<'a> { obj, fd: None, pinned: false, - btf_fd: btf_fd.as_ref().map(Arc::clone), }; let fd = match map.obj.pinning() { PinningType::ByName => { @@ -494,7 +493,7 @@ impl<'a> BpfLoader<'a> { fd as RawFd } Err(_) => { - let fd = map.create(&name)?; + let fd = map.create(&name, btf_fd.as_deref().map(|f| f.as_fd()))?; map.pin(&name, path).map_err(|error| MapError::PinError { name: Some(name.to_string()), error, @@ -503,7 +502,7 @@ impl<'a> BpfLoader<'a> { } } } - PinningType::None => map.create(&name)?, + PinningType::None => map.create(&name, btf_fd.as_deref().map(|f| f.as_fd()))?, }; if !map.obj.data().is_empty() && map.obj.section_kind() != BpfSectionKind::Bss { bpf_map_update_elem_ptr(fd, &0 as *const _, map.obj.data_mut().as_mut_ptr(), 0) diff --git a/aya/src/maps/bloom_filter.rs b/aya/src/maps/bloom_filter.rs index b38bccf4..03374b4e 100644 --- a/aya/src/maps/bloom_filter.rs +++ b/aya/src/maps/bloom_filter.rs @@ -116,7 +116,6 @@ mod tests { obj: new_obj_map(), fd: None, pinned: false, - btf_fd: None, }; assert_matches!( BloomFilter::<_, u16>::new(&map), @@ -145,7 +144,6 @@ mod tests { }), fd: None, pinned: false, - btf_fd: None, }; let map = Map::PerfEventArray(map_data); @@ -162,7 +160,6 @@ mod tests { obj: new_obj_map(), fd: None, pinned: false, - btf_fd: None, }; assert_matches!( @@ -177,7 +174,6 @@ mod tests { obj: new_obj_map(), fd: Some(42), pinned: false, - btf_fd: None, }; assert!(BloomFilter::<_, u32>::new(&mut map).is_ok()); @@ -189,7 +185,6 @@ mod tests { obj: new_obj_map(), fd: Some(42), pinned: false, - btf_fd: None, }; let map = Map::BloomFilter(map_data); @@ -204,7 +199,6 @@ mod tests { obj: new_obj_map(), fd: Some(42), pinned: false, - btf_fd: None, }; let bloom_filter = BloomFilter::<_, u32>::new(&mut map).unwrap(); @@ -228,7 +222,6 @@ mod tests { obj: new_obj_map(), fd: Some(42), pinned: false, - btf_fd: None, }; let bloom_filter = BloomFilter::<_, u32>::new(&mut map).unwrap(); @@ -242,7 +235,6 @@ mod tests { obj: new_obj_map(), fd: Some(42), pinned: false, - btf_fd: None, }; let bloom_filter = BloomFilter::<_, u32>::new(&map).unwrap(); @@ -265,7 +257,6 @@ mod tests { obj: new_obj_map(), fd: Some(42), pinned: false, - btf_fd: None, }; let bloom_filter = BloomFilter::<_, u32>::new(&map).unwrap(); diff --git a/aya/src/maps/hash_map/hash_map.rs b/aya/src/maps/hash_map/hash_map.rs index 0cb8e408..cbd2fc81 100644 --- a/aya/src/maps/hash_map/hash_map.rs +++ b/aya/src/maps/hash_map/hash_map.rs @@ -147,7 +147,6 @@ mod tests { obj: new_obj_map(), fd: None, pinned: false, - btf_fd: None, }; assert_matches!( HashMap::<_, u8, u32>::new(&map), @@ -164,7 +163,6 @@ mod tests { obj: new_obj_map(), fd: None, pinned: false, - btf_fd: None, }; assert_matches!( HashMap::<_, u32, u16>::new(&map), @@ -181,7 +179,6 @@ mod tests { obj: new_obj_map(), fd: None, pinned: false, - btf_fd: None, }; let map = Map::Array(map_data); @@ -197,7 +194,6 @@ mod tests { obj: new_obj_map(), fd: None, pinned: false, - btf_fd: None, }; let map = Map::HashMap(map_data); @@ -216,7 +212,6 @@ mod tests { obj: new_obj_map(), fd: None, pinned: false, - btf_fd: None, }; assert_matches!( @@ -231,7 +226,6 @@ mod tests { obj: new_obj_map(), fd: Some(42), pinned: false, - btf_fd: None, }; assert!(HashMap::<_, u32, u32>::new(&mut map).is_ok()); @@ -243,7 +237,6 @@ mod tests { obj: new_obj_map(), fd: Some(42), pinned: false, - btf_fd: None, }; let map = Map::HashMap(map_data); @@ -268,7 +261,6 @@ mod tests { }), fd: Some(42), pinned: false, - btf_fd: None, }; let map = Map::HashMap(map_data); @@ -284,7 +276,6 @@ mod tests { obj: new_obj_map(), fd: Some(42), pinned: false, - btf_fd: None, }; let mut hm = HashMap::<_, u32, u32>::new(&mut map).unwrap(); @@ -308,7 +299,6 @@ mod tests { obj: new_obj_map(), fd: Some(42), pinned: false, - btf_fd: None, }; let mut hm = HashMap::<_, u32, u32>::new(&mut map).unwrap(); @@ -329,7 +319,6 @@ mod tests { obj: new_obj_map(), fd: Some(42), pinned: false, - btf_fd: None, }; let mut hm = HashMap::<_, u32, u32>::new(&mut map).unwrap(); @@ -344,7 +333,6 @@ mod tests { obj: new_obj_map(), fd: Some(42), pinned: false, - btf_fd: None, }; let mut hm = HashMap::<_, u32, u32>::new(&mut map).unwrap(); @@ -368,7 +356,6 @@ mod tests { obj: new_obj_map(), fd: Some(42), pinned: false, - btf_fd: None, }; let mut hm = HashMap::<_, u32, u32>::new(&mut map).unwrap(); @@ -382,7 +369,6 @@ mod tests { obj: new_obj_map(), fd: Some(42), pinned: false, - btf_fd: None, }; let hm = HashMap::<_, u32, u32>::new(&map).unwrap(); @@ -405,7 +391,6 @@ mod tests { obj: new_obj_map(), fd: Some(42), pinned: false, - btf_fd: None, }; let hm = HashMap::<_, u32, u32>::new(&map).unwrap(); @@ -442,7 +427,6 @@ mod tests { obj: new_obj_map(), fd: Some(42), pinned: false, - btf_fd: None, }; let hm = HashMap::<_, u32, u32>::new(&map).unwrap(); let keys = hm.keys().collect::, _>>(); @@ -491,7 +475,6 @@ mod tests { obj: new_obj_map(), fd: Some(42), pinned: false, - btf_fd: None, }; let hm = HashMap::<_, u32, u32>::new(&map).unwrap(); @@ -524,7 +507,6 @@ mod tests { obj: new_obj_map(), fd: Some(42), pinned: false, - btf_fd: None, }; let hm = HashMap::<_, u32, u32>::new(&map).unwrap(); @@ -562,7 +544,6 @@ mod tests { obj: new_obj_map(), fd: Some(42), pinned: false, - btf_fd: None, }; let hm = HashMap::<_, u32, u32>::new(&map).unwrap(); let items = hm.iter().collect::, _>>().unwrap(); @@ -600,7 +581,6 @@ mod tests { obj: new_obj_map(), fd: Some(42), pinned: false, - btf_fd: None, }; let hm = HashMap::<_, u32, u32>::new(&map).unwrap(); @@ -639,7 +619,6 @@ mod tests { obj: new_obj_map(), fd: Some(42), pinned: false, - btf_fd: None, }; let hm = HashMap::<_, u32, u32>::new(&map).unwrap(); @@ -687,7 +666,6 @@ mod tests { obj: new_obj_map(), fd: Some(42), pinned: false, - btf_fd: None, }; let hm = HashMap::<_, u32, u32>::new(&map).unwrap(); diff --git a/aya/src/maps/lpm_trie.rs b/aya/src/maps/lpm_trie.rs index 075aa558..864eb431 100644 --- a/aya/src/maps/lpm_trie.rs +++ b/aya/src/maps/lpm_trie.rs @@ -238,7 +238,6 @@ mod tests { obj: new_obj_map(), fd: None, pinned: false, - btf_fd: None, }; assert_matches!( LpmTrie::<_, u16, u32>::new(&map), @@ -255,7 +254,6 @@ mod tests { obj: new_obj_map(), fd: None, pinned: false, - btf_fd: None, }; assert_matches!( LpmTrie::<_, u32, u16>::new(&map), @@ -283,7 +281,6 @@ mod tests { data: Vec::new(), }), fd: None, - btf_fd: None, pinned: false, }; @@ -301,7 +298,6 @@ mod tests { obj: new_obj_map(), fd: None, pinned: false, - btf_fd: None, }; assert_matches!( @@ -316,7 +312,6 @@ mod tests { obj: new_obj_map(), fd: Some(42), pinned: false, - btf_fd: None, }; assert!(LpmTrie::<_, u32, u32>::new(&mut map).is_ok()); @@ -328,7 +323,6 @@ mod tests { obj: new_obj_map(), fd: Some(42), pinned: false, - btf_fd: None, }; let map = Map::LpmTrie(map_data); @@ -343,7 +337,6 @@ mod tests { obj: new_obj_map(), fd: Some(42), pinned: false, - btf_fd: None, }; let mut trie = LpmTrie::<_, u32, u32>::new(&mut map).unwrap(); let ipaddr = Ipv4Addr::new(8, 8, 8, 8); @@ -368,7 +361,6 @@ mod tests { obj: new_obj_map(), fd: Some(42), pinned: false, - btf_fd: None, }; let mut trie = LpmTrie::<_, u32, u32>::new(&mut map).unwrap(); @@ -385,7 +377,6 @@ mod tests { obj: new_obj_map(), fd: Some(42), pinned: false, - btf_fd: None, }; let mut trie = LpmTrie::<_, u32, u32>::new(&mut map).unwrap(); let ipaddr = Ipv4Addr::new(8, 8, 8, 8); @@ -410,7 +401,6 @@ mod tests { obj: new_obj_map(), fd: Some(42), pinned: false, - btf_fd: None, }; let mut trie = LpmTrie::<_, u32, u32>::new(&mut map).unwrap(); let ipaddr = Ipv4Addr::new(8, 8, 8, 8); @@ -425,7 +415,6 @@ mod tests { obj: new_obj_map(), fd: Some(42), pinned: false, - btf_fd: None, }; let trie = LpmTrie::<_, u32, u32>::new(&map).unwrap(); let ipaddr = Ipv4Addr::new(8, 8, 8, 8); @@ -450,7 +439,6 @@ mod tests { obj: new_obj_map(), fd: Some(42), pinned: false, - btf_fd: None, }; let trie = LpmTrie::<_, u32, u32>::new(&map).unwrap(); let ipaddr = Ipv4Addr::new(8, 8, 8, 8); diff --git a/aya/src/maps/mod.rs b/aya/src/maps/mod.rs index 25ca21be..28fd688e 100644 --- a/aya/src/maps/mod.rs +++ b/aya/src/maps/mod.rs @@ -42,10 +42,9 @@ use std::{ marker::PhantomData, mem, ops::Deref, - os::fd::{AsFd as _, AsRawFd, IntoRawFd as _, OwnedFd, RawFd}, + os::fd::{AsFd as _, AsRawFd, BorrowedFd, IntoRawFd as _, OwnedFd, RawFd}, path::Path, ptr, - sync::Arc, }; use crate::util::KernelVersion; @@ -482,14 +481,17 @@ pub(crate) fn check_v_size(map: &MapData) -> Result<(), MapError> { pub struct MapData { pub(crate) obj: obj::Map, pub(crate) fd: Option, - pub(crate) btf_fd: Option>, /// Indicates if this map has been pinned to bpffs pub pinned: bool, } impl MapData { /// Creates a new map with the provided `name` - pub fn create(&mut self, name: &str) -> Result { + pub fn create( + &mut self, + name: &str, + btf_fd: Option>, + ) -> Result { if self.fd.is_some() { return Err(MapError::AlreadyCreated { name: name.into() }); } @@ -500,23 +502,19 @@ impl MapData { let kernel_version = KernelVersion::current().unwrap(); #[cfg(test)] let kernel_version = KernelVersion::new(0xff, 0xff, 0xff); - let fd = bpf_create_map( - &c_name, - &self.obj, - self.btf_fd.as_ref().map(|f| f.as_fd()), - kernel_version, - ) - .map_err(|(code, io_error)| { - if kernel_version < KernelVersion::new(5, 11, 0) { - maybe_warn_rlimit(); - } + let fd = bpf_create_map(&c_name, &self.obj, btf_fd, kernel_version).map_err( + |(code, io_error)| { + if kernel_version < KernelVersion::new(5, 11, 0) { + maybe_warn_rlimit(); + } - MapError::CreateError { - name: name.into(), - code, - io_error, - } - })?; + MapError::CreateError { + name: name.into(), + code, + io_error, + } + }, + )?; Ok(*self.fd.insert(fd as RawFd)) } @@ -561,7 +559,6 @@ impl MapData { Ok(MapData { obj: parse_map_info(info, PinningType::ByName), fd: Some(fd.into_raw_fd()), - btf_fd: None, pinned: true, }) } @@ -577,7 +574,6 @@ impl MapData { Ok(MapData { obj: parse_map_info(info, PinningType::None), fd: Some(fd.into_raw_fd()), - btf_fd: None, pinned: false, }) } @@ -629,7 +625,6 @@ impl Clone for MapData { MapData { obj: self.obj.clone(), fd: self.fd.map(|fd| unsafe { libc::dup(fd) }), - btf_fd: self.btf_fd.as_ref().map(Arc::clone), pinned: self.pinned, } } @@ -864,7 +859,6 @@ mod tests { obj: new_obj_map(), fd: None, pinned: false, - btf_fd: None, } } @@ -879,9 +873,12 @@ mod tests { }); let mut map = new_map(); - assert_matches!(map.create("foo"), Ok(42)); + assert_matches!(map.create("foo", None), Ok(42)); assert_eq!(map.fd, Some(42)); - assert_matches!(map.create("foo"), Err(MapError::AlreadyCreated { .. })); + assert_matches!( + map.create("foo", None), + Err(MapError::AlreadyCreated { .. }) + ); } #[test] @@ -889,7 +886,7 @@ mod tests { override_syscall(|_| Err((-42, io::Error::from_raw_os_error(EFAULT)))); let mut map = new_map(); - let ret = map.create("foo"); + let ret = map.create("foo", None); assert_matches!(ret, Err(MapError::CreateError { .. })); if let Err(MapError::CreateError { name, diff --git a/aya/src/sys/bpf.rs b/aya/src/sys/bpf.rs index 4e3c3602..1cc144c7 100644 --- a/aya/src/sys/bpf.rs +++ b/aya/src/sys/bpf.rs @@ -732,10 +732,9 @@ pub(crate) fn is_bpf_global_data_supported() -> bool { }), fd: None, pinned: false, - btf_fd: None, }; - if let Ok(map_fd) = map_data.create("aya_global") { + if let Ok(map_fd) = map_data.create("aya_global", None) { insns[0].imm = map_fd; let gpl = b"GPL\0"; diff --git a/xtask/public-api/aya.txt b/xtask/public-api/aya.txt index 69f57fb4..c8e54bec 100644 --- a/xtask/public-api/aya.txt +++ b/xtask/public-api/aya.txt @@ -1230,7 +1230,7 @@ pub fn aya::maps::lpm_trie::LpmTrie::from(t: T) -> T pub struct aya::maps::MapData pub aya::maps::MapData::pinned: bool impl aya::maps::MapData -pub fn aya::maps::MapData::create(&mut self, name: &str) -> core::result::Result +pub fn aya::maps::MapData::create(&mut self, name: &str, btf_fd: core::option::Option>) -> core::result::Result pub fn aya::maps::MapData::fd(&self) -> core::option::Option pub fn aya::maps::MapData::from_fd(fd: std::os::fd::owned::OwnedFd) -> core::result::Result pub fn aya::maps::MapData::from_pin>(path: P) -> core::result::Result