Message-ID: <642f6b68-0691-44a1-844f-a8cddec41fd0@linux.dev>
Date: Mon, 1 Dec 2025 11:46:17 -0800
From: Ihor Solodrai <ihor.solodrai@...ux.dev>
To: Donglin Peng <dolinux.peng@...il.com>
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 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

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.

> 
> [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