From 6f483c24831542748883cf418d5a892358db4caa Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20M=C3=BCller?= Date: Mon, 20 Nov 2023 12:15:03 -0800 Subject: [PATCH] Generate mutable and shared datasec accessors MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit It's not particularly user friendly to only have exclusive shared datasec accessor variants available, as that can lead to unnecessary borrow conflicts. With this change we revamp the generation logic to generate shared and exclusive accessors when possible. Refs: #606 Signed-off-by: Daniel Müller --- examples/capable/src/main.rs | 6 +++--- examples/runqslower/src/main.rs | 6 +++--- examples/tproxy/src/main.rs | 6 +++--- libbpf-cargo/CHANGELOG.md | 2 ++ libbpf-cargo/src/gen/mod.rs | 37 ++++++++++++++++++++------------- libbpf-cargo/src/test.rs | 8 +++---- libbpf-rs/src/skeleton.rs | 22 +++++++++++++++----- 7 files changed, 55 insertions(+), 32 deletions(-) diff --git a/examples/capable/src/main.rs b/examples/capable/src/main.rs index e68ad5de..835398a9 100644 --- a/examples/capable/src/main.rs +++ b/examples/capable/src/main.rs @@ -186,9 +186,9 @@ fn main() -> Result<()> { let mut open_skel = skel_builder.open()?; //Pass configuration to BPF - open_skel.rodata().tool_config.tgid = opts.pid; //tgid in kernel is pid in userland - open_skel.rodata().tool_config.verbose = opts.verbose; - open_skel.rodata().tool_config.unique_type = opts.unique_type; + open_skel.rodata_mut().tool_config.tgid = opts.pid; //tgid in kernel is pid in userland + open_skel.rodata_mut().tool_config.verbose = opts.verbose; + open_skel.rodata_mut().tool_config.unique_type = opts.unique_type; let mut skel = open_skel.load()?; skel.attach()?; diff --git a/examples/runqslower/src/main.rs b/examples/runqslower/src/main.rs index 1ab237af..a1862f39 100644 --- a/examples/runqslower/src/main.rs +++ b/examples/runqslower/src/main.rs @@ -90,9 +90,9 @@ fn main() -> Result<()> { let mut open_skel = skel_builder.open()?; // Write arguments into prog - open_skel.rodata().min_us = opts.latency; - open_skel.rodata().targ_pid = opts.pid; - open_skel.rodata().targ_tgid = opts.tid; + open_skel.rodata_mut().min_us = opts.latency; + open_skel.rodata_mut().targ_pid = opts.pid; + open_skel.rodata_mut().targ_tgid = opts.tid; // Begin tracing let mut skel = open_skel.load()?; diff --git a/examples/tproxy/src/main.rs b/examples/tproxy/src/main.rs index 71b5ef7e..f1916da8 100644 --- a/examples/tproxy/src/main.rs +++ b/examples/tproxy/src/main.rs @@ -64,9 +64,9 @@ fn main() -> Result<()> { // Set constants let mut open_skel = skel_builder.open()?; - open_skel.rodata().target_port = opts.port.to_be(); - open_skel.rodata().proxy_addr = proxy_addr.to_be(); - open_skel.rodata().proxy_port = opts.proxy_port.to_be(); + open_skel.rodata_mut().target_port = opts.port.to_be(); + open_skel.rodata_mut().proxy_addr = proxy_addr.to_be(); + open_skel.rodata_mut().proxy_port = opts.proxy_port.to_be(); // Load into kernel let skel = open_skel.load()?; diff --git a/libbpf-cargo/CHANGELOG.md b/libbpf-cargo/CHANGELOG.md index afcf160e..cd782b2c 100644 --- a/libbpf-cargo/CHANGELOG.md +++ b/libbpf-cargo/CHANGELOG.md @@ -1,5 +1,7 @@ Unreleased ---------- +- Adjusted skeleton creation logic to generate shared and exclusive datasec + accessor functions - Removed `Error` enum in favor of `anyhow::Error` - Bumped minimum Rust version to `1.65` diff --git a/libbpf-cargo/src/gen/mod.rs b/libbpf-cargo/src/gen/mod.rs index f6bbd52d..d41fd3eb 100644 --- a/libbpf-cargo/src/gen/mod.rs +++ b/libbpf-cargo/src/gen/mod.rs @@ -530,24 +530,33 @@ fn gen_skel_datasec_getters( None => continue, }; let struct_name = format!("{obj_name}_{name}_types::{name}"); - let mutability = if loaded && map_is_readonly(map) { - "" + let immutable = loaded && map_is_readonly(map); + let mutabilities = if immutable { + [false].as_ref() } else { - "mut" + [false, true].as_ref() }; - write!( - skel, - r#" - pub fn {name}(&{mutability} self) -> &{mutability} {struct_name} {{ - unsafe {{ - std::mem::transmute::<*mut std::ffi::c_void, &{mutability} {struct_name}>( - self.skel_config.map_mmap_ptr({idx}).unwrap() - ) + for mutable in mutabilities { + let (ref_suffix, ptr_suffix, fn_suffix) = if *mutable { + ("mut", "mut", "_mut") + } else { + ("", "const", "") + }; + + write!( + skel, + r#" + pub fn {name}{fn_suffix}(&{ref_suffix} self) -> &{ref_suffix} {struct_name} {{ + unsafe {{ + std::mem::transmute::<*{ptr_suffix} std::ffi::c_void, &{ref_suffix} {struct_name}>( + self.skel_config.map_mmap_ptr{fn_suffix}({idx}).unwrap() + ) + }} }} - }} - "# - )?; + "# + )?; + } } Ok(()) diff --git a/libbpf-cargo/src/test.rs b/libbpf-cargo/src/test.rs index eb54eda0..bdf4dc90 100644 --- a/libbpf-cargo/src/test.rs +++ b/libbpf-cargo/src/test.rs @@ -653,17 +653,17 @@ fn test_skeleton_datasec() { .expect("failed to open skel"); // Check that we set rodata vars before load - open_skel.rodata().myconst = std::ptr::null_mut(); + open_skel.rodata_mut().myconst = std::ptr::null_mut(); // We can always set bss vars - open_skel.bss().myglobal = 42; + open_skel.bss_mut().myglobal = 42; let mut skel = open_skel .load() .expect("failed to load skel"); // We can always set bss vars - skel.bss().myglobal = 24; + skel.bss_mut().myglobal = 24; // Read only for rodata after load let _rodata: &prog_rodata_types::rodata = skel.rodata(); @@ -924,7 +924,7 @@ fn test_skeleton_builder_arrays_ptrs() { fn main() {{ let builder = ProgSkelBuilder::default(); - let mut open_skel = builder + let open_skel = builder .open() .expect("failed to open skel"); diff --git a/libbpf-rs/src/skeleton.rs b/libbpf-rs/src/skeleton.rs index 80d94608..9b89ea1f 100644 --- a/libbpf-rs/src/skeleton.rs +++ b/libbpf-rs/src/skeleton.rs @@ -18,6 +18,7 @@ use libbpf_sys::bpf_object_skeleton; use libbpf_sys::bpf_prog_skeleton; use libbpf_sys::bpf_program; +use crate::error::IntoError as _; use crate::libbpf_sys; use crate::util; use crate::Error; @@ -263,17 +264,28 @@ impl ObjectSkeletonConfig<'_> { /// `ObjectSkeletonConfigBuilder::map`. Index starts at 0. /// /// Warning: the returned pointer is only valid while the `ObjectSkeletonConfig` is alive. - pub fn map_mmap_ptr(&mut self, index: usize) -> Result<*mut c_void> { + pub fn map_mmap_ptr(&self, index: usize) -> Result<*const c_void> { if index >= self.maps.len() { return Err(Error::with_invalid_data(format!( "Invalid map index: {index}" ))); } - self.maps[index].mmaped.as_ref().map_or_else( - || Err(Error::with_invalid_data("Map does not have mmaped ptr")), - |p| Ok(**p), - ) + let p = self.maps[index] + .mmaped + .as_ref() + .ok_or_invalid_data(|| "Map does not have mmaped ptr")?; + Ok(**p) + } + + /// Returns the `mmaped` pointer for a map at the specified `index`. + /// + /// The index is determined by the order in which the map was passed to + /// `ObjectSkeletonConfigBuilder::map`. Index starts at 0. + /// + /// Warning: the returned pointer is only valid while the `ObjectSkeletonConfig` is alive. + pub fn map_mmap_ptr_mut(&mut self, index: usize) -> Result<*mut c_void> { + self.map_mmap_ptr(index).map(|p| p.cast_mut()) } /// Returns the link pointer for a prog at the specified `index`.