Update RingBufs API - allow referring individual maps #318
Conversation
This commit introduces a change to the way RingBuf maps are handled. Previously, they were coalesced with PerfEventArray maps so if a user wanted to handle different maps in different async tasks, they needed to redirect events to e.g. mpsc queues to handle them separately. This change simplifies the API and implementation by allowing to refer to BPF maps individually through a corresponding key in a `HashMap`. Signed-off-by: Nikita Baksalyar <nikita.baksalyar@gmail.com>
| /// } | ||
| /// # }; | ||
| /// ``` | ||
| pub ringbufs: HashMap<String, RingBufMessageStream>, |
There was a problem hiding this comment.
I have a reservation about how this is exposed in the API: there's a discrepancy in how ringbuf and perfmap events are handled.
I think it would make sense to use the same approach for PerfMaps too? This would be a breaking change though.
There was a problem hiding this comment.
As per our agreement with @rhdxmr we are going to bump major version for the next release due to the change in default LLVM. The time to break APIs is now.
There was a problem hiding this comment.
Sounds good! In this case, I'd advocate for adding a changelog - created an issue #319
|
@nbaksalyar What's your plan here? Do you want to get this PR merged, or you'd rather amend this PR with the API changes you mentioned above? |
Yes, I think it's better to address these within this PR because the API will change anyway. It is a bit non-trivial though. I'm trying to figure out how to make it universal and not a chore to use. Most likely it'll require using boxed traits or enums for different map types. I'll prioritise finishing this PR. |
As was discussed in #251, the current ringbufs API has a disadvantage: in order to handle different maps in different async tasks, a user needs to redirect events to e.g. mpsc queues to handle them separately.
This change simplifies the API and implementation by allowing to refer to BPF maps individually through a corresponding key in a
HashMap.cc @yobiscus