Load eBPF programs with relocated instructions and user-specified program type

[aditighag]

Currently, the library exposes 2 public APIs to load BPF programs - 1) NewProgram* 2) LoadCollectionSpec. While (1) takes a user-specified program type as an argument, it also expects user to provide asm instructions. (2) on the other hand, automatically loads asm instructions for a detected program section, but tries to infer the program type based on the section name. Users are thus required to encode the program type in section, which can be limiting in certain cases if the same program needs to be attached with different types.

Can we extend (2) (or expose a new API specifically to load a program spec) to take user-defined program type in account while loading a program?

[ti-mo]

Thanks for flagging this, this also bit me in the past. Perhaps a map of progName -> ProgramType, AttachType to override an inference decision for a particular program, which wouldn't force the caller to suddenly explicitly have to specify all types of all programs in the ELF if they just want to override one.

In case we go for the latter, we could go for LoadCollectionSpec(file string, config *CollectionSpecConfig) or similar config struct to influence the behaviour of parts of the ELF loader pipeline.

@aditighag I have a vague suspicion about the specific use case in Cilium, but could you elaborate a bit? Is this also related to having a generic ctx struct for both skb and xdp contexts? (cilium/cilium@1dc3e39)

[lmb]

Knowing more about the use case would be great. So far I assumed that most BPF context are basically incompatible with each other, I'm curious to hear how you'd work around different helpers being available, etc.

FWIW I'm not a big fan of the section convention either (especially that every program needs a separate section), but it's what libbpf does and I've not really come up with a better solution.

[aditighag]

BPF helpers like socket lookup too can have different contexts depending on the hook points.

One use case we have is being able to potentially load tail call programs at different hook points.

However, I've filed this issue with also keeping a more general use case in mind - can we expose a hybrid API like I mentioned in the description, whereby users can pass program types, and library loads the relocated instructions? None of the Cilium BPF programs have program types encoded in their respective section names. Also, expecting users to pass asm instructions while loading a program seems tedious (and error-prone) unless I'm missing something?

It seems to me that we already have the infrastructure in place to support a more flexible API. I'm interested in knowing your thoughts. Happy to work on this depending on what we decide.

[lmb]

BPF helpers like socket lookup too can have different contexts depending on the hook points. One use case we have is being able to potentially load tail call programs at different hook points.

Do you have programs like this somewhere, or is this hypothetical at this point? From my POV sharing programs across execution contexts like this is possible only very rarely, so adding explicit API to support it doesn't seem worth it. With concrete examples this might change.

can we expose a hybrid API like I mentioned in the description, whereby users can pass program types, and library loads the relocated instructions?

That is essentially what you get when loading a collection spec, no? For example, you can clone a ProgramSpec and change .Type to what you need. I think you can even use custom section names with the ELF loader right now, ProgramSpec.Type will end up as UnspecifiedProgram and you can edit it to whatever value you need. So you don't have to follow the libbpf conventions.

expecting users to pass asm instructions while loading a program seems tedious (and error-prone) unless I'm missing something?

I don't expect users to make up ProgramSpec on their own, but instead go via ELF and CollectionSpec. NewProgram() is a low level API: here are the exact things that mape up a program, load them into the kernel. Taking asm.Instructions is a feature, e.g. we use it to generate eBPF from cBPF on the fly: https://pkg.go.dev/github.com/cloudflare/cbpfc?utm_source=godoc#ToEBPF

[aditighag]

With concrete examples this might change.

Sure, let's revisit this once we have a concrete use case.

I don't expect users to make up ProgramSpec on their own, but instead go via ELF and CollectionSpec. NewProgram() is a low level API

It doesn't hurt to add an explicit note to the godoc indicating so.

I think you can even use custom section names with the ELF loader right now, ProgramSpec.Type will end up as UnspecifiedProgram and you can edit it to whatever value you need.

LoadCollectionSpec doesn't return a program spec for UnspecifiedProgram types -

ebpf/elf_reader.go

Line 299 in 2e76685

libs = append(libs, spec)

. That's how I discovered the section based inference logic. Was this the same code you were referring to?

[aditighag]

I think you can even use custom section names with the ELF loader right now, ProgramSpec.Type will end up as UnspecifiedProgram and you can edit it to whatever value you need.

LoadCollectionSpec doesn't return a program spec for UnspecifiedProgram types -

ebpf/elf_reader.go

Line 299 in 2e76685

libs = append(libs, spec)

. That's how I discovered the section based inference logic. Was this the same code you were referring to?

@lmb Could you respond when you get a chance? I wanted to check this with you first before drafting a fix.

[lmb]

Ok, I had forgotten about that bit. Just to make sure, the goal is to keep current Cilium section names as they are (aka not conforming to libbpf) to ease transition?

Some more context around the code you highlighted, it's used for bpf-to-bpf calls.

static int other_func() { ... };

SEC("xdp") int prog(void *ctx) { return other_func(); }

Since other_func has no section annotation it'll be placed in .text. The derived type for that is UnspecifiedProgram. The ELF reader goes and collects all UnspecifiedProgram sections into libs. As a final step, all sections where the type is != UnspecifiedProgram are linked against libs. Basically, we use the fact that we recognise the section as a signal to make it available in CollectionSpec. .text isn't exposed on its own, since that doesn't make much sense.

Would it be sufficient if you could provide func(sectionName string) (ProgType, ...) to override the type detection? Otherwise we need to figure out a different heuristic as to which functions should be exported, which could get complicated.

[aditighag]

Thanks for the context, @lmb . This is helpful.

Would it be sufficient if you could provide func(sectionName string) (ProgType, ...) to override the type detection?

Yeah, I had something similar in mind. I'll see how I can extend the existing API.

[aditighag]

Given that libbpf recently introduced mandatory conventions around specifying the program type, I'm closing this bug for now. Let's cross the bridge when we come to it.

[ti-mo]

My bad, forgot about this issue. This was actually fixed through #508 and #529, as this was the last blocker for loading the Cilium datapath unmodified using the lib.
Any program that is emitted to a section will now appear in the CollectionSpec.

Finishing up last release blocker and planning to cut a release tomorrow.

We currently don't see much point in forcing section names on our users, I thought the libbpf maintainers also changed their minds about it.

[aditighag]

Thanks for the follow-up. 🚀

We currently don't see much point in forcing section names on our users, I thought the libbpf maintainers also changed their minds about it.

+1
Good to know that libbpf maintainers also removed the mandate about encoding section names.