diff --git a/dpd-client/tests/integration_tests/common.rs b/dpd-client/tests/integration_tests/common.rs index 7102b235..c0d1f485 100644 --- a/dpd-client/tests/integration_tests/common.rs +++ b/dpd-client/tests/integration_tests/common.rs @@ -690,10 +690,19 @@ impl Switch { } pub fn packet_test( + &self, + send: Vec, + expect: Vec, + ) -> TestResult { + self.packet_test_matched(send, expect)?; + Ok(()) + } + + pub fn packet_test_matched( &self, mut send: Vec, mut expect: Vec, - ) -> TestResult { + ) -> anyhow::Result> { // NOTE: This timeout is critical. // // The tests are normally run against the Tofino simulator, a software @@ -755,11 +764,12 @@ impl Switch { } // We should also see the packets we expected to see + let mut matched = Vec::with_capacity(expect.len()); for (e, expected) in expect.iter().enumerate() { for (g, got) in captured.iter().enumerate() { if expected == got { found.push(e); - captured.remove(g); + matched.push(captured.remove(g)); break; } } @@ -802,7 +812,7 @@ impl Switch { if !errors.is_empty() { bail!("{}", errors.join(", ")); } - Ok(()) + Ok(matched) } /// If no client_name is given, this returns the number of packets dropped diff --git a/dpd-client/tests/integration_tests/counters.rs b/dpd-client/tests/integration_tests/counters.rs index f793f2a6..b9801d47 100644 --- a/dpd-client/tests/integration_tests/counters.rs +++ b/dpd-client/tests/integration_tests/counters.rs @@ -18,9 +18,11 @@ use crate::integration_tests::common; use crate::integration_tests::common::prelude::*; use crate::integration_tests::icmp_ipv4; use ::common::network::MacAddr; +use ::common::network::Vni; use dpd_client::ClientInfo; use dpd_client::types::Ipv4Entry; use dpd_client::types::Ipv6Entry; +use dpd_client::types::NatTarget; // Run a single drop test. This sends a packet that we expect to be dropped, // and verifies that the expected drop counter is bumped by one. If the test @@ -181,6 +183,42 @@ async fn test_ipv4_ttl_invalid() -> TestResult { test_ipv4_ttl_test(0, "ipv4_ttl_invalid").await } +#[tokio::test] +#[ignore] +async fn test_ipv4_ttl_invalid_nat_ingress() -> TestResult { + let switch = &*get_switch().await; + let ingress = PhysPort(14); + let nat_ip = "192.168.1.2"; + let nat_port = 2000; + + let tgt = NatTarget { + internal_ip: "fd00:1122:7788:0101::4".parse().unwrap(), + inner_mac: "04:01:01:01:01:01".parse::()?.into(), + vni: Vni::new(7654).unwrap().into(), + }; + switch + .client + .nat_ipv4_create(&nat_ip.parse().unwrap(), 1024, 2048, &tgt) + .await + .unwrap(); + + let mut packet = common::gen_udp_packet( + Endpoint::parse("02:aa:bb:cc:dd:ee", "10.10.10.32", 4444).unwrap(), + Endpoint::parse( + &switch.get_port_mac(ingress).unwrap().to_string(), + nat_ip, + nat_port, + ) + .unwrap(), + ); + packet.hdrs.ipv4_hdr.as_mut().unwrap().ipv4_ttl = 0; + packet::ipv4::Ipv4Hdr::update_checksum(&mut packet); + + assert!(one_drop_test(switch, ingress, packet, "ipv4_ttl_invalid").await?); + + Ok(()) +} + /// Set a port to accept only inbound traffic with a corresponding NAT mapping. /// Send in a packet that has no corresponding NAT entry, and check the /// nat_ingress_miss counter. diff --git a/dpd-client/tests/integration_tests/mcast.rs b/dpd-client/tests/integration_tests/mcast.rs index 568a17e8..53bda3d3 100644 --- a/dpd-client/tests/integration_tests/mcast.rs +++ b/dpd-client/tests/integration_tests/mcast.rs @@ -1860,13 +1860,13 @@ async fn test_multicast_ttl_one() -> TestResult { let expected_pkts = vec![]; let ctr_baseline = - switch.get_counter("ipv4_ttl_invalid", None).await.unwrap(); + switch.get_counter("multicast_ttl_exceeded", None).await.unwrap(); switch.packet_test(vec![test_pkt], expected_pkts).unwrap(); check_counter_incremented( switch, - "ipv4_ttl_invalid", + "multicast_ttl_exceeded", ctr_baseline, 1, None, @@ -2642,6 +2642,726 @@ async fn test_encapped_multicast_geneve_mcast_tag_to_underlay_and_external_membe cleanup_test_group(switch, MULTICAST_NAT_IP.into(), TEST_TAG).await } +fn build_inner_ttl_geneve_pkt( + external_ip: IpAddr, + src_mac: MacAddr, + vlan: Option, + nat_target: &types::NatTarget, + inner_ttl: u8, + mcast_tag: u8, + inner_eth: bool, +) -> (packet::Packet, packet::Packet) { + let src_port = 3333; + let dst_port = 4444; + + let og_pkt = match external_ip { + IpAddr::V4(_) => { + let mut pkt = create_ipv4_multicast_packet( + external_ip, + src_mac, + "192.168.1.10", + src_port, + dst_port, + vlan, + ); + let current = pkt.hdrs.ipv4_hdr.as_ref().unwrap().ipv4_ttl; + ipv4::Ipv4Hdr::adjust_ttl( + &mut pkt, + i16::from(inner_ttl) - i16::from(current), + ); + pkt + } + IpAddr::V6(_) => { + let mut pkt = create_ipv6_multicast_packet( + external_ip, + src_mac, + "2001:db8::1", + src_port, + dst_port, + vlan, + ); + let current = pkt.hdrs.ipv6_hdr.as_ref().unwrap().ipv6_hop_lim; + ipv6::Ipv6Hdr::adjust_hlim( + &mut pkt, + i16::from(inner_ttl) - i16::from(current), + ); + pkt + } + }; + + let (payload, ether_type) = if inner_eth { + let mut inner = og_pkt.clone(); + inner.hdrs.eth_hdr.as_mut().unwrap().eth_8021q = None; + (inner.deparse().unwrap().to_vec(), eth::ETHER_ETHER) + } else { + let eth_hdr_len = og_pkt.hdrs.eth_hdr.as_ref().unwrap().hdr_len(); + let ether_type = match external_ip { + IpAddr::V4(_) => eth::ETHER_IPV4, + IpAddr::V6(_) => eth::ETHER_IPV6, + }; + (og_pkt.deparse().unwrap()[eth_hdr_len..].to_vec(), ether_type) + }; + + let geneve_pkt = common::gen_geneve_packet_with_mcast_tag( + Endpoint::parse( + GIMLET_MAC, + &GIMLET_IP.to_string(), + geneve::GENEVE_UDP_PORT, + ) + .unwrap(), + Endpoint::parse( + &MacAddr::from(nat_target.internal_ip.derive_multicast_mac()) + .to_string(), + &nat_target.internal_ip.to_string(), + geneve::GENEVE_UDP_PORT, + ) + .unwrap(), + ether_type, + nat_target.vni.clone().into(), + mcast_tag, + &payload, + ); + + (og_pkt, geneve_pkt) +} + +async fn run_bifurcated_inner_ttl_test( + switch: &Switch, + external_ip: IpAddr, + inner_ttl: u8, + drop_label: Option<&str>, + inner_eth: bool, +) -> TestResult { + let ingress = PhysPort(10); + let egress_external = PhysPort(15); + let egress_underlay = PhysPort(19); + + let internal_multicast_ip = IpAddr::V6(MULTICAST_NAT_IP); + create_test_multicast_group( + switch, + internal_multicast_ip, + Some(TEST_TAG), + &[ + (egress_external, types::Direction::External), + (egress_underlay, types::Direction::Underlay), + ], + types::InternalForwarding { nat_target: None }, + types::ExternalForwarding { vlan_id: None }, + None, + ) + .await; + + let vlan = Some(10); + let nat_target = match external_ip { + IpAddr::V4(_) => create_nat_target_ipv4(), + IpAddr::V6(_) => create_nat_target_ipv6(), + }; + + let created_group = create_test_multicast_group( + switch, + external_ip, + Some(TEST_TAG), + &[], + types::InternalForwarding { nat_target: Some(nat_target.clone()) }, + types::ExternalForwarding { vlan_id: vlan }, + None, + ) + .await; + + let src_mac = switch.get_port_mac(ingress).unwrap(); + let (og_pkt, geneve_pkt) = build_inner_ttl_geneve_pkt( + external_ip, + src_mac, + vlan, + &nat_target, + inner_ttl, + 2, + inner_eth, + ); + + let test_pkt = + TestPacket { packet: Arc::new(geneve_pkt.clone()), port: ingress }; + + let underlay_pkt = prepare_expected_pkt( + switch, + &geneve_pkt, + None, + None, + Some(egress_underlay), + ); + + let mut expected_pkts = Vec::new(); + if drop_label.is_none() { + let external_pkt = prepare_expected_pkt( + switch, + &og_pkt, + vlan, + None, + Some(egress_external), + ); + expected_pkts.push(TestPacket { + packet: Arc::new(external_pkt), + port: egress_external, + }); + } + expected_pkts.push(TestPacket { + packet: Arc::new(underlay_pkt), + port: egress_underlay, + }); + + let port_label_external = switch.port_label(egress_external).unwrap(); + let port_label_underlay = switch.port_label(egress_underlay).unwrap(); + + let ctr_baseline_external = switch + .get_counter(&port_label_external, Some("multicast_external")) + .await + .unwrap(); + let ctr_baseline_underlay = switch + .get_counter(&port_label_underlay, Some("multicast_underlay")) + .await + .unwrap(); + let ctr_baseline_drop_port = switch + .get_counter(&port_label_external, Some("egress_drop_port")) + .await + .unwrap(); + let ctr_baseline_drop_reason = match drop_label { + Some(label) => { + switch.get_counter(label, Some("egress_drop_reason")).await.unwrap() + } + None => 0, + }; + + let matched = + switch.packet_test_matched(vec![test_pkt], expected_pkts).unwrap(); + let underlay_got = + matched.iter().find(|p| p.port == egress_underlay).unwrap(); + assert_eq!( + underlay_got.packet.body, geneve_pkt.body, + "underlay replica must carry the unmodified inner frame" + ); + + check_counter_incremented( + switch, + &port_label_underlay, + ctr_baseline_underlay, + 1, + Some("multicast_underlay"), + ) + .await + .unwrap(); + + match drop_label { + Some(label) => { + check_counter_incremented( + switch, + label, + ctr_baseline_drop_reason, + 1, + Some("egress_drop_reason"), + ) + .await + .unwrap(); + check_counter_incremented( + switch, + &port_label_external, + ctr_baseline_drop_port, + 1, + Some("egress_drop_port"), + ) + .await + .unwrap(); + + let ctr_external = switch + .get_counter(&port_label_external, Some("multicast_external")) + .await + .unwrap(); + assert_eq!( + ctr_external, ctr_baseline_external, + "dropped external replica must not hit the external counter" + ); + } + None => { + check_counter_incremented( + switch, + &port_label_external, + ctr_baseline_external, + 1, + Some("multicast_external"), + ) + .await + .unwrap(); + + let ctr_drop_port = switch + .get_counter(&port_label_external, Some("egress_drop_port")) + .await + .unwrap(); + assert_eq!( + ctr_drop_port, ctr_baseline_drop_port, + "forwarded external replica must not be counted as dropped" + ); + } + } + + cleanup_test_group(switch, get_group_ip(&created_group), TEST_TAG) + .await + .unwrap(); + cleanup_test_group(switch, MULTICAST_NAT_IP.into(), TEST_TAG).await +} + +#[tokio::test] +#[ignore] +async fn test_inner_ttl_bifurcated_v4_zero() -> TestResult { + let switch = &*get_switch().await; + run_bifurcated_inner_ttl_test( + switch, + IpAddr::V4(MULTICAST_TEST_IPV4), + 0, + Some("multicast_ttl_exceeded"), + false, + ) + .await +} + +#[tokio::test] +#[ignore] +async fn test_inner_ttl_bifurcated_v4_one() -> TestResult { + let switch = &*get_switch().await; + run_bifurcated_inner_ttl_test( + switch, + IpAddr::V4(MULTICAST_TEST_IPV4), + 1, + Some("multicast_ttl_exceeded"), + false, + ) + .await +} + +#[tokio::test] +#[ignore] +async fn test_inner_ttl_bifurcated_v4_two() -> TestResult { + let switch = &*get_switch().await; + run_bifurcated_inner_ttl_test( + switch, + IpAddr::V4(MULTICAST_TEST_IPV4), + 2, + None, + false, + ) + .await +} + +#[tokio::test] +#[ignore] +async fn test_inner_ttl_bifurcated_v6_zero() -> TestResult { + let switch = &*get_switch().await; + run_bifurcated_inner_ttl_test( + switch, + IpAddr::V6(MULTICAST_TEST_IPV6), + 0, + Some("multicast_ttl_exceeded"), + false, + ) + .await +} + +#[tokio::test] +#[ignore] +async fn test_inner_ttl_bifurcated_v6_one() -> TestResult { + let switch = &*get_switch().await; + run_bifurcated_inner_ttl_test( + switch, + IpAddr::V6(MULTICAST_TEST_IPV6), + 1, + Some("multicast_ttl_exceeded"), + false, + ) + .await +} + +#[tokio::test] +#[ignore] +async fn test_inner_ttl_bifurcated_v6_two() -> TestResult { + let switch = &*get_switch().await; + run_bifurcated_inner_ttl_test( + switch, + IpAddr::V6(MULTICAST_TEST_IPV6), + 2, + None, + false, + ) + .await +} + +async fn run_external_only_inner_ttl_test( + switch: &Switch, + external_ip: IpAddr, + inner_ttl: u8, + expect_drop: bool, + inner_eth: bool, +) -> TestResult { + let ingress = PhysPort(10); + let egress1 = PhysPort(15); + let egress2 = PhysPort(17); + + let internal_multicast_ip = IpAddr::V6(MULTICAST_NAT_IP); + create_test_multicast_group( + switch, + internal_multicast_ip, + Some(TEST_TAG), + &[ + (egress1, types::Direction::External), + (egress2, types::Direction::External), + ], + types::InternalForwarding { nat_target: None }, + types::ExternalForwarding { vlan_id: None }, + None, + ) + .await; + + let vlan = Some(10); + let nat_target = match external_ip { + IpAddr::V4(_) => create_nat_target_ipv4(), + IpAddr::V6(_) => create_nat_target_ipv6(), + }; + + let created_group = create_test_multicast_group( + switch, + external_ip, + Some(TEST_TAG), + &[], + types::InternalForwarding { nat_target: Some(nat_target.clone()) }, + types::ExternalForwarding { vlan_id: vlan }, + None, + ) + .await; + + let src_mac = switch.get_port_mac(ingress).unwrap(); + let (og_pkt, geneve_pkt) = build_inner_ttl_geneve_pkt( + external_ip, + src_mac, + vlan, + &nat_target, + inner_ttl, + 0, + inner_eth, + ); + + let test_pkt = TestPacket { packet: Arc::new(geneve_pkt), port: ingress }; + + let expected_pkts = if expect_drop { + Vec::new() + } else { + vec![ + TestPacket { + packet: Arc::new(prepare_expected_pkt( + switch, + &og_pkt, + vlan, + None, + Some(egress1), + )), + port: egress1, + }, + TestPacket { + packet: Arc::new(prepare_expected_pkt( + switch, + &og_pkt, + vlan, + None, + Some(egress2), + )), + port: egress2, + }, + ] + }; + + 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_drop_reason = + switch.get_counter("multicast_ttl_exceeded", None).await.unwrap(); + let ctr_baseline_drop_port = switch + .get_counter(&port_label_ingress, Some("ingress_drop_port")) + .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(); + + switch.packet_test(vec![test_pkt], expected_pkts).unwrap(); + + if expect_drop { + check_counter_incremented( + switch, + "multicast_ttl_exceeded", + ctr_baseline_drop_reason, + 1, + None, + ) + .await + .unwrap(); + check_counter_incremented( + switch, + &port_label_ingress, + ctr_baseline_drop_port, + 1, + Some("ingress_drop_port"), + ) + .await + .unwrap(); + + let ctr_external_egress1 = switch + .get_counter(&port_label_egress1, Some("multicast_external")) + .await + .unwrap(); + let ctr_external_egress2 = switch + .get_counter(&port_label_egress2, Some("multicast_external")) + .await + .unwrap(); + assert_eq!( + ctr_external_egress1, ctr_baseline_external_egress1, + "dropped packet must not be replicated to external members" + ); + assert_eq!( + ctr_external_egress2, ctr_baseline_external_egress2, + "dropped packet must not be replicated to external members" + ); + } else { + 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_drop_reason = + switch.get_counter("multicast_ttl_exceeded", None).await.unwrap(); + assert_eq!( + ctr_drop_reason, ctr_baseline_drop_reason, + "forwarded packet must not be counted as a TTL drop" + ); + } + + cleanup_test_group(switch, get_group_ip(&created_group), TEST_TAG) + .await + .unwrap(); + cleanup_test_group(switch, MULTICAST_NAT_IP.into(), TEST_TAG).await +} + +#[tokio::test] +#[ignore] +async fn test_inner_ttl_external_v4_zero() -> TestResult { + let switch = &*get_switch().await; + run_external_only_inner_ttl_test( + switch, + IpAddr::V4(MULTICAST_TEST_IPV4), + 0, + true, + false, + ) + .await +} + +#[tokio::test] +#[ignore] +async fn test_inner_ttl_external_v4_one() -> TestResult { + let switch = &*get_switch().await; + run_external_only_inner_ttl_test( + switch, + IpAddr::V4(MULTICAST_TEST_IPV4), + 1, + true, + false, + ) + .await +} + +#[tokio::test] +#[ignore] +async fn test_inner_ttl_external_v4_two() -> TestResult { + let switch = &*get_switch().await; + run_external_only_inner_ttl_test( + switch, + IpAddr::V4(MULTICAST_TEST_IPV4), + 2, + false, + false, + ) + .await +} + +#[tokio::test] +#[ignore] +async fn test_inner_ttl_external_v6_zero() -> TestResult { + let switch = &*get_switch().await; + run_external_only_inner_ttl_test( + switch, + IpAddr::V6(MULTICAST_TEST_IPV6), + 0, + true, + false, + ) + .await +} + +#[tokio::test] +#[ignore] +async fn test_inner_ttl_external_v6_one() -> TestResult { + let switch = &*get_switch().await; + run_external_only_inner_ttl_test( + switch, + IpAddr::V6(MULTICAST_TEST_IPV6), + 1, + true, + false, + ) + .await +} + +#[tokio::test] +#[ignore] +async fn test_inner_ttl_external_v6_two() -> TestResult { + let switch = &*get_switch().await; + run_external_only_inner_ttl_test( + switch, + IpAddr::V6(MULTICAST_TEST_IPV6), + 2, + false, + false, + ) + .await +} + +#[tokio::test] +#[ignore] +async fn test_inner_ttl_bifurcated_v4_eth_one() -> TestResult { + let switch = &*get_switch().await; + run_bifurcated_inner_ttl_test( + switch, + IpAddr::V4(MULTICAST_TEST_IPV4), + 1, + Some("multicast_ttl_exceeded"), + true, + ) + .await +} + +#[tokio::test] +#[ignore] +async fn test_inner_ttl_bifurcated_v6_eth_one() -> TestResult { + let switch = &*get_switch().await; + run_bifurcated_inner_ttl_test( + switch, + IpAddr::V6(MULTICAST_TEST_IPV6), + 1, + Some("multicast_ttl_exceeded"), + true, + ) + .await +} + +#[tokio::test] +#[ignore] +async fn test_inner_ttl_external_v4_eth_one() -> TestResult { + let switch = &*get_switch().await; + run_external_only_inner_ttl_test( + switch, + IpAddr::V4(MULTICAST_TEST_IPV4), + 1, + true, + true, + ) + .await +} + +#[tokio::test] +#[ignore] +async fn test_inner_ttl_external_v6_eth_one() -> TestResult { + let switch = &*get_switch().await; + run_external_only_inner_ttl_test( + switch, + IpAddr::V6(MULTICAST_TEST_IPV6), + 1, + true, + true, + ) + .await +} + +#[tokio::test] +#[ignore] +async fn test_inner_ttl_bifurcated_v4_eth_two() -> TestResult { + let switch = &*get_switch().await; + run_bifurcated_inner_ttl_test( + switch, + IpAddr::V4(MULTICAST_TEST_IPV4), + 2, + None, + true, + ) + .await +} + +#[tokio::test] +#[ignore] +async fn test_inner_ttl_bifurcated_v6_eth_two() -> TestResult { + let switch = &*get_switch().await; + run_bifurcated_inner_ttl_test( + switch, + IpAddr::V6(MULTICAST_TEST_IPV6), + 2, + None, + true, + ) + .await +} + +#[tokio::test] +#[ignore] +async fn test_inner_ttl_external_v4_eth_two() -> TestResult { + let switch = &*get_switch().await; + run_external_only_inner_ttl_test( + switch, + IpAddr::V4(MULTICAST_TEST_IPV4), + 2, + false, + true, + ) + .await +} + +#[tokio::test] +#[ignore] +async fn test_inner_ttl_external_v6_eth_two() -> TestResult { + let switch = &*get_switch().await; + run_external_only_inner_ttl_test( + switch, + IpAddr::V6(MULTICAST_TEST_IPV6), + 2, + false, + true, + ) + .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 @@ -3004,13 +3724,13 @@ async fn test_ipv6_multicast_hop_limit_one() -> TestResult { let expected_pkts = vec![]; let ctr_baseline = - switch.get_counter("ipv6_ttl_invalid", None).await.unwrap(); + switch.get_counter("multicast_ttl_exceeded", None).await.unwrap(); switch.packet_test(vec![test_pkt], expected_pkts).unwrap(); check_counter_incremented( switch, - "ipv6_ttl_invalid", + "multicast_ttl_exceeded", ctr_baseline, 1, None, diff --git a/dpd-client/tests/integration_tests/nat.rs b/dpd-client/tests/integration_tests/nat.rs index f8fba25b..dda07822 100644 --- a/dpd-client/tests/integration_tests/nat.rs +++ b/dpd-client/tests/integration_tests/nat.rs @@ -189,7 +189,12 @@ struct NatTest { geneve_vni: u32, } -async fn test_nat_egress(switch: &Switch, test: &NatTest) -> TestResult { +async fn test_nat_egress( + switch: &Switch, + test: &NatTest, + inner_ttl: Option, + outer_hop_limit: Option, +) -> TestResult { let router_mac = test.router_mac.parse()?; // set up the switch internals so we can route the packet from the gimlet @@ -241,10 +246,10 @@ async fn test_nat_egress(switch: &Switch, test: &NatTest) -> TestResult { // responsibility, then this needs to be done on the 'expected' packet // rather than the 'to_send' packet. let inner = { - if let Some(mut ipv4) = payload_pkt.hdrs.ipv4_hdr { + if let Some(ipv4) = payload_pkt.hdrs.ipv4_hdr.as_mut() { ipv4.ipv4_src_ip = test.uplink_port_external.parse()?; Ok(eth::ETHER_IPV4) - } else if let Some(mut ipv6) = payload_pkt.hdrs.ipv6_hdr { + } else if let Some(ipv6) = payload_pkt.hdrs.ipv6_hdr.as_mut() { ipv6.ipv6_src_ip = test.uplink_port_external.parse()?; Ok(eth::ETHER_IPV6) } else { @@ -270,13 +275,21 @@ async fn test_nat_egress(switch: &Switch, test: &NatTest) -> TestResult { } }; + if let Some(ttl) = inner_ttl { + if let Some(ipv4) = payload_pkt.hdrs.ipv4_hdr.as_mut() { + ipv4.ipv4_ttl = ttl; + } else if let Some(ipv6) = payload_pkt.hdrs.ipv6_hdr.as_mut() { + ipv6.ipv6_hop_lim = ttl; + } + } + if inner == eth::ETHER_IPV4 { ipv4::Ipv4Hdr::update_checksum(&mut payload_pkt); } let switch_mac = switch.get_port_mac(test.gimlet_port).unwrap().to_string(); let payload = payload_pkt.deparse().unwrap().to_vec(); - let to_send = common::gen_geneve_packet( + let mut to_send = common::gen_geneve_packet( Endpoint::parse(&test.gimlet_mac, &test.gimlet_ip, 3333).unwrap(), Endpoint::parse( &switch_mac, @@ -290,25 +303,61 @@ async fn test_nat_egress(switch: &Switch, test: &NatTest) -> TestResult { &payload[14..], ); - let expected = match test.uplink_port_registered { - true => { - let mut to_recv = common::gen_packet_routed( - switch, - test.uplink_port, - &payload_pkt, - ); - eth::EthHdr::rewrite_dmac(&mut to_recv, router_mac); - vec![TestPacket { - packet: Arc::new(to_recv), - port: test.uplink_port, - }] - } - false => Vec::new(), + if let Some(hop_limit) = outer_hop_limit { + to_send.hdrs.ipv6_hdr.as_mut().unwrap().ipv6_hop_lim = hop_limit; + } + + let ttl_expired = matches!(inner_ttl, Some(0 | 1)); + let expected = if ttl_expired { + let mut punt = payload_pkt.clone(); + let eth_hdr = punt.hdrs.eth_hdr.as_mut().unwrap(); + eth_hdr.eth_smac = test.gimlet_mac.parse()?; + eth_hdr.eth_dmac = switch_mac.parse()?; + let icmp_type = match inner { + eth::ETHER_IPV4 => icmp::ICMP_TIME_EXCEEDED, + _ => icmp::ICMP6_TIME_EXCEEDED, + }; + common::set_icmp_needed( + switch, + &mut punt, + test.gimlet_port, + NO_PORT, + icmp_type, + 0, + ); + vec![TestPacket { packet: Arc::new(punt), port: SERVICE_PORT }] + } else if test.uplink_port_registered { + let mut to_recv = + common::gen_packet_routed(switch, test.uplink_port, &payload_pkt); + eth::EthHdr::rewrite_dmac(&mut to_recv, router_mac); + vec![TestPacket { packet: Arc::new(to_recv), port: test.uplink_port }] + } else { + // Egress to a non-uplink port is blocked by NatEgressFilter, and the + // counter check below confirms the drop reason. + Vec::new() }; let send = TestPacket { packet: Arc::new(to_send), port: test.gimlet_port }; - switch.packet_test(vec![send], expected) + if ttl_expired || test.uplink_port_registered { + return switch.packet_test(vec![send], expected); + } + + let blocked = "nat_egress_blocked"; + let ctr_baseline = + switch.get_counter(blocked, Some("egress_drop_reason")).await?; + + switch.packet_test(vec![send], expected)?; + + check_counter_incremented( + switch, + blocked, + ctr_baseline, + 1, + Some("egress_drop_reason"), + ) + .await?; + Ok(()) } async fn test_nat_ingress(switch: &Switch, test: &NatTest) -> TestResult { @@ -418,12 +467,11 @@ async fn test_nat_ingress(switch: &Switch, test: &NatTest) -> TestResult { } // packet to/from IPv4 addresses, with an IPv6 address for the OPTE host -async fn test_egress_ipv4( - switch: &Switch, +fn egress_ipv4_test( l4_protocol: L4Protocol, uplink_port_registered: bool, -) -> TestResult { - let test = NatTest { +) -> NatTest { + NatTest { uplink_port: PhysPort(14), uplink_port_external: "192.168.1.2".to_string(), uplink_port_registered, @@ -446,9 +494,25 @@ async fn test_egress_ipv4( nat_l4_port: 10, l4_protocol, geneve_vni: 1, // not used on egress tests - }; + } +} - test_nat_egress(switch, &test).await +async fn test_egress_ipv4( + switch: &Switch, + l4_protocol: L4Protocol, + uplink_port_registered: bool, + inner_ttl: Option, +) -> TestResult { + let test = egress_ipv4_test(l4_protocol, uplink_port_registered); + test_nat_egress(switch, &test, inner_ttl, None).await +} + +#[tokio::test] +#[ignore] +async fn test_egress_ipv4_outer_hop_limit_zero() -> TestResult { + let switch = &*get_switch().await; + let test = egress_ipv4_test(L4Protocol::Udp, true); + test_nat_egress(switch, &test, None, Some(0)).await } // UDP packet to/from IPv4 addresses @@ -456,7 +520,7 @@ async fn test_egress_ipv4( #[ignore] async fn test_egress_ipv4_udp() -> TestResult { let switch = &*get_switch().await; - test_egress_ipv4(switch, L4Protocol::Udp, true).await + test_egress_ipv4(switch, L4Protocol::Udp, true, None).await } // TCP packet to/from IPv4 addresses @@ -464,7 +528,7 @@ async fn test_egress_ipv4_udp() -> TestResult { #[ignore] async fn test_egress_ipv4_tcp() -> TestResult { let switch = &*get_switch().await; - test_egress_ipv4(switch, L4Protocol::Tcp, true).await + test_egress_ipv4(switch, L4Protocol::Tcp, true, None).await } // ICMP packet to/from IPv4 addresses @@ -472,7 +536,7 @@ async fn test_egress_ipv4_tcp() -> TestResult { #[ignore] async fn test_egress_ipv4_icmp() -> TestResult { let switch = &*get_switch().await; - test_egress_ipv4(switch, L4Protocol::Icmp, true).await + test_egress_ipv4(switch, L4Protocol::Icmp, true, None).await } // UDP packet to an IPv4 address, egressing a backplane port. @@ -480,7 +544,7 @@ async fn test_egress_ipv4_icmp() -> TestResult { #[ignore] async fn test_backplane_egress_ipv4_udp() -> TestResult { let switch = &*get_switch().await; - test_egress_ipv4(switch, L4Protocol::Udp, false).await + test_egress_ipv4(switch, L4Protocol::Udp, false, None).await } async fn test_ingress_ipv4( @@ -556,6 +620,7 @@ async fn test_egress_ipv6( switch: &Switch, l4_protocol: L4Protocol, uplink_port_registered: bool, + inner_ttl: Option, ) -> TestResult { let test = NatTest { uplink_port: PhysPort(14), @@ -582,28 +647,70 @@ async fn test_egress_ipv6( geneve_vni: 1, // not used on egress tests }; - test_nat_egress(switch, &test).await + test_nat_egress(switch, &test, inner_ttl, None).await } #[tokio::test] #[ignore] async fn test_egress_ipv6_udp() -> TestResult { let switch = &*get_switch().await; - test_egress_ipv6(switch, L4Protocol::Udp, true).await + test_egress_ipv6(switch, L4Protocol::Udp, true, None).await } #[tokio::test] #[ignore] async fn test_egress_ipv6_tcp() -> TestResult { let switch = &*get_switch().await; - test_egress_ipv6(switch, L4Protocol::Tcp, true).await + test_egress_ipv6(switch, L4Protocol::Tcp, true, None).await } #[tokio::test] #[ignore] async fn test_backplane_egress_ipv6_tcp() -> TestResult { let switch = &*get_switch().await; - test_egress_ipv6(switch, L4Protocol::Tcp, false).await + test_egress_ipv6(switch, L4Protocol::Tcp, false, None).await +} + +#[tokio::test] +#[ignore] +async fn test_inner_ttl_egress_v4_zero() -> TestResult { + let switch = &*get_switch().await; + test_egress_ipv4(switch, L4Protocol::Udp, true, Some(0)).await +} + +#[tokio::test] +#[ignore] +async fn test_inner_ttl_egress_v4_one() -> TestResult { + let switch = &*get_switch().await; + test_egress_ipv4(switch, L4Protocol::Udp, true, Some(1)).await +} + +#[tokio::test] +#[ignore] +async fn test_inner_ttl_egress_v4_two() -> TestResult { + let switch = &*get_switch().await; + test_egress_ipv4(switch, L4Protocol::Udp, true, Some(2)).await +} + +#[tokio::test] +#[ignore] +async fn test_inner_ttl_egress_v6_zero() -> TestResult { + let switch = &*get_switch().await; + test_egress_ipv6(switch, L4Protocol::Udp, true, Some(0)).await +} + +#[tokio::test] +#[ignore] +async fn test_inner_ttl_egress_v6_one() -> TestResult { + let switch = &*get_switch().await; + test_egress_ipv6(switch, L4Protocol::Udp, true, Some(1)).await +} + +#[tokio::test] +#[ignore] +async fn test_inner_ttl_egress_v6_two() -> TestResult { + let switch = &*get_switch().await; + test_egress_ipv6(switch, L4Protocol::Udp, true, Some(2)).await } async fn test_ingress_ipv6( diff --git a/dpd-client/tests/integration_tests/route_ipv6.rs b/dpd-client/tests/integration_tests/route_ipv6.rs index 81932cac..f0d3c201 100644 --- a/dpd-client/tests/integration_tests/route_ipv6.rs +++ b/dpd-client/tests/integration_tests/route_ipv6.rs @@ -543,6 +543,45 @@ async fn test_ipv6_link_local_multicast_hop_limit_one() -> TestResult { result } +#[tokio::test] +#[ignore] +async fn test_ipv6_link_local_mcast_hop_limit_zero() -> TestResult { + let switch = &*get_switch().await; + + let ingress = PhysPort(10); + + let src = Endpoint::parse("e0:d5:5e:67:89:ab", "fe80::1", 3333).unwrap(); + let dst = Endpoint::parse("33:33:00:00:00:01", "ff02::1", 4444).unwrap(); + + let mut send = common::gen_udp_packet(src, dst); + ipv6::Ipv6Hdr::adjust_hlim(&mut send, -255); + + let test_pkt = TestPacket { packet: Arc::new(send.clone()), port: ingress }; + + let mut recv = send.clone(); + common::add_sidecar_hdr( + switch, + &mut recv, + sidecar::SC_FWD_TO_USERSPACE, + ingress, + NO_PORT, + None, + ); + + let expected = + vec![TestPacket { packet: Arc::new(recv), port: SERVICE_PORT }]; + + let ctr_baseline = + switch.get_counter("ipv6_ttl_invalid", None).await.unwrap(); + + let result = switch.packet_test(vec![test_pkt], expected); + + let ctr_final = switch.get_counter("ipv6_ttl_invalid", None).await.unwrap(); + assert_eq!(ctr_final, ctr_baseline, "link-local mcast at hop limit 0"); + + result +} + #[tokio::test] #[ignore] async fn test_reset() -> TestResult { diff --git a/dpd-client/tests/integration_tests/service.rs b/dpd-client/tests/integration_tests/service.rs index 2aeff4a5..4ebe39bd 100644 --- a/dpd-client/tests/integration_tests/service.rs +++ b/dpd-client/tests/integration_tests/service.rs @@ -9,7 +9,8 @@ use std::sync::Arc; use ::common::network::MacAddr; use dpd_client::ClientInfo; use dpd_client::types::Ipv4Entry; -use packet::{Endpoint, eth, icmp, ipv4, sidecar}; +use dpd_client::types::Ipv6Entry; +use packet::{Endpoint, eth, icmp, ipv4, ipv6, sidecar}; use crate::integration_tests::common; use crate::integration_tests::common::prelude::*; @@ -57,6 +58,102 @@ async fn test_service_ipv4_unicast() -> TestResult { switch.packet_test(vec![send], vec![expected]) } +#[tokio::test] +#[ignore] +async fn test_service_ipv4_unicast_ttl_zero() -> TestResult { + let switch = &*get_switch().await; + + let ingress = PhysPort(12); + let router_ip = "10.10.10.1"; + let router_mac = "02:78:39:45:b9:00"; + + let (port_id, link_id) = switch.link_id(ingress).unwrap(); + let entry = Ipv4Entry { + addr: router_ip.parse().unwrap(), + tag: switch.client.inner().tag.clone(), + }; + switch.client.link_ipv4_create(&port_id, &link_id, &entry).await.unwrap(); + + let mut send_pkt = common::gen_udp_packet( + Endpoint::parse("e0:d5:5e:67:89:ab", "10.10.10.10", 3333).unwrap(), + Endpoint::parse(router_mac, router_ip, 4444).unwrap(), + ); + send_pkt.hdrs.ipv4_hdr.as_mut().unwrap().ipv4_ttl = 0; + ipv4::Ipv4Hdr::update_checksum(&mut send_pkt); + let mut recv_pkt = send_pkt.clone(); + + let send = TestPacket { packet: Arc::new(send_pkt), port: ingress }; + + common::add_sidecar_hdr( + switch, + &mut recv_pkt, + sidecar::SC_FWD_TO_USERSPACE, + ingress, + NO_PORT, + None, + ); + let expected = + TestPacket { packet: Arc::new(recv_pkt), port: SERVICE_PORT }; + + let ctr_baseline = + switch.get_counter("ipv4_ttl_invalid", None).await.unwrap(); + + let result = switch.packet_test(vec![send], vec![expected]); + + let ctr_final = switch.get_counter("ipv4_ttl_invalid", None).await.unwrap(); + assert_eq!(ctr_final, ctr_baseline, "ttl 0 to a switch address"); + + result +} + +#[tokio::test] +#[ignore] +async fn test_service_ipv6_unicast_hop_limit_zero() -> TestResult { + let switch = &*get_switch().await; + + let ingress = PhysPort(12); + let router_ip = "fd00:1122:3344:0100::1"; + let router_mac = "02:78:39:45:b9:00"; + + let (port_id, link_id) = switch.link_id(ingress).unwrap(); + let entry = Ipv6Entry { + addr: router_ip.parse().unwrap(), + tag: switch.client.inner().tag.clone(), + }; + switch.client.link_ipv6_create(&port_id, &link_id, &entry).await.unwrap(); + + let mut send_pkt = common::gen_udp_packet( + Endpoint::parse("e0:d5:5e:67:89:ab", "fd00:1122:3344:0200::1", 3333) + .unwrap(), + Endpoint::parse(router_mac, router_ip, 4444).unwrap(), + ); + ipv6::Ipv6Hdr::adjust_hlim(&mut send_pkt, -255); + let mut recv_pkt = send_pkt.clone(); + + let send = TestPacket { packet: Arc::new(send_pkt), port: ingress }; + + common::add_sidecar_hdr( + switch, + &mut recv_pkt, + sidecar::SC_FWD_TO_USERSPACE, + ingress, + NO_PORT, + None, + ); + let expected = + TestPacket { packet: Arc::new(recv_pkt), port: SERVICE_PORT }; + + let ctr_baseline = + switch.get_counter("ipv6_ttl_invalid", None).await.unwrap(); + + let result = switch.packet_test(vec![send], vec![expected]); + + let ctr_final = switch.get_counter("ipv6_ttl_invalid", None).await.unwrap(); + assert_eq!(ctr_final, ctr_baseline, "hop limit 0 to a switch address"); + + result +} + // Packets sent to an IP address assigned to a switch port should be forwarded // to the service port with an added sidecar header. #[tokio::test] @@ -260,7 +357,7 @@ async fn test_ttl_exceeded() -> TestResult { switch, &mut recv_pkt, ingress, - SERVICE_PORT, + NO_PORT, icmp::ICMP_TIME_EXCEEDED, 0, ); diff --git a/dpd/p4/constants.p4 b/dpd/p4/constants.p4 index 225301b6..d565722a 100644 --- a/dpd/p4/constants.p4 +++ b/dpd/p4/constants.p4 @@ -96,5 +96,9 @@ const bit<8> DROP_GENEVE_OPTIONS_TOO_LONG = 0x18; const bit<8> DROP_GENEVE_OPTION_MALFORMED = 0x19; const bit<8> DROP_GENEVE_OPTION_UNKNOWN = 0x1A; const bit<8> DROP_SCTP = 0x1B; +// No Time Exceeded for multicast +// (see RFC 1812 §4.3.2.7, §5.3.1; +// RFC 4443 §2.4(e.3)) +const bit<8> DROP_MULTICAST_TTL_EXCEEDED = 0x1C; // MAX(DROP_xxx) + 1 -const bit<32> DROP_REASON_MAX = 0x1C; +const bit<32> DROP_REASON_MAX = 0x1D; diff --git a/dpd/p4/parser.p4 b/dpd/p4/parser.p4 index 4b14d479..763e3476 100644 --- a/dpd/p4/parser.p4 +++ b/dpd/p4/parser.p4 @@ -152,11 +152,24 @@ parser IngressParser( state validate_ttl { transition select(hdr.ipv4.ttl) { - 8w0: invalidate_ttl; + 8w0: defer_ttl_drop; default: goto_proto_ipv4; } } + // Records the TTL drop reason without setting `meta.dropped`. A packet + // addressed to the switch must still be received (RFC 1812 §4.2.2.9), and + // whether the destination is a switch address depends on Filter's switch + // address tables, which run after the parser. + // + // NatIngress and Router4/Router6 drop on the forwarding paths, although + // the NAT egress (Geneve decap) path is exempt from a Router4/Router6 drop + // and still punts for an inner TTL of 0. + state defer_ttl_drop { + meta.drop_reason = DROP_IPV4_TTL_INVALID; + transition goto_proto_ipv4; + } + state set_mcast_ipv4 { meta.is_mcast = true; transition validate_mcast_ttl; @@ -165,7 +178,7 @@ parser IngressParser( state validate_mcast_ttl { transition select(hdr.ipv4.ttl) { 8w0: invalidate_ttl; - 8w1: invalidate_ttl; + 8w1: mcast_ttl_exceeded; default: goto_proto_ipv4; } } @@ -178,6 +191,12 @@ parser IngressParser( transition accept; } + state mcast_ttl_exceeded { + meta.drop_reason = DROP_MULTICAST_TTL_EXCEEDED; + meta.dropped = true; + transition accept; + } + state goto_proto_ipv4 { transition select(hdr.ipv4.protocol) { IPPROTO_ICMP: parse_icmp; @@ -219,7 +238,7 @@ parser IngressParser( state set_link_local_mcast { meta.is_link_local_mcastv6 = true; meta.is_mcast = true; - transition validate_mcast_link_local_hop_limit; + transition validate_hop_limit; } state check_ipv6_mcast { @@ -233,26 +252,21 @@ parser IngressParser( state validate_hop_limit { transition select(hdr.ipv6.hop_limit) { - 8w0: invalidate_hop_limit; + 8w0: defer_hop_limit_drop; default: goto_proto_ipv6; } } - state validate_mcast_link_local_hop_limit { - // For link-local multicast, we allow a hop limit of 1. - // This is to ensure that link-local multicast packets - // are not forwarded beyond the local link. - transition select(hdr.ipv6.hop_limit) { - 8w0: invalidate_hop_limit; - 8w1: goto_proto_ipv6; - default: goto_proto_ipv6; - } + // Similar to `defer_ttl_drop`, but for IPv6 (RFC 8200 §3). + state defer_hop_limit_drop { + meta.drop_reason = DROP_IPV6_TTL_INVALID; + transition goto_proto_ipv6; } state validate_mcast_hop_limit { transition select(hdr.ipv6.hop_limit) { 8w0: invalidate_hop_limit; - 8w1: invalidate_hop_limit; + 8w1: mcast_ttl_exceeded; default: goto_proto_ipv6; } } diff --git a/dpd/p4/sidecar.p4 b/dpd/p4/sidecar.p4 index d73dd3f6..73a4ea46 100644 --- a/dpd/p4/sidecar.p4 +++ b/dpd/p4/sidecar.p4 @@ -373,7 +373,10 @@ control Services( action mcast_inbound_link_local() { hdr.sidecar.sc_code = SC_FWD_TO_USERSPACE; hdr.sidecar.sc_ingress = (bit<16>)ig_intr_md.ingress_port; - hdr.sidecar.sc_egress = (bit<16>)ig_tm_md.ucast_egress_port; + // Inbound packets do not have an egress port yet, and reading + // the unset ucast_egress_port yields an unspecified value: + // https://p4.org/wp-content/uploads/sites/53/p4-spec/docs/P4-16-v1.2.4.html#sec-uninitialized-values-and-writing-invalid-headers + hdr.sidecar.sc_egress = 0; hdr.sidecar.sc_ether_type = hdr.ethernet.ether_type; hdr.sidecar.sc_payload = 0; hdr.sidecar.setValid(); @@ -723,10 +726,20 @@ control NatIngress ( if (ingress_hit.apply().hit) { if (hdr.ipv4.isValid()) { - CalculateIPv4Len.apply(hdr, meta); - encap_ipv4(); + if (hdr.ipv4.ttl == 0) { + meta.drop_reason = DROP_IPV4_TTL_INVALID; + meta.dropped = true; + } else { + CalculateIPv4Len.apply(hdr, meta); + encap_ipv4(); + } } else if (hdr.ipv6.isValid()) { - encap_ipv6(); + if (hdr.ipv6.hop_limit == 0) { + meta.drop_reason = DROP_IPV6_TTL_INVALID; + meta.dropped = true; + } else { + encap_ipv6(); + } } if (hdr.vlan.isValid()) { @@ -1234,7 +1247,10 @@ control Router4 ( action icmp_error(bit<8> type, bit<8> code) { hdr.sidecar.sc_code = SC_ICMP_NEEDED; hdr.sidecar.sc_ingress = (bit<16>)ig_intr_md.ingress_port; - hdr.sidecar.sc_egress = (bit<16>)ig_tm_md.ucast_egress_port; + // No path into this action has yet assigned ucast_egress_port, + // and reading it before assignment yields an unspecified value: + // https://p4.org/wp-content/uploads/sites/53/p4-spec/docs/P4-16-v1.2.4.html#sec-uninitialized-values-and-writing-invalid-headers + hdr.sidecar.sc_egress = 0; hdr.sidecar.sc_ether_type = hdr.ethernet.ether_type; hdr.sidecar.sc_payload = (bit<128>)type << 8 | (bit<128>)code; hdr.sidecar.setValid(); @@ -1244,6 +1260,12 @@ control Router4 ( } apply { + if (hdr.ipv4.ttl == 0 && !meta.nat_egress_hit) { + meta.dropped = true; + meta.drop_reason = DROP_IPV4_TTL_INVALID; + return; + } + route4_result_t fwd; fwd.is_v6 = false; fwd.nexthop6 = 0; @@ -1266,7 +1288,7 @@ control Router4 ( // Dont set meta.dropped because we want an error packet // to go out. meta.drop_reason = DROP_IPV4_UNROUTEABLE; - } else if (hdr.ipv4.ttl == 1 && !IS_SERVICE(fwd.port)) { + } else if (hdr.ipv4.ttl <= 8w1 && !IS_SERVICE(fwd.port)) { icmp_error(ICMP_TIME_EXCEEDED, ICMP_EXC_TTL); // Dont set meta.dropped because we want an error packet // to go out. @@ -1285,23 +1307,10 @@ control Router4 ( control MulticastRouter4( inout sidecar_headers_t hdr, inout sidecar_ingress_meta_t meta, - in ingress_intrinsic_metadata_t ig_intr_md, inout ingress_intrinsic_metadata_for_tm_t ig_tm_md ) { DirectCounter>(CounterType_t.PACKETS_AND_BYTES) ctr; - action icmp_error(bit<8> type, bit<8> code) { - hdr.sidecar.sc_code = SC_ICMP_NEEDED; - hdr.sidecar.sc_ingress = (bit<16>)ig_intr_md.ingress_port; - hdr.sidecar.sc_egress = (bit<16>)ig_tm_md.ucast_egress_port; - hdr.sidecar.sc_ether_type = hdr.ethernet.ether_type; - hdr.sidecar.sc_payload = (bit<128>)type << 8 | (bit<128>)code; - hdr.sidecar.setValid(); - hdr.ethernet.ether_type = ETHERTYPE_SIDECAR; - meta.service_routed = true; - ig_tm_md.ucast_egress_port = USER_SPACE_SERVICE_PORT; - } - action unreachable() { ctr.count(); } @@ -1344,16 +1353,15 @@ control MulticastRouter4( hdr.vlan.setInvalid(); } + // ICMP errors must not be sent for multicast destinations: + // https://www.rfc-editor.org/rfc/rfc1812.html#section-4.3.2.7 if (!tbl.apply().hit) { - icmp_error(ICMP_DEST_UNREACH, ICMP_DST_UNREACH_NET); meta.drop_reason = DROP_IPV4_UNROUTEABLE; - // Dont set meta.dropped because we want an error packet - // to go out. - } else if (hdr.ipv4.ttl == 1 && !meta.service_routed) { - icmp_error(ICMP_TIME_EXCEEDED, ICMP_EXC_TTL); - meta.drop_reason = DROP_IPV4_TTL_INVALID; - // Dont set meta.dropped because we want an error packet - // to go out. + meta.dropped = true; + } else if (hdr.ipv4.ttl <= 8w1) { + // https://www.rfc-editor.org/rfc/rfc1812.html#section-5.3.1 + meta.drop_reason = DROP_MULTICAST_TTL_EXCEEDED; + meta.dropped = true; } else { // Set the destination port to an invalid value ig_tm_md.ucast_egress_port = (PortId_t)0x1ff; @@ -1375,7 +1383,10 @@ control Router6 ( action icmp_error(bit<8> type, bit<8> code) { hdr.sidecar.sc_code = SC_ICMP_NEEDED; hdr.sidecar.sc_ingress = (bit<16>)ig_intr_md.ingress_port; - hdr.sidecar.sc_egress = (bit<16>)ig_tm_md.ucast_egress_port; + // No path into this action has yet assigned ucast_egress_port, + // and reading it before assignment yields an unspecified value: + // https://p4.org/wp-content/uploads/sites/53/p4-spec/docs/P4-16-v1.2.4.html#sec-uninitialized-values-and-writing-invalid-headers + hdr.sidecar.sc_egress = 0; hdr.sidecar.sc_ether_type = hdr.ethernet.ether_type; hdr.sidecar.sc_payload = (bit<128>)type << 8 | (bit<128>)code; hdr.sidecar.setValid(); @@ -1385,6 +1396,12 @@ control Router6 ( } apply { + if (hdr.ipv6.hop_limit == 0 && !meta.nat_egress_hit) { + meta.dropped = true; + meta.drop_reason = DROP_IPV6_TTL_INVALID; + return; + } + route6_result_t fwd; fwd.nexthop = 0; fwd.port = 0; @@ -1405,7 +1422,7 @@ control Router6 ( meta.drop_reason = DROP_IPV6_UNROUTEABLE; // Dont set meta.dropped because we want an error packet // to go out. - } else if (hdr.ipv6.hop_limit == 1 && !IS_SERVICE(fwd.port)) { + } else if (hdr.ipv6.hop_limit <= 8w1 && !IS_SERVICE(fwd.port)) { icmp_error(ICMP6_TIME_EXCEEDED, ICMP_EXC_TTL); meta.drop_reason = DROP_IPV6_TTL_EXCEEDED; // Dont set meta.dropped because we want an error packet @@ -1422,23 +1439,10 @@ control Router6 ( control MulticastRouter6 ( inout sidecar_headers_t hdr, inout sidecar_ingress_meta_t meta, - in ingress_intrinsic_metadata_t ig_intr_md, inout ingress_intrinsic_metadata_for_tm_t ig_tm_md ) { DirectCounter>(CounterType_t.PACKETS_AND_BYTES) ctr; - action icmp_error(bit<8> type, bit<8> code) { - hdr.sidecar.sc_code = SC_ICMP_NEEDED; - hdr.sidecar.sc_ingress = (bit<16>)ig_intr_md.ingress_port; - hdr.sidecar.sc_egress = (bit<16>)ig_tm_md.ucast_egress_port; - hdr.sidecar.sc_ether_type = hdr.ethernet.ether_type; - hdr.sidecar.sc_payload = (bit<128>)type << 8 | (bit<128>)code; - hdr.sidecar.setValid(); - hdr.ethernet.ether_type = ETHERTYPE_SIDECAR; - meta.service_routed = true; - ig_tm_md.ucast_egress_port = USER_SPACE_SERVICE_PORT; - } - action unreachable() { ctr.count(); } @@ -1480,16 +1484,17 @@ control MulticastRouter6 ( hdr.vlan.setInvalid(); } + // ICMPv6 errors must not be originated for multicast + // destinations; neither exception in RFC 4443 §2.4(e.3) + // (Packet Too Big, Parameter Problem code 2) applies here: + // https://www.rfc-editor.org/rfc/rfc4443.html#section-2.4 if (!tbl.apply().hit) { - icmp_error(ICMP6_DST_UNREACH, ICMP6_DST_UNREACH_NOROUTE); meta.drop_reason = DROP_IPV6_UNROUTEABLE; - // Dont set meta.dropped because we want an error packet - // to go out. - } else if (hdr.ipv6.hop_limit == 1) { - icmp_error(ICMP6_TIME_EXCEEDED, ICMP_EXC_TTL); - meta.drop_reason = DROP_IPV6_TTL_EXCEEDED; - // Dont set meta.dropped because we want an error packet - // to go out. + meta.dropped = true; + } else if (hdr.ipv6.hop_limit <= 8w1) { + // https://www.rfc-editor.org/rfc/rfc8200.html#section-3 + meta.drop_reason = DROP_MULTICAST_TTL_EXCEEDED; + meta.dropped = true; } else { // Set the destination port to an invalid value ig_tm_md.ucast_egress_port = (PortId_t)0x1ff; @@ -1507,13 +1512,13 @@ control L3Router( apply { if (hdr.ipv4.isValid()) { if (meta.is_mcast && !meta.is_link_local_mcastv6) { - MulticastRouter4.apply(hdr, meta, ig_intr_md, ig_tm_md); + MulticastRouter4.apply(hdr, meta, ig_tm_md); } else { Router4.apply(hdr, meta, ig_intr_md, ig_tm_md); } } else if (hdr.ipv6.isValid()) { if (meta.is_mcast && !meta.is_link_local_mcastv6) { - MulticastRouter6.apply(hdr, meta, ig_intr_md, ig_tm_md); + MulticastRouter6.apply(hdr, meta, ig_tm_md); } else { Router6.apply(hdr, meta, ig_intr_md, ig_tm_md); } @@ -1867,7 +1872,10 @@ control MulticastIngress ( * 3. asic_id_to_port : Map ASIC port ID to logical port number (0-255) * 4. port_bitmap_check : Test port's bit in bitmap (see port_bitmap_check.p4) * 5. modify_hdr : If bitmap_result != 0, decapsulate packet (strip - * outer headers, decrement TTL/hop limit, handle VLAN) + * outer headers, decrement TTL/hop limit, handle VLAN), + * unless the inner TTL/hop limit is <= 1, in which + * case the replica is dropped with + * DROP_MULTICAST_TTL_EXCEEDED * * The bitmap marks which egress ports require decapsulation (typically external * customer-facing ports) vs which keep encapsulation (underlay/sled-bound). @@ -1875,8 +1883,7 @@ control MulticastIngress ( control MulticastEgress ( inout sidecar_headers_t hdr, inout sidecar_egress_meta_t meta, - in egress_intrinsic_metadata_t eg_intr_md, - in egress_intrinsic_metadata_for_deparser_t eg_dprsr_md + in egress_intrinsic_metadata_t eg_intr_md ) { action set_decap_ports( @@ -2058,8 +2065,22 @@ control MulticastEgress ( port_bitmap_check.apply(); } if (meta.bitmap_result != 0) { - meta.ipv4_checksum_recalc = true; - modify_hdr.apply(); + // A decapsulated packet is forwarded + // as a normal hop, which requires the + // inner TTL check before modify_hdr + // actually decrements it: + // https://www.rfc-editor.org/rfc/rfc2003.html#section-3.1 + // https://www.rfc-editor.org/rfc/rfc2473.html#section-3.3 + if (hdr.inner_ipv4.isValid() && + hdr.inner_ipv4.ttl <= 8w1) { + meta.drop_reason = DROP_MULTICAST_TTL_EXCEEDED; + } else if (hdr.inner_ipv6.isValid() && + hdr.inner_ipv6.hop_limit <= 8w1) { + meta.drop_reason = DROP_MULTICAST_TTL_EXCEEDED; + } else { + meta.ipv4_checksum_recalc = true; + modify_hdr.apply(); + } } } } @@ -2386,8 +2407,10 @@ control Egress( // the packet meta.drop_reason = DROP_MULTICAST_PATH_FILTERED; } else { - mcast_egress.apply(hdr, meta, eg_intr_md, eg_dprsr_md); - mac_rewrite.apply(hdr, eg_intr_md.egress_port); + mcast_egress.apply(hdr, meta, eg_intr_md); + if (meta.drop_reason == 0) { + mac_rewrite.apply(hdr, eg_intr_md.egress_port); + } } } else if (meta.bridge_hdr.nat_egress_hit) { egress_filter.apply(meta, eg_intr_md); diff --git a/dpd/src/counters.rs b/dpd/src/counters.rs index fc25c0da..e65cd33c 100644 --- a/dpd/src/counters.rs +++ b/dpd/src/counters.rs @@ -199,6 +199,7 @@ enum DropReason { GeneveOptionMalformed, GeneveOptionUnknown, DropSctp, + MulticastTtlExceeded, } impl TryFrom for DropReason { @@ -234,6 +235,7 @@ impl TryFrom for DropReason { 25 => Ok(DropReason::GeneveOptionMalformed), 26 => Ok(DropReason::GeneveOptionUnknown), 27 => Ok(DropReason::DropSctp), + 28 => Ok(DropReason::MulticastTtlExceeded), x => Err(format!("Unrecognized drop reason: {x}")), } } @@ -263,7 +265,7 @@ fn reason_label(ctr: u8) -> Result, String> { DropReason::Ipv4Unrouteable => "ipv4_unrouteable".to_string(), DropReason::Ipv6Unrouteable => "ipv6_unrouteable".to_string(), DropReason::NatIngressMiss => "nat_ingress_miss".to_string(), - DropReason::NatEgressBlocked => "nat_engress_blocked".to_string(), + DropReason::NatEgressBlocked => "nat_egress_blocked".to_string(), DropReason::MulticastNoGroup => "multicast_no_group".to_string(), DropReason::MulticastInvalidMac => "multicast_invalid_mac".to_string(), DropReason::MulticastCpuCopy => "multicast_cpu_copy".to_string(), @@ -281,6 +283,9 @@ fn reason_label(ctr: u8) -> Result, String> { } DropReason::GeneveOptionUnknown => "geneve_option_unknown".to_string(), DropReason::DropSctp => "drop_sctp".to_string(), + DropReason::MulticastTtlExceeded => { + "multicast_ttl_exceeded".to_string() + } }; Ok(Some(label)) }