[TBS] addressing dave's comments

aya-ebpf-macros/src/lib.rs

@@ -544,9 +544,38 @@ pub fn fexit(attrs: TokenStream, item: TokenStream) -> TokenStream {
     .into()
 }
 
-/// Marks a function as an eBPF Flow Dissector program that can be attached to
+/// Marks a function as an eBPF Flow Dissector program.
-/// a network namespace.
+///
+/// Flow dissector is a program type that parses metadata out of the packets.
+///
+/// BPF flow dissectors can be attached per network namespace. These programs
+/// are given a packet and expected to populate the fields of
+/// `FlowDissectorContext::flow_keys`. The return code of the BPF program is
+/// either [`BPF_OK`] to indicate successful dissection, [`BPF_DROP`] to
+/// indicate parsing error, or [`BPF_FLOW_DISSECTOR_CONTINUE`] to indicate that
+/// no custom dissection was performed, and fallback to standard dissector is
+/// requested.
+///
+/// # Minimum kernel version
+///
+/// The minimum kernel version required to use this feature is 4.20.
+///
+/// # Examples
+///
+/// ```no_run
+/// use aya_ebpf::{macros::flow_dissector, programs::FlowDissectorContext};
+///
+/// #[flow_dissector]
+/// pub fn dissect(_ctx: FlowDissectorContext) -> u32 {
+///     // TODO: do something useful here.
+///     return 0
+/// }
+/// ```
 ///
+/// [`FlowDissectorContext::flow_keys`]: aya_ebpf::programs::FlowDissectorContext::flow_keys
+/// [`BPF_OK`]: aya_ebpf::bindings::bpf_ret_code::BPF_OK
+/// [`BPF_DROP`]: aya_ebpf::bindings::bpf_ret_code::BPF_DROP
+/// [`BPF_FLOW_DISSECTOR_CONTINUE`]: aya_ebpf::bindings::bpf_ret_code::BPF_FLOW_DISSECTOR_CONTINUE
 #[proc_macro_attribute]
 pub fn flow_dissector(attrs: TokenStream, item: TokenStream) -> TokenStream {
     match FlowDissector::parse(attrs.into(), item.into()) {

aya/src/programs/flow_dissector.rs

@@ -9,13 +9,18 @@ use aya_obj::generated::{
 use crate::{
     programs::{FdLink, FdLinkId, ProgramData, ProgramError, define_link_wrapper, load_program},
     sys::{LinkTarget, SyscallError, bpf_link_create},
+    util::KernelVersion,
 };
 
-/// A program that can be attached as a Flow Dissector routine
+/// Flow dissector is a program type that parses metadata out of the packets.
 ///
-/// ['FlowDissector'] programs operate on an __sk_buff.
+/// BPF flow dissectors can be attached per network namespace. These programs
-/// However, only the limited set of fields is allowed: data, data_end and flow_keys.
+/// are given a packet and expected to populate the fields of
-/// flow_keys is struct bpf_flow_keys and contains flow dissector input and output arguments.
+/// `FlowDissectorContext::flow_keys`. The return code of the BPF program is
+/// either [`BPF_OK`] to indicate successful dissection, [`BPF_DROP`] to
+/// indicate parsing error, or [`BPF_FLOW_DISSECTOR_CONTINUE`] to indicate that
+/// no custom dissection was performed, and fallback to standard dissector is
+/// requested.
 ///
 /// # Minimum kernel version
 ///
@@ -45,6 +50,11 @@ use crate::{
 /// program.attach(net_ns)?;
 /// # Ok::<(), Error>(())
 /// ```
+///
+/// [`FlowDissectorContext::flow_keys`]: aya_ebpf::programs::FlowDissectorContext::flow_keys
+/// [`BPF_OK`]: aya_ebpf::bindings::bpf_ret_code::BPF_OK
+/// [`BPF_DROP`]: aya_ebpf::bindings::bpf_ret_code::BPF_DROP
+/// [`BPF_FLOW_DISSECTOR_CONTINUE`]: aya_ebpf::bindings::bpf_ret_code::BPF_FLOW_DISSECTOR_CONTINUE
 #[derive(Debug)]
 #[doc(alias = "BPF_PROG_TYPE_FLOW_DISSECTOR")]
 pub struct FlowDissector {
@@ -66,20 +76,24 @@ impl FlowDissector {
         let prog_fd = prog_fd.as_fd();
         let netns_fd = netns.as_fd();
 
-        let link_fd = bpf_link_create(
+        if KernelVersion::at_least(5, 7, 0) {
-            prog_fd,
+            let link_fd = bpf_link_create(
-            LinkTarget::Fd(netns_fd),
+                prog_fd,
-            BPF_FLOW_DISSECTOR,
+                LinkTarget::Fd(netns_fd),
-            0,
+                BPF_FLOW_DISSECTOR,
-            None,
+                0,
-        )
+                None,
-        .map_err(|io_error| SyscallError {
+            )
-            call: "bpf_link_create",
+            .map_err(|io_error| SyscallError {
-            io_error,
+                call: "bpf_link_create",
-        })?;
+                io_error,
-        self.data
+            })?;
-            .links
+            self.data
-            .insert(FlowDissectorLink::new(FdLink::new(link_fd)))
+                .links
+                .insert(FlowDissectorLink::new(FdLink::new(link_fd)))
+        } else {
+            todo!("support attachment via BPF_PROG_ATTACH")
+        }
     }
 }
 

ebpf/aya-ebpf/src/programs/flow_dissector.rs

@@ -1,34 +1,44 @@
-use aya_ebpf_cty::c_void;
+use aya_ebpf_cty::{c_long, c_void};
 
-use crate::{EbpfContext, bindings::__sk_buff};
+use crate::{
+    EbpfContext,
+    bindings::{__sk_buff, bpf_flow_keys},
+    programs::sk_buff::SkBuff,
+};
 
 pub struct FlowDissectorContext {
-    skb: *mut __sk_buff,
+    skb: SkBuff,
 }
 
 impl FlowDissectorContext {
     pub fn new(skb: *mut __sk_buff) -> FlowDissectorContext {
+        let skb = SkBuff { skb };
         FlowDissectorContext { skb }
     }
 
     #[inline]
     pub fn data(&self) -> usize {
-        unsafe { (*self.skb).data as usize }
+        self.skb.data()
     }
 
     #[inline]
     pub fn data_end(&self) -> usize {
-        unsafe { (*self.skb).data_end as usize }
+        self.skb.data_end()
     }
 
     #[inline]
-    pub fn flow_keys(&self) -> usize {
+    pub fn flow_keys(&self) -> &bpf_flow_keys {
-        unsafe { (*self.skb).__bindgen_anon_1.flow_keys as usize }
+        unsafe { &*(*self.skb.skb).__bindgen_anon_1.flow_keys }
+    }
+
+    #[inline(always)]
+    pub fn load_bytes(&self, offset: usize, dst: &mut [u8]) -> Result<usize, c_long> {
+        self.skb.load_bytes(offset, dst)
     }
 }
 
 impl EbpfContext for FlowDissectorContext {
     fn as_ptr(&self) -> *mut c_void {
-        self.skb as *mut _
+        self.skb.as_ptr()
     }
 }

test/integration-ebpf/src/test.rs

@@ -48,6 +48,8 @@ pub fn test_uretprobe(_ctx: RetProbeContext) -> u32 {
 
 #[flow_dissector]
 pub fn test_flow(_ctx: FlowDissectorContext) -> u32 {
+    // TODO: write an actual flow dissector. See tools/testing/selftests/bpf/progs/bpf_flow.c in the
+    // Linux kernel for inspiration.
     0
 }