On Thu Jul 16, 2026 at 6:09 PM -03, Ihor Solodrai wrote:
> 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 <[email protected]>
>>
>> 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/[email protected]/
>>
>>> ---
>>> 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?
I'll check and send v2 accordingly
>
> 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) \
>>
>>
>>
>>
