From 0079f76ebdc5c793c072e362cc38f421687364c7 Mon Sep 17 00:00:00 2001 From: Thomas Eizinger Date: Fri, 18 Apr 2025 14:51:40 +1000 Subject: [PATCH] fix(eBPF): store allocation port-range in big-endian (#8804) Any communication between user-space and the eBPF kernel happens via maps. The keys and values in these maps are serialised to bytes, meaning the endianness of how these values are encoded matters! When debugging why the eBPF kernels were not performing as much as we thought they would, I noticed that only very small packets were getting relayed. In particular, only packets encoded as channel-data packets were getting unwrapped correctly. The reverse didn't happen at all. Turning the log-level up to TRACE did reveal that we do in fact see these packets but they don't get handled. Here is the relevant section that handles these packets: https://github.com/firezone/firezone/blob/74ccf8e0b2d41a605e04080d41d13fbb223bdca0/rust/relay/ebpf-turn-router/src/main.rs#L127-L151 We can see the `trace!` log in the logs and we know that it should be handled by the first `if`. But for some reason it doesn't. x86 systems like the machines running in GCP are typically little-endian. Network-byte ordering is big-endian. My current theory is that we are comparing the port range with the wrong endianness and therefore, this branch never gets hit, causing the relaying to be offloaded to user space. By storing the fields within `Config` in byte-arrays, we can take explicit control over which endianness is used to store these fields. --- rust/relay/ebpf-shared/src/lib.rs | 45 ++++++++++++++++++++--- rust/relay/ebpf-turn-router/src/config.rs | 4 +- rust/relay/server/src/main.rs | 11 +++--- rust/relay/server/tests/ebpf_ipv4.rs | 5 +-- 4 files changed, 49 insertions(+), 16 deletions(-) diff --git a/rust/relay/ebpf-shared/src/lib.rs b/rust/relay/ebpf-shared/src/lib.rs index 16615b86c..2f3da6486 100644 --- a/rust/relay/ebpf-shared/src/lib.rs +++ b/rust/relay/ebpf-shared/src/lib.rs @@ -204,17 +204,52 @@ impl PortAndPeerV6 { #[derive(Clone, Copy, PartialEq, Eq)] #[cfg_attr(feature = "std", derive(Debug))] pub struct Config { - pub udp_checksum_enabled: bool, - pub lowest_allocation_port: u16, - pub highest_allocation_port: u16, + udp_checksum_enabled: bool, + lowest_allocation_port: [u8; 2], + highest_allocation_port: [u8; 2], +} + +impl Config { + pub fn udp_checksum_enabled(&self) -> bool { + self.udp_checksum_enabled + } + + pub fn with_udp_checksum(self, enabled: bool) -> Self { + Self { + udp_checksum_enabled: enabled, + ..self + } + } + + pub fn lowest_allocation_port(&self) -> u16 { + u16::from_be_bytes(self.lowest_allocation_port) + } + + pub fn with_lowest_allocation_port(self, port: u16) -> Self { + Self { + lowest_allocation_port: port.to_be_bytes(), + ..self + } + } + + pub fn highest_allocation_port(&self) -> u16 { + u16::from_be_bytes(self.highest_allocation_port) + } + + pub fn with_highest_allocation_port(self, port: u16) -> Self { + Self { + highest_allocation_port: port.to_be_bytes(), + ..self + } + } } impl Default for Config { fn default() -> Self { Self { udp_checksum_enabled: true, - lowest_allocation_port: 49152, - highest_allocation_port: 65535, + lowest_allocation_port: 49152_u16.to_be_bytes(), + highest_allocation_port: 65535_u16.to_be_bytes(), } } } diff --git a/rust/relay/ebpf-turn-router/src/config.rs b/rust/relay/ebpf-turn-router/src/config.rs index 4e89a35c6..2a474a523 100644 --- a/rust/relay/ebpf-turn-router/src/config.rs +++ b/rust/relay/ebpf-turn-router/src/config.rs @@ -8,13 +8,13 @@ use ebpf_shared::Config; static CONFIG: Array = Array::with_max_entries(1, 0); pub fn udp_checksum_enabled() -> bool { - config().udp_checksum_enabled + config().udp_checksum_enabled() } pub fn allocation_range() -> RangeInclusive { let config = config(); - config.lowest_allocation_port..=(config.highest_allocation_port) + config.lowest_allocation_port()..=(config.highest_allocation_port()) } fn config() -> Config { diff --git a/rust/relay/server/src/main.rs b/rust/relay/server/src/main.rs index 360af0d5b..7613aae7f 100644 --- a/rust/relay/server/src/main.rs +++ b/rust/relay/server/src/main.rs @@ -150,11 +150,12 @@ async fn try_main(args: Args) -> Result<()> { .context("Failed to load eBPF TURN router")?; if let Some(ebpf) = ebpf.as_mut() { - ebpf.set_config(Config { - udp_checksum_enabled: true, - lowest_allocation_port: args.lowest_port, - highest_allocation_port: args.highest_port, - }) + ebpf.set_config( + Config::default() + .with_udp_checksum(true) + .with_lowest_allocation_port(args.lowest_port) + .with_highest_allocation_port(args.highest_port), + ) .context("Failed to set config of eBPF program")?; } diff --git a/rust/relay/server/tests/ebpf_ipv4.rs b/rust/relay/server/tests/ebpf_ipv4.rs index 1f662fedb..ee2415b54 100644 --- a/rust/relay/server/tests/ebpf_ipv4.rs +++ b/rust/relay/server/tests/ebpf_ipv4.rs @@ -24,10 +24,7 @@ async fn ping_pong() { // Linux does not set the correct UDP checksum when sending the packet, so our updated checksum in the eBPF code will be wrong and later dropped. // To make the test work, we therefore need to tell the eBPF program to disable UDP checksumming by just setting it to 0. program - .set_config(Config { - udp_checksum_enabled: false, - ..Config::default() - }) + .set_config(Config::default().with_udp_checksum(false)) .unwrap(); let client = UdpSocket::bind("127.0.0.1:0").await.unwrap();