Re: [PATCH bpf-next v6 2/9] selftests/bpf: drop stale and no-op lines

> selftests/bpf: drop stale and no-op lines
>
> Four target-specific lines name objects nothing builds. Three went
> dead when commit afef88e65554 ("selftests/bpf: Store BPF object files
> with .bpf.o extension") renamed the BPF objects; the
> flow_dissector_load.o dependency names an intermediate the generic
> one-step compile+link rule does not produce.
>
> Three linked-skeleton dependency map entries (xsk_xdp_progs,
> xdp_hw_metadata, xdp_features) were dead on arrival: commit
> f0a249df1b07 ("selftests/xsk: get rid of built-in XDP program"),
> commit 297a3f124155 ("selftests/bpf: Simple program to dump XDP RX
> metadata") and commit 4dba3e7852b7 ("selftests/bpf: introduce XDP
> compliance test tool") each added one, mimicking the neighboring
> entries without adding the skeleton to LINKED_SKELS, the only place
> the -deps map is read from; these three skeletons are generated by
> the regular pattern rule, which never consults it.
>
> The '-fno-inline' pair (test_l4lb_noinline, test_xdp_noinline)
> applied the flag until the rename; both programs have compiled
> without it since, and nothing was lost: every function in them
> carries a noinline annotation (the single __always_inline helper in
> each is a deliberate exception the flag never overrode), and
> compiling with -fno-inline restored yields byte-identical objects.
>
> The two header dependencies with remaining value are restored in
> working form. flow_dissector_load.h moves to the binary itself, which
> is linked straight from its .c by the generic '$(OUTPUT)/%:%.c' rule
> - editing the header now rebuilds it. cgroup_getset_retval_hooks.h -
> added by commit e7215f574079 ("selftests/bpf: Make sure
> bpf_{g,s}et_retval is exposed everywhere") days before the rename
> orphaned it - lived in the top directory, outside the progs/*.h
> blanket prerequisite of the BPF object rules, so editing it never
> rebuilt the BPF object. Move it under progs/, next to its only BPF
> consumer, where the blanket prerequisite covers it - the arrangement
> several other headers shared between progs/ and prog_tests/ already
> use. Its userspace consumer is tracked exactly by the
> compiler-generated dependency files and now includes it by its new
> path.
>
> 'CURDIR := $(abspath .)' redefines make's builtin to the value it
> already has, and 'OBJCOPY ?= $(CROSS_COMPILE)objcopy' defines a
> variable nothing in the selftests build or the included kselftest
> infrastructure ever reads; drop both.
>
> Signed-off-by: Mykola Lysenko <[email protected]>

This isn't a bug, but would it read more clearly to split the
cgroup_getset_retval_hooks.h move (and its include update) into its own
patch, or to widen the subject so the additions are visible from the
shortlog? The subject describes the patch purely as a removal ('drop stale
and no-op lines'), but the diff also adds a new prerequisite and renames a
tracked file across directories.

> diff --git a/tools/testing/selftests/bpf/Makefile b/tools/testing/selftests/bpf/Makefile
> index 118a7c356e26..8d71345c2e86 100644
> --- a/tools/testing/selftests/bpf/Makefile
> +++ b/tools/testing/selftests/bpf/Makefile

[ ... ]

> @@ -480,11 +478,7 @@ BPF_CFLAGS = -g -Wall -Werror -D__TARGET_ARCH_$(SRCARCH) $(MENDIAN)	\
>
>  CLANG_CFLAGS = $(CLANG_SYS_INCLUDES)
>
> -$(OUTPUT)/test_l4lb_noinline.o: BPF_CFLAGS += -fno-inline
> -$(OUTPUT)/test_xdp_noinline.o: BPF_CFLAGS += -fno-inline
> -
> -$(OUTPUT)/flow_dissector_load.o: flow_dissector_load.h
> -$(OUTPUT)/cgroup_getset_retval_hooks.o: cgroup_getset_retval_hooks.h
> +$(OUTPUT)/flow_dissector_load: flow_dissector_load.h
>
>  # Build BPF object using Clang
>  # $1 - input .c file

This isn't a bug, but could the flow_dissector_load prerequisite join
the existing '$(OUTPUT)/flow_dissector_load: $(TESTING_HELPERS)'
declaration in the earlier 'override lib.mk's default rules' section,
so both prerequisites for that binary sit together instead of one
landing in the clang CFLAGS block?

[ ... ]

> diff --git a/tools/testing/selftests/bpf/prog_tests/cgroup_getset_retval.c b/tools/testing/selftests/bpf/prog_tests/cgroup_getset_retval.c
> index 2bb5773d6f99..ea342859d6ce 100644
> --- a/tools/testing/selftests/bpf/prog_tests/cgroup_getset_retval.c
> +++ b/tools/testing/selftests/bpf/prog_tests/cgroup_getset_retval.c
> @@ -465,7 +465,7 @@ struct exposed_hook {
>  		.expected_err = EXPECTED_ERR, \
>  	},
>
> -#include "cgroup_getset_retval_hooks.h"
> +#include "progs/cgroup_getset_retval_hooks.h"
>
>  #undef BPF_RETVAL_HOOK
>  };
> diff --git a/tools/testing/selftests/bpf/cgroup_getset_retval_hooks.h b/tools/testing/selftests/bpf/progs/cgroup_getset_retval_hooks.h
> similarity index 100%
> rename from tools/testing/selftests/bpf/cgroup_getset_retval_hooks.h
> rename to tools/testing/selftests/bpf/progs/cgroup_getset_retval_hooks.h


---
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/32610372822