On Tue, Apr 18, 2023 at 11:10 AM Andrii Nakryiko
<andrii.nakryiko@xxxxxxxxx> wrote:
>
> On Mon, Apr 17, 2023 at 6:10 PM Alexei Starovoitov
> <alexei.starovoitov@xxxxxxxxx> wrote:
> >
> > On Mon, Apr 17, 2023 at 5:22 PM Andrii Nakryiko <andrii@xxxxxxxxxx> wrote:
> > > insn[0].imm = ext->ksym.kernel_btf_id;
> > > insn[0].off = ext->ksym.btf_fd_idx;
> > > - } else { /* unresolved weak kfunc */
> > > - insn[0].imm = 0;
> > > - insn[0].off = 0;
> > > + } else { /* unresolved weak kfunc call */
> > > + poison_kfunc_call(prog, i, relo->insn_idx, insn,
> > > + relo->ext_idx, ext);
> >
> > With that done should we remove:
> > /* skip for now, but return error when we find this in fixup_kfunc_call */
> > if (!insn->imm)
> > return 0;
> > in check_kfunc_call()...
> >
> > and if (!func_id && !offset) in add_kfunc_call() ?
> >
> > That was added in commit a5d827275241 ("bpf: Be conservative while
> > processing invalid kfunc calls")
>
> I guess?.. I don't know if there was any other situation that this fix
> was handling, but if it's only due to unresolved kfuncs by libbpf,
> then yep.
It was specifically to support weak kfunc with imm==0 off==0.
With libbpf doing poisoning and converting call kfunc into call
unknown helper that code is no longer needed.
The question is whether we should try to support new kernel plus old
libbpf combination.
