Re: [PATCH 1/4] selftests/bpf: Install bpftool where test_progs expects to find it

[Ihor Solodrai]

On 7/13/26 8:32 AM, Alexis Lothoré wrote:
> Hi,
> 
> On Sat Jul 11, 2026 at 12:09 AM CEST, Ricardo B. Marlière wrote:
>> bpftool_helpers.c looks for bpftool at "tools/sbin/bpftool" or
>> "../tools/sbin/bpftool" relative to the running test_progs binary, matching
>> the build directory layout where bpftool sits at
>> $(OUTPUT)/tools/sbin/bpftool.
>>
>> INSTALL_RULE installs bpftool through the generic TEST_GEN_PROGS_EXTENDED
>> mechanism, which flattens it straight into $(INSTALL_PATH) and loses the
>> tools/sbin/ prefix. After "make install", every flavor of test_progs,
>> including the default one, fails to locate bpftool, and bpftool_maps_access
>> and bpftool_metadata error out with "Failed to detect bpftool path", even
>> though bpftool itself built and installed fine.
>>
>> Explicitly install bpftool under tools/sbin/ in the installed tree
>> so its relative location matches what detect_bpftool_path() already
>> expects, restoring the pre-install layout instead of introducing a
>> new one.
>>
>> Fixes: f21fae577446 ("selftests/bpf: Add a few helpers for bpftool testing")
>> Signed-off-by: Ricardo B. Marlière <rbm@xxxxxxxx>
> 
> I did not face the issue because I am running the selftests in-tree, and
> the CI seems to do the same, but for users using selftests after running
> `make install`, I guess the change makes sense.
> 
> That's not the first fix aiming to improve bpftool
> detection/installation (eg the BPFTOOL env variable, see [1]), so it
> makes me wonder if we shouldn't just make bpftool part of
> TRUNNER_EXTRA_FILES for all flavours of test_progs, similarly to other
> binaries needed during tests (eg: urandom_read), instead of just adding
> some bpftool-specific details in this generic install rule, and just
> make the test runners assume that the needed binary is in the same
> directory.

Alexis, using TRUNNER_EXTRA_FILES is a bit worse because that would
copy bpftool into every flavor subdir. I think just doing a single
rsync on install is fine.

> 
> Alexis
> 
> [1] https://lore.kernel.org/all/20260223191118.655185-2-ihor.solodrai@xxxxxxxxx/
> 
>> ---
>>  tools/testing/selftests/bpf/Makefile | 2 ++
>>  1 file changed, 2 insertions(+)
>>
>> diff --git a/tools/testing/selftests/bpf/Makefile b/tools/testing/selftests/bpf/Makefile
>> index b642ee489ea6..47911c9b4977 100644
>> --- a/tools/testing/selftests/bpf/Makefile
>> +++ b/tools/testing/selftests/bpf/Makefile
>> @@ -1055,6 +1055,8 @@ endif
>>  DEFAULT_INSTALL_RULE := $(INSTALL_RULE)
>>  override define INSTALL_RULE
>>  	$(DEFAULT_INSTALL_RULE)
>> +	@mkdir -p $(INSTALL_PATH)/tools/sbin
>> +	@rsync -a $(if $(PERMISSIVE),--ignore-missing-args) $(TRUNNER_BPFTOOL) $(INSTALL_PATH)/tools/sbin/


It looks like we still have a redundant copy of bpftool because of:

selftests/bpf/Makefile:327

        TEST_GEN_PROGS_EXTENDED += $(TRUNNER_BPFTOOL)

selftets/lib.mk:153

        $(eval INSTALL_LIST = $(TEST_GEN_PROGS_EXTENDED)) $(INSTALL_SINGLE_RULE)

Ricardo, does it makes sense to remove it with your fix?

diff --git a/tools/testing/selftests/bpf/Makefile b/tools/testing/selftests/bpf/Makefile
index 7260d672acf7..c776913bc32f 100644
--- a/tools/testing/selftests/bpf/Makefile
+++ b/tools/testing/selftests/bpf/Makefile
@@ -324,8 +324,6 @@ TRUNNER_BPFTOOL := $(DEFAULT_BPFTOOL)
 USE_BOOTSTRAP := "bootstrap/"
 endif
 
-TEST_GEN_PROGS_EXTENDED += $(TRUNNER_BPFTOOL)
-
 $(TEST_GEN_PROGS) $(TEST_GEN_PROGS_EXTENDED): $(BPFOBJ)
 
 TESTING_HELPERS        := $(OUTPUT)/testing_helpers.o

>>  	@for DIR in $(TEST_INST_SUBDIRS); do				  \
>>  		mkdir -p $(INSTALL_PATH)/$$DIR;				  \
>>  		rsync -a $(if $(PERMISSIVE),--ignore-missing-args)	  \
> 
> 
> 
>