Browse Source

Merge pull request #378 from dave-tucker/pin-fixes-again

aya: More pinning fixes
Dave Tucker 2 years ago
parent
commit
98e25ca5e6

+ 3 - 3
aya-tool/src/generate.rs

@@ -62,7 +62,7 @@ pub fn generate<T: AsRef<str>>(
     let (c_header, name) = match &input_file {
         InputFile::Btf(path) => (c_header_from_btf(path)?, "kernel_types.h"),
         InputFile::Header(header) => (
-            fs::read_to_string(&header).map_err(Error::ReadHeaderFile)?,
+            fs::read_to_string(header).map_err(Error::ReadHeaderFile)?,
             header.file_name().unwrap().to_str().unwrap(),
         ),
     };
@@ -92,9 +92,9 @@ pub fn generate<T: AsRef<str>>(
 
 fn c_header_from_btf(path: &Path) -> Result<String, Error> {
     let output = Command::new("bpftool")
-        .args(&["btf", "dump", "file"])
+        .args(["btf", "dump", "file"])
         .arg(path)
-        .args(&["format", "c"])
+        .args(["format", "c"])
         .output()
         .map_err(Error::BpfTool)?;
 

+ 4 - 4
aya/src/programs/extension.rs

@@ -148,8 +148,8 @@ impl Extension {
 fn get_btf_info(prog_fd: i32, func_name: &str) -> Result<(RawFd, u32), ProgramError> {
     // retrieve program information
     let info =
-        sys::bpf_obj_get_info_by_fd(prog_fd).map_err(|io_error| ProgramError::SyscallError {
-            call: "bpf_obj_get_info_by_fd".to_owned(),
+        sys::bpf_prog_get_info_by_fd(prog_fd).map_err(|io_error| ProgramError::SyscallError {
+            call: "bpf_prog_get_info_by_fd".to_owned(),
             io_error,
         })?;
 
@@ -175,7 +175,7 @@ fn get_btf_info(prog_fd: i32, func_name: &str) -> Result<(RawFd, u32), ProgramEr
                 let btf_info =
                     sys::btf_obj_get_info_by_fd(btf_fd, &mut buf).map_err(|io_error| {
                         ProgramError::SyscallError {
-                            call: "bpf_obj_get_info_by_fd".to_owned(),
+                            call: "btf_obj_get_info_by_fd".to_owned(),
                             io_error,
                         }
                     })?;
@@ -185,7 +185,7 @@ fn get_btf_info(prog_fd: i32, func_name: &str) -> Result<(RawFd, u32), ProgramEr
             }
         }
         Err(io_error) => Err(ProgramError::SyscallError {
-            call: "bpf_obj_get_info_by_fd".to_owned(),
+            call: "btf_obj_get_info_by_fd".to_owned(),
             io_error,
         }),
     }?;

+ 30 - 1
aya/src/programs/links.rs

@@ -14,7 +14,7 @@ use crate::{
     generated::bpf_attach_type,
     pin::PinError,
     programs::ProgramError,
-    sys::{bpf_pin_object, bpf_prog_detach},
+    sys::{bpf_get_object, bpf_pin_object, bpf_prog_detach},
 };
 
 /// A Link.
@@ -162,6 +162,12 @@ impl Drop for FdLink {
     }
 }
 
+impl From<PinnedLink> for FdLink {
+    fn from(p: PinnedLink) -> Self {
+        p.inner
+    }
+}
+
 /// A pinned file descriptor link.
 ///
 /// This link has been pinned to the BPF filesystem. On drop, the file descriptor that backs
@@ -181,6 +187,18 @@ impl PinnedLink {
         }
     }
 
+    /// Creates a [`PinnedLink`] from a valid path on bpffs.
+    pub fn from_path<P: AsRef<Path>>(path: P) -> Result<Self, LinkError> {
+        let path_string = CString::new(path.as_ref().to_string_lossy().to_string()).unwrap();
+        let fd =
+            bpf_get_object(&path_string).map_err(|(code, io_error)| LinkError::SyscallError {
+                call: "BPF_OBJ_GET".to_string(),
+                code,
+                io_error,
+            })? as RawFd;
+        Ok(PinnedLink::new(path.as_ref().to_path_buf(), fd))
+    }
+
     /// Removes the pinned link from the filesystem and returns an [`FdLink`].
     pub fn unpin(self) -> Result<FdLink, io::Error> {
         std::fs::remove_file(self.path)?;
@@ -272,6 +290,17 @@ pub enum LinkError {
     /// Invalid link.
     #[error("Invalid link")]
     InvalidLink,
+    /// Syscall failed.
+    #[error("the `{call}` syscall failed with code {code}")]
+    SyscallError {
+        /// Syscall Name.
+        call: String,
+        /// Error code.
+        code: libc::c_long,
+        #[source]
+        /// Original io::Error.
+        io_error: io::Error,
+    },
 }
 
 #[cfg(test)]

+ 3 - 3
aya/src/programs/lirc_mode2.rs

@@ -4,7 +4,7 @@ use std::os::unix::prelude::{AsRawFd, RawFd};
 use crate::{
     generated::{bpf_attach_type::BPF_LIRC_MODE2, bpf_prog_type::BPF_PROG_TYPE_LIRC_MODE2},
     programs::{load_program, query, Link, ProgramData, ProgramError, ProgramInfo},
-    sys::{bpf_obj_get_info_by_fd, bpf_prog_attach, bpf_prog_detach, bpf_prog_get_fd_by_id},
+    sys::{bpf_prog_attach, bpf_prog_detach, bpf_prog_get_fd_by_id, bpf_prog_get_info_by_fd},
 };
 
 use libc::{close, dup};
@@ -132,10 +132,10 @@ impl LircLink {
 
     /// Get ProgramInfo from this link
     pub fn info(&self) -> Result<ProgramInfo, ProgramError> {
-        match bpf_obj_get_info_by_fd(self.prog_fd) {
+        match bpf_prog_get_info_by_fd(self.prog_fd) {
             Ok(info) => Ok(ProgramInfo(info)),
             Err(io_error) => Err(ProgramError::SyscallError {
-                call: "bpf_obj_get_info_by_fd".to_owned(),
+                call: "bpf_prog_get_info_by_fd".to_owned(),
                 io_error,
             }),
         }

+ 4 - 4
aya/src/programs/mod.rs

@@ -106,8 +106,8 @@ use crate::{
     obj::{self, btf::BtfError, Function, KernelVersion},
     pin::PinError,
     sys::{
-        bpf_get_object, bpf_load_program, bpf_obj_get_info_by_fd, bpf_pin_object,
-        bpf_prog_get_fd_by_id, bpf_prog_query, retry_with_verifier_logs, BpfLoadProgramAttrs,
+        bpf_get_object, bpf_load_program, bpf_pin_object, bpf_prog_get_fd_by_id,
+        bpf_prog_get_info_by_fd, bpf_prog_query, retry_with_verifier_logs, BpfLoadProgramAttrs,
     },
     util::VerifierLog,
 };
@@ -826,8 +826,8 @@ impl ProgramInfo {
                 io_error,
             })? as RawFd;
 
-        let info = bpf_obj_get_info_by_fd(fd).map_err(|io_error| ProgramError::SyscallError {
-            call: "bpf_obj_get_info_by_fd".to_owned(),
+        let info = bpf_prog_get_info_by_fd(fd).map_err(|io_error| ProgramError::SyscallError {
+            call: "bpf_prog_get_info_by_fd".to_owned(),
             io_error,
         })?;
         unsafe {

+ 24 - 1
aya/src/programs/xdp.rs

@@ -7,6 +7,7 @@ use thiserror::Error;
 use crate::{
     generated::{
         bpf_attach_type::{self, BPF_XDP},
+        bpf_link_type,
         bpf_prog_type::BPF_PROG_TYPE_XDP,
         XDP_FLAGS_DRV_MODE, XDP_FLAGS_HW_MODE, XDP_FLAGS_REPLACE, XDP_FLAGS_SKB_MODE,
         XDP_FLAGS_UPDATE_IF_NOEXIST,
@@ -14,7 +15,10 @@ use crate::{
     programs::{
         define_link_wrapper, load_program, FdLink, Link, LinkError, ProgramData, ProgramError,
     },
-    sys::{bpf_link_create, bpf_link_update, kernel_version, netlink_set_xdp_fd},
+    sys::{
+        bpf_link_create, bpf_link_get_info_by_fd, bpf_link_update, kernel_version,
+        netlink_set_xdp_fd,
+    },
 };
 
 /// The type returned when attaching an [`Xdp`] program fails on kernels `< 5.9`.
@@ -250,6 +254,25 @@ impl TryFrom<XdpLink> for FdLink {
     }
 }
 
+impl TryFrom<FdLink> for XdpLink {
+    type Error = LinkError;
+
+    fn try_from(fd_link: FdLink) -> Result<Self, Self::Error> {
+        // unwrap of fd_link.fd will not panic since it's only None when being dropped.
+        let info = bpf_link_get_info_by_fd(fd_link.fd.unwrap()).map_err(|io_error| {
+            LinkError::SyscallError {
+                call: "BPF_OBJ_GET_INFO_BY_FD".to_string(),
+                code: 0,
+                io_error,
+            }
+        })?;
+        if info.type_ == (bpf_link_type::BPF_LINK_TYPE_XDP as u32) {
+            return Ok(XdpLink(XdpLinkInner::FdLink(fd_link)));
+        }
+        Err(LinkError::InvalidLink)
+    }
+}
+
 define_link_wrapper!(
     /// The link used by [Xdp] programs.
     XdpLink,

+ 26 - 17
aya/src/sys/bpf.rs

@@ -1,16 +1,3 @@
-use crate::{
-    generated::BPF_F_REPLACE,
-    obj::{
-        btf::{
-            BtfParam, BtfType, DataSec, DataSecEntry, DeclTag, Float, Func, FuncLinkage, FuncProto,
-            Int, IntEncoding, Ptr, TypeTag, Var, VarLinkage,
-        },
-        copy_instructions,
-    },
-    Btf,
-};
-use libc::{c_char, c_long, close, ENOENT, ENOSPC};
-
 use std::{
     cmp::{self, min},
     ffi::{CStr, CString},
@@ -20,18 +7,25 @@ use std::{
     slice,
 };
 
+use libc::{c_char, c_long, close, ENOENT, ENOSPC};
+
 use crate::{
     generated::{
-        bpf_attach_type, bpf_attr, bpf_btf_info, bpf_cmd, bpf_insn, bpf_prog_info, bpf_prog_type,
+        bpf_attach_type, bpf_attr, bpf_btf_info, bpf_cmd, bpf_insn, bpf_link_info, bpf_prog_info,
+        bpf_prog_type, BPF_F_REPLACE,
     },
     maps::PerCpuValues,
     obj::{
         self,
-        btf::{FuncSecInfo, LineSecInfo},
+        btf::{
+            BtfParam, BtfType, DataSec, DataSecEntry, DeclTag, Float, Func, FuncLinkage, FuncProto,
+            FuncSecInfo, Int, IntEncoding, LineSecInfo, Ptr, TypeTag, Var, VarLinkage,
+        },
+        copy_instructions,
     },
     sys::{kernel_version, syscall, SysResult, Syscall},
     util::VerifierLog,
-    Pod, BPF_OBJ_NAME_LEN,
+    Btf, Pod, BPF_OBJ_NAME_LEN,
 };
 
 pub(crate) fn bpf_create_map(name: &CStr, def: &obj::Map, btf_fd: Option<RawFd>) -> SysResult {
@@ -442,7 +436,7 @@ pub(crate) fn bpf_prog_get_fd_by_id(prog_id: u32) -> Result<RawFd, io::Error> {
     }
 }
 
-pub(crate) fn bpf_obj_get_info_by_fd(prog_fd: RawFd) -> Result<bpf_prog_info, io::Error> {
+pub(crate) fn bpf_prog_get_info_by_fd(prog_fd: RawFd) -> Result<bpf_prog_info, io::Error> {
     let mut attr = unsafe { mem::zeroed::<bpf_attr>() };
     // info gets entirely populated by the kernel
     let info = unsafe { MaybeUninit::zeroed().assume_init() };
@@ -457,6 +451,21 @@ pub(crate) fn bpf_obj_get_info_by_fd(prog_fd: RawFd) -> Result<bpf_prog_info, io
     }
 }
 
+pub(crate) fn bpf_link_get_info_by_fd(link_fd: RawFd) -> Result<bpf_link_info, io::Error> {
+    let mut attr = unsafe { mem::zeroed::<bpf_attr>() };
+    // info gets entirely populated by the kernel
+    let info = unsafe { MaybeUninit::zeroed().assume_init() };
+
+    attr.info.bpf_fd = link_fd as u32;
+    attr.info.info = &info as *const _ as u64;
+    attr.info.info_len = mem::size_of::<bpf_link_info>() as u32;
+
+    match sys_bpf(bpf_cmd::BPF_OBJ_GET_INFO_BY_FD, &attr) {
+        Ok(_) => Ok(info),
+        Err((_, err)) => Err(err),
+    }
+}
+
 pub(crate) fn btf_obj_get_info_by_fd(
     prog_fd: RawFd,
     buf: &mut [u8],

+ 52 - 16
test/integration-test/src/tests/load.rs

@@ -3,7 +3,10 @@ use std::{process::Command, thread, time};
 use aya::{
     include_bytes_aligned,
     maps::{Array, MapRefMut},
-    programs::{links::FdLink, TracePoint, Xdp, XdpFlags},
+    programs::{
+        links::{FdLink, PinnedLink},
+        TracePoint, Xdp, XdpFlags,
+    },
     Bpf,
 };
 
@@ -60,14 +63,14 @@ fn multiple_btf_maps() -> anyhow::Result<()> {
     Ok(())
 }
 
-fn is_loaded() -> bool {
+fn is_loaded(name: &str) -> bool {
     let output = Command::new("bpftool").args(["prog"]).output().unwrap();
     let stdout = String::from_utf8(output.stdout).unwrap();
-    stdout.contains("test_unload")
+    stdout.contains(name)
 }
 
-fn assert_loaded(loaded: bool) {
-    let state = is_loaded();
+fn assert_loaded(name: &str, loaded: bool) {
+    let state = is_loaded(name);
     if state == loaded {
         return;
     }
@@ -84,19 +87,19 @@ fn unload() -> anyhow::Result<()> {
     {
         let _link_owned = prog.take_link(link);
         prog.unload().unwrap();
-        assert_loaded(true);
+        assert_loaded("test_unload", true);
     };
 
-    assert_loaded(false);
+    assert_loaded("test_unload", false);
     prog.load().unwrap();
 
-    assert_loaded(true);
+    assert_loaded("test_unload", true);
     prog.attach("lo", XdpFlags::default()).unwrap();
 
-    assert_loaded(true);
+    assert_loaded("test_unload", true);
     prog.unload().unwrap();
 
-    assert_loaded(false);
+    assert_loaded("test_unload", false);
     Ok(())
 }
 
@@ -108,24 +111,57 @@ fn pin_link() -> anyhow::Result<()> {
     prog.load().unwrap();
     let link_id = prog.attach("lo", XdpFlags::default()).unwrap();
     let link = prog.take_link(link_id)?;
-    assert_loaded(true);
+    assert_loaded("test_unload", true);
 
     let fd_link: FdLink = link.try_into()?;
     let pinned = fd_link.pin("/sys/fs/bpf/aya-xdp-test-lo")?;
 
     // because of the pin, the program is still attached
     prog.unload()?;
-    assert_loaded(true);
+    assert_loaded("test_unload", true);
 
     // delete the pin, but the program is still attached
     let new_link = pinned.unpin()?;
-    println!("third assert");
-    assert_loaded(true);
+    assert_loaded("test_unload", true);
 
     // finally when new_link is dropped we're detached
     drop(new_link);
-    println!("final assert");
-    assert_loaded(false);
+    assert_loaded("test_unload", false);
+
+    Ok(())
+}
+
+#[integration_test]
+fn pin_lifecycle() -> anyhow::Result<()> {
+    let bytes = include_bytes_aligned!("../../../../target/bpfel-unknown-none/debug/pass");
+
+    // 1. Load Program and Pin
+    {
+        let mut bpf = Bpf::load(bytes)?;
+        let prog: &mut Xdp = bpf.program_mut("pass").unwrap().try_into().unwrap();
+        prog.load().unwrap();
+        let link_id = prog.attach("lo", XdpFlags::default()).unwrap();
+        let link = prog.take_link(link_id)?;
+        let fd_link: FdLink = link.try_into()?;
+        fd_link.pin("/sys/fs/bpf/aya-xdp-test-lo")?;
+    }
+
+    // should still be loaded since link was pinned
+    assert_loaded("pass", true);
+
+    // 2. Load a new version of the program, unpin link, and atomically replace old program
+    {
+        let mut bpf = Bpf::load(bytes)?;
+        let prog: &mut Xdp = bpf.program_mut("pass").unwrap().try_into().unwrap();
+        prog.load().unwrap();
+
+        let link = PinnedLink::from_path("/sys/fs/bpf/aya-xdp-test-lo")?.unpin()?;
+        prog.attach_to_link(link.try_into()?)?;
+        assert_loaded("pass", true);
+    }
+
+    // program should be unloaded
+    assert_loaded("pass", false);
 
     Ok(())
 }