On Thursday, July 18th, 2024 at 8:34 AM, Andrii Nakryiko <andrii.nakryiko@xxxxxxxxx> wrote: [...] > > > > - by adding a catch-all clause in the makefile, e.g. making test > > > > runner depend on all .bpf.o files. > > > > > > do we actually need to rebuild final binary if we are still just > > > loading .bpf.o from disk? We are not embedding such .bpf.o (embedding > > > is what skeleton headers are adding), so why rebuild .bpf.o? > > > > > > Actually thinking about this again, I guess, if we don't want to add > > > skel.h to track precise dependencies, we don't really need to do > > > anything extra for those progs/*.c files that are not used through > > > skeletons. We just need to make sure that they are rebuilt if they are > > > changed. The rest will work as is because test runner binary will just > > > load them from disk at the next run (and user space part doesn't have > > > to be rebuilt, unless it itself changed). > > > > Good point. This can be achieved by making $(OUTPUT)/$(TRUNNER_BINARY) > > dependency on $(TRUNNER_BPF_OBJS) order-only, e.g. here is a modified > > version of the v2: https://tinyurl.com/4wnhkt32 > > > +1 I agree. I'll submit v4 with this change. > > [...] > > > > > another side benefit of completely switching to .skel.h is that we can > > > stop copying all .bpf.o files into BPF CI, because test_progs will be > > > self-contained (thought that's not 100% true due to btf__* and maybe a > > > few files more, which is sad and a bit different problem) > > > > Hm, this might make sense. > > There are 410Mb of .bpf.o files generated currently. > > On the other hand, as you note, one would still need a list of some > > .bpf.o files, because there are at-least several tests that verify > > operation on ELF files, not ELF bytes. This seems worthwhile to look into, although I think it's a task independent of this patch. > > [...] > > > > > keep in mind that we do want to rebuild .bpf.o if libbpf's BPF-side > > > headers changed, so let's make sure that stays (or happens, if we > > > don't do it already) > > > > Commands below cause full rebuild (.test.o, .bpf.o) on v2 of this > > patch-set: > > $ touch tools/lib/bpf/bpf.h > > $ touch tools/lib/bpf/libbpf.h > > > yeah, ideally they wouldn't cause bpf.o rebuilds... I think we should > tune .bpf.o to depend only on BPF-side headers (we'd need to hard-code > them, but they don't change often: usdt.bpf.h, bpf_tracing.h, > bpf_helpers.h, etc). I don't think we can get rid of BPF skeletons' > dependency on bpftool (which depends on any libbpf change), though, > so .skel.h will be regenerated due to any tiny libbpf change, but > that's still better, as bpf.o building is probably the slowest part. I tried a small experiment, and specifying particular lib/bpf headers didn't help because of vmlinux.h I grepped the list of headers with: $ grep -rh 'include <bpf/' progs | sort -u #include <bpf/bpf_core_read.h> #include <bpf/bpf_endian.h> #include <bpf/bpf_helpers.h> #include <bpf/bpf_tracing.h> #include <bpf/usdt.bpf.h> Then, changed $(TRUNNER_BPF_OBJS) dependencies like this: diff --git a/tools/testing/selftests/bpf/Makefile b/tools/testing/selftests/bpf/Makefile index 66478446af9d..6fb03bb9b33a 100644 --- a/tools/testing/selftests/bpf/Makefile +++ b/tools/testing/selftests/bpf/Makefile @@ -480,6 +480,13 @@ xdp_features.skel.h-deps := xdp_features.bpf.o LINKED_BPF_OBJS := $(foreach skel,$(LINKED_SKELS),$($(skel)-deps)) LINKED_BPF_SRCS := $(patsubst %.bpf.o,%.c,$(LINKED_BPF_OBJS)) +HEADERS_FOR_BPF_OBJS := bpf_core_read.h \ + bpf_endian.h \ + bpf_helpers.h \ + bpf_tracing.h \ + usdt.bpf.h +HEADERS_FOR_BPF_OBJS := $(addprefix $(BPFDIR)/,$(HEADERS_FOR_BPF_OBJS)) + # Set up extra TRUNNER_XXX "temporary" variables in the environment (relies on # $eval()) and pass control to DEFINE_TEST_RUNNER_RULES. # Parameters: @@ -530,14 +537,15 @@ $(TRUNNER_BPF_OBJS): $(TRUNNER_OUTPUT)/%.bpf.o: \ $(TRUNNER_BPF_PROGS_DIR)/%.c \ $(TRUNNER_BPF_PROGS_DIR)/*.h \ $$(INCLUDE_DIR)/vmlinux.h \ - $(wildcard $(BPFDIR)/bpf_*.h) \ - $(wildcard $(BPFDIR)/*.bpf.h) \ + $(HEADERS_FOR_BPF_OBJS) \ | $(TRUNNER_OUTPUT) $$(BPFOBJ) $$(call $(TRUNNER_BPF_BUILD_RULE),$$<,$$@, \ $(TRUNNER_BPF_CFLAGS) \ $$($$<-CFLAGS) \ $$($$<-$2-CFLAGS)) This didn't help because $ touch ~/work/kernel-clean/tools/lib/bpf/bpf.h triggers rebuild of vmlinux.h, which depends on $(BPFTOOL), and bpftool depends on $(HOST_BPFOBJ) or $(BPFOBJ), and they in turn depend on all files in lib/bpf. And there is a direct dependency of $(TRUNNER_BPF_OBJS) on vmlinux.h, which looks like a real dependency to me, but maybe I don't know something.
