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

[Ilya Leoshkevich <iii@linux.ibm.com>]

On Thu, 2023-02-09 at 09:47 +0100, Jiri Olsa wrote:
> 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?

Hi Jiri,

sure, I'll give this a try.

Best regards,
Ilya

> 
> thanks,
> jirka