Skip to content

Commit

Permalink
aya: MapData::fd is non-optional
Browse files Browse the repository at this point in the history 
The primary driver of change here is that `MapData::create` is now a
factory function that returns `Result<Self, _>` rather than mutating
`&mut self`. The remaining changes are consequences of that change, the
most notable of which is the removal of several errors which are no
longer possible.
  • Loading branch information
@tamird
tamird committed Aug 17, 2023
1 parent c7b5cd5 commit 89bc255
Show file tree
Hide file tree
Showing 21 changed files with 284 additions and 539 deletions.
36 changes: 11 additions & 25 deletions aya-obj/src/relocation.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -73,15 +73,6 @@ pub enum RelocationError {
address: u64,
},

/// Referenced map not created yet
#[error("the map `{name}` at section `{section_index}` has not been created")]
MapNotCreated {
/// The section index
section_index: usize,
/// The map name
name: String,
},

/// Invalid relocation offset
#[error("invalid offset `{offset}` applying relocation #{relocation_number}")]
InvalidRelocationOffset {
Expand Down Expand Up @@ -114,7 +105,7 @@ pub(crate) struct Symbol {

impl Object {
/// Relocates the map references
pub fn relocate_maps<'a, I: Iterator<Item = (&'a str, Option<i32>, &'a Map)>>(
pub fn relocate_maps<'a, I: Iterator<Item = (&'a str, i32, &'a Map)>>(
&mut self,
maps: I,
text_sections: &HashSet<usize>,
Expand Down Expand Up @@ -187,8 +178,8 @@ impl Object {
fn relocate_maps<'a, I: Iterator<Item = &'a Relocation>>(
fun: &mut Function,
relocations: I,
maps_by_section: &HashMap<usize, (&str, Option<i32>, &Map)>,
maps_by_symbol: &HashMap<usize, (&str, Option<i32>, &Map)>,
maps_by_section: &HashMap<usize, (&str, i32, &Map)>,
maps_by_symbol: &HashMap<usize, (&str, i32, &Map)>,
symbol_table: &HashMap<usize, Symbol>,
text_sections: &HashSet<usize>,
) -> Result<(), RelocationError> {
Expand Down Expand Up @@ -230,7 +221,7 @@ fn relocate_maps<'a, I: Iterator<Item = &'a Relocation>>(
continue;
}

let (name, fd, map) = if let Some(m) = maps_by_symbol.get(&rel.symbol_index) {
let (_name, fd, map) = if let Some(m) = maps_by_symbol.get(&rel.symbol_index) {
let map = &m.2;
debug!(
"relocating map by symbol index {:?}, kind {:?} at insn {ins_index} in section {}",
Expand Down Expand Up @@ -266,18 +257,13 @@ fn relocate_maps<'a, I: Iterator<Item = &'a Relocation>>(
};
debug_assert_eq!(map.section_index(), section_index);

let map_fd = fd.ok_or_else(|| RelocationError::MapNotCreated {
name: (*name).into(),
section_index,
})?;

if !map.data().is_empty() {
instructions[ins_index].set_src_reg(BPF_PSEUDO_MAP_VALUE as u8);
instructions[ins_index + 1].imm = instructions[ins_index].imm + sym.address as i32;
} else {
instructions[ins_index].set_src_reg(BPF_PSEUDO_MAP_FD as u8);
}
instructions[ins_index].imm = map_fd;
instructions[ins_index].imm = *fd;
}

Ok(())
Expand Down Expand Up @@ -588,7 +574,7 @@ mod test {
let maps_by_section = HashMap::new();

let map = fake_legacy_map(1);
let maps_by_symbol = HashMap::from([(1, ("test_map", Some(1), &map))]);
let maps_by_symbol = HashMap::from([(1, ("test_map", 1, &map))]);

relocate_maps(
&mut fun,
Expand Down Expand Up @@ -642,8 +628,8 @@ mod test {
let map_1 = fake_legacy_map(1);
let map_2 = fake_legacy_map(2);
let maps_by_symbol = HashMap::from([
(1, ("test_map_1", Some(1), &map_1)),
(2, ("test_map_2", Some(2), &map_2)),
(1, ("test_map_1", 1, &map_1)),
(2, ("test_map_2", 2, &map_2)),
]);

relocate_maps(
Expand Down Expand Up @@ -683,7 +669,7 @@ mod test {
let maps_by_section = HashMap::new();

let map = fake_btf_map(1);
let maps_by_symbol = HashMap::from([(1, ("test_map", Some(1), &map))]);
let maps_by_symbol = HashMap::from([(1, ("test_map", 1, &map))]);

relocate_maps(
&mut fun,
Expand Down Expand Up @@ -737,8 +723,8 @@ mod test {
let map_1 = fake_btf_map(1);
let map_2 = fake_btf_map(2);
let maps_by_symbol = HashMap::from([
(1, ("test_map_1", Some(1), &map_1)),
(2, ("test_map_2", Some(2), &map_2)),
(1, ("test_map_1", 1, &map_1)),
(2, ("test_map_2", 2, &map_2)),
]);

relocate_maps(
Expand Down
34 changes: 7 additions & 27 deletions aya/src/bpf.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ use std::{
ffi::CString,
fs, io,
os::{
fd::{AsFd as _, OwnedFd, RawFd},
fd::{AsFd as _, OwnedFd},
raw::c_int,
},
path::{Path, PathBuf},
Expand Down Expand Up @@ -475,35 +475,15 @@ impl<'a> BpfLoader<'a> {
}
}
}
let mut map = MapData {
obj,
fd: None,
pinned: false,
};
let fd = match map.obj.pinning() {
let btf_fd = btf_fd.as_deref().map(|fd| fd.as_fd());
let mut map = match obj.pinning() {
PinningType::None => MapData::create(obj, &name, btf_fd)?,
PinningType::ByName => {
let path = match &map_pin_path {
Some(p) => p,
None => return Err(BpfError::NoPinPath),
};
// try to open map in case it's already pinned
match map.open_pinned(&name, path) {
Ok(fd) => {
map.pinned = true;
fd as RawFd
}
Err(_) => {
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,
})?;
fd
}
}
let path = map_pin_path.as_ref().ok_or(BpfError::NoPinPath)?;
MapData::create_pinned(path, obj, &name, btf_fd)?
}
PinningType::None => map.create(&name, btf_fd.as_deref().map(|f| f.as_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)
.map_err(|(_, io_error)| SyscallError {
Expand Down
6 changes: 2 additions & 4 deletions aya/src/maps/array/array.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -39,8 +39,6 @@ impl<T: Borrow<MapData>, V: Pod> Array<T, V> {
let data = map.borrow();
check_kv_size::<u32, V>(data)?;

let _fd = data.fd_or_err()?;

Ok(Array {
inner: map,
_v: PhantomData,
Expand All @@ -63,7 +61,7 @@ impl<T: Borrow<MapData>, V: Pod> Array<T, V> {
pub fn get(&self, index: &u32, flags: u64) -> Result<V, MapError> {
let data = self.inner.borrow();
check_bounds(data, *index)?;
let fd = data.fd_or_err()?;
let fd = data.fd;

let value =
bpf_map_lookup_elem(fd, index, flags).map_err(|(_, io_error)| SyscallError {
Expand All @@ -90,7 +88,7 @@ impl<T: BorrowMut<MapData>, V: Pod> Array<T, V> {
pub fn set(&mut self, index: u32, value: impl Borrow<V>, flags: u64) -> Result<(), MapError> {
let data = self.inner.borrow_mut();
check_bounds(data, index)?;
let fd = data.fd_or_err()?;
let fd = data.fd;
bpf_map_update_elem(fd, Some(&index), value.borrow(), flags).map_err(|(_, io_error)| {
SyscallError {
call: "bpf_map_update_elem",
Expand Down
6 changes: 2 additions & 4 deletions aya/src/maps/array/per_cpu_array.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -58,8 +58,6 @@ impl<T: Borrow<MapData>, V: Pod> PerCpuArray<T, V> {
let data = map.borrow();
check_kv_size::<u32, V>(data)?;

let _fd = data.fd_or_err()?;

Ok(PerCpuArray {
inner: map,
_v: PhantomData,
Expand All @@ -82,7 +80,7 @@ impl<T: Borrow<MapData>, V: Pod> PerCpuArray<T, V> {
pub fn get(&self, index: &u32, flags: u64) -> Result<PerCpuValues<V>, MapError> {
let data = self.inner.borrow();
check_bounds(data, *index)?;
let fd = data.fd_or_err()?;
let fd = data.fd;

let value = bpf_map_lookup_elem_per_cpu(fd, index, flags).map_err(|(_, io_error)| {
SyscallError {
Expand Down Expand Up @@ -110,7 +108,7 @@ impl<T: BorrowMut<MapData>, V: Pod> PerCpuArray<T, V> {
pub fn set(&mut self, index: u32, values: PerCpuValues<V>, flags: u64) -> Result<(), MapError> {
let data = self.inner.borrow_mut();
check_bounds(data, index)?;
let fd = data.fd_or_err()?;
let fd = data.fd;

bpf_map_update_elem_per_cpu(fd, &index, &values, flags).map_err(|(_, io_error)| {
SyscallError {
Expand Down
6 changes: 2 additions & 4 deletions aya/src/maps/array/program_array.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -56,8 +56,6 @@ impl<T: Borrow<MapData>> ProgramArray<T> {
let data = map.borrow();
check_kv_size::<u32, RawFd>(data)?;

let _fd = data.fd_or_err()?;

Ok(ProgramArray { inner: map })
}

Expand All @@ -76,7 +74,7 @@ impl<T: BorrowMut<MapData>> ProgramArray<T> {
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_or_err()?;
let fd = data.fd;
let prog_fd = program.as_fd();
let prog_fd = prog_fd.as_raw_fd();

Expand All @@ -96,7 +94,7 @@ impl<T: BorrowMut<MapData>> ProgramArray<T> {
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_or_err()?;
let fd = self.inner.borrow_mut().fd;

bpf_map_delete_elem(fd, index)
.map(|_| ())
Expand Down
Loading

0 comments on commit 89bc255

Please sign in to comment.