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..32bc6317 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; @@ -791,16 +792,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,23 +811,20 @@ 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()) } } diff --git a/dpd-client/tests/integration_tests/counters.rs b/dpd-client/tests/integration_tests/counters.rs index 1f50d7ff..d62343a1 100644 --- a/dpd-client/tests/integration_tests/counters.rs +++ b/dpd-client/tests/integration_tests/counters.rs @@ -11,7 +11,9 @@ use std::net::Ipv4Addr; use std::sync::Arc; +use std::time::Duration; +use anyhow::bail; use packet::Endpoint; use crate::integration_tests::common; @@ -22,6 +24,13 @@ use dpd_client::ClientInfo; use dpd_client::types::Ipv4Entry; use dpd_client::types::Ipv6Entry; +// 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; + /// Awaits an increase in a given counter's value from `baseline` to /// `baseline + increment`. /// @@ -33,6 +42,12 @@ pub async fn check_counter_incremented( 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; // Briefly delay before reading the counter each time, as there is @@ -40,25 +55,23 @@ pub async fn check_counter_incremented( // simulator and when that updated value is available to the SDE. To // avoid a long pointless delay here, we try multiple times with a short // sleep rather than once with a long sleep. - for _ in 0..20 { - tokio::time::sleep(std::time::Duration::from_millis(100)).await; - new_value = - switch.get_counter(counter_name, client_name).await.unwrap(); + for _ in 0..COUNTER_POLL_ATTEMPTS { + tokio::time::sleep(COUNTER_POLL_INTERVAL).await; + new_value = switch.get_counter(counter_name, client_name).await?; - if new_value == baseline + expected_increment { + 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. - Err(anyhow::anyhow!( - "Counter '{}' expected to increase by {} (from {} to {}), but only reached {}", - counter_name, - expected_increment, - baseline, - baseline + expected_increment, - new_value - )) + bail!( + "{incr_expect}, but reached {new_value} after \ + {COUNTER_POLL_ATTEMPTS} polls" + ) } /// Process a single packet, asserting that a given counter is incremented by @@ -68,10 +81,11 @@ pub async fn one_counted_test( packet: TestPacket, packet_after: Option, counter: &str, + client_name: Option<&str>, ) -> anyhow::Result { - let old = switch.get_counter(counter, None).await?; + let old = switch.get_counter(counter, client_name).await?; switch.packet_test(vec![packet], packet_after.into_iter().collect())?; - check_counter_incremented(switch, counter, old, 1, None) + check_counter_incremented(switch, counter, old, 1, client_name) .await .map(|new| old + 1 == new) } @@ -89,6 +103,7 @@ async fn one_drop_test( TestPacket { packet: Arc::new(packet), port }, None, counter, + None, ) .await } diff --git a/dpd-client/tests/integration_tests/mcast.rs b/dpd-client/tests/integration_tests/mcast.rs index b6d78ee3..267e99dd 100644 --- a/dpd-client/tests/integration_tests/mcast.rs +++ b/dpd-client/tests/integration_tests/mcast.rs @@ -13,12 +13,13 @@ use std::{ use crate::integration_tests::common; use crate::integration_tests::common::prelude::*; use crate::integration_tests::counters::check_counter_incremented; +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 = @@ -2125,9 +2126,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(); @@ -2141,6 +2160,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(); @@ -2255,9 +2312,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(); @@ -2271,6 +2346,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(); @@ -2398,9 +2510,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(); @@ -2414,12 +2562,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 7e2d2aac..d1f0975b 100644 --- a/dpd-client/tests/integration_tests/route_ipv4.rs +++ b/dpd-client/tests/integration_tests/route_ipv4.rs @@ -9,11 +9,13 @@ use std::net::Ipv4Addr; use std::sync::Arc; use anyhow::anyhow; +use anyhow::bail; use oxnet::Ipv4Net; use reqwest::StatusCode; use crate::integration_tests::common; use crate::integration_tests::common::prelude::*; +use crate::integration_tests::counters::check_counter_incremented; use crate::integration_tests::counters::one_counted_test; use packet::Endpoint; use packet::eth::EthQHdr; @@ -90,9 +92,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 { @@ -127,14 +127,45 @@ async fn test_route_ipv4_unicast_impl(switch: &Switch) -> TestResult { let send = TestPacket { packet: Arc::new(to_send), port: ingress }; let expected = TestPacket { packet: Arc::new(to_recv), port: egress }; - switch.packet_test(vec![send], vec![expected]) + let label = switch.port_label(egress).unwrap(); + + assert!( + one_counted_test( + switch, + send, + Some(expected), + &label, + Some("unicast"), + ) + .await? + ); + + Ok(()) } #[tokio::test] #[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 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, 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 { @@ -161,8 +192,14 @@ async fn test_deleted_route_ipv4_impl(switch: &Switch) -> TestResult { let expected = TestPacket { packet: Arc::new(to_recv), port: egress }; assert!( - one_counted_test(switch, send, Some(expected), "ipv4_unrouteable") - .await? + one_counted_test( + switch, + send, + Some(expected), + "ipv4_unrouteable", + None, + ) + .await? ); Ok(()) @@ -269,8 +306,14 @@ async fn test_route_ipv4_unrouteable() -> TestResult { let expected = TestPacket { packet: Arc::new(to_recv), port: egress }; assert!( - one_counted_test(switch, send, Some(expected), "ipv4_unrouteable") - .await? + one_counted_test( + switch, + send, + Some(expected), + "ipv4_unrouteable", + None, + ) + .await? ); Ok(()) @@ -594,7 +637,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, @@ -604,20 +648,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] @@ -957,8 +1000,14 @@ async fn test_v4_over_v6() -> TestResult { let expected = TestPacket { packet: Arc::new(to_recv), port: SERVICE_PORT }; assert!( - one_counted_test(switch, send, Some(expected), "ipv4_unrouteable") - .await? + one_counted_test( + switch, + send, + Some(expected), + "ipv4_unrouteable", + None, + ) + .await? ); // Re-add the route and verify forwarding works again. @@ -996,8 +1045,14 @@ async fn test_v4_over_v6() -> TestResult { let send = TestPacket { packet: Arc::new(to_send), port: ingress }; let expected = TestPacket { packet: Arc::new(to_recv), port: SERVICE_PORT }; assert!( - one_counted_test(switch, send, Some(expected), "ipv4_unrouteable") - .await? + one_counted_test( + switch, + send, + Some(expected), + "ipv4_unrouteable", + None, + ) + .await? ); Ok(()) diff --git a/dpd-client/tests/integration_tests/route_ipv6.rs b/dpd-client/tests/integration_tests/route_ipv6.rs index 1ae6398f..76f6250f 100644 --- a/dpd-client/tests/integration_tests/route_ipv6.rs +++ b/dpd-client/tests/integration_tests/route_ipv6.rs @@ -203,8 +203,14 @@ async fn test_deleted_unicast_impl(switch: &Switch) -> TestResult { common::set_icmp6_unreachable(switch, &mut to_recv, ingress); let expected = TestPacket { packet: Arc::new(to_recv), port: egress }; assert!( - one_counted_test(switch, send, Some(expected), "ipv6_unrouteable") - .await? + one_counted_test( + switch, + send, + Some(expected), + "ipv6_unrouteable", + None, + ) + .await? ); Ok(()) @@ -286,8 +292,14 @@ async fn test_unrouteable() -> TestResult { common::set_icmp6_unreachable(switch, &mut to_recv, ingress); let expected = TestPacket { packet: Arc::new(to_recv), port: egress }; assert!( - one_counted_test(switch, send, Some(expected), "ipv6_unrouteable") - .await? + one_counted_test( + switch, + send, + Some(expected), + "ipv6_unrouteable", + None, + ) + .await? ); Ok(()) diff --git a/dpd-client/tests/integration_tests/service.rs b/dpd-client/tests/integration_tests/service.rs index 78e70bee..af6bd7d9 100644 --- a/dpd-client/tests/integration_tests/service.rs +++ b/dpd-client/tests/integration_tests/service.rs @@ -223,7 +223,10 @@ async fn test_arp_needed() -> TestResult { let expected = TestPacket { packet: Arc::new(recv_pkt), port: SERVICE_PORT }; - assert!(one_counted_test(switch, send, Some(expected), "arp_miss").await?); + assert!( + one_counted_test(switch, send, Some(expected), "arp_miss", None) + .await? + ); Ok(()) } @@ -272,7 +275,10 @@ async fn test_ndp_needed() -> TestResult { let expected = TestPacket { packet: Arc::new(recv_pkt), port: SERVICE_PORT }; - assert!(one_counted_test(switch, send, Some(expected), "ndp_miss").await?); + assert!( + one_counted_test(switch, send, Some(expected), "ndp_miss", None) + .await? + ); Ok(()) } @@ -321,8 +327,14 @@ async fn test_ttl_exceeded() -> TestResult { TestPacket { packet: Arc::new(recv_pkt), port: SERVICE_PORT }; assert!( - one_counted_test(switch, send, Some(expected), "ipv4_ttl_exceeded") - .await? + one_counted_test( + switch, + send, + Some(expected), + "ipv4_ttl_exceeded", + None, + ) + .await? ); Ok(()) @@ -374,8 +386,14 @@ async fn test_hop_limit_exceeded() -> TestResult { TestPacket { packet: Arc::new(recv_pkt), port: SERVICE_PORT }; assert!( - one_counted_test(switch, send, Some(expected), "ipv6_ttl_exceeded") - .await? + one_counted_test( + switch, + send, + Some(expected), + "ipv6_ttl_exceeded", + None, + ) + .await? ); Ok(()) 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 efd0a729..6d49b000 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 { @@ -2195,6 +2195,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) { @@ -2380,11 +2382,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) { @@ -2415,14 +2414,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.