Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752513AbcDTTej (ORCPT ); Wed, 20 Apr 2016 15:34:39 -0400 Received: from mail.kernel.org ([198.145.29.136]:34854 "EHLO mail.kernel.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752260AbcDTTeh (ORCPT ); Wed, 20 Apr 2016 15:34:37 -0400 Date: Wed, 20 Apr 2016 16:34:31 -0300 From: Arnaldo Carvalho de Melo To: Wang Nan Cc: jolsa@redhat.com, brendan.d.gregg@gmail.com, linux-kernel@vger.kernel.org, pi3orama@163.com, Arnaldo Carvalho de Melo , Alexei Starovoitov , Jiri Olsa , Li Zefan Subject: Re: [RFC PATCH 07/13] bpf tools: Load ubpf program Message-ID: <20160420193431.GW3677@kernel.org> References: <1461175313-38310-1-git-send-email-wangnan0@huawei.com> <1461175313-38310-8-git-send-email-wangnan0@huawei.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <1461175313-38310-8-git-send-email-wangnan0@huawei.com> X-Url: http://acmel.wordpress.com User-Agent: Mutt/1.5.24 (2015-08-30) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 5654 Lines: 182 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 > Cc: Arnaldo Carvalho de Melo > Cc: Alexei Starovoitov > Cc: Brendan Gregg > Cc: Jiri Olsa > Cc: Li Zefan > --- > 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