From 3cfd886dc512872fd3948cdf3baa8c99fe27ef0f Mon Sep 17 00:00:00 2001 From: Tamir Duberstein Date: Tue, 8 Aug 2023 18:48:51 -0400 Subject: [PATCH] log: annotate logging functions inlining Some of these functions fail to compile when not inlined, so we should be explicit. Before deciding on this approach I tried various ways of making all these functions #[inline(never)] to save instructions but I ran into blockers: - These functions currently return Result, which is a structure. This is not permitted in BPF. - I tried inventing a newtype that is a #[repr(transparent)] wrapper of u16, and having these functions return that; however it seems that even if the object code is legal, the verifier will reject such functions because the BTF (if present, and it was in my local experiments) would indicate that the return is a structure. - I tried having these functions return a plain u16 where 0 means error, but the verifier still rejected the BTF because the receiver (even if made into &self) is considered a structure, and forbidden. We can eventually overcome these problems by "lying" in our BTF once support for it matures in the bpf-linker repo (e.g. Option should be perfectly legal as it is guaranteed to be word-sized), but we aren't there yet, and this is the safest thing we can do for now. --- aya-log-common/src/lib.rs | 27 ++++++++++++++++++++++++++ test/integration-ebpf/src/log.rs | 9 ++++++++- test/integration-test/src/tests/log.rs | 2 +- 3 files changed, 36 insertions(+), 2 deletions(-) diff --git a/aya-log-common/src/lib.rs b/aya-log-common/src/lib.rs index c634c51e..ba20876f 100644 --- a/aya-log-common/src/lib.rs +++ b/aya-log-common/src/lib.rs @@ -147,6 +147,10 @@ pub enum DisplayHint { UpperMac, } +// Must be inlined, else the BPF backend emits: +// +// llvm: :0:0: in function _ZN14aya_log_common5write17hc9ed05433e23a663E { i64, i64 } (i8, ptr, i64, ptr, i64): only integer returns supported +#[inline(always)] pub(crate) fn write(tag: u8, value: &[u8], buf: &mut [u8]) -> Result { let wire_len: LogValueLength = value .len() @@ -175,6 +179,9 @@ pub trait WriteToBuf { macro_rules! impl_write_to_buf { ($type:ident, $arg_type:expr) => { impl WriteToBuf for $type { + // This need not be inlined because the return value is Result where N is + // mem::size_of<$type>, which is a compile-time constant. + #[inline(never)] fn write(self, buf: &mut [u8]) -> Result { write($arg_type.into(), &self.to_ne_bytes(), buf) } @@ -198,12 +205,18 @@ impl_write_to_buf!(f32, Argument::F32); impl_write_to_buf!(f64, Argument::F64); impl WriteToBuf for [u8; 16] { + // This need not be inlined because the return value is Result where N is 16, which is a + // compile-time constant. + #[inline(never)] fn write(self, buf: &mut [u8]) -> Result { write(Argument::ArrU8Len16.into(), &self, buf) } } impl WriteToBuf for [u16; 8] { + // This need not be inlined because the return value is Result where N is 16, which is a + // compile-time constant. + #[inline(never)] fn write(self, buf: &mut [u8]) -> Result { let bytes = unsafe { core::mem::transmute::<_, [u8; 16]>(self) }; write(Argument::ArrU16Len8.into(), &bytes, buf) @@ -211,24 +224,38 @@ impl WriteToBuf for [u16; 8] { } impl WriteToBuf for [u8; 6] { + // This need not be inlined because the return value is Result where N is 6, which is a + // compile-time constant. + #[inline(never)] fn write(self, buf: &mut [u8]) -> Result { write(Argument::ArrU8Len6.into(), &self, buf) } } impl WriteToBuf for &[u8] { + // Must be inlined, else the BPF backend emits: + // + // llvm: :0:0: in function _ZN63_$LT$$RF$$u5b$u8$u5d$$u20$as$u20$aya_log_common..WriteToBuf$GT$5write17h08f30a45f7b9f09dE { i64, i64 } (ptr, i64, ptr, i64): only integer returns supported + #[inline(always)] fn write(self, buf: &mut [u8]) -> Result { write(Argument::Bytes.into(), self, buf) } } impl WriteToBuf for &str { + // Must be inlined, else the BPF backend emits: + // + // llvm: :0:0: in function _ZN54_$LT$$RF$str$u20$as$u20$aya_log_common..WriteToBuf$GT$5write17h7e2d1ccaa758e2b5E { i64, i64 } (ptr, i64, ptr, i64): only integer returns supported + #[inline(always)] fn write(self, buf: &mut [u8]) -> Result { write(Argument::Str.into(), self.as_bytes(), buf) } } impl WriteToBuf for DisplayHint { + // This need not be inlined because the return value is Result where N is 1, which is a + // compile-time constant. + #[inline(never)] fn write(self, buf: &mut [u8]) -> Result { let v: u8 = self.into(); write(Argument::DisplayHint.into(), &v.to_ne_bytes(), buf) diff --git a/test/integration-ebpf/src/log.rs b/test/integration-ebpf/src/log.rs index 03cdde76..51c1a46d 100644 --- a/test/integration-ebpf/src/log.rs +++ b/test/integration-ebpf/src/log.rs @@ -7,7 +7,14 @@ use aya_log_ebpf::{debug, error, info, trace, warn}; #[uprobe] pub fn test_log(ctx: ProbeContext) { debug!(&ctx, "Hello from eBPF!"); - error!(&ctx, "{}, {}, {}", 69, 420i32, "wao"); + error!( + &ctx, + "{}, {}, {}, {:x}", + 69, + 420i32, + "wao", + "wao".as_bytes() + ); let ipv4 = 167772161u32; // 10.0.0.1 let ipv6 = [ 32u8, 1u8, 13u8, 184u8, 0u8, 0u8, 0u8, 0u8, 0u8, 0u8, 0u8, 0u8, 0u8, 0u8, 0u8, 1u8, diff --git a/test/integration-test/src/tests/log.rs b/test/integration-test/src/tests/log.rs index 8645762b..65ad34eb 100644 --- a/test/integration-test/src/tests/log.rs +++ b/test/integration-test/src/tests/log.rs @@ -92,7 +92,7 @@ async fn log() { assert_eq!( records.next(), Some(&CapturedLog { - body: "69, 420, wao".into(), + body: "69, 420, wao, 77616f".into(), level: Level::Error, target: "log".into(), })