Re: [PATCH v3 bpf-next 5/5] bpf: Convert bpf_preload.ko to use light skeleton.

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Wed, Feb 09, 2022 at 11:52:35AM IST, Kumar Kartikeya Dwivedi wrote:
> On Wed, Feb 09, 2022 at 11:13:15AM IST, Alexei Starovoitov wrote:
> > From: Alexei Starovoitov <ast@xxxxxxxxxx>
> >
> > The main change is a move of the single line
> >   #include "iterators.lskel.h"
> > from iterators/iterators.c to bpf_preload_kern.c.
> > Which means that generated light skeleton can be used from user space or
> > user mode driver like iterators.c or from the kernel module or the kernel itself.
> > The direct use of light skeleton from the kernel module simplifies the code,
> > since UMD is no longer necessary. The libbpf.a required user space and UMD. The
> > CO-RE in the kernel and generated "loader bpf program" used by the light
> > skeleton are capable to perform complex loading operations traditionally
> > provided by libbpf. In addition UMD approach was launching UMD process
> > every time bpffs has to be mounted. With light skeleton in the kernel
> > the bpf_preload kernel module loads bpf iterators once and pins them
> > multiple times into different bpffs mounts.
> >
> > Acked-by: Yonghong Song <yhs@xxxxxx>
> > Acked-by: Andrii Nakryiko <andrii@xxxxxxxxxx>
> > Signed-off-by: Alexei Starovoitov <ast@xxxxxxxxxx>
> > ---
> >  kernel/bpf/inode.c                            |  39 ++----
> >  kernel/bpf/preload/Kconfig                    |   7 +-
> >  kernel/bpf/preload/Makefile                   |  14 +--
> >  kernel/bpf/preload/bpf_preload.h              |   8 +-
> >  kernel/bpf/preload/bpf_preload_kern.c         | 119 ++++++++----------
> >  kernel/bpf/preload/bpf_preload_umd_blob.S     |   7 --
> >  .../preload/iterators/bpf_preload_common.h    |  13 --
> >  kernel/bpf/preload/iterators/iterators.c      | 108 ----------------
> >  kernel/bpf/syscall.c                          |   2 +
> >  9 files changed, 70 insertions(+), 247 deletions(-)
> >  delete mode 100644 kernel/bpf/preload/bpf_preload_umd_blob.S
> >  delete mode 100644 kernel/bpf/preload/iterators/bpf_preload_common.h
> >  delete mode 100644 kernel/bpf/preload/iterators/iterators.c
> >
> > [...]
> >
> > -static int __init load_umd(void)
> > +static int __init load(void)
> >  {
> >  	int err;
> >
> > -	err = umd_load_blob(&umd_ops.info, &bpf_preload_umd_start,
> > -			    &bpf_preload_umd_end - &bpf_preload_umd_start);
> > +	err = load_skel();
> >  	if (err)
> >  		return err;
> > -	bpf_preload_ops = &umd_ops;
> > +	bpf_preload_ops = &ops;
> >  	return err;
> >  }
> >
> > -static void __exit fini_umd(void)
> > +static void __exit fini(void)
> >  {
> > -	struct pid *tgid;
> > -
> >  	bpf_preload_ops = NULL;
> > -
> > -	/* kill UMD in case it's still there due to earlier error */
> > -	tgid = umd_ops.info.tgid;
> > -	if (tgid) {
> > -		kill_pid(tgid, SIGKILL, 1);
> > -
> > -		wait_event(tgid->wait_pidfd, thread_group_exited(tgid));
> > -		umd_cleanup_helper(&umd_ops.info);
> > -	}
> > -	umd_unload_blob(&umd_ops.info);
> > +	free_links_and_skel();
> >  }
> > -late_initcall(load_umd);
> > -module_exit(fini_umd);
> > +late_initcall(load);
>
> Since load_skel invokes the loader which resolve kfuncs, and if the module is
> builtin, btf_vmlinux's kfunc_set_tab is not fully populated yet (other
> register_btf_id_kfunc_set calls may also happen in late_initcall), so I think it
> may be a problem.
>
> When I worked on it we didn't have this case where BPF syscall can be invoked at
> this point, but obviously that is true now. So I think ordering it later than
> late_initcall (or moving register_btf_id_kfunc_set invoking initcalls before) is
> required. WDYT?
>

To be more clear, I know this series is not using kfuncs anywhere, but this is
probably the right time to fix it, while we are changing the assumptions.

> > [...]
>
> --
> Kartikeya

--
Kartikeya



[Index of Archives]     [Linux Samsung SoC]     [Linux Rockchip SoC]     [Linux Actions SoC]     [Linux for Synopsys ARC Processors]     [Linux NFS]     [Linux NILFS]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]


  Powered by Linux