Add stage to count packet processing results

cli/bin/cmdtree_dp.rs

@@ -287,6 +287,14 @@ fn cmd_show_config_summary() -> Node {
     root
 }
 
+fn cmd_show_packet_stats() -> Node {
+    let mut root = Node::new("packet");
+    root += Node::new("stats")
+        .desc("Show packet stats")
+        .action(CliAction::ShowPacketStats as u16);
+    root
+}
+
 fn cmd_show_tech() -> Node {
     Node::new("tech")
         .desc("Dump dataplanes state")
@@ -307,6 +315,7 @@ fn cmd_show() -> Node {
     root += cmd_show_flow_filter();
     root += cmd_show_gateway();
     root += cmd_show_config_summary();
+    root += cmd_show_packet_stats();
     root += cmd_show_tech();
     root
 }

cli/src/cliproto.rs

@@ -184,6 +184,9 @@ pub enum CliAction {
     // NF: flow filter
     ShowFlowFilter,
 
+    // NF: Packet stats
+    ShowPacketStats,
+
     // internal config
     ShowConfigInternal,
 

dataplane/src/packet_processor/mod.rs

@@ -19,9 +19,10 @@ use nat::portfw::{PortForwarder, PortFwTableWriter};
 use nat::stateful::NatAllocatorWriter;
 use nat::stateless::NatTablesWriter;
 use nat::{IcmpErrorHandler, StatefulNat, StatelessNat};
+use net::packet::PacketStats;
 
 use net::buffer::PacketBufferMut;
-use pipeline::sample_nfs::PacketDumper;
+use pipeline::sample_nfs::{PacketDumper, PacketStatsNF};
 use pipeline::{DynPipeline, PipelineData};
 
 use routing::{CliSources, Router, RouterError, RouterParams};
@@ -68,6 +69,7 @@ pub(crate) fn start_router<Buf: PacketBufferMut>(
     let portfw_w = PortFwTableWriter::new();
     let portfw_factory = portfw_w.reader().factory();
     let pdata = Arc::from(PipelineData::new(0));
+    let pkt_stats = Arc::from(PacketStats::new());
 
     // collect readers and the like for cli
     let cli_sources = CliSources {
@@ -76,6 +78,7 @@ pub(crate) fn start_router<Buf: PacketBufferMut>(
         portfw_table: Some(Box::new(portfw_w.reader().inner())),
         nat_tables: Some(Box::new(nattabler_factory.handle().inner())),
         masquerade_state: Some(Box::new(natallocator_factory.handle().inner())),
+        pkt_stats: Some(Box::new(pkt_stats.clone())),
     };
 
     // create router
@@ -109,6 +112,7 @@ pub(crate) fn start_router<Buf: PacketBufferMut>(
             portfw_factory.handle(),
             flow_table.clone(),
         );
+        let pkt_stats_nf = PacketStatsNF::new(pkt_stats.clone());
 
         // Build the pipeline for a router. The composition of the pipeline (in stages) is currently
         // hard-coded. In any pipeline, the Stats and ExpirationsNF stages should go last
@@ -126,6 +130,7 @@ pub(crate) fn start_router<Buf: PacketBufferMut>(
             .add_stage(stage_egress)
             .add_stage(flow_expirations_nf)
             .add_stage(pktdump)
+            .add_stage(pkt_stats_nf)
             .add_stage(stats_stage)
     };
 

nat/src/portfw/nf.rs

@@ -84,7 +84,7 @@ impl PortForwarder {
             && (!tcp.syn() || tcp.ack())
         {
             debug!("Dropping TCP segment: it has no SYN (or ack) and we have no state for it");
-            packet.done(DoneReason::Filtered);
+            packet.done(DoneReason::NatNotPortForwarded);
             return None;
         }
         let Some(dst_port) = transport.dst_port() else {
@@ -143,7 +143,7 @@ impl PortForwarder {
 
         // check if the packet can be port forwarded at all
         let Some((key, dst_ip, dst_port)) = Self::can_be_port_forwarded(packet) else {
-            packet.done(DoneReason::Filtered);
+            packet.done(DoneReason::NatNotPortForwarded);
             let reason = packet.get_done().unwrap_or_else(|| unreachable!());
             debug!("{nfi}: packet cannot be port-forwarded. Dropping it (reason:{reason})");
             return;
@@ -152,15 +152,15 @@ impl PortForwarder {
         // lookup the port-forwarding rule, using the given key, that contains the destination port
         let Some(entry) = pfwtable.lookup_matching_rule(key, dst_ip.inner(), dst_port) else {
             debug!("{nfi}: no rule found for port-forwarding key {key}. Dropping packet.");
-            packet.done(DoneReason::Filtered);
+            packet.done(DoneReason::NatNotPortForwarded);
             return;
         };
 
         // map the destination address and port
         let Some((new_dst_ip, new_dst_port)) = entry.map_address_port(dst_ip.inner(), dst_port)
         else {
             debug!("{nfi}: Unable to build usable address and port"); // FIXME:
-            packet.done(DoneReason::Filtered);
+            packet.done(DoneReason::InternalFailure);
             return;
         };
 
@@ -319,7 +319,7 @@ impl PortForwarder {
                 debug!("Packet hit Active flow referring to STALE port-forwarding rule.");
                 let Some(entry) = Self::get_rule_from_pkt(packet, pfwtable, &state) else {
                     debug!("Packet should no longer be forwarded. Will drop and invalidate state");
-                    packet.done(DoneReason::Filtered);
+                    packet.done(DoneReason::NatNotPortForwarded);
                     invalidate_flow_state(packet);
                     return;
                 };

nat/src/portfw/test.rs

@@ -268,7 +268,7 @@ mod nf_test {
         let tcp = packet.try_tcp_mut().unwrap();
         tcp.set_syn(false);
         let output = process_packet(&mut pipeline, packet);
-        assert_eq!(output.get_done(), Some(DoneReason::Filtered));
+        assert_eq!(output.get_done(), Some(DoneReason::NatNotPortForwarded));
 
         // process a packet in reverse direction: no flow info should have been found
         let packet = tcp_packet_reverse_reply();
@@ -528,8 +528,7 @@ mod nf_test {
 
         let packet = tcp_packet_to_port_forward();
         let output = process_packet(&mut pipeline, packet);
-        assert_eq!(output.get_done(), Some(DoneReason::Filtered));
-        //        assert!(get_pfw_flow_status(&output).is_none());
+        assert_eq!(output.get_done(), Some(DoneReason::NatNotPortForwarded));
 
         println!("{flow_table}");
 
@@ -538,7 +537,7 @@ mod nf_test {
 
         let packet = tcp_packet_to_port_forward();
         let output = process_packet(&mut pipeline, packet);
-        assert_eq!(output.get_done(), Some(DoneReason::Filtered)); // should be filtered
+        assert_eq!(output.get_done(), Some(DoneReason::NatNotPortForwarded));
         assert_eq!(get_flow_status(&output), None); // expiration NF should have removed the flow
         assert!(get_pfw_flow_status(&output).is_none());
         println!("{flow_table}");
@@ -817,7 +816,7 @@ mod nf_test {
         );
         let rule_referenced = get_pfw_flow_state_rule(&output);
         assert!(rule_referenced.is_none()); // flow did not get a new reference to a rule
-        assert_eq!(output.get_done(), Some(DoneReason::Filtered)); // packet was dropped
+        assert_eq!(output.get_done(), Some(DoneReason::NatNotPortForwarded)); // packet was dropped
 
         println!("{flow_table}");
     }

nat/src/stateful/mod.rs

@@ -535,7 +535,7 @@ fn translate_error(error: &StatefulNatError) -> DoneReason {
 
         StatefulNatError::BadTransportHeader
         | StatefulNatError::AllocationFailure(AllocatorError::UnsupportedProtocol(_)) => {
-            DoneReason::UnsupportedTransport
+            DoneReason::NatUnsupportedProto
         }
 
         StatefulNatError::TupleParseError | StatefulNatError::InvalidPort(_) => {

nat/src/stateless/mod.rs

@@ -362,7 +362,7 @@ fn translate_error(error: &StatelessNatError) -> DoneReason {
         StatelessNatError::NoIpHeader
         | StatelessNatError::IcmpErrorMsg(IcmpErrorMsgError::BadIpHeader) => DoneReason::NotIp,
 
-        StatelessNatError::UnsupportedTranslation => DoneReason::UnsupportedTransport,
+        StatelessNatError::UnsupportedTranslation => DoneReason::NatUnsupportedProto,
 
         StatelessNatError::MissingTable(_) => DoneReason::Unroutable,
 
@@ -379,7 +379,7 @@ fn translate_error(error: &StatelessNatError) -> DoneReason {
 
         StatelessNatError::IcmpErrorMsg(
             IcmpErrorMsgError::BadChecksumIcmp(_) | IcmpErrorMsgError::BadChecksumInnerIpv4(_),
-        ) => DoneReason::Filtered,
+        ) => DoneReason::InvalidChecksum,
     }
 }
 

net/Cargo.toml

@@ -19,6 +19,7 @@ atomic-instant-full = { workspace = true }
 bitflags = { workspace = true }
 bolero = { workspace = true, features = ["alloc", "arbitrary", "std"], optional = true }
 bytecheck = { workspace = true }
+common = { workspace = true }
 concurrency = { workspace = true }
 derive_builder = { workspace = true, features = ["alloc"] }
 downcast-rs = { workspace = true, features = ["sync"] }
@@ -28,6 +29,8 @@ linux-raw-sys = { workspace = true, features = ["std", "if_ether"] }
 multi_index_map = { workspace = true, default-features = false, features = ["serde"] }
 rapidhash = { workspace = true }
 rkyv = { workspace = true, features = ["alloc", "bytecheck"]}
+strum = { workspace = true }
+strum_macros = { workspace = true }
 serde = { workspace = true, features = ["derive", "std"] }
 thiserror = { workspace = true }
 tracing = { workspace = true }

net/src/packet/display.rs

@@ -16,7 +16,7 @@ use crate::vlan::Vlan;
 use crate::buffer::PacketBufferMut;
 use crate::checksum::Checksum;
 #[allow(deprecated)]
-use crate::packet::{BridgeDomain, DoneReason, InvalidPacket, Packet, PacketMeta};
+use crate::packet::{BridgeDomain, InvalidPacket, Packet, PacketMeta};
 use std::fmt::{Display, Formatter};
 
 impl Display for Eth {
@@ -226,12 +226,6 @@ impl Display for Headers {
 }
 
 /* ============= METADATA =============== */
-impl Display for DoneReason {
-    fn fmt(&self, f: &mut Formatter<'_>) -> std::fmt::Result {
-        // using Debug at the moment since the enum may soon change
-        write!(f, "{self:?}")
-    }
-}
 
 fn fmt_opt<T: Display>(
     f: &mut Formatter<'_>,

net/src/packet/meta.rs

@@ -12,9 +12,9 @@ use crate::vxlan::Vni;
 use bitflags::bitflags;
 use concurrency::sync::Arc;
 use serde::Serialize;
-use std::collections::HashMap;
 use std::fmt::Display;
 use std::net::IpAddr;
+use strum_macros::EnumCount;
 use tracing::error;
 
 /// Every VRF is univocally identified with a numerical VRF id
@@ -77,36 +77,47 @@ impl Display for VpcDiscriminant {
     }
 }
 
+#[repr(u8)]
 #[allow(unused)]
-#[derive(Copy, Clone, Debug, Eq, Hash, PartialEq)]
+#[derive(Copy, Clone, Debug, Eq, Hash, PartialEq, EnumCount)]
 pub enum DoneReason {
     InternalFailure,      /* catch-all for internal issues */
-    NotEthernet,          /* could not get eth header */
-    NotIp,                /* could not get IP header - maybe it's not ip */
-    UnsupportedTransport, /* unsupported transport layer */
-    MacNotForUs,          /* frame is not broadcast nor for us */
+    InterfaceUnknown,     /* the interface cannot be found */
     InterfaceDetached,    /* interface has not been attached to any VRF */
     InterfaceAdmDown,     /* interface is admin down */
     InterfaceOperDown,    /* interface is oper down : no link */
-    InterfaceUnknown,     /* the interface cannot be found */
     InterfaceUnsupported, /* the operation is not supported on the interface */
-    NatOutOfResources,    /* can't do NAT due to lack of resources */
+    NotEthernet,          /* could not get eth header */
+    Unhandled,            /* there exists no support to handle this type of packet */
+    MacNotForUs,          /* frame is not broadcast nor for us */
+    InvalidDstMac,        /* dropped the packet since it had to have an invalid destination mac */
+    MissingEtherType,     /* couldn't determine ethertype to use */
+    Filtered,             /* The packet was administratively filtered */
+    NotIp,                /* could not get IP header or packet is not IP */
     RouteFailure,         /* missing routing information */
     RouteDrop,            /* routing explicitly requests pkts to be dropped */
     HopLimitExceeded,     /* TTL / Hop count was exceeded */
-    Filtered,             /* The packet was administratively filtered */
-    Unhandled,            /* there exists no support to handle this type of packet */
     MissL2resolution,     /* adjacency failure: we don't know mac of some ip next-hop */
-    InvalidDstMac,        /* dropped the packet since it had to have an invalid destination mac */
+    NatOutOfResources,    /* can't do NAT due to lack of resources */
+    NatUnsupportedProto,  /* unsupported transport protocol for NATing */
+    NatFailure,           /* It was not possible to NAT the packet */
+    NatNotPortForwarded,  /* Packet was sent to port forwarder and it rejected it */
     Malformed,            /* the packet does not conform / is malformed */
-    MissingEtherType,     /* can't determine ethertype to use */
     Unroutable,           /* we don't have state to forward the packet */
-    NatFailure,           /* It was not possible to NAT the packet */
-    Local,                /* the packet has to be locally consumed by kernel */
-    Delivered,            /* the packet buffer was delivered by the NF - e.g. for xmit */
-    InternalDrop,         /* the packet was dropped internally due to a queue being full */
     InvalidChecksum,      /* the validation of a checksum for this packet failed */
     IcmpErrorIncomplete,  /* the packet is an ICMP error but it does not contain data it should */
+    InternalDrop,         /* the packet was dropped internally due to a queue being full */
+    Local,                /* the packet has to be locally consumed by kernel */
+    Delivered,            /* the packet buffer was delivered by the NF - e.g. for xmit */
+}
+
+impl From<u8> for DoneReason {
+    fn from(value: u8) -> Self {
+        #[allow(unsafe_code)]
+        unsafe {
+            std::mem::transmute(value)
+        }
+    }
 }
 
 bitflags! {
@@ -256,63 +267,3 @@ impl Drop for PacketMeta {
         }
     }
 }
-
-#[derive(Default, Debug)]
-#[allow(unused)]
-pub struct PacketDropStats {
-    pub name: String,
-    reasons: HashMap<DoneReason, u64>,
-    //Fredi: Todo: replace by ahash or use a small vec indexed by the DropReason value
-}
-
-impl PacketDropStats {
-    #[allow(dead_code)]
-    #[must_use]
-    pub fn new(name: &str) -> Self {
-        Self {
-            name: name.to_owned(),
-            reasons: HashMap::default(),
-        }
-    }
-    #[allow(dead_code)]
-    pub fn incr(&mut self, reason: DoneReason, value: u64) {
-        self.reasons
-            .entry(reason)
-            .and_modify(|counter| *counter += value)
-            .or_insert(value);
-    }
-    #[allow(dead_code)]
-    #[must_use]
-    pub fn get_stat(&self, reason: DoneReason) -> Option<u64> {
-        self.reasons.get(&reason).copied()
-    }
-    #[allow(dead_code)]
-    #[must_use]
-    pub fn get_stats(&self) -> &HashMap<DoneReason, u64> {
-        &self.reasons
-    }
-}
-
-#[cfg(test)]
-pub mod test {
-    use super::DoneReason;
-    use super::PacketDropStats;
-
-    #[test]
-    fn test_packet_drop_stats() {
-        let mut stats = PacketDropStats::new("Stats:pipeline-FOO-stage-BAR");
-        stats.incr(DoneReason::InterfaceAdmDown, 10);
-        stats.incr(DoneReason::InterfaceAdmDown, 1);
-        stats.incr(DoneReason::RouteFailure, 9);
-        stats.incr(DoneReason::Unroutable, 13);
-
-        // look up some particular stats
-        assert_eq!(stats.get_stat(DoneReason::InterfaceAdmDown), Some(11));
-        assert_eq!(stats.get_stat(DoneReason::Unroutable), Some(13));
-        assert_eq!(stats.get_stat(DoneReason::InterfaceUnsupported), None);
-
-        // access the whole stats map
-        let read = stats.get_stats();
-        assert_eq!(read.get(&DoneReason::InterfaceAdmDown), Some(11).as_ref());
-    }
-}

net/src/packet/mod.rs

@@ -6,6 +6,9 @@
 mod display;
 mod hash;
 mod meta;
+mod stats;
+
+pub use stats::PacketStats;
 
 #[cfg(any(test, feature = "bolero"))]
 pub use contract::*;