Message-ID: <6cdae76c99bb74c3389a05e39c34732c2ca172c6.camel@gmail.com>
Date: Thu, 04 Dec 2025 11:14:01 -0800
From: Eduard Zingerman <eddyz87@...il.com>
To: Ihor Solodrai <ihor.solodrai@...ux.dev>, Alexei Starovoitov
 <ast@...nel.org>,  Daniel Borkmann <daniel@...earbox.net>, Andrii Nakryiko
 <andrii@...nel.org>, Martin KaFai Lau <martin.lau@...ux.dev>,  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>,
 Donglin Peng	 <dolinux.peng@...il.com>
Cc: 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 Thu, 2025-12-04 at 11:04 -0800, Ihor Solodrai wrote:
> On 12/4/25 10:06 AM, Eduard Zingerman wrote:
> > On Thu, 2025-12-04 at 09:29 -0800, Ihor Solodrai wrote:
> > 
> > [...]
> > 
> > > Ok, it seems you're conflating two separate issues.
> > > 
> > > There is a requirement to *link* .BTF section into vmlinux, because it
> > > must have a SHF_ALLOC flag, which makes objcopying the section data
> > > insufficient: linker has to do some magic under the hood.
> > > 
> > > The patch doesn't change this behavior, and this was (and is) covered
> > > in the script comments.
> > > 
> > > A separate issue is what resolve_btfids does: updates ELF in-place
> > > (before the patch) or outputs detached section data (after patch).
> > > 
> > > The paragraph in the commit message attempted to explain the decision
> > > to output raw section data. And apparently I did a bad job of
> > > that. I'll rewrite this part it in the next revision.
> > > 
> > > And I feel I should clarify that I didn't claim that libelf is buggy.
> > > I meant that using it is complicated, which makes resolve_btfids buggy.
> > 
> > So, pahole does the following:
> > - elf_begin(fildes: fd, cmd: ELF_C_RDWR, ref: NULL);
> > - selects a section to modify and modifies it
> > - elf_flagdata(data: btf_data, cmd: ELF_C_SET, flags: ELF_F_DIRTY);
> > - elf_update(elf, cmd: ELF_C_WRITE)
> > - elf_end(elf)
> > 
> > What exactly is complicated about that?
> 
> Take a look at the resolve_btfids code that is removed in this patch,
> as a consequence of switching to read-only ELF.
> 
> Also consider that before these changes resolve_btfids had a simple
> job: update data buffer of a single section, importantly, without
> changing its size.
> 
> Now let's say we keep "update in-place" approach (which I tried to do,
> btw). In addition to previous .BTF_ids data update, resolve_btfids may
> need to either add or update .BTF section changing its size (triggering
> reorg of sections in ELF, depending on the flags) and add .BTF.base
> section. There is also a question of how to do it: do we elf_update()
> multiple times or try to "batch" the updates?
> 
> All of this is possible, but the alternative is much simpler:
> 
>     ${OBJCOPY} --add-section .BTF=${ELF_FILE}.btf ${ELF_FILE}
> 
> Why re-implement our own incomplete version of objcopy if we can just
> use it to deal with the details of the ELF update?
> 
> Note also that even in pahole "add .BTF section" is implemented via
> llvm-objcopy call. My guess is: to avoid the headache of figuring out
> correct libelf usage.

Please put this motivation in the commit message.

Powered by blists - more mailing lists