netdev - Re: [PATCH bpf-next] bpf/preload: Clean up .gitignore and "clean-files" target

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Date:   Wed, 20 Oct 2021 10:30:09 +0100
From:   Quentin Monnet <quentin@...valent.com>
To:     Andrii Nakryiko <andrii.nakryiko@...il.com>
Cc:     Alexei Starovoitov <ast@...nel.org>,
        Daniel Borkmann <daniel@...earbox.net>,
        Andrii Nakryiko <andrii@...nel.org>,
        Networking <netdev@...r.kernel.org>, bpf <bpf@...r.kernel.org>
Subject: Re: [PATCH bpf-next] bpf/preload: Clean up .gitignore and
 "clean-files" target

2021-10-19 16:50 UTC-0700 ~ Andrii Nakryiko <andrii.nakryiko@...il.com>
> On Sat, Oct 9, 2021 at 5:24 PM Quentin Monnet <quentin@...valent.com> wrote:
>>
>> kernel/bpf/preload/Makefile was recently updated to have it install
>> libbpf's headers locally instead of pulling them from tools/lib/bpf. But
>> two items still need to be addressed.
>>
>> First, the local .gitignore file was not adjusted to ignore the files
>> generated in the new kernel/bpf/preload/libbpf output directory.
>>
>> Second, the "clean-files" target is now incorrect. The old artefacts
>> names were not removed from the target, while the new ones were added
>> incorrectly. This is because "clean-files" expects names relative to
>> $(obj), but we passed the absolute path instead. This results in the
>> output and header-destination directories for libbpf (and their
>> contents) not being removed from kernel/bpf/preload on "make clean" from
>> the root of the repository.
>>
>> This commit fixes both issues. Note that $(userprogs) needs not be added
>> to "clean-files", because the cleaning infrastructure already accounts
>> for it.
>>
>> Cleaning the files properly also prevents make from printing the
>> following message, for builds coming after a "make clean":
>> "make[4]: Nothing to be done for 'install_headers'."
>>
>> Fixes: bf60791741d4 ("bpf: preload: Install libbpf headers when building")
>> Signed-off-by: Quentin Monnet <quentin@...valent.com>
>> ---
>>  kernel/bpf/preload/.gitignore | 4 +---
>>  kernel/bpf/preload/Makefile   | 3 +--
>>  2 files changed, 2 insertions(+), 5 deletions(-)
>>
>> diff --git a/kernel/bpf/preload/.gitignore b/kernel/bpf/preload/.gitignore
>> index 856a4c5ad0dd..9452322902a5 100644
>> --- a/kernel/bpf/preload/.gitignore
>> +++ b/kernel/bpf/preload/.gitignore
>> @@ -1,4 +1,2 @@
>> -/FEATURE-DUMP.libbpf
>> -/bpf_helper_defs.h
>> -/feature
>> +/libbpf
>>  /bpf_preload_umd
>> diff --git a/kernel/bpf/preload/Makefile b/kernel/bpf/preload/Makefile
>> index 469d35e890eb..d8379af88161 100644
>> --- a/kernel/bpf/preload/Makefile
>> +++ b/kernel/bpf/preload/Makefile
>> @@ -27,8 +27,7 @@ userccflags += -I $(srctree)/tools/include/ -I $(srctree)/tools/include/uapi \
>>
>>  userprogs := bpf_preload_umd
>>
>> -clean-files := $(userprogs) bpf_helper_defs.h FEATURE-DUMP.libbpf staticobjs/ feature/
>> -clean-files += $(LIBBPF_OUT) $(LIBBPF_DESTDIR)
>> +clean-files := $(subst $(abspath $(obj))/,,$(LIBBPF_OUT) $(LIBBPF_DESTDIR))
> 
> why so complicated? also isn't LIBBPF_OUT and LIBBPF_DESTDIR the same?

They're the same. My reasoning was that since we had these two variables
for output directories, it felt logical somehow to add both, in case one
changes in the future.

> Wouldn't just this work and be super clear:
> 
> clean-files: libbpf/

It does look simpler. OK I'll change and submit a new version.

Thanks,
Quentin