From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1757995AbbJ2WoP (ORCPT ); Thu, 29 Oct 2015 18:44:15 -0400 Received: from mail.kernel.org ([198.145.29.136]:36179 "EHLO mail.kernel.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1756932AbbJ2WoN (ORCPT ); Thu, 29 Oct 2015 18:44:13 -0400 Date: Thu, 29 Oct 2015 19:44:07 -0300 From: Arnaldo Carvalho de Melo To: Wang Nan Cc: ast@plumgrid.com, brendan.d.gregg@gmail.com, a.p.zijlstra@chello.nl, daniel@iogearbox.net, dsahern@gmail.com, hekuang@huawei.com, jolsa@kernel.org, lizefan@huawei.com, masami.hiramatsu.pt@hitachi.com, namhyung@kernel.org, paulus@samba.org, linux-kernel@vger.kernel.org, pi3orama@163.com, xiakaixu@huawei.com Subject: Re: [PATCH 13/31] bpf tools: Load a program with different instances using preprocessor Message-ID: <20151029224407.GM2923@kernel.org> References: <1444826502-49291-1-git-send-email-wangnan0@huawei.com> <1444826502-49291-14-git-send-email-wangnan0@huawei.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <1444826502-49291-14-git-send-email-wangnan0@huawei.com> X-Url: http://acmel.wordpress.com User-Agent: Mutt/1.5.23 (2014-03-12) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Em Wed, Oct 14, 2015 at 12:41:24PM +0000, Wang Nan escreveu: > In this patch, caller of libbpf is able to control the loaded programs > by installing a preprocessor callback for a BPF program. With > preprocessor, different instances can be created from one BPF program. Why would one want to do that? I'm new to eBPF, as many other here, I guess, so giving some more context as to what will this achieve will help me in processing these patches. > This patch will be used by perf to generate different prologue for > different 'struct probe_trace_event' instances matched by one > 'struct perf_probe_event'. > > bpf_program__set_prep() is added to support this feature. Caller > should pass libbpf the number of instances should be created and a that > preprocessor function which will be called when doing real loading. the > The callback should return instructions arrays for each instances. instruction arrays for each instance. > > fd field in bpf_programs is replaced by instance, which has an nr field The fd field in the bpf_programs struct is replaced by instance (instance of what?) > and fds array. bpf_program__nth_fd() is introduced for read fd of > instances. Old interface bpf_program__fd() is reimplemented by > returning the first fd. > > Signed-off-by: Wang Nan > Signed-off-by: He Kuang > Cc: Alexei Starovoitov > Cc: Brendan Gregg > Cc: Daniel Borkmann > Cc: David Ahern > Cc: He Kuang > Cc: Jiri Olsa > Cc: Kaixu Xia > Cc: Masami Hiramatsu > Cc: Namhyung Kim > Cc: Paul Mackerras > Cc: Peter Zijlstra > Cc: Zefan Li > Cc: pi3orama@163.com > Cc: Arnaldo Carvalho de Melo > Link: http://lkml.kernel.org/n/ebpf-6yw9eg0ej3l4jnqhinngkw86@git.kernel.org > --- > tools/lib/bpf/libbpf.c | 143 +++++++++++++++++++++++++++++++++++++++++++++---- > tools/lib/bpf/libbpf.h | 22 ++++++++ > 2 files changed, 156 insertions(+), 9 deletions(-) > > diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c > index 4252fc2..6a07b26 100644 > --- a/tools/lib/bpf/libbpf.c > +++ b/tools/lib/bpf/libbpf.c > @@ -98,7 +98,11 @@ struct bpf_program { > } *reloc_desc; > int nr_reloc; > > - int fd; > + struct { > + int nr; > + int *fds; > + } instance; > + bpf_program_prep_t preprocessor; > > struct bpf_object *obj; > void *priv; > @@ -152,10 +156,24 @@ struct bpf_object { > > static void bpf_program__unload(struct bpf_program *prog) > { > + int i; > + > if (!prog) > return; > > - zclose(prog->fd); > + /* > + * If the object is opened but the program is never loaded, > + * it is possible that prog->instance.nr == -1. > + */ > + if (prog->instance.nr > 0) { > + for (i = 0; i < prog->instance.nr; i++) > + zclose(prog->instance.fds[i]); > + } else if (prog->instance.nr != -1) > + pr_warning("Internal error: instance.nr is %d\n", > + prog->instance.nr); > + > + prog->instance.nr = -1; > + zfree(&prog->instance.fds); > } > > static void bpf_program__exit(struct bpf_program *prog) > @@ -206,7 +224,8 @@ bpf_program__init(void *data, size_t size, char *name, int idx, > memcpy(prog->insns, data, > prog->insns_cnt * sizeof(struct bpf_insn)); > prog->idx = idx; > - prog->fd = -1; > + prog->instance.fds = NULL; > + prog->instance.nr = -1; > > return 0; > errout: > @@ -795,13 +814,71 @@ static int > bpf_program__load(struct bpf_program *prog, > char *license, u32 kern_version) > { > - int err, fd; > + int err = 0, fd, i; > + > + if (prog->instance.nr < 0 || !prog->instance.fds) { > + if (prog->preprocessor) { > + pr_warning("Internal error: can't load program '%s'\n", > + prog->section_name); > + return -EINVAL; > + } > + > + prog->instance.fds = malloc(sizeof(int)); > + if (!prog->instance.fds) { > + pr_warning("No enough memory for fds\n"); > + return -ENOMEM; > + } > + prog->instance.nr = 1; > + prog->instance.fds[0] = -1; > + } > + > + if (!prog->preprocessor) { > + if (prog->instance.nr != 1) > + pr_warning("Program '%s' inconsistent: nr(%d) not 1\n", > + prog->section_name, prog->instance.nr); > > - err = load_program(prog->insns, prog->insns_cnt, > - license, kern_version, &fd); > - if (!err) > - prog->fd = fd; > + err = load_program(prog->insns, prog->insns_cnt, > + license, kern_version, &fd); > + if (!err) > + prog->instance.fds[0] = fd; > + goto out; > + } > + > + for (i = 0; i < prog->instance.nr; i++) { > + struct bpf_prog_prep_result result; > + bpf_program_prep_t preprocessor = prog->preprocessor; > + > + bzero(&result, sizeof(result)); > + err = preprocessor(prog, i, prog->insns, > + prog->insns_cnt, &result); > + if (err) { > + pr_warning("Preprocessing %dth instance of program '%s' failed\n", > + i, prog->section_name); > + goto out; > + } > + > + if (!result.new_insn_ptr || !result.new_insn_cnt) { > + pr_debug("Skip loading %dth instance of program '%s'\n", > + i, prog->section_name); > + prog->instance.fds[i] = -1; > + continue; > + } > + > + err = load_program(result.new_insn_ptr, > + result.new_insn_cnt, > + license, kern_version, &fd); > + > + if (err) { > + pr_warning("Loading %dth instance of program '%s' failed\n", > + i, prog->section_name); > + goto out; > + } > > + if (result.pfd) > + *result.pfd = fd; > + prog->instance.fds[i] = fd; > + } > +out: > if (err) > pr_warning("failed to load program '%s'\n", > prog->section_name); > @@ -1052,5 +1129,53 @@ const char *bpf_program__title(struct bpf_program *prog, bool dup) > > int bpf_program__fd(struct bpf_program *prog) > { > - return prog->fd; > + return bpf_program__nth_fd(prog, 0); > +} > + > +int bpf_program__set_prep(struct bpf_program *prog, int nr_instance, > + bpf_program_prep_t prep) > +{ > + int *instance_fds; > + > + if (nr_instance <= 0 || !prep) > + return -EINVAL; > + > + if (prog->instance.nr > 0 || prog->instance.fds) { > + pr_warning("Can't set pre-processor after loading\n"); > + return -EINVAL; > + } > + > + instance_fds = malloc(sizeof(int) * nr_instance); > + if (!instance_fds) { > + pr_warning("alloc memory failed for instance of fds\n"); > + return -ENOMEM; > + } > + > + /* fill all fd with -1 */ > + memset(instance_fds, 0xff, sizeof(int) * nr_instance); > + > + prog->instance.nr = nr_instance; > + prog->instance.fds = instance_fds; > + prog->preprocessor = prep; > + return 0; > +} > + > +int bpf_program__nth_fd(struct bpf_program *prog, int n) > +{ > + int fd; > + > + if (n >= prog->instance.nr || n < 0) { > + pr_warning("Can't get the %dth fd from program %s: only %d instances\n", > + n, prog->section_name, prog->instance.nr); > + return -EINVAL; > + } > + > + fd = prog->instance.fds[n]; > + if (fd < 0) { > + pr_warning("%dth instance of program '%s' is invalid\n", > + n, prog->section_name); > + return -ENOENT; > + } > + > + return fd; > } > diff --git a/tools/lib/bpf/libbpf.h b/tools/lib/bpf/libbpf.h > index f16170c..d82b89e 100644 > --- a/tools/lib/bpf/libbpf.h > +++ b/tools/lib/bpf/libbpf.h > @@ -67,6 +67,28 @@ const char *bpf_program__title(struct bpf_program *prog, bool dup); > > int bpf_program__fd(struct bpf_program *prog); > > +struct bpf_insn; > +struct bpf_prog_prep_result { > + /* > + * If not NULL, load new instruction array. > + * If set to NULL, don't load this instance. > + */ > + struct bpf_insn *new_insn_ptr; > + int new_insn_cnt; > + > + /* If not NULL, result fd is set to it */ > + int *pfd; > +}; > + > +typedef int (*bpf_program_prep_t)(struct bpf_program *, int n, > + struct bpf_insn *, int insn_cnt, > + struct bpf_prog_prep_result *res); > + > +int bpf_program__set_prep(struct bpf_program *prog, int nr_instance, > + bpf_program_prep_t prep); > + > +int bpf_program__nth_fd(struct bpf_program *prog, int n); > + > /* > * We don't need __attribute__((packed)) now since it is > * unnecessary for 'bpf_map_def' because they are all aligned. > -- > 1.8.3.4