| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <20160420193431.GW3677@kernel.org>
Date: Wed, 20 Apr 2016 16:34:31 -0300
From: Arnaldo Carvalho de Melo <acme@...nel.org>
To: Wang Nan <wangnan0@...wei.com>
Cc: jolsa@...hat.com, brendan.d.gregg@...il.com,
linux-kernel@...r.kernel.org, pi3orama@....com,
Arnaldo Carvalho de Melo <acme@...hat.com>,
Alexei Starovoitov <ast@...nel.org>,
Jiri Olsa <jolsa@...nel.org>, Li Zefan <lizefan@...wei.com>
Subject: Re: [RFC PATCH 07/13] bpf tools: Load ubpf program
Em Wed, Apr 20, 2016 at 06:01:47PM +0000, Wang Nan escreveu:
> In bpf_program__load(), load ubpf program according to its engine type.
>
> API is improvemented to hold 'struct ubpf_vm *'.
>
> Signed-off-by: Wang Nan <wangnan0@...wei.com>
> Cc: Arnaldo Carvalho de Melo <acme@...hat.com>
> Cc: Alexei Starovoitov <ast@...nel.org>
> Cc: Brendan Gregg <brendan.d.gregg@...il.com>
> Cc: Jiri Olsa <jolsa@...nel.org>
> Cc: Li Zefan <lizefan@...wei.com>
> ---
> tools/lib/bpf/libbpf.c | 66 +++++++++++++++++++++++++++++++++++++++-----------
> tools/lib/bpf/libbpf.h | 11 +++++++--
> 2 files changed, 61 insertions(+), 16 deletions(-)
>
> diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
> index 3a969fd..e4a1e77 100644
> --- a/tools/lib/bpf/libbpf.c
> +++ b/tools/lib/bpf/libbpf.c
> @@ -81,6 +81,7 @@ static const char *libbpf_strerror_table[NR_ERRNO] = {
> [ERRCODE_OFFSET(PROG2BIG)] = "Program too big",
> [ERRCODE_OFFSET(KVER)] = "Incorrect kernel version",
> [ERRCODE_OFFSET(NOUBPF)] = "UBPF support is not compiled",
> + [ERRCODE_OFFSET(LOADUBPF)] = "Failed to load user space BPF program",
> };
>
> int libbpf_strerror(int err, char *buf, size_t size)
> @@ -949,6 +950,31 @@ static int bpf_object__collect_reloc(struct bpf_object *obj)
> return 0;
> }
>
> +#ifdef HAVE_UBPF_SUPPORT
> +static int
> +load_ubpf_program(struct bpf_insn *insns, int insns_cnt,
> + struct ubpf_vm **pvm)
> +{
> + struct ubpf_vm *vm = ubpf_create();
> + char *message;
> + int err;
> +
> + if (!vm) {
> + pr_warning("Failed to create ubpf vm\n");
> + return -LIBBPF_ERRNO__LOADUBPF;
> + }
> +
> + err = ubpf_load(vm, insns, insns_cnt * sizeof(insns[0]), &message);
> + if (err < 0) {
> + pr_warning("Failed to load ubpf program: %s\n", message);
> + return -LIBBPF_ERRNO__LOADUBPF;
> + }
> +
> + *pvm = vm;
> + return 0;
> +}
> +#endif
> +
> static int
> load_program(struct bpf_insn *insns, int insns_cnt,
> char *license, u32 kern_version, int *pfd)
> @@ -1002,11 +1028,12 @@ bpf_program__load(struct bpf_program *prog,
> char *license, u32 kern_version)
> {
> int err = 0, fd, i;
> +#ifdef HAVE_UBPF_SUPPORT
> + struct ubpf_vm *vm;
> +#endif
>
> if (prog->engine == ENGINE_UNKNOWN)
> prog->engine = ENGINE_KBPF;
> - if (prog->engine != ENGINE_KBPF)
> - return -EINVAL;
>
> if (prog->instances.nr < 0 || !prog->instances.array) {
> if (prog->preprocessor) {
> @@ -1029,10 +1056,15 @@ bpf_program__load(struct bpf_program *prog,
> pr_warning("Program '%s' is inconsistent: nr(%d) != 1\n",
> prog->section_name, prog->instances.nr);
> }
> - err = load_program(prog->insns, prog->insns_cnt,
> - license, kern_version, &fd);
> +
> + if_engine(prog,
> + (err = load_program(prog->insns, prog->insns_cnt,
> + license, kern_version, &fd)),
> + (err = load_ubpf_program(prog->insns, prog->insns_cnt,
> + &vm)));
Same thing here, with that struct bpf_engine fops you would just do:
err = engine->load(prog, arg);
> +
> if (!err)
> - prog->instan_fd(0) = fd;
> + set_instance(prog, 0, fd, vm);
> goto out;
> }
>
> @@ -1052,15 +1084,21 @@ bpf_program__load(struct bpf_program *prog,
> if (!result.new_insn_ptr || !result.new_insn_cnt) {
> pr_debug("Skip loading the %dth instance of program '%s'\n",
> i, prog->section_name);
> - prog->instan_fd(i) = -1;
> - if (result.pfd)
> - *result.pfd = -1;
> +
> + if_engine(prog, prog->instan_fd(i) = -1,
> + prog->instan_vm(i) = NULL);
> + if (result.ptr)
> + if_engine(prog, *result.pfd = -1, *result.pvm = NULL);
> continue;
> }
>
> - err = load_program(result.new_insn_ptr,
> - result.new_insn_cnt,
> - license, kern_version, &fd);
> + if_engine(prog,
> + (err = load_program(result.new_insn_ptr,
> + result.new_insn_cnt,
> + license, kern_version, &fd)),
> + (err = load_ubpf_program(result.new_insn_ptr,
> + result.new_insn_cnt,
> + &vm)));
>
> if (err) {
> pr_warning("Loading the %dth instance of program '%s' failed\n",
> @@ -1068,9 +1106,9 @@ bpf_program__load(struct bpf_program *prog,
> goto out;
> }
>
> - if (result.pfd)
> - *result.pfd = fd;
> - prog->instan_fd(i) = fd;
> + if (result.ptr)
> + if_engine(prog, *result.pfd = fd, *result.pvm = vm);
> + set_instance(prog, i, fd, vm);
> }
> out:
> if (err)
> diff --git a/tools/lib/bpf/libbpf.h b/tools/lib/bpf/libbpf.h
> index f6965ce..8e69c6f 100644
> --- a/tools/lib/bpf/libbpf.h
> +++ b/tools/lib/bpf/libbpf.h
> @@ -27,6 +27,7 @@ enum libbpf_errno {
> LIBBPF_ERRNO__PROG2BIG, /* Program too big */
> LIBBPF_ERRNO__KVER, /* Incorrect kernel version */
> LIBBPF_ERRNO__NOUBPF, /* UBPF support is not compiled */
> + LIBBPF_ERRNO__LOADUBPF, /* Failed to load user space BPF program */
> __LIBBPF_ERRNO__END,
> };
>
> @@ -123,6 +124,8 @@ struct bpf_insn;
> * bpf_program__nth_fd(prog, 0).
> */
>
> +struct ubpf_vm;
> +
> struct bpf_prog_prep_result {
> /*
> * If not NULL, load new instruction array.
> @@ -131,8 +134,12 @@ struct bpf_prog_prep_result {
> struct bpf_insn *new_insn_ptr;
> int new_insn_cnt;
>
> - /* If not NULL, result fd is set to it */
> - int *pfd;
> + /* If not NULL, result is set to it */
> + union {
> + void *ptr;
> + int *pfd;
> + struct ubpf_vm **pvm;
> + };
> };
see? Here you use anonymous unions, but after all I wrote, I think that
you really need a fops struct, like we have so many in the kernel
sources, see struct inode_operations, file_operations, etc, and then
that bpf_program->instance.entries needs just to be a void pointer.
- Arnaldo
Powered by blists - more mailing lists