bpf_*_spin_*: remove tracing programs from program types

[AliGhaffarian]

eBPF verifier states "tracing progs cannot use bpf_spin_lock yet", so let's remove them from available program types section.

Apparently, bpf_res_spin_lock behaves the same.

Whether a program is tracing type or not is determined by the following function:

static bool is_tracing_prog_type(enum bpf_prog_type type)
{
	switch (type) {
	case BPF_PROG_TYPE_KPROBE:
	case BPF_PROG_TYPE_TRACEPOINT:
	case BPF_PROG_TYPE_PERF_EVENT:
	case BPF_PROG_TYPE_RAW_TRACEPOINT:
	case BPF_PROG_TYPE_RAW_TRACEPOINT_WRITABLE:
		return true;
	default:
		return false;
	}
}

[dylandreimerink]

Hey, I think these changes are correct, at least when it comes to the helper functions. Its just that the fix in the docs will need to look different. We generate these big lists from the data files because its otherwise very painful to maintain these properly.

The data file for the helpers works by modeling the functions as written in the kernel to resolve which program types are allowed to use which helpers. Functions which are re-used are put into groups which can be included. But for these helpers the verifier has special casing, which breaks with the model.

ebpf-docs/data/helpers-functions.yaml

Line 164 in 4b71e67

# TODO tracing program are not allowed to use `bpf_spin_lock`, limited in the verifier

So, to do this properly, we should teach the helper-ref-gen tool that some helpers need to be excluded after all the groups have been resolved, to match what the verifier does. https://github.com/isovalent/ebpf-docs/blob/master/tools/helper-ref-gen/main.go

For the kfuncs it works much the same. We have the kfunc gen tool and a separate data file.

But I am not sure if these kfuncs are restricted. They are part of the rqspinlock_kfunc_ids set, which is registered for UNSPEC which means all program types. And there is no additional filter func specified to narrow that down.

What they seem to have done is to restrict maps with values containing spinlocks, and not the usage of the kfunc itself. https://elixir.bootlin.com/linux/v6.18.6/source/kernel/bpf/verifier.c#L20423. So that sort of make documenting challenging. You are technically allowed to use the kfunc, its just that in practice you can't because you will not be able to get a map with a spinlock value loaded.

Not sure how to encode that sort of info in the data files. Perhaps we add an exception layer here as well, and place a comment in the yaml explaining why the data file does not match the func set registration logic.

I imagine this would also be useful for other helpers / kfuncs which can't work due to similar restrictions in check_map_prog_compatibility.

[AliGhaffarian]

But I am not sure if these kfuncs are restricted. They are part of the rqspinlock_kfunc_ids set, which is registered for UNSPEC which means all program types. And there is no additional filter func specified to narrow that down.

I couldn't pinpoint where it is restricted in the source code, but this little program complains as if i used bpf_spin_lock

#include "vmlinux.h"
#include <bpf/bpf_helpers.h>

struct data {
    int data;
    struct bpf_res_spin_lock lock;
};

struct {
    __uint(type, BPF_MAP_TYPE_HASH);
    __uint(max_entries, 1024);
    __type(key, __u32);
    __type(value, struct data);
}test_map SEC(".maps");

SEC("tp/syscalls/sys_enter_execve")
int test_func(void *ctx){
    __u32 zero = 0;
    struct data *elem = bpf_map_lookup_elem(&test_map, &zero);

    if(!elem) return 0;

    bpf_res_spin_lock(&elem->lock);
    bpf_res_spin_unlock(&elem->lock);
    return 0;
}

char __license[] SEC("license") = "GPL";

output of loader:

libbpf: prog 'test_func': BPF program load failed: Invalid argument
libbpf: prog 'test_func': -- BEGIN PROG LOAD LOG --
tracing progs cannot use bpf_spin_lock yet
processed 0 insns (limit 1000000) max_states_per_insn 0 total_states 0 peak_states 0 mark_read 0
-- END PROG LOAD LOG --
libbpf: prog 'test_func': failed to load: -22
libbpf: failed to load object 'test_func'
libbpf: failed to load BPF skeleton 'test_func': -22

[AliGhaffarian]

Not sure how to encode that sort of info in the data files. Perhaps we add an exception layer here as well, and place a comment in the yaml explaining why the data file does not match the func set registration logic.

I imagine this would also be useful for other helpers / kfuncs which can't work due to similar restrictions in check_map_prog_compatibility.

Hi, thanks for the review!

Maybe we can translate check_map_prog_compatibility to go, but not implement the functions it calls, and try the combinations of prog and map that we need. For example, to determine which map types to exclude for sleep-able programs, we can set prog->sleepable to true and then try all of the values within the range of map type enumeration for map->map_type and see if the translated check_map_prog_compatibility returns an error..
This would make the data files more accurate, but probably not 100% accurate.

P.S I'm having a hard time staying connected, i may respond a little late after this

[dylandreimerink]

I couldn't pinpoint where it is restricted in the source code, but this little program complains as if i used bpf_spin_lock

You are hitting https://elixir.bootlin.com/linux/v6.18.6/source/kernel/bpf/verifier.c#L20423. They just did it odd, they check the BTF of your map, and then throw an error for using the helper/kfunc. So in theory if you use the helper / kfunc but not have a lock struct in the map it would continue. Of course you would later give another error since it needs a lock in a map. But they didn't block the helper in the usual way they restrict helpers.

For the purposes of documentation I think we can just say that the helper / kfunc itself is unusable. It is a nuance that is not important to most people. Its just that our tool to generate the docs does not have a way to specify these kinds of exceptions at the moment.

I think the shortest way to do this for the helpers is to add a exclude field to the helperDef and then to modify the code to remove that helper. That way we can exclude the spin lock helpers at the program type level even though they are defined in the base group.

I can make a PR for the tooling part, should be easier for me since I am familiar with how it works, then you can just use it via the data files.

I will look into something similar for kfuncs. In other places they also have these super specific restrictions that apply on top of the normal program type <-> kfunc association. So perhaps we can allow proper documentation of a range of these.

[dylandreimerink]

I opened #252 to supersede this one, added you as co-author on the relevant commits.