yanguangshaonian
/
aya
mirror of https://github.com/aya-rs/aya
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

aya-ebpf: Improve documentation of `maps::hash_map` module

Browse Source 
* Use the third person.
* Make use of `#[doc]` and split out the most repetetive chunk into a
  separate file.
* Make the `Safety` comment for `get*` operations more clear, provide
  context and inks.
reviewable/pr1367/r3
Michal R 1 month ago
parent 29dc775535
commit 70c77c5ea7
2 changed files with 64 additions and 56 deletions
Show Stats Download Patch File Download Diff File

88
ebpf/aya-ebpf/src/maps/hash_map.rs
Unescape Escape View File

@ -48,35 +48,28 @@ impl<K, V> HashMap<K, V> {
} }
} }
/// Retrieve the value associate with `key` from the map. #[doc = "Retrieves the value associated with `key` from the map."]
/// #[doc = include_str!("map_safety.md")]
/// # Safety
///
/// Unless the map flag `BPF_F_NO_PREALLOC` is used, the kernel does not guarantee the atomicity
/// of `insert` or `remove`, and any element removed from the map might get aliased by another
/// element in the map, causing garbage to be read, or corruption in case of writes.
#[inline] #[inline]
pub unsafe fn get(&self, key: impl Borrow<K>) -> Option<&V> { pub unsafe fn get(&self, key: impl Borrow<K>) -> Option<&V> {
unsafe { get(self.def.get(), key.borrow()) } unsafe { get(self.def.get(), key.borrow()) }
} }
/// Retrieve the value associate with `key` from the map. #[doc = "Retrieves the pointer associated with `key` from the map."]
/// The same caveat as `get` applies, but this returns a raw pointer and it's up to the caller #[doc = include_str!("map_safety.md")]
/// to decide whether it's safe to dereference the pointer or not.
#[inline] #[inline]
pub fn get_ptr(&self, key: impl Borrow<K>) -> Option<*const V> { pub fn get_ptr(&self, key: impl Borrow<K>) -> Option<*const V> {
get_ptr(self.def.get(), key.borrow()) get_ptr(self.def.get(), key.borrow())
} }
/// Retrieve the value associate with `key` from the map. #[doc = "Retrieves the mutable pointer associated with `key` from the map."]
/// The same caveat as `get` applies, and additionally cares should be taken to avoid #[doc = include_str!("map_safety.md")]
/// concurrent writes, but it's up to the caller to decide whether it's safe to dereference the
/// pointer or not.
#[inline] #[inline]
pub fn get_ptr_mut(&self, key: impl Borrow<K>) -> Option<*mut V> { pub fn get_ptr_mut(&self, key: impl Borrow<K>) -> Option<*mut V> {
get_ptr_mut(self.def.get(), key.borrow()) get_ptr_mut(self.def.get(), key.borrow())
} }
/// Inserts a key-value pair into the map.
#[inline] #[inline]
pub fn insert( pub fn insert(
&self, &self,
@ -87,6 +80,7 @@ impl<K, V> HashMap<K, V> {
insert(self.def.get().cast(), key.borrow(), value.borrow(), flags) insert(self.def.get().cast(), key.borrow(), value.borrow(), flags)
} }
/// Removes a key from the map.
#[inline] #[inline]
pub fn remove(&self, key: impl Borrow<K>) -> Result<(), c_long> { pub fn remove(&self, key: impl Borrow<K>) -> Result<(), c_long> {
remove(self.def.get().cast(), key.borrow()) remove(self.def.get().cast(), key.borrow())
@ -129,35 +123,28 @@ impl<K, V> LruHashMap<K, V> {
} }
} }
/// Retrieve the value associate with `key` from the map. #[doc = "Retrieves the value associated with `key` from the map."]
/// #[doc = include_str!("map_safety.md")]
/// # Safety
///
/// Unless the map flag `BPF_F_NO_PREALLOC` is used, the kernel does not guarantee the atomicity
/// of `insert` or `remove`, and any element removed from the map might get aliased by another
/// element in the map, causing garbage to be read, or corruption in case of writes.
#[inline] #[inline]
pub unsafe fn get(&self, key: impl Borrow<K>) -> Option<&V> { pub unsafe fn get(&self, key: impl Borrow<K>) -> Option<&V> {
unsafe { get(self.def.get(), key.borrow()) } unsafe { get(self.def.get(), key.borrow()) }
} }
/// Retrieve the value associate with `key` from the map. #[doc = "Retrieves the pointer associated with `key` from the map."]
/// The same caveat as `get` applies, but this returns a raw pointer and it's up to the caller #[doc = include_str!("map_safety.md")]
/// to decide whether it's safe to dereference the pointer or not.
#[inline] #[inline]
pub fn get_ptr(&self, key: impl Borrow<K>) -> Option<*const V> { pub fn get_ptr(&self, key: impl Borrow<K>) -> Option<*const V> {
get_ptr(self.def.get(), key.borrow()) get_ptr(self.def.get(), key.borrow())
} }
/// Retrieve the value associate with `key` from the map. #[doc = "Retrieves the mutable pointer associated with `key` from the map."]
/// The same caveat as `get` applies, and additionally cares should be taken to avoid #[doc = include_str!("map_safety.md")]
/// concurrent writes, but it's up to the caller to decide whether it's safe to dereference the
/// pointer or not.
#[inline] #[inline]
pub fn get_ptr_mut(&self, key: impl Borrow<K>) -> Option<*mut V> { pub fn get_ptr_mut(&self, key: impl Borrow<K>) -> Option<*mut V> {
get_ptr_mut(self.def.get(), key.borrow()) get_ptr_mut(self.def.get(), key.borrow())
} }
/// Inserts a key-value pair into the map.
#[inline] #[inline]
pub fn insert( pub fn insert(
&self, &self,
@ -168,6 +155,7 @@ impl<K, V> LruHashMap<K, V> {
insert(self.def.get().cast(), key.borrow(), value.borrow(), flags) insert(self.def.get().cast(), key.borrow(), value.borrow(), flags)
} }
/// Removes a key from the map.
#[inline] #[inline]
pub fn remove(&self, key: impl Borrow<K>) -> Result<(), c_long> { pub fn remove(&self, key: impl Borrow<K>) -> Result<(), c_long> {
remove(self.def.get().cast(), key.borrow()) remove(self.def.get().cast(), key.borrow())
@ -210,35 +198,28 @@ impl<K, V> PerCpuHashMap<K, V> {
} }
} }
/// Retrieve the value associate with `key` from the map. #[doc = "Retrieves the value associated with `key` from the map."]
/// #[doc = include_str!("map_safety.md")]
/// # Safety
///
/// Unless the map flag `BPF_F_NO_PREALLOC` is used, the kernel does not guarantee the atomicity
/// of `insert` or `remove`, and any element removed from the map might get aliased by another
/// element in the map, causing garbage to be read, or corruption in case of writes.
#[inline] #[inline]
pub unsafe fn get(&self, key: impl Borrow<K>) -> Option<&V> { pub unsafe fn get(&self, key: impl Borrow<K>) -> Option<&V> {
unsafe { get(self.def.get(), key.borrow()) } unsafe { get(self.def.get(), key.borrow()) }
} }
/// Retrieve the value associate with `key` from the map. #[doc = "Retrieves the pointer associated with `key` from the map."]
/// The same caveat as `get` applies, but this returns a raw pointer and it's up to the caller #[doc = include_str!("map_safety.md")]
/// to decide whether it's safe to dereference the pointer or not.
#[inline] #[inline]
pub fn get_ptr(&self, key: impl Borrow<K>) -> Option<*const V> { pub fn get_ptr(&self, key: impl Borrow<K>) -> Option<*const V> {
get_ptr(self.def.get(), key.borrow()) get_ptr(self.def.get(), key.borrow())
} }
/// Retrieve the value associate with `key` from the map. #[doc = "Retrieves the mutable pointer associated with `key` from the map."]
/// The same caveat as `get` applies, and additionally cares should be taken to avoid #[doc = include_str!("map_safety.md")]
/// concurrent writes, but it's up to the caller to decide whether it's safe to dereference the
/// pointer or not.
#[inline] #[inline]
pub fn get_ptr_mut(&self, key: impl Borrow<K>) -> Option<*mut V> { pub fn get_ptr_mut(&self, key: impl Borrow<K>) -> Option<*mut V> {
get_ptr_mut(self.def.get(), key.borrow()) get_ptr_mut(self.def.get(), key.borrow())
} }
/// Inserts a key-value pair into the map.
#[inline] #[inline]
pub fn insert( pub fn insert(
&self, &self,
@ -249,6 +230,7 @@ impl<K, V> PerCpuHashMap<K, V> {
insert(self.def.get().cast(), key.borrow(), value.borrow(), flags) insert(self.def.get().cast(), key.borrow(), value.borrow(), flags)
} }
/// Removes a key from the map.
#[inline] #[inline]
pub fn remove(&self, key: impl Borrow<K>) -> Result<(), c_long> { pub fn remove(&self, key: impl Borrow<K>) -> Result<(), c_long> {
remove(self.def.get().cast(), key.borrow()) remove(self.def.get().cast(), key.borrow())
@ -291,35 +273,28 @@ impl<K, V> LruPerCpuHashMap<K, V> {
} }
} }
/// Retrieve the value associate with `key` from the map. #[doc = "Retrieves the value associated with `key` from the map."]
/// #[doc = include_str!("map_safety.md")]
/// # Safety
///
/// Unless the map flag `BPF_F_NO_PREALLOC` is used, the kernel does not guarantee the atomicity
/// of `insert` or `remove`, and any element removed from the map might get aliased by another
/// element in the map, causing garbage to be read, or corruption in case of writes.
#[inline] #[inline]
pub unsafe fn get(&self, key: impl Borrow<K>) -> Option<&V> { pub unsafe fn get(&self, key: impl Borrow<K>) -> Option<&V> {
unsafe { get(self.def.get(), key.borrow()) } unsafe { get(self.def.get(), key.borrow()) }
} }
/// Retrieve the value associate with `key` from the map. #[doc = "Retrieves the pointer associated with `key` from the map."]
/// The same caveat as `get` applies, but this returns a raw pointer and it's up to the caller #[doc = include_str!("map_safety.md")]
/// to decide whether it's safe to dereference the pointer or not.
#[inline] #[inline]
pub fn get_ptr(&self, key: impl Borrow<K>) -> Option<*const V> { pub fn get_ptr(&self, key: impl Borrow<K>) -> Option<*const V> {
get_ptr(self.def.get(), key.borrow()) get_ptr(self.def.get(), key.borrow())
} }
/// Retrieve the value associate with `key` from the map. #[doc = "Retrieves the mutable pointer associated with `key` from the map."]
/// The same caveat as `get` applies, and additionally cares should be taken to avoid #[doc = include_str!("map_safety.md")]
/// concurrent writes, but it's up to the caller to decide whether it's safe to dereference the
/// pointer or not.
#[inline] #[inline]
pub fn get_ptr_mut(&self, key: impl Borrow<K>) -> Option<*mut V> { pub fn get_ptr_mut(&self, key: impl Borrow<K>) -> Option<*mut V> {
get_ptr_mut(self.def.get(), key.borrow()) get_ptr_mut(self.def.get(), key.borrow())
} }
/// Inserts a key-value pair into the map.
#[inline] #[inline]
pub fn insert( pub fn insert(
&self, &self,
@ -330,6 +305,7 @@ impl<K, V> LruPerCpuHashMap<K, V> {
insert(self.def.get().cast(), key.borrow(), value.borrow(), flags) insert(self.def.get().cast(), key.borrow(), value.borrow(), flags)
} }
/// Removes a key from the map.
#[inline] #[inline]
pub fn remove(&self, key: impl Borrow<K>) -> Result<(), c_long> { pub fn remove(&self, key: impl Borrow<K>) -> Result<(), c_long> {
remove(self.def.get().cast(), key.borrow()) remove(self.def.get().cast(), key.borrow())

32
ebpf/aya-ebpf/src/maps/map_safety.md
Unescape Escape View File

@ -0,0 +1,32 @@
# Safety
The pointer returned by a BPF map lookup is only stable until a concurrent
update or delete. In the kernel’s default *preallocated* mode (no
`BPF_F_NO_PREALLOC`), deleted elements are immediately recycled onto a
per-CPU freelist and may be reused by another update before an RCU grace
period elapses. Readers can therefore observe aliasing (values changing
underneath them) or, in rare cases, false-positive lookups when an old and
new key overlap. This behavior was reported on [LKML in 2018][lkml-2018].
Using `BPF_F_NO_PREALLOC` historically forced RCU-delayed freeing, but since
the switch to `bpf_mem_alloc`, both prealloc and no-prealloc modes may
recycle elements quickly; the main distinction now is
[memory vs. allocation overhead][htab-atomic-overwrite].
The [official kernel docs][kernel-doc-map-hash] describe `BPF_F_NO_PREALLOC`
as a *memory-usage knob*, not a safety guarantee.
Patches in 2020 mitigated some issues (e.g.
[zero-filling reused per-CPU slots][zero-filling]) but did not eliminate reuse
races.
A 2023 patch by Alexei proposed a fallback scheme to
[delay reuse via RCU grace periods in certain conditions][reuse-delay] (rather
than always reusing immediately). However, this approach is not universally
applied, and immediate reuse is still considered a “known quirk” in many cases.
[lkml-2018]: https://lore.kernel.org/lkml/CAG48ez1-WZH55+Wa2vgwZY_hpZJfnDxMzxGLtuN1hG1z6hKf5Q@mail.gmail.com/T/
[htab-atomic-overwrite]: https://lore.kernel.org/bpf/20250204082848.13471-2-hotforest@gmail.com/T/
[kernel-doc-map-hash]: https://www.kernel.org/doc/html/v6.10/bpf/map_hash.html
[zero-filling]: https://lore.kernel.org/all/20201104112332.15191-1-david.verbeiren@tessares.net/
[reuse-delay]: https://lore.kernel.org/bpf/20230706033447.54696-13-alexei.starovoitov@gmail.com/
Write Preview
Loading…
Cancel
Save