On Sun, Apr 28, 2024 at 12:03 PM Jose E. Marchesi <jose.marchesi@xxxxxxxxxx> wrote: > > > >> Also please check CI failures ([0]). > >> > >> [0] https://github.com/kernel-patches/bpf/actions/runs/8846180836/job/24291582343 > > > > How weird. This means something is going on in my local testing > > environment. > > Ok, I think I know what is going on: the CI failures had nothing to do > with the patch changes per-se, but with the fact the patch changes > bpf_tracing.h and a little problem in the build system. > > If I change tools/lib/bpf/bpf_tracing.h in bpf-next master, then > execute: > > $ cd bpf-next/ > $ git clean -xf > $ cd tools/testing/selftests/bpf/ > $ ./vmtest.sh -- ./test_progs > > in tools/testing/sefltests/bpf, I get this: > > make[2]: *** No rule to make target '/home/jemarch/gnu/src/bpf-next/tools/testing/selftests/bpf/tools/build/libbpflibbpfbpf_helper_defs.h', needed by '/home/jemarch/gnu/src/bpf-next/tools/testing/selftests/bpf/tools/build/libbpf/include/bpf/libbpfbpf_helper_defs.h'. Stop. > > > Same thing happens if I have a built tree and I do `make' in > tools/testing/selftests/bpf. > > In tools/lib/bpf/Makefile there is: > > BPF_HELPER_DEFS := $(OUTPUT)bpf_helper_defs.h > > which assumes OUTPUT always has a trailing slash, which seems to be a > common expectation for OUTPUT among all the Makefiles. > > In tools/bpf/runqslower/Makefile we find: > > BPFTOOL_OUTPUT := $(OUTPUT)bpftool/ > DEFAULT_BPFTOOL := $(BPFTOOL_OUTPUT)bootstrap/bpftool > [...] > $(BPFOBJ): $(wildcard $(LIBBPF_SRC)/*.[ch] $(LIBBPF_SRC)/Makefile) | $(BPFOBJ_OUTPUT) > $(Q)$(MAKE) $(submake_extras) -C $(LIBBPF_SRC) OUTPUT=$(BPFOBJ_OUTPUT) \ > DESTDIR=$(BPFOBJ_OUTPUT) prefix= $(abspath $@) install_headers > > which is ok because BPFTOOL_OUTPUT is defined with a trailing slash. > > However in tools/testing/selftests/bpf/Makefile an explicit value for > BPFTOOL_OUTPUT is specified, that lacks a trailing slash: > > $(OUTPUT)/runqslower: $(BPFOBJ) | $(DEFAULT_BPFTOOL) $(RUNQSLOWER_OUTPUT) > $(Q)$(MAKE) $(submake_extras) -C $(TOOLSDIR)/bpf/runqslower \ > OUTPUT=$(RUNQSLOWER_OUTPUT) VMLINUX_BTF=$(VMLINUX_BTF) \ > BPFTOOL_OUTPUT=$(HOST_BUILD_DIR)/bpftool/ \ > BPFOBJ_OUTPUT=$(BUILD_DIR)/libbpf \ > BPFOBJ=$(BPFOBJ) BPF_INCLUDE=$(INCLUDE_DIR) \ > EXTRA_CFLAGS='-g $(OPT_FLAGS) $(SAN_CFLAGS)' \ > EXTRA_LDFLAGS='$(SAN_LDFLAGS)' && \ > cp $(RUNQSLOWER_OUTPUT)runqslower $@ > > This results in a malformed > > BPF_HELPER_DEFS := $(OUTPUT)bpf_helper_defs.h > > in tools/lib/bpf/Makefile. > > The patch below fixes this, but there are other many possible fixes > (like changing tools/bpf/runqslower/Makefile in order to pass > OUTPUT=$(BPFOBJ_OUTPUT)/, or changing tools/lib/bpf/Makefile to use > $(OUTPUT)/bpf_helper_defs.h) and I don't know which one you would > prefer. > > Also, since the involved rules have not been changed recently, I am > wondering why this is being noted only now. Is people using another > set-up/workflow that somehow doesn't trigger this? Let's fix runqslower submake rule, yes, but I think it's irrelevant here. Failures that CI caught were in samples/bpf (samples/bpf/tracex2.bpf.c), while this is runqslower rule. The reason you haven't caught it is because selftests/bpf/Makefile doesn't build samples/bpf, but our BPF CI does have an extra step to build samples/bpf. > > diff --git a/tools/testing/selftests/bpf/Makefile b/tools/testing/selftests/bpf/Makefile > index ca8b73f7c774..665a5c1e9b8e 100644 > --- a/tools/testing/selftests/bpf/Makefile > +++ b/tools/testing/selftests/bpf/Makefile > @@ -274,7 +274,7 @@ $(OUTPUT)/runqslower: $(BPFOBJ) | $(DEFAULT_BPFTOOL) $(RUNQSLOWER_OUTPUT) > $(Q)$(MAKE) $(submake_extras) -C $(TOOLSDIR)/bpf/runqslower \ > OUTPUT=$(RUNQSLOWER_OUTPUT) VMLINUX_BTF=$(VMLINUX_BTF) \ > BPFTOOL_OUTPUT=$(HOST_BUILD_DIR)/bpftool/ \ > - BPFOBJ_OUTPUT=$(BUILD_DIR)/libbpf \ > + BPFOBJ_OUTPUT=$(BUILD_DIR)/libbpf/ \ > BPFOBJ=$(BPFOBJ) BPF_INCLUDE=$(INCLUDE_DIR) \ > EXTRA_CFLAGS='-g $(OPT_FLAGS) $(SAN_CFLAGS)' \ > EXTRA_LDFLAGS='$(SAN_LDFLAGS)' && \
