From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-22.2 required=3.0 tests=BAYES_00, HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_CR_TRAILER,INCLUDES_PATCH, MAILING_LIST_MULTI,MENTIONS_GIT_HOSTING,NICE_REPLY_A,SPF_HELO_NONE,SPF_PASS, URIBL_BLOCKED,USER_AGENT_SANE_1 autolearn=ham autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 1B97BC433F5 for ; Thu, 9 Sep 2021 21:43:39 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id EB4F26103E for ; Thu, 9 Sep 2021 21:43:38 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1346228AbhIIVor (ORCPT ); Thu, 9 Sep 2021 17:44:47 -0400 Received: from www62.your-server.de ([213.133.104.62]:37508 "EHLO www62.your-server.de" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1345868AbhIIVor (ORCPT ); Thu, 9 Sep 2021 17:44:47 -0400 Received: from sslproxy06.your-server.de ([78.46.172.3]) by www62.your-server.de with esmtpsa (TLSv1.3:TLS_AES_256_GCM_SHA384:256) (Exim 4.92.3) (envelope-from ) id 1mORpX-0005TL-KE; Thu, 09 Sep 2021 23:43:35 +0200 Received: from [85.5.47.65] (helo=linux.home) by sslproxy06.your-server.de with esmtpsa (TLSv1.3:TLS_AES_256_GCM_SHA384:256) (Exim 4.92) (envelope-from ) id 1mORpW-0005Gn-Tz; Thu, 09 Sep 2021 23:43:35 +0200 Subject: Re: [PATCH v3 bpf-next] libbpf: add LIBBPF_DEPRECATED_SINCE macro for scheduling API deprecations To: Andrii Nakryiko Cc: Andrii Nakryiko , bpf , Alexei Starovoitov , Quentin Monnet , Kernel Team References: <20210908213226.1871016-1-andrii@kernel.org> From: Daniel Borkmann Message-ID: <4287e56e-a91a-1f39-5dae-14ecabe537db@iogearbox.net> Date: Thu, 9 Sep 2021 23:43:34 +0200 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:60.0) Gecko/20100101 Thunderbird/60.7.2 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset=utf-8; format=flowed Content-Language: en-US Content-Transfer-Encoding: 7bit X-Authenticated-Sender: daniel@iogearbox.net X-Virus-Scanned: Clear (ClamAV 0.103.2/26289/Thu Sep 9 10:20:35 2021) Precedence: bulk List-ID: X-Mailing-List: bpf@vger.kernel.org On 9/9/21 8:08 PM, Andrii Nakryiko wrote: > On Thu, Sep 9, 2021 at 9:37 AM Andrii Nakryiko > wrote: >> On Thu, Sep 9, 2021 at 5:58 AM Daniel Borkmann wrote: >>> On 9/8/21 11:32 PM, Andrii Nakryiko wrote: >>>> From: Quentin Monnet >>>> >>>> Introduce a macro LIBBPF_DEPRECATED_SINCE(major, minor, message) to prepare >>>> the deprecation of two API functions. This macro marks functions as deprecated >>>> when libbpf's version reaches the values passed as an argument. >>>> >>>> As part of this change libbpf_version.h header is added with recorded major >>>> (LIBBPF_MAJOR_VERSION) and minor (LIBBPF_MINOR_VERSION) libbpf version macros. >>>> They are now part of libbpf public API and can be relied upon by user code. >>>> libbpf_version.h is installed system-wide along other libbpf public headers. >>>> >>>> Due to this new build-time auto-generated header, in-kernel applications >>>> relying on libbpf (resolve_btfids, bpftool, bpf_preload) are updated to >>>> include libbpf's output directory as part of a list of include search paths. >>>> Better fix would be to use libbpf's make_install target to install public API >>>> headers, but that clean up is left out as a future improvement. The build >>>> changes were tested by building kernel (with KBUILD_OUTPUT and O= specified >>>> explicitly), bpftool, libbpf, selftests/bpf, and resolve_btfids builds. No >>>> problems were detected. >>>> >>>> Note that because of the constraints of the C preprocessor we have to write >>>> a few lines of macro magic for each version used to prepare deprecation (0.6 >>>> for now). >>>> >>>> Also, use LIBBPF_DEPRECATED_SINCE() to schedule deprecation of >>>> btf__get_from_id() and btf__load(), which are replaced by >>>> btf__load_from_kernel_by_id() and btf__load_into_kernel(), respectively, >>>> starting from future libbpf v0.6. This is part of libbpf 1.0 effort ([0]). >>>> >>>> [0] Closes: https://github.com/libbpf/libbpf/issues/278 >>>> >>>> Co-developed-by: Quentin Monnet >>>> Signed-off-by: Quentin Monnet >>>> Signed-off-by: Andrii Nakryiko >>>> --- >>>> v2->v3: >>>> - adding `sleep 10` revealed two more missing dependencies in resolve_btfids >>>> and selftest/bpf's bench, which were fixed (BPF CI); >>>> v1->v2: >>>> - fix bpf_preload build by adding dependency for iterators/iterators.o on >>>> libbpf.a generation (caught by BPF CI); >>>> >>>> kernel/bpf/preload/Makefile | 7 +++++-- >>>> tools/bpf/bpftool/Makefile | 4 ++++ >>>> tools/bpf/resolve_btfids/Makefile | 6 ++++-- >>>> tools/lib/bpf/Makefile | 24 +++++++++++++++++------- >>>> tools/lib/bpf/btf.h | 2 ++ >>>> tools/lib/bpf/libbpf_common.h | 19 +++++++++++++++++++ >>>> tools/testing/selftests/bpf/Makefile | 4 ++-- >>>> 7 files changed, 53 insertions(+), 13 deletions(-) >>>> >>>> diff --git a/kernel/bpf/preload/Makefile b/kernel/bpf/preload/Makefile >>>> index 1951332dd15f..ac29d4e9a384 100644 >>>> --- a/kernel/bpf/preload/Makefile >>>> +++ b/kernel/bpf/preload/Makefile >>>> @@ -10,12 +10,15 @@ LIBBPF_OUT = $(abspath $(obj)) >>>> $(LIBBPF_A): >>>> $(Q)$(MAKE) -C $(LIBBPF_SRCS) O=$(LIBBPF_OUT)/ OUTPUT=$(LIBBPF_OUT)/ $(LIBBPF_OUT)/libbpf.a >>>> >>>> -userccflags += -I $(srctree)/tools/include/ -I $(srctree)/tools/include/uapi \ >>>> +userccflags += -I$(LIBBPF_OUT) -I $(srctree)/tools/include/ \ >>>> + -I $(srctree)/tools/include/uapi \ >>>> -I $(srctree)/tools/lib/ -Wno-unused-result >>>> >>>> userprogs := bpf_preload_umd >>>> >>>> -clean-files := $(userprogs) bpf_helper_defs.h FEATURE-DUMP.libbpf staticobjs/ feature/ >>>> +clean-files := $(userprogs) libbpf_version.h bpf_helper_defs.h FEATURE-DUMP.libbpf staticobjs/ feature/ >>>> + >>>> +$(obj)/iterators/iterators.o: $(LIBBPF_A) >>>> >>>> bpf_preload_umd-objs := iterators/iterators.o >>>> bpf_preload_umd-userldlibs := $(LIBBPF_A) -lelf -lz >>> >>> One small issue I ran into by accident while testing: >>> >>> [root@linux bpf-next]# make -j8 kernel/bpf/ >>> SYNC include/config/auto.conf.cmd >>> DESCEND objtool >>> CALL scripts/atomic/check-atomics.sh >>> CALL scripts/checksyscalls.sh >>> CC kernel/bpf/syscall.o >>> AR kernel/bpf/preload/built-in.a >>> CC [M] kernel/bpf/preload/bpf_preload_kern.o >>> CC [U] kernel/bpf/preload/iterators/iterators.o >>> In file included from ./tools/lib/bpf/libbpf.h:20, >>> from kernel/bpf/preload/iterators/iterators.c:10: >>> ./tools/lib/bpf/libbpf_common.h:13:10: fatal error: libbpf_version.h: No such file or directory >>> 13 | #include "libbpf_version.h" >>> | ^~~~~~~~~~~~~~~~~~ >>> compilation terminated. >>> make[3]: *** [scripts/Makefile.userprogs:43: kernel/bpf/preload/iterators/iterators.o] Error 1 >>> make[3]: *** Waiting for unfinished jobs.... >>> make[2]: *** [scripts/Makefile.build:540: kernel/bpf/preload] Error 2 >>> make[2]: *** Waiting for unfinished jobs.... >>> make[1]: *** [scripts/Makefile.build:540: kernel/bpf] Error 2 >>> make: *** [Makefile:1872: kernel] Error 2 >>> >>> For me it was the case where tools/lib/bpf/ was already built _before_ this patch >>> was applied, then I applied it, and just ran make -j8 kernel/bpf/ where the above >>> can then be reproduced. I'd assume that as-is, this would affect many folks on update. >> >> We had a similar issue even before these changes with resolve_btfids >> build, because Kbuild doesn't record dependency on libbpf build >> properly. I'll see how hard it is to record that in a non-intrusive >> way for both resolve_btfids and preload/iterators, because doing `make >> resolve_btfids_clean` or, worse, `make clean` isn't great. > > Hm... I can't repro this. Both preload/iterators and resolve_btfids > have $(wildcard $(LIBBPF_SRC)/*.[ch] $(LIBBPF_SRC)/Makefile) > dependency which will trigger libbpf rebuild if any of libbpf source > files changes. I added `sleep 1000` and everything under kernel/bpf is > blocked on this until libbpf build completes. For full kernel build it > also shouldn't happen because we build resolve_btfids first before > proceeding to building other files and resolve_btfids should trigger > libbpf build. > > I assume `make clean kernel/bpf` fixed the issue for you in that case? No, fixing on that node was done via re-building libbpf via tools/lib/bpf/. > Also, how did you build tools/lib/bpf before running `make -j8 kernel/bpf`? Basically just i) cd tools/lib/bpf, ii) make clean, iii) make, then applying the patch, and then running make -j8 kernel/bpf. I just tried on a different build node with newer user space, and there it indeed triggers a rebuild of libbpf. On my laptop where I was running into this it didn't. I would suspect a potential diff in make's behavior (not seen in GNU Make 4.3, seen in GNU Make 4.2.1, fwiw). Anyway, for this niche case, manual rebuild of tools/lib/bpf does it ... given it's neither seen in your case nor on newer uspace, I took it in, lets see how it goes. Thanks! > Honestly, hard to tell how this happens without being able to repro > this. So far everything seems to be working for me. I've tried with > and without the KBUILD_OUTPUT envvar set. I also tried O=. > >>> >>> Thanks, >>> Daniel