Re: [PATCH v3 3/4] selftests/hid: build the BPF program via the shared lib.bpf.mk

[bot+bpf-ci@kernel.org]

[-- Attachment #1: Type: text/plain, Size: 3631 bytes --]

> The generated hid.skel.h public API is same before and after, hid_bpf
> and hidraw are built and linked the same way.

This isn't a bug, but could the changelog mention that the BPF compile
flags change with the shared fragment (added -Wall/-std=gnu11/
-fno-strict-aliasing, added include paths, auto-detected -mcpu instead
of a hardcoded v3) rather than describing the build as unchanged?

The flags used to compile progs/hid.c do change: the shared BPF_CFLAGS
add -Wall, -std=gnu11, -fno-strict-aliasing, -I$(APIDIR)
-I$(TOOLSINCDIR) and the clang system-include set (which the old
Makefile computed into CLANG_SYS_INCLUDES but never passed), and -mcpu
becomes auto-detected (v3/v2) instead of hardcoded -mcpu=v3. The
-Wno-unused-variable line the patch has to add is direct evidence that
the compile flags are not identical.

> diff --git a/tools/testing/selftests/hid/.gitignore b/tools/testing/selftests/hid/.gitignore

[ ... ]

> diff --git a/tools/testing/selftests/hid/Makefile b/tools/testing/selftests/hid/Makefile
> index 2f423de83147..1d7c64ec992c 100644
> --- a/tools/testing/selftests/hid/Makefile
> +++ b/tools/testing/selftests/hid/Makefile

[ ... ]

> +# progs/hid.c is built into a skeleton by ../lib.bpf.mk.  hid keeps the legacy
> +# progs/<name>.c layout, hence BPF_PROG_EXT.  BPF_EXTRA_HDRS is deferred: it
> +# uses $(BPFDIR), which lib.bpf.mk defines.
> +BPF_SRCS       := $(wildcard progs/*.c)
> +BPF_PROG_EXT   := .c
> +BPF_EXTRA_HDRS  = $(wildcard progs/*.h) $(wildcard $(BPFDIR)/hid_bpf_*.h) \
> +		  $(wildcard $(BPFDIR)/*.bpf.h)
> +# hid.c predates the shared -Wall and has a few unused locals.
> +BPF_EXTRA_CFLAGS := -Wno-unused-variable

This isn't a bug, but would dropping the four unused locals in
progs/hid.c (the `int i` in the three hid_user_* syscall programs and
the unused `int ret` in hid_test_hidraw_output_report) let this
directory build without the -Wno-unused-variable override?

The shared lib.bpf.mk compiles BPF objects with -Wall -Werror, and this
patch reacts by permanently disabling -Wunused-variable for every BPF
program in this directory. The trigger is exactly four dead locals:
`int i` in hid_user_raw_request(), hid_user_output_report() and
hid_user_input_report(), plus an `int ret;` in
hid_test_hidraw_output_report() that is never assigned or read.
Deleting those four declarations would let the directory build with the
shared warning set unmodified, and would keep the suppression from
silently hiding unused locals in BPF programs added here later.

[ ... ]

> +EXTRA_CLEAN += feature bpftool $(addprefix $(OUTPUT)/,*.o no_alu32)

This isn't a bug, but since the no_alu32 build rule goes away in this
patch, is the `no_alu32` (and `feature`) entry in EXTRA_CLEAN still
needed, or could this line shrink to just `$(OUTPUT)/*.o`?

The rewritten EXTRA_CLEAN keeps the `no_alu32` entry, but this patch
removes the only thing that could ever have produced such an artifact:
the CLANG_NOALU32_BPF_BUILD_RULE definition (and hid never had the
selftests/bpf per-flavor subdirectory machinery that creates a
no_alu32/ tree in the first place). Now that the flavor rule is gone,
nothing in this directory can create $(OUTPUT)/no_alu32. The `feature`
entry is in a similar position - libbpf/bpftool feature-detection
output lands under $(BUILD_DIR)/libbpf/, which is already covered by
the $(SCRATCH_DIR) entry that lib.bpf.mk adds.


---
AI reviewed your patch. Please fix the bug or email reply why it's not a bug.
See: https://github.com/kernel-patches/vmtest/blob/master/ci/claude/README.md

CI run summary: https://github.com/kernel-patches/bpf/actions/runs/31820214629