Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

aya: add feature probing for program and map type #1063

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

tyrone-wu
Copy link
Contributor

@tyrone-wu tyrone-wu commented Oct 19, 2024

I tested the new integration tests against a ton of ubuntu mainline kernels. Basically on every version where the program/map type was introduced I was able to verify that they were passing as expected.


This change is Reviewable

Copy link

netlify bot commented Oct 19, 2024

Deploy Preview for aya-rs-docs ready!

Built without sensitive environment variables

Name Link
🔨 Latest commit f4df1a9
🔍 Latest deploy log https://app.netlify.com/sites/aya-rs-docs/deploys/67c5f253f8e7c100086d9896
😎 Deploy Preview https://deploy-preview-1063--aya-rs-docs.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

Copy link

mergify bot commented Oct 19, 2024

Hey @alessandrod, this pull request changes the Aya Public API and requires your review.

@mergify mergify bot added the api/needs-review Makes an API change that needs review label Oct 19, 2024
@mergify mergify bot requested a review from alessandrod October 19, 2024 20:16
@mergify mergify bot added aya This is about aya (userspace) aya-obj Relating to the aya-obj crate test A PR that improves test cases or CI labels Oct 19, 2024
@tyrone-wu tyrone-wu force-pushed the aya/probe-prog-types branch from 97dcc2b to d9bc9b7 Compare October 19, 2024 20:32
@tyrone-wu tyrone-wu changed the title aya: add feature probing program types aya: add feature probing for program and map type Oct 21, 2024
@tyrone-wu tyrone-wu force-pushed the aya/probe-prog-types branch 5 times, most recently from 0e67fa4 to 6075a11 Compare October 24, 2024 17:59
Copy link

mergify bot commented Jan 1, 2025

@tyrone-wu, this pull request is now in conflict and requires a rebase.

@mergify mergify bot added the needs-rebase label Jan 1, 2025
@tyrone-wu tyrone-wu force-pushed the aya/probe-prog-types branch from 6075a11 to c976fc0 Compare February 3, 2025 04:24
@mergify mergify bot removed the needs-rebase label Feb 3, 2025
@tyrone-wu tyrone-wu force-pushed the aya/probe-prog-types branch from c976fc0 to 294b950 Compare February 3, 2025 04:28
@tyrone-wu tyrone-wu marked this pull request as ready for review February 3, 2025 05:01
@tamird
Copy link
Member

tamird commented Mar 1, 2025

@tyrone-wu can you rebase this please?

Copy link

mergify bot commented Mar 1, 2025

@tyrone-wu, this pull request is now in conflict and requires a rebase.

@mergify mergify bot added the needs-rebase label Mar 1, 2025
@tamird tamird force-pushed the aya/probe-prog-types branch from 294b950 to c9352ec Compare March 3, 2025 17:55
@tamird
Copy link
Member

tamird commented Mar 3, 2025

@tyrone-wu I rebased this for you.

@mergify mergify bot removed the needs-rebase label Mar 3, 2025
@tamird tamird force-pushed the aya/probe-prog-types branch 2 times, most recently from 5a19189 to 9ba0907 Compare March 3, 2025 18:00
Adds API that probes whether kernel supports a program type.

Assertions for `LircMode2` and `Lsm` are disabled because they require
certain kernel configs to be enabled, which are not by default in VM
tests.
Add API that probes whether kernel supports a map type.

Assertion for `InodeStorage` are disabled because they require
CONFIG_BPF_LSM to be enabled, which is not be default in VM tests.
@tamird tamird force-pushed the aya/probe-prog-types branch from 9ba0907 to 480338e Compare March 3, 2025 18:15
Cached probed for ProgramInfo fields instead of exposing it through
global FEATURE. Probing occurs on cache miss, which happens when first
accessing the field, *and* if the field is 0.
@tamird tamird force-pushed the aya/probe-prog-types branch from 480338e to f4df1a9 Compare March 3, 2025 18:17
Copy link
Member

@tamird tamird left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewed 1 of 51 files at r1, 38 of 47 files at r2, 7 of 12 files at r4, 9 of 11 files at r7, 3 of 6 files at r8, 1 of 1 files at r9, 2 of 8 files at r10, 1 of 4 files at r12, 6 of 9 files at r13, all commit messages.
Reviewable status: 48 of 51 files reviewed, 5 unresolved discussions (waiting on @alessandrod)


aya/Cargo.toml line 25 at r13 (raw file):

log = { workspace = true }
object = { workspace = true, features = ["elf", "read_core", "std", "write"] }
once_cell = { workspace = true, features = ["race"] }

back this out? see comment in info.rs


aya/src/programs/info.rs line 11 at r13 (raw file):

use aya_obj::generated::{bpf_prog_info, bpf_prog_type};
use once_cell::race::OnceBool;

why can't we use std::cell::OnceCell here?


aya/src/programs/info.rs line 125 at r13 (raw file):

        } else {
            Ok(None)
        }

this transformation doesn't look right to me. What am I missing?

Code quote:

    pub fn map_ids(&self) -> Result<Option<Vec<u32>>, ProgramError> {
        if self.0.nr_map_ids > 0 {
            let mut map_ids = vec![0u32; self.0.nr_map_ids as usize];
            bpf_prog_get_info_by_fd(self.fd()?.as_fd(), &mut map_ids)?;
            return Ok(Some(map_ids));
        }

        static CACHE: OnceBool = OnceBool::new();
        if CACHE.get_or_init(|| matches!(is_prog_info_map_ids_supported(), Ok(true))) {
            Ok(Some(vec![]))
        } else {
            Ok(None)
        }

aya/src/programs/info.rs line 152 at r13 (raw file):

    pub fn gpl_compatible(&self) -> Option<bool> {
        if self.0.gpl_compatible() != 0 {
            return Some(true);

the previous code checked both conditions, why is it ok to short circuit now?


test/integration-test/src/tests/feature_probe.rs line 199 at r7 (raw file):

}

// Back-to-back calls can be flaky and return `EPERM`.

That's weird. Citation?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
api/needs-review Makes an API change that needs review aya This is about aya (userspace) aya-obj Relating to the aya-obj crate test A PR that improves test cases or CI
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants