Re: [PATCHv3 bpf-next 0/9] bpf: Move kernel test kfuncs into bpf_testmod

Jiri Olsa
2023-02-06T09:15:46Z

On Sun, Feb 05, 2023 at 07:36:14PM +0100, Ilya Leoshkevich wrote:
> On Sun, 2023-02-05 at 19:17 +0100, Jiri Olsa wrote:
> > On Sat, Feb 04, 2023 at 01:21:13AM -0800, Alexei Starovoitov wrote:
> > > On Fri, Feb 3, 2023 at 8:23 AM Jiri Olsa <jolsa@kernel.org> wrote:
> > > >
> > > > hi,
> > > > I noticed several times in discussions that we should move test
> > > > kfuncs
> > > > into kernel module, now perhaps even more pressing with all the
> > > > kfunc
> > > > effort. This patchset moves all the test kfuncs into bpf_testmod.
> > > >
> > > > I added bpf_testmod/bpf_testmod_kfunc.h header that is shared
> > > > between
> > > > bpf_testmod kernel module and BPF programs, which brings some
> > > > difficulties
> > > > with __ksym define. But I'm not sure having separate headers for
> > > > BPF
> > > > programs and for kernel module would be better.
> > > >
> > > > This patchset also needs:
> > > >   74bc3a5acc82 bpf: Add missing btf_put to
> > > > register_btf_id_dtor_kfuncs
> > > > which is only in bpf/master now.
> > >
> > > I thought you've added this patch to CI,
> > > but cb_refs is still failing on s390...
> >
> > the CI now fails for s390 with messages like:
> >    2023-02-04T07:04:32.5185267Z    RES: address of kernel function
> > bpf_kfunc_call_test_fail1 is out of range
> >
> > so now that we have test kfuncs in the module, the 's32 imm' value of
> > the bpf call instructions can overflow when the offset between module
> > and kernel is greater than 2GB ... as explained in the commit that
> > added the verifier check:
> >
> >   8cbf062a250e bpf: Reject kfunc calls that overflow insn->imm
> >
> > not sure we can do anything about that on bpf side.. cc-ing s390 list
> > and Ilya for ideas/thoughts
> >
> > maybe we could make bpf_testmod in-tree module and compile it as
> > module
> > just for some archs
> >
> > thoughts?
>
> Hi,
>
> I'd rather have this fixed - I guess the problem can affect the users.
> The ksyms_module test is already denylisted because of that.
> Unfortunately getting the kernel and the modules close together on
> s390x is unlikely to happen in the foreseeable future.
>
> What do you think about keeping the BTF ID inside the insn->imm field
> and putting the 64-bit delta into bpf_insn_aux_data, replacing the
> call_imm field that we already have there?

seems tricky wrt other archs.. how about saving address of the kfunc
in bpf_insn_aux_data and use that in s390 jit code instead of the
'__bpf_call_base + imm' calculation

jirka