aya: Fix BTF verifier output

Currently errors can occur if the verifier output is > buffer as we get
ENOMEM. We should only provide a log_buf if initial load failed, then
retry up to 10 times to get full verifier output.

To DRY this logic it has been moved to a function so its shared with
program loading

Signed-off-by: Dave Tucker <dave@dtucker.co.uk>

one verifier loop to rule them all

Signed-off-by: Dave Tucker <dave@dtucker.co.uk>

aya/src/bpf.rs

@@ -30,6 +30,7 @@ use crate::{
         bpf_load_btf, bpf_map_freeze, bpf_map_update_elem_ptr, is_btf_datasec_supported,
         is_btf_decl_tag_supported, is_btf_float_supported, is_btf_func_global_supported,
         is_btf_func_supported, is_btf_supported, is_btf_type_tag_supported, is_prog_name_supported,
+        retry_with_verifier_logs,
     },
     util::{bytes_of, possible_cpus, VerifierLog, POSSIBLE_CPUS},
 };
@@ -314,22 +315,8 @@ impl<'a> BpfLoader<'a> {
 
                 // load btf to the kernel
                 let raw_btf = btf.to_bytes();
-                let mut log_buf = VerifierLog::new();
-                log_buf.grow();
-                let ret = bpf_load_btf(raw_btf.as_slice(), &mut log_buf);
-                match ret {
-                    Ok(fd) => Some(fd),
-                    Err(io_error) => {
-                        log_buf.truncate();
-                        return Err(BpfError::BtfError(BtfError::LoadError {
-                            io_error,
-                            verifier_log: log_buf
-                                .as_c_str()
-                                .map(|s| s.to_string_lossy().to_string())
-                                .unwrap_or_else(|| "[none]".to_owned()),
-                        }));
-                    }
-                }
+                let fd = load_btf(raw_btf)?;
+                Some(fd)
             } else {
                 None
             }
@@ -768,3 +755,23 @@ pub enum BpfError {
     /// A program error
     ProgramError(#[from] ProgramError),
 }
+
+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)
+    });
+    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()),
+            })
+        }
+    }
+}

aya/src/programs/mod.rs

@@ -97,7 +97,7 @@ use crate::{
     obj::{self, btf::BtfError, Function, KernelVersion},
     sys::{
         bpf_get_object, bpf_load_program, bpf_obj_get_info_by_fd, bpf_pin_object, bpf_prog_detach,
-        bpf_prog_get_fd_by_id, bpf_prog_query, BpfLoadProgramAttrs,
+        bpf_prog_get_fd_by_id, bpf_prog_query, retry_with_verifier_logs, BpfLoadProgramAttrs,
     },
     util::VerifierLog,
 };
@@ -412,9 +412,7 @@ fn load_program(prog_type: bpf_prog_type, data: &mut ProgramData) -> Result<(),
         _ => (*kernel_version).into(),
     };
 
-    let mut log_buf = VerifierLog::new();
-    let mut retries = 0;
-    let mut ret;
+    let mut logger = VerifierLog::new();
 
     let prog_name = if let Some(name) = &data.name {
         let mut name = name.clone();
@@ -428,53 +426,40 @@ fn load_program(prog_type: bpf_prog_type, data: &mut ProgramData) -> Result<(),
         None
     };
 
-    loop {
-        let attr = BpfLoadProgramAttrs {
-            name: prog_name.clone(),
-            ty: prog_type,
-            insns: instructions,
-            license,
-            kernel_version: target_kernel_version,
-            expected_attach_type: data.expected_attach_type,
-            prog_btf_fd: data.btf_fd,
-            attach_btf_obj_fd: data.attach_btf_obj_fd,
-            attach_btf_id: data.attach_btf_id,
-            attach_prog_fd: data.attach_prog_fd,
-            log: &mut log_buf,
-            func_info_rec_size: *func_info_rec_size,
-            func_info: func_info.clone(),
-            line_info_rec_size: *line_info_rec_size,
-            line_info: line_info.clone(),
-        };
-        ret = bpf_load_program(attr);
-        match &ret {
-            Ok(prog_fd) => {
-                *fd = Some(*prog_fd as RawFd);
-                return Ok(());
-            }
-            Err((_, io_error)) if retries == 0 || io_error.raw_os_error() == Some(ENOSPC) => {
-                if retries == 10 {
-                    break;
-                }
-                retries += 1;
-                log_buf.grow();
-            }
-            Err(_) => break,
-        };
-    }
+    let attr = BpfLoadProgramAttrs {
+        name: prog_name,
+        ty: prog_type,
+        insns: instructions,
+        license,
+        kernel_version: target_kernel_version,
+        expected_attach_type: data.expected_attach_type,
+        prog_btf_fd: data.btf_fd,
+        attach_btf_obj_fd: data.attach_btf_obj_fd,
+        attach_btf_id: data.attach_btf_id,
+        attach_prog_fd: data.attach_prog_fd,
+        func_info_rec_size: *func_info_rec_size,
+        func_info: func_info.clone(),
+        line_info_rec_size: *line_info_rec_size,
+        line_info: line_info.clone(),
+    };
+    let ret = retry_with_verifier_logs(10, &mut logger, |logger| bpf_load_program(&attr, logger));
 
-    if let Err((_, io_error)) = ret {
-        log_buf.truncate();
-        return Err(ProgramError::LoadError {
-            io_error,
-            verifier_log: log_buf
-                .as_c_str()
-                .map(|s| s.to_string_lossy().to_string())
-                .unwrap_or_else(|| "[none]".to_owned()),
-        });
+    match ret {
+        Ok(prog_fd) => {
+            *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()),
+            });
+        }
     }
-
-    Ok(())
 }
 
 pub(crate) fn query<T: AsRawFd>(

aya/src/sys/bpf.rs

@@ -3,7 +3,7 @@ use crate::{
     obj::{btf::BtfType, copy_instructions},
     Btf,
 };
-use libc::{c_char, c_long, close, ENOENT};
+use libc::{c_char, c_long, close, ENOENT, ENOSPC};
 
 use std::{
     cmp::{self, min},
@@ -78,19 +78,21 @@ pub(crate) struct BpfLoadProgramAttrs<'a> {
     pub(crate) attach_btf_obj_fd: Option<u32>,
     pub(crate) attach_btf_id: Option<u32>,
     pub(crate) attach_prog_fd: Option<RawFd>,
-    pub(crate) log: &'a mut VerifierLog,
     pub(crate) func_info_rec_size: usize,
     pub(crate) func_info: FuncSecInfo,
     pub(crate) line_info_rec_size: usize,
     pub(crate) line_info: LineSecInfo,
 }
 
-pub(crate) fn bpf_load_program(aya_attr: BpfLoadProgramAttrs) -> SysResult {
+pub(crate) fn bpf_load_program(
+    aya_attr: &BpfLoadProgramAttrs,
+    logger: &mut VerifierLog,
+) -> SysResult {
     let mut attr = unsafe { mem::zeroed::<bpf_attr>() };
 
     let u = unsafe { &mut attr.__bindgen_anon_3 };
 
-    if let Some(prog_name) = aya_attr.name {
+    if let Some(prog_name) = &aya_attr.name {
         let mut name: [c_char; 16] = [0; 16];
         let name_bytes = prog_name.to_bytes();
         let len = min(name.len(), name_bytes.len());
@@ -127,7 +129,7 @@ pub(crate) fn bpf_load_program(aya_attr: BpfLoadProgramAttrs) -> SysResult {
             u.func_info_rec_size = aya_attr.func_info_rec_size as u32;
         }
     }
-    let log_buf = aya_attr.log.buf();
+    let log_buf = logger.buf();
     if log_buf.capacity() > 0 {
         u.log_level = 7;
         u.log_buf = log_buf.as_mut_ptr() as u64;
@@ -443,7 +445,7 @@ 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) -> Result<RawFd, io::Error> {
+pub(crate) fn bpf_load_btf(raw_btf: &[u8], log: &mut VerifierLog) -> 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;
@@ -454,10 +456,7 @@ pub(crate) fn bpf_load_btf(raw_btf: &[u8], log: &mut VerifierLog) -> Result<RawF
         u.btf_log_buf = log_buf.as_mut_ptr() as u64;
         u.btf_log_size = log_buf.capacity() as u32;
     }
-    match sys_bpf(bpf_cmd::BPF_BTF_LOAD, &attr) {
-        Ok(v) => Ok(v as RawFd),
-        Err((_, err)) => Err(err),
-    }
+    sys_bpf(bpf_cmd::BPF_BTF_LOAD, &attr)
 }
 
 pub(crate) fn bpf_btf_get_fd_by_id(id: u32) -> Result<RawFd, io::Error> {
@@ -733,3 +732,37 @@ pub(crate) fn is_btf_type_tag_supported() -> bool {
 pub fn sys_bpf(cmd: bpf_cmd, attr: &bpf_attr) -> SysResult {
     syscall(Syscall::Bpf { cmd, attr })
 }
+
+pub(crate) fn retry_with_verifier_logs<F>(
+    max_retries: usize,
+    log: &mut VerifierLog,
+    f: F,
+) -> SysResult
+where
+    F: Fn(&mut VerifierLog) -> SysResult,
+{
+    // 1. Try the syscall
+    let ret = f(log);
+    if ret.is_ok() {
+        return ret;
+    }
+
+    // 2. Grow the log buffer so we can capture verifier output
+    //    Retry this up to max_retries times
+    log.grow();
+    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));
+                }
+                retries += 1;
+                log.grow();
+            }
+            r => return r,
+        }
+    }
+}