feat(sys): expose test-utils feature for mocking BPF syscalls#1565
feat(sys): expose test-utils feature for mocking BPF syscalls#1565OliverGavin wants to merge 1 commit into
Conversation
✅ Deploy Preview for aya-rs-docs ready!Built without sensitive environment variables
To edit notification comments on pull requests, go to your Netlify project configuration. |
vadorovsky
left a comment
There was a problem hiding this comment.
Thanks! Overall I agree with the direction and this looks like a good first step towards making different pieces of Aya mockable.
ed8c3e9 to
0dd36f2
Compare
tamird
left a comment
There was a problem hiding this comment.
@tamird reviewed 5 files and all commit messages, and made 6 comments.
Reviewable status: all files reviewed, 10 unresolved discussions (waiting on Brskt and OliverGavin).
-- commits line 9 at r2:
nit: can just be "Closes #999."
aya/src/sys/fake.rs line 7 at r2 (raw file):
type SyscallFn = unsafe fn(Syscall<'_>) -> SysResult; #[cfg(test)]
in other places this changed to
#[cfg(any(test, feature = "test-utils"))]
why not in this file?
aya/src/sys/mod.rs line 9 at r2 (raw file):
#[cfg(any(test, feature = "test-utils"))] pub(crate) mod fake;
why pub(crate) here?
aya/src/sys/mod.rs line 19 at r2 (raw file):
use aya_obj::generated::{bpf_attr, bpf_cmd, bpf_stats_type, perf_event_attr}; pub(crate) use bpf::*; #[cfg(any(test, feature = "test-utils"))]
i don't think you need the pub(crate) use fake::* under test-utils? i think it doesn't change the crate's API, so I don't see why you would
aya/src/sys/mod.rs line 50 at r2 (raw file):
#[cfg(feature = "test-utils")] #[cfg_attr(docsrs, doc(cfg(feature = "test-utils")))] pub mod test_utils {
could we rename fake to test_utils so that the decision to make certain testing utils externally visible lives on each item rather than having this parallel export layer?
aya/src/sys/mod.rs line 59 at r2 (raw file):
/// syscalls that create FDs (e.g. `BPF_MAP_CREATE`). pub const fn mock_fd() -> i64 { crate::MockableFd::mock_signed_fd() as i64
why i64 rather than the underlying i32?
Add a 'test-utils' feature that exposes override_syscall and the Syscall enum publicly via sys::test_utils module. This allows downstream crates to intercept BPF syscalls in their tests without needing to reimplement Aya's internal types. Closes aya-rs#999.
0dd36f2 to
f12521d
Compare
|
Thanks for the review @tamird! I've addressed all your feedback in the latest push:
Done.
The entire file is now gated by the
Great suggestion — done. I've renamed
Agreed. I've split it:
Good point — changed |
tamird
left a comment
There was a problem hiding this comment.
@codex review
@tamird reviewed 4 files and all commit messages, made 1 comment, and resolved 6 discussions.
Reviewable status: all files reviewed, 4 unresolved discussions (waiting on Brskt and OliverGavin).
|
Codex Review: Didn't find any major issues. Another round soon, please! ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
There was a problem hiding this comment.
Pull request overview
This PR introduces a new test-utils feature intended to let downstream crates mock/intercept Aya’s BPF-related syscalls by exposing an opt-in aya::sys::test_utils module and making some internal syscall types public.
Changes:
- Adds a
test-utilsfeature to theayacrate and a newsys::test_utilsmodule that re-exports syscall mocking helpers/types. - Moves syscall mocking infrastructure from the old test-only
sys/fake.rsintosys/test_utils.rsand wires syscall/mmap/munmap to use it undercfg(any(test, feature = "test-utils")). - Promotes
Syscall,PerfEventIoctlRequest, andSysResultvisibility to public (affecting the public API surface).
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
aya/Cargo.toml |
Adds the test-utils feature definition. |
aya/src/sys/mod.rs |
Exposes sys::test_utils and routes syscalls/mmap/munmap through test hooks under test-utils. |
aya/src/sys/test_utils.rs |
New public module providing override_syscall, mock_fd, and re-exports for mocking. |
aya/src/lib.rs |
Extends MockableFd test-only behavior to also apply when test-utils is enabled. |
aya/src/sys/fake.rs |
Removes the old test-only fake syscall module. |
xtask/public-api/aya.txt |
Updates the recorded public API surface to include the newly exposed items. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| pub use super::{PerfEventIoctlRequest, SysResult, Syscall}; | ||
|
|
||
| type SyscallFn = unsafe fn(Syscall<'_>) -> SysResult; | ||
|
|
||
| thread_local! { | ||
| pub(crate) static TEST_SYSCALL: RefCell<SyscallFn> = RefCell::new(test_syscall); | ||
| pub(crate) static TEST_MMAP_RET: RefCell<*mut c_void> = const { RefCell::new(ptr::null_mut()) }; | ||
| } | ||
|
|
||
| unsafe fn test_syscall(_call: Syscall<'_>) -> SysResult { | ||
| Err((-1, io::Error::from_raw_os_error(libc::EINVAL))) | ||
| } | ||
|
|
||
| /// Overrides the syscall implementation for testing purposes. | ||
| /// | ||
| /// This function replaces the BPF syscall with a user-provided function, | ||
| /// allowing tests to intercept and mock kernel interactions. | ||
| /// | ||
| /// # Example | ||
| /// | ||
| /// ```ignore | ||
| /// use aya::sys::test_utils::{override_syscall, Syscall}; | ||
| /// | ||
| /// override_syscall(|call| match call { | ||
| /// Syscall::Ebpf { cmd, attr } => Ok(0), | ||
| /// _ => Ok(0), | ||
| /// }); | ||
| /// ``` | ||
| pub fn override_syscall(call: unsafe fn(Syscall<'_>) -> SysResult) { | ||
| TEST_SYSCALL.with(|test_impl| *test_impl.borrow_mut() = call); | ||
| } |
| fn syscall(call: Syscall<'_>) -> SysResult { | ||
| #[cfg(test)] | ||
| #[cfg(any(test, feature = "test-utils"))] | ||
| { | ||
| TEST_SYSCALL.with(|test_impl| unsafe { test_impl.borrow()(call) }) | ||
| } | ||
|
|
||
| #[cfg(not(test))] | ||
| #[cfg(not(any(test, feature = "test-utils")))] | ||
| { |
| [package] | ||
| description = "An eBPF library with a focus on developer experience and operability." | ||
| documentation = "https://docs.rs/aya" | ||
| keywords = ["bpf", "ebpf", "kernel", "linux"] | ||
| name = "aya" | ||
| readme = "README.md" | ||
| version = "0.13.2" | ||
|
|
||
| authors.workspace = true | ||
| edition.workspace = true | ||
| homepage.workspace = true | ||
| license.workspace = true | ||
| repository.workspace = true | ||
| rust-version.workspace = true | ||
|
|
||
| [features] | ||
| default = [] | ||
| # Expose test utilities for mocking syscalls in user tests. | ||
| # | ||
| # When enabled, the `sys::test_utils` module becomes available, providing | ||
| # `override_syscall` and the `Syscall` enum so that downstream crates can | ||
| # intercept BPF syscalls in their own `#[cfg(test)]` code. | ||
| test-utils = [] |
Summary
Add a
test-utilsfeature that exposesoverride_syscalland theSyscallenum publicly viasys::test_utilsmodule. This allows downstream crates to intercept BPF syscalls in their tests without needing to reimplement aya's internal types.Motivation
As discussed in #999, testing userspace code that interacts with BPF maps currently requires reimplementing aya's type signatures in mock crates. Aya already has an internal
override_syscallmechanism used in its own tests — this PR exposes it behind an opt-in feature.Changes
test-utilsfeature toaya/Cargo.tomlsys/fake.rsnow compiles undercfg(any(test, feature = "test-utils"))sys::test_utilsmodule (feature-gated) re-exportsoverride_syscall,Syscall,SysResult,bpf_attr,bpf_cmd, andmock_fd()Syscall,PerfEventIoctlRequest, andSysResultmadepub(previouslypub(crate))MockableFdinternals gated onany(test, feature = "test-utils")Usage
Closes #999
Related: #36
This change is