aya,aya-obj,integration-test: add better support in `ProgramInfo` & `MapInfo` for old kernels
This adds support for the field availability in ProgramInfo and MapInfo.
Srry this is long, let me know and I can try to TL;DR it.
For ProgramInfo:
- Have
program_type()to returnbpf_prog_typeinstead of the integer representation. - For fields that can't be
0, returnOption<NonZero*>type to indicateNoneas field being unavailable. - Have
name_as_str()use thebpf_name()feature probe as an additional check for whether field is available. Besides invalid unicode,Noneis returned if probe does not detect feature. - For
mad_ids():- Now returns a result of
Option<Vec<NonZeroU32>>, withNoneindicating field is unavailable. - On kernels below v4.15, the init closure that fills in the map info causes an
E2BIGerror sincecheck_uarg_tail_zero()expects the entire struct to be zero bytes. I initially worked around this by havingbpf_prog_get_info_by_fd()do an additional syscall if the first failed withE2BIG. - Now,
map_ids()andbpf_prog_get_info_by_fd()both uses a new feature probeprog_info_map_ids()to detect if field available.
- Now returns a result of
- For
gpl_compatible(), it now returnsOption<bool>, withNoneindicating field is unavailable. It uses a new feature probeprog_info_gpl_compatible()to detect for field availability.
There was a redundant export of loaded_programs() which gave it 2 method of access: aya::loaded_programs and aya::programs::loaded_programs. To avoid confusion, it should now only be aya::programs::loaded_programs.
For MapInfo:
- Have
map_type()to returnbpf_map_typeinstead of the integer representation. - For fields that can't be
0, returnOption<NonZero*>type to indicateNoneas field being unavailable. - For
name_as_str(), it uses thebpf_name()feature probe to detect for field availability. Although the probe performs the check on program name, map name was introduced in the same commit as program name so it should suffice.
Integration tests for ProgramInfo and MapInfo are updated to use kernel_assert/kernel_assert_eq macro.
The macro first performs the assertion check. If assertion passes, continue with test. If failed, then check host's kernel version.
If host is below the feature version, then continue with test and log it in stderr (can be observed with -- --nocapture). Otherwise, panic 😰.
I verified that this passes on versions (i use ubuntu mainline images for my integration tests):
- 4.13.0
- 4.15.0
- 5.15.0
- 6.1.0
Deploy Preview for aya-rs-docs ready!
Built without sensitive environment variables
| Name | Link |
|---|---|
| Latest commit | fbb09304a2de0d8baf7ea20c9727fcd2e4fb7f41 |
| Latest deploy log | https://app.netlify.com/sites/aya-rs-docs/deploys/66d5def807650600083b06fb |
| Deploy Preview | https://deploy-preview-1007--aya-rs-docs.netlify.app |
| Preview on mobile | Toggle QR Code...Use your smartphone camera to open QR code link. |
To edit notification comments on pull requests, go to your Netlify site configuration.
Hey @alessandrod, this pull request changes the Aya Public API and requires your review.
@tyrone-wu, this pull request is now in conflict and requires a rebase.
Overall, I agree with @alessandrod, we shouldn't use
Option<NonZero*>as return values butOption<IntegerType>instead (0 mapping to None)
Why is that better? If we know they can't be zero it's better to communicate that in the type.
Why is that better? If we know they can't be zero it's better to communicate that in the type.
Because API ergonomics matter.
NonZero<T> doesn't Deref to T, so I have to call .get(). Having to call get seems worse than having extra information there's no use for, especially since NonZero<T>::new(v) return Option<NonZero<T>>, so I have to match and get.
// This _does_ provide useful information - I can't call f(0)
fn f(x: NonZeroU32) {
}
// Option tells me whether the map_id is there or not. Then what use is it to me to know that the map id won't be 0?
// This would be useful if I were to pass the return value to something else, but we have stronger wrappers for things like ProgramIds etc.
fn map_id(&self) -> Option<NonZeroU32> {
}
Why is that better? If we know they can't be zero it's better to communicate that in the type.
Because API ergonomics matter.
NonZero<T>doesn't Deref to T, so I have to call.get(). Having to call get seems worse than having extra information there's no use for, especially sinceNonZero<T>::new(v)returnOption<NonZero<T>>, so I have to match and get.// This _does_ provide useful information - I can't call f(0) fn f(x: NonZeroU32) { } // Option tells me whether the map_id is there or not. Then what use is it to me to know that the map id won't be 0? // This would be useful if I were to pass the return value to something else, but we have stronger wrappers for things like ProgramIds etc. fn map_id(&self) -> Option<NonZeroU32> { }
Then why expose the scalar at all? If the value doesn't matter, make it opaque.
Then why expose the scalar at all? If the value doesn't matter, make it opaque.
We should definitely have types for all these IDs, but babysteps. Even without stronger ids this PR was an improvement over what was there before, and removing NonZeroU32 is an improvement over what's there, imo.