Fixup the load time bpf Maps API

[astoycos]

The Problem

Originally (since I've been a contributor) we we're only given a single overloaded API by aya regarding bpf maps at load time, i.e .map_pin_path().

On the eBPF side itself users were also provided the LIBBPF_PIN_BY_NAME flag.

Originally (before some recent patch sets) .map_pin_path() + LIBBPF_PIN_BY_NAME actually resulted in 2 functions:

• Allowed the user to specify an existing bpffs directory where a map(s) would be pinned by name at load time if the user also defined the map with the LIBBPF_MAP_PIN_BY_NAME flag.
• If the map pins already existed the maps (and their existing entries) were opened and used instead of creating new ones.

Current Behavior

Today the API(s) has been cleaned up and works like the following but IMO is still broken:

1. If .map_pin_path(PATH) IS NOT provided and LIBBPF_PIN_BY_NAME IS NOT set, the map is created and not pinned anywhere.
2. If .map_pin_path(PATH) IS provided but LIBBPF_PIN_BY_NAME IS NOT set, the map is created and not pinned anywhere.
3. If .map_pin_path(PATH) IS NOT provided and LIBBPF_PIN_BY_NAME IS set, the map is created and pinned by name at /sys/fs/bpf, pins are re-used if they exist
4. if .map_pin_path(PATH) IS provided and LIBBPF_PIN_BY_NAME IS set, the map is created and pinned by name at PATH, pins are re-used if they exist

Moving forward (our supported/documented use cases)

I propose adding/fixing the APIs to behave in a more predictable manner while also at a high level allowing users to:

1. Specify a map pin path at load time allowing the user to pin a map by name OR via a direct arbitrary path, always re-creating the map.
2. Specify a map pin path at load time allowing the user to re-use a map pin by name OR re-use a pin from a driect arbitrary path, always re-using the map and failing if it doesn't already exist (OR possibly from a previously created map see Allow maps to be created from user-space #46).
3. Specify that all maps should be re-created and pinned by name at load time.
4. Specify that all maps should be re-used from pins by name at load time, failing if they don't exist. (notice this is takes explicit user intent rather than being bundled into 3.)
5. Allow LIBBPF_PIN_BY_NAME to work if:
   • a map pin path is provided OR
   • a map pin path is not provided (pin to /sys/fs/bpf)
6. ALWAYS check for the existence of + try and reuse maps from the path determined in 5. if LIBBPF_PIN_BY_NAME is set.

I would like to align on these use cases BEFORE moving forward with the API design.

[dave-tucker]

On the eBPF side itself users were also provided the LIBBPF_PIN_BY_NAME flag.

This is not true. It's only required if the BPF-side wants to indicate to the loader that it SHOULD be pinned.
Userland can pin it regardless of whether the flag is present or not, but it won't happen automagically.

Today the API(s) has been cleaned up and works like the following but IMO is still broken:

Broken how?
The examples you've given are fixed on .map_pin_path, which only comes into play when used with LIBBPF_PIN_BY_NAME, but this only one of the may ways in which you can pin maps.
See also: https://docs.aya-rs.dev/aya/maps/enum.map#method.pin

The interactions you describe between the root pin path + the LIBBPF_PIN_BY_NAME flag are, as I recall, exactly the same as you get from libbpf so I'm 👎 on changing anything there.

Specify a map pin path at load time allowing the user to pin a map by name OR via a direct arbitrary path, always re-creating the map.

This doesn't make sense to me. How can you always re-create the map if there is always something at the pinned location?
I'm not sure that doing this in BpfLoader makes sense either - e.g .map_pin_path("foo", "/path/to/foo").map_pin_path("bar", "/path/to/bar"). I'll touch on this again later.

Specify that all maps should be re-created and pinned by name at load time.

What do you do when you have a conflict on pin path? E.g one or more maps already exist?

Specify that all maps should be re-used from pins by name at load time, failing if they don't exist. (notice this is takes explicit user intent rather than being bundled into 3.)

I mean sure, but I think this gets weird pretty quickly.

Specify a map pin path at load time allowing the user to re-use a map pin by name OR re-use a pin from a driect arbitrary path, always re-using the map and failing if it doesn't already exist (OR possibly from a previously created map see

I'm ok with this as a use-case, but I don't think this should be the "default" behaviour.

...

Pinning workflows are pretty advanced BPF stuff, and I don't think we should touch the existing user experience too much.
I do however think that part of the problem is in BpfLoader::load - we've bumped up against multiple use-cases for splitting map creation and loading the bytecode into the kernel and I think that should be the focus of the work you're doing.

For example, look at what you can do with libbpf:

• You can set a root pin path in bpf_object__open_*
• You can pin all an objects maps using bpf_object__pin_maps (I imagine that will exclude read-only maps like .rodata and .bss etc...)
• You can pin a map using bpf_map__pin - I'd assume the root path is inherited from the the original open call, but you can also provide a path
• You can use bpf_map__set_pin_path to change the pin path on a per-map-basis
• You can pin a program/map it's set path OR a specific path with bpf_map__pin
• bpf_map__set_autocreate disables a map from being autocreated when you load it into the kernel.

(Some investigation required to know whether it appends name to the path parameter in all cases)

Having equivalent APIs in Aya to expose the same features would be great.

TL:DR

I think that where you need to focus on this here.
Specifically:

• make BpfLoader::new() into BpfLoader::new(bytes)
• The first thing BpfLoader does is to parse the bytecode using aya_obj as it does today
• APIs on BpfLoader should be able to manipulate the parsed bytecode - that should provide better experience for things like set_global or marking programs as bpf_prog_type_ext, manipulating maps, creating your own maps from userspace etc...
• BpfLoader::load() is still the last call in the chain, and yields an aya::Bpf... it just doesn't take a path to the bytecode anymore.

With ☝️ done, manipulating stuff before loading can now fail early - i.e we know that global variable "bar" doesn't exist... or that map "foo" isn't there... etc...

[dave-tucker]

Oh just a note, all of the weird and wonderful pinning use-cases you mentioned can be expressed as one-or-more calls to BpfLoader APIs before calling BpfLoader::load(). I'd much rather we did that than, say, adding lots of options like always_reuse_maps_from_pins_or_else_error(true) etc...