From 157493579f54540c02abaec844c474e9a4e6105d Mon Sep 17 00:00:00 2001 From: Dave Tucker Date: Tue, 1 Aug 2023 20:14:35 +0100 Subject: [PATCH] aya-bpf-macros: Remove pop_required_string_args I got a little carried away with refactoring the macros and started making some macro arguments required when they previously were not. While I'm all for doing this eventually, for stronger type guarantees and de-duplicating information from BPF and userspace, it's premature to do that now. Signed-off-by: Dave Tucker --- aya-bpf-macros/src/args.rs | 24 ------------ aya-bpf-macros/src/btf_tracepoint.rs | 48 ++++++++++++++++++++--- aya-bpf-macros/src/fentry.rs | 58 +++++++++++++++++++++------- aya-bpf-macros/src/fexit.rs | 55 ++++++++++++++++++++------ aya-bpf-macros/src/lsm.rs | 28 ++++++++------ aya-bpf-macros/src/raw_tracepoint.rs | 26 ++++++++----- 6 files changed, 164 insertions(+), 75 deletions(-) diff --git a/aya-bpf-macros/src/args.rs b/aya-bpf-macros/src/args.rs index 5547dbaa..c4e600fd 100644 --- a/aya-bpf-macros/src/args.rs +++ b/aya-bpf-macros/src/args.rs @@ -70,30 +70,6 @@ pub(crate) fn pop_bool_arg(args: &mut Args, name: &str) -> bool { value.is_some() } -pub(crate) fn pop_required_string_arg(args: &mut Args, name: &str) -> Result { - let value = match args.args.iter().position(|arg| match arg { - Arg::String(name_val) => name_val.name == name, - Arg::Bool(_) => false, - }) { - Some(index) => Some(args.args.remove(index)), - None => None, - }; - match value { - Some(Arg::String(value)) => Ok(value.value.value()), - Some(Arg::Bool(_)) => unreachable!("arg bool were filtered out"), - None => { - let tokens = match args.args.first().unwrap() { - Arg::String(name_val) => &name_val.name, - Arg::Bool(ident) => ident, - }; - Err(Error::new_spanned( - tokens, - format!("missing required argument `{}`", name), - )) - } - } -} - pub(crate) fn err_on_unknown_args(args: &Args) -> Result<()> { if let Some(arg) = args.args.get(0) { let tokens = match arg { diff --git a/aya-bpf-macros/src/btf_tracepoint.rs b/aya-bpf-macros/src/btf_tracepoint.rs index 62c6ab5e..1c7b02af 100644 --- a/aya-bpf-macros/src/btf_tracepoint.rs +++ b/aya-bpf-macros/src/btf_tracepoint.rs @@ -4,24 +4,33 @@ use proc_macro2::TokenStream; use quote::quote; use syn::{ItemFn, Result}; -use crate::args::{err_on_unknown_args, pop_required_string_arg, Args}; +use crate::args::{err_on_unknown_args, pop_string_arg, Args}; pub(crate) struct BtfTracePoint { item: ItemFn, - function: String, + function: Option, } impl BtfTracePoint { pub(crate) fn parse(attrs: TokenStream, item: TokenStream) -> Result { - let mut args: Args = syn::parse2(attrs)?; let item = syn::parse2(item)?; - let function = pop_required_string_arg(&mut args, "function")?; - err_on_unknown_args(&args)?; + let mut function = None; + if !attrs.is_empty() { + let mut args: Args = syn::parse2(attrs)?; + if let Some(f) = pop_string_arg(&mut args, "function") { + function = Some(f); + } + err_on_unknown_args(&args)?; + } Ok(BtfTracePoint { item, function }) } pub(crate) fn expand(&self) -> Result { - let section_name: Cow<'_, _> = format!("tp_btf/{}", self.function).into(); + let section_name: Cow<'_, _> = if self.function.is_none() { + "tp_btf".into() + } else { + format!("tp_btf/{}", self.function.as_ref().unwrap()).into() + }; let fn_vis = &self.item.vis; let fn_name = self.item.sig.ident.clone(); let item = &self.item; @@ -45,6 +54,33 @@ mod tests { #[test] fn test_btf_tracepoint() { + let prog = BtfTracePoint::parse( + parse_quote!(), + parse_quote!( + fn foo(ctx: BtfTracePointContext) -> i32 { + 0 + } + ), + ) + .unwrap(); + let expanded = prog.expand().unwrap(); + let expected = quote!( + #[no_mangle] + #[link_section = "tp_btf"] + fn foo(ctx: *mut ::core::ffi::c_void) -> i32 { + let _ = foo(::aya_bpf::programs::BtfTracePointContext::new(ctx)); + return 0; + + fn foo(ctx: BtfTracePointContext) -> i32 { + 0 + } + } + ); + assert_eq!(expected.to_string(), expanded.to_string()); + } + + #[test] + fn test_btf_tracepoint_with_function() { let prog = BtfTracePoint::parse( parse_quote!(function = "some_func"), parse_quote!( diff --git a/aya-bpf-macros/src/fentry.rs b/aya-bpf-macros/src/fentry.rs index d1a2ed7f..870057da 100644 --- a/aya-bpf-macros/src/fentry.rs +++ b/aya-bpf-macros/src/fentry.rs @@ -1,28 +1,30 @@ use std::borrow::Cow; use proc_macro2::TokenStream; -use proc_macro_error::abort; use quote::quote; use syn::{ItemFn, Result}; -use crate::args::{err_on_unknown_args, pop_bool_arg, pop_required_string_arg}; +use crate::args::{err_on_unknown_args, pop_bool_arg, pop_string_arg}; pub(crate) struct FEntry { item: ItemFn, - function: String, + function: Option, sleepable: bool, } impl FEntry { pub(crate) fn parse(attrs: TokenStream, item: TokenStream) -> Result { - if attrs.is_empty() { - abort!(attrs, "missing function name"); - } - let mut args = syn::parse2(attrs)?; let item = syn::parse2(item)?; - let function = pop_required_string_arg(&mut args, "function")?; - let sleepable = pop_bool_arg(&mut args, "sleepable"); - err_on_unknown_args(&args)?; + let mut function = None; + let mut sleepable = false; + if !attrs.is_empty() { + let mut args = syn::parse2(attrs)?; + if let Some(f) = pop_string_arg(&mut args, "function") { + function = Some(f); + }; + sleepable = pop_bool_arg(&mut args, "sleepable"); + err_on_unknown_args(&args)?; + } Ok(FEntry { item, function, @@ -32,7 +34,11 @@ impl FEntry { pub(crate) fn expand(&self) -> Result { let section_prefix = if self.sleepable { "fentry.s" } else { "fentry" }; - let section_name: Cow<'_, _> = format!("{}/{}", section_prefix, self.function).into(); + let section_name: Cow<'_, _> = if self.function.is_none() { + section_prefix.into() + } else { + format!("{}/{}", section_prefix, self.function.as_ref().unwrap()).into() + }; let fn_vis = &self.item.vis; let fn_name = self.item.sig.ident.clone(); let item = &self.item; @@ -56,6 +62,33 @@ mod tests { #[test] fn test_fentry() { + let prog = FEntry::parse( + parse_quote! {}, + parse_quote! { + fn sys_clone(ctx: &mut aya_bpf::programs::FEntryContext) -> i32 { + 0 + } + }, + ) + .unwrap(); + let expanded = prog.expand().unwrap(); + let expected = quote! { + #[no_mangle] + #[link_section = "fentry"] + fn sys_clone(ctx: *mut ::core::ffi::c_void) -> i32 { + let _ = sys_clone(::aya_bpf::programs::FEntryContext::new(ctx)); + return 0; + + fn sys_clone(ctx: &mut aya_bpf::programs::FEntryContext) -> i32 { + 0 + } + } + }; + assert_eq!(expected.to_string(), expanded.to_string()); + } + + #[test] + fn test_fentry_with_function() { let prog = FEntry::parse( parse_quote! { function = "sys_clone" @@ -87,7 +120,6 @@ mod tests { fn test_fentry_sleepable() { let prog = FEntry::parse( parse_quote! { - function = "sys_clone", sleepable }, parse_quote! { @@ -100,7 +132,7 @@ mod tests { let expanded = prog.expand().unwrap(); let expected = quote! { #[no_mangle] - #[link_section = "fentry.s/sys_clone"] + #[link_section = "fentry.s"] fn sys_clone(ctx: *mut ::core::ffi::c_void) -> i32 { let _ = sys_clone(::aya_bpf::programs::FEntryContext::new(ctx)); return 0; diff --git a/aya-bpf-macros/src/fexit.rs b/aya-bpf-macros/src/fexit.rs index 80a63b96..756330ad 100644 --- a/aya-bpf-macros/src/fexit.rs +++ b/aya-bpf-macros/src/fexit.rs @@ -1,28 +1,30 @@ use std::borrow::Cow; use proc_macro2::TokenStream; -use proc_macro_error::abort; use quote::quote; use syn::{ItemFn, Result}; -use crate::args::{err_on_unknown_args, pop_bool_arg, pop_required_string_arg}; +use crate::args::{err_on_unknown_args, pop_bool_arg, pop_string_arg}; pub(crate) struct FExit { item: ItemFn, - function: String, + function: Option, sleepable: bool, } impl FExit { pub(crate) fn parse(attrs: TokenStream, item: TokenStream) -> Result { - if attrs.is_empty() { - abort!(attrs, "missing function name"); - } - let mut args = syn::parse2(attrs)?; let item = syn::parse2(item)?; - let function = pop_required_string_arg(&mut args, "function")?; - let sleepable = pop_bool_arg(&mut args, "sleepable"); - err_on_unknown_args(&args)?; + let mut function = None; + let mut sleepable = false; + if !attrs.is_empty() { + let mut args = syn::parse2(attrs)?; + if let Some(f) = pop_string_arg(&mut args, "function") { + function = Some(f); + }; + sleepable = pop_bool_arg(&mut args, "sleepable"); + err_on_unknown_args(&args)?; + } Ok(FExit { item, function, @@ -32,7 +34,11 @@ impl FExit { pub(crate) fn expand(&self) -> Result { let section_prefix = if self.sleepable { "fexit.s" } else { "fexit" }; - let section_name: Cow<'_, _> = format!("{}/{}", section_prefix, self.function).into(); + let section_name: Cow<'_, _> = if self.function.is_none() { + section_prefix.into() + } else { + format!("{}/{}", section_prefix, self.function.as_ref().unwrap()).into() + }; let fn_vis = &self.item.vis; let fn_name = self.item.sig.ident.clone(); let item = &self.item; @@ -56,6 +62,33 @@ mod tests { #[test] fn test_fexit() { + let prog = FExit::parse( + parse_quote! {}, + parse_quote! { + fn sys_clone(ctx: &mut FExitContext) -> i32 { + 0 + } + }, + ) + .unwrap(); + let expanded = prog.expand().unwrap(); + let expected = quote! { + #[no_mangle] + #[link_section = "fexit"] + fn sys_clone(ctx: *mut ::core::ffi::c_void) -> i32 { + let _ = sys_clone(::aya_bpf::programs::FExitContext::new(ctx)); + return 0; + + fn sys_clone(ctx: &mut FExitContext) -> i32 { + 0 + } + } + }; + assert_eq!(expected.to_string(), expanded.to_string()); + } + + #[test] + fn test_fexit_with_function() { let prog = FExit::parse( parse_quote! { function = "sys_clone" diff --git a/aya-bpf-macros/src/lsm.rs b/aya-bpf-macros/src/lsm.rs index fb358b0c..78aaea95 100644 --- a/aya-bpf-macros/src/lsm.rs +++ b/aya-bpf-macros/src/lsm.rs @@ -1,28 +1,30 @@ use std::borrow::Cow; use proc_macro2::TokenStream; -use proc_macro_error::abort; use quote::quote; use syn::{ItemFn, Result}; -use crate::args::{err_on_unknown_args, pop_bool_arg, pop_required_string_arg}; +use crate::args::{err_on_unknown_args, pop_bool_arg, pop_string_arg}; pub(crate) struct Lsm { item: ItemFn, - hook: String, + hook: Option, sleepable: bool, } impl Lsm { pub(crate) fn parse(attrs: TokenStream, item: TokenStream) -> Result { - if attrs.is_empty() { - abort!(attrs, "missing hook name"); - } - let mut args = syn::parse2(attrs)?; let item = syn::parse2(item)?; - let hook = pop_required_string_arg(&mut args, "hook")?; - let sleepable = pop_bool_arg(&mut args, "sleepable"); - err_on_unknown_args(&args)?; + let mut hook = None; + let mut sleepable = false; + if !attrs.is_empty() { + let mut args = syn::parse2(attrs)?; + if let Some(h) = pop_string_arg(&mut args, "hook") { + hook = Some(h); + }; + sleepable = pop_bool_arg(&mut args, "sleepable"); + err_on_unknown_args(&args)?; + } Ok(Lsm { item, hook, @@ -32,7 +34,11 @@ impl Lsm { pub(crate) fn expand(&self) -> Result { let section_prefix = if self.sleepable { "lsm.s" } else { "lsm" }; - let section_name: Cow<'_, _> = format!("{}/{}", section_prefix, self.hook).into(); + let section_name: Cow<'_, _> = if self.hook.is_none() { + section_prefix.into() + } else { + format!("{}/{}", section_prefix, self.hook.as_ref().unwrap()).into() + }; let fn_vis = &self.item.vis; let fn_name = self.item.sig.ident.clone(); let item = &self.item; diff --git a/aya-bpf-macros/src/raw_tracepoint.rs b/aya-bpf-macros/src/raw_tracepoint.rs index 4594b993..ef038f3a 100644 --- a/aya-bpf-macros/src/raw_tracepoint.rs +++ b/aya-bpf-macros/src/raw_tracepoint.rs @@ -1,31 +1,37 @@ use std::borrow::Cow; use proc_macro2::TokenStream; -use proc_macro_error::abort; + use quote::quote; use syn::{ItemFn, Result}; -use crate::args::{err_on_unknown_args, pop_required_string_arg}; +use crate::args::{err_on_unknown_args, pop_string_arg}; pub(crate) struct RawTracePoint { item: ItemFn, - tracepoint: String, + tracepoint: Option, } impl RawTracePoint { pub(crate) fn parse(attrs: TokenStream, item: TokenStream) -> Result { - if attrs.is_empty() { - abort!(attrs, "expected `tracepoint` attribute") - } - let mut args = syn::parse2(attrs)?; let item = syn::parse2(item)?; - let tracepoint = pop_required_string_arg(&mut args, "tracepoint")?; - err_on_unknown_args(&args)?; + let mut tracepoint = None; + if !attrs.is_empty() { + let mut args = syn::parse2(attrs)?; + if let Some(tp) = pop_string_arg(&mut args, "tracepoint") { + tracepoint = Some(tp) + }; + err_on_unknown_args(&args)?; + } Ok(RawTracePoint { item, tracepoint }) } pub(crate) fn expand(&self) -> Result { - let section_name: Cow<'_, _> = format!("raw_tp/{}", self.tracepoint).into(); + let section_name: Cow<'_, _> = if self.tracepoint.is_none() { + "raw_tp".into() + } else { + format!("raw_tp/{}", self.tracepoint.as_ref().unwrap()).into() + }; let fn_vis = &self.item.vis; let fn_name = self.item.sig.ident.clone(); let item = &self.item;