From f41592663cda156082255b93db145cfdd19378e5 Mon Sep 17 00:00:00 2001 From: Tamir Duberstein Date: Tue, 29 Aug 2023 17:19:20 -0400 Subject: [PATCH 1/3] maps: `MapFd` and `SockMapFd` are owned `MapData::fd` is now a `MapFd`. This means that `MapData` now closes the file descriptor on drop. In the future we might consider making `MapFd` hold a `BorrowedFd` but this requires API design work due to overlapping borrows. Since `SockMapFd` is no longer `Copy`, attach methods to take it by reference to allow callers to use it multiple times as they are accustomed to doing. `SockMapFd` implements `try_clone`. `MapFd` and `SockMapFd` are now returned by reference to allow callers to avoid file descriptor cloning when desired. This is an API breaking change. Updates #612. --- aya-obj/src/lib.rs | 6 ++ aya-obj/src/relocation.rs | 6 +- aya/src/bpf.rs | 14 ++-- aya/src/maps/array/array.rs | 5 +- aya/src/maps/array/per_cpu_array.rs | 5 +- aya/src/maps/array/program_array.rs | 4 +- aya/src/maps/bloom_filter.rs | 5 +- aya/src/maps/hash_map/hash_map.rs | 3 +- aya/src/maps/hash_map/mod.rs | 5 +- aya/src/maps/hash_map/per_cpu_hash_map.rs | 5 +- aya/src/maps/lpm_trie.rs | 7 +- aya/src/maps/mod.rs | 94 +++++++++++------------ aya/src/maps/perf/perf_event_array.rs | 4 +- aya/src/maps/queue.rs | 5 +- aya/src/maps/sock/mod.rs | 26 +++++-- aya/src/maps/sock/sock_hash.rs | 18 +++-- aya/src/maps/sock/sock_map.rs | 32 +++++--- aya/src/maps/stack.rs | 5 +- aya/src/maps/stack_trace.rs | 4 +- aya/src/programs/links.rs | 8 +- aya/src/programs/mod.rs | 7 +- aya/src/programs/sk_msg.rs | 6 +- aya/src/programs/sk_skb.rs | 19 ++++- aya/src/programs/utils.rs | 2 +- aya/src/sys/bpf.rs | 55 +++++++------ test/integration-test/src/tests/rbpf.rs | 2 +- xtask/public-api/aya-obj.txt | 4 +- xtask/public-api/aya.txt | 41 ++++------ 28 files changed, 222 insertions(+), 175 deletions(-) diff --git a/aya-obj/src/lib.rs b/aya-obj/src/lib.rs index d6d2a8a5..ea0e5670 100644 --- a/aya-obj/src/lib.rs +++ b/aya-obj/src/lib.rs @@ -77,6 +77,12 @@ mod std { pub use core_error::Error; } pub use core::*; + + pub mod os { + pub mod fd { + pub type RawFd = core::ffi::c_int; + } + } } pub mod btf; diff --git a/aya-obj/src/relocation.rs b/aya-obj/src/relocation.rs index 8269f6ca..83879fc8 100644 --- a/aya-obj/src/relocation.rs +++ b/aya-obj/src/relocation.rs @@ -105,7 +105,7 @@ pub(crate) struct Symbol { impl Object { /// Relocates the map references - pub fn relocate_maps<'a, I: Iterator>( + pub fn relocate_maps<'a, I: Iterator>( &mut self, maps: I, text_sections: &HashSet, @@ -178,8 +178,8 @@ impl Object { fn relocate_maps<'a, I: Iterator>( fun: &mut Function, relocations: I, - maps_by_section: &HashMap, - maps_by_symbol: &HashMap, + maps_by_section: &HashMap, + maps_by_symbol: &HashMap, symbol_table: &HashMap, text_sections: &HashSet, ) -> Result<(), RelocationError> { diff --git a/aya/src/bpf.rs b/aya/src/bpf.rs index f967c737..d81007c7 100644 --- a/aya/src/bpf.rs +++ b/aya/src/bpf.rs @@ -3,7 +3,7 @@ use std::{ collections::{HashMap, HashSet}, fs, io, os::{ - fd::{AsFd as _, OwnedFd}, + fd::{AsFd as _, AsRawFd as _, OwnedFd}, raw::c_int, }, path::{Path, PathBuf}, @@ -482,17 +482,17 @@ impl<'a> BpfLoader<'a> { MapData::create_pinned(path, obj, &name, btf_fd)? } }; - let fd = map.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) + let MapData { obj, fd, pinned: _ } = &mut map; + if !obj.data().is_empty() && obj.section_kind() != BpfSectionKind::Bss { + bpf_map_update_elem_ptr(fd.as_fd(), &0 as *const _, obj.data_mut().as_mut_ptr(), 0) .map_err(|(_, io_error)| SyscallError { call: "bpf_map_update_elem", io_error, }) .map_err(MapError::from)?; } - if map.obj.section_kind() == BpfSectionKind::Rodata { - bpf_map_freeze(fd) + if obj.section_kind() == BpfSectionKind::Rodata { + bpf_map_freeze(fd.as_fd()) .map_err(|(_, io_error)| SyscallError { call: "bpf_map_freeze", io_error, @@ -510,7 +510,7 @@ impl<'a> BpfLoader<'a> { obj.relocate_maps( maps.iter() - .map(|(s, data)| (s.as_str(), data.fd, &data.obj)), + .map(|(s, data)| (s.as_str(), data.fd().as_fd().as_raw_fd(), &data.obj)), &text_sections, )?; obj.relocate_calls(&text_sections)?; diff --git a/aya/src/maps/array/array.rs b/aya/src/maps/array/array.rs index 64454a3c..5d137d05 100644 --- a/aya/src/maps/array/array.rs +++ b/aya/src/maps/array/array.rs @@ -1,6 +1,7 @@ use std::{ borrow::{Borrow, BorrowMut}, marker::PhantomData, + os::fd::AsFd as _, }; use crate::{ @@ -61,7 +62,7 @@ impl, V: Pod> Array { pub fn get(&self, index: &u32, flags: u64) -> Result { let data = self.inner.borrow(); check_bounds(data, *index)?; - let fd = data.fd; + let fd = data.fd().as_fd(); let value = bpf_map_lookup_elem(fd, index, flags).map_err(|(_, io_error)| SyscallError { @@ -88,7 +89,7 @@ impl, V: Pod> Array { pub fn set(&mut self, index: u32, value: impl Borrow, flags: u64) -> Result<(), MapError> { let data = self.inner.borrow_mut(); check_bounds(data, index)?; - let fd = data.fd; + let fd = data.fd().as_fd(); bpf_map_update_elem(fd, Some(&index), value.borrow(), flags).map_err(|(_, io_error)| { SyscallError { call: "bpf_map_update_elem", diff --git a/aya/src/maps/array/per_cpu_array.rs b/aya/src/maps/array/per_cpu_array.rs index db6169bd..f384b713 100644 --- a/aya/src/maps/array/per_cpu_array.rs +++ b/aya/src/maps/array/per_cpu_array.rs @@ -1,6 +1,7 @@ use std::{ borrow::{Borrow, BorrowMut}, marker::PhantomData, + os::fd::AsFd as _, }; use crate::{ @@ -80,7 +81,7 @@ impl, V: Pod> PerCpuArray { pub fn get(&self, index: &u32, flags: u64) -> Result, MapError> { let data = self.inner.borrow(); check_bounds(data, *index)?; - let fd = data.fd; + let fd = data.fd().as_fd(); let value = bpf_map_lookup_elem_per_cpu(fd, index, flags).map_err(|(_, io_error)| { SyscallError { @@ -108,7 +109,7 @@ impl, V: Pod> PerCpuArray { pub fn set(&mut self, index: u32, values: PerCpuValues, flags: u64) -> Result<(), MapError> { let data = self.inner.borrow_mut(); check_bounds(data, index)?; - let fd = data.fd; + let fd = data.fd().as_fd(); bpf_map_update_elem_per_cpu(fd, &index, &values, flags).map_err(|(_, io_error)| { SyscallError { diff --git a/aya/src/maps/array/program_array.rs b/aya/src/maps/array/program_array.rs index a38ce023..48bb6b9d 100644 --- a/aya/src/maps/array/program_array.rs +++ b/aya/src/maps/array/program_array.rs @@ -74,7 +74,7 @@ impl> ProgramArray { pub fn set(&mut self, index: u32, program: &ProgramFd, flags: u64) -> Result<(), MapError> { let data = self.inner.borrow_mut(); check_bounds(data, index)?; - let fd = data.fd; + let fd = data.fd().as_fd(); let prog_fd = program.as_fd(); let prog_fd = prog_fd.as_raw_fd(); @@ -94,7 +94,7 @@ impl> ProgramArray { pub fn clear_index(&mut self, index: &u32) -> Result<(), MapError> { let data = self.inner.borrow_mut(); check_bounds(data, *index)?; - let fd = self.inner.borrow_mut().fd; + let fd = data.fd().as_fd(); bpf_map_delete_elem(fd, index) .map(|_| ()) diff --git a/aya/src/maps/bloom_filter.rs b/aya/src/maps/bloom_filter.rs index 0396d0f6..dfe4d2cb 100644 --- a/aya/src/maps/bloom_filter.rs +++ b/aya/src/maps/bloom_filter.rs @@ -2,6 +2,7 @@ use std::{ borrow::{Borrow, BorrowMut}, marker::PhantomData, + os::fd::AsFd as _, }; use crate::{ @@ -52,7 +53,7 @@ impl, V: Pod> BloomFilter { /// Query the existence of the element. pub fn contains(&self, mut value: &V, flags: u64) -> Result<(), MapError> { - let fd = self.inner.borrow().fd; + let fd = self.inner.borrow().fd().as_fd(); bpf_map_lookup_elem_ptr::(fd, None, &mut value, flags) .map_err(|(_, io_error)| SyscallError { @@ -67,7 +68,7 @@ impl, V: Pod> BloomFilter { impl, V: Pod> BloomFilter { /// Inserts a value into the map. pub fn insert(&mut self, value: impl Borrow, flags: u64) -> Result<(), MapError> { - let fd = self.inner.borrow_mut().fd; + let fd = self.inner.borrow_mut().fd().as_fd(); bpf_map_push_elem(fd, value.borrow(), flags).map_err(|(_, io_error)| SyscallError { call: "bpf_map_push_elem", io_error, diff --git a/aya/src/maps/hash_map/hash_map.rs b/aya/src/maps/hash_map/hash_map.rs index 8d091582..9d587b33 100644 --- a/aya/src/maps/hash_map/hash_map.rs +++ b/aya/src/maps/hash_map/hash_map.rs @@ -1,6 +1,7 @@ use std::{ borrow::{Borrow, BorrowMut}, marker::PhantomData, + os::fd::AsFd as _, }; use crate::{ @@ -52,7 +53,7 @@ impl, K: Pod, V: Pod> HashMap { /// Returns a copy of the value associated with the key. pub fn get(&self, key: &K, flags: u64) -> Result { - let fd = self.inner.borrow().fd; + let fd = self.inner.borrow().fd().as_fd(); let value = bpf_map_lookup_elem(fd, key, flags).map_err(|(_, io_error)| SyscallError { call: "bpf_map_lookup_elem", io_error, diff --git a/aya/src/maps/hash_map/mod.rs b/aya/src/maps/hash_map/mod.rs index 5e18cb19..40c171ce 100644 --- a/aya/src/maps/hash_map/mod.rs +++ b/aya/src/maps/hash_map/mod.rs @@ -4,6 +4,7 @@ use crate::{ sys::{bpf_map_delete_elem, bpf_map_update_elem, SyscallError}, Pod, }; +use std::os::fd::AsFd as _; #[allow(clippy::module_inception)] mod hash_map; @@ -20,7 +21,7 @@ pub(crate) fn insert( value: &V, flags: u64, ) -> Result<(), MapError> { - let fd = map.fd; + let fd = map.fd().as_fd(); bpf_map_update_elem(fd, Some(key), value, flags).map_err(|(_, io_error)| SyscallError { call: "bpf_map_update_elem", io_error, @@ -30,7 +31,7 @@ pub(crate) fn insert( } pub(crate) fn remove(map: &MapData, key: &K) -> Result<(), MapError> { - let fd = map.fd; + let fd = map.fd().as_fd(); bpf_map_delete_elem(fd, key) .map(|_| ()) .map_err(|(_, io_error)| { diff --git a/aya/src/maps/hash_map/per_cpu_hash_map.rs b/aya/src/maps/hash_map/per_cpu_hash_map.rs index aabc71f6..56fc2ae2 100644 --- a/aya/src/maps/hash_map/per_cpu_hash_map.rs +++ b/aya/src/maps/hash_map/per_cpu_hash_map.rs @@ -2,6 +2,7 @@ use std::{ borrow::{Borrow, BorrowMut}, marker::PhantomData, + os::fd::AsFd as _, }; use crate::{ @@ -61,7 +62,7 @@ impl, K: Pod, V: Pod> PerCpuHashMap { /// Returns a slice of values - one for each CPU - associated with the key. pub fn get(&self, key: &K, flags: u64) -> Result, MapError> { - let fd = self.inner.borrow().fd; + let fd = self.inner.borrow().fd().as_fd(); let values = bpf_map_lookup_elem_per_cpu(fd, key, flags).map_err(|(_, io_error)| SyscallError { call: "bpf_map_lookup_elem", @@ -118,7 +119,7 @@ impl, K: Pod, V: Pod> PerCpuHashMap { values: PerCpuValues, flags: u64, ) -> Result<(), MapError> { - let fd = self.inner.borrow_mut().fd; + let fd = self.inner.borrow_mut().fd().as_fd(); bpf_map_update_elem_per_cpu(fd, key.borrow(), &values, flags).map_err( |(_, io_error)| SyscallError { call: "bpf_map_update_elem", diff --git a/aya/src/maps/lpm_trie.rs b/aya/src/maps/lpm_trie.rs index dcdc443d..d4e1e678 100644 --- a/aya/src/maps/lpm_trie.rs +++ b/aya/src/maps/lpm_trie.rs @@ -2,6 +2,7 @@ use std::{ borrow::{Borrow, BorrowMut}, marker::PhantomData, + os::fd::AsFd as _, }; use crate::{ @@ -126,7 +127,7 @@ impl, K: Pod, V: Pod> LpmTrie { /// Returns a copy of the value associated with the longest prefix matching key in the LpmTrie. pub fn get(&self, key: &Key, flags: u64) -> Result { - let fd = self.inner.borrow().fd; + let fd = self.inner.borrow().fd().as_fd(); let value = bpf_map_lookup_elem(fd, key, flags).map_err(|(_, io_error)| SyscallError { call: "bpf_map_lookup_elem", io_error, @@ -155,7 +156,7 @@ impl, K: Pod, V: Pod> LpmTrie { value: impl Borrow, flags: u64, ) -> Result<(), MapError> { - let fd = self.inner.borrow().fd; + let fd = self.inner.borrow().fd().as_fd(); bpf_map_update_elem(fd, Some(key), value.borrow(), flags).map_err(|(_, io_error)| { SyscallError { call: "bpf_map_update_elem", @@ -170,7 +171,7 @@ impl, K: Pod, V: Pod> LpmTrie { /// /// Both the prefix and data must match exactly - this method does not do a longest prefix match. pub fn remove(&mut self, key: &Key) -> Result<(), MapError> { - let fd = self.inner.borrow().fd; + let fd = self.inner.borrow().fd().as_fd(); bpf_map_delete_elem(fd, key) .map(|_| ()) .map_err(|(_, io_error)| { diff --git a/aya/src/maps/mod.rs b/aya/src/maps/mod.rs index 4d8183e9..85ba5d94 100644 --- a/aya/src/maps/mod.rs +++ b/aya/src/maps/mod.rs @@ -18,17 +18,28 @@ //! the [`TryFrom`] or [`TryInto`] trait. For example: //! //! ```no_run +//! # #[derive(Debug, thiserror::Error)] +//! # enum Error { +//! # #[error(transparent)] +//! # IO(#[from] std::io::Error), +//! # #[error(transparent)] +//! # Map(#[from] aya::maps::MapError), +//! # #[error(transparent)] +//! # Program(#[from] aya::programs::ProgramError), +//! # #[error(transparent)] +//! # Bpf(#[from] aya::BpfError) +//! # } //! # let mut bpf = aya::Bpf::load(&[])?; //! use aya::maps::SockMap; //! use aya::programs::SkMsg; //! //! let intercept_egress = SockMap::try_from(bpf.map_mut("INTERCEPT_EGRESS").unwrap())?; -//! let map_fd = intercept_egress.fd()?; +//! let map_fd = intercept_egress.fd().try_clone()?; //! let prog: &mut SkMsg = bpf.program_mut("intercept_egress_packet").unwrap().try_into()?; //! prog.load()?; -//! prog.attach(map_fd)?; +//! prog.attach(&map_fd)?; //! -//! # Ok::<(), aya::BpfError>(()) +//! # Ok::<(), Error>(()) //! ``` //! //! # Maps and `Pod` values @@ -42,7 +53,7 @@ use std::{ marker::PhantomData, mem, ops::Deref, - os::fd::{AsFd as _, AsRawFd, BorrowedFd, IntoRawFd as _, OwnedFd, RawFd}, + os::fd::{AsFd, BorrowedFd, OwnedFd}, path::Path, ptr, }; @@ -177,11 +188,13 @@ pub enum MapError { } /// A map file descriptor. -pub struct MapFd(RawFd); +#[derive(Debug)] +pub struct MapFd(OwnedFd); -impl AsRawFd for MapFd { - fn as_raw_fd(&self) -> RawFd { - self.0 +impl AsFd for MapFd { + fn as_fd(&self) -> BorrowedFd<'_> { + let Self(fd) = self; + fd.as_fd() } } @@ -396,7 +409,7 @@ pub(crate) fn check_v_size(map: &MapData) -> Result<(), MapError> { #[derive(Debug)] pub struct MapData { pub(crate) obj: obj::Map, - pub(crate) fd: RawFd, + pub(crate) fd: MapFd, /// Indicates if this map has been pinned to bpffs pub pinned: bool, } @@ -426,9 +439,7 @@ impl MapData { io_error, } })?; - - #[allow(trivial_numeric_casts)] - let fd = fd as RawFd; + let fd = MapFd(fd); Ok(Self { obj, fd, @@ -459,11 +470,14 @@ impl MapData { call: "BPF_OBJ_GET", io_error, }) { - Ok(fd) => Ok(Self { - obj, - fd: fd.into_raw_fd(), - pinned: false, - }), + Ok(fd) => { + let fd = MapFd(fd); + Ok(Self { + obj, + fd, + pinned: false, + }) + } Err(_) => { let mut map = Self::create(obj, name, btf_fd)?; map.pin(name, path).map_err(|error| MapError::PinError { @@ -493,12 +507,13 @@ impl MapData { call: "BPF_OBJ_GET", io_error, })?; + let fd = MapFd(fd); let info = bpf_map_get_info_by_fd(fd.as_fd())?; Ok(Self { obj: parse_map_info(info, PinningType::ByName), - fd: fd.into_raw_fd(), + fd, pinned: true, }) } @@ -511,9 +526,10 @@ impl MapData { pub fn from_fd(fd: OwnedFd) -> Result { let info = bpf_map_get_info_by_fd(fd.as_fd())?; + let fd = MapFd(fd); Ok(Self { obj: parse_map_info(info, PinningType::None), - fd: fd.into_raw_fd(), + fd, pinned: false, }) } @@ -528,7 +544,7 @@ impl MapData { let path = path.as_ref().join(name); let path_string = CString::new(path.as_os_str().as_bytes()) .map_err(|error| PinError::InvalidPinPath { path, error })?; - bpf_pin_object(*fd, &path_string).map_err(|(_, io_error)| SyscallError { + bpf_pin_object(fd.as_fd(), &path_string).map_err(|(_, io_error)| SyscallError { call: "BPF_OBJ_PIN", io_error, })?; @@ -537,30 +553,13 @@ impl MapData { } /// Returns the file descriptor of the map. - /// - /// Can be converted to [`RawFd`] using [`AsRawFd`]. - pub fn fd(&self) -> MapFd { - MapFd(self.fd) - } -} - -impl Drop for MapData { - fn drop(&mut self) { - // TODO: Replace this with an OwnedFd once that is stabilized. - // - // SAFETY: `drop` is only called once. - unsafe { libc::close(self.fd) }; - } -} - -impl Clone for MapData { - fn clone(&self) -> Self { - let Self { obj, fd, pinned } = self; - Self { - obj: obj.clone(), - fd: unsafe { libc::dup(*fd) }, - pinned: *pinned, - } + pub fn fd(&self) -> &MapFd { + let Self { + obj: _, + fd, + pinned: _, + } = self; + fd } } @@ -598,7 +597,7 @@ impl Iterator for MapKeys<'_, K> { return None; } - let fd = self.map.fd; + let fd = self.map.fd().as_fd(); let key = bpf_map_get_next_key(fd, self.key.as_ref()).map_err(|(_, io_error)| SyscallError { call: "bpf_map_get_next_key", @@ -754,6 +753,7 @@ impl Deref for PerCpuValues { mod tests { use assert_matches::assert_matches; use libc::EFAULT; + use std::os::fd::AsRawFd as _; use crate::{ bpf_map_def, @@ -795,9 +795,9 @@ mod tests { MapData::create(new_obj_map(), "foo", None), Ok(MapData { obj: _, - fd: 42, + fd, pinned: false - }) + }) => assert_eq!(fd.as_fd().as_raw_fd(), 42) ); } diff --git a/aya/src/maps/perf/perf_event_array.rs b/aya/src/maps/perf/perf_event_array.rs index 1d19d373..ebeef672 100644 --- a/aya/src/maps/perf/perf_event_array.rs +++ b/aya/src/maps/perf/perf_event_array.rs @@ -4,7 +4,7 @@ use std::{ borrow::{Borrow, BorrowMut}, ops::Deref, - os::fd::{AsRawFd, RawFd}, + os::fd::{AsFd as _, AsRawFd, RawFd}, sync::Arc, }; @@ -181,7 +181,7 @@ impl> PerfEventArray { // FIXME: keep track of open buffers let map_data: &MapData = self.map.deref().borrow(); - let map_fd = map_data.fd; + let map_fd = map_data.fd().as_fd(); let buf = PerfBuffer::open(index, self.page_size, page_count.unwrap_or(2))?; bpf_map_update_elem(map_fd, Some(&index), &buf.as_raw_fd(), 0) .map_err(|(_, io_error)| io_error)?; diff --git a/aya/src/maps/queue.rs b/aya/src/maps/queue.rs index 6eebbf01..31c44633 100644 --- a/aya/src/maps/queue.rs +++ b/aya/src/maps/queue.rs @@ -2,6 +2,7 @@ use std::{ borrow::{Borrow, BorrowMut}, marker::PhantomData, + os::fd::AsFd as _, }; use crate::{ @@ -60,7 +61,7 @@ impl, V: Pod> Queue { /// Returns [`MapError::ElementNotFound`] if the queue is empty, [`MapError::SyscallError`] /// if `bpf_map_lookup_and_delete_elem` fails. pub fn pop(&mut self, flags: u64) -> Result { - let fd = self.inner.borrow().fd; + let fd = self.inner.borrow().fd().as_fd(); let value = bpf_map_lookup_and_delete_elem::(fd, None, flags).map_err( |(_, io_error)| SyscallError { @@ -77,7 +78,7 @@ impl, V: Pod> Queue { /// /// [`MapError::SyscallError`] if `bpf_map_update_elem` fails. pub fn push(&mut self, value: impl Borrow, flags: u64) -> Result<(), MapError> { - let fd = self.inner.borrow().fd; + let fd = self.inner.borrow().fd().as_fd(); bpf_map_push_elem(fd, value.borrow(), flags).map_err(|(_, io_error)| SyscallError { call: "bpf_map_push_elem", io_error, diff --git a/aya/src/maps/sock/mod.rs b/aya/src/maps/sock/mod.rs index 05fe112c..0c3262a2 100644 --- a/aya/src/maps/sock/mod.rs +++ b/aya/src/maps/sock/mod.rs @@ -5,17 +5,29 @@ mod sock_map; pub use sock_hash::SockHash; pub use sock_map::SockMap; -use std::os::fd::{AsFd, BorrowedFd, RawFd}; +use std::{ + io, + os::fd::{AsFd, BorrowedFd}, +}; /// A socket map file descriptor. -#[derive(Copy, Clone)] -pub struct SockMapFd(RawFd); +#[repr(transparent)] +pub struct SockMapFd(super::MapFd); + +impl SockMapFd { + /// Creates a new instance that shares the same underlying file description as [`self`]. + pub fn try_clone(&self) -> io::Result { + let Self(inner) = self; + let super::MapFd(inner) = inner; + let inner = inner.try_clone()?; + let inner = super::MapFd(inner); + Ok(Self(inner)) + } +} impl AsFd for SockMapFd { fn as_fd(&self) -> BorrowedFd<'_> { - // SAFETY: This isn't necessarily safe, we need to find ways - // to enforce that the file descriptor is still - // valid. TODO(#612) - unsafe { BorrowedFd::borrow_raw(self.0) } + let Self(fd) = self; + fd.as_fd() } } diff --git a/aya/src/maps/sock/sock_hash.rs b/aya/src/maps/sock/sock_hash.rs index 503dcaae..99d0f871 100644 --- a/aya/src/maps/sock/sock_hash.rs +++ b/aya/src/maps/sock/sock_hash.rs @@ -1,12 +1,13 @@ use std::{ borrow::{Borrow, BorrowMut}, marker::PhantomData, - os::fd::{AsRawFd, RawFd}, + os::fd::{AsFd as _, AsRawFd, RawFd}, }; use crate::{ maps::{ - check_kv_size, hash_map, sock::SockMapFd, IterableMap, MapData, MapError, MapIter, MapKeys, + check_kv_size, hash_map, sock::SockMapFd, IterableMap, MapData, MapError, MapFd, MapIter, + MapKeys, }, sys::{bpf_map_lookup_elem, SyscallError}, Pod, @@ -47,11 +48,11 @@ use crate::{ /// use aya::programs::SkMsg; /// /// let mut intercept_egress = SockHash::<_, u32>::try_from(bpf.map("INTERCEPT_EGRESS").unwrap())?; -/// let map_fd = intercept_egress.fd()?; +/// let map_fd = intercept_egress.fd().try_clone()?; /// /// let prog: &mut SkMsg = bpf.program_mut("intercept_egress_packet").unwrap().try_into()?; /// prog.load()?; -/// prog.attach(map_fd)?; +/// prog.attach(&map_fd)?; /// /// let mut client = TcpStream::connect("127.0.0.1:1234")?; /// let mut intercept_egress = SockHash::try_from(bpf.map_mut("INTERCEPT_EGRESS").unwrap())?; @@ -81,7 +82,7 @@ impl, K: Pod> SockHash { /// Returns the fd of the socket stored at the given key. pub fn get(&self, key: &K, flags: u64) -> Result { - let fd = self.inner.borrow().fd; + let fd = self.inner.borrow().fd().as_fd(); let value = bpf_map_lookup_elem(fd, key, flags).map_err(|(_, io_error)| SyscallError { call: "bpf_map_lookup_elem", io_error, @@ -105,8 +106,11 @@ impl, K: Pod> SockHash { /// /// The returned file descriptor can be used to attach programs that work with /// socket maps, like [`SkMsg`](crate::programs::SkMsg) and [`SkSkb`](crate::programs::SkSkb). - pub fn fd(&self) -> Result { - Ok(SockMapFd(self.inner.borrow().fd)) + pub fn fd(&self) -> &SockMapFd { + let fd: &MapFd = self.inner.borrow().fd(); + // TODO(https://github.com/rust-lang/rfcs/issues/3066): avoid this unsafe. + // SAFETY: `SockMapFd` is #[repr(transparent)] over `MapFd`. + unsafe { std::mem::transmute(fd) } } } diff --git a/aya/src/maps/sock/sock_map.rs b/aya/src/maps/sock/sock_map.rs index 8194cd71..42246c8b 100644 --- a/aya/src/maps/sock/sock_map.rs +++ b/aya/src/maps/sock/sock_map.rs @@ -2,11 +2,11 @@ use std::{ borrow::{Borrow, BorrowMut}, - os::fd::{AsRawFd, RawFd}, + os::fd::{AsFd as _, AsRawFd, RawFd}, }; use crate::{ - maps::{check_bounds, check_kv_size, sock::SockMapFd, MapData, MapError, MapKeys}, + maps::{check_bounds, check_kv_size, sock::SockMapFd, MapData, MapError, MapFd, MapKeys}, sys::{bpf_map_delete_elem, bpf_map_update_elem, SyscallError}, }; @@ -26,18 +26,29 @@ use crate::{ /// # Examples /// /// ```no_run +/// # #[derive(Debug, thiserror::Error)] +/// # enum Error { +/// # #[error(transparent)] +/// # IO(#[from] std::io::Error), +/// # #[error(transparent)] +/// # Map(#[from] aya::maps::MapError), +/// # #[error(transparent)] +/// # Program(#[from] aya::programs::ProgramError), +/// # #[error(transparent)] +/// # Bpf(#[from] aya::BpfError) +/// # } /// # let mut bpf = aya::Bpf::load(&[])?; /// use aya::maps::SockMap; /// use aya::programs::SkSkb; /// /// let intercept_ingress = SockMap::try_from(bpf.map("INTERCEPT_INGRESS").unwrap())?; -/// let map_fd = intercept_ingress.fd()?; +/// let map_fd = intercept_ingress.fd().try_clone()?; /// /// let prog: &mut SkSkb = bpf.program_mut("intercept_ingress_packet").unwrap().try_into()?; /// prog.load()?; -/// prog.attach(map_fd)?; +/// prog.attach(&map_fd)?; /// -/// # Ok::<(), aya::BpfError>(()) +/// # Ok::<(), Error>(()) /// ``` #[doc(alias = "BPF_MAP_TYPE_SOCKMAP")] pub struct SockMap { @@ -62,8 +73,11 @@ impl> SockMap { /// /// The returned file descriptor can be used to attach programs that work with /// socket maps, like [`SkMsg`](crate::programs::SkMsg) and [`SkSkb`](crate::programs::SkSkb). - pub fn fd(&self) -> Result { - Ok(SockMapFd(self.inner.borrow().fd)) + pub fn fd(&self) -> &SockMapFd { + let fd: &MapFd = self.inner.borrow().fd(); + // TODO(https://github.com/rust-lang/rfcs/issues/3066): avoid this unsafe. + // SAFETY: `SockMapFd` is #[repr(transparent)] over `MapFd`. + unsafe { std::mem::transmute(&fd) } } } @@ -71,7 +85,7 @@ impl> SockMap { /// Stores a socket into the map. pub fn set(&mut self, index: u32, socket: &I, flags: u64) -> Result<(), MapError> { let data = self.inner.borrow_mut(); - let fd = data.fd; + let fd = data.fd().as_fd(); check_bounds(data, index)?; bpf_map_update_elem(fd, Some(&index), &socket.as_raw_fd(), flags).map_err( |(_, io_error)| SyscallError { @@ -85,7 +99,7 @@ impl> SockMap { /// Removes the socket stored at `index` from the map. pub fn clear_index(&mut self, index: &u32) -> Result<(), MapError> { let data = self.inner.borrow_mut(); - let fd = data.fd; + let fd = data.fd().as_fd(); check_bounds(data, *index)?; bpf_map_delete_elem(fd, index) .map(|_| ()) diff --git a/aya/src/maps/stack.rs b/aya/src/maps/stack.rs index c4afeebb..ac1453cd 100644 --- a/aya/src/maps/stack.rs +++ b/aya/src/maps/stack.rs @@ -2,6 +2,7 @@ use std::{ borrow::{Borrow, BorrowMut}, marker::PhantomData, + os::fd::AsFd as _, }; use crate::{ @@ -60,7 +61,7 @@ impl, V: Pod> Stack { /// Returns [`MapError::ElementNotFound`] if the stack is empty, [`MapError::SyscallError`] /// if `bpf_map_lookup_and_delete_elem` fails. pub fn pop(&mut self, flags: u64) -> Result { - let fd = self.inner.borrow().fd; + let fd = self.inner.borrow().fd().as_fd(); let value = bpf_map_lookup_and_delete_elem::(fd, None, flags).map_err( |(_, io_error)| SyscallError { @@ -77,7 +78,7 @@ impl, V: Pod> Stack { /// /// [`MapError::SyscallError`] if `bpf_map_update_elem` fails. pub fn push(&mut self, value: impl Borrow, flags: u64) -> Result<(), MapError> { - let fd = self.inner.borrow().fd; + let fd = self.inner.borrow().fd().as_fd(); bpf_map_update_elem(fd, None::<&u32>, value.borrow(), flags).map_err(|(_, io_error)| { SyscallError { call: "bpf_map_update_elem", diff --git a/aya/src/maps/stack_trace.rs b/aya/src/maps/stack_trace.rs index 07281a2b..fdf1d937 100644 --- a/aya/src/maps/stack_trace.rs +++ b/aya/src/maps/stack_trace.rs @@ -1,7 +1,7 @@ //! A hash map of kernel or user space stack traces. //! //! See [`StackTraceMap`] for documentation and examples. -use std::{borrow::Borrow, fs, io, mem, path::Path, str::FromStr}; +use std::{borrow::Borrow, fs, io, mem, os::fd::AsFd as _, path::Path, str::FromStr}; use crate::{ maps::{IterableMap, MapData, MapError, MapIter, MapKeys}, @@ -103,7 +103,7 @@ impl> StackTraceMap { /// Returns [`MapError::KeyNotFound`] if there is no stack trace with the /// given `stack_id`, or [`MapError::SyscallError`] if `bpf_map_lookup_elem` fails. pub fn get(&self, stack_id: &u32, flags: u64) -> Result { - let fd = self.inner.borrow().fd; + let fd = self.inner.borrow().fd().as_fd(); let mut frames = vec![0; self.max_stack_depth]; bpf_map_lookup_elem_ptr(fd, Some(stack_id), frames.as_mut_ptr(), flags) diff --git a/aya/src/programs/links.rs b/aya/src/programs/links.rs index d06a62e4..e8eeb417 100644 --- a/aya/src/programs/links.rs +++ b/aya/src/programs/links.rs @@ -154,11 +154,9 @@ impl FdLink { error, } })?; - bpf_pin_object(self.fd.as_raw_fd(), &path_string).map_err(|(_, io_error)| { - SyscallError { - call: "BPF_OBJ_PIN", - io_error, - } + bpf_pin_object(self.fd.as_fd(), &path_string).map_err(|(_, io_error)| SyscallError { + call: "BPF_OBJ_PIN", + io_error, })?; Ok(PinnedLink::new(path.into(), self)) } diff --git a/aya/src/programs/mod.rs b/aya/src/programs/mod.rs index 616244e0..e41bdeed 100644 --- a/aya/src/programs/mod.rs +++ b/aya/src/programs/mod.rs @@ -218,9 +218,8 @@ pub enum ProgramError { pub struct ProgramFd(OwnedFd); impl ProgramFd { - /// Creates a new `ProgramFd` instance that shares the same underlying file - /// description as the existing `ProgramFd` instance. - pub fn try_clone(&self) -> Result { + /// Creates a new instance that shares the same underlying file description as [`self`]. + pub fn try_clone(&self) -> io::Result { let Self(inner) = self; let inner = inner.try_clone()?; Ok(Self(inner)) @@ -536,7 +535,7 @@ fn pin_program>(data: &ProgramData, path: P) -> Resul path: path.into(), error, })?; - bpf_pin_object(fd.as_fd().as_raw_fd(), &path_string).map_err(|(_, io_error)| SyscallError { + bpf_pin_object(fd.as_fd(), &path_string).map_err(|(_, io_error)| SyscallError { call: "BPF_OBJ_PIN", io_error, })?; diff --git a/aya/src/programs/sk_msg.rs b/aya/src/programs/sk_msg.rs index 554c3e86..f169447b 100644 --- a/aya/src/programs/sk_msg.rs +++ b/aya/src/programs/sk_msg.rs @@ -43,11 +43,11 @@ use crate::{ /// use aya::programs::SkMsg; /// /// let intercept_egress: SockHash<_, u32> = bpf.map("INTERCEPT_EGRESS").unwrap().try_into()?; -/// let map_fd = intercept_egress.fd()?; +/// let map_fd = intercept_egress.fd().try_clone()?; /// /// let prog: &mut SkMsg = bpf.program_mut("intercept_egress_packet").unwrap().try_into()?; /// prog.load()?; -/// prog.attach(map_fd)?; +/// prog.attach(&map_fd)?; /// /// let mut client = TcpStream::connect("127.0.0.1:1234")?; /// let mut intercept_egress: SockHash<_, u32> = bpf.map_mut("INTERCEPT_EGRESS").unwrap().try_into()?; @@ -77,7 +77,7 @@ impl SkMsg { /// Attaches the program to the given sockmap. /// /// The returned value can be used to detach, see [SkMsg::detach]. - pub fn attach(&mut self, map: SockMapFd) -> Result { + pub fn attach(&mut self, map: &SockMapFd) -> Result { let prog_fd = self.fd()?; let prog_fd = prog_fd.as_fd(); let link = ProgAttachLink::attach(prog_fd, map.as_fd(), BPF_SK_MSG_VERDICT)?; diff --git a/aya/src/programs/sk_skb.rs b/aya/src/programs/sk_skb.rs index dac5ba3f..81bc9534 100644 --- a/aya/src/programs/sk_skb.rs +++ b/aya/src/programs/sk_skb.rs @@ -37,18 +37,29 @@ pub enum SkSkbKind { /// # Examples /// /// ```no_run +/// # #[derive(Debug, thiserror::Error)] +/// # enum Error { +/// # #[error(transparent)] +/// # IO(#[from] std::io::Error), +/// # #[error(transparent)] +/// # Map(#[from] aya::maps::MapError), +/// # #[error(transparent)] +/// # Program(#[from] aya::programs::ProgramError), +/// # #[error(transparent)] +/// # Bpf(#[from] aya::BpfError) +/// # } /// # let mut bpf = aya::Bpf::load(&[])?; /// use aya::maps::SockMap; /// use aya::programs::SkSkb; /// /// let intercept_ingress: SockMap<_> = bpf.map("INTERCEPT_INGRESS").unwrap().try_into()?; -/// let map_fd = intercept_ingress.fd()?; +/// let map_fd = intercept_ingress.fd().try_clone()?; /// /// let prog: &mut SkSkb = bpf.program_mut("intercept_ingress_packet").unwrap().try_into()?; /// prog.load()?; -/// prog.attach(map_fd)?; +/// prog.attach(&map_fd)?; /// -/// # Ok::<(), aya::BpfError>(()) +/// # Ok::<(), Error>(()) /// ``` /// /// [socket maps]: crate::maps::sock @@ -70,7 +81,7 @@ impl SkSkb { /// Attaches the program to the given socket map. /// /// The returned value can be used to detach, see [SkSkb::detach]. - pub fn attach(&mut self, map: SockMapFd) -> Result { + pub fn attach(&mut self, map: &SockMapFd) -> Result { let prog_fd = self.fd()?; let prog_fd = prog_fd.as_fd(); diff --git a/aya/src/programs/utils.rs b/aya/src/programs/utils.rs index 0f8f61ae..a32d9f93 100644 --- a/aya/src/programs/utils.rs +++ b/aya/src/programs/utils.rs @@ -82,7 +82,7 @@ pub(crate) fn get_fdinfo(fd: BorrowedFd<'_>, key: &str) -> Result>, kernel_version: KernelVersion, -) -> SysResult { +) -> SysResult { let mut attr = unsafe { mem::zeroed::() }; let u = unsafe { &mut attr.__bindgen_anon_1 }; @@ -91,13 +91,14 @@ pub(crate) fn bpf_create_map( .copy_from_slice(unsafe { slice::from_raw_parts(name.as_ptr(), name_len) }); } - sys_bpf(bpf_cmd::BPF_MAP_CREATE, &mut attr) + // SAFETY: BPF_MAP_CREATE returns a new file descriptor. + unsafe { fd_sys_bpf(bpf_cmd::BPF_MAP_CREATE, &mut attr) } } -pub(crate) fn bpf_pin_object(fd: RawFd, path: &CStr) -> SysResult { +pub(crate) fn bpf_pin_object(fd: BorrowedFd<'_>, path: &CStr) -> SysResult { let mut attr = unsafe { mem::zeroed::() }; let u = unsafe { &mut attr.__bindgen_anon_4 }; - u.bpf_fd = fd as u32; + u.bpf_fd = fd.as_raw_fd() as u32; u.pathname = path.as_ptr() as u64; sys_bpf(bpf_cmd::BPF_OBJ_PIN, &mut attr) } @@ -194,7 +195,7 @@ pub(crate) fn bpf_load_program( } fn lookup( - fd: RawFd, + fd: BorrowedFd<'_>, key: Option<&K>, flags: u64, cmd: bpf_cmd, @@ -203,7 +204,7 @@ fn lookup( let mut value = MaybeUninit::zeroed(); let u = unsafe { &mut attr.__bindgen_anon_2 }; - u.map_fd = fd as u32; + u.map_fd = fd.as_raw_fd() as u32; if let Some(key) = key { u.key = key as *const _ as u64; } @@ -218,7 +219,7 @@ fn lookup( } pub(crate) fn bpf_map_lookup_elem( - fd: RawFd, + fd: BorrowedFd<'_>, key: &K, flags: u64, ) -> Result, (c_long, io::Error)> { @@ -226,7 +227,7 @@ pub(crate) fn bpf_map_lookup_elem( } pub(crate) fn bpf_map_lookup_and_delete_elem( - fd: RawFd, + fd: BorrowedFd<'_>, key: Option<&K>, flags: u64, ) -> Result, (c_long, io::Error)> { @@ -234,7 +235,7 @@ pub(crate) fn bpf_map_lookup_and_delete_elem( } pub(crate) fn bpf_map_lookup_elem_per_cpu( - fd: RawFd, + fd: BorrowedFd<'_>, key: &K, flags: u64, ) -> Result>, (c_long, io::Error)> { @@ -247,7 +248,7 @@ pub(crate) fn bpf_map_lookup_elem_per_cpu( } pub(crate) fn bpf_map_lookup_elem_ptr( - fd: RawFd, + fd: BorrowedFd<'_>, key: Option<&K>, value: *mut V, flags: u64, @@ -255,7 +256,7 @@ pub(crate) fn bpf_map_lookup_elem_ptr( let mut attr = unsafe { mem::zeroed::() }; let u = unsafe { &mut attr.__bindgen_anon_2 }; - u.map_fd = fd as u32; + u.map_fd = fd.as_raw_fd() as u32; if let Some(key) = key { u.key = key as *const _ as u64; } @@ -270,7 +271,7 @@ pub(crate) fn bpf_map_lookup_elem_ptr( } pub(crate) fn bpf_map_update_elem( - fd: RawFd, + fd: BorrowedFd<'_>, key: Option<&K>, value: &V, flags: u64, @@ -278,7 +279,7 @@ pub(crate) fn bpf_map_update_elem( let mut attr = unsafe { mem::zeroed::() }; let u = unsafe { &mut attr.__bindgen_anon_2 }; - u.map_fd = fd as u32; + u.map_fd = fd.as_raw_fd() as u32; if let Some(key) = key { u.key = key as *const _ as u64; } @@ -288,11 +289,15 @@ pub(crate) fn bpf_map_update_elem( sys_bpf(bpf_cmd::BPF_MAP_UPDATE_ELEM, &mut attr) } -pub(crate) fn bpf_map_push_elem(fd: RawFd, value: &V, flags: u64) -> SysResult { +pub(crate) fn bpf_map_push_elem( + fd: BorrowedFd<'_>, + value: &V, + flags: u64, +) -> SysResult { let mut attr = unsafe { mem::zeroed::() }; let u = unsafe { &mut attr.__bindgen_anon_2 }; - u.map_fd = fd as u32; + u.map_fd = fd.as_raw_fd() as u32; u.__bindgen_anon_1.value = value as *const _ as u64; u.flags = flags; @@ -300,7 +305,7 @@ pub(crate) fn bpf_map_push_elem(fd: RawFd, value: &V, flags: u64) -> Sys } pub(crate) fn bpf_map_update_elem_ptr( - fd: RawFd, + fd: BorrowedFd<'_>, key: *const K, value: *mut V, flags: u64, @@ -308,7 +313,7 @@ pub(crate) fn bpf_map_update_elem_ptr( let mut attr = unsafe { mem::zeroed::() }; let u = unsafe { &mut attr.__bindgen_anon_2 }; - u.map_fd = fd as u32; + u.map_fd = fd.as_raw_fd() as u32; u.key = key as u64; u.__bindgen_anon_1.value = value as u64; u.flags = flags; @@ -317,7 +322,7 @@ pub(crate) fn bpf_map_update_elem_ptr( } pub(crate) fn bpf_map_update_elem_per_cpu( - fd: RawFd, + fd: BorrowedFd<'_>, key: &K, values: &PerCpuValues, flags: u64, @@ -326,25 +331,25 @@ pub(crate) fn bpf_map_update_elem_per_cpu( bpf_map_update_elem_ptr(fd, key, mem.as_mut_ptr(), flags) } -pub(crate) fn bpf_map_delete_elem(fd: RawFd, key: &K) -> SysResult { +pub(crate) fn bpf_map_delete_elem(fd: BorrowedFd<'_>, key: &K) -> SysResult { let mut attr = unsafe { mem::zeroed::() }; let u = unsafe { &mut attr.__bindgen_anon_2 }; - u.map_fd = fd as u32; + u.map_fd = fd.as_raw_fd() as u32; u.key = key as *const _ as u64; sys_bpf(bpf_cmd::BPF_MAP_DELETE_ELEM, &mut attr) } pub(crate) fn bpf_map_get_next_key( - fd: RawFd, + fd: BorrowedFd<'_>, key: Option<&K>, ) -> Result, (c_long, io::Error)> { let mut attr = unsafe { mem::zeroed::() }; let mut next_key = MaybeUninit::uninit(); let u = unsafe { &mut attr.__bindgen_anon_2 }; - u.map_fd = fd as u32; + u.map_fd = fd.as_raw_fd() as u32; if let Some(key) = key { u.key = key as *const _ as u64; } @@ -358,10 +363,10 @@ pub(crate) fn bpf_map_get_next_key( } // since kernel 5.2 -pub(crate) fn bpf_map_freeze(fd: RawFd) -> SysResult { +pub(crate) fn bpf_map_freeze(fd: BorrowedFd<'_>) -> SysResult { let mut attr = unsafe { mem::zeroed::() }; let u = unsafe { &mut attr.__bindgen_anon_2 }; - u.map_fd = fd as u32; + u.map_fd = fd.as_raw_fd() as u32; sys_bpf(bpf_cmd::BPF_MAP_FREEZE, &mut attr) } @@ -754,7 +759,7 @@ pub(crate) fn is_bpf_global_data_supported() -> bool { ); if let Ok(map) = map { - insns[0].imm = map.fd; + insns[0].imm = map.fd().as_fd().as_raw_fd(); let gpl = b"GPL\0"; u.license = gpl.as_ptr() as u64; diff --git a/test/integration-test/src/tests/rbpf.rs b/test/integration-test/src/tests/rbpf.rs index da44863f..9caab94c 100644 --- a/test/integration-test/src/tests/rbpf.rs +++ b/test/integration-test/src/tests/rbpf.rs @@ -58,7 +58,7 @@ fn use_map_with_rbpf() { ); let map_id = if name == "map_1" { 0 } else { 1 }; - let fd = map_id as i32 | 0xCAFE00; + let fd = map_id as std::os::fd::RawFd | 0xCAFE00; maps.insert(name.to_owned(), (fd, map.clone())); unsafe { diff --git a/xtask/public-api/aya-obj.txt b/xtask/public-api/aya-obj.txt index 574862f4..c6dea1ba 100644 --- a/xtask/public-api/aya-obj.txt +++ b/xtask/public-api/aya-obj.txt @@ -5972,7 +5972,7 @@ impl aya_obj::Object pub fn aya_obj::Object::relocate_btf(&mut self, target_btf: &aya_obj::btf::Btf) -> core::result::Result<(), aya_obj::btf::BtfRelocationError> impl aya_obj::Object pub fn aya_obj::Object::relocate_calls(&mut self, text_sections: &std::collections::hash::set::HashSet) -> core::result::Result<(), aya_obj::relocation::BpfRelocationError> -pub fn aya_obj::Object::relocate_maps<'a, I: core::iter::traits::iterator::Iterator>(&mut self, maps: I, text_sections: &std::collections::hash::set::HashSet) -> core::result::Result<(), aya_obj::relocation::BpfRelocationError> +pub fn aya_obj::Object::relocate_maps<'a, I: core::iter::traits::iterator::Iterator>(&mut self, maps: I, text_sections: &std::collections::hash::set::HashSet) -> core::result::Result<(), aya_obj::relocation::BpfRelocationError> impl core::clone::Clone for aya_obj::Object pub fn aya_obj::Object::clone(&self) -> aya_obj::Object impl core::fmt::Debug for aya_obj::Object @@ -6684,7 +6684,7 @@ impl aya_obj::Object pub fn aya_obj::Object::relocate_btf(&mut self, target_btf: &aya_obj::btf::Btf) -> core::result::Result<(), aya_obj::btf::BtfRelocationError> impl aya_obj::Object pub fn aya_obj::Object::relocate_calls(&mut self, text_sections: &std::collections::hash::set::HashSet) -> core::result::Result<(), aya_obj::relocation::BpfRelocationError> -pub fn aya_obj::Object::relocate_maps<'a, I: core::iter::traits::iterator::Iterator>(&mut self, maps: I, text_sections: &std::collections::hash::set::HashSet) -> core::result::Result<(), aya_obj::relocation::BpfRelocationError> +pub fn aya_obj::Object::relocate_maps<'a, I: core::iter::traits::iterator::Iterator>(&mut self, maps: I, text_sections: &std::collections::hash::set::HashSet) -> core::result::Result<(), aya_obj::relocation::BpfRelocationError> impl core::clone::Clone for aya_obj::Object pub fn aya_obj::Object::clone(&self) -> aya_obj::Object impl core::fmt::Debug for aya_obj::Object diff --git a/xtask/public-api/aya.txt b/xtask/public-api/aya.txt index dfbdde83..3f6dfad6 100644 --- a/xtask/public-api/aya.txt +++ b/xtask/public-api/aya.txt @@ -559,7 +559,7 @@ pub fn aya::maps::queue::Queue::from(t: T) -> T pub mod aya::maps::sock pub struct aya::maps::sock::SockHash impl, K: aya::Pod> aya::maps::SockHash -pub fn aya::maps::SockHash::fd(&self) -> core::result::Result +pub fn aya::maps::SockHash::fd(&self) -> &aya::maps::sock::SockMapFd pub fn aya::maps::SockHash::get(&self, key: &K, flags: u64) -> core::result::Result pub fn aya::maps::SockHash::iter(&self) -> aya::maps::MapIter<'_, K, std::os::fd::raw::RawFd, Self> pub fn aya::maps::SockHash::keys(&self) -> aya::maps::MapKeys<'_, K> @@ -601,7 +601,7 @@ impl core::convert::From for aya::maps::SockHash pub fn aya::maps::SockHash::from(t: T) -> T pub struct aya::maps::sock::SockMap impl> aya::maps::SockMap -pub fn aya::maps::SockMap::fd(&self) -> core::result::Result +pub fn aya::maps::SockMap::fd(&self) -> &aya::maps::sock::SockMapFd pub fn aya::maps::SockMap::indices(&self) -> aya::maps::MapKeys<'_, u32> impl> aya::maps::SockMap pub fn aya::maps::SockMap::clear_index(&mut self, index: &u32) -> core::result::Result<(), aya::maps::MapError> @@ -636,12 +636,11 @@ impl core::borrow::BorrowMut for aya::maps::SockMap where T: core::mark pub fn aya::maps::SockMap::borrow_mut(&mut self) -> &mut T impl core::convert::From for aya::maps::SockMap pub fn aya::maps::SockMap::from(t: T) -> T -pub struct aya::maps::sock::SockMapFd(_) +#[repr(transparent)] pub struct aya::maps::sock::SockMapFd(_) +impl aya::maps::sock::SockMapFd +pub fn aya::maps::sock::SockMapFd::try_clone(&self) -> std::io::error::Result impl std::os::fd::owned::AsFd for aya::maps::sock::SockMapFd pub fn aya::maps::sock::SockMapFd::as_fd(&self) -> std::os::fd::owned::BorrowedFd<'_> -impl core::clone::Clone for aya::maps::sock::SockMapFd -pub fn aya::maps::sock::SockMapFd::clone(&self) -> aya::maps::sock::SockMapFd -impl core::marker::Copy for aya::maps::sock::SockMapFd impl core::marker::Send for aya::maps::sock::SockMapFd impl core::marker::Sync for aya::maps::sock::SockMapFd impl core::marker::Unpin for aya::maps::sock::SockMapFd @@ -655,10 +654,6 @@ pub fn aya::maps::sock::SockMapFd::try_from(value: U) -> core::result::Result core::convert::TryInto for aya::maps::sock::SockMapFd where U: core::convert::TryFrom pub type aya::maps::sock::SockMapFd::Error = >::Error pub fn aya::maps::sock::SockMapFd::try_into(self) -> core::result::Result>::Error> -impl alloc::borrow::ToOwned for aya::maps::sock::SockMapFd where T: core::clone::Clone -pub type aya::maps::sock::SockMapFd::Owned = T -pub fn aya::maps::sock::SockMapFd::clone_into(&self, target: &mut T) -pub fn aya::maps::sock::SockMapFd::to_owned(&self) -> T impl core::any::Any for aya::maps::sock::SockMapFd where T: 'static + core::marker::Sized pub fn aya::maps::sock::SockMapFd::type_id(&self) -> core::any::TypeId impl core::borrow::Borrow for aya::maps::sock::SockMapFd where T: core::marker::Sized @@ -1226,13 +1221,9 @@ pub struct aya::maps::MapData pub aya::maps::MapData::pinned: bool impl aya::maps::MapData pub fn aya::maps::MapData::create(obj: aya_obj::maps::Map, name: &str, btf_fd: core::option::Option>) -> core::result::Result -pub fn aya::maps::MapData::fd(&self) -> aya::maps::MapFd +pub fn aya::maps::MapData::fd(&self) -> &aya::maps::MapFd 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 -impl core::clone::Clone for aya::maps::MapData -pub fn aya::maps::MapData::clone(&self) -> Self -impl core::ops::drop::Drop for aya::maps::MapData -pub fn aya::maps::MapData::drop(&mut self) impl core::fmt::Debug for aya::maps::MapData pub fn aya::maps::MapData::fmt(&self, f: &mut core::fmt::Formatter<'_>) -> core::fmt::Result impl core::marker::Send for aya::maps::MapData @@ -1248,10 +1239,6 @@ pub fn aya::maps::MapData::try_from(value: U) -> core::result::Result core::convert::TryInto for aya::maps::MapData where U: core::convert::TryFrom pub type aya::maps::MapData::Error = >::Error pub fn aya::maps::MapData::try_into(self) -> core::result::Result>::Error> -impl alloc::borrow::ToOwned for aya::maps::MapData where T: core::clone::Clone -pub type aya::maps::MapData::Owned = T -pub fn aya::maps::MapData::clone_into(&self, target: &mut T) -pub fn aya::maps::MapData::to_owned(&self) -> T impl core::any::Any for aya::maps::MapData where T: 'static + core::marker::Sized pub fn aya::maps::MapData::type_id(&self) -> core::any::TypeId impl core::borrow::Borrow for aya::maps::MapData where T: core::marker::Sized @@ -1261,8 +1248,10 @@ pub fn aya::maps::MapData::borrow_mut(&mut self) -> &mut T impl core::convert::From for aya::maps::MapData pub fn aya::maps::MapData::from(t: T) -> T pub struct aya::maps::MapFd(_) -impl std::os::fd::raw::AsRawFd for aya::maps::MapFd -pub fn aya::maps::MapFd::as_raw_fd(&self) -> std::os::fd::raw::RawFd +impl std::os::fd::owned::AsFd for aya::maps::MapFd +pub fn aya::maps::MapFd::as_fd(&self) -> std::os::fd::owned::BorrowedFd<'_> +impl core::fmt::Debug for aya::maps::MapFd +pub fn aya::maps::MapFd::fmt(&self, f: &mut core::fmt::Formatter<'_>) -> core::fmt::Result impl core::marker::Send for aya::maps::MapFd impl core::marker::Sync for aya::maps::MapFd impl core::marker::Unpin for aya::maps::MapFd @@ -1566,7 +1555,7 @@ impl core::convert::From for aya::maps::queue::Queue pub fn aya::maps::queue::Queue::from(t: T) -> T pub struct aya::maps::SockHash impl, K: aya::Pod> aya::maps::SockHash -pub fn aya::maps::SockHash::fd(&self) -> core::result::Result +pub fn aya::maps::SockHash::fd(&self) -> &aya::maps::sock::SockMapFd pub fn aya::maps::SockHash::get(&self, key: &K, flags: u64) -> core::result::Result pub fn aya::maps::SockHash::iter(&self) -> aya::maps::MapIter<'_, K, std::os::fd::raw::RawFd, Self> pub fn aya::maps::SockHash::keys(&self) -> aya::maps::MapKeys<'_, K> @@ -1608,7 +1597,7 @@ impl core::convert::From for aya::maps::SockHash pub fn aya::maps::SockHash::from(t: T) -> T pub struct aya::maps::SockMap impl> aya::maps::SockMap -pub fn aya::maps::SockMap::fd(&self) -> core::result::Result +pub fn aya::maps::SockMap::fd(&self) -> &aya::maps::sock::SockMapFd pub fn aya::maps::SockMap::indices(&self) -> aya::maps::MapKeys<'_, u32> impl> aya::maps::SockMap pub fn aya::maps::SockMap::clear_index(&mut self, index: &u32) -> core::result::Result<(), aya::maps::MapError> @@ -6190,7 +6179,7 @@ impl core::convert::From for aya::programs::perf_event::PerfEvent pub fn aya::programs::perf_event::PerfEvent::from(t: T) -> T pub struct aya::programs::ProgramFd(_) impl aya::programs::ProgramFd -pub fn aya::programs::ProgramFd::try_clone(&self) -> core::result::Result +pub fn aya::programs::ProgramFd::try_clone(&self) -> std::io::error::Result impl std::os::fd::owned::AsFd for aya::programs::ProgramFd pub fn aya::programs::ProgramFd::as_fd(&self) -> std::os::fd::owned::BorrowedFd<'_> impl core::fmt::Debug for aya::programs::ProgramFd @@ -6402,7 +6391,7 @@ impl core::convert::From for aya::programs::SkLookup pub fn aya::programs::SkLookup::from(t: T) -> T pub struct aya::programs::SkMsg impl aya::programs::SkMsg -pub fn aya::programs::SkMsg::attach(&mut self, map: aya::maps::sock::SockMapFd) -> core::result::Result +pub fn aya::programs::SkMsg::attach(&mut self, map: &aya::maps::sock::SockMapFd) -> core::result::Result pub fn aya::programs::SkMsg::detach(&mut self, link_id: SkMsgLinkId) -> core::result::Result<(), aya::programs::ProgramError> pub fn aya::programs::SkMsg::load(&mut self) -> core::result::Result<(), aya::programs::ProgramError> pub fn aya::programs::SkMsg::take_link(&mut self, link_id: SkMsgLinkId) -> core::result::Result @@ -6450,7 +6439,7 @@ impl core::convert::From for aya::programs::SkMsg pub fn aya::programs::SkMsg::from(t: T) -> T pub struct aya::programs::SkSkb impl aya::programs::SkSkb -pub fn aya::programs::SkSkb::attach(&mut self, map: aya::maps::sock::SockMapFd) -> core::result::Result +pub fn aya::programs::SkSkb::attach(&mut self, map: &aya::maps::sock::SockMapFd) -> core::result::Result pub fn aya::programs::SkSkb::detach(&mut self, link_id: SkSkbLinkId) -> core::result::Result<(), aya::programs::ProgramError> pub fn aya::programs::SkSkb::from_pin>(path: P, kind: aya::programs::SkSkbKind) -> core::result::Result pub fn aya::programs::SkSkb::load(&mut self) -> core::result::Result<(), aya::programs::ProgramError> From b4d5a1e8dbb82fc6fca543ad3b6e2f8175ae83b6 Mon Sep 17 00:00:00 2001 From: Tamir Duberstein Date: Fri, 1 Sep 2023 13:53:03 -0400 Subject: [PATCH 2/3] maps: MapData::{obj, fd} are private --- aya/src/bpf.rs | 33 ++++++++------------------------- aya/src/maps/mod.rs | 40 +++++++++++++++++++++++++++++++++++----- 2 files changed, 43 insertions(+), 30 deletions(-) diff --git a/aya/src/bpf.rs b/aya/src/bpf.rs index d81007c7..a5fb9273 100644 --- a/aya/src/bpf.rs +++ b/aya/src/bpf.rs @@ -36,12 +36,11 @@ use crate::{ SkMsg, SkSkb, SkSkbKind, SockOps, SocketFilter, TracePoint, UProbe, Xdp, }, sys::{ - bpf_load_btf, bpf_map_freeze, bpf_map_update_elem_ptr, is_bpf_cookie_supported, - is_bpf_global_data_supported, is_btf_datasec_supported, is_btf_decl_tag_supported, - is_btf_enum64_supported, is_btf_float_supported, is_btf_func_global_supported, - is_btf_func_supported, is_btf_supported, is_btf_type_tag_supported, is_perf_link_supported, + bpf_load_btf, is_bpf_cookie_supported, is_bpf_global_data_supported, + is_btf_datasec_supported, is_btf_decl_tag_supported, is_btf_enum64_supported, + is_btf_float_supported, is_btf_func_global_supported, is_btf_func_supported, + is_btf_supported, is_btf_type_tag_supported, is_perf_link_supported, is_probe_read_kernel_supported, is_prog_name_supported, retry_with_verifier_logs, - SyscallError, }, util::{bytes_of, bytes_of_slice, possible_cpus, POSSIBLE_CPUS}, }; @@ -482,23 +481,7 @@ impl<'a> BpfLoader<'a> { MapData::create_pinned(path, obj, &name, btf_fd)? } }; - let MapData { obj, fd, pinned: _ } = &mut map; - if !obj.data().is_empty() && obj.section_kind() != BpfSectionKind::Bss { - bpf_map_update_elem_ptr(fd.as_fd(), &0 as *const _, obj.data_mut().as_mut_ptr(), 0) - .map_err(|(_, io_error)| SyscallError { - call: "bpf_map_update_elem", - io_error, - }) - .map_err(MapError::from)?; - } - if obj.section_kind() == BpfSectionKind::Rodata { - bpf_map_freeze(fd.as_fd()) - .map_err(|(_, io_error)| SyscallError { - call: "bpf_map_freeze", - io_error, - }) - .map_err(MapError::from)?; - } + map.finalize()?; maps.insert(name, map); } @@ -510,7 +493,7 @@ impl<'a> BpfLoader<'a> { obj.relocate_maps( maps.iter() - .map(|(s, data)| (s.as_str(), data.fd().as_fd().as_raw_fd(), &data.obj)), + .map(|(s, data)| (s.as_str(), data.fd().as_fd().as_raw_fd(), data.obj())), &text_sections, )?; obj.relocate_calls(&text_sections)?; @@ -691,7 +674,7 @@ impl<'a> BpfLoader<'a> { if !*allow_unsupported_maps { maps.iter().try_for_each(|(_, x)| match x { Map::Unsupported(map) => Err(BpfError::MapError(MapError::Unsupported { - map_type: map.obj.map_type(), + map_type: map.obj().map_type(), })), _ => Ok(()), })?; @@ -705,7 +688,7 @@ fn parse_map(data: (String, MapData)) -> Result<(String, Map), BpfError> { let name = data.0; let map = data.1; let map_type = - bpf_map_type::try_from(map.obj.map_type()).map_err(|e| MapError::InvalidMapType { + bpf_map_type::try_from(map.obj().map_type()).map_err(|e| MapError::InvalidMapType { map_type: e.map_type, })?; let map = match map_type { diff --git a/aya/src/maps/mod.rs b/aya/src/maps/mod.rs index 85ba5d94..f5052ed5 100644 --- a/aya/src/maps/mod.rs +++ b/aya/src/maps/mod.rs @@ -64,11 +64,11 @@ use log::warn; use thiserror::Error; use crate::{ - obj::{self, parse_map_info}, + obj::{self, parse_map_info, BpfSectionKind}, pin::PinError, sys::{ - bpf_create_map, bpf_get_object, bpf_map_get_info_by_fd, bpf_map_get_next_key, - bpf_pin_object, SyscallError, + bpf_create_map, bpf_get_object, bpf_map_freeze, bpf_map_get_info_by_fd, + bpf_map_get_next_key, bpf_map_update_elem_ptr, bpf_pin_object, SyscallError, }, util::nr_cpus, PinningType, Pod, @@ -408,8 +408,8 @@ pub(crate) fn check_v_size(map: &MapData) -> Result<(), MapError> { /// You should never need to use this unless you're implementing a new map type. #[derive(Debug)] pub struct MapData { - pub(crate) obj: obj::Map, - pub(crate) fd: MapFd, + obj: obj::Map, + fd: MapFd, /// Indicates if this map has been pinned to bpffs pub pinned: bool, } @@ -489,6 +489,27 @@ impl MapData { } } + pub(crate) fn finalize(&mut self) -> Result<(), MapError> { + let Self { obj, fd, pinned: _ } = self; + if !obj.data().is_empty() && obj.section_kind() != BpfSectionKind::Bss { + bpf_map_update_elem_ptr(fd.as_fd(), &0 as *const _, obj.data_mut().as_mut_ptr(), 0) + .map_err(|(_, io_error)| SyscallError { + call: "bpf_map_update_elem", + io_error, + }) + .map_err(MapError::from)?; + } + if obj.section_kind() == BpfSectionKind::Rodata { + bpf_map_freeze(fd.as_fd()) + .map_err(|(_, io_error)| SyscallError { + call: "bpf_map_freeze", + io_error, + }) + .map_err(MapError::from)?; + } + Ok(()) + } + /// Loads a map from a pinned path in bpffs. pub fn from_pin>(path: P) -> Result { use std::os::unix::ffi::OsStrExt as _; @@ -561,6 +582,15 @@ impl MapData { } = self; fd } + + pub(crate) fn obj(&self) -> &obj::Map { + let Self { + obj, + fd: _, + pinned: _, + } = self; + obj + } } /// An iterable map From 0dacb34d449f71b1998b0a23cd58c0023277c2ef Mon Sep 17 00:00:00 2001 From: Tamir Duberstein Date: Fri, 1 Sep 2023 13:40:43 -0400 Subject: [PATCH 3/3] maps: fix typos, avoid fallible conversions --- aya/src/maps/mod.rs | 41 ++++++++++++++++++++--------------------- 1 file changed, 20 insertions(+), 21 deletions(-) diff --git a/aya/src/maps/mod.rs b/aya/src/maps/mod.rs index f5052ed5..163ce127 100644 --- a/aya/src/maps/mod.rs +++ b/aya/src/maps/mod.rs @@ -59,7 +59,7 @@ use std::{ }; use crate::util::KernelVersion; -use libc::{getrlimit, rlimit, RLIMIT_MEMLOCK, RLIM_INFINITY}; +use libc::{getrlimit, rlim_t, rlimit, RLIMIT_MEMLOCK, RLIM_INFINITY}; use log::warn; use thiserror::Error; @@ -198,20 +198,6 @@ impl AsFd for MapFd { } } -#[derive(PartialEq, Eq, PartialOrd, Ord)] -struct RlimitSize(usize); -impl fmt::Display for RlimitSize { - fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { - if self.0 < 1024 { - write!(f, "{} bytes", self.0) - } else if self.0 < 1024 * 1024 { - write!(f, "{} KiB", self.0 / 1024) - } else { - write!(f, "{} MiB", self.0 / 1024 / 1024) - } - } -} - /// Raises a warning about rlimit. Should be used only if creating a map was not /// successful. fn maybe_warn_rlimit() { @@ -220,15 +206,28 @@ fn maybe_warn_rlimit() { if ret == 0 { let limit = unsafe { limit.assume_init() }; - let limit: RlimitSize = RlimitSize(limit.rlim_cur.try_into().unwrap()); - if limit.0 == RLIM_INFINITY.try_into().unwrap() { + if limit.rlim_cur == RLIM_INFINITY { return; } + struct HumanSize(rlim_t); + + impl fmt::Display for HumanSize { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + let &Self(size) = self; + if size < 1024 { + write!(f, "{} bytes", size) + } else if size < 1024 * 1024 { + write!(f, "{} KiB", size / 1024) + } else { + write!(f, "{} MiB", size / 1024 / 1024) + } + } + } warn!( - "RLIMIT_MEMLOCK value is {}, not RLIM_INFNITY; if experiencing problems with creating \ - maps, try raising RMILIT_MEMLOCK either to RLIM_INFINITY or to a higher value sufficient \ - for size of your maps", - limit + "RLIMIT_MEMLOCK value is {}, not RLIM_INFINITY; if experiencing problems with creating \ + maps, try raising RLIMIT_MEMLOCK either to RLIM_INFINITY or to a higher value sufficient \ + for the size of your maps", + HumanSize(limit.rlim_cur) ); } }