Re: [PATCH bpf-next] bpf: avoid casts from pointers to enums in bpf_tracing.h

["Jose E. Marchesi" <jose.marchesi@oracle.com>]

> On Sun, Apr 28, 2024 at 12:03 PM Jose E. Marchesi
> <jose.marchesi@oracle.com> 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.

Ok good to know.  Then we better start building it too as part of our
local testing.

Will look at what is going on there with this fix, and will also send a
separated patch for the Makefile.

Thanks.

>
>>
>> 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)' &&                          \