ebpf icon indicating copy to clipboard operation
ebpf copied to clipboard

features: add HaveMapFlag API

Open paulcacheux opened this issue 2 years ago • 2 comments

HaveMapFlag(flag uint32) error would allow a user to probe running kernel for support for map flags. The main target of this would be to probe for availability of mmapable array maps.

This PR could also be replaced by just exporting https://github.com/cilium/ebpf/blob/314c7732ecc76e7587edf487d1048fbc165983e3/syscalls.go#L91

paulcacheux avatar May 13 '22 17:05 paulcacheux

Thanks for the PR @paulcacheux, I'll give it a closer look tomorrow. Back when we added the HaveMapType API there was some discussion around adding a HaveMapFlags API, roughly starting at this message on #235. Maybe you can give it a look in the meantime. But the gist of it was that having HaveMapFlag(MapType, Flag) opens up a lot of possible combinations that we would need to test for.

rgo3 avatar May 17 '22 20:05 rgo3

@paulcacheux Feel free to swing by #ebpf-go-dev on our community Slack if you want to discuss this more interactively. We have a few questions around this proposal:

  1. What is the practical use case? Please include this in any future contributions so we get a better idea of what you're trying to achieve. (also, commit message :pray:)
  2. Would an API that only takes a flag (not a map type) be sufficient? We could probe using a map type that is known to support the flag.
  3. Building on the previous point, are there known cases in the kernel's development history where a flag was introduced on one map type, and later implemented on more map types? This is important to know if we can get away with (2).

Something else to take into account is the caller can probe for map type A with flag B, but the kernel doesn't necessarily support map type A. Of course, this means that the (A, B) combo is definitely not supported, but this overlaps a bit too much with HaveMapType. Knowing more about the use case would really help here.

Thanks!

ti-mo avatar May 25 '22 19:05 ti-mo

Just added the comment, absolutely agreed for the followup PR. I will launch a discussion on slack on the best way to implement this

paulcacheux avatar Aug 17 '22 08:08 paulcacheux

@paulcacheux thanks for reviving this PR! Can you tell me when you'll be able to do the follow up work? I'd like the new map flags type before we tag any more releases.

lmb avatar Aug 23 '22 08:08 lmb

I have been working a bit on the follow up work, the issue I'm facing is that the map flags are defined in an anonymous enum by the kernel (https://elixir.bootlin.com/linux/latest/source/include/uapi/linux/bpf.h#L1192), making it not easy to generate the go code like it's done for maps. Here is a PR to start the work on https://github.com/cilium/ebpf/pull/766

paulcacheux avatar Aug 23 '22 09:08 paulcacheux

Cool! I think for now it's ok to:

  1. Add sys.MapFlags
  2. Make the various sys.MapCreate, etc. struct use MapFlags
  3. Alias ebpf.MapFlags, features.MapFlags to sys.MapFlags.
  4. Add const (...) to features

Tackling the anonymous enum can happen at some other point.

lmb avatar Aug 23 '22 09:08 lmb