From 07a82d225455e09b29f64acf9a89f6a7cc3e4a05 Mon Sep 17 00:00:00 2001 From: Thomas Eizinger Date: Mon, 7 Apr 2025 03:31:22 +0000 Subject: [PATCH] chore(relay): remove feature flag for eBPF TURN router (#8681) The original idea of this feature flag was that we can easily disable the eBPF router in case it causes issues in production. However, something seems to be not working in reliably turning this on / off. Without an explicit toggle of the feature-flag, the eBPF program doesn't seem to be loaded correctly. The uncertainty in this makes me not the trust the metrics that we are seeing because we don't know, whether really all relays are using the eBPF router to relay TURN traffic. In order to draw truthful conclusions as too how much traffic we are relaying via eBPF, this patch removes the feature flag again. As of #8656, we can disable the eBPF program by not setting the `EBPF_OFFLOADING` env variable. This requires a re-deploy / restart of relays to take effect which isn't quite as fast as toggling a feature flag but much reliable and easier to maintain. --- rust/relay/ebpf-shared/src/lib.rs | 2 -- rust/relay/ebpf-turn-router/src/config.rs | 4 ---- rust/relay/ebpf-turn-router/src/main.rs | 4 ---- rust/relay/server/src/main.rs | 17 ----------------- rust/telemetry/src/feature_flags.rs | 9 --------- 5 files changed, 36 deletions(-) diff --git a/rust/relay/ebpf-shared/src/lib.rs b/rust/relay/ebpf-shared/src/lib.rs index cb88aa5aa..16615b86c 100644 --- a/rust/relay/ebpf-shared/src/lib.rs +++ b/rust/relay/ebpf-shared/src/lib.rs @@ -204,7 +204,6 @@ impl PortAndPeerV6 { #[derive(Clone, Copy, PartialEq, Eq)] #[cfg_attr(feature = "std", derive(Debug))] pub struct Config { - pub relaying_enabled: bool, pub udp_checksum_enabled: bool, pub lowest_allocation_port: u16, pub highest_allocation_port: u16, @@ -213,7 +212,6 @@ pub struct Config { impl Default for Config { fn default() -> Self { Self { - relaying_enabled: true, udp_checksum_enabled: true, lowest_allocation_port: 49152, highest_allocation_port: 65535, diff --git a/rust/relay/ebpf-turn-router/src/config.rs b/rust/relay/ebpf-turn-router/src/config.rs index 33aea8630..4e89a35c6 100644 --- a/rust/relay/ebpf-turn-router/src/config.rs +++ b/rust/relay/ebpf-turn-router/src/config.rs @@ -11,10 +11,6 @@ pub fn udp_checksum_enabled() -> bool { config().udp_checksum_enabled } -pub fn relaying_enabled() -> bool { - config().relaying_enabled -} - pub fn allocation_range() -> RangeInclusive { let config = config(); diff --git a/rust/relay/ebpf-turn-router/src/main.rs b/rust/relay/ebpf-turn-router/src/main.rs index 4b5fa24d6..f47af7e11 100644 --- a/rust/relay/ebpf-turn-router/src/main.rs +++ b/rust/relay/ebpf-turn-router/src/main.rs @@ -97,10 +97,6 @@ pub fn handle_turn(ctx: XdpContext) -> u32 { #[inline(always)] fn try_handle_turn(ctx: &XdpContext) -> Result { - if !config::relaying_enabled() { - return Ok(xdp_action::XDP_PASS); - } - let eth = Eth::parse(ctx)?; match eth.ether_type() { diff --git a/rust/relay/server/src/main.rs b/rust/relay/server/src/main.rs index 55eaf17e5..823a8b7f2 100644 --- a/rust/relay/server/src/main.rs +++ b/rust/relay/server/src/main.rs @@ -117,7 +117,6 @@ fn main() { VERSION.unwrap_or("unknown"), RELAY_DSN, ); - Telemetry::set_firezone_id(uuid::Uuid::new_v4().to_string()); } let runtime = tokio::runtime::Builder::new_current_thread() @@ -148,7 +147,6 @@ async fn try_main(args: Args) -> Result<()> { if let Some(ebpf) = ebpf.as_mut() { ebpf.set_config(Config { - relaying_enabled: true, udp_checksum_enabled: true, lowest_allocation_port: args.lowest_port, highest_allocation_port: args.highest_port, @@ -649,21 +647,6 @@ where ready = true; } - if let Some(ebpf) = self.ebpf.as_mut() { - let is_enabled = ebpf.config().relaying_enabled; - let should_be_enabled = - firezone_telemetry::feature_flags::ebpf_turn_router_enabled(); - - if is_enabled != should_be_enabled { - tracing::info!(%is_enabled, %should_be_enabled, "eBPF router feature-flag changed"); - - ebpf.set_config(Config { - relaying_enabled: should_be_enabled, - ..ebpf.config() - })?; - } - } - if !ready { break Poll::Pending; } diff --git a/rust/telemetry/src/feature_flags.rs b/rust/telemetry/src/feature_flags.rs index 730fecca6..3643a951c 100644 --- a/rust/telemetry/src/feature_flags.rs +++ b/rust/telemetry/src/feature_flags.rs @@ -24,10 +24,6 @@ pub fn drop_llmnr_nxdomain_responses() -> bool { FEATURE_FLAGS.read().drop_llmnr_nxdomain_responses } -pub fn ebpf_turn_router_enabled() -> bool { - FEATURE_FLAGS.read().ebpf_turn_router_enabled -} - pub(crate) fn reevaluate(user_id: String, env: &str) { let api_key = match env { crate::env::PRODUCTION => POSTHOG_API_KEY_PROD, @@ -134,8 +130,6 @@ struct FeatureFlags { icmp_unreachable_instead_of_nat64: bool, #[serde(default)] drop_llmnr_nxdomain_responses: bool, - #[serde(default)] - ebpf_turn_router_enabled: bool, } fn sentry_flag_context(flags: FeatureFlags) -> sentry::protocol::Context { @@ -144,14 +138,12 @@ fn sentry_flag_context(flags: FeatureFlags) -> sentry::protocol::Context { enum SentryFlag { IcmpUnreachableInsteadOfNat64 { result: bool }, DropLlmnrNxdomainResponses { result: bool }, - EbpfTurnRouterEnabled { result: bool }, } // Exhaustive destruction so we don't forget to update this when we add a flag. let FeatureFlags { icmp_unreachable_instead_of_nat64, drop_llmnr_nxdomain_responses, - ebpf_turn_router_enabled, } = flags; let value = serde_json::json!({ @@ -160,7 +152,6 @@ fn sentry_flag_context(flags: FeatureFlags) -> sentry::protocol::Context { result: icmp_unreachable_instead_of_nat64, }, SentryFlag::DropLlmnrNxdomainResponses { result: drop_llmnr_nxdomain_responses }, - SentryFlag::EbpfTurnRouterEnabled { result: ebpf_turn_router_enabled } ] });