Add Unsupported Map type #520

astoycos · 2023-02-10T20:22:49Z

Just because aya doesn't support working with some map types doesn't mean we should't allow them to be loaded.

Add a new Unsupported map type to facilitate this, also add a debug log to let the user know we just loaded and unsupported map type.

netlify · 2023-02-10T20:22:55Z

✅ Deploy Preview for aya-rs-docs ready!

Built without sensitive environment variables

Name	Link
🔨 Latest commit	`17930a8`
🔍 Latest deploy log	https://app.netlify.com/sites/aya-rs-docs/deploys/6492056877b1c6000860e33c
😎 Deploy Preview	https://deploy-preview-520--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 settings.

aya/src/maps/mod.rs

dave-tucker

Hey @astoycos thanks for the PR!

I do think we need this BUT I also think that we need to protect users from footguns.
Therefore could we add a new option to BpfLoader called allow_unsupported_maps() that will enable this?

Then in the normal load() path we'd be able to iter the maps list and return a nice error to users if we encounter unsupported maps (and allow_unsupported_maps is not set). For example:

The map ${map name} is of type ${map_type} which is currently unsupported in Aya

astoycos · 2023-02-27T15:07:22Z

Yep I agree, let me work on adding this!

astoycos · 2023-03-07T21:43:52Z

Ok I think this should do it @dave-tucker.

The way I had to re-write the parse_map function as an impl of bpfLoader in order to get access to the new allow_unsupported_maps field within it was a bit tricky....so If you want me to redo that just let me know

Also are there existing tests for these bpfLoader impl functions? I couldn't find any.

aya/src/bpf.rs

dave-tucker

LGTM

aya/src/bpf.rs

dave-tucker · 2023-03-10T14:43:37Z

@alessandrod for the API changes:

BpfLoader::allow_unsupported_maps()

And the functional change, which is:

If you attempt to load bytecode which uses an unimplemented map type, you get BpfError::UnsupportedMap. This was previously a BpfError::MapError
If you set BpfLoader::allow_unsupported_maps() then we allow the bytecode to be loaded anyway, even if the map can't be interacted with from the Aya code. This is to support usecases like bpfd, where we're loading code on behalf of others, but aren't interacting with their maps.

astoycos · 2023-05-02T20:21:40Z

/assign @alessandrod

astoycos · 2023-05-02T20:22:02Z

Shoot that won't work and I still can't assign folks here :(

alessandrod

Thanks and sorry it took so long to review

aya/src/bpf.rs

alessandrod · 2023-05-27T04:54:40Z

aya/src/bpf.rs

+            && maps
+                .iter()
+                .filter(|(_, x)| matches!(x, Map::Unsupported(_)))
+                .count()


This should use any

So I ended up changing this logic a bit in order to support MapError::Unsupported { map_type: u32 }

More specifically we only search our maps if allow_unsupported_maps is set and use try_for_each to decide if the map is unsupported and build the error to return.

aya/src/bpf.rs

alessandrod · 2023-05-27T04:58:34Z

Shoot that won't work and I still can't assign folks here :(

Just sent you an invite

Just because aya doesn't support working with some map types doesn't mean we should't allow them to be loaded. Add a new `Unsupported` map type to facilitate this, Next add a user configurable option `allow_unsupported_maps()` which can be called against the `bpfLoader`. Additionally add a nice warning log message when a program is being loaded by Aya and contains an unsupported map type. Signed-off-by: Andrew Stoycos <astoycos@redhat.com>

Move BpfError::UnsupportedMap into MapError and add a map_type field to the error. Update the `allow_unsupported_maps` function comment. Update the logic to check if any unsupported maps are being used. More specifically only search a program's maps if `allow_unsupported_maps` is set and then use the iter function `try_for_each` with a match statement which allows us to return the inner map type if it's unsupported. Signed-off-by: Andrew Stoycos <astoycos@redhat.com>

dave-tucker reviewed Feb 14, 2023

View reviewed changes

aya/src/maps/mod.rs Outdated Show resolved Hide resolved

astoycos force-pushed the unsupported-map branch 2 times, most recently from 50deb73 to 56498fe Compare February 15, 2023 03:01

dave-tucker added api/needs-review Makes an API change that needs review feature A PR that implements a new feature or enhancement aya This is about aya (userspace) labels Feb 23, 2023

dave-tucker requested changes Feb 23, 2023

View reviewed changes

astoycos force-pushed the unsupported-map branch 4 times, most recently from 49fa4cf to 05b7744 Compare March 7, 2023 21:41

astoycos force-pushed the unsupported-map branch from 05b7744 to 8bfe557 Compare March 7, 2023 21:48

astoycos mentioned this pull request Mar 8, 2023

BPFD integration with NetObserv bpfman/bpfman#322

Closed

astoycos requested a review from dave-tucker March 8, 2023 18:41

dave-tucker reviewed Mar 8, 2023

View reviewed changes

aya/src/bpf.rs Outdated Show resolved Hide resolved

astoycos force-pushed the unsupported-map branch 2 times, most recently from 82fb951 to b03cf2c Compare March 9, 2023 18:32

astoycos requested a review from dave-tucker March 9, 2023 18:33

dave-tucker approved these changes Mar 10, 2023

View reviewed changes

dave-tucker reviewed Mar 10, 2023

View reviewed changes

aya/src/bpf.rs Outdated Show resolved Hide resolved

astoycos force-pushed the unsupported-map branch from b03cf2c to 20d0236 Compare March 13, 2023 14:00

astoycos mentioned this pull request Mar 28, 2023

Point back to aya: main bpfman/bpfman#380

Closed

dave-tucker requested a review from alessandrod May 3, 2023 16:46

alessandrod requested changes May 27, 2023

View reviewed changes

astoycos added 2 commits June 20, 2023 13:37

astoycos force-pushed the unsupported-map branch from 20d0236 to 17930a8 Compare June 20, 2023 20:00

astoycos requested a review from alessandrod June 20, 2023 20:02

astoycos mentioned this pull request Jul 10, 2023

List ALL bpfPrograms bpfman/bpfman#570

Merged

alessandrod approved these changes Jul 13, 2023

View reviewed changes

alessandrod merged commit eb60d65 into aya-rs:main Jul 13, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add Unsupported Map type #520

Add Unsupported Map type #520

astoycos commented Feb 10, 2023

netlify bot commented Feb 10, 2023 •

edited

Loading

dave-tucker left a comment •

edited

Loading

astoycos commented Feb 27, 2023

astoycos commented Mar 7, 2023

dave-tucker left a comment

dave-tucker commented Mar 10, 2023 •

edited

Loading

astoycos commented May 2, 2023

astoycos commented May 2, 2023

alessandrod left a comment

alessandrod May 27, 2023

astoycos Jun 20, 2023 •

edited

Loading

alessandrod commented May 27, 2023

Add Unsupported Map type #520

Add Unsupported Map type #520

Conversation

astoycos commented Feb 10, 2023

netlify bot commented Feb 10, 2023 • edited Loading

✅ Deploy Preview for aya-rs-docs ready!

dave-tucker left a comment • edited Loading

Choose a reason for hiding this comment

astoycos commented Feb 27, 2023

astoycos commented Mar 7, 2023

dave-tucker left a comment

Choose a reason for hiding this comment

dave-tucker commented Mar 10, 2023 • edited Loading

astoycos commented May 2, 2023

astoycos commented May 2, 2023

alessandrod left a comment

Choose a reason for hiding this comment

alessandrod May 27, 2023

Choose a reason for hiding this comment

astoycos Jun 20, 2023 • edited Loading

Choose a reason for hiding this comment

alessandrod commented May 27, 2023

netlify bot commented Feb 10, 2023 •

edited

Loading

dave-tucker left a comment •

edited

Loading

dave-tucker commented Mar 10, 2023 •

edited

Loading

astoycos Jun 20, 2023 •

edited

Loading