Browse Source

aya-log, aya-log-common: economize bytes

- Replace all `#[repr(usize)]` with `#[repr(u8)]`; this saves
  3*(sizeof(word)-1) bytes per log message.
- Encode payload length as u16 rather than usize; this saves
  sizeof(word)-1 bytes per log message. This is safe because the maximum
  size of a log message is less than (1 << 16 - 1).

This changes `level` to a require field in every log message. It was
already always present, but was treated as optional when reading.
Tamir Duberstein 1 year ago
parent
commit
a4a69a6
2 changed files with 67 additions and 36 deletions
  1. 41 21
      aya-log-common/src/lib.rs
  2. 26 15
      aya-log/src/lib.rs

+ 41 - 21
aya-log-common/src/lib.rs

@@ -1,6 +1,6 @@
 #![no_std]
 
-use core::{cmp, mem, ptr, slice};
+use core::{mem, num, ptr, slice};
 
 use num_enum::IntoPrimitive;
 
@@ -8,8 +8,10 @@ pub const LOG_BUF_CAPACITY: usize = 8192;
 
 pub const LOG_FIELDS: usize = 6;
 
-#[repr(usize)]
-#[derive(Copy, Clone, Eq, PartialEq, Debug, Hash)]
+pub type LogValueLength = u16;
+
+#[repr(u8)]
+#[derive(Copy, Clone, Eq, PartialEq, Debug, Hash, IntoPrimitive)]
 pub enum Level {
     /// The "error" level.
     ///
@@ -33,7 +35,7 @@ pub enum Level {
     Trace,
 }
 
-#[repr(usize)]
+#[repr(u8)]
 #[derive(Copy, Clone, Debug)]
 pub enum RecordField {
     Target = 1,
@@ -46,7 +48,7 @@ pub enum RecordField {
 
 /// Types which are supported by aya-log and can be safely sent from eBPF
 /// programs to userspace.
-#[repr(usize)]
+#[repr(u8)]
 #[derive(Copy, Clone, Debug)]
 pub enum Argument {
     DisplayHint,
@@ -111,26 +113,29 @@ where
     }
 
     pub(crate) fn write(&self, mut buf: &mut [u8]) -> Result<usize, ()> {
-        let size = mem::size_of::<T>() + mem::size_of::<usize>() + self.value.len();
-        let remaining = cmp::min(buf.len(), LOG_BUF_CAPACITY);
-        // Check if the size doesn't exceed the buffer bounds.
-        if size > remaining {
+        // Break the abstraction to please the verifier.
+        if buf.len() > LOG_BUF_CAPACITY {
+            buf = &mut buf[..LOG_BUF_CAPACITY];
+        }
+        let Self { tag, value } = self;
+        let len = value.len();
+        let wire_len: LogValueLength = value
+            .len()
+            .try_into()
+            .map_err(|num::TryFromIntError { .. }| ())?;
+        let size = mem::size_of_val(tag) + mem::size_of_val(&wire_len) + len;
+        if size > buf.len() {
             return Err(());
         }
 
-        unsafe { ptr::write_unaligned(buf.as_mut_ptr() as *mut _, self.tag) };
-        buf = &mut buf[mem::size_of::<T>()..];
+        unsafe { ptr::write_unaligned(buf.as_mut_ptr() as *mut _, *tag) };
+        buf = &mut buf[mem::size_of_val(tag)..];
 
-        unsafe { ptr::write_unaligned(buf.as_mut_ptr() as *mut _, self.value.len()) };
-        buf = &mut buf[mem::size_of::<usize>()..];
+        unsafe { ptr::write_unaligned(buf.as_mut_ptr() as *mut _, wire_len) };
+        buf = &mut buf[mem::size_of_val(&wire_len)..];
+
+        unsafe { ptr::copy_nonoverlapping(value.as_ptr(), buf.as_mut_ptr(), len) };
 
-        let len = cmp::min(buf.len(), self.value.len());
-        // The verifier isn't happy with `len` being unbounded, so compare it
-        // with `LOG_BUF_CAPACITY`.
-        if len > LOG_BUF_CAPACITY {
-            return Err(());
-        }
-        buf[..len].copy_from_slice(&self.value[..len]);
         Ok(size)
     }
 }
@@ -210,10 +215,11 @@ pub fn write_record_header(
     line: u32,
     num_args: usize,
 ) -> Result<usize, ()> {
+    let level: u8 = level.into();
     let mut size = 0;
     for attr in [
         TagLenValue::<RecordField>::new(RecordField::Target, target.as_bytes()),
-        TagLenValue::<RecordField>::new(RecordField::Level, &(level as usize).to_ne_bytes()),
+        TagLenValue::<RecordField>::new(RecordField::Level, &level.to_ne_bytes()),
         TagLenValue::<RecordField>::new(RecordField::Module, module.as_bytes()),
         TagLenValue::<RecordField>::new(RecordField::File, file.as_bytes()),
         TagLenValue::<RecordField>::new(RecordField::Line, &line.to_ne_bytes()),
@@ -224,3 +230,17 @@ pub fn write_record_header(
 
     Ok(size)
 }
+
+#[cfg(test)]
+mod test {
+    use super::*;
+
+    fn log_value_length_sufficient() {
+        assert!(
+            LOG_BUF_CAPACITY >= LogValueLength::MAX.into(),
+            "{} < {}",
+            LOG_BUF_CAPACITY,
+            LogValueLength::MAX
+        );
+    }
+}

+ 26 - 15
aya-log/src/lib.rs

@@ -59,9 +59,11 @@ use std::{
 
 const MAP_NAME: &str = "AYA_LOGS";
 
-use aya_log_common::{Argument, DisplayHint, RecordField, LOG_BUF_CAPACITY, LOG_FIELDS};
+use aya_log_common::{
+    Argument, DisplayHint, Level, LogValueLength, RecordField, LOG_BUF_CAPACITY, LOG_FIELDS,
+};
 use bytes::BytesMut;
-use log::{error, Level, Log, Record};
+use log::{error, Log, Record};
 use thiserror::Error;
 
 use aya::{
@@ -116,9 +118,7 @@ impl BpfLogger {
 
             let log = logger.clone();
             tokio::spawn(async move {
-                let mut buffers = (0..10)
-                    .map(|_| BytesMut::with_capacity(LOG_BUF_CAPACITY))
-                    .collect::<Vec<_>>();
+                let mut buffers = vec![BytesMut::with_capacity(LOG_BUF_CAPACITY); 10];
 
                 loop {
                     let events = buf.read_events(&mut buffers).await.unwrap();
@@ -360,7 +360,7 @@ pub enum Error {
 
 fn log_buf(mut buf: &[u8], logger: &dyn Log) -> Result<(), ()> {
     let mut target = None;
-    let mut level = Level::Trace;
+    let mut level = None;
     let mut module = None;
     let mut file = None;
     let mut line = None;
@@ -371,16 +371,25 @@ fn log_buf(mut buf: &[u8], logger: &dyn Log) -> Result<(), ()> {
 
         match tag {
             RecordField::Target => {
-                target = Some(std::str::from_utf8(value).map_err(|_| ())?);
+                target = Some(str::from_utf8(value).map_err(|_| ())?);
             }
             RecordField::Level => {
-                level = unsafe { ptr::read_unaligned(value.as_ptr() as *const _) }
+                level = Some({
+                    let level = unsafe { ptr::read_unaligned(value.as_ptr() as *const _) };
+                    match level {
+                        Level::Error => log::Level::Error,
+                        Level::Warn => log::Level::Warn,
+                        Level::Info => log::Level::Info,
+                        Level::Debug => log::Level::Debug,
+                        Level::Trace => log::Level::Trace,
+                    }
+                })
             }
             RecordField::Module => {
-                module = Some(std::str::from_utf8(value).map_err(|_| ())?);
+                module = Some(str::from_utf8(value).map_err(|_| ())?);
             }
             RecordField::File => {
-                file = Some(std::str::from_utf8(value).map_err(|_| ())?);
+                file = Some(str::from_utf8(value).map_err(|_| ())?);
             }
             RecordField::Line => {
                 line = Some(u32::from_ne_bytes(value.try_into().map_err(|_| ())?));
@@ -505,7 +514,7 @@ fn log_buf(mut buf: &[u8], logger: &dyn Log) -> Result<(), ()> {
         &Record::builder()
             .args(format_args!("{full_log_msg}"))
             .target(target.ok_or(())?)
-            .level(level)
+            .level(level.ok_or(())?)
             .module_path(module)
             .file(file)
             .line(line)
@@ -516,16 +525,18 @@ fn log_buf(mut buf: &[u8], logger: &dyn Log) -> Result<(), ()> {
 }
 
 fn try_read<T: Pod>(mut buf: &[u8]) -> Result<(T, &[u8], &[u8]), ()> {
-    if buf.len() < mem::size_of::<T>() + mem::size_of::<usize>() {
+    if buf.len() < mem::size_of::<T>() + mem::size_of::<LogValueLength>() {
         return Err(());
     }
 
     let tag = unsafe { ptr::read_unaligned(buf.as_ptr() as *const T) };
     buf = &buf[mem::size_of::<T>()..];
 
-    let len = usize::from_ne_bytes(buf[..mem::size_of::<usize>()].try_into().unwrap());
-    buf = &buf[mem::size_of::<usize>()..];
+    let len =
+        LogValueLength::from_ne_bytes(buf[..mem::size_of::<LogValueLength>()].try_into().unwrap());
+    buf = &buf[mem::size_of::<LogValueLength>()..];
 
+    let len: usize = len.into();
     if buf.len() < len {
         return Err(());
     }
@@ -538,7 +549,7 @@ fn try_read<T: Pod>(mut buf: &[u8]) -> Result<(T, &[u8], &[u8]), ()> {
 mod test {
     use super::*;
     use aya_log_common::{write_record_header, WriteToBuf};
-    use log::logger;
+    use log::{logger, Level};
 
     fn new_log(args: usize) -> Result<(usize, Vec<u8>), ()> {
         let mut buf = vec![0; 8192];