libbpf-cargo: Honor ObjectBuilder in skeleton infrastructure

With commit d105bf2 ("libbpf-cargo: Open skeleton without options
by default") we changed the default behavior of the generated
SkelBuilder to only use open options if explicitly provided to the newly
added open_opts() constructor. What wasn't handled correctly was the
existing options present in the obj_builder member.

Fix this by trying to be more clever and auto detect whether the
ObjectBuilder has had any changes over its default representation.

Closes: libbpf#1309
Reported-by: Francisco Javier Honduvilla Coto <[email protected]>
Signed-off-by: Daniel Müller <[email protected]>

Author: d-e-s-o

libbpf-cargo/CHANGELOG.md

@@ -1,6 +1,8 @@
 Unreleased
 ----------
 - Fixed Rust type generation for trailing bitfields in composite C types
+- Fixed handling of `XxxSkelBuilder::obj_builder` customizations when
+  using `open()` constructor
 - Allowlisted `libbpf-sys` `1.6.2`
 
 

libbpf-cargo/src/gen/mod.rs

@@ -860,6 +860,7 @@ fn gen_skel_contents(raw_obj_name: &str, obj_file_path: &Path) -> Result<String>
         use libbpf_rs::skel::SkelBuilder;
         use libbpf_rs::AsRawLibbpf as _;
         use libbpf_rs::MapCore as _;
+        use libbpf_rs::ObjectBuilder;
         "
     )?;
 
@@ -983,7 +984,15 @@ fn gen_skel_contents(raw_obj_name: &str, obj_file_path: &Path) -> Result<String>
                 self,
                 object: &'obj mut std::mem::MaybeUninit<libbpf_rs::OpenObject>,
             ) -> libbpf_rs::Result<Open{name}Skel<'obj>> {{
-                self.open_opts_impl(std::ptr::null(), object)
+                // Only produce a pointer to our custom open opts object
+                // if customizations have been made. This works around a
+                // bug in older versions of libbpf.
+                let opts = if self.obj_builder.ne(&ObjectBuilder::default()) {{
+                    self.obj_builder.as_libbpf_object().as_ptr().cast_const()
+                }} else {{
+                    std::ptr::null()
+                }};
+                self.open_opts_impl(opts, object)
             }}
 
             fn open_opts(

libbpf-rs/src/object.rs

@@ -1,12 +1,12 @@
 use core::ffi::c_void;
+use std::cell::OnceCell;
 use std::ffi::CStr;
 use std::ffi::CString;
 use std::ffi::OsStr;
 use std::mem;
 use std::os::unix::ffi::OsStrExt as _;
 use std::path::Path;
 use std::ptr;
-use std::ptr::addr_of;
 use std::ptr::NonNull;
 
 use crate::map::map_fd;
@@ -107,36 +107,55 @@ pub struct ObjectBuilder {
     name: Option<CString>,
     pin_root_path: Option<CString>,
 
-    opts: libbpf_sys::bpf_object_open_opts,
+    opts: OnceCell<libbpf_sys::bpf_object_open_opts>,
 }
 
 impl Default for ObjectBuilder {
     fn default() -> Self {
-        let opts = libbpf_sys::bpf_object_open_opts {
-            sz: mem::size_of::<libbpf_sys::bpf_object_open_opts>() as libbpf_sys::size_t,
-            object_name: ptr::null(),
-            relaxed_maps: false,
-            pin_root_path: ptr::null(),
-            kconfig: ptr::null(),
-            btf_custom_path: ptr::null(),
-            kernel_log_buf: ptr::null_mut(),
-            kernel_log_size: 0,
-            kernel_log_level: 0,
-            ..Default::default()
-        };
         Self {
             name: None,
             pin_root_path: None,
-            opts,
+            opts: OnceCell::new(),
         }
     }
 }
 
+impl PartialEq for ObjectBuilder {
+    fn eq(&self, other: &Self) -> bool {
+        let Self {
+            name,
+            pin_root_path,
+            opts,
+        } = self;
+
+        // `bpf_object_open_opts` doesn't implement `PartialEq` and we
+        // don't want to manually compare fields. Just render our
+        // objects "uncomparable" if it is present.
+        opts.get().is_none()
+            && other.opts.get().is_none()
+            && name == &other.name
+            && pin_root_path == &other.pin_root_path
+    }
+}
+
 impl ObjectBuilder {
+    fn opts(&self) -> &libbpf_sys::bpf_object_open_opts {
+        self.opts.get_or_init(|| libbpf_sys::bpf_object_open_opts {
+            sz: mem::size_of::<libbpf_sys::bpf_object_open_opts>() as libbpf_sys::size_t,
+            ..Default::default()
+        })
+    }
+
+    fn opts_mut(&mut self) -> &mut libbpf_sys::bpf_object_open_opts {
+        let _opts = self.opts();
+        // SANITY: We just made sure to initialize the object above.
+        self.opts.get_mut().unwrap()
+    }
+
     /// Override the generated name that would have been inferred from the constructor.
     pub fn name<T: AsRef<str>>(&mut self, name: T) -> Result<&mut Self> {
         self.name = Some(util::str_to_cstring(name.as_ref())?);
-        self.opts.object_name = self.name.as_ref().map_or(ptr::null(), |p| p.as_ptr());
+        self.opts_mut().object_name = self.name.as_ref().map_or(ptr::null(), |p| p.as_ptr());
         Ok(self)
     }
 
@@ -145,7 +164,7 @@ impl ObjectBuilder {
     /// By default, this is NULL which bpf translates to /sys/fs/bpf
     pub fn pin_root_path<T: AsRef<Path>>(&mut self, path: T) -> Result<&mut Self> {
         self.pin_root_path = Some(util::path_to_cstring(path)?);
-        self.opts.pin_root_path = self
+        self.opts_mut().pin_root_path = self
             .pin_root_path
             .as_ref()
             .map_or(ptr::null(), |p| p.as_ptr());
@@ -154,7 +173,7 @@ impl ObjectBuilder {
 
     /// Option to parse map definitions non-strictly, allowing extra attributes/data
     pub fn relaxed_maps(&mut self, relaxed_maps: bool) -> &mut Self {
-        self.opts.relaxed_maps = relaxed_maps;
+        self.opts_mut().relaxed_maps = relaxed_maps;
         self
     }
 
@@ -208,7 +227,7 @@ impl AsRawLibbpf for ObjectBuilder {
     /// Retrieve the underlying [`libbpf_sys::bpf_object_open_opts`].
     fn as_libbpf_object(&self) -> NonNull<Self::LibbpfType> {
         // SAFETY: A reference is always a valid pointer.
-        unsafe { NonNull::new_unchecked(addr_of!(self.opts).cast_mut()) }
+        unsafe { NonNull::new_unchecked(ptr::from_ref(self.opts()).cast_mut()) }
     }
 }