Message-ID: <CAErzpmsoeFJBhqXZF1ttUCDx5HSFVawdiVfsG2vWSOq4DBBruQ@mail.gmail.com>
Date: Tue, 2 Dec 2025 10:01:30 +0800
From: Donglin Peng <dolinux.peng@...il.com>
To: Ihor Solodrai <ihor.solodrai@...ux.dev>
Cc: Alexei Starovoitov <ast@...nel.org>, Daniel Borkmann <daniel@...earbox.net>, 
	Andrii Nakryiko <andrii@...nel.org>, Martin KaFai Lau <martin.lau@...ux.dev>, 
	Eduard Zingerman <eddyz87@...il.com>, Song Liu <song@...nel.org>, 
	Yonghong Song <yonghong.song@...ux.dev>, John Fastabend <john.fastabend@...il.com>, 
	KP Singh <kpsingh@...nel.org>, Stanislav Fomichev <sdf@...ichev.me>, Hao Luo <haoluo@...gle.com>, 
	Jiri Olsa <jolsa@...nel.org>, Nathan Chancellor <nathan@...nel.org>, 
	Nicolas Schier <nicolas.schier@...ux.dev>, 
	Nick Desaulniers <nick.desaulniers+lkml@...il.com>, Bill Wendling <morbo@...gle.com>, 
	Justin Stitt <justinstitt@...gle.com>, Alan Maguire <alan.maguire@...cle.com>, bpf@...r.kernel.org, 
	dwarves@...r.kernel.org, linux-kernel@...r.kernel.org, 
	linux-kbuild@...r.kernel.org
Subject: Re: [PATCH bpf-next v2 4/4] resolve_btfids: change in-place update
 with raw binary output

On Tue, Dec 2, 2025 at 3:46 AM Ihor Solodrai <ihor.solodrai@...ux.dev> wrote:
>
> On 11/27/25 9:52 PM, Ihor Solodrai wrote:
> > On 11/27/25 7:20 PM, Donglin Peng wrote:
> >> On Fri, Nov 28, 2025 at 2:53 AM Ihor Solodrai <ihor.solodrai@...ux.dev> wrote:
> >>>
> >>> [...]
> >>>
> >>> diff --git a/tools/testing/selftests/bpf/Makefile b/tools/testing/selftests/bpf/Makefile
> >>> index bac22265e7ff..ec7e2a7721c7 100644
> >>> --- a/tools/testing/selftests/bpf/Makefile
> >>> +++ b/tools/testing/selftests/bpf/Makefile
> >>> @@ -4,6 +4,7 @@ include ../../../scripts/Makefile.arch
> >>>  include ../../../scripts/Makefile.include
> >>>
> >>>  CXX ?= $(CROSS_COMPILE)g++
> >>> +OBJCOPY ?= $(CROSS_COMPILE)objcopy
> >>>
> >>>  CURDIR := $(abspath .)
> >>>  TOOLSDIR := $(abspath ../../..)
> >>> @@ -716,6 +717,10 @@ $(OUTPUT)/$(TRUNNER_BINARY): $(TRUNNER_TEST_OBJS)                  \
> >>>         $$(call msg,BINARY,,$$@)
> >>>         $(Q)$$(CC) $$(CFLAGS) $$(filter %.a %.o,$$^) $$(LDLIBS) $$(LLVM_LDLIBS) $$(LDFLAGS) $$(LLVM_LDFLAGS) -o $$@
> >>>         $(Q)$(RESOLVE_BTFIDS) --btf $(TRUNNER_OUTPUT)/btf_data.bpf.o $$@
> >>> +       $(Q)if [ -f $$@...f_ids ]; then \
> >>> +               $(OBJCOPY) --update-section .BTF_ids=$$@...f_ids $$@; \
> >>
> >> I encountered a resolve_btfids self-test failure when enabling the
> >> BTF sorting feature, with the following error output:
> >>
> >> All error logs:
> >> resolve_symbols:PASS:resolve 0 nsec
> >> test_resolve_btfids:PASS:id_check 0 nsec
> >> test_resolve_btfids:PASS:id_check 0 nsec
> >> test_resolve_btfids:FAIL:id_check wrong ID for T (7 != 5)
> >> #369     resolve_btfids:FAIL
> >>
> >> The root cause is that prog_tests/resolve_btfids.c retrieves type IDs
> >> from btf_data.bpf.o and compares them against the IDs in test_progs.
> >> However, while the IDs in test_progs are sorted, those in btf_data.bpf.o
> >> remain in their original unsorted state, causing the validation to fail.
> >>
> >> This presents two potential solutions:
> >> 1. Update the relevant .BTF.* section datas in btf_data.bpf.o, including
> >>     the .BTF and .BTF.ext sections
> >> 2. Modify prog_tests/resolve_btfids.c to retrieve IDs from test_progs.btf
> >>     instead. However, I discovered that test_progs.btf is deleted in the
> >>     subsequent code section.
> >>
> >> What do you think of it?
> >
> > Within resolve_btfids it's clear that we have to update (sort in this
> > case) BTF first, and then resolve the ids based on the changed BTF.
> >
> > As for the test, we should probably change it to become closer to an
> > actual resolve_btfids use-case. Maybe even replace or remove it.
> >
> > resolve_btfids operates on BTF generated by pahole for
> > kernel/module. And the .BTF_ids section makes sense only in kernel
> > space AFAIU (might be wrong, let me know if I am).
> >
> > And in this test we are using BTF produced by LLVM for a BPF program,
> > and then create a .BTF_ids section in a user-space app (test_progs /
> > resolve_btfids.test.o), although using proper kernel macros.
> >
> > By the way, the test was written more than 5y ago [1], so it might be
> > outdated too.
> >
> > I think the behavior that we care about is already indirectly tested
> > by bpf_testmod module tests, with custom BPF kfuncs and BTF_ID_*
> > declarations etc. If resolve_btfids is broken, those tests will fail.
> >
> > But it's also reasonable to have some tests targeting resolve_btfids
> > app itself, of course. This one doesn't fit though IMO.
> >
> > I'll try to think of something.
>
> Hi Donglin,
>
> I discussed this off-list with Andrii, and we agreed that the selftest
> itself is reasonable with respect to testing resolve_btfids output.
>
> In this series, I only have to change the test_progs build recipe.
>
> The problem that you've encountered I think can be fixed in the test,
> which is basically what you suggested as option 2:
>
>   static int resolve_symbols(void)
>   {
>         struct btf *btf;
>         int type_id;
>         __u32 nr;
>
>         btf = btf__parse_elf("btf_data.bpf.o", NULL); /* <--- this */
>
>         [...]
>
> Instead of reading in the source BTF, we have to load .btf produced by
> resolve_btfids. A complication is that it's going to be a different
> file for every TRUNNER_BINARY, which has to be accounted for, although
> the BTF itself would be identical between relevant runners.
>
> If go this route, I think we should add .btf cleanup to the Makefile
> and update local .gitignore

Thanks, could the following modification be accepted?

diff --git a/tools/testing/selftests/bpf/.gitignore
b/tools/testing/selftests/bpf/.gitignore
index be1ee7ba7ce0..38ac369cd701 100644
--- a/tools/testing/selftests/bpf/.gitignore
+++ b/tools/testing/selftests/bpf/.gitignore
@@ -45,3 +45,4 @@ xdp_synproxy
 xdp_hw_metadata
 xdp_features
 verification_cert.h
+*.btf
diff --git a/tools/testing/selftests/bpf/Makefile
b/tools/testing/selftests/bpf/Makefile
index 2a027ff9ceaf..a1188129229f 100644
--- a/tools/testing/selftests/bpf/Makefile
+++ b/tools/testing/selftests/bpf/Makefile
@@ -720,7 +720,7 @@ $(OUTPUT)/$(TRUNNER_BINARY): $(TRUNNER_TEST_OBJS)
                 \
        $(Q)if [ -f $$@...f_ids ]; then \
                $(OBJCOPY) --update-section .BTF_ids=$$@...f_ids $$@; \
        fi
-       $(Q)rm -f $$@...f_ids $$@...f
+       $(Q)rm -f $$@...f_ids
        $(Q)ln -sf $(if $2,..,.)/tools/build/bpftool/$(USE_BOOTSTRAP)bpftool \
                   $(OUTPUT)/$(if $2,$2/)bpftool

@@ -908,7 +908,7 @@ EXTRA_CLEAN := $(SCRATCH_DIR) $(HOST_SCRATCH_DIR)
                 \
        prog_tests/tests.h map_tests/tests.h verifier/tests.h           \
        feature bpftool $(TEST_KMOD_TARGETS)                            \
        $(addprefix $(OUTPUT)/,*.o *.d *.skel.h *.lskel.h *.subskel.h   \
-                              no_alu32 cpuv4 bpf_gcc                   \
+                              *.btf no_alu32 cpuv4 bpf_gcc             \
                               liburandom_read.so)                      \
        $(OUTPUT)/FEATURE-DUMP.selftests

diff --git a/tools/testing/selftests/bpf/prog_tests/resolve_btfids.c
b/tools/testing/selftests/bpf/prog_tests/resolve_btfids.c
index 51544372f52e..00883ff16569 100644
--- a/tools/testing/selftests/bpf/prog_tests/resolve_btfids.c
+++ b/tools/testing/selftests/bpf/prog_tests/resolve_btfids.c
@@ -101,7 +101,7 @@ static int resolve_symbols(void)
        int type_id;
        __u32 nr;

-       btf = btf__parse_elf("btf_data.bpf.o", NULL);
+       btf = btf__parse_raw("test_progs.btf");
        if (CHECK(libbpf_get_error(btf), "resolve",
                  "Failed to load BTF from btf_data.bpf.o\n"))
                return -1;

Thanks,
Donglin

>
> This change is not strictly necessary in this series, but it is for
> the BTF sorting series. Let me know if you would like to take this on,
> so we don't do the same work twice.

Thanks, I will take it on.

>
> >
> > [1] https://lore.kernel.org/bpf/20200703095111.3268961-10-jolsa@kernel.org/
> >
> >
> >>
> >> Thanks,
> >> Donglin
> >>
> >>> +       fi
> >>> +       $(Q)rm -f $$@...f_ids $$@...f
> >>>         $(Q)ln -sf $(if $2,..,.)/tools/build/bpftool/$(USE_BOOTSTRAP)bpftool \
> >>>                    $(OUTPUT)/$(if $2,$2/)bpftool
> >>>
> >>> --
> >>> 2.52.0
> >>>
> >
>

Powered by blists - more mailing lists