Core refactor of Map API

Build completing tests passing

Refactor the Map API to better align
with the aya programs API.  Specifically
remove all internal locking mechanisms
and custom Deref/DerefMut implementations.
They are replaced with a Map enum
and AsRef/AsMut implementations.

All Try_From implementations have been moved
to standardized enums, with a slightly
special one for PerfEventArray's.

Also cleanup/fix all associated tests and
documentation.

Signed-off-by: Andrew Stoycos <astoycos@redhat.com>

aya-log/src/lib.rs

@@ -90,7 +90,7 @@ impl BpfLogger {
         logger: T,
     ) -> Result<BpfLogger, Error> {
         let logger = Arc::new(logger);
-        let mut logs: AsyncPerfEventArray<_> = bpf.map_mut("AYA_LOGS")?.try_into()?;
+        let mut logs: AsyncPerfEventArray<_> = bpf.take_map("AYA_LOGS")?.try_into()?;
 
         for cpu_id in online_cpus().map_err(Error::InvalidOnlineCpu)? {
             let mut buf = logs.open(cpu_id, None)?;

aya/src/bpf.rs

@@ -13,10 +13,10 @@ use thiserror::Error;
 
 use crate::{
     generated::{
-        bpf_map_type::BPF_MAP_TYPE_PERF_EVENT_ARRAY, AYA_PERF_EVENT_IOC_DISABLE,
-        AYA_PERF_EVENT_IOC_ENABLE, AYA_PERF_EVENT_IOC_SET_BPF,
+        bpf_map_type::*, AYA_PERF_EVENT_IOC_DISABLE, AYA_PERF_EVENT_IOC_ENABLE,
+        AYA_PERF_EVENT_IOC_SET_BPF,
     },
-    maps::{Map, MapError, MapLock, MapRef, MapRefMut},
+    maps::{Map, MapData, MapError},
     obj::{
         btf::{Btf, BtfError},
         MapKind, Object, ParseError, ProgramSection,
@@ -451,7 +451,7 @@ impl<'a> BpfLoader<'a> {
                     }
                 }
             }
-            let mut map = Map {
+            let mut map = MapData {
                 obj,
                 fd: None,
                 pinned: false,
@@ -638,14 +638,41 @@ impl<'a> BpfLoader<'a> {
                 (name, program)
             })
             .collect();
-        let maps = maps
-            .drain()
-            .map(|(name, map)| (name, MapLock::new(map)))
-            .collect();
-        Ok(Bpf { maps, programs })
+        let maps: Result<HashMap<String, Map>, BpfError> = maps.drain().map(parse_map).collect();
+
+        Ok(Bpf {
+            maps: maps?,
+            programs,
+        })
     }
 }
 
+fn parse_map(data: (String, MapData)) -> Result<(String, Map), BpfError> {
+    let name = data.0;
+    let map = data.1;
+    let map_type = map.map_type().map_err(BpfError::MapError)?;
+    let map = match map_type {
+        BPF_MAP_TYPE_ARRAY => Ok(Map::Array(map)),
+        BPF_MAP_TYPE_PERCPU_ARRAY => Ok(Map::PerCpuArray(map)),
+        BPF_MAP_TYPE_PROG_ARRAY => Ok(Map::ProgramArray(map)),
+        BPF_MAP_TYPE_HASH => Ok(Map::HashMap(map)),
+        BPF_MAP_TYPE_PERCPU_HASH => Ok(Map::PerCpuHashMap(map)),
+        BPF_MAP_TYPE_PERF_EVENT_ARRAY => Ok(Map::PerfEventArray(map)),
+        BPF_MAP_TYPE_SOCKHASH => Ok(Map::SockHash(map)),
+        BPF_MAP_TYPE_SOCKMAP => Ok(Map::SockMap(map)),
+        BPF_MAP_TYPE_BLOOM_FILTER => Ok(Map::BloomFilter(map)),
+        BPF_MAP_TYPE_LPM_TRIE => Ok(Map::LpmTrie(map)),
+        BPF_MAP_TYPE_STACK => Ok(Map::Stack(map)),
+        BPF_MAP_TYPE_STACK_TRACE => Ok(Map::StackTraceMap(map)),
+        BPF_MAP_TYPE_QUEUE => Ok(Map::Queue(map)),
+        m => Err(BpfError::MapError(MapError::InvalidMapType {
+            map_type: m as u32,
+        })),
+    }?;
+
+    Ok((name, map))
+}
+
 impl<'a> Default for BpfLoader<'a> {
     fn default() -> Self {
         BpfLoader::new()
@@ -655,7 +682,7 @@ impl<'a> Default for BpfLoader<'a> {
 /// The main entry point into the library, used to work with eBPF programs and maps.
 #[derive(Debug)]
 pub struct Bpf {
-    maps: HashMap<String, MapLock>,
+    maps: HashMap<String, Map>,
     programs: HashMap<String, Program>,
 }
 
@@ -717,19 +744,11 @@ impl Bpf {
     ///
     /// # Errors
     ///
-    /// Returns [`MapError::MapNotFound`] if the map does not exist. If the map is already borrowed
-    /// mutably with [map_mut](Self::map_mut) then [`MapError::BorrowError`] is returned.
-    pub fn map(&self, name: &str) -> Result<MapRef, MapError> {
-        self.maps
-            .get(name)
-            .ok_or_else(|| MapError::MapNotFound {
-                name: name.to_owned(),
-            })
-            .and_then(|lock| {
-                lock.try_read().map_err(|_| MapError::BorrowError {
-                    name: name.to_owned(),
-                })
-            })
+    /// Returns [`MapError::MapNotFound`] if the map does not exist.
+    pub fn map(&self, name: &str) -> Result<&Map, MapError> {
+        self.maps.get(name).ok_or_else(|| MapError::MapNotFound {
+            name: name.to_owned(),
+        })
     }
 
     /// Returns a mutable reference to the map with the given name.
@@ -742,19 +761,32 @@ impl Bpf {
     ///
     /// # Errors
     ///
-    /// Returns [`MapError::MapNotFound`] if the map does not exist. If the map is already borrowed
-    /// mutably with [map_mut](Self::map_mut) then [`MapError::BorrowError`] is returned.
-    pub fn map_mut(&self, name: &str) -> Result<MapRefMut, MapError> {
+    /// Returns [`MapError::MapNotFound`] if the map does not exist.
+    pub fn map_mut(&mut self, name: &str) -> Result<&mut Map, MapError> {
         self.maps
-            .get(name)
+            .get_mut(name)
             .ok_or_else(|| MapError::MapNotFound {
                 name: name.to_owned(),
             })
-            .and_then(|lock| {
-                lock.try_write().map_err(|_| MapError::BorrowError {
-                    name: name.to_owned(),
-                })
-            })
+    }
+
+    /// Returns a map with the given name.
+    ///
+    /// WARNING: This transfers ownership of the map to the user.
+    ///
+    /// The returned type is mostly opaque. In order to do anything useful with it you need to
+    /// convert it to a [typed map](crate::maps).
+    ///
+    /// For more details and examples on maps and their usage, see the [maps module
+    /// documentation][crate::maps].
+    ///
+    /// # Errors
+    ///
+    /// Returns [`MapError::MapNotFound`] if the map does not exist.
+    pub fn take_map(&mut self, name: &str) -> Result<Map, MapError> {
+        self.maps.remove(name).ok_or_else(|| MapError::MapNotFound {
+            name: name.to_owned(),
+        })
     }
 
     /// An iterator over all the maps.
@@ -764,22 +796,14 @@ impl Bpf {
     /// # let mut bpf = aya::Bpf::load(&[])?;
     /// for (name, map) in bpf.maps() {
     ///     println!(
-    ///         "found map `{}` of type `{:?}`",
+    ///         "found map `{}`",
     ///         name,
-    ///         map?.map_type().unwrap()
     ///     );
     /// }
     /// # Ok::<(), aya::BpfError>(())
     /// ```
-    pub fn maps(&self) -> impl Iterator<Item = (&str, Result<MapRef, MapError>)> {
-        let ret = self.maps.iter().map(|(name, lock)| {
-            (
-                name.as_str(),
-                lock.try_read()
-                    .map_err(|_| MapError::BorrowError { name: name.clone() }),
-            )
-        });
-        ret
+    pub fn maps(&self) -> impl Iterator<Item = (&str, Result<&Map, MapError>)> {
+        self.maps.iter().map(|(name, map)| (name.as_str(), Ok(map)))
     }
 
     /// Returns a reference to the program with the given name.

aya/src/maps/array/array.rs

@@ -1,12 +1,11 @@
 use std::{
+    convert::{AsMut, AsRef},
     marker::PhantomData,
     mem,
-    ops::{Deref, DerefMut},
 };
 
 use crate::{
-    generated::bpf_map_type::BPF_MAP_TYPE_ARRAY,
-    maps::{IterableMap, Map, MapError, MapRef, MapRefMut},
+    maps::{array, IterableMap, MapData, MapError},
     sys::{bpf_map_lookup_elem, bpf_map_update_elem},
     Pod,
 };
@@ -22,38 +21,35 @@ use crate::{
 ///
 /// # Examples
 /// ```no_run
-/// # let bpf = aya::Bpf::load(&[])?;
+/// # let mut bpf = aya::Bpf::load(&[])?;
 /// use aya::maps::Array;
 ///
-/// let mut array = Array::try_from(bpf.map_mut("ARRAY")?)?;
+/// let mut array: Array<_, u32> = bpf.map_mut("ARRAY")?.try_into()?;
 /// array.set(1, 42, 0)?;
 /// assert_eq!(array.get(&1, 0)?, 42);
 /// # Ok::<(), aya::BpfError>(())
 /// ```
 #[doc(alias = "BPF_MAP_TYPE_ARRAY")]
-pub struct Array<T: Deref<Target = Map>, V: Pod> {
+pub struct Array<T, V: Pod> {
     inner: T,
     _v: PhantomData<V>,
 }
 
-impl<T: Deref<Target = Map>, V: Pod> Array<T, V> {
-    fn new(map: T) -> Result<Array<T, V>, MapError> {
-        let map_type = map.obj.map_type();
-        if map_type != BPF_MAP_TYPE_ARRAY as u32 {
-            return Err(MapError::InvalidMapType { map_type });
-        }
+impl<T: AsRef<MapData>, V: Pod> Array<T, V> {
+    pub(crate) fn new(map: T) -> Result<Array<T, V>, MapError> {
+        let data = map.as_ref();
         let expected = mem::size_of::<u32>();
-        let size = map.obj.key_size() as usize;
+        let size = data.obj.key_size() as usize;
         if size != expected {
             return Err(MapError::InvalidKeySize { size, expected });
         }
 
         let expected = mem::size_of::<V>();
-        let size = map.obj.value_size() as usize;
+        let size = data.obj.value_size() as usize;
         if size != expected {
             return Err(MapError::InvalidValueSize { size, expected });
         }
-        let _fd = map.fd_or_err()?;
+        let _fd = data.fd_or_err()?;
 
         Ok(Array {
             inner: map,
@@ -65,7 +61,7 @@ impl<T: Deref<Target = Map>, V: Pod> Array<T, V> {
     ///
     /// This corresponds to the value of `bpf_map_def::max_entries` on the eBPF side.
     pub fn len(&self) -> u32 {
-        self.inner.obj.max_entries()
+        self.inner.as_ref().obj.max_entries()
     }
 
     /// Returns the value stored at the given index.
@@ -75,8 +71,9 @@ impl<T: Deref<Target = Map>, V: Pod> Array<T, V> {
     /// Returns [`MapError::OutOfBounds`] if `index` is out of bounds, [`MapError::SyscallError`]
     /// if `bpf_map_lookup_elem` fails.
     pub fn get(&self, index: &u32, flags: u64) -> Result<V, MapError> {
-        self.check_bounds(*index)?;
-        let fd = self.inner.fd_or_err()?;
+        let data = self.inner.as_ref();
+        array::check_bounds(data, *index)?;
+        let fd = data.fd_or_err()?;
 
         let value = bpf_map_lookup_elem(fd, index, flags).map_err(|(_, io_error)| {
             MapError::SyscallError {
@@ -92,27 +89,19 @@ impl<T: Deref<Target = Map>, V: Pod> Array<T, V> {
     pub fn iter(&self) -> impl Iterator<Item = Result<V, MapError>> + '_ {
         (0..self.len()).map(move |i| self.get(&i, 0))
     }
-
-    fn check_bounds(&self, index: u32) -> Result<(), MapError> {
-        let max_entries = self.inner.obj.max_entries();
-        if index >= self.inner.obj.max_entries() {
-            Err(MapError::OutOfBounds { index, max_entries })
-        } else {
-            Ok(())
-        }
-    }
 }
 
-impl<T: Deref<Target = Map> + DerefMut<Target = Map>, V: Pod> Array<T, V> {
+impl<T: AsMut<MapData>, V: Pod> Array<T, V> {
     /// Sets the value of the element at the given index.
     ///
     /// # Errors
     ///
     /// Returns [`MapError::OutOfBounds`] if `index` is out of bounds, [`MapError::SyscallError`]
     /// if `bpf_map_update_elem` fails.
     pub fn set(&mut self, index: u32, value: V, flags: u64) -> Result<(), MapError> {
-        let fd = self.inner.fd_or_err()?;
-        self.check_bounds(index)?;
+        let data = self.inner.as_mut();
+        array::check_bounds(data, index)?;
+        let fd = data.fd_or_err()?;
         bpf_map_update_elem(fd, Some(&index), &value, flags).map_err(|(_, io_error)| {
             MapError::SyscallError {
                 call: "bpf_map_update_elem".to_owned(),
@@ -123,28 +112,12 @@ impl<T: Deref<Target = Map> + DerefMut<Target = Map>, V: Pod> Array<T, V> {
     }
 }
 
-impl<T: Deref<Target = Map>, V: Pod> IterableMap<u32, V> for Array<T, V> {
-    fn map(&self) -> &Map {
-        &self.inner
+impl<T: AsRef<MapData>, V: Pod> IterableMap<u32, V> for Array<T, V> {
+    fn map(&self) -> &MapData {
+        self.inner.as_ref()
     }
 
     fn get(&self, index: &u32) -> Result<V, MapError> {
         self.get(index, 0)
     }
 }
-
-impl<V: Pod> TryFrom<MapRef> for Array<MapRef, V> {
-    type Error = MapError;
-
-    fn try_from(a: MapRef) -> Result<Array<MapRef, V>, MapError> {
-        Array::new(a)
-    }
-}
-
-impl<V: Pod> TryFrom<MapRefMut> for Array<MapRefMut, V> {
-    type Error = MapError;
-
-    fn try_from(a: MapRefMut) -> Result<Array<MapRefMut, V>, MapError> {
-        Array::new(a)
-    }
-}

aya/src/maps/array/mod.rs

@@ -4,6 +4,17 @@ mod array;
 mod per_cpu_array;
 mod program_array;
 
-pub use array::Array;
+pub use array::*;
 pub use per_cpu_array::PerCpuArray;
 pub use program_array::ProgramArray;
+
+use crate::maps::{MapData, MapError};
+
+pub(crate) fn check_bounds(map: &MapData, index: u32) -> Result<(), MapError> {
+    let max_entries = map.obj.max_entries();
+    if index >= map.obj.max_entries() {
+        Err(MapError::OutOfBounds { index, max_entries })
+    } else {
+        Ok(())
+    }
+}