The CI is unhappy with this change. This should take module BTF into account.

The error is fixing up kfuncs: find target in modules: parse types for module bpf_testmod: lookup type name for id 1: offset 1489950 is out of bounds of string table

I think that makes sense if we think about it. Kernel modules use split BTF, they essentially extend the vmlinux the kernel ships with. Strings in the module BTF are encoded as {length of base string table} + offset in module string table. So when we provide a custom BTF for the base in this way, we are unable to understand the module BTF.

I see two ways of approaching this. First, If we are in a situation where a custom BTF is provided, simply do not attempt to use the module BTF. If the user needs type from a module, he has to make sure that the custom BTF contains the types from the module as well.

Second, is we could allow users to provide a set of module BTFs which we would consult instead of the module BTFs from the kernel itself (if any exist). It would be up to the user to ensure they are generated with the provided vmlinux BTF as base. This can be sometime we add later as addition to the first approach.

Second, is we could allow users to provide a set of module BTFs which we would consult instead of the module BTFs from the kernel itself (if any exist). It would be up to the user to ensure they are generated with the provided vmlinux BTF as base. This can be sometime we add later as addition to the first approach.

To add a data point to this discussion, BTFHub ships a single blob per kernel and seems to disregard kernel modules entirely. (I could be wrong, of course!) libbpf also accepts only a single blob in .btf_custom_path: https://www.mankier.com/8/bpftool-gen#Examples-min_core_btf:

LIBBPF_OPTS(bpf_object_open_opts, opts, .btf_custom_path = "5.4.0-smaller.btf");
struct bpf_object *obj;

obj = bpf_object__open_file("one.bpf.o", &opts);

Purely for doing CO-RE, all kmods could technically be merged into a single blob, but it doesn't seem like the btfhub tooling currently supports that. And since these kernels aren't getting any younger, I doubt there's much incentive to do so.

Can't find the source, but if .btf_custom_path is set, libbpf ignores all other sources of BTF (like kmods). I think that only makes sense.

I don't think it's possible to use external kmod BTF for kfuncs in the first place. See the code which adds the module handle to fdArray: that handle needs to point at a kmod BTF, you can't just load external BTF into the kernel.

I forget how kfuncs targeting the kernel work exactly. Is it even possible without having BTF built in? AKA, not sure this change makes sense at all.

I don't think it's possible to use external kmod BTF for kfuncs in the first place. See the code which adds the module handle to fdArray: that handle needs to point at a kmod BTF, you can't just load external BTF into the kernel.

I forget how kfuncs targeting the kernel work exactly. Is it even possible without having BTF built in? AKA, not sure this change makes sense at all.

Good points! I assume it may be possible to invoke vmlinux kfuncs using external BTF if a distro/admin decided to ship BTF separately instead of in /sys/kernel/btf as a 'security' measure? Iirc kfuncs are referred to by btf id. We probably shouldn't take kmod BTF into account during resolving in any case.

Iirc kfuncs are referred to by btf id.

Exactly, so how does the kernel figure out what to attach to if it doesn't have BTF included? It also needs to do the id -> kfunc lookup, no? And if it needs BTF included, why is it not available in /sys/kernel/btf?

@bogushevich can you tell us a bit more when / how this is useful / necessary?

Exactly, so how does the kernel figure out what to attach to if it doesn't have BTF included? It also needs to do the id -> kfunc lookup, no? And if it needs BTF included, why is it not available in /sys/kernel/btf?

ACK. From what I understand from the kconfigs and header files is that when compiling with BTF, the kfunc BTF ids are encoded in the ".BTF_ids" ELF section of the kernel, which are then read at runtime to know which ids are kfuncs with what flags. If you don't compile the kernel with BTF, these are not included, thus no kfuncs. Even if you were to create BTF from the DWARF like those on BTFHub.

@bogushevich how about we document this on fixupKfuncs so that future generations don't go down the same rabbit hole?

Sorry, it was my mistake. You are right, after this commit my program is loaded successfully, but doesn't work properly.
My code is like this:

volatile const __u8 conntrack_enabled = 0;

extern struct nf_conn *bpf_skb_ct_lookup(
    struct __sk_buff *,
    struct bpf_sock_tuple *, u32,
    struct bpf_ct_opts___local *, u32
) __ksym __weak;

if (conntrack_enabled) {
   struct nf_conn *ct = bpf_skb_ct_lookup(/*...*/)
   if (ct) {/*...*/}
}

So if flag conntrack_enabled is not set from user space, program is successfully loaded even on kernels that don’t support necessary kfuncs. But if there is no system BTF, program fallback to custom BTF and loading is failed on fixupKfuncs step regardless of flag value.
Unfortunately some modern Linux distributions still don’t enable BTF for some reason. I just dont’t want to have two similar BPF program: one with conntrack support and one without.

So if flag conntrack_enabled is not set from user space, program is successfully loaded even on kernels that don’t support necessary kfuncs. But if there is no system BTF, program fallback to custom BTF and loading is failed on fixupKfuncs step regardless of flag value. Unfortunately some modern Linux distributions still don’t enable BTF for some reason. I just dont’t want to have two similar BPF program: one with conntrack support and one without.

I think this makes a lot of sense to support. @dylandreimerink @lmb Should we treat an error from btf.LoadKernelSpec in fixupKfuncs as recoverable and poison all weak kfunc calls? IMO that's minimally invasive.

Unfortunately some modern Linux distributions still don’t enable BTF for some reason. I just dont’t want to have two similar BPF program: one with conntrack support and one without.

I think this makes a lot of sense to support. @dylandreimerink @lmb Should we treat an error from btf.LoadKernelSpec in fixupKfuncs as recoverable and poison all weak kfunc calls? IMO that's minimally invasive.

I think so. I suggest we tolerate the ErrNotSupported error, then short-circuit findTargetInKernel. So when no system BTF is found we essentially act as if kfuncs can never be found. This allows use cases such as these to mark their kfunc as
__weak and allow you to write code that checks if the kfunc is available and fall back if not using CO-RE. If we find strong kfuncs, we will throw the usual error.

extern struct nf_conn *bpf_skb_ct_lookup(
   struct __sk_buff *,
   struct bpf_sock_tuple *, u32,
   struct bpf_ct_opts___local *, u32
) __ksym __weak;

if (bpf_ksym_exists(bpf_skb_ct_lookup)) {
  struct nf_conn *ct = bpf_skb_ct_lookup(/*...*/)
  if (ct) {/*...*/}
}