Browse Source

Hide details of VerifierLog

This type is really only used by one function.
Tamir Duberstein 1 year ago
parent
commit
6b94b20
5 changed files with 49 additions and 112 deletions
  1. 1 1
      aya-obj/src/btf/btf.rs
  2. 7 15
      aya/src/bpf.rs
  3. 7 15
      aya/src/programs/mod.rs
  4. 33 29
      aya/src/sys/bpf.rs
  5. 1 52
      aya/src/util.rs

+ 1 - 1
aya-obj/src/btf/btf.rs

@@ -135,7 +135,7 @@ pub enum BtfError {
         #[source]
         io_error: std::io::Error,
         /// The error log produced by the kernel verifier.
-        verifier_log: String,
+        verifier_log: Cow<'static, str>,
     },
 
     /// offset not found for symbol

+ 7 - 15
aya/src/bpf.rs

@@ -39,7 +39,7 @@ use crate::{
         is_btf_supported, is_btf_type_tag_supported, is_perf_link_supported,
         is_probe_read_kernel_supported, is_prog_name_supported, retry_with_verifier_logs,
     },
-    util::{bytes_of, bytes_of_slice, possible_cpus, VerifierLog, POSSIBLE_CPUS},
+    util::{bytes_of, bytes_of_slice, possible_cpus, POSSIBLE_CPUS},
 };
 
 pub(crate) const BPF_OBJ_NAME_LEN: usize = 16;
@@ -907,22 +907,14 @@ pub enum BpfError {
 }
 
 fn load_btf(raw_btf: Vec<u8>) -> Result<RawFd, BtfError> {
-    let mut logger = VerifierLog::new();
-    let ret = retry_with_verifier_logs(10, &mut logger, |logger| {
-        bpf_load_btf(raw_btf.as_slice(), logger)
-    });
+    let (ret, verifier_log) =
+        retry_with_verifier_logs(10, |logger| bpf_load_btf(raw_btf.as_slice(), logger));
     match ret {
         Ok(fd) => Ok(fd as RawFd),
-        Err((_, io_error)) => {
-            logger.truncate();
-            Err(BtfError::LoadError {
-                io_error,
-                verifier_log: logger
-                    .as_c_str()
-                    .map(|s| s.to_string_lossy().to_string())
-                    .unwrap_or_else(|| "[none]".to_owned()),
-            })
-        }
+        Err((_, io_error)) => Err(BtfError::LoadError {
+            io_error,
+            verifier_log,
+        }),
     }
 }
 

+ 7 - 15
aya/src/programs/mod.rs

@@ -67,6 +67,7 @@ pub mod xdp;
 use libc::ENOSPC;
 use procfs::KernelVersion;
 use std::{
+    borrow::Cow,
     ffi::CString,
     io,
     os::unix::io::{AsRawFd, RawFd},
@@ -113,7 +114,6 @@ use crate::{
         bpf_prog_get_fd_by_id, bpf_prog_get_info_by_fd, bpf_prog_get_next_id, bpf_prog_query,
         retry_with_verifier_logs, BpfLoadProgramAttrs,
     },
-    util::VerifierLog,
 };
 
 /// Error type returned when working with programs.
@@ -142,7 +142,7 @@ pub enum ProgramError {
         #[source]
         io_error: io::Error,
         /// The error log produced by the kernel verifier.
-        verifier_log: String,
+        verifier_log: Cow<'static, str>,
     },
 
     /// A syscall failed.
@@ -583,8 +583,6 @@ fn load_program<T: Link>(
         (u32::from(major) << 16) + (u32::from(minor) << 8) + u32::from(patch)
     });
 
-    let mut logger = VerifierLog::new();
-
     let prog_name = if let Some(name) = &data.name {
         let mut name = name.clone();
         if name.len() > 15 {
@@ -616,7 +614,7 @@ fn load_program<T: Link>(
     };
 
     let verifier_log_level = data.verifier_log_level;
-    let ret = retry_with_verifier_logs(10, &mut logger, |logger| {
+    let (ret, verifier_log) = retry_with_verifier_logs(10, |logger| {
         bpf_load_program(&attr, logger, verifier_log_level)
     });
 
@@ -625,16 +623,10 @@ fn load_program<T: Link>(
             *fd = Some(prog_fd as RawFd);
             Ok(())
         }
-        Err((_, io_error)) => {
-            logger.truncate();
-            return Err(ProgramError::LoadError {
-                io_error,
-                verifier_log: logger
-                    .as_c_str()
-                    .map(|s| s.to_string_lossy().to_string())
-                    .unwrap_or_else(|| "[none]".to_owned()),
-            });
-        }
+        Err((_, io_error)) => Err(ProgramError::LoadError {
+            io_error,
+            verifier_log,
+        }),
     }
 }
 

+ 33 - 29
aya/src/sys/bpf.rs

@@ -1,4 +1,5 @@
 use std::{
+    borrow::Cow,
     cmp::{self, min},
     ffi::{CStr, CString},
     io,
@@ -29,7 +30,6 @@ use crate::{
         copy_instructions,
     },
     sys::{syscall, SysResult, Syscall},
-    util::VerifierLog,
     Btf, Pod, BPF_OBJ_NAME_LEN,
 };
 
@@ -129,7 +129,7 @@ pub(crate) struct BpfLoadProgramAttrs<'a> {
 
 pub(crate) fn bpf_load_program(
     aya_attr: &BpfLoadProgramAttrs,
-    logger: &mut VerifierLog,
+    log_buf: &mut [u8],
     verifier_log_level: u32,
 ) -> SysResult {
     let mut attr = unsafe { mem::zeroed::<bpf_attr>() };
@@ -174,11 +174,10 @@ pub(crate) fn bpf_load_program(
             u.func_info_rec_size = aya_attr.func_info_rec_size as u32;
         }
     }
-    let log_buf = logger.buf();
-    if log_buf.capacity() > 0 {
+    if !log_buf.is_empty() {
         u.log_level = verifier_log_level;
         u.log_buf = log_buf.as_mut_ptr() as u64;
-        u.log_size = log_buf.capacity() as u32;
+        u.log_size = log_buf.len() as u32;
     }
     if let Some(v) = aya_attr.attach_btf_obj_fd {
         u.__bindgen_anon_1.attach_btf_obj_fd = v;
@@ -549,16 +548,15 @@ pub(crate) fn bpf_raw_tracepoint_open(name: Option<&CStr>, prog_fd: RawFd) -> Sy
     sys_bpf(bpf_cmd::BPF_RAW_TRACEPOINT_OPEN, &attr)
 }
 
-pub(crate) fn bpf_load_btf(raw_btf: &[u8], log: &mut VerifierLog) -> SysResult {
+pub(crate) fn bpf_load_btf(raw_btf: &[u8], log_buf: &mut [u8]) -> SysResult {
     let mut attr = unsafe { mem::zeroed::<bpf_attr>() };
     let u = unsafe { &mut attr.__bindgen_anon_7 };
     u.btf = raw_btf.as_ptr() as *const _ as u64;
     u.btf_size = mem::size_of_val(raw_btf) as u32;
-    let log_buf = log.buf();
-    if log_buf.capacity() > 0 {
+    if !log_buf.is_empty() {
         u.btf_log_level = 1;
         u.btf_log_buf = log_buf.as_mut_ptr() as u64;
-        u.btf_log_size = log_buf.capacity() as u32;
+        u.btf_log_size = log_buf.len() as u32;
     }
     sys_bpf(bpf_cmd::BPF_BTF_LOAD, &attr)
 }
@@ -993,35 +991,41 @@ pub(crate) fn bpf_prog_get_next_id(id: u32) -> Result<Option<u32>, (c_long, io::
 
 pub(crate) fn retry_with_verifier_logs<F>(
     max_retries: usize,
-    log: &mut VerifierLog,
     f: F,
-) -> SysResult
+) -> (SysResult, Cow<'static, str>)
 where
-    F: Fn(&mut VerifierLog) -> SysResult,
+    F: Fn(&mut [u8]) -> SysResult,
 {
-    // 1. Try the syscall
-    let ret = f(log);
-    if ret.is_ok() {
-        return ret;
-    }
+    const MIN_LOG_BUF_SIZE: usize = 1024 * 10;
+    const MAX_LOG_BUF_SIZE: usize = (std::u32::MAX >> 8) as usize;
 
-    // 2. Grow the log buffer so we can capture verifier output
-    //    Retry this up to max_retries times
-    log.grow();
+    let mut log_buf = Vec::new();
     let mut retries = 0;
-
     loop {
-        let ret = f(log);
-        match ret {
-            Err((v, io_error)) if retries == 0 || io_error.raw_os_error() == Some(ENOSPC) => {
-                if retries == max_retries {
-                    return Err((v, io_error));
+        let ret = f(log_buf.as_mut_slice());
+        if retries != max_retries {
+            if let Err((_, io_error)) = &ret {
+                if retries == 0 || io_error.raw_os_error() == Some(ENOSPC) {
+                    let len = (log_buf.capacity() * 10).clamp(MIN_LOG_BUF_SIZE, MAX_LOG_BUF_SIZE);
+                    log_buf.resize(len, 0);
+                    if let Some(first) = log_buf.first_mut() {
+                        *first = 0;
+                    }
+                    retries += 1;
+                    continue;
                 }
-                retries += 1;
-                log.grow();
             }
-            r => return r,
         }
+        if let Some(pos) = log_buf.iter().position(|b| *b == 0) {
+            log_buf.truncate(pos);
+        }
+        let log_buf = if log_buf.is_empty() {
+            "none".into()
+        } else {
+            String::from_utf8(log_buf).unwrap().into()
+        };
+
+        break (ret, log_buf);
     }
 }
 

+ 1 - 52
aya/src/util.rs

@@ -1,7 +1,7 @@
 //! Utility functions.
 use std::{
     collections::BTreeMap,
-    ffi::{CStr, CString},
+    ffi::CString,
     fs::{self, File},
     io::{self, BufReader},
     mem, slice,
@@ -200,57 +200,6 @@ pub(crate) fn bytes_of_slice<T: Pod>(val: &[T]) -> &[u8] {
     unsafe { slice::from_raw_parts(val.as_ptr().cast(), size) }
 }
 
-const MIN_LOG_BUF_SIZE: usize = 1024 * 10;
-const MAX_LOG_BUF_SIZE: usize = (std::u32::MAX >> 8) as usize;
-
-pub(crate) struct VerifierLog {
-    buf: Vec<u8>,
-}
-
-impl VerifierLog {
-    pub(crate) fn new() -> VerifierLog {
-        VerifierLog { buf: Vec::new() }
-    }
-
-    pub(crate) fn buf(&mut self) -> &mut Vec<u8> {
-        &mut self.buf
-    }
-
-    pub(crate) fn grow(&mut self) {
-        let len = (self.buf.capacity() * 10).clamp(MIN_LOG_BUF_SIZE, MAX_LOG_BUF_SIZE);
-        self.buf.resize(len, 0);
-        self.reset();
-    }
-
-    pub(crate) fn reset(&mut self) {
-        if !self.buf.is_empty() {
-            self.buf[0] = 0;
-        }
-    }
-
-    pub(crate) fn truncate(&mut self) {
-        if self.buf.is_empty() {
-            return;
-        }
-
-        let pos = self
-            .buf
-            .iter()
-            .position(|b| *b == 0)
-            .unwrap_or(self.buf.len() - 1);
-        self.buf[pos] = 0;
-        self.buf.truncate(pos + 1);
-    }
-
-    pub(crate) fn as_c_str(&self) -> Option<&CStr> {
-        if self.buf.is_empty() {
-            None
-        } else {
-            Some(CStr::from_bytes_with_nul(&self.buf).unwrap())
-        }
-    }
-}
-
 #[cfg(test)]
 mod tests {
     use super::*;