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: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?

> [...]

--
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