-
Notifications
You must be signed in to change notification settings - Fork 305
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
Refactor map API #397
Refactor map API #397
Conversation
✅ Deploy Preview for aya-rs-docs ready!Built without sensitive environment variables
To edit notification comments on pull requests, go to your Netlify site settings. |
Still need to clean up commits and other stuff but the core bits are building and passing unit tests |
2b95d14
to
8f2b5ca
Compare
8db7617
to
67bc3b9
Compare
b312c6b
to
bf46b2a
Compare
bf46b2a
to
5442552
Compare
5442552
to
aa02ed4
Compare
@alessandrod @dave-tucker I think this is ready for you to start reviewing....I'm happy to add more testing or anything else you would like to this PR or in follow up Issues, Thanks! |
(also I can't assign you since I'm not a member of the org, any chance I could get added? ) |
Ah #398 Merged updating now :) |
aa02ed4
to
1b23006
Compare
fb6898b
to
939d16c
Compare
Remove MapError::UnexpectedMapType Add Macro for converting from aya::Map to u32 (map type) for use in `MapError::InvalidMapType { map_type: x }` Signed-off-by: Andrew Stoycos <[email protected]>
aya/src/maps/mod.rs
Outdated
macro_rules! impl_try_from_map_generic_key_or_value { | ||
($($typ:ty),+ $(,)?) => { | ||
$( | ||
impl From<$typ> for u32 { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Someone is getting excited with these macros! :P
I think this should be just an internal utility method and not a from impl (nor a macro). Right now you've made it (accidentally) a public API and I don't think we want to expose Map => u32 publicly
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Lol yeah I go over excited, I forgot this was as simple as adding a simple aya::Map
Method, and I do like Macros :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looking great now, see last thing about the from impl, then it's good to go
Remove From method and replace with internal helper function. Signed-off-by: Andrew Stoycos <[email protected]>
Should be all set! :) |
4d65b3d
to
7e55323
Compare
switch map() and map_mut() from returning a `Result` to an `Option` since it's just getting a value from a Hashmap, and to stay in line with the Programs API. Remove `MapError::MapNotFound` Signed-off-by: Andrew Stoycos <[email protected]>
7e55323
to
f3262e8
Compare
aya/src/bpf.rs
Outdated
/// | ||
/// # Errors | ||
/// This API is intended for cases where the map must be moved into spawned |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Use this when borrowing with [`map`] or [`map_mut`] is not possible (eg when using the map from an async task). The returned map will be closed on `Drop`, therefore the caller is responsible for managing its lifetime.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done!
8b04b33
to
9b2b990
Compare
Fix some broken rust doc links. Make sure rustdoc build fail on warnings so we catch these broken links in CI. Signed-off-by: Andrew Stoycos <[email protected]>
9b2b990
to
82edd68
Compare
@@ -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").unwrap().try_into()?; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this ?
-> .unwrap()
means that bpf programs that don't log will result in a panic on the userspace side.
Sent #476 to fix this.
They were broken by aya-rs#397
These were broken by #89 which pulled in aya-rs/aya#397 and other changes which were not reflected in the templates.
These were broken by #89 which pulled in aya-rs/aya#397 and other changes which were not reflected in the templates.
Fixes #386