From a4ca6f6aa53e34a8f3e9a700d019817a7a162e18 Mon Sep 17 00:00:00 2001 From: Trey Aspelund Date: Tue, 3 Nov 2026 14:22:53 -0700 Subject: [PATCH 1/2] tfportd: handle cross-AF nexthop resolution tfportd's handling of SC_ARP_NEEDED and SC_NEIGHBOR_NEEDED sidecar packets bailed early if the original packet (within the sidecar payload) did not have a valid IP header with the same IP version as the nexthop. This logic does not account for BGP unnumbered or other cross-AF route installations (e.g. 10.0.0.0/8 via fe80::1%qsfp0), resulting in tfportd never attempting to resolve the nexthop. Refactors handle_arp_needed and handle_ndp_needed into a single helper which tries to resolve (asic_id, ip) from the punted packet. Adds unit tests that ensure intra-AF and cross-AF resolution works, and that non-IP packets fail. Signed-off-by: Trey Aspelund --- dpd-client/tests/integration_tests/service.rs | 56 +++++++ packet/src/sidecar.rs | 140 +++++++++++++++++- tfportd/src/sidecar.rs | 41 ++--- 3 files changed, 205 insertions(+), 32 deletions(-) diff --git a/dpd-client/tests/integration_tests/service.rs b/dpd-client/tests/integration_tests/service.rs index 78e70bee..47f98302 100644 --- a/dpd-client/tests/integration_tests/service.rs +++ b/dpd-client/tests/integration_tests/service.rs @@ -277,6 +277,62 @@ async fn test_ndp_needed() -> TestResult { Ok(()) } +// An IPv4 packet routed via an IPv6 nexthop (e.g. BGP unnumbered) with no +// neighbor entry should come up to the service port asking for NDP resolution +// of the IPv6 nexthop. +#[tokio::test] +#[ignore] +async fn test_ndp_needed_ipv4_via_ipv6() -> TestResult { + let switch = &*get_switch().await; + + let ingress_port = PhysPort(10); + let egress_port = PhysPort(15); + let router_ip = "fe80::50"; + let src_mac = "e0:d5:5e:67:89:ab"; + let router_mac = switch.get_port_mac(ingress_port).unwrap(); + + common::set_route_ipv4_over_ipv6( + switch, + "10.10.50.0/24", + egress_port, + router_ip, + ) + .await?; + + let (send_pkt, mut recv_pkt) = common::gen_udp_routed_pair( + switch, + egress_port, + router_mac, + Endpoint::parse(src_mac, "10.10.10.10", 3333).unwrap(), + Endpoint::parse("e0:d5:5e:67:89:ac", "10.10.50.11", 4444).unwrap(), + ); + + let send = TestPacket { packet: Arc::new(send_pkt), port: ingress_port }; + + let payload = router_ip.parse::().unwrap().octets(); + common::add_sidecar_hdr( + switch, + &mut recv_pkt, + sidecar::SC_NEIGHBOR_NEEDED, + ingress_port, + egress_port, + Some(&payload[..]), + ); + + // Because the packet wasn't successfully routed, the src/dst mac addresses + // should be unchanged. + let src_eth = send.packet.hdrs.eth_hdr.as_ref().unwrap(); + eth::EthHdr::rewrite_smac(&mut recv_pkt, src_eth.eth_smac); + eth::EthHdr::rewrite_dmac(&mut recv_pkt, src_eth.eth_dmac); + + let expected = + TestPacket { packet: Arc::new(recv_pkt), port: SERVICE_PORT }; + + assert!(one_counted_test(switch, send, Some(expected), "ndp_miss").await?); + + Ok(()) +} + #[tokio::test] #[ignore] async fn test_ttl_exceeded() -> TestResult { diff --git a/packet/src/sidecar.rs b/packet/src/sidecar.rs index fbd4cc62..90c53b61 100644 --- a/packet/src/sidecar.rs +++ b/packet/src/sidecar.rs @@ -2,10 +2,11 @@ // License, v. 2.0. If a copy of the MPL was not distributed with this // file, You can obtain one at https://mozilla.org/MPL/2.0/ // -// Copyright 2025 Oxide Computer Company +// Copyright 2026 Oxide Computer Company use std::convert::TryInto; use std::fmt; +use std::net::IpAddr; use bytes::BufMut; use bytes::BytesMut; @@ -132,6 +133,35 @@ pub fn code_to_str(code: u8) -> String { .to_string() } +/// For an SC_ARP_NEEDED or SC_NEIGHBOR_NEEDED packet, returns the egress +/// ASIC port and the nexthop address awaiting resolution. The nexthop's +/// address family comes from the sidecar code rather than the encapsulated +/// packet, since a route may have a nexthop of the other address family +/// (e.g. 10.0.0.0/8 via fe80::1). +pub fn nexthop_needed(p: &Packet) -> Option<(u16, IpAddr)> { + if p.hdrs.ipv4_hdr.is_none() && p.hdrs.ipv6_hdr.is_none() { + // not an IP packet, nothing to resolve + return None; + } + + let sc = p.hdrs.sidecar_hdr.as_ref()?; + let ip = match sc.sc_code { + SC_ARP_NEEDED => { + let x = + u32::from_be_bytes(sc.sc_payload[12..16].try_into().unwrap()); + IpAddr::V4(x.into()) + } + SC_NEIGHBOR_NEEDED => { + let x = + u128::from_be_bytes(sc.sc_payload[0..16].try_into().unwrap()); + IpAddr::V6(x.into()) + } + // wrong sidecar code, nothing to resolve + _ => return None, + }; + Some((sc.sc_egress, ip)) +} + impl fmt::Display for SidecarHdr { fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { write!( @@ -157,3 +187,111 @@ impl fmt::Debug for SidecarHdr { ) } } + +#[cfg(test)] +mod tests { + use super::*; + use std::net::{Ipv4Addr, Ipv6Addr}; + + const SC_EGRESS: u16 = 7; + + fn endpoints(src: &str, dst: &str) -> (Endpoint, Endpoint) { + ( + Endpoint::parse("e0:d5:5e:67:89:ab", src, 3333).unwrap(), + Endpoint::parse("e0:d5:5e:67:89:ac", dst, 4444).unwrap(), + ) + } + + fn udp_pkt(src: &str, dst: &str) -> Packet { + let (src_ep, dst_ep) = endpoints(src, dst); + let stack = if src.contains(':') { + vec![ipv6::IPPROTO_UDP.into(), eth::ETHER_IPV6] + } else { + vec![ipv4::IPPROTO_UDP.into(), eth::ETHER_IPV4] + }; + Packet::generate(src_ep, dst_ep, stack, None).unwrap() + } + + // Wrap `p` in a sidecar header, as the switch does when it punts a packet + // to userspace. + fn add_sidecar(mut p: Packet, sc_code: u8, sc_payload: [u8; 16]) -> Packet { + let eth = p.hdrs.eth_hdr.as_mut().unwrap(); + p.hdrs.sidecar_hdr = Some(SidecarHdr { + sc_code, + sc_pad: 0, + sc_ingress: 1, + sc_egress: SC_EGRESS, + sc_ether_type: eth.eth_type, + sc_payload, + }); + eth.eth_type = eth::ETHER_SIDECAR; + p + } + + // The P4 code widens the 32-bit IPv4 nexthop to 128 bits, so the address + // lands in the last four bytes of the payload. + fn v4_payload(ip: Ipv4Addr) -> [u8; 16] { + let mut payload = [0u8; 16]; + payload[12..].copy_from_slice(&ip.octets()); + payload + } + + #[test] + fn nexthop_needed_ipv4_via_ipv4() { + let nh: Ipv4Addr = "192.0.2.1".parse().unwrap(); + let p = add_sidecar( + udp_pkt("10.0.0.1", "10.0.0.2"), + SC_ARP_NEEDED, + v4_payload(nh), + ); + assert_eq!(nexthop_needed(&p), Some((SC_EGRESS, nh.into()))); + } + + #[test] + fn nexthop_needed_ipv6_via_ipv6() { + let nh: Ipv6Addr = "fd00:1122:3344::1".parse().unwrap(); + let p = add_sidecar( + udp_pkt("fd00::1", "fd00::2"), + SC_NEIGHBOR_NEEDED, + nh.octets(), + ); + assert_eq!(nexthop_needed(&p), Some((SC_EGRESS, nh.into()))); + } + + #[test] + fn nexthop_needed_ipv4_via_ipv6() { + let nh: Ipv6Addr = "fe80::1".parse().unwrap(); + let p = add_sidecar( + udp_pkt("10.0.0.1", "10.0.0.2"), + SC_NEIGHBOR_NEEDED, + nh.octets(), + ); + assert_eq!(nexthop_needed(&p), Some((SC_EGRESS, nh.into()))); + } + + #[test] + fn nexthop_needed_ipv6_via_ipv4() { + let nh: Ipv4Addr = "192.0.2.1".parse().unwrap(); + let p = add_sidecar( + udp_pkt("fd00::1", "fd00::2"), + SC_ARP_NEEDED, + v4_payload(nh), + ); + assert_eq!(nexthop_needed(&p), Some((SC_EGRESS, nh.into()))); + } + + #[test] + fn nexthop_needed_non_ip_pkt() { + let (src, dst) = endpoints("10.0.0.1", "10.0.0.2"); + let arp_pkt = Packet::generate( + src, + dst, + vec![arp::ARPOP_REQUEST, eth::ETHER_ARP], + None, + ) + .unwrap(); + let nh: Ipv4Addr = "192.0.2.1".parse().unwrap(); + let p = add_sidecar(arp_pkt, SC_ARP_NEEDED, v4_payload(nh)); + assert_eq!(nexthop_needed(&p), None); + } +} diff --git a/tfportd/src/sidecar.rs b/tfportd/src/sidecar.rs index d87c0298..aba7d7fc 100644 --- a/tfportd/src/sidecar.rs +++ b/tfportd/src/sidecar.rs @@ -2,7 +2,7 @@ // License, v. 2.0. If a copy of the MPL was not distributed with this // file, You can obtain one at https://mozilla.org/MPL/2.0/ // -// Copyright 2025 Oxide Computer Company +// Copyright 2026 Oxide Computer Company use std::net::IpAddr; use std::sync::Arc; @@ -100,34 +100,6 @@ pub fn resolution_needed( } } -pub fn handle_neighbor_needed(g: &Global, p: packet::Packet) { - if p.hdrs.ipv6_hdr.is_none() { - return error!(g.log, "not an ipv6 packet"); - } - - let sc = p.hdrs.sidecar_hdr.unwrap(); - let asic_id = p.hdrs.sidecar_hdr.as_ref().unwrap().sc_egress; - let ipv6 = { - let x = u128::from_be_bytes(sc.sc_payload[0..16].try_into().unwrap()); - x.into() - }; - resolution_needed(g, asic_id, IpAddr::V6(ipv6), p); -} - -pub fn handle_arp_needed(g: &Global, p: packet::Packet) { - if p.hdrs.ipv4_hdr.is_none() { - return error!(g.log, "not an ipv4 packet: {:?}", p); - } - - let sc = p.hdrs.sidecar_hdr.unwrap(); - let asic_id = p.hdrs.sidecar_hdr.as_ref().unwrap().sc_egress; - let ipv4 = { - let x = u32::from_be_bytes(sc.sc_payload[12..16].try_into().unwrap()); - x.into() - }; - resolution_needed(g, asic_id, IpAddr::V4(ipv4), p); -} - fn handle_packet(g: &Global, p: packet::Packet) { let sc_code = match &p.hdrs.sidecar_hdr { Some(sc) => sc.sc_code, @@ -142,8 +114,15 @@ fn handle_packet(g: &Global, p: packet::Packet) { // These packets were addressed to a port that doesn't have a corresponding // tfport link. There's nothing we want to do with them. } - sidecar::SC_ARP_NEEDED => handle_arp_needed(g, p), - sidecar::SC_NEIGHBOR_NEEDED => handle_neighbor_needed(g, p), + sidecar::SC_ARP_NEEDED | sidecar::SC_NEIGHBOR_NEEDED => { + match sidecar::nexthop_needed(&p) { + Some((asic_id, ip)) => resolution_needed(g, asic_id, ip, p), + None => error!( + g.log, + "not an ip packet (sc_code: {sc_code}): {p:?}" + ), + } + } // TODO-completeness: implement this as per dendrite#156 sidecar::SC_ICMP_NEEDED => {} x => debug!(g.log, "unrecognized sidecar code: {:x}", x), From b957a19ca3aa51b3457362a274ebcde1c61b00c3 Mon Sep 17 00:00:00 2001 From: Trey Aspelund Date: Fri, 6 Nov 2026 20:31:55 -0600 Subject: [PATCH 2/2] packet: parse sidecar nexthop without unwrap Signed-off-by: Trey Aspelund --- packet/src/sidecar.rs | 16 ++++++---------- 1 file changed, 6 insertions(+), 10 deletions(-) diff --git a/packet/src/sidecar.rs b/packet/src/sidecar.rs index 90c53b61..6e3e3885 100644 --- a/packet/src/sidecar.rs +++ b/packet/src/sidecar.rs @@ -6,7 +6,7 @@ use std::convert::TryInto; use std::fmt; -use std::net::IpAddr; +use std::net::{IpAddr, Ipv4Addr, Ipv6Addr}; use bytes::BufMut; use bytes::BytesMut; @@ -146,16 +146,13 @@ pub fn nexthop_needed(p: &Packet) -> Option<(u16, IpAddr)> { let sc = p.hdrs.sidecar_hdr.as_ref()?; let ip = match sc.sc_code { + // The P4 code zero-extends the 32-bit nexthop to fill the 128-bit + // payload, so the address is in the last four bytes. SC_ARP_NEEDED => { - let x = - u32::from_be_bytes(sc.sc_payload[12..16].try_into().unwrap()); - IpAddr::V4(x.into()) - } - SC_NEIGHBOR_NEEDED => { - let x = - u128::from_be_bytes(sc.sc_payload[0..16].try_into().unwrap()); - IpAddr::V6(x.into()) + let [.., a, b, c, d] = sc.sc_payload; + IpAddr::V4(Ipv4Addr::new(a, b, c, d)) } + SC_NEIGHBOR_NEEDED => IpAddr::V6(Ipv6Addr::from(sc.sc_payload)), // wrong sidecar code, nothing to resolve _ => return None, }; @@ -191,7 +188,6 @@ impl fmt::Debug for SidecarHdr { #[cfg(test)] mod tests { use super::*; - use std::net::{Ipv4Addr, Ipv6Addr}; const SC_EGRESS: u16 = 7;