On 7/1/25 22:28, Andrii Nakryiko wrote: > On Tue, Jul 1, 2025 at 12:50 PM Alexei Starovoitov > <alexei.starovoitov@xxxxxxxxx> wrote: >> >> On Tue, Jul 1, 2025 at 12:43 PM Viktor Malik <vmalik@xxxxxxxxxx> wrote: >>> >>> On 7/1/25 19:46, Alexei Starovoitov wrote: >>>> On Mon, Jun 30, 2025 at 6:35 AM Viktor Malik <vmalik@xxxxxxxxxx> wrote: >>>>> >>>>> BPF selftests compilation fails on systems with CONFIG_NET_SCH_BPF=n. >>>>> The reason is that qdisc-related kfuncs are included via vmlinux.h but >>>>> when qdisc is disabled, they are not defined and do not appear in >>>>> vmlinux.h. >>>> >>>> Yes and that's expected behavior. It's not a bug. >>>> That's why we have CONFIG_NET_SCH_BPF=y in >>>> selftests/bpf/config >>>> and CI picks it up automatically. >>>> >>>> If we add these kfuncs to bpf_qdisc_common.h where would we >>>> draw the line when the kfuncs should be added or not ? >>> >>> I'd say that we should add kfuncs which are only included in vmlinux.h >>> under certain configurations. Obviously stuff like CONFIG_BPF=y can be >>> presumed but there're tons of configs options which may be disabled on a >>> system and it still makes sense to compile and run at least a part of >>> test_progs on them. >>> >>>> Currently we don't add any new kfuncs, since they all >>>> should be in vmlinux.h >>> >>> This way, we're preventing people to build and therefore run *any* >>> test_progs on systems which do not have all the configs required in >>> selftests/bpf/config. Running selftests on such systems may reveal bugs >>> not captured by the CI so I think that it may be eventually beneficial >>> for everyone. >> >> Not quite. What's stopping people to build selftests >> with 'make -k' ? >> Some bpf progs will not compile, but test_progs binary will be built and >> it will run the rest of the tests. I don't think test_progs will be built if some of the objects from progs/ do not build. I just tried to run `make -k` on a kernel with CONFIG_NET_SCH_BPF=n. The compilation finished, some test binaries exist, but not test_progs. In addition, we generally don't want to ignore all the errors as some of them may be important. >> >> We can take this patch, but let's define the rules for adding >> kfuncs explicitly. > > Note, we have a VMLINUX_H argument that can be passed into BPF > selftests' makefile. We used to use this for libbpf CI to build latest > selftests against (very) old kernels, and it worked well. > > I don't think we need to make exceptions for a few kfuncs, all it > takes is to have vmlinux.h generated from kernel image built from > proper configuration. > > Also note, that "proper configuration" only applies to *built* kernel, > not the actually running host kernel. See how VMLINUX_BTF_PATHS is > defined and handled: host kernel is the last thing we use for > vmlinux.h generation, only if all other options are unavailable. This is a good point but the problem here is the extra kernel build. If you want to check that BPF in your kernel is working properly, you don't want to do another kernel build with a different config just for the sake of being able to build selftests. >> What are you proposing exactly ? >> Anything that is gated by some CONFIG_FOO _must_ be added explicitly ? >> Assuming we won't be going back and retroactively adding them ? Yes, exactly like that. Except for this qdisc one, we haven't run into any issues for a long time, so I don't think that it's necessary to retroactively add the kfuncs. But if you prefer to have it unified, I can take some time to clean it up - add config-gated kfuncs where they are missing and replace universally-available kfuncs by vmlinux.h.
