tracing tune up

dataplane/Cargo.toml

@@ -35,9 +35,9 @@ routing = { workspace = true }
 serde = { workspace = true, features = ["derive"] }
 stats = { workspace = true }
 tokio = { workspace = true }
-tracing = { workspace = true }
-tracing-subscriber = { workspace = true, features = ["default"] }
 tracectl = { workspace = true }
+tracing = { workspace = true }
+tracing-subscriber = { workspace = true, default-features = true }
 vpcmap = { workspace = true }
 
 [dev-dependencies]

dataplane/src/packet_processor/ingress.rs

@@ -9,7 +9,7 @@ use tracing::{debug, trace, warn};
 
 use net::buffer::PacketBufferMut;
 use net::eth::mac::Mac;
-use net::headers::{TryEth, TryIpv4, TryIpv6};
+use net::headers::{TryEth, TryIp};
 use net::packet::{DoneReason, Packet};
 use pipeline::NetworkFunction;
 
@@ -19,7 +19,7 @@ use routing::interfaces::interface::{Attachment, IfState, IfType, Interface};
 use tracectl::trace_target;
 trace_target!("ingress", LevelFilter::WARN, &["pipeline"]);
 
-#[allow(unused)]
+#[derive(Debug)]
 pub struct Ingress {
     name: String,
     iftr: IfTableReader,
@@ -45,7 +45,7 @@ impl Ingress {
         packet: &mut Packet<Buf>,
     ) {
         let nfi = self.name();
-        if packet.try_ipv4().is_some() || packet.try_ipv6().is_some() {
+        if packet.try_ip().is_some() {
             match &interface.attachment {
                 Some(Attachment::VRF(fibr)) => {
                     let Some(vrfid) = fibr.get_id().map(|x| x.as_u32()) else {
@@ -69,30 +69,39 @@ impl Ingress {
         }
     }
 
+    #[tracing::instrument(level = "trace")]
     fn interface_ingress_eth_non_local<Buf: PacketBufferMut>(
         &self,
-        _interface: &Interface,
+        interface: &Interface,
         dst_mac: Mac,
         packet: &mut Packet<Buf>,
     ) {
-        let nfi = self.name();
         /* Here we would check if the interface is part of some
         bridge domain. But we don't support bridging yet. */
-        trace!("{nfi}: Recvd frame for mac {dst_mac} (not for us)");
+        trace!(
+            "{nfi}: Recvd frame for mac {dst_mac} (not for us) (interface {ifname})",
+            nfi = self.name(),
+            ifname = interface.name
+        );
         packet.done(DoneReason::MacNotForUs);
     }
 
+    #[tracing::instrument(level = "trace")]
     fn interface_ingress_eth_bcast<Buf: PacketBufferMut>(
         &self,
-        _interface: &Interface,
+        interface: &Interface,
         packet: &mut Packet<Buf>,
     ) {
         let nfi = self.name();
         packet.get_meta_mut().set_l2bcast(true);
         packet.done(DoneReason::Unhandled);
-        warn!("{nfi}: Processing of broadcast ethernet frames is not supported");
+        warn!(
+            "{nfi}: Processing of broadcast ethernet frames is not supported (interface {ifname})",
+            ifname = interface.name
+        );
     }
 
+    #[tracing::instrument(level = "trace")]
     fn interface_ingress_eth<Buf: PacketBufferMut>(
         &self,
         interface: &Interface,
@@ -122,6 +131,7 @@ impl Ingress {
         }
     }
 
+    #[tracing::instrument(level = "trace")]
     fn interface_ingress<Buf: PacketBufferMut>(
         &self,
         interface: &Interface,
@@ -150,7 +160,6 @@ impl<Buf: PacketBufferMut> NetworkFunction<Buf> for Ingress {
         &'a mut self,
         input: Input,
     ) -> impl Iterator<Item = Packet<Buf>> + 'a {
-        trace!("{}", self.name);
         input.filter_map(move |mut packet| {
             let nfi = self.name();
             if !packet.is_done() {

dataplane/src/packet_processor/ipforward.rs

@@ -6,13 +6,12 @@
 #![allow(clippy::similar_names)]
 
 use arrayvec::ArrayVec;
-use std::net::IpAddr;
-use tracing::{debug, error, trace, warn};
-
 use net::headers::{TryHeadersMut, TryIpv4Mut, TryIpv6Mut};
 use net::packet::{DoneReason, Packet};
 use net::{buffer::PacketBufferMut, checksum::Checksum};
 use pipeline::NetworkFunction;
+use std::net::IpAddr;
+use tracing::{debug, error, trace, warn};
 
 use routing::fib::fibobjects::{EgressObject, FibEntry, PktInstruction};
 use routing::fib::fibtable::FibTable;
@@ -66,7 +65,7 @@ impl IpForwarder {
 
         /* get destination ip address */
         let Some(dst) = packet.ip_destination() else {
-            error!("{nfi}: Failed to get destination ip address for packet");
+            error!("{nfi}: logic error, failed to get destination ip address for packet");
             packet.done(DoneReason::InternalFailure);
             return;
         };
@@ -86,13 +85,13 @@ impl IpForwarder {
         };
         /* Lookup the fib which needs to be consulted */
         let Some(fibr) = fibtr.get_fib(&fibid) else {
-            error!("{nfi}: Unable to find fib with id {fibid} for vrf {vrfid}");
+            warn!("{nfi}: Unable to find fib with id {fibid} for vrf {vrfid}");
             packet.done(DoneReason::InternalFailure);
             return;
         };
         /* Read-only access to fib */
         let Some(fib) = fibr.enter() else {
-            error!("{nfi}: Unable to read from fib {fibid}");
+            warn!("{nfi}: Unable to read from fib {fibid}");
             packet.done(DoneReason::InternalFailure);
             return;
         };
@@ -106,14 +105,14 @@ impl IpForwarder {
             if !fibentry.is_iplocal() {
                 Self::decrement_ttl(packet, dst);
                 if packet.is_done() {
-                    warn!("TTL/Hop-count limit exceeded!");
+                    debug!("TTL/Hop-count limit exceeded!");
                     return;
                 }
             }
             /* execute instructions according to FIB */
             self.packet_exec_instructions(&fibtr, packet, fibentry, fib.get_vtep());
         } else {
-            error!("Could not get fib group for {prefix}. Will drop packet...");
+            debug!("Could not get fib group for {prefix}. Will drop packet...");
             packet.done(DoneReason::InternalFailure);
         }
     }
@@ -142,7 +141,7 @@ impl IpForwarder {
                     return;
                 };
                 let Some(next_vrf) = fib.get_id().map(|id| id.as_u32()) else {
-                    error!("{nfi}: Failed to access fib {fibid} to determine vrf");
+                    debug!("{nfi}: Failed to access fib {fibid} to determine vrf");
                     packet.done(DoneReason::InternalFailure);
                     return;
                 };
@@ -156,7 +155,7 @@ impl IpForwarder {
                 packet.get_meta_mut().set_nat(true);
             }
             Some(Err(bad)) => {
-                warn!("The decapsulated packet is malformed!: {bad:?}");
+                debug!("The decapsulated packet is malformed!: {bad:#?}");
                 packet.done(DoneReason::Malformed);
             }
             None => {
@@ -266,7 +265,7 @@ impl IpForwarder {
         // build vxlan headers for encapsulation
         match Self::build_vxlan_headers(vxlan, vtep) {
             Err(e) => {
-                error!("{nfi}: Failed to build VxLAN headers: {e}");
+                warn!("{nfi}: Failed to build VxLAN headers: {e}");
                 packet.done(DoneReason::InternalFailure);
             }
             Ok(vxlan_headers) => match packet.vxlan_encap(&vxlan_headers) {
@@ -388,6 +387,7 @@ impl IpForwarder {
 }
 
 impl<Buf: PacketBufferMut> NetworkFunction<Buf> for IpForwarder {
+    #[tracing::instrument(level = "trace", skip(self, input))]
     fn process<'a, Input: Iterator<Item = Packet<Buf>> + 'a>(
         &'a mut self,
         input: Input,

pipeline/src/pipeline.rs

@@ -35,7 +35,6 @@ pub enum PipelineError {
 
 impl<Buf: PacketBufferMut> DynPipeline<Buf> {
     /// Create a [`DynPipeline`].
-    #[allow(unused)]
     #[must_use]
     pub fn new() -> Self {
         Self {
@@ -47,7 +46,6 @@ impl<Buf: PacketBufferMut> DynPipeline<Buf> {
     ///
     /// This method takes a [`NetworkFunction`] and adds it to the pipeline.
     ///
-    #[allow(unused)]
     #[must_use]
     pub fn add_stage<NF: NetworkFunction<Buf> + 'static>(self, nf: NF) -> Self {
         self.add_stage_dyn(nf_dyn(nf))
@@ -144,7 +142,6 @@ impl<Buf: PacketBufferMut> DynPipeline<Buf> {
     ///
     /// # See Also
     ///
-    #[allow(unused)]
     pub fn get_stage_by_id<T: NetworkFunction<Buf> + 'static>(
         &self,
         id: &StageId<Buf>,
@@ -159,7 +156,6 @@ impl<Buf: PacketBufferMut> DynPipeline<Buf> {
     ///
     /// # See Also
     ///
-    #[allow(unused)]
     #[must_use]
     pub fn get_stage_dyn_by_id<T: DynNetworkFunction<Buf>>(&self, id: &StageId<Buf>) -> Option<&T> {
         self.nfs

routing/src/interfaces/interface.rs

@@ -67,7 +67,7 @@ pub enum IfState {
     Up = 2,
 }
 
-#[derive(Clone)]
+#[derive(Debug, Clone)]
 pub enum Attachment {
     VRF(FibReader),
     BD,
@@ -122,7 +122,7 @@ impl RouterInterfaceConfig {
     }
 }
 
-#[derive(Clone)]
+#[derive(Debug, Clone)]
 /// An object representing a network interface and its state
 pub struct Interface {
     pub name: String,