diff --git a/common/src/counters.rs b/common/src/counters.rs index 48bcb3e4..6c132f1f 100644 --- a/common/src/counters.rs +++ b/common/src/counters.rs @@ -13,6 +13,10 @@ pub use dpd_types::counters::{ FecRSCounters, PcsCounters, RMonCounters, RMonCountersAll, }; +/// Per-port counter label for the userspace service port, which has no link +/// of its own on the current switch. +pub const SERVICE_PORT_LABEL: &str = "service"; + /// sidecar.p4 defines the following set of indirect counters. #[derive( Debug, diff --git a/dpd-client/tests/integration_tests/common.rs b/dpd-client/tests/integration_tests/common.rs index 4eaabdf2..7102b235 100644 --- a/dpd-client/tests/integration_tests/common.rs +++ b/dpd-client/tests/integration_tests/common.rs @@ -14,6 +14,7 @@ use std::{fmt, thread}; use anyhow::Context; use anyhow::anyhow; +use anyhow::bail; use oxnet::Ipv4Net; use oxnet::Ipv6Net; use slog::Drain; @@ -45,6 +46,13 @@ pub const ADMIN_LOCAL_MULTICAST_PREFIX: u16 = 0xFF04; // packets before notifying us that any are ready. const TEST_PCAP_TIMEOUT_MS: i32 = 1; +// Interval and attempt count for polling a counter until it reaches an +// expected value. +// +// Note: counter updates from the model can lag the packet itself. +const COUNTER_POLL_INTERVAL: Duration = Duration::from_millis(100); +const COUNTER_POLL_ATTEMPTS: usize = 20; + // Physical port number #[derive(Clone, Copy, PartialOrd, Ord, Hash, PartialEq, Eq)] pub struct PhysPort(pub u16); @@ -791,16 +799,10 @@ impl Switch { } } - match errors.len() { - 0 => Ok(()), - _ => Err(anyhow!( - errors - .iter() - .map(|e| e.to_string()) - .collect::>() - .join(", ") - )), + if !errors.is_empty() { + bail!("{}", errors.join(", ")); } + Ok(()) } /// If no client_name is given, this returns the number of packets dropped @@ -816,26 +818,58 @@ impl Switch { counter: &str, client_name: Option<&str>, ) -> anyhow::Result { - let client_name = if let Some(client_name) = client_name { - client_name.to_string() - } else { - "ingress_drop_reason".to_string() - }; + let client_name = client_name.unwrap_or("ingress_drop_reason"); - self.client - .counter_get(&client_name, true) + let entries = self + .client + .counter_get(client_name, true) .await - .map_err(|e| anyhow::anyhow!("failed to fetch counters: {e:?}")) - .and_then(|entries| { - entries - .iter() - .find(|e| e.keys.get("label").unwrap().as_str() == counter) - .map(|e| e.data.pkts.unwrap()) - .ok_or(anyhow::anyhow!("no such counter: {counter}")) - }) + .map_err(|e| anyhow!("failed to fetch counters: {e:?}"))?; + let Some(entry) = entries + .iter() + .find(|e| e.keys.get("label").unwrap().as_str() == counter) + else { + bail!("no such counter: {counter} in {client_name}"); + }; + Ok(entry.data.pkts.unwrap()) } } +pub async fn check_counter_incremented( + switch: &Switch, + counter_name: &str, + baseline: u64, + expected_increment: u64, + client_name: Option<&str>, +) -> anyhow::Result { + let expected = baseline + expected_increment; + let table = client_name.unwrap_or("ingress_drop_reason"); + let incr_expect = format!( + "counter '{counter_name}' in {table} expected to increase by \ + {expected_increment} ({baseline} -> {expected})" + ); + let mut new_value = 0; + + // Poll for the counter value (with timeout) + for _i in 0..COUNTER_POLL_ATTEMPTS { + thread::sleep(COUNTER_POLL_INTERVAL); + new_value = switch.get_counter(counter_name, client_name).await?; + + if new_value == expected { + return Ok(new_value); + } + if new_value > expected { + bail!("{incr_expect}, but overshot to {new_value}"); + } + } + + // Counter didn't increment as expected + bail!( + "{incr_expect}, but reached {new_value} after \ + {COUNTER_POLL_ATTEMPTS} polls" + ) +} + // Construct a single TCP packet with an optional payload pub fn gen_tcp_packet_loaded( src: Endpoint, @@ -1516,5 +1550,6 @@ pub mod prelude { pub use super::Switch; pub use super::TestPacket; pub use super::TestResult; + pub use super::check_counter_incremented; pub use super::get_switch; } diff --git a/dpd-client/tests/integration_tests/mcast.rs b/dpd-client/tests/integration_tests/mcast.rs index 2eda88c0..568a17e8 100644 --- a/dpd-client/tests/integration_tests/mcast.rs +++ b/dpd-client/tests/integration_tests/mcast.rs @@ -12,12 +12,13 @@ use std::{ use crate::integration_tests::common; use crate::integration_tests::common::prelude::*; +use ::common::counters::SERVICE_PORT_LABEL; use ::common::network::MacAddr; use anyhow::anyhow; use dpd_client::{Error, types}; use futures::TryStreamExt; use oxnet::MulticastMac; -use packet::{Endpoint, eth, geneve, ipv4, ipv6, udp}; +use packet::{Endpoint, eth, geneve, ipv4, ipv6, sidecar, udp}; const MULTICAST_TEST_IPV4: Ipv4Addr = Ipv4Addr::new(224, 0, 1, 0); const MULTICAST_TEST_IPV6: Ipv6Addr = @@ -71,37 +72,6 @@ fn has_vlan_action_for_ip( }) } -async fn check_counter_incremented( - switch: &Switch, - counter_name: &str, - baseline: u64, - expected_increment: u64, - client_name: Option<&str>, -) -> anyhow::Result { - let mut new_value = 0; - - // Poll for the counter value (with timeout) - for _i in 0..20 { - std::thread::sleep(std::time::Duration::from_millis(100)); - new_value = - switch.get_counter(counter_name, client_name).await.unwrap(); - - if new_value == baseline + expected_increment { - return Ok(new_value); - } - } - - // Counter didn't increment as expected - Err(anyhow!( - "Counter '{}' expected to increase by {} (from {} to {}), but only reached {}", - counter_name, - expected_increment, - baseline, - baseline + expected_increment, - new_value - )) -} - async fn create_test_multicast_group( switch: &Switch, group_ip: IpAddr, @@ -2155,9 +2125,27 @@ async fn test_encapped_multicast_geneve_mcast_tag_to_external_members() ]; let port_label_ingress = switch.port_label(ingress).unwrap(); + let port_label_egress1 = switch.port_label(egress1).unwrap(); + let port_label_egress2 = switch.port_label(egress2).unwrap(); let ctr_baseline_ingress = switch.get_counter(&port_label_ingress, Some("ingress")).await.unwrap(); + let ctr_baseline_external_egress1 = switch + .get_counter(&port_label_egress1, Some("multicast_external")) + .await + .unwrap(); + let ctr_baseline_external_egress2 = switch + .get_counter(&port_label_egress2, Some("multicast_external")) + .await + .unwrap(); + let ctr_baseline_underlay_egress1 = switch + .get_counter(&port_label_egress1, Some("multicast_underlay")) + .await + .unwrap(); + let ctr_baseline_underlay_egress2 = switch + .get_counter(&port_label_egress2, Some("multicast_underlay")) + .await + .unwrap(); switch.packet_test(vec![test_pkt], expected_pkts).unwrap(); @@ -2171,6 +2159,44 @@ async fn test_encapped_multicast_geneve_mcast_tag_to_external_members() .await .unwrap(); + // Decapped replicas to external members must be attributed to the + // external counter, not the underlay one. + check_counter_incremented( + switch, + &port_label_egress1, + ctr_baseline_external_egress1, + 1, + Some("multicast_external"), + ) + .await + .unwrap(); + check_counter_incremented( + switch, + &port_label_egress2, + ctr_baseline_external_egress2, + 1, + Some("multicast_external"), + ) + .await + .unwrap(); + + let ctr_underlay_egress1 = switch + .get_counter(&port_label_egress1, Some("multicast_underlay")) + .await + .unwrap(); + let ctr_underlay_egress2 = switch + .get_counter(&port_label_egress2, Some("multicast_underlay")) + .await + .unwrap(); + assert_eq!( + ctr_underlay_egress1, ctr_baseline_underlay_egress1, + "decapped replicas must not hit the underlay counter" + ); + assert_eq!( + ctr_underlay_egress2, ctr_baseline_underlay_egress2, + "decapped replicas must not hit the underlay counter" + ); + cleanup_test_group(switch, get_group_ip(&created_group), TEST_TAG) .await .unwrap(); @@ -2285,9 +2311,27 @@ async fn test_encapped_multicast_geneve_mcast_tag_to_underlay_members() ]; let port_label_ingress = switch.port_label(ingress).unwrap(); + let port_label_egress3 = switch.port_label(egress3).unwrap(); + let port_label_egress4 = switch.port_label(egress4).unwrap(); let ctr_baseline_ingress = switch.get_counter(&port_label_ingress, Some("ingress")).await.unwrap(); + let ctr_baseline_underlay_egress3 = switch + .get_counter(&port_label_egress3, Some("multicast_underlay")) + .await + .unwrap(); + let ctr_baseline_underlay_egress4 = switch + .get_counter(&port_label_egress4, Some("multicast_underlay")) + .await + .unwrap(); + let ctr_baseline_external_egress3 = switch + .get_counter(&port_label_egress3, Some("multicast_external")) + .await + .unwrap(); + let ctr_baseline_external_egress4 = switch + .get_counter(&port_label_egress4, Some("multicast_external")) + .await + .unwrap(); switch.packet_test(vec![test_pkt], expected_pkts).unwrap(); @@ -2301,6 +2345,43 @@ async fn test_encapped_multicast_geneve_mcast_tag_to_underlay_members() .await .unwrap(); + // Encapped replicas to underlay members increment the underlay counter. + check_counter_incremented( + switch, + &port_label_egress3, + ctr_baseline_underlay_egress3, + 1, + Some("multicast_underlay"), + ) + .await + .unwrap(); + check_counter_incremented( + switch, + &port_label_egress4, + ctr_baseline_underlay_egress4, + 1, + Some("multicast_underlay"), + ) + .await + .unwrap(); + + let ctr_external_egress3 = switch + .get_counter(&port_label_egress3, Some("multicast_external")) + .await + .unwrap(); + let ctr_external_egress4 = switch + .get_counter(&port_label_egress4, Some("multicast_external")) + .await + .unwrap(); + assert_eq!( + ctr_external_egress3, ctr_baseline_external_egress3, + "underlay-tagged replicas must not hit the external counter" + ); + assert_eq!( + ctr_external_egress4, ctr_baseline_external_egress4, + "underlay-tagged replicas must not hit the external counter" + ); + cleanup_test_group(switch, get_group_ip(&created_group), TEST_TAG) .await .unwrap(); @@ -2428,9 +2509,45 @@ async fn test_encapped_multicast_geneve_mcast_tag_to_underlay_and_external_membe ]; let port_label_ingress = switch.port_label(ingress).unwrap(); + let port_label_egress1 = switch.port_label(egress1).unwrap(); + let port_label_egress2 = switch.port_label(egress2).unwrap(); + let port_label_egress3 = switch.port_label(egress3).unwrap(); + let port_label_egress4 = switch.port_label(egress4).unwrap(); let ctr_baseline_ingress = switch.get_counter(&port_label_ingress, Some("ingress")).await.unwrap(); + let ctr_baseline_underlay_egress1 = switch + .get_counter(&port_label_egress1, Some("multicast_underlay")) + .await + .unwrap(); + let ctr_baseline_underlay_egress2 = switch + .get_counter(&port_label_egress2, Some("multicast_underlay")) + .await + .unwrap(); + let ctr_baseline_underlay_egress3 = switch + .get_counter(&port_label_egress3, Some("multicast_underlay")) + .await + .unwrap(); + let ctr_baseline_underlay_egress4 = switch + .get_counter(&port_label_egress4, Some("multicast_underlay")) + .await + .unwrap(); + let ctr_baseline_external_egress1 = switch + .get_counter(&port_label_egress1, Some("multicast_external")) + .await + .unwrap(); + let ctr_baseline_external_egress2 = switch + .get_counter(&port_label_egress2, Some("multicast_external")) + .await + .unwrap(); + let ctr_baseline_external_egress3 = switch + .get_counter(&port_label_egress3, Some("multicast_external")) + .await + .unwrap(); + let ctr_baseline_external_egress4 = switch + .get_counter(&port_label_egress4, Some("multicast_external")) + .await + .unwrap(); switch.packet_test(vec![test_pkt], expected_pkts).unwrap(); @@ -2444,12 +2561,223 @@ async fn test_encapped_multicast_geneve_mcast_tag_to_underlay_and_external_membe .await .unwrap(); + // Still-encapsulated replicas to underlay members count as underlay, + // not external, for UNDERLAY_EXTERNAL tagged groups. + check_counter_incremented( + switch, + &port_label_egress3, + ctr_baseline_underlay_egress3, + 1, + Some("multicast_underlay"), + ) + .await + .unwrap(); + check_counter_incremented( + switch, + &port_label_egress4, + ctr_baseline_underlay_egress4, + 1, + Some("multicast_underlay"), + ) + .await + .unwrap(); + + // Decapped replicas to external members count as external. + check_counter_incremented( + switch, + &port_label_egress1, + ctr_baseline_external_egress1, + 1, + Some("multicast_external"), + ) + .await + .unwrap(); + check_counter_incremented( + switch, + &port_label_egress2, + ctr_baseline_external_egress2, + 1, + Some("multicast_external"), + ) + .await + .unwrap(); + + let ctr_external_egress3 = switch + .get_counter(&port_label_egress3, Some("multicast_external")) + .await + .unwrap(); + let ctr_external_egress4 = switch + .get_counter(&port_label_egress4, Some("multicast_external")) + .await + .unwrap(); + assert_eq!( + ctr_external_egress3, ctr_baseline_external_egress3, + "underlay replicas must not hit the external counter" + ); + assert_eq!( + ctr_external_egress4, ctr_baseline_external_egress4, + "underlay replicas must not hit the external counter" + ); + + let ctr_underlay_egress1 = switch + .get_counter(&port_label_egress1, Some("multicast_underlay")) + .await + .unwrap(); + let ctr_underlay_egress2 = switch + .get_counter(&port_label_egress2, Some("multicast_underlay")) + .await + .unwrap(); + assert_eq!( + ctr_underlay_egress1, ctr_baseline_underlay_egress1, + "decapped replicas must not hit the underlay counter" + ); + assert_eq!( + ctr_underlay_egress2, ctr_baseline_underlay_egress2, + "decapped replicas must not hit the underlay counter" + ); + cleanup_test_group(switch, get_group_ip(&created_group), TEST_TAG) .await .unwrap(); cleanup_test_group(switch, MULTICAST_NAT_IP.into(), TEST_TAG).await } +/// Link-local replicas reach egress with a zero replication id, so the +/// counter block reaches them through `is_link_local_ipv6_mcast` rather +/// than `egress_rid`. Both the underlay and external conditions exclude +/// that case, and this checks the exclusion remains after compilation. +#[tokio::test] +#[ignore] +async fn test_link_local_multicast_counter_attribution() -> TestResult { + let switch = &*get_switch().await; + + let egress = PhysPort(10); + + let src = + Endpoint::parse("e0:d5:5e:67:89:ab", "fd00:1122:7788:0101::4", 3333) + .unwrap(); + let dst = Endpoint::parse("33:33:00:00:00:01", "ff02::1", 4444).unwrap(); + + // Sourced from userspace so the packet egresses without replication, + // mirroring test_link_local_multicast_outbound in route_ipv6. + let mut send = common::gen_udp_packet(src, dst); + common::add_sidecar_hdr( + switch, + &mut send, + sidecar::SC_FWD_FROM_USERSPACE, + NO_PORT, + egress, + None, + ); + let test_pkt = TestPacket { packet: Arc::new(send), port: SERVICE_PORT }; + let expected_pkts = vec![TestPacket { + packet: Arc::new(common::gen_udp_packet(src, dst)), + port: egress, + }]; + + let port_label_egress = switch.port_label(egress).unwrap(); + + let ctr_baseline_link_local = switch + .get_counter(&port_label_egress, Some("multicast_link_local")) + .await + .unwrap(); + let ctr_baseline_external = switch + .get_counter(&port_label_egress, Some("multicast_external")) + .await + .unwrap(); + let ctr_baseline_underlay = switch + .get_counter(&port_label_egress, Some("multicast_underlay")) + .await + .unwrap(); + + switch.packet_test(vec![test_pkt], expected_pkts).unwrap(); + + check_counter_incremented( + switch, + &port_label_egress, + ctr_baseline_link_local, + 1, + Some("multicast_link_local"), + ) + .await + .unwrap(); + + let ctr_external = switch + .get_counter(&port_label_egress, Some("multicast_external")) + .await + .unwrap(); + let ctr_underlay = switch + .get_counter(&port_label_egress, Some("multicast_underlay")) + .await + .unwrap(); + assert_eq!( + ctr_external, ctr_baseline_external, + "link-local replicas must not hit the external counter" + ); + assert_eq!( + ctr_underlay, ctr_baseline_underlay, + "link-local replicas must not hit the underlay counter" + ); + + Ok(()) +} + +#[tokio::test] +#[ignore] +async fn test_link_local_multicast_inbound_counter_attribution() -> TestResult { + let switch = &*get_switch().await; + + let ingress = PhysPort(10); + + let src = + Endpoint::parse("e0:d5:5e:67:89:ab", "fd00:1122:7788:0101::4", 3333) + .unwrap(); + let dst = Endpoint::parse("33:33:00:00:00:01", "ff02::1", 4444).unwrap(); + + let send = TestPacket { + packet: Arc::new(common::gen_udp_packet(src, dst)), + port: ingress, + }; + let mut recv = common::gen_udp_packet(src, dst); + common::add_sidecar_hdr( + switch, + &mut recv, + sidecar::SC_FWD_TO_USERSPACE, + ingress, + NO_PORT, + None, + ); + let expected_pkts = + vec![TestPacket { packet: Arc::new(recv), port: SERVICE_PORT }]; + + let ctr_baseline_link_local = switch + .get_counter(SERVICE_PORT_LABEL, Some("multicast_link_local")) + .await?; + let ctr_baseline_unicast = + switch.get_counter(SERVICE_PORT_LABEL, Some("unicast")).await?; + + switch.packet_test(vec![send], expected_pkts)?; + + check_counter_incremented( + switch, + SERVICE_PORT_LABEL, + ctr_baseline_link_local, + 1, + Some("multicast_link_local"), + ) + .await?; + check_counter_incremented( + switch, + SERVICE_PORT_LABEL, + ctr_baseline_unicast, + 0, + Some("unicast"), + ) + .await?; + + Ok(()) +} + #[tokio::test] #[ignore] async fn test_ipv4_multicast_drops_ingress_is_egress_port() -> TestResult { diff --git a/dpd-client/tests/integration_tests/route_ipv4.rs b/dpd-client/tests/integration_tests/route_ipv4.rs index 75f7eceb..482d2a23 100644 --- a/dpd-client/tests/integration_tests/route_ipv4.rs +++ b/dpd-client/tests/integration_tests/route_ipv4.rs @@ -9,6 +9,7 @@ use std::net::Ipv4Addr; use std::sync::Arc; use anyhow::anyhow; +use anyhow::bail; use oxnet::Ipv4Net; use reqwest::StatusCode; @@ -89,9 +90,7 @@ async fn validate_routes( ) -> TestResult { if expected.is_empty() { match client.route_ipv4_get(&cidr).await { - Ok(f) => { - Err(anyhow!("found {} targets - expected no route", f.len())) - } + Ok(f) => bail!("found {} targets - expected no route", f.len()), Err(_) => Ok(()), } } else { @@ -133,7 +132,28 @@ async fn test_route_ipv4_unicast_impl(switch: &Switch) -> TestResult { #[ignore] async fn test_route_ipv4_unicast() -> TestResult { let switch = &*get_switch().await; - test_route_ipv4_unicast_impl(switch).await + let label = switch.port_label(PhysPort(14)).unwrap(); + + let unicast = switch.get_counter(&label, Some("unicast")).await?; + let mcast = switch.get_counter(&label, Some("multicast")).await?; + let link_local = + switch.get_counter(&label, Some("multicast_link_local")).await?; + + test_route_ipv4_unicast_impl(switch).await?; + + check_counter_incremented(switch, &label, unicast, 1, Some("unicast")) + .await?; + check_counter_incremented(switch, &label, mcast, 0, Some("multicast")) + .await?; + check_counter_incremented( + switch, + &label, + link_local, + 0, + Some("multicast_link_local"), + ) + .await?; + Ok(()) } async fn test_deleted_route_ipv4_impl(switch: &Switch) -> TestResult { @@ -583,7 +603,8 @@ async fn fill_until_ipv4_filler_alloc_unavailable( Err(e) if e.status() == Some(StatusCode::INSUFFICIENT_STORAGE) => { assert!( i > 0, - "alloc({SHRINK_TEST_FILLER_TARGETS}) failed before any IPv4 fillers" + "alloc({SHRINK_TEST_FILLER_TARGETS}) failed before any \ + IPv4 fillers" ); validate_routes( client, @@ -593,20 +614,19 @@ async fn fill_until_ipv4_filler_alloc_unavailable( .await?; return Ok(fillers); } - Err(e) => { - return Err(anyhow!( - "unexpected error probing IPv4 \ - alloc({SHRINK_TEST_FILLER_TARGETS}) availability at \ - filler prefix {cidr}: {e:?}" - )); - } + Err(e) => bail!( + "unexpected error probing IPv4 \ + alloc({SHRINK_TEST_FILLER_TARGETS}) availability at \ + filler prefix {cidr}: {e:?}" + ), } } - Err(anyhow!( - "IPv4 route target allocation of size {SHRINK_TEST_FILLER_TARGETS} did not fail after \ + bail!( + "IPv4 route target allocation of size \ + {SHRINK_TEST_FILLER_TARGETS} did not fail after \ {MAX_ROUTE_TARGET_FILLERS} filler prefixes" - )) + ) } #[tokio::test] diff --git a/dpd/p4/metadata.p4 b/dpd/p4/metadata.p4 index 26616e73..17173f74 100644 --- a/dpd/p4/metadata.p4 +++ b/dpd/p4/metadata.p4 @@ -11,7 +11,8 @@ header bridge_h { PortId_t ingress_port; bool nat_egress_hit; - bit<6> reserved; + bool is_link_local_mcastv6; + bit<5> reserved; } struct sidecar_ingress_meta_t { diff --git a/dpd/p4/parser.p4 b/dpd/p4/parser.p4 index 73bae827..4b14d479 100644 --- a/dpd/p4/parser.p4 +++ b/dpd/p4/parser.p4 @@ -57,6 +57,7 @@ parser IngressParser( meta.bridge_hdr.setValid(); meta.bridge_hdr.ingress_port = ig_intr_md.ingress_port; meta.bridge_hdr.nat_egress_hit = false; + meta.bridge_hdr.is_link_local_mcastv6 = false; meta.bridge_hdr.reserved = 0; transition port_metadata; diff --git a/dpd/p4/sidecar.p4 b/dpd/p4/sidecar.p4 index b037f099..d73dd3f6 100644 --- a/dpd/p4/sidecar.p4 +++ b/dpd/p4/sidecar.p4 @@ -1832,7 +1832,7 @@ control MulticastIngress ( } else if (hdr.geneve.isValid() && hdr.inner_ipv6.isValid()) { // Check if the inner destination address is an IPv6 multicast // address (ff00::/8). Apply source filtering for both SSM - // (ff3x::/16) and ASM ranges. + // (ff3x::/32) and ASM ranges. if (hdr.inner_ipv6.dst_addr[127:120] == 8w0xff) { mcast_source_filter_ipv6.apply(); } else { @@ -2189,6 +2189,8 @@ control Ingress( if (meta.nat_egress_hit && !meta.service_routed) { meta.bridge_hdr.nat_egress_hit = true; } + } else if (meta.is_link_local_mcastv6) { + meta.bridge_hdr.is_link_local_mcastv6 = true; } if (meta.encap_needed) { @@ -2374,11 +2376,8 @@ control Egress( // Check multicast egress packets by checking that RID is not 0. bool is_egress_rid_mcast = eg_intr_md.egress_rid > 0; // We track IPv6 multicast packets separately for counters. - bool is_link_local_ipv6_mcast = false; - if (hdr.ipv6.isValid()) { - bit<16> ipv6_prefix = (bit<16>)hdr.ipv6.dst_addr[127:112]; - is_link_local_ipv6_mcast = (ipv6_prefix == IPV6_LINK_LOCAL_16); - } + bool is_link_local_ipv6_mcast = + meta.bridge_hdr.is_link_local_mcastv6; bool is_mcast = is_egress_rid_mcast || is_link_local_ipv6_mcast; if (is_egress_rid_mcast == true) { @@ -2409,14 +2408,36 @@ control Egress( if (is_mcast == true) { mcast_ctr.count(eg_intr_md.egress_port); - if (is_link_local_ipv6_mcast) { - link_local_mcast_ctr.count(eg_intr_md.egress_port); - } else if (hdr.geneve.isValid()) { - external_mcast_ctr.count(eg_intr_md.egress_port); - } else if (hdr.geneve.isValid() && - hdr.geneve_opts.oxg_mcast.isValid() && - hdr.geneve_opts.oxg_mcast.mcast_tag == MULTICAST_TAG_UNDERLAY) { + // Per-type counters: + // + // - link-local (ff02::/16 outer dst) -> always + // forwarded without PRE (Packet Replication Engine) + // replication, arriving with egress_rid == 0. + // - egress_rid > 0 with a valid Geneve header -> an + // underlay replica, still encapsulated. + // - egress_rid > 0 without a valid Geneve header -> an + // external replica, decapped at ingress (external-only + // groups) or by mcast_egress above (bifurcated groups). + // + // Geneve validity separates the two replica cases. + // Underlay replicas stay encapsulated whether or not the + // group is bifurcated, meaning that we can skip an + // mcast_tag check. + // + // This chain branches on egress_rid first. Within + // the scope of is_mcast, a zero rid already implies + // link-local, making the last branch avoid a test of + // is_link_local_ipv6_mcast. Scope 2 groups are rejected + // at the time of creation, meaning that no PRE replica + // would carry a link-local outer destination at this + // point. + + if (is_egress_rid_mcast && hdr.geneve.isValid()) { underlay_mcast_ctr.count(eg_intr_md.egress_port); + } else if (is_egress_rid_mcast) { + external_mcast_ctr.count(eg_intr_md.egress_port); + } else { + link_local_mcast_ctr.count(eg_intr_md.egress_port); } } else { unicast_ctr.count(eg_intr_md.egress_port); diff --git a/dpd/src/counters.rs b/dpd/src/counters.rs index b189cb6f..fc25c0da 100644 --- a/dpd/src/counters.rs +++ b/dpd/src/counters.rs @@ -24,11 +24,26 @@ use aal_macros::*; use asic::Handle; use common::counters::CounterId; use common::counters::EgressCounterId; +use common::counters::SERVICE_PORT_LABEL; use common::table::TableType; use anyhow::Context; use dpd_types::table::TableCounterEntry; +/// The ASIC port carrying packets to and from userspace, matching +/// `USER_SPACE_SERVICE_PORT` in `sidecar.p4`. +/// +/// On the current switch, this is the PCIe CPU port, which has no link. The +/// `int0` link maps to the Ethernet CPU port, as reported by +/// `bf_eth_cpu_port_get` (ASIC port 2 on the Tofino model). +/// Without a label for ASIC port 0, its per-port counters get omitted from +/// the response. +/// +/// Note: the SDE's PCIe CPU port selection can be found within +/// `lld_get_pcie_cpu_port`: +/// +const USER_SPACE_SERVICE_PORT: u16 = 0; + // Counters in an indirect table are accessed by their index number rather than // a key. Still, we define a key anyway to allow us to use the direct counter // infrastructure to extract the counter data. @@ -274,10 +289,13 @@ fn reason_label(ctr: u8) -> Result, String> { // dozen are associated with configured links at any given time. If this ID // doesn't match to a port, it's not an error. async fn port_label(switch: &Switch, ctr: u16) -> Option { - switch - .asic_port_id_to_port_link(ctr) - .map(|(port, link)| format!("{port}/{link}")) - .ok() + match switch.asic_port_id_to_port_link(ctr) { + Ok((port, link)) => Some(format!("{port}/{link}")), + Err(_) if ctr == USER_SPACE_SERVICE_PORT => { + Some(SERVICE_PORT_LABEL.to_string()) + } + Err(_) => None, + } } /// Fetch all of the accumulated values for this counter.