Move expected_revision out of the LinkOrder struct

Create a new TcxOptions struct similar to NlOptions containing LinkOrder
and expected_revision.

Signed-off-by: Andre Fredette <afredette@redhat.com>

aya/src/programs/links.rs

@@ -410,7 +410,7 @@ impl LinkId {
     }
 }
 
-#[derive(Debug)]
+#[derive(Debug, Copy, Clone)]
 pub(crate) enum LinkRef {
     Id(u32),
     Fd(RawFd),
@@ -438,19 +438,26 @@ bitflags::bitflags! {
 ///
 ///```no_run
 /// # let mut bpf = aya::Ebpf::load(&[])?;
-/// use aya::programs::{tc, SchedClassifier, TcAttachType, tc::TcAttachOptions, LinkOrder};
+/// use aya::{
+///     programs::{
+///         tc::{TcAttachOptions, TcxOptions},
+///         LinkOrder, SchedClassifier, TcAttachType,
+///     },
+/// };
 ///
 /// let prog: &mut SchedClassifier = bpf.program_mut("redirect_ingress").unwrap().try_into()?;
 /// prog.load()?;
-/// let options = TcAttachOptions::TcxOrder(LinkOrder::first());
+/// let options = TcAttachOptions::Tcx(TcxOptions {
+///     link_order: LinkOrder::first(),
+///     expected_revision: None, // incorrect expected_revision
+/// });
 /// prog.attach_with_options("eth0", TcAttachType::Ingress, options)?;
 ///
 /// # Ok::<(), aya::EbpfError>(())
 /// ```
-#[derive(Debug)]
+#[derive(Debug, Copy, Clone)]
 pub struct LinkOrder {
     pub(crate) link_ref: LinkRef,
-    pub(crate) expected_revision: Option<u64>,
     pub(crate) flags: MprogFlags,
 }
 
@@ -460,7 +467,6 @@ impl Default for LinkOrder {
         Self {
             link_ref: LinkRef::Fd(0),
             flags: MprogFlags::AFTER,
-            expected_revision: None,
         }
     }
 }
@@ -471,7 +477,6 @@ impl LinkOrder {
         Self {
             link_ref: LinkRef::Id(0),
             flags: MprogFlags::BEFORE,
-            expected_revision: None,
         }
     }
 
@@ -480,7 +485,6 @@ impl LinkOrder {
         Self {
             link_ref: LinkRef::Id(0),
             flags: MprogFlags::AFTER,
-            expected_revision: None,
         }
     }
 
@@ -489,7 +493,6 @@ impl LinkOrder {
         Ok(Self {
             link_ref: LinkRef::Fd(link.fd()?.as_raw_fd()),
             flags: MprogFlags::BEFORE | MprogFlags::LINK,
-            expected_revision: None,
         })
     }
 
@@ -498,7 +501,6 @@ impl LinkOrder {
         Ok(Self {
             link_ref: LinkRef::Fd(link.fd()?.as_raw_fd()),
             flags: MprogFlags::AFTER | MprogFlags::LINK,
-            expected_revision: None,
         })
     }
 
@@ -507,7 +509,6 @@ impl LinkOrder {
         Ok(Self {
             link_ref: LinkRef::Id(id.0),
             flags: MprogFlags::BEFORE | MprogFlags::LINK | MprogFlags::ID,
-            expected_revision: None,
         })
     }
 
@@ -516,7 +517,6 @@ impl LinkOrder {
         Ok(Self {
             link_ref: LinkRef::Id(id.0),
             flags: MprogFlags::AFTER | MprogFlags::LINK | MprogFlags::ID,
-            expected_revision: None,
         })
     }
 
@@ -525,7 +525,6 @@ impl LinkOrder {
         Ok(Self {
             link_ref: LinkRef::Fd(program.fd()?.as_raw_fd()),
             flags: MprogFlags::BEFORE,
-            expected_revision: None,
         })
     }
 
@@ -534,7 +533,6 @@ impl LinkOrder {
         Ok(Self {
             link_ref: LinkRef::Fd(program.fd()?.as_raw_fd()),
             flags: MprogFlags::AFTER,
-            expected_revision: None,
         })
     }
 
@@ -543,7 +541,6 @@ impl LinkOrder {
         Self {
             link_ref: LinkRef::Id(id.0),
             flags: MprogFlags::BEFORE | MprogFlags::ID,
-            expected_revision: None,
         }
     }
 
@@ -552,17 +549,8 @@ impl LinkOrder {
         Self {
             link_ref: LinkRef::Id(id.0),
             flags: MprogFlags::AFTER | MprogFlags::ID,
-            expected_revision: None,
         }
     }
-
-    /// set the expected revision for the link, the revision changes
-    /// with each modification of the list of attached programs. User space
-    /// can pass an expected revision when creating a new link. The kernel
-    /// then rejects the update if the revision has changed.
-    pub fn set_expected_revision(&mut self, revision: u64) {
-        self.expected_revision = Some(revision);
-    }
 }
 
 #[cfg(test)]

aya/src/programs/tc.rs

@@ -128,16 +128,16 @@ impl TcAttachType {
 /// older than 6.6.0 must utilize netlink for attachments, while newer kernels
 /// can utilize the modern TCX eBPF link type which supports the kernel's
 /// multi-prog API.
-#[derive(Debug)]
+#[derive(Debug, Copy, Clone)]
 pub enum TcAttachOptions {
     /// Netlink attach options.
     Netlink(NlOptions),
     /// Tcx attach options.
-    TcxOrder(LinkOrder),
+    Tcx(TcxOptions),
 }
 
 /// Options for SchedClassifier attach via netlink.
-#[derive(Debug, Default, Hash, Eq, PartialEq)]
+#[derive(Debug, Default, Hash, Eq, PartialEq, Copy, Clone)]
 pub struct NlOptions {
     /// Priority assigned to tc program with lower number = higher priority.
     /// If set to default (0), the system chooses the next highest priority or 49152 if no filters exist yet
@@ -147,6 +147,21 @@ pub struct NlOptions {
     pub handle: u32,
 }
 
+/// Options for SchedClassifier attach via TCX.
+#[derive(Debug, Default, Copy, Clone)]
+pub struct TcxOptions {
+    /// Attributes that define the position of the program in the TCX chain.
+    pub link_order: LinkOrder,
+    /// Expected revision for the attach operation.  The kernel maintains a
+    /// revision for each attach point (interface/direction).  Each time a
+    /// program is attached or detached to a given attach point, the revision
+    /// for that attach point is incremented.  If the expected revision does not
+    /// match the revision, the attach will fail.
+    ///
+    /// TODO: Implement an API to query the current revision for an interface.
+    pub expected_revision: Option<u64>,
+}
+
 impl SchedClassifier {
     /// Loads the program inside the kernel.
     pub fn load(&mut self) -> Result<(), ProgramError> {
@@ -179,7 +194,7 @@ impl SchedClassifier {
             self.attach_with_options(
                 interface,
                 attach_type,
-                TcAttachOptions::TcxOrder(LinkOrder::default()),
+                TcAttachOptions::Tcx(TcxOptions::default()),
             )
         } else {
             self.attach_with_options(
@@ -294,14 +309,14 @@ impl SchedClassifier {
                         handle,
                     })))
             }
-            TcAttachOptions::TcxOrder(options) => {
+            TcAttachOptions::Tcx(options) => {
                 let link_fd = bpf_link_create(
                     prog_fd,
                     LinkTarget::IfIndex(if_index),
                     attach_type.tcx_attach_type()?,
                     None,
-                    options.flags.bits(),
-                    Some(&options.link_ref),
+                    options.link_order.flags.bits(),
+                    Some(&options.link_order.link_ref),
                     options.expected_revision,
                 )
                 .map_err(|(_, io_error)| SyscallError {

test/integration-test/src/tests/tcx.rs

@@ -5,7 +5,10 @@ use std::{
 };
 
 use aya::{
-    programs::{tc::TcAttachOptions, LinkOrder, SchedClassifier, TcAttachType},
+    programs::{
+        tc::{TcAttachOptions, TcxOptions},
+        LinkOrder, SchedClassifier, TcAttachType,
+    },
     util::KernelVersion,
     Ebpf, EbpfLoader,
 };
@@ -105,17 +108,19 @@ async fn tcx_ordering() {
     prog3.load().unwrap();
 
     // Test LinkOrder::last() with correct expected_revision
-    let mut order: LinkOrder = LinkOrder::last();
-    order.set_expected_revision(1);
-    let options = TcAttachOptions::TcxOrder(order);
+    let options = TcAttachOptions::Tcx(TcxOptions {
+        link_order: LinkOrder::first(),
+        expected_revision: Some(1),
+    });
     prog0
         .attach_with_options("lo", TcAttachType::Ingress, options)
         .unwrap();
 
     // Test LinkOrder::after_program() with correct expected_revision
-    let mut order = LinkOrder::after_program(prog0).unwrap();
-    order.set_expected_revision(2);
-    let options = TcAttachOptions::TcxOrder(order);
+    let options = TcAttachOptions::Tcx(TcxOptions {
+        link_order: LinkOrder::after_program(prog0).unwrap(),
+        expected_revision: Some(2),
+    });
     let prog1_link_id = prog1
         .attach_with_options("lo", TcAttachType::Ingress, options)
         .unwrap();
@@ -123,23 +128,33 @@ async fn tcx_ordering() {
     let prog1_link = prog1.take_link(prog1_link_id).unwrap();
 
     // Test incorrect expected_revision and expect an error
-    let mut order = LinkOrder::after_link(&prog1_link).unwrap();
-    order.set_expected_revision(7);
-    let options = TcAttachOptions::TcxOrder(order);
-    let result = prog2.attach_with_options("lo", TcAttachType::Ingress, options);
+    let mut tcx_options = TcxOptions {
+        link_order: LinkOrder::after_link(&prog1_link).unwrap(),
+        expected_revision: Some(7), // incorrect expected_revision
+    };
+    let result = prog2.attach_with_options(
+        "lo",
+        TcAttachType::Ingress,
+        TcAttachOptions::Tcx(tcx_options),
+    );
     assert!(result.is_err());
 
-    // Test LinkOrder::after_link() again with expected_revision == 0 which
-    // means the expected_revision should be ignored.
-    let mut order = LinkOrder::after_link(&prog1_link).unwrap();
-    order.set_expected_revision(0);
-    let options = TcAttachOptions::TcxOrder(order);
+    // Test LinkOrder::after_link() again after updating expected_revision to 3
+    // which should be the correct revision.
+    tcx_options.expected_revision = Some(3);
     prog2
-        .attach_with_options("lo", TcAttachType::Ingress, options)
+        .attach_with_options(
+            "lo",
+            TcAttachType::Ingress,
+            TcAttachOptions::Tcx(tcx_options),
+        )
         .unwrap();
 
     // Test LinkOrder::last() with no expected_revision
-    let options = TcAttachOptions::TcxOrder(LinkOrder::last());
+    let options = TcAttachOptions::Tcx(TcxOptions {
+        link_order: LinkOrder::last(),
+        expected_revision: None, // incorrect expected_revision
+    });
     prog3
         .attach_with_options("lo", TcAttachType::Ingress, options)
         .unwrap();