From fa91fb4f59be3505664f8088b6e3e8da2c372253 Mon Sep 17 00:00:00 2001 From: Tamir Duberstein Date: Thu, 13 Jul 2023 10:16:59 -0400 Subject: [PATCH] Remove "async" feature This feature is equivalent to async_tokio || async_std; removing it avoids warnings emitted during `cargo hack check --feature-powerset` where async is selected without either of the other features. Use cargo hack to ensure clippy runs on the powerset of features. --- .github/workflows/build-aya.yml | 10 +-- .github/workflows/lint.yml | 5 +- aya-log/src/lib.rs | 10 ++- aya/Cargo.toml | 20 +++--- aya/src/maps/mod.rs | 8 +-- aya/src/maps/perf/async_perf_event_array.rs | 68 +++++++++------------ aya/src/maps/perf/mod.rs | 8 +-- aya/src/programs/links.rs | 8 +-- bpf/aya-bpf/src/helpers.rs | 1 - test/integration-test/Cargo.toml | 9 ++- 10 files changed, 62 insertions(+), 85 deletions(-) diff --git a/.github/workflows/build-aya.yml b/.github/workflows/build-aya.yml index 998043d9..01f8a909 100644 --- a/.github/workflows/build-aya.yml +++ b/.github/workflows/build-aya.yml @@ -30,19 +30,15 @@ jobs: - uses: dtolnay/rust-toolchain@master with: toolchain: stable - - - uses: taiki-e/install-action@cargo-hack - - name: Check - run: cargo hack check --all-targets --feature-powerset --ignore-private + targets: ${{ matrix.arch }} - uses: Swatinem/rust-cache@v2 - - name: Prereqs - run: cargo install cross --git https://github.com/cross-rs/cross + - uses: taiki-e/install-action@cross - name: Build run: cross build --verbose --target ${{ matrix.arch }} - - name: Run test + - name: Test env: RUST_BACKTRACE: full run: cross test --verbose --target ${{ matrix.arch }} diff --git a/.github/workflows/lint.yml b/.github/workflows/lint.yml index fae57e42..e53d9d19 100644 --- a/.github/workflows/lint.yml +++ b/.github/workflows/lint.yml @@ -25,11 +25,14 @@ jobs: toolchain: nightly components: rustfmt, clippy, miri, rust-src + - uses: Swatinem/rust-cache@v2 + - name: Check formatting run: cargo fmt --all -- --check + - uses: taiki-e/install-action@cargo-hack - name: Run clippy - run: cargo clippy --all-targets --workspace -- --deny warnings + run: cargo hack clippy --all-targets --feature-powerset --workspace -- --deny warnings - name: Run miri run: cargo miri test --all-targets diff --git a/aya-log/src/lib.rs b/aya-log/src/lib.rs index 055923d3..854a63a4 100644 --- a/aya-log/src/lib.rs +++ b/aya-log/src/lib.rs @@ -68,7 +68,7 @@ use thiserror::Error; use aya::{ maps::{ - perf::{AsyncPerfEventArray, PerfBufferError}, + perf::{AsyncPerfEventArray, Events, PerfBufferError}, MapError, }, util::online_cpus, @@ -121,12 +121,10 @@ impl BpfLogger { let mut buffers = vec![BytesMut::with_capacity(LOG_BUF_CAPACITY); 10]; loop { - let events = buf.read_events(&mut buffers).await.unwrap(); + let Events { read, lost: _ } = buf.read_events(&mut buffers).await.unwrap(); - #[allow(clippy::needless_range_loop)] - for i in 0..events.read { - let buf = &mut buffers[i]; - log_buf(buf, &*log).unwrap(); + for buf in buffers.iter().take(read) { + log_buf(buf.as_ref(), &*log).unwrap(); } } }); diff --git a/aya/Cargo.toml b/aya/Cargo.toml index e91da921..dd4392d9 100644 --- a/aya/Cargo.toml +++ b/aya/Cargo.toml @@ -2,7 +2,7 @@ name = "aya" version = "0.11.0" description = "An eBPF library with a focus on developer experience and operability." -keywords = ["ebpf", "bpf", "linux", "kernel"] +keywords = ["bpf", "ebpf", "kernel", "linux"] license = "MIT OR Apache-2.0" authors = ["The Aya Contributors"] repository = "https://github.com/aya-rs/aya" @@ -19,19 +19,14 @@ lazy_static = "1" libc = { version = "0.2.105" } log = "0.4" object = { version = "0.31", default-features = false, features = [ - "std", - "read_core", "elf", + "read_core", + "std", ] } parking_lot = { version = "0.12.0", features = ["send_guard"] } text_io = "0.1.12" thiserror = "1" -tokio = { version = "1.24.0", features = [ - "macros", - "rt", - "rt-multi-thread", - "net", -], optional = true } +tokio = { version = "1.24.0", features = ["rt"], optional = true } [dev-dependencies] futures = { version = "0.3.12", default-features = false, features = ["std"] } @@ -39,10 +34,9 @@ matches = "0.1.8" [features] default = [] -async = [] -async_tokio = ["tokio", "async"] -async_std = ["async-io", "async"] +async_tokio = ["tokio/net"] +async_std = ["async-io"] [package.metadata.docs.rs] all-features = true -rustdoc-args = ["--cfg", "docsrs", "-D", "warnings"] +rustdoc-args = ["--cfg", "-D", "docsrs", "warnings"] diff --git a/aya/src/maps/mod.rs b/aya/src/maps/mod.rs index dbb94e35..7a455861 100644 --- a/aya/src/maps/mod.rs +++ b/aya/src/maps/mod.rs @@ -77,8 +77,8 @@ pub use array::{Array, PerCpuArray, ProgramArray}; pub use bloom_filter::BloomFilter; pub use hash_map::{HashMap, PerCpuHashMap}; pub use lpm_trie::LpmTrie; -#[cfg(feature = "async")] -#[cfg_attr(docsrs, doc(cfg(feature = "async")))] +#[cfg(any(feature = "async_tokio", feature = "async_std"))] +#[cfg_attr(docsrs, doc(cfg(any(feature = "async_tokio", feature = "async_std"))))] pub use perf::AsyncPerfEventArray; pub use perf::PerfEventArray; pub use queue::Queue; @@ -349,8 +349,8 @@ impl_try_from_map!( StackTraceMap from Map::StackTraceMap, ); -#[cfg(feature = "async")] -#[cfg_attr(docsrs, doc(cfg(feature = "async")))] +#[cfg(any(feature = "async_tokio", feature = "async_std"))] +#[cfg_attr(docsrs, doc(cfg(any(feature = "async_tokio", feature = "async_std"))))] impl_try_from_map!( AsyncPerfEventArray from Map::PerfEventArray, ); diff --git a/aya/src/maps/perf/async_perf_event_array.rs b/aya/src/maps/perf/async_perf_event_array.rs index a42eab8d..4de4dd86 100644 --- a/aya/src/maps/perf/async_perf_event_array.rs +++ b/aya/src/maps/perf/async_perf_event_array.rs @@ -4,6 +4,10 @@ use std::{ os::fd::{AsRawFd, RawFd}, }; +// See https://doc.rust-lang.org/cargo/reference/features.html#mutually-exclusive-features. +// +// We should eventually split async functionality out into separate crates "aya-async-tokio" and +// "async-async-std". Presently we arbitrarily choose tokio over async-std when both are requested. #[cfg(all(not(feature = "async_tokio"), feature = "async_std"))] use async_io::Async; @@ -98,16 +102,17 @@ impl + Borrow> AsyncPerfEventArray { index: u32, page_count: Option, ) -> Result, PerfBufferError> { - let buf = self.perf_map.open(index, page_count)?; + let Self { perf_map } = self; + let buf = perf_map.open(index, page_count)?; let fd = buf.as_raw_fd(); Ok(AsyncPerfEventArrayBuffer { buf, #[cfg(feature = "async_tokio")] - async_fd: AsyncFd::new(fd)?, + async_tokio_fd: AsyncFd::new(fd)?, #[cfg(all(not(feature = "async_tokio"), feature = "async_std"))] - async_fd: Async::new(fd)?, + async_std_fd: Async::new(fd)?, }) } } @@ -131,13 +136,12 @@ pub struct AsyncPerfEventArrayBuffer { buf: PerfEventArrayBuffer, #[cfg(feature = "async_tokio")] - async_fd: AsyncFd, + async_tokio_fd: AsyncFd, #[cfg(all(not(feature = "async_tokio"), feature = "async_std"))] - async_fd: Async, + async_std_fd: Async, } -#[cfg(feature = "async_tokio")] impl + Borrow> AsyncPerfEventArrayBuffer { /// Reads events from the buffer. /// @@ -152,46 +156,30 @@ impl + Borrow> AsyncPerfEventArrayBuffer { &mut self, buffers: &mut [BytesMut], ) -> Result { + let Self { + buf, + #[cfg(feature = "async_tokio")] + async_tokio_fd, + #[cfg(all(not(feature = "async_tokio"), feature = "async_std"))] + async_std_fd, + } = self; loop { - let mut guard = self.async_fd.readable_mut().await?; + #[cfg(feature = "async_tokio")] + let mut guard = async_tokio_fd.readable_mut().await?; - match self.buf.read_events(buffers) { - Ok(events) if events.read > 0 || events.lost > 0 => return Ok(events), - Ok(_) => { - guard.clear_ready(); - continue; - } - Err(e) => return Err(e), + #[cfg(all(not(feature = "async_tokio"), feature = "async_std"))] + if !buf.readable() { + async_std_fd.readable().await?; } - } - } -} -#[cfg(all(not(feature = "async_tokio"), feature = "async_std"))] -impl + Borrow> AsyncPerfEventArrayBuffer { - /// Reads events from the buffer. - /// - /// This method reads events into the provided slice of buffers, filling - /// each buffer in order stopping when there are no more events to read or - /// all the buffers have been filled. - /// - /// Returns the number of events read and the number of events lost. Events - /// are lost when user space doesn't read events fast enough and the ring - /// buffer fills up. - pub async fn read_events( - &mut self, - buffers: &mut [BytesMut], - ) -> Result { - loop { - if !self.buf.readable() { - let _ = self.async_fd.readable().await?; + let events = buf.read_events(buffers)?; + const EMPTY: Events = Events { read: 0, lost: 0 }; + if events != EMPTY { + break Ok(events); } - match self.buf.read_events(buffers) { - Ok(events) if events.read > 0 || events.lost > 0 => return Ok(events), - Ok(_) => continue, - Err(e) => return Err(e), - } + #[cfg(feature = "async_tokio")] + guard.clear_ready(); } } } diff --git a/aya/src/maps/perf/mod.rs b/aya/src/maps/perf/mod.rs index 334ba4c6..80f08b61 100644 --- a/aya/src/maps/perf/mod.rs +++ b/aya/src/maps/perf/mod.rs @@ -1,14 +1,14 @@ //! Ring buffer types used to receive events from eBPF programs using the linux `perf` API. //! //! See the [`PerfEventArray`](crate::maps::PerfEventArray) and [`AsyncPerfEventArray`](crate::maps::perf::AsyncPerfEventArray). -#[cfg(feature = "async")] -#[cfg_attr(docsrs, doc(cfg(feature = "async")))] +#[cfg(any(feature = "async_tokio", feature = "async_std"))] +#[cfg_attr(docsrs, doc(cfg(any(feature = "async_tokio", feature = "async_std"))))] mod async_perf_event_array; mod perf_buffer; mod perf_event_array; -#[cfg(feature = "async")] -#[cfg_attr(docsrs, doc(cfg(feature = "async")))] +#[cfg(any(feature = "async_tokio", feature = "async_std"))] +#[cfg_attr(docsrs, doc(cfg(any(feature = "async_tokio", feature = "async_std"))))] pub use async_perf_event_array::*; pub use perf_buffer::*; pub use perf_event_array::*; diff --git a/aya/src/programs/links.rs b/aya/src/programs/links.rs index 7b3055f4..649d1f9b 100644 --- a/aya/src/programs/links.rs +++ b/aya/src/programs/links.rs @@ -17,10 +17,6 @@ use crate::{ sys::{bpf_get_object, bpf_pin_object, bpf_prog_detach}, }; -// for docs link -#[allow(unused)] -use crate::programs::cgroup_skb::CgroupSkb; - /// A Link. pub trait Link: std::fmt::Debug + 'static { /// Unique Id @@ -88,8 +84,8 @@ pub struct FdLinkId(pub(crate) RawFd); /// A file descriptor link. /// /// Fd links are returned directly when attaching some program types (for -/// instance [`CgroupSkb`]), or can be obtained by converting other link -/// types (see the `TryFrom` implementations). +/// instance [`crate::programs::cgroup_skb::CgroupSkb`]), or can be obtained by +/// converting other link types (see the `TryFrom` implementations). /// /// An important property of fd links is that they can be pinned. Pinning /// can be used keep a link attached "in background" even after the program diff --git a/bpf/aya-bpf/src/helpers.rs b/bpf/aya-bpf/src/helpers.rs index 5387c2d4..ca408601 100644 --- a/bpf/aya-bpf/src/helpers.rs +++ b/bpf/aya-bpf/src/helpers.rs @@ -585,7 +585,6 @@ pub unsafe fn bpf_probe_read_kernel_str_bytes( /// # Errors /// /// On failure, this function returns a negative value wrapped in an `Err`. -#[allow(clippy::fn_to_numeric_cast_with_truncation)] #[inline] pub unsafe fn bpf_probe_write_user(dst: *mut T, src: *const T) -> Result<(), c_long> { let ret = gen::bpf_probe_write_user( diff --git a/test/integration-test/Cargo.toml b/test/integration-test/Cargo.toml index b4d1dd10..8fb11bda 100644 --- a/test/integration-test/Cargo.toml +++ b/test/integration-test/Cargo.toml @@ -13,12 +13,15 @@ libc = { version = "0.2.105" } log = "0.4" matches = "0.1.8" object = { version = "0.31", default-features = false, features = [ - "std", - "read_core", "elf", + "read_core", + "std", ] } rbpf = "0.2.0" -tokio = { version = "1.24", default-features = false, features = ["time"] } +tokio = { version = "1.24", default-features = false, features = [ + "macros", + "time", +] } [build-dependencies] cargo_metadata = "0.15.4"