[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::{bindings::bpf_ret_code, macros::flow_dissector, programs::FlowDissectorContext};
+///
+/// #[flow_dissector]
+/// pub fn dissect(_ctx: FlowDissectorContext) -> u32 {
+///     // TODO: do something useful here.
+///     bpf_ret_code::BPF_FLOW_DISSECTOR_CONTINUE
+/// }
+/// ```
 ///
+/// [`FlowDissectorContext::flow_keys`]: ../aya_ebpf/programs/flow_dissector/struct.FlowDissectorContext.html#method.flow_keys
+/// [`BPF_OK`]: ../aya_ebpf/bindings/bpf_ret_code/constant.bpf_ok
+/// [`BPF_DROP`]: ../aya_ebpf/bindings/bpf_ret_code/constant.bpf_drop
+/// [`BPF_FLOW_DISSECTOR_CONTINUE`]: ../aya_ebpf/bindings/bpf_ret_code/constant.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

@@ -7,15 +7,23 @@ use aya_obj::generated::{
 };
 
 use crate::{
-    programs::{FdLink, FdLinkId, ProgramData, ProgramError, define_link_wrapper, load_program},
+    programs::{
+        CgroupAttachMode, FdLink, Link, ProgAttachLink, ProgramData, ProgramError,
+        define_link_wrapper, id_as_key, 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 +53,11 @@ use crate::{
 /// program.attach(net_ns)?;
 /// # Ok::<(), Error>(())
 /// ```
+///
+/// [`FlowDissectorContext::flow_keys`]: ../../../aya-ebpf/programs/flow_dissector/struct.FlowDissectorContext.html#method.flow_keys
+/// [`BPF_OK`]: ../../../aya-ebpf/bindings/bpf_ret_code/constant.bpf_ok
+/// [`BPF_DROP`]: ../../../aya-ebpf/bindings/bpf_ret_code/constant.bpf_drop
+/// [`BPF_FLOW_DISSECTOR_CONTINUE`]: ../../../aya-ebpf/bindings/bpf_ret_code/constant.bpf_flow_dissector_continue
 #[derive(Debug)]
 #[doc(alias = "BPF_PROG_TYPE_FLOW_DISSECTOR")]
 pub struct FlowDissector {
@@ -66,29 +79,78 @@ 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(FlowDissectorLinkInner::Fd(
+                    FdLink::new(link_fd),
+                )))
+        } else {
+            let link = ProgAttachLink::attach(
+                prog_fd,
+                netns_fd,
+                BPF_FLOW_DISSECTOR,
+                CgroupAttachMode::Single,
+            )?;
+
+            self.data
+                .links
+                .insert(FlowDissectorLink::new(FlowDissectorLinkInner::ProgAttach(
+                    link,
+                )))
+        }
+    }
+}
+
+#[derive(Debug, Hash, Eq, PartialEq)]
+enum FlowDissectorLinkIdInner {
+    Fd(<FdLink as Link>::Id),
+    ProgAttach(<ProgAttachLink as Link>::Id),
+}
+
+#[derive(Debug)]
+enum FlowDissectorLinkInner {
+    Fd(FdLink),
+    ProgAttach(ProgAttachLink),
+}
+
+impl Link for FlowDissectorLinkInner {
+    type Id = FlowDissectorLinkIdInner;
+
+    fn id(&self) -> Self::Id {
+        match self {
+            Self::Fd(fd) => FlowDissectorLinkIdInner::Fd(fd.id()),
+            Self::ProgAttach(p) => FlowDissectorLinkIdInner::ProgAttach(p.id()),
+        }
+    }
+
+    fn detach(self) -> Result<(), ProgramError> {
+        match self {
+            Self::Fd(fd) => fd.detach(),
+            Self::ProgAttach(p) => p.detach(),
+        }
     }
 }
 
+id_as_key!(FlowDissectorLinkInner, FlowDissectorLinkIdInner);
+
 define_link_wrapper!(
     /// The link used by [FlowDissector] programs.
     FlowDissectorLink,
     /// The type returned by [FlowDissector::attach]. Can be passed to [FlowDissector::detach].
     FlowDissectorLinkId,
-    FdLink,
+    FlowDissectorLinkInner,
-    FdLinkId,
+    FlowDissectorLinkIdInner,
     FlowDissector,
 );

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(&mut self) -> &mut bpf_flow_keys {
-        unsafe { (*self.skb).__bindgen_anon_1.flow_keys as usize }
+        unsafe { &mut *(*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

@@ -2,7 +2,7 @@
 #![no_main]
 
 use aya_ebpf::{
-    bindings::xdp_action,
+    bindings::{bpf_ret_code, xdp_action},
     macros::{flow_dissector, kprobe, kretprobe, tracepoint, uprobe, uretprobe, xdp},
     programs::{
         FlowDissectorContext, ProbeContext, RetProbeContext, TracePointContext, XdpContext,
@@ -48,7 +48,9 @@ pub fn test_uretprobe(_ctx: RetProbeContext) -> u32 {
 
 #[flow_dissector]
 pub fn test_flow(_ctx: FlowDissectorContext) -> u32 {
-    0
+    // TODO: write an actual flow dissector. See tools/testing/selftests/bpf/progs/bpf_flow.c in the
+    // Linux kernel for inspiration.
+    bpf_ret_code::BPF_FLOW_DISSECTOR_CONTINUE
 }
 
 #[cfg(not(test))]

xtask/public-api/aya-ebpf.txt

@@ -1457,7 +1457,8 @@ pub struct aya_ebpf::programs::flow_dissector::FlowDissectorContext
 impl aya_ebpf::programs::flow_dissector::FlowDissectorContext
 pub fn aya_ebpf::programs::flow_dissector::FlowDissectorContext::data(&self) -> usize
 pub fn aya_ebpf::programs::flow_dissector::FlowDissectorContext::data_end(&self) -> usize
-pub fn aya_ebpf::programs::flow_dissector::FlowDissectorContext::flow_keys(&self) -> usize
+pub fn aya_ebpf::programs::flow_dissector::FlowDissectorContext::flow_keys(&mut self) -> &mut aya_ebpf_bindings::x86_64::bindings::bpf_flow_keys
+pub fn aya_ebpf::programs::flow_dissector::FlowDissectorContext::load_bytes(&self, offset: usize, dst: &mut [u8]) -> core::result::Result<usize, aya_ebpf_cty::od::c_long>
 pub fn aya_ebpf::programs::flow_dissector::FlowDissectorContext::new(skb: *mut aya_ebpf_bindings::x86_64::bindings::__sk_buff) -> aya_ebpf::programs::flow_dissector::FlowDissectorContext
 impl aya_ebpf::EbpfContext for aya_ebpf::programs::flow_dissector::FlowDissectorContext
 pub fn aya_ebpf::programs::flow_dissector::FlowDissectorContext::as_ptr(&self) -> *mut aya_ebpf_cty::c_void
@@ -2196,7 +2197,8 @@ pub struct aya_ebpf::programs::FlowDissectorContext
 impl aya_ebpf::programs::flow_dissector::FlowDissectorContext
 pub fn aya_ebpf::programs::flow_dissector::FlowDissectorContext::data(&self) -> usize
 pub fn aya_ebpf::programs::flow_dissector::FlowDissectorContext::data_end(&self) -> usize
-pub fn aya_ebpf::programs::flow_dissector::FlowDissectorContext::flow_keys(&self) -> usize
+pub fn aya_ebpf::programs::flow_dissector::FlowDissectorContext::flow_keys(&mut self) -> &mut aya_ebpf_bindings::x86_64::bindings::bpf_flow_keys
+pub fn aya_ebpf::programs::flow_dissector::FlowDissectorContext::load_bytes(&self, offset: usize, dst: &mut [u8]) -> core::result::Result<usize, aya_ebpf_cty::od::c_long>
 pub fn aya_ebpf::programs::flow_dissector::FlowDissectorContext::new(skb: *mut aya_ebpf_bindings::x86_64::bindings::__sk_buff) -> aya_ebpf::programs::flow_dissector::FlowDissectorContext
 impl aya_ebpf::EbpfContext for aya_ebpf::programs::flow_dissector::FlowDissectorContext
 pub fn aya_ebpf::programs::flow_dissector::FlowDissectorContext::as_ptr(&self) -> *mut aya_ebpf_cty::c_void

xtask/public-api/aya.txt

@@ -3801,10 +3801,6 @@ pub fn aya::programs::flow_dissector::FlowDissectorLink::id(&self) -> Self::Id
 impl core::cmp::Eq for aya::programs::flow_dissector::FlowDissectorLink
 impl core::cmp::PartialEq for aya::programs::flow_dissector::FlowDissectorLink
 pub fn aya::programs::flow_dissector::FlowDissectorLink::eq(&self, other: &Self) -> bool
-impl core::convert::From<aya::programs::flow_dissector::FlowDissectorLink> for aya::programs::links::FdLink
-pub fn aya::programs::links::FdLink::from(w: aya::programs::flow_dissector::FlowDissectorLink) -> aya::programs::links::FdLink
-impl core::convert::From<aya::programs::links::FdLink> for aya::programs::flow_dissector::FlowDissectorLink
-pub fn aya::programs::flow_dissector::FlowDissectorLink::from(b: aya::programs::links::FdLink) -> aya::programs::flow_dissector::FlowDissectorLink
 impl core::fmt::Debug for aya::programs::flow_dissector::FlowDissectorLink
 pub fn aya::programs::flow_dissector::FlowDissectorLink::fmt(&self, f: &mut core::fmt::Formatter<'_>) -> core::fmt::Result
 impl core::hash::Hash for aya::programs::flow_dissector::FlowDissectorLink
@@ -4294,16 +4290,12 @@ impl core::convert::From<aya::programs::fentry::FEntryLink> for aya::programs::l
 pub fn aya::programs::links::FdLink::from(w: aya::programs::fentry::FEntryLink) -> aya::programs::links::FdLink
 impl core::convert::From<aya::programs::fexit::FExitLink> for aya::programs::links::FdLink
 pub fn aya::programs::links::FdLink::from(w: aya::programs::fexit::FExitLink) -> aya::programs::links::FdLink
-impl core::convert::From<aya::programs::flow_dissector::FlowDissectorLink> for aya::programs::links::FdLink
-pub fn aya::programs::links::FdLink::from(w: aya::programs::flow_dissector::FlowDissectorLink) -> aya::programs::links::FdLink
 impl core::convert::From<aya::programs::links::FdLink> for aya::programs::extension::ExtensionLink
 pub fn aya::programs::extension::ExtensionLink::from(b: aya::programs::links::FdLink) -> aya::programs::extension::ExtensionLink
 impl core::convert::From<aya::programs::links::FdLink> for aya::programs::fentry::FEntryLink
 pub fn aya::programs::fentry::FEntryLink::from(b: aya::programs::links::FdLink) -> aya::programs::fentry::FEntryLink
 impl core::convert::From<aya::programs::links::FdLink> for aya::programs::fexit::FExitLink
 pub fn aya::programs::fexit::FExitLink::from(b: aya::programs::links::FdLink) -> aya::programs::fexit::FExitLink
-impl core::convert::From<aya::programs::links::FdLink> for aya::programs::flow_dissector::FlowDissectorLink
-pub fn aya::programs::flow_dissector::FlowDissectorLink::from(b: aya::programs::links::FdLink) -> aya::programs::flow_dissector::FlowDissectorLink
 impl core::convert::From<aya::programs::links::FdLink> for aya::programs::lsm::LsmLink
 pub fn aya::programs::lsm::LsmLink::from(b: aya::programs::links::FdLink) -> aya::programs::lsm::LsmLink
 impl core::convert::From<aya::programs::links::FdLink> for aya::programs::raw_trace_point::RawTracePointLink