From 8feedbb02a304c526b21179ee309d078e31cfd0c Mon Sep 17 00:00:00 2001 From: Zeeshan Lakhani Date: Tue, 13 Oct 2026 06:11:37 +0000 Subject: [PATCH 1/2] [multicast] check inner TTL on decap MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The parser's TTL/hop limit checks cover only the outer header thus far. A zero inner TTL could wrap to 255 on decap, while on bifurcated egress, we could forward TTL 1 as 0. This work drops expired multicast packets before decrementing the TTL/hop limit, occurring both at ingress for external-only groups and at egress for bifurcated (replicated) groups. Underlay replicas do retain their inner packet. Drop counting now adds a `multicast_ttl_exceeded` reason to account for expiry. Also, this includes: - zero TTL/hop limit on unicast NAT egress now goes to Time Exceeded, along with TTL=1 (it used to wrap to 255) - no ICMP error requests for multicast route misses or expiry (RFC 1812 §4.3.2.7, RFC 4443 §2.4[e.3]) - `sc_egress` is set to a constant 0 on the sidecar paths that read `ucast_egress_port` before anything actually sets it (see P4-16 §8.25, unspecified value) - the mcast MAC rewrite table no longer runs on replicas we're dropping due to TTL (its direct counter had been counting them) - fix spelling: `nat_engress_blocked` -> `nat_egress_blocked` on the drop reason label (typo from #197) --- dpd-client/tests/integration_tests/common.rs | 16 +- dpd-client/tests/integration_tests/mcast.rs | 720 ++++++++++++++++++ dpd-client/tests/integration_tests/nat.rs | 142 +++- dpd-client/tests/integration_tests/service.rs | 2 +- dpd/p4/constants.p4 | 6 +- dpd/p4/sidecar.p4 | 113 +-- dpd/src/counters.rs | 7 +- 7 files changed, 917 insertions(+), 89 deletions(-) diff --git a/dpd-client/tests/integration_tests/common.rs b/dpd-client/tests/integration_tests/common.rs index d52eb08e..f70bd34b 100644 --- a/dpd-client/tests/integration_tests/common.rs +++ b/dpd-client/tests/integration_tests/common.rs @@ -680,10 +680,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 @@ -745,11 +754,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; } } @@ -790,7 +800,7 @@ impl Switch { } match errors.len() { - 0 => Ok(()), + 0 => Ok(matched), _ => Err(anyhow!( errors .iter() diff --git a/dpd-client/tests/integration_tests/mcast.rs b/dpd-client/tests/integration_tests/mcast.rs index 1d8a003b..3982735e 100644 --- a/dpd-client/tests/integration_tests/mcast.rs +++ b/dpd-client/tests/integration_tests/mcast.rs @@ -2633,6 +2633,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 diff --git a/dpd-client/tests/integration_tests/nat.rs b/dpd-client/tests/integration_tests/nat.rs index 9d7372a1..0b4c9e9c 100644 --- a/dpd-client/tests/integration_tests/nat.rs +++ b/dpd-client/tests/integration_tests/nat.rs @@ -195,7 +195,11 @@ 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, +) -> TestResult { let router_mac = test.router_mac.parse()?; // set up the switch internals so we can route the packet from the gimlet @@ -247,10 +251,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 { @@ -276,6 +280,14 @@ 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); } @@ -296,25 +308,57 @@ 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(), + 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 { @@ -466,6 +510,7 @@ async fn test_egress_ipv4( switch: &Switch, l4_protocol: L4Protocol, uplink_port_registered: bool, + inner_ttl: Option, ) -> TestResult { let test = NatTest { uplink_port: PhysPort(14), @@ -492,7 +537,7 @@ async fn test_egress_ipv4( geneve_vni: 1, // not used on egress tests }; - test_nat_egress(switch, &test).await + test_nat_egress(switch, &test, inner_ttl).await } // UDP packet to/from IPv4 addresses @@ -500,7 +545,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 @@ -508,7 +553,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 @@ -516,7 +561,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. @@ -524,7 +569,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( @@ -593,6 +638,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), @@ -619,28 +665,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).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/service.rs b/dpd-client/tests/integration_tests/service.rs index 2aeff4a5..bdf99cac 100644 --- a/dpd-client/tests/integration_tests/service.rs +++ b/dpd-client/tests/integration_tests/service.rs @@ -260,7 +260,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/sidecar.p4 b/dpd/p4/sidecar.p4 index 9cd96b8d..5e8cba49 100644 --- a/dpd/p4/sidecar.p4 +++ b/dpd/p4/sidecar.p4 @@ -342,7 +342,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(); @@ -1235,7 +1238,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(); @@ -1272,7 +1278,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. @@ -1291,23 +1297,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(); } @@ -1348,16 +1341,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; @@ -1380,7 +1372,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(); @@ -1415,7 +1410,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 @@ -1432,23 +1427,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(); } @@ -1488,16 +1470,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; @@ -1515,13 +1498,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); } @@ -1864,7 +1847,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). @@ -1872,8 +1858,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( @@ -2055,8 +2040,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(); + } } } } @@ -2337,8 +2336,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 b189cb6f..7ae3ec65 100644 --- a/dpd/src/counters.rs +++ b/dpd/src/counters.rs @@ -184,6 +184,7 @@ enum DropReason { GeneveOptionMalformed, GeneveOptionUnknown, DropSctp, + MulticastTtlExceeded, } impl TryFrom for DropReason { @@ -219,6 +220,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}")), } } @@ -248,7 +250,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(), @@ -266,6 +268,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)) } From 7e10cfbe43c8370cbb0a53bde42ba2fa19fe0b7e Mon Sep 17 00:00:00 2001 From: Zeeshan Lakhani Date: Mon, 19 Oct 2026 07:27:55 +0000 Subject: [PATCH 2/2] [p4] receive ttl/hop limit 0 when the switch is the destination MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Ry flagged this on oxidecomputer/sidecar-lite#152 (its handling of ff02 reject was emulating this sidecar). Ever since #14, the parser dropped ttl/hl (hop-limit) 0 before `services` was involved, which meant that a unicast packet headed to a switch address and `ff02::/16` headed to the scrimlet would both be dropped. RFC 1812 §4.2.2.9 and RFC 8200 §3 mention that the destination should still accept it, while the forwarding pass still has to drop (per RFC 1812 §5.3.1). This work updates the sidecar parser to record the reason via meta and then continue parsing. For unicast and IPv6 link-local multicast, the drop has been moved to the forwarding side. Note: some of the arrangement here is to make this change while operating at our 20-stage max. Includes: - mcast packets at ttl/hl 1 now get counted as `multicast_ttl_exceeded` - Geneve encapped packets headed to a switch address with an outer hop limit of 0 get decapped again, as it was before #14 --- .../tests/integration_tests/counters.rs | 38 +++++++ dpd-client/tests/integration_tests/mcast.rs | 8 +- dpd-client/tests/integration_tests/nat.rs | 37 +++++-- .../tests/integration_tests/route_ipv6.rs | 39 ++++++++ dpd-client/tests/integration_tests/service.rs | 99 ++++++++++++++++++- dpd/p4/parser.p4 | 42 +++++--- dpd/p4/sidecar.p4 | 28 +++++- 7 files changed, 260 insertions(+), 31 deletions(-) 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 9b70b791..119e040f 100644 --- a/dpd-client/tests/integration_tests/mcast.rs +++ b/dpd-client/tests/integration_tests/mcast.rs @@ -1858,13 +1858,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, @@ -3716,13 +3716,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 0b4c9e9c..391a4c13 100644 --- a/dpd-client/tests/integration_tests/nat.rs +++ b/dpd-client/tests/integration_tests/nat.rs @@ -199,6 +199,7 @@ async fn test_nat_egress( switch: &Switch, test: &NatTest, inner_ttl: Option, + outer_hop_limit: Option, ) -> TestResult { let router_mac = test.router_mac.parse()?; @@ -294,7 +295,7 @@ async fn test_nat_egress( 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, @@ -308,6 +309,10 @@ async fn test_nat_egress( &payload[14..], ); + 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(); @@ -506,13 +511,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, - inner_ttl: Option, -) -> TestResult { - let test = NatTest { +) -> NatTest { + NatTest { uplink_port: PhysPort(14), uplink_port_external: "192.168.1.2".to_string(), uplink_port_registered, @@ -535,9 +538,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, inner_ttl).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 @@ -665,7 +684,7 @@ async fn test_egress_ipv6( geneve_vni: 1, // not used on egress tests }; - test_nat_egress(switch, &test, inner_ttl).await + test_nat_egress(switch, &test, inner_ttl, None).await } #[tokio::test] diff --git a/dpd-client/tests/integration_tests/route_ipv6.rs b/dpd-client/tests/integration_tests/route_ipv6.rs index dbdf6917..8d9a6e88 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 bdf99cac..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] diff --git a/dpd/p4/parser.p4 b/dpd/p4/parser.p4 index 7de38b6b..b9974c23 100644 --- a/dpd/p4/parser.p4 +++ b/dpd/p4/parser.p4 @@ -151,11 +151,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; + } + // IPv4 Multicast Address Validation (RFC 1112, RFC 7042). // // This validates that IPv4 multicast packets (224.0.0.0/4) use the proper @@ -204,7 +217,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; } } @@ -217,6 +230,12 @@ parser IngressParser( transition accept; } + state mcast_ttl_exceeded { + meta.drop_reason = DROP_MULTICAST_TTL_EXCEEDED; + meta.dropped = true; + transition accept; + } + state invalidate_ipv4_mcast { meta.is_mcast = true; meta.drop_reason = DROP_MULTICAST_INVALID_MAC; @@ -267,7 +286,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 { @@ -281,26 +300,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 c44e1b4e..18a62d04 100644 --- a/dpd/p4/sidecar.p4 +++ b/dpd/p4/sidecar.p4 @@ -726,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()) { @@ -1251,6 +1261,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; @@ -1385,6 +1401,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;