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

[Jiri Olsa <olsajiri@gmail.com>]

On Mon, Feb 06, 2023 at 10:15:37AM +0100, Jiri Olsa wrote:
> 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

any other ideas/thoughts on this?

I don't have s390 server available, so can't really fix/test that..
any chance you work on that?

thanks,
jirka