Some test progress

Author: daniel-noland

Cargo.toml

@@ -30,3 +30,4 @@ name = "dump_packet_links"
 pcap-file = "1.1.1"
 netlink-sys = { version = "0.8.5" }
 pretty_assertions = "0.7.2"
+hex = "0.4.3"

src/tc/actions/action.rs

@@ -2,29 +2,29 @@
 
 use anyhow::Context;
 use byteorder::{ByteOrder, NativeEndian};
-
-use crate::tc::actions::tunnel_key::{
-    TcActionTunnelKey, TcActionTunnelKeyOption,
-};
 use netlink_packet_utils::{
     nla::{DefaultNla, Nla, NlaBuffer, NlasIterator},
     parsers::{parse_string, parse_u32},
     traits::{Emitable, Parseable, ParseableParametrized},
     DecodeError,
 };
 
+use crate::tc::actions::tunnel_key::{
+    TcActionTunnelKey, TcActionTunnelKeyOption,
+};
+use crate::tc::TcStats2;
+
 use super::{
     TcActionMirror, TcActionMirrorOption, TcActionNat, TcActionNatOption,
 };
-use crate::tc::TcStats2;
 
 const TCA_ACT_TAB: u16 = 1;
 
 /// `TcAction` is a netlink message attribute that describes a [tc-action].
 #[derive(Debug, PartialEq, Eq, Clone)]
 #[non_exhaustive]
 pub struct TcAction {
-    /// Table id.  Should always be `TCA_ACT_TAB` (which equals 1).
+    /// Table id.  Corresponds to the `Kind` of the action.
     pub tab: u16,
     /// Attributes of the action.
     pub attributes: Vec<TcActionAttribute>,
@@ -55,62 +55,53 @@ impl Nla for TcAction {
 
 impl<'a, T: AsRef<[u8]> + ?Sized> Parseable<NlaBuffer<&'a T>> for TcAction {
     fn parse(buf: &NlaBuffer<&'a T>) -> Result<Self, DecodeError> {
-        let mut attributes = vec![];
-        let mut kind = String::new();
-
-        for iter in NlasIterator::new(buf.value()) {
-            let buf = iter.context("invalid action nla")?;
-            let payload = buf.value();
-            attributes.push(match buf.kind() {
-                TCA_ACT_KIND => {
-                    kind = parse_string(payload)
-                        .context("failed to parse TCA_ACT_KIND")?;
-                    TcActionAttribute::Kind(kind.clone())
-                }
-                TCA_ACT_OPTIONS => {
-                    let mut nlas = vec![];
-                    for nla in NlasIterator::new(payload) {
-                        let nla = nla.context("invalid TCA_ACT_OPTIONS")?;
-                        nlas.push(
-                            TcActionOption::parse_with_param(&nla, &kind)
-                                .context(format!(
-                                    "failed to parse TCA_ACT_OPTIONS \
-                                    for kind {kind}"
-                                ))?,
-                        );
+        // We need to find the `Kind` attribute before we can parse the others,
+        // as kind is used in calls to parse_with_param for the other
+        // attributes.
+        // Messages of this type which do not specify `Kind`, or which specify
+        // `Kind` more than once are malformed and should be rejected.
+        // We cannot ensure that `Kind` will be the first attribute in the
+        // `attributes` `Vec` (although it usually is).
+        // As a result, we need to determine `Kind` first, then parse the rest
+        // of the attributes.
+        let kind = match NlasIterator::new(buf.value())
+            .filter_map(|nla| {
+                let nla = match nla {
+                    Ok(nla) => nla,
+                    Err(e) => {
+                        return Some(
+                            Err(e).context("failed to parse action nla"),
+                        )
                     }
-                    TcActionAttribute::Options(nlas)
+                };
+                match nla.kind() {
+                    TCA_ACT_KIND => Some(
+                        parse_string(nla.value())
+                            .context("failed to parse TCA_ACT_KIND"),
+                    ),
+                    _ => None,
                 }
-                TCA_ACT_INDEX => TcActionAttribute::Index(
-                    parse_u32(payload)
-                        .context("failed to parse TCA_ACT_INDEX")?,
-                ),
-                TCA_ACT_STATS => {
-                    let mut nlas = vec![];
-                    for nla in NlasIterator::new(payload) {
-                        let nla = nla.context("invalid TCA_ACT_STATS")?;
-                        nlas.push(
-                            TcStats2::parse_with_param(&nla, &kind).context(
-                                format!(
-                                    "failed to parse TCA_ACT_STATS for \
-                                    kind {kind}",
-                                ),
-                            )?,
-                        );
-                    }
-                    TcActionAttribute::Stats(nlas)
+            })
+            .collect::<Result<Vec<_>, _>>()
+        {
+            Ok(kinds) => {
+                if kinds.is_empty() {
+                    return Err(DecodeError::from("Missing TCA_ACT_KIND"));
                 }
-                TCA_ACT_COOKIE => TcActionAttribute::Cookie(payload.to_vec()),
-                TCA_ACT_IN_HW_COUNT => TcActionAttribute::InHwCount(
-                    parse_u32(payload)
-                        .context("failed to parse TCA_ACT_IN_HW_COUNT")?,
-                ),
-                _ => TcActionAttribute::Other(
-                    DefaultNla::parse(&buf)
-                        .context("failed to parse action nla")?,
-                ),
-            });
-        }
+                if kinds.len() > 1 {
+                    return Err(DecodeError::from("Duplicate TCA_ACT_KIND"));
+                }
+                kinds[0].clone()
+            }
+            Err(e) => return Err(DecodeError::from(e.to_string())),
+        };
+
+        let attributes = NlasIterator::new(buf.value())
+            .map(|nla| {
+                TcActionAttribute::parse_with_param(&nla?, kind.as_str())
+            })
+            .collect::<Result<Vec<_>, _>>()?;
+
         Ok(Self {
             tab: buf.kind(),
             attributes,
@@ -141,6 +132,8 @@ pub enum TcActionAttribute {
     ///
     /// This is used to identify the action in the kernel.
     /// Each action `Kind` has a unique table of actions.
+    /// That is, each action `Kind` has its own set of `Index`
+    /// values.
     ///
     /// If `Index` is zero on action creation,
     /// the kernel will assign a unique index to the new action.
@@ -149,14 +142,14 @@ pub enum TcActionAttribute {
     /// future.
     ///
     /// For example, one action can be used by multiple different filters by
-    /// referencing the action's `Index`.
+    /// referencing the action's `Index` when creating that filter.
     /// Such multiply referenced actions will aggregate their statistics.
     ///
     /// The kernel will reject attempts to delete an action if it is in use by
     /// a filter.
     /// Remove all referencing filters before deleting the action.
     Index(u32),
-    /// Statistics about the action (e.g. number of bytes and or packets
+    /// Statistics about the action (e.g., number of bytes and or packets
     /// processed).
     Stats(Vec<TcStats2>),
     /// An arbitrary identifier which _is not interpreted by the kernel at
@@ -213,11 +206,15 @@ impl Nla for TcActionAttribute {
     }
 }
 
-impl<'a, T> Parseable<NlaBuffer<&'a T>> for TcActionAttribute
+impl<'a, T, P> ParseableParametrized<NlaBuffer<&'a T>, P> for TcActionAttribute
 where
     T: AsRef<[u8]> + ?Sized,
+    P: AsRef<str>,
 {
-    fn parse(buf: &NlaBuffer<&'a T>) -> Result<Self, DecodeError> {
+    fn parse_with_param(
+        buf: &NlaBuffer<&'a T>,
+        kind: P,
+    ) -> Result<Self, DecodeError> {
         Ok(match buf.kind() {
             TCA_ACT_KIND => {
                 let buf_value = buf.value();
@@ -226,19 +223,33 @@ where
                         .context("failed to parse TCA_ACT_KIND")?,
                 )
             }
-            // TCA_ACT_OPTIONS => {
-            //     let mut nlas = vec![];
-            //     for nla in NlasIterator::new(payload) {
-            //         let nla = nla.context("invalid TCA_ACT_OPTIONS")?;
-            //         nlas.push(
-            //             TcActionOption::parse_with_param(&nla, kind)
-            //                 .context(format!(
-            //                     "failed to parse TCA_ACT_OPTIONS for kind
-            // {kind}"                 ))?,
-            //         )
-            //     }
-            //     TcActionAttribute::Options(nlas)
-            // }
+            TCA_ACT_OPTIONS => TcActionAttribute::Options(
+                NlasIterator::new(buf.value())
+                    .map(|nla| {
+                        let nla = nla.context("invalid TCA_ACT_OPTIONS")?;
+                        TcActionOption::parse_with_param(&nla, kind.as_ref())
+                            .context("failed to parse TCA_ACT_OPTIONS")
+                    })
+                    .collect::<Result<Vec<_>, _>>()?,
+            ),
+            TCA_ACT_INDEX => TcActionAttribute::Index(
+                parse_u32(buf.value())
+                    .context("failed to parse TCA_ACT_INDEX")?,
+            ),
+            TCA_ACT_STATS => TcActionAttribute::Stats(
+                NlasIterator::new(buf.value())
+                    .map(|nla| {
+                        let nla = nla.context("invalid TCA_ACT_STATS")?;
+                        TcStats2::parse_with_param(&nla, kind.as_ref())
+                            .context("failed to parse TCA_ACT_STATS")
+                    })
+                    .collect::<Result<Vec<_>, _>>()?,
+            ),
+            TCA_ACT_COOKIE => TcActionAttribute::Cookie(buf.value().to_vec()),
+            TCA_ACT_IN_HW_COUNT => TcActionAttribute::InHwCount(
+                parse_u32(buf.value())
+                    .context("failed to parse TCA_ACT_IN_HW_COUNT")?,
+            ),
             _ => TcActionAttribute::Other(
                 DefaultNla::parse(buf).context("failed to parse action nla")?,
             ),

src/tc/actions/message.rs

@@ -58,8 +58,8 @@ bitflags! {
 /// `selector` field will be ignored.
 #[derive(Debug, PartialEq, Eq, Clone, Copy, Default, PartialOrd, Ord, Hash)]
 pub struct TcActionMessageFlagsWithSelector {
-    flags: TcActionMessageFlags,
-    selector: TcActionMessageFlags,
+    pub flags: TcActionMessageFlags,
+    pub selector: TcActionMessageFlags,
 }
 
 impl Nla for TcActionMessageFlagsWithSelector {