wrap OwnedFd of programs and maps around Arc

this allows ProgramFd, MapFd, and SockFd to not have lifetimes tied to
the programs/maps. This does mean these structs are no longer
Copy-able but that feels like a less of a breaking change
reviewable/pr614/r8
Andrés Medina 2 years ago
parent 4452e2f6e2
commit c2c4095737

@ -88,7 +88,7 @@ mod tests {
sys::{override_syscall, SysResult, Syscall}, sys::{override_syscall, SysResult, Syscall},
}; };
use libc::{EFAULT, ENOENT}; use libc::{EFAULT, ENOENT};
use std::{env, fs::File, io, os::fd::OwnedFd}; use std::{env, fs::File, io, os::fd::OwnedFd, sync::Arc};
fn new_obj_map() -> obj::Map { fn new_obj_map() -> obj::Map {
obj::Map::Legacy(LegacyMap { obj::Map::Legacy(LegacyMap {
@ -275,10 +275,12 @@ mod tests {
)); ));
} }
fn create_fd() -> OwnedFd { fn create_fd() -> Arc<OwnedFd> {
let dir = env::temp_dir(); let dir = env::temp_dir();
File::create(dir.join("f1")) Arc::new(
.expect("unable to create file in tmpdir") File::create(dir.join("f1"))
.into() .expect("unable to create file in tmpdir")
.into(),
)
} }
} }

@ -105,7 +105,7 @@ impl<T: Borrow<MapData>, K: Pod, V: Pod> IterableMap<K, V> for HashMap<T, K, V>
#[cfg(test)] #[cfg(test)]
mod tests { mod tests {
use std::{env, fs::File, io, os::fd::OwnedFd}; use std::{env, fs::File, io, os::fd::OwnedFd, sync::Arc};
use libc::{EFAULT, ENOENT}; use libc::{EFAULT, ENOENT};
@ -696,10 +696,12 @@ mod tests {
assert!(matches!(iter.next(), None)); assert!(matches!(iter.next(), None));
} }
fn create_fd() -> OwnedFd { fn create_fd() -> Arc<OwnedFd> {
let dir = env::temp_dir(); let dir = env::temp_dir();
File::create(dir.join("f1")) Arc::new(
.expect("unable to create file in tmpdir") File::create(dir.join("f1"))
.into() .expect("unable to create file in tmpdir")
.into(),
)
} }
} }

@ -207,7 +207,7 @@ mod tests {
sys::{override_syscall, SysResult, Syscall}, sys::{override_syscall, SysResult, Syscall},
}; };
use libc::{EFAULT, ENOENT}; use libc::{EFAULT, ENOENT};
use std::{env, fs::File, io, mem, net::Ipv4Addr, os::fd::OwnedFd}; use std::{env, fs::File, io, mem, net::Ipv4Addr, os::fd::OwnedFd, sync::Arc};
fn new_obj_map() -> obj::Map { fn new_obj_map() -> obj::Map {
obj::Map::Legacy(LegacyMap { obj::Map::Legacy(LegacyMap {
@ -456,10 +456,12 @@ mod tests {
assert!(matches!(trie.get(&key, 0), Err(MapError::KeyNotFound))); assert!(matches!(trie.get(&key, 0), Err(MapError::KeyNotFound)));
} }
fn create_fd() -> OwnedFd { fn create_fd() -> Arc<OwnedFd> {
let dir = env::temp_dir(); let dir = env::temp_dir();
File::create(dir.join("f1")) Arc::new(
.expect("unable to create file in tmpdir") File::create(dir.join("f1"))
.into() .expect("unable to create file in tmpdir")
.into(),
)
} }
} }

@ -46,6 +46,7 @@ use std::{
os::fd::{AsFd, AsRawFd, BorrowedFd, OwnedFd, RawFd}, os::fd::{AsFd, AsRawFd, BorrowedFd, OwnedFd, RawFd},
path::Path, path::Path,
ptr, ptr,
sync::Arc,
}; };
use libc::{getrlimit, rlimit, RLIMIT_MEMLOCK, RLIM_INFINITY}; use libc::{getrlimit, rlimit, RLIMIT_MEMLOCK, RLIM_INFINITY};
@ -186,15 +187,15 @@ pub enum MapError {
} }
/// A map file descriptor. /// A map file descriptor.
pub struct MapFd<'f>(BorrowedFd<'f>); pub struct MapFd(Arc<OwnedFd>);
impl AsRawFd for MapFd<'_> { impl AsRawFd for MapFd {
fn as_raw_fd(&self) -> RawFd { fn as_raw_fd(&self) -> RawFd {
self.0.as_raw_fd() self.0.as_raw_fd()
} }
} }
impl AsFd for MapFd<'_> { impl AsFd for MapFd {
fn as_fd(&self) -> BorrowedFd<'_> { fn as_fd(&self) -> BorrowedFd<'_> {
self.0.as_fd() self.0.as_fd()
} }
@ -478,10 +479,10 @@ pub(crate) fn check_v_size<V>(map: &MapData) -> Result<(), MapError> {
/// A generic handle to a BPF map. /// A generic handle to a BPF map.
/// ///
/// You should never need to use this unless you're implementing a new map type. /// You should never need to use this unless you're implementing a new map type.
#[derive(Debug)] #[derive(Debug, Clone)]
pub struct MapData { pub struct MapData {
pub(crate) obj: obj::Map, pub(crate) obj: obj::Map,
pub(crate) fd: Option<OwnedFd>, pub(crate) fd: Option<Arc<OwnedFd>>,
pub(crate) btf_fd: Option<RawFd>, pub(crate) btf_fd: Option<RawFd>,
/// Indicates if this map has been pinned to bpffs /// Indicates if this map has been pinned to bpffs
pub pinned: bool, pub pinned: bool,
@ -509,7 +510,7 @@ impl MapData {
} }
})?; })?;
self.fd = Some(fd); self.fd = Some(Arc::new(fd));
Ok(()) Ok(())
} }
@ -528,7 +529,7 @@ impl MapData {
call: "BPF_OBJ_GET".to_string(), call: "BPF_OBJ_GET".to_string(),
io_error, io_error,
})?; })?;
self.fd = Some(fd); self.fd = Some(Arc::new(fd));
Ok(()) Ok(())
} }
@ -558,7 +559,7 @@ impl MapData {
Ok(MapData { Ok(MapData {
obj: parse_map_info(info, PinningType::ByName), obj: parse_map_info(info, PinningType::ByName),
fd: Some(fd), fd: Some(Arc::new(fd)),
btf_fd: None, btf_fd: None,
pinned: true, pinned: true,
}) })
@ -578,7 +579,7 @@ impl MapData {
Ok(MapData { Ok(MapData {
obj: parse_map_info(info, PinningType::None), obj: parse_map_info(info, PinningType::None),
fd: Some(fd), fd: Some(Arc::new(fd)),
btf_fd: None, btf_fd: None,
pinned: false, pinned: false,
}) })
@ -615,32 +616,9 @@ impl MapData {
/// Returns the file descriptor of the map. /// Returns the file descriptor of the map.
/// ///
/// Can be converted to [`RawFd`] using [`AsRawFd`]. /// Can be converted to [`RawFd`] using [`AsRawFd`].
pub fn fd(&self) -> Option<MapFd<'_>> { pub fn fd(&self) -> Option<MapFd> {
self.fd.as_ref().map(|fd| MapFd(fd.as_fd())) let fd = self.fd.clone()?;
} Some(MapFd(fd))
}
impl MapData {
/// Attempts to duplicate MapData
///
/// Fails if if the inner file descriptor could not be cloned
// TODO (AM): should we return MapError? New variant? Maybe make MapError non_exhaustive?
pub fn try_clone(&self) -> io::Result<Self> {
let fd = self.fd.as_ref().map(|f| f.try_clone()).transpose()?;
Ok(MapData {
fd,
obj: self.obj.clone(),
btf_fd: self.btf_fd,
pinned: self.pinned,
})
}
}
// TODO (AM): DELETE?
impl Clone for MapData {
fn clone(&self) -> MapData {
self.try_clone().unwrap()
} }
} }

@ -5,22 +5,25 @@ mod sock_map;
pub use sock_hash::SockHash; pub use sock_hash::SockHash;
pub use sock_map::SockMap; pub use sock_map::SockMap;
use std::os::{ use std::{
fd::{AsFd, BorrowedFd}, os::{
unix::io::{AsRawFd, RawFd}, fd::{AsFd, BorrowedFd, OwnedFd},
unix::io::{AsRawFd, RawFd},
},
sync::Arc,
}; };
/// A socket map file descriptor. /// A socket map file descriptor.
#[derive(Copy, Clone)] #[derive(Clone)]
pub struct SockMapFd<'f>(BorrowedFd<'f>); pub struct SockMapFd(Arc<OwnedFd>);
impl AsRawFd for SockMapFd<'_> { impl AsRawFd for SockMapFd {
fn as_raw_fd(&self) -> RawFd { fn as_raw_fd(&self) -> RawFd {
self.0.as_raw_fd() self.0.as_raw_fd()
} }
} }
impl AsFd for SockMapFd<'_> { impl AsFd for SockMapFd {
fn as_fd(&self) -> BorrowedFd<'_> { fn as_fd(&self) -> BorrowedFd<'_> {
self.0.as_fd() self.0.as_fd()
} }

@ -49,10 +49,9 @@ use crate::{
/// let mut intercept_egress = SockHash::<_, u32>::try_from(bpf.map("INTERCEPT_EGRESS").unwrap())?; /// 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()?;
/// ///
/// // TODO (AM) : figure out lifetimes /// let prog: &mut SkMsg = bpf.program_mut("intercept_egress_packet").unwrap().try_into()?;
/// // let prog: &mut SkMsg = bpf.program_mut("intercept_egress_packet").unwrap().try_into()?; /// prog.load()?;
/// // prog.load()?; /// prog.attach(map_fd)?;
/// // prog.attach(map_fd)?;
/// ///
/// let mut client = TcpStream::connect("127.0.0.1:1234")?; /// let mut client = TcpStream::connect("127.0.0.1:1234")?;
/// let mut intercept_egress = SockHash::try_from(bpf.map_mut("INTERCEPT_EGRESS").unwrap())?; /// let mut intercept_egress = SockHash::try_from(bpf.map_mut("INTERCEPT_EGRESS").unwrap())?;
@ -109,8 +108,10 @@ impl<T: Borrow<MapData>, K: Pod> SockHash<T, K> {
/// ///
/// The returned file descriptor can be used to attach programs that work with /// 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). /// socket maps, like [`SkMsg`](crate::programs::SkMsg) and [`SkSkb`](crate::programs::SkSkb).
pub fn fd(&self) -> Result<SockMapFd<'_>, MapError> { pub fn fd(&self) -> Result<SockMapFd, MapError> {
Ok(SockMapFd(self.inner.borrow().fd_or_err()?)) Ok(SockMapFd(
self.inner.borrow().fd.clone().ok_or(MapError::NotCreated)?,
))
} }
} }

@ -33,10 +33,9 @@ use crate::{
/// let intercept_ingress = SockMap::try_from(bpf.map("INTERCEPT_INGRESS").unwrap())?; /// let intercept_ingress = SockMap::try_from(bpf.map("INTERCEPT_INGRESS").unwrap())?;
/// let map_fd = intercept_ingress.fd()?; /// let map_fd = intercept_ingress.fd()?;
/// ///
/// // TODO (AM): figure out lifetimes /// let prog: &mut SkSkb = bpf.program_mut("intercept_ingress_packet").unwrap().try_into()?;
/// // let prog: &mut SkSkb = bpf.program_mut("intercept_ingress_packet").unwrap().try_into()?; /// prog.load()?;
/// // prog.load()?; /// prog.attach(map_fd)?;
/// // prog.attach(map_fd)?;
/// ///
/// # Ok::<(), aya::BpfError>(()) /// # Ok::<(), aya::BpfError>(())
/// ``` /// ```
@ -65,8 +64,10 @@ impl<T: Borrow<MapData>> SockMap<T> {
/// ///
/// The returned file descriptor can be used to attach programs that work with /// 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). /// socket maps, like [`SkMsg`](crate::programs::SkMsg) and [`SkSkb`](crate::programs::SkSkb).
pub fn fd(&self) -> Result<SockMapFd<'_>, MapError> { pub fn fd(&self) -> Result<SockMapFd, MapError> {
Ok(SockMapFd(self.inner.borrow().fd_or_err()?)) Ok(SockMapFd(
self.inner.borrow().fd.clone().ok_or(MapError::NotCreated)?,
))
} }
} }

@ -42,11 +42,9 @@ pub enum ExtensionError {
/// prog.attach("eth0", XdpFlags::default())?; /// prog.attach("eth0", XdpFlags::default())?;
/// ///
/// let prog_fd = prog.fd().unwrap(); /// let prog_fd = prog.fd().unwrap();
/// // TODO (AM): can't borrow here becauase prog_fd is holding a reference already /// let ext: &mut Extension = bpf.program_mut("extension").unwrap().try_into()?;
/// // need to confirm what the API wants to be /// ext.load(prog_fd, "function_to_replace")?;
/// // let ext: &mut Extension = bpf.program_mut("extension").unwrap().try_into()?; /// ext.attach()?;
/// // ext.load(prog_fd, "function_to_replace")?;
/// // ext.attach()?;
/// Ok::<(), aya::BpfError>(()) /// Ok::<(), aya::BpfError>(())
/// ``` /// ```
#[derive(Debug)] #[derive(Debug)]

@ -73,6 +73,7 @@ use std::{
unix::io::{AsRawFd, RawFd}, unix::io::{AsRawFd, RawFd},
}, },
path::{Path, PathBuf}, path::{Path, PathBuf},
sync::Arc,
}; };
use thiserror::Error; use thiserror::Error;
@ -217,16 +218,16 @@ pub enum ProgramError {
} }
/// A [`Program`] file descriptor. /// A [`Program`] file descriptor.
#[derive(Copy, Clone)] #[derive(Clone)]
pub struct ProgramFd<'f>(BorrowedFd<'f>); pub struct ProgramFd(Arc<OwnedFd>);
impl AsRawFd for ProgramFd<'_> { impl AsRawFd for ProgramFd {
fn as_raw_fd(&self) -> RawFd { fn as_raw_fd(&self) -> RawFd {
self.0.as_raw_fd() self.0.as_raw_fd()
} }
} }
impl AsFd for ProgramFd<'_> { impl AsFd for ProgramFd {
fn as_fd(&self) -> BorrowedFd<'_> { fn as_fd(&self) -> BorrowedFd<'_> {
self.0.as_fd() self.0.as_fd()
} }
@ -415,7 +416,7 @@ impl Program {
pub(crate) struct ProgramData<T: Link> { pub(crate) struct ProgramData<T: Link> {
pub(crate) name: Option<String>, pub(crate) name: Option<String>,
pub(crate) obj: Option<(obj::Program, obj::Function)>, pub(crate) obj: Option<(obj::Program, obj::Function)>,
pub(crate) fd: Option<OwnedFd>, pub(crate) fd: Option<Arc<OwnedFd>>,
pub(crate) links: LinkMap<T>, pub(crate) links: LinkMap<T>,
pub(crate) expected_attach_type: Option<bpf_attach_type>, pub(crate) expected_attach_type: Option<bpf_attach_type>,
pub(crate) attach_btf_obj_fd: Option<OwnedFd>, pub(crate) attach_btf_obj_fd: Option<OwnedFd>,
@ -476,7 +477,7 @@ impl<T: Link> ProgramData<T> {
Ok(ProgramData { Ok(ProgramData {
name, name,
obj: None, obj: None,
fd: Some(fd), fd: Some(Arc::new(fd)),
links: LinkMap::new(), links: LinkMap::new(),
expected_attach_type: None, expected_attach_type: None,
attach_btf_obj_fd, attach_btf_obj_fd,
@ -630,7 +631,7 @@ fn load_program<T: Link>(
match ret { match ret {
Ok(prog_fd) => { Ok(prog_fd) => {
*fd = Some(prog_fd); *fd = Some(Arc::new(prog_fd));
Ok(()) Ok(())
} }
Err((_, io_error)) => { Err((_, io_error)) => {
@ -739,8 +740,9 @@ macro_rules! impl_fd {
$( $(
impl $struct_name { impl $struct_name {
/// Returns the file descriptor of this Program. /// Returns the file descriptor of this Program.
pub fn fd(&self) -> Option<ProgramFd<'_>> { pub fn fd(&self) -> Option<ProgramFd> {
self.data.fd.as_ref().map(|fd| ProgramFd(fd.as_fd())) let fd = self.data.fd.clone()?;
Some(ProgramFd(fd))
} }
} }
)+ )+

@ -46,10 +46,9 @@ use crate::{
/// let intercept_egress: SockHash<_, u32> = bpf.map("INTERCEPT_EGRESS").unwrap().try_into()?; /// let intercept_egress: SockHash<_, u32> = bpf.map("INTERCEPT_EGRESS").unwrap().try_into()?;
/// let map_fd = intercept_egress.fd()?; /// let map_fd = intercept_egress.fd()?;
/// ///
/// // TODO (AM): Figure out lifetimes /// let prog: &mut SkMsg = bpf.program_mut("intercept_egress_packet").unwrap().try_into()?;
/// // let prog: &mut SkMsg = bpf.program_mut("intercept_egress_packet").unwrap().try_into()?; /// prog.load()?;
/// // prog.load()?; /// prog.attach(map_fd)?;
/// // prog.attach(map_fd)?;
/// ///
/// let mut client = TcpStream::connect("127.0.0.1:1234")?; /// let mut client = TcpStream::connect("127.0.0.1:1234")?;
/// let mut intercept_egress: SockHash<_, u32> = bpf.map_mut("INTERCEPT_EGRESS").unwrap().try_into()?; /// let mut intercept_egress: SockHash<_, u32> = bpf.map_mut("INTERCEPT_EGRESS").unwrap().try_into()?;
@ -79,7 +78,7 @@ impl SkMsg {
/// Attaches the program to the given sockmap. /// Attaches the program to the given sockmap.
/// ///
/// The returned value can be used to detach, see [SkMsg::detach]. /// The returned value can be used to detach, see [SkMsg::detach].
pub fn attach(&mut self, map: SockMapFd<'_>) -> Result<SkMsgLinkId, ProgramError> { pub fn attach(&mut self, map: SockMapFd) -> Result<SkMsgLinkId, ProgramError> {
let prog_fd = self.data.fd_or_err()?; let prog_fd = self.data.fd_or_err()?;
let map_fd = map.as_fd(); let map_fd = map.as_fd();

@ -47,10 +47,9 @@ pub enum SkSkbKind {
/// let intercept_ingress: SockMap<_> = bpf.map("INTERCEPT_INGRESS").unwrap().try_into()?; /// let intercept_ingress: SockMap<_> = bpf.map("INTERCEPT_INGRESS").unwrap().try_into()?;
/// let map_fd = intercept_ingress.fd()?; /// let map_fd = intercept_ingress.fd()?;
/// ///
/// // TODO (AM): figure out lifetimes /// let prog: &mut SkSkb = bpf.program_mut("intercept_ingress_packet").unwrap().try_into()?;
/// // let prog: &mut SkSkb = bpf.program_mut("intercept_ingress_packet").unwrap().try_into()?; /// prog.load()?;
/// // prog.load()?; /// prog.attach(map_fd)?;
/// // prog.attach(map_fd)?;
/// ///
/// # Ok::<(), aya::BpfError>(()) /// # Ok::<(), aya::BpfError>(())
/// ``` /// ```
@ -74,7 +73,7 @@ impl SkSkb {
/// Attaches the program to the given socket map. /// Attaches the program to the given socket map.
/// ///
/// The returned value can be used to detach, see [SkSkb::detach]. /// The returned value can be used to detach, see [SkSkb::detach].
pub fn attach(&mut self, map: SockMapFd<'_>) -> Result<SkSkbLinkId, ProgramError> { pub fn attach(&mut self, map: SockMapFd) -> Result<SkSkbLinkId, ProgramError> {
let prog_fd = self.data.fd_or_err()?; let prog_fd = self.data.fd_or_err()?;
let map_fd = map.as_fd(); let map_fd = map.as_fd();

Loading…
Cancel
Save