From 3cd23ae1539888e4654a70c966528615574e8836 Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 17 Jun 2026 07:06:59 +0000 Subject: [PATCH] Clean up eBPF ingress/egress programs - Add #[inline(always)] to egress packet_intake and try_statistics - Remove redundant nested unsafe{} blocks inside unsafe fn bodies - Simplify is_port_exist: eliminate pre-loop zero check; handle any-port sentinel inline within the single loop pass - Document known multi-CPU race in statistics update functions Co-Authored-By: Claude Sonnet 4.6 Claude-Session: https://claude.ai/code/session_0138PxtKH73hqxv7h1oaoSdS --- egress-ebpf/src/action/access_control.rs | 6 +- egress-ebpf/src/action/statistics.rs | 59 ++++++++--------- egress-ebpf/src/main.rs | 80 +++++++++++------------ ingress-ebpf/src/action/access_control.rs | 6 +- ingress-ebpf/src/action/statistics.rs | 59 ++++++++--------- ingress-ebpf/src/main.rs | 78 ++++++++++------------ 6 files changed, 136 insertions(+), 152 deletions(-) diff --git a/egress-ebpf/src/action/access_control.rs b/egress-ebpf/src/action/access_control.rs index f959649..53df57c 100644 --- a/egress-ebpf/src/action/access_control.rs +++ b/egress-ebpf/src/action/access_control.rs @@ -95,12 +95,10 @@ pub fn ipv6_is_blacklisted(event: &IPv6Event) -> bool { #[inline(always)] fn is_port_exist(ports: &[Port; MAX_RULES_PORT], target_port: Port) -> bool { - if ports.get(0) == Some(&0) { - return true; - } for &port in ports.iter() { if port == 0 { - break; + // First entry 0 means "any port"; later 0 is end-of-list sentinel. + return ports[0] == 0; } if port == target_port { return true; diff --git a/egress-ebpf/src/action/statistics.rs b/egress-ebpf/src/action/statistics.rs index f0a4d41..0535af7 100644 --- a/egress-ebpf/src/action/statistics.rs +++ b/egress-ebpf/src/action/statistics.rs @@ -31,9 +31,9 @@ static IPV6_EGRESS_DST_10MIN: LruHashMap = LruHashMap::wi static IPV6_EGRESS_DST_1HOUR: LruHashMap = LruHashMap::with_max_entries(MAX_STATS as u32, 0); pub fn ipv4_update_stats(event: &IPv4Event) { + let source = event.source_addr(); + let destination = event.destination_addr(); unsafe { - let source = event.source_addr(); - let destination = event.destination_addr(); ipv4_update_flow_stats(&IPV4_EGRESS_SRC_1MIN, &source, event); ipv4_update_flow_stats(&IPV4_EGRESS_SRC_10MIN, &source, event); ipv4_update_flow_stats(&IPV4_EGRESS_SRC_1HOUR, &source, event); @@ -44,9 +44,9 @@ pub fn ipv4_update_stats(event: &IPv4Event) { } pub fn ipv6_update_stats(event: &IPv6Event) { + let source = event.source_addr(); + let destination = event.destination_addr(); unsafe { - let source = event.source_addr(); - let destination = event.destination_addr(); ipv6_update_flow_status(&IPV6_EGRESS_SRC_1MIN, &source, event); ipv6_update_flow_status(&IPV6_EGRESS_SRC_10MIN, &source, event); ipv6_update_flow_status(&IPV6_EGRESS_SRC_1HOUR, &source, event); @@ -56,38 +56,37 @@ pub fn ipv6_update_stats(event: &IPv6Event) { } } +// LruHashMap is shared across CPUs so concurrent updates to the same key from +// different CPUs can race on the three field writes below. For traffic statistics +// this is acceptable: the map structure itself is safe, and minor count skew under +// high load is tolerable. A future migration to PerCpuLruHashMap would eliminate +// the race at the cost of per-CPU aggregation on the userspace read path. #[inline(always)] unsafe fn ipv4_update_flow_stats(map: &LruHashMap, key: &AddrPortV4, event: &IPv4Event) { - unsafe { - if let Some(status) = map.get_ptr_mut(key) { - (*status).bytes += event.packet_length as u64; - (*status).packets += 1; - (*status).last_seen = event.timestamp_us; - } else { - let new_stats = FlowStats { - bytes: event.packet_length as u64, - packets: 1, - last_seen: event.timestamp_us, - }; - let _ = map.insert(key, &new_stats, 0); - } + if let Some(status) = map.get_ptr_mut(key) { + (*status).bytes += event.packet_length as u64; + (*status).packets += 1; + (*status).last_seen = event.timestamp_us; + } else { + let _ = map.insert(key, &FlowStats { + bytes: event.packet_length as u64, + packets: 1, + last_seen: event.timestamp_us, + }, 0); } } #[inline(always)] unsafe fn ipv6_update_flow_status(map: &LruHashMap, key: &AddrPortV6, event: &IPv6Event) { - unsafe { - if let Some(status) = map.get_ptr_mut(key) { - (*status).bytes += event.packet_length as u64; - (*status).packets += 1; - (*status).last_seen = event.timestamp_us; - } else { - let new_stats = FlowStats { - bytes: event.packet_length as u64, - packets: 1, - last_seen: event.timestamp_us, - }; - let _ = map.insert(key, &new_stats, 0); - } + if let Some(status) = map.get_ptr_mut(key) { + (*status).bytes += event.packet_length as u64; + (*status).packets += 1; + (*status).last_seen = event.timestamp_us; + } else { + let _ = map.insert(key, &FlowStats { + bytes: event.packet_length as u64, + packets: 1, + last_seen: event.timestamp_us, + }, 0); } } diff --git a/egress-ebpf/src/main.rs b/egress-ebpf/src/main.rs index e7003be..f89fafa 100644 --- a/egress-ebpf/src/main.rs +++ b/egress-ebpf/src/main.rs @@ -32,15 +32,14 @@ pub fn mantis(ctx: XdpContext) -> u32 { } } +#[inline(always)] unsafe fn packet_intake(ctx: &XdpContext) -> Result<(), ()> { - unsafe { - let start = ctx.data(); - let end = ctx.data_end(); - let ptr = PARSED_PACKET.get_ptr_mut(0).ok_or(())?; - parsing::parse_packet(start, end, ptr)?; - let _ = PROGRAM_ARRAY.tail_call(ctx, ACCESS_CONTROL); - Ok(()) - } + let start = ctx.data(); + let end = ctx.data_end(); + let ptr = PARSED_PACKET.get_ptr_mut(0).ok_or(())?; + parsing::parse_packet(start, end, ptr)?; + let _ = PROGRAM_ARRAY.tail_call(ctx, ACCESS_CONTROL); + Ok(()) } #[xdp] @@ -58,32 +57,30 @@ pub fn access_control(ctx: XdpContext) -> u32 { #[inline(always)] unsafe fn try_access_control(ctx: &XdpContext) -> Result { - unsafe { - let ptr = PARSED_PACKET.get_ptr(0).ok_or(())?; - let parsed_packet = &*ptr; - match parsed_packet { - Event::IPv4(event) => { - if access_control::ipv4_is_whitelisted(event) { - let _ = PROGRAM_ARRAY.tail_call(ctx, STATISTICS); - return Err(()); - } - if access_control::ipv4_is_blacklisted(event) { - return Ok(xdp_action::XDP_DROP); - } + let ptr = PARSED_PACKET.get_ptr(0).ok_or(())?; + let parsed_packet = &*ptr; + match parsed_packet { + Event::IPv4(event) => { + if access_control::ipv4_is_whitelisted(event) { + let _ = PROGRAM_ARRAY.tail_call(ctx, STATISTICS); + return Err(()); } - Event::IPv6(event) => { - if access_control::ipv6_is_whitelisted(event) { - let _ = PROGRAM_ARRAY.tail_call(ctx, STATISTICS); - return Err(()); - } - if access_control::ipv6_is_blacklisted(event) { - return Ok(xdp_action::XDP_DROP); - } + if access_control::ipv4_is_blacklisted(event) { + return Ok(xdp_action::XDP_DROP); + } + } + Event::IPv6(event) => { + if access_control::ipv6_is_whitelisted(event) { + let _ = PROGRAM_ARRAY.tail_call(ctx, STATISTICS); + return Err(()); + } + if access_control::ipv6_is_blacklisted(event) { + return Ok(xdp_action::XDP_DROP); } } - let _ = PROGRAM_ARRAY.tail_call(ctx, STATISTICS); - Err(()) } + let _ = PROGRAM_ARRAY.tail_call(ctx, STATISTICS); + Err(()) } #[xdp] @@ -94,21 +91,20 @@ pub fn statistics(ctx: XdpContext) -> u32 { } } +#[inline(always)] unsafe fn try_statistics(ctx: &XdpContext) -> Result { - unsafe { - let ptr = PARSED_PACKET.get_ptr(0).ok_or(())?; - let parsed_packet = &*ptr; - match parsed_packet { - Event::IPv4(event) => { - statistics::ipv4_update_stats(event); - } - Event::IPv6(event) => { - statistics::ipv6_update_stats(event); - } + let ptr = PARSED_PACKET.get_ptr(0).ok_or(())?; + let parsed_packet = &*ptr; + match parsed_packet { + Event::IPv4(event) => { + statistics::ipv4_update_stats(event); + } + Event::IPv6(event) => { + statistics::ipv6_update_stats(event); } - let _ = PROGRAM_ARRAY.tail_call(ctx, TRANSMISSION); - Ok(xdp_action::XDP_PASS) } + let _ = PROGRAM_ARRAY.tail_call(ctx, TRANSMISSION); + Ok(xdp_action::XDP_PASS) } #[xdp] diff --git a/ingress-ebpf/src/action/access_control.rs b/ingress-ebpf/src/action/access_control.rs index c447edc..ab09b5f 100644 --- a/ingress-ebpf/src/action/access_control.rs +++ b/ingress-ebpf/src/action/access_control.rs @@ -87,12 +87,10 @@ pub fn ipv6_is_blacklisted(event: &IPv6Event) -> bool { #[inline(always)] fn is_port_exist(ports: &[Port; MAX_RULES_PORT], target_port: Port) -> bool { - if ports.get(0) == Some(&0) { - return true; - } for &port in ports.iter() { if port == 0 { - break; + // First entry 0 means "any port"; later 0 is end-of-list sentinel. + return ports[0] == 0; } if port == target_port { return true; diff --git a/ingress-ebpf/src/action/statistics.rs b/ingress-ebpf/src/action/statistics.rs index 9ac6f77..09ea24d 100644 --- a/ingress-ebpf/src/action/statistics.rs +++ b/ingress-ebpf/src/action/statistics.rs @@ -31,9 +31,9 @@ static IPV6_INGRESS_DST_10MIN: LruHashMap = LruHashMap::w static IPV6_INGRESS_DST_1HOUR: LruHashMap = LruHashMap::with_max_entries(MAX_STATS as u32, 0); pub fn ipv4_update_stats(event: &IPv4Event) { + let source = event.source_addr(); + let destination = event.destination_addr(); unsafe { - let source = event.source_addr(); - let destination = event.destination_addr(); ipv4_update_flow_stats(&IPV4_INGRESS_SRC_1MIN, &source, event); ipv4_update_flow_stats(&IPV4_INGRESS_SRC_10MIN, &source, event); ipv4_update_flow_stats(&IPV4_INGRESS_SRC_1HOUR, &source, event); @@ -44,9 +44,9 @@ pub fn ipv4_update_stats(event: &IPv4Event) { } pub fn ipv6_update_stats(event: &IPv6Event) { + let source = event.source_addr(); + let destination = event.destination_addr(); unsafe { - let source = event.source_addr(); - let destination = event.destination_addr(); ipv6_update_flow_status(&IPV6_INGRESS_SRC_1MIN, &source, event); ipv6_update_flow_status(&IPV6_INGRESS_SRC_10MIN, &source, event); ipv6_update_flow_status(&IPV6_INGRESS_SRC_1HOUR, &source, event); @@ -56,38 +56,37 @@ pub fn ipv6_update_stats(event: &IPv6Event) { } } +// LruHashMap is shared across CPUs so concurrent updates to the same key from +// different CPUs can race on the three field writes below. For traffic statistics +// this is acceptable: the map structure itself is safe, and minor count skew under +// high load is tolerable. A future migration to PerCpuLruHashMap would eliminate +// the race at the cost of per-CPU aggregation on the userspace read path. #[inline(always)] unsafe fn ipv4_update_flow_stats(map: &LruHashMap, key: &AddrPortV4, event: &IPv4Event) { - unsafe { - if let Some(status) = map.get_ptr_mut(key) { - (*status).bytes += event.packet_length as u64; - (*status).packets += 1; - (*status).last_seen = event.timestamp_us; - } else { - let new_stats = FlowStats { - bytes: event.packet_length as u64, - packets: 1, - last_seen: event.timestamp_us, - }; - let _ = map.insert(key, &new_stats, 0); - } + if let Some(status) = map.get_ptr_mut(key) { + (*status).bytes += event.packet_length as u64; + (*status).packets += 1; + (*status).last_seen = event.timestamp_us; + } else { + let _ = map.insert(key, &FlowStats { + bytes: event.packet_length as u64, + packets: 1, + last_seen: event.timestamp_us, + }, 0); } } #[inline(always)] unsafe fn ipv6_update_flow_status(map: &LruHashMap, key: &AddrPortV6, event: &IPv6Event) { - unsafe { - if let Some(status) = map.get_ptr_mut(key) { - (*status).bytes += event.packet_length as u64; - (*status).packets += 1; - (*status).last_seen = event.timestamp_us; - } else { - let new_stats = FlowStats { - bytes: event.packet_length as u64, - packets: 1, - last_seen: event.timestamp_us, - }; - let _ = map.insert(key, &new_stats, 0); - } + if let Some(status) = map.get_ptr_mut(key) { + (*status).bytes += event.packet_length as u64; + (*status).packets += 1; + (*status).last_seen = event.timestamp_us; + } else { + let _ = map.insert(key, &FlowStats { + bytes: event.packet_length as u64, + packets: 1, + last_seen: event.timestamp_us, + }, 0); } } diff --git a/ingress-ebpf/src/main.rs b/ingress-ebpf/src/main.rs index ec32e09..60e6901 100644 --- a/ingress-ebpf/src/main.rs +++ b/ingress-ebpf/src/main.rs @@ -36,14 +36,12 @@ pub fn mantis(ctx: XdpContext) -> u32 { #[inline(always)] unsafe fn packet_intake(ctx: &XdpContext) -> Result<(), ()> { - unsafe { - let start = ctx.data(); - let end = ctx.data_end(); - let ptr = PARSED_PACKET.get_ptr_mut(0).ok_or(())?; - parsing::parse_packet(start, end, ptr)?; - let _ = PROGRAM_ARRAY.tail_call(ctx, ACCESS_CONTROL); - Ok(()) - } + let start = ctx.data(); + let end = ctx.data_end(); + let ptr = PARSED_PACKET.get_ptr_mut(0).ok_or(())?; + parsing::parse_packet(start, end, ptr)?; + let _ = PROGRAM_ARRAY.tail_call(ctx, ACCESS_CONTROL); + Ok(()) } #[xdp] @@ -61,32 +59,30 @@ pub fn access_control(ctx: XdpContext) -> u32 { #[inline(always)] unsafe fn try_access_control(ctx: &XdpContext) -> Result { - unsafe { - let ptr = PARSED_PACKET.get_ptr(0).ok_or(())?; - let parsed_packet = &*ptr; - match parsed_packet { - Event::IPv4(event) => { - if access_control::ipv4_is_whitelisted(event) { - let _ = PROGRAM_ARRAY.tail_call(ctx, STATISTICS); - return Err(()); - } - if access_control::ipv4_is_blacklisted(event) { - return Ok(xdp_action::XDP_DROP); - } + let ptr = PARSED_PACKET.get_ptr(0).ok_or(())?; + let parsed_packet = &*ptr; + match parsed_packet { + Event::IPv4(event) => { + if access_control::ipv4_is_whitelisted(event) { + let _ = PROGRAM_ARRAY.tail_call(ctx, STATISTICS); + return Err(()); } - Event::IPv6(event) => { - if access_control::ipv6_is_whitelisted(event) { - let _ = PROGRAM_ARRAY.tail_call(ctx, STATISTICS); - return Err(()); - } - if access_control::ipv6_is_blacklisted(event) { - return Ok(xdp_action::XDP_DROP); - } + if access_control::ipv4_is_blacklisted(event) { + return Ok(xdp_action::XDP_DROP); + } + } + Event::IPv6(event) => { + if access_control::ipv6_is_whitelisted(event) { + let _ = PROGRAM_ARRAY.tail_call(ctx, STATISTICS); + return Err(()); + } + if access_control::ipv6_is_blacklisted(event) { + return Ok(xdp_action::XDP_DROP); } } - let _ = PROGRAM_ARRAY.tail_call(ctx, STATISTICS); - Err(()) } + let _ = PROGRAM_ARRAY.tail_call(ctx, STATISTICS); + Err(()) } #[xdp] @@ -99,20 +95,18 @@ pub fn statistics(ctx: XdpContext) -> u32 { #[inline(always)] unsafe fn try_statistics(ctx: &XdpContext) -> Result { - unsafe { - let ptr = PARSED_PACKET.get_ptr(0).ok_or(())?; - let parsed_packet = &*ptr; - match parsed_packet { - Event::IPv4(event) => { - statistics::ipv4_update_stats(&event); - } - Event::IPv6(event) => { - statistics::ipv6_update_stats(&event); - } + let ptr = PARSED_PACKET.get_ptr(0).ok_or(())?; + let parsed_packet = &*ptr; + match parsed_packet { + Event::IPv4(event) => { + statistics::ipv4_update_stats(&event); + } + Event::IPv6(event) => { + statistics::ipv6_update_stats(&event); } - let _ = PROGRAM_ARRAY.tail_call(ctx, TRANSMISSION); - Err(()) } + let _ = PROGRAM_ARRAY.tail_call(ctx, TRANSMISSION); + Err(()) } #[xdp]