From ad3087d7eb4f235c161dead521431ed1a5a6b500 Mon Sep 17 00:00:00 2001 From: Tuetuopay Date: Sat, 5 Aug 2023 00:15:41 +0200 Subject: [PATCH] bpf: Update XDP maps implementation The implementation changed since the original commit was written, and some mistakes went in: - missing bpf_redirect_map wrapper - extra bpf_map_lookup_elem on maps for which it is forbidden --- bpf/aya-bpf/src/maps/xdp/cpu_map.rs | 33 +++++++++---------- bpf/aya-bpf/src/maps/xdp/dev_map.rs | 36 +++++++++++++------- bpf/aya-bpf/src/maps/xdp/dev_map_hash.rs | 41 ++++++++++++++--------- bpf/aya-bpf/src/maps/xdp/xsk_map.rs | 42 +++++++++++++++--------- 4 files changed, 93 insertions(+), 59 deletions(-) diff --git a/bpf/aya-bpf/src/maps/xdp/cpu_map.rs b/bpf/aya-bpf/src/maps/xdp/cpu_map.rs index bc16c271..3cf970e5 100644 --- a/bpf/aya-bpf/src/maps/xdp/cpu_map.rs +++ b/bpf/aya-bpf/src/maps/xdp/cpu_map.rs @@ -1,22 +1,22 @@ -use core::{mem, ptr::NonNull}; - -use aya_bpf_cty::c_void; +use core::{cell::UnsafeCell, mem}; use crate::{ bindings::{bpf_map_def, bpf_map_type::BPF_MAP_TYPE_CPUMAP}, - helpers::bpf_map_lookup_elem, + helpers::bpf_redirect_map, maps::PinningType, }; #[repr(transparent)] pub struct CpuMap { - def: bpf_map_def, + def: UnsafeCell, } +unsafe impl Sync for CpuMap {} + impl CpuMap { pub const fn with_max_entries(max_entries: u32, flags: u32) -> CpuMap { CpuMap { - def: bpf_map_def { + def: UnsafeCell::new(bpf_map_def { type_: BPF_MAP_TYPE_CPUMAP, key_size: mem::size_of::() as u32, value_size: mem::size_of::() as u32, @@ -24,32 +24,31 @@ impl CpuMap { map_flags: flags, id: 0, pinning: PinningType::None as u32, - }, + }), } } pub const fn pinned(max_entries: u32, flags: u32) -> CpuMap { CpuMap { - def: bpf_map_def { + def: UnsafeCell::new(bpf_map_def { type_: BPF_MAP_TYPE_CPUMAP, key_size: mem::size_of::() as u32, value_size: mem::size_of::() as u32, max_entries, map_flags: flags, id: 0, - pinning: PinningType::None as u32, - }, + pinning: PinningType::ByName as u32, + }), } } - pub fn get(&mut self, index: u32) -> Option<&u32> { + #[inline(always)] + pub fn redirect(&self, index: u32, flags: u64) -> u32 { unsafe { - let value = bpf_map_lookup_elem( - &mut self.def as *mut _ as *mut _, - &index as *const _ as *const c_void, - ); - // FIXME: alignment - NonNull::new(value as *mut u32).map(|p| p.as_ref()) + // Return XDP_REDIRECT on success, or the value of the two lower bits of the flags + // argument on error. Thus I have no idea why it returns a long (i64) instead of + // something saner, hence the unsigned_abs. + bpf_redirect_map(self.def.get() as *mut _, index.into(), flags).unsigned_abs() as u32 } } } diff --git a/bpf/aya-bpf/src/maps/xdp/dev_map.rs b/bpf/aya-bpf/src/maps/xdp/dev_map.rs index 8a9415ec..968deb69 100644 --- a/bpf/aya-bpf/src/maps/xdp/dev_map.rs +++ b/bpf/aya-bpf/src/maps/xdp/dev_map.rs @@ -1,22 +1,24 @@ -use core::{mem, ptr::NonNull}; +use core::{cell::UnsafeCell, mem, ptr::NonNull}; use aya_bpf_cty::c_void; use crate::{ bindings::{bpf_map_def, bpf_map_type::BPF_MAP_TYPE_DEVMAP}, - helpers::bpf_map_lookup_elem, + helpers::{bpf_map_lookup_elem, bpf_redirect_map}, maps::PinningType, }; #[repr(transparent)] pub struct DevMap { - def: bpf_map_def, + def: UnsafeCell, } +unsafe impl Sync for DevMap {} + impl DevMap { pub const fn with_max_entries(max_entries: u32, flags: u32) -> DevMap { DevMap { - def: bpf_map_def { + def: UnsafeCell::new(bpf_map_def { type_: BPF_MAP_TYPE_DEVMAP, key_size: mem::size_of::() as u32, value_size: mem::size_of::() as u32, @@ -24,32 +26,42 @@ impl DevMap { map_flags: flags, id: 0, pinning: PinningType::None as u32, - }, + }), } } pub const fn pinned(max_entries: u32, flags: u32) -> DevMap { DevMap { - def: bpf_map_def { + def: UnsafeCell::new(bpf_map_def { type_: BPF_MAP_TYPE_DEVMAP, key_size: mem::size_of::() as u32, value_size: mem::size_of::() as u32, max_entries, map_flags: flags, id: 0, - pinning: PinningType::None as u32, - }, + pinning: PinningType::ByName as u32, + }), } } - pub fn get(&mut self, index: u32) -> Option<&u32> { + #[inline(always)] + pub fn get(&self, index: u32) -> Option { unsafe { let value = bpf_map_lookup_elem( - &mut self.def as *mut _ as *mut _, + self.def.get() as *mut _, &index as *const _ as *const c_void, ); - // FIXME: alignment - NonNull::new(value as *mut u32).map(|p| p.as_ref()) + NonNull::new(value as *mut u32).map(|p| *p.as_ref()) + } + } + + #[inline(always)] + pub fn redirect(&self, index: u32, flags: u64) -> u32 { + // Return XDP_REDIRECT on success, or the value of the two lower bits of the flags + // argument on error. Thus I have no idea why it returns a long (i64) instead of + // something saner, hence the unsigned_abs. + unsafe { + bpf_redirect_map(self.def.get() as *mut _, index.into(), flags).unsigned_abs() as u32 } } } diff --git a/bpf/aya-bpf/src/maps/xdp/dev_map_hash.rs b/bpf/aya-bpf/src/maps/xdp/dev_map_hash.rs index 6a0b1903..4933146b 100644 --- a/bpf/aya-bpf/src/maps/xdp/dev_map_hash.rs +++ b/bpf/aya-bpf/src/maps/xdp/dev_map_hash.rs @@ -1,22 +1,25 @@ -use core::{mem, ptr::NonNull}; +use core::{cell::UnsafeCell, mem, ptr::NonNull}; +use aya_bpf_bindings::bindings::bpf_devmap_val; use aya_bpf_cty::c_void; use crate::{ bindings::{bpf_map_def, bpf_map_type::BPF_MAP_TYPE_DEVMAP_HASH}, - helpers::bpf_map_lookup_elem, + helpers::{bpf_map_lookup_elem, bpf_redirect_map}, maps::PinningType, }; #[repr(transparent)] pub struct DevMapHash { - def: bpf_map_def, + def: UnsafeCell, } +unsafe impl Sync for DevMapHash {} + impl DevMapHash { pub const fn with_max_entries(max_entries: u32, flags: u32) -> DevMapHash { DevMapHash { - def: bpf_map_def { + def: UnsafeCell::new(bpf_map_def { type_: BPF_MAP_TYPE_DEVMAP_HASH, key_size: mem::size_of::() as u32, value_size: mem::size_of::() as u32, @@ -24,32 +27,40 @@ impl DevMapHash { map_flags: flags, id: 0, pinning: PinningType::None as u32, - }, + }), } } pub const fn pinned(max_entries: u32, flags: u32) -> DevMapHash { DevMapHash { - def: bpf_map_def { + def: UnsafeCell::new(bpf_map_def { type_: BPF_MAP_TYPE_DEVMAP_HASH, key_size: mem::size_of::() as u32, value_size: mem::size_of::() as u32, max_entries, map_flags: flags, id: 0, - pinning: PinningType::None as u32, - }, + pinning: PinningType::ByName as u32, + }), } } - pub fn get(&mut self, index: u32) -> Option<&u32> { + #[inline(always)] + pub unsafe fn get(&self, index: u32) -> Option<&bpf_devmap_val> { + let value = bpf_map_lookup_elem( + self.def.get() as *mut _, + &index as *const _ as *const c_void, + ); + NonNull::new(value as *mut bpf_devmap_val).map(|p| p.as_ref()) + } + + #[inline(always)] + pub fn redirect(&self, index: u32, flags: u64) -> u32 { unsafe { - let value = bpf_map_lookup_elem( - &mut self.def as *mut _ as *mut _, - &index as *const _ as *const c_void, - ); - // FIXME: alignment - NonNull::new(value as *mut u32).map(|p| p.as_ref()) + // Return XDP_REDIRECT on success, or the value of the two lower bits of the flags + // argument on error. Thus I have no idea why it returns a long (i64) instead of + // something saner, hence the unsigned_abs. + bpf_redirect_map(self.def.get() as *mut _, index.into(), flags).unsigned_abs() as u32 } } } diff --git a/bpf/aya-bpf/src/maps/xdp/xsk_map.rs b/bpf/aya-bpf/src/maps/xdp/xsk_map.rs index 39fba557..362f258e 100644 --- a/bpf/aya-bpf/src/maps/xdp/xsk_map.rs +++ b/bpf/aya-bpf/src/maps/xdp/xsk_map.rs @@ -1,22 +1,25 @@ -use core::{mem, ptr::NonNull}; +use core::{cell::UnsafeCell, mem, ptr::NonNull}; +use aya_bpf_bindings::bindings::bpf_xdp_sock; use aya_bpf_cty::c_void; use crate::{ bindings::{bpf_map_def, bpf_map_type::BPF_MAP_TYPE_XSKMAP}, - helpers::bpf_map_lookup_elem, + helpers::{bpf_map_lookup_elem, bpf_redirect_map}, maps::PinningType, }; #[repr(transparent)] pub struct XskMap { - def: bpf_map_def, + def: UnsafeCell, } +unsafe impl Sync for XskMap {} + impl XskMap { pub const fn with_max_entries(max_entries: u32, flags: u32) -> XskMap { XskMap { - def: bpf_map_def { + def: UnsafeCell::new(bpf_map_def { type_: BPF_MAP_TYPE_XSKMAP, key_size: mem::size_of::() as u32, value_size: mem::size_of::() as u32, @@ -24,32 +27,41 @@ impl XskMap { map_flags: flags, id: 0, pinning: PinningType::None as u32, - }, + }), } } pub const fn pinned(max_entries: u32, flags: u32) -> XskMap { XskMap { - def: bpf_map_def { + def: UnsafeCell::new(bpf_map_def { type_: BPF_MAP_TYPE_XSKMAP, key_size: mem::size_of::() as u32, value_size: mem::size_of::() as u32, max_entries, map_flags: flags, id: 0, - pinning: PinningType::None as u32, - }, + pinning: PinningType::ByName as u32, + }), } } - pub fn get(&mut self, index: u32) -> Option<&u32> { + #[inline(always)] + pub unsafe fn get(&self, index: u32) -> Option<&bpf_xdp_sock> { + let value = bpf_map_lookup_elem( + self.def.get() as *mut _, + &index as *const _ as *const c_void, + ); + // FIXME: alignment + NonNull::new(value as *mut bpf_xdp_sock).map(|p| p.as_ref()) + } + + #[inline(always)] + pub fn redirect(&self, index: u32, flags: u64) -> u32 { unsafe { - let value = bpf_map_lookup_elem( - &mut self.def as *mut _ as *mut _, - &index as *const _ as *const c_void, - ); - // FIXME: alignment - NonNull::new(value as *mut u32).map(|p| p.as_ref()) + // Return XDP_REDIRECT on success, or the value of the two lower bits of the flags + // argument on error. Thus I have no idea why it returns a long (i64) instead of + // something saner, hence the unsigned_abs. + bpf_redirect_map(self.def.get() as *mut _, index.into(), flags).unsigned_abs() as u32 } } }