Very interesting idea! Essentially, you're plumbing the btf type of a map value into the relocation process, correct?

Something like:

LoadPinnedMap('path')
ValueTypeFromMap(map) // Doesn't exist at this point I think?
CORERelocate(..., []*Spec{map, ...})

Seems like a very useful pattern, so I wonder if this is something we could generalise.

Very interesting idea! Essentially, you're plumbing the btf type of a map value into the relocation process, correct?

That's correct. Let me add some more details on what we're doing. We have a process that creates a shared map with a value structure like:

struct sockets_value {
	// Fields that are always present must go at the beginning. Ideally these
	// fields aren't never changed, however it's possible to change their order,
	// type, etc. as CO-RE will produce relocations for the gadgets using them.
	__u64 foo;
	__u64 bar;

	// These fields are optional and can be disabled in the socket enricher
	// operator. Gadgets using these fields MUST check if they exist by using
	// bpf_core_field_exists() and get their size with bpf_core_field_size().
	char cwd[SE_PATH_MAX];
	char exepath[SE_PATH_MAX];
};

The cwd and exepath fields require a lot of memory in some cases, so we made them configurable. We dynamically generate the BTF data for that structure and pass it to the relocation process to allow eBPF programs to read and write those fields at the correct offset.

The programs that write and read the eBPF map are on the same process, so we don't pin it, and given that the BTF information of the map is generated, we don't need something like ValueTypeFromMap.

Yeah, makes sense re not pinning the map. I was thinking about how this could fit tetragon which does have persistent maps. It could be interesting to use this mechanism to handle backwards compat.

The one thing I'm unsure is whether passing it as another target in CORERelocate does make sense this way. The function operates on the assumption that all passed specs share a "namespace" aka the kernel. My gut feeling says that it'd be good to distinguish between the relocation target being the kernel vs. custom BTF. Maybe it makes sense to run two relocations?

Maybe it makes sense to run two relocations?

I tried to prototype something but it's not that straightforward. The main issue that I'm facing is that the relocations don't have any information about their target, so I need to run them against all targets and see if any of them successfully resolves.

Something like:

fixups := relocateAgainstKernelAndModules()

// loop over all fixups applying only the resolved ones 
for i, fixup := range fixups {
  if fixup.Unresolved() {
    continue
  }

 apply()...
}

// relocate against custom types
customFixups := relocateAgainstCustomFixups() 

// loop over all fixups applying only the resolved ones 
for i, fixup := range fixups {
  if fixup.Unresolved() {
    continue
  }

  apply()...
}

// check if there is still any missing one to poison them!

I was using the poison boolean to check if a fixup is unresolved, but then I realized for checksForExistence relos they are resolved to target: 0 when the target doesn't exist, so I'd have to check it in a different way.

One issue that I see with this approach is that it'll be harder to check if a relocation is ambiguous between the kernel and custom types.

Do you think it's still worth it to separate the relocations? If yes, do you have any suggestion about the implementation? Does my idea work I should I try something different?

All right, had another look at the CORE headers and such. I think your idea is the most straightforward. If a user passes an extra BTF with conflicting types they will get an ambiguous relocation error anyways. Can we do the following:

• Remove ProgramOptions.KernelModuleTypes. I can't find any open source code using it, and the original reason for it's existence is not valid anymore: in Add CO-RE support for kernel modules #1300 we wanted a way to specify external kmod BTF. Turns out that nowadays btfhub merges all module BTF into a single file anyways so that point is moot.
• newBTFCache will then populate btf.Cache.LoadedModules with an empty slice if KernelTypes != nil. This short circuits trying to find kmod btf in /sys when calling Cache.Modules() in applyRelocations.
• Add ProgramOptions.ExtraRelocationTargets as it is now. This allows you to pass your custom BTF, and if someone really needs external kmod BTF they can emulate it using this mechanism.

@lmb thanks a lot for the clear information, it was so easy to make those changes!

I also removed the logic calling c.Modules() in applyRelocations. I suppose we don't want to remove the modules functionality from the BTFCache as it might be useful for other folks.