| Message ID | 20220217131916.50615-4-jolsa@kernel.org (mailing list archive) |
|---|---|
| State | Superseded |
| Delegated to: | BPF |
| Headers | show |
| Series | perf/bpf: Replace deprecated code | expand |
| Context | Check | Description |
|---|---|---|
| bpf/vmtest-bpf-PR | fail | merge-conflict |
| netdev/tree_selection | success | Not a local patch |
On Thu, Feb 17, 2022 at 5:19 AM Jiri Olsa <jolsa@kernel.org> wrote: > > Some functions we use now for bpf prologue generation are > going to be deprecated, so reworking the current code not > to use them. > > We need to replace following functions/struct: > bpf_program__set_prep > bpf_program__nth_fd > struct bpf_prog_prep_result > > Current code uses bpf_program__set_prep to hook perf callback > before the program is loaded and provide new instructions with > the prologue. > > We workaround this by using objects's 'unloaded' programs instructions > for that specific program and load new ebpf programs with prologue > using separate bpf_prog_load calls. > > We keep new ebpf program instances descriptors in bpf programs > private struct. > > Suggested-by: Andrii Nakryiko <andrii@kernel.org> > Signed-off-by: Jiri Olsa <jolsa@kernel.org> > --- > tools/perf/util/bpf-loader.c | 122 +++++++++++++++++++++++++++++------ > 1 file changed, 104 insertions(+), 18 deletions(-) > [...] > errout: > @@ -696,7 +718,7 @@ static int hook_load_preprocessor(struct bpf_program *prog) > struct bpf_prog_priv *priv = program_priv(prog); > struct perf_probe_event *pev; > bool need_prologue = false; > - int err, i; > + int i; > > if (IS_ERR_OR_NULL(priv)) { > pr_debug("Internal error when hook preprocessor\n"); > @@ -727,6 +749,12 @@ static int hook_load_preprocessor(struct bpf_program *prog) > return 0; > } > > + /* > + * Do not load programs that need prologue, because we need > + * to add prologue first, check bpf_object__load_prologue. > + */ > + bpf_program__set_autoload(prog, false); if you set autoload to false, program instructions might be invalid in the end. Libbpf doesn't apply some (all?) relocations to such programs, doesn't resolve CO-RE, etc, etc. You have to let "prototypal" BPF program to be loaded before you can grab final instructions. It's not great, but in your case it should work, right? > + > priv->need_prologue = true; > priv->insns_buf = malloc(sizeof(struct bpf_insn) * BPF_MAXINSNS); > if (!priv->insns_buf) { > @@ -734,6 +762,13 @@ static int hook_load_preprocessor(struct bpf_program *prog) > return -ENOMEM; > } > [...] > + /* > + * For each program that needs prologue we do following: > + * > + * - take its current instructions and use them > + * to generate the new code with prologue > + * > + * - load new instructions with bpf_prog_load > + * and keep the fd in proglogue_fds > + * > + * - new fd will be used bpf__foreach_event > + * to connect this program with perf evsel > + */ > + orig_insns = bpf_program__insns(prog); > + orig_insns_cnt = bpf_program__insn_cnt(prog); > + > + pev = &priv->pev; > + for (i = 0; i < pev->ntevs; i++) { > + err = preproc_gen_prologue(prog, i, orig_insns, > + orig_insns_cnt, &res); > + if (err) > + return err; > + > + fd = bpf_prog_load(bpf_program__get_type(prog), nit: bpf_program__type() is preferred (we are deprecating/discouraging "get_" prefixed getters in libbpf 1.0) > + bpf_program__name(prog), "GPL", would it make sense to give each clone a distinct name? > + res.new_insn_ptr, > + res.new_insn_cnt, NULL); > + if (fd < 0) { > + char bf[128]; > + [...]
On Thu, Feb 17, 2022 at 01:53:16PM -0800, Andrii Nakryiko wrote: > On Thu, Feb 17, 2022 at 5:19 AM Jiri Olsa <jolsa@kernel.org> wrote: > > > > Some functions we use now for bpf prologue generation are > > going to be deprecated, so reworking the current code not > > to use them. > > > > We need to replace following functions/struct: > > bpf_program__set_prep > > bpf_program__nth_fd > > struct bpf_prog_prep_result > > > > Current code uses bpf_program__set_prep to hook perf callback > > before the program is loaded and provide new instructions with > > the prologue. > > > > We workaround this by using objects's 'unloaded' programs instructions > > for that specific program and load new ebpf programs with prologue > > using separate bpf_prog_load calls. > > > > We keep new ebpf program instances descriptors in bpf programs > > private struct. > > > > Suggested-by: Andrii Nakryiko <andrii@kernel.org> > > Signed-off-by: Jiri Olsa <jolsa@kernel.org> > > --- > > tools/perf/util/bpf-loader.c | 122 +++++++++++++++++++++++++++++------ > > 1 file changed, 104 insertions(+), 18 deletions(-) > > > > [...] > > > errout: > > @@ -696,7 +718,7 @@ static int hook_load_preprocessor(struct bpf_program *prog) > > struct bpf_prog_priv *priv = program_priv(prog); > > struct perf_probe_event *pev; > > bool need_prologue = false; > > - int err, i; > > + int i; > > > > if (IS_ERR_OR_NULL(priv)) { > > pr_debug("Internal error when hook preprocessor\n"); > > @@ -727,6 +749,12 @@ static int hook_load_preprocessor(struct bpf_program *prog) > > return 0; > > } > > > > + /* > > + * Do not load programs that need prologue, because we need > > + * to add prologue first, check bpf_object__load_prologue. > > + */ > > + bpf_program__set_autoload(prog, false); > > if you set autoload to false, program instructions might be invalid in > the end. Libbpf doesn't apply some (all?) relocations to such > programs, doesn't resolve CO-RE, etc, etc. You have to let > "prototypal" BPF program to be loaded before you can grab final > instructions. It's not great, but in your case it should work, right? hum, do we care? it should all be done when the 'new' program with the prologue is loaded, right? I switched it off because the verifier failed to load the program without the prologue.. because in the originaal program there's no code to grab the arguments that the rest of the code depends on, so the verifier sees invalid access > > > + > > priv->need_prologue = true; > > priv->insns_buf = malloc(sizeof(struct bpf_insn) * BPF_MAXINSNS); > > if (!priv->insns_buf) { > > @@ -734,6 +762,13 @@ static int hook_load_preprocessor(struct bpf_program *prog) > > return -ENOMEM; > > } > > > > [...] > > > + /* > > + * For each program that needs prologue we do following: > > + * > > + * - take its current instructions and use them > > + * to generate the new code with prologue > > + * > > + * - load new instructions with bpf_prog_load > > + * and keep the fd in proglogue_fds > > + * > > + * - new fd will be used bpf__foreach_event > > + * to connect this program with perf evsel > > + */ > > + orig_insns = bpf_program__insns(prog); > > + orig_insns_cnt = bpf_program__insn_cnt(prog); > > + > > + pev = &priv->pev; > > + for (i = 0; i < pev->ntevs; i++) { > > + err = preproc_gen_prologue(prog, i, orig_insns, > > + orig_insns_cnt, &res); > > + if (err) > > + return err; > > + > > + fd = bpf_prog_load(bpf_program__get_type(prog), > > nit: bpf_program__type() is preferred (we are deprecating/discouraging > "get_" prefixed getters in libbpf 1.0) ok, will change > > > + bpf_program__name(prog), "GPL", > > would it make sense to give each clone a distinct name? AFAICS the original code uses same prog name for instances, so I'd rather keep it that way thanks, jirka > > > + res.new_insn_ptr, > > + res.new_insn_cnt, NULL); > > + if (fd < 0) { > > + char bf[128]; > > + > > [...]
On Fri, Feb 18, 2022 at 10:01:45AM +0100, Jiri Olsa wrote: > On Thu, Feb 17, 2022 at 01:53:16PM -0800, Andrii Nakryiko wrote: > > On Thu, Feb 17, 2022 at 5:19 AM Jiri Olsa <jolsa@kernel.org> wrote: > > > > > > Some functions we use now for bpf prologue generation are > > > going to be deprecated, so reworking the current code not > > > to use them. > > > > > > We need to replace following functions/struct: > > > bpf_program__set_prep > > > bpf_program__nth_fd > > > struct bpf_prog_prep_result > > > > > > Current code uses bpf_program__set_prep to hook perf callback > > > before the program is loaded and provide new instructions with > > > the prologue. > > > > > > We workaround this by using objects's 'unloaded' programs instructions > > > for that specific program and load new ebpf programs with prologue > > > using separate bpf_prog_load calls. > > > > > > We keep new ebpf program instances descriptors in bpf programs > > > private struct. > > > > > > Suggested-by: Andrii Nakryiko <andrii@kernel.org> > > > Signed-off-by: Jiri Olsa <jolsa@kernel.org> > > > --- > > > tools/perf/util/bpf-loader.c | 122 +++++++++++++++++++++++++++++------ > > > 1 file changed, 104 insertions(+), 18 deletions(-) > > > > > > > [...] > > > > > errout: > > > @@ -696,7 +718,7 @@ static int hook_load_preprocessor(struct bpf_program *prog) > > > struct bpf_prog_priv *priv = program_priv(prog); > > > struct perf_probe_event *pev; > > > bool need_prologue = false; > > > - int err, i; > > > + int i; > > > > > > if (IS_ERR_OR_NULL(priv)) { > > > pr_debug("Internal error when hook preprocessor\n"); > > > @@ -727,6 +749,12 @@ static int hook_load_preprocessor(struct bpf_program *prog) > > > return 0; > > > } > > > > > > + /* > > > + * Do not load programs that need prologue, because we need > > > + * to add prologue first, check bpf_object__load_prologue. > > > + */ > > > + bpf_program__set_autoload(prog, false); > > > > if you set autoload to false, program instructions might be invalid in > > the end. Libbpf doesn't apply some (all?) relocations to such > > programs, doesn't resolve CO-RE, etc, etc. You have to let > > "prototypal" BPF program to be loaded before you can grab final > > instructions. It's not great, but in your case it should work, right? > > hum, do we care? it should all be done when the 'new' program with > the prologue is loaded, right? > > I switched it off because the verifier failed to load the program > without the prologue.. because in the originaal program there's no > code to grab the arguments that the rest of the code depends on, > so the verifier sees invalid access > > > > > > + > > > priv->need_prologue = true; > > > priv->insns_buf = malloc(sizeof(struct bpf_insn) * BPF_MAXINSNS); > > > if (!priv->insns_buf) { > > > @@ -734,6 +762,13 @@ static int hook_load_preprocessor(struct bpf_program *prog) > > > return -ENOMEM; > > > } > > > > > > > [...] > > > > > + /* > > > + * For each program that needs prologue we do following: > > > + * > > > + * - take its current instructions and use them > > > + * to generate the new code with prologue > > > + * > > > + * - load new instructions with bpf_prog_load > > > + * and keep the fd in proglogue_fds > > > + * > > > + * - new fd will be used bpf__foreach_event > > > + * to connect this program with perf evsel > > > + */ > > > + orig_insns = bpf_program__insns(prog); > > > + orig_insns_cnt = bpf_program__insn_cnt(prog); > > > + > > > + pev = &priv->pev; > > > + for (i = 0; i < pev->ntevs; i++) { > > > + err = preproc_gen_prologue(prog, i, orig_insns, > > > + orig_insns_cnt, &res); > > > + if (err) > > > + return err; > > > + > > > + fd = bpf_prog_load(bpf_program__get_type(prog), > > > > nit: bpf_program__type() is preferred (we are deprecating/discouraging > > "get_" prefixed getters in libbpf 1.0) > > ok, will change hum, I can't see bpf_program__type.. what do I miss? jirka
On Fri, Feb 18, 2022 at 02:03:28PM +0100, Jiri Olsa wrote: > On Fri, Feb 18, 2022 at 10:01:45AM +0100, Jiri Olsa wrote: > > On Thu, Feb 17, 2022 at 01:53:16PM -0800, Andrii Nakryiko wrote: > > > On Thu, Feb 17, 2022 at 5:19 AM Jiri Olsa <jolsa@kernel.org> wrote: > > > > > > > > Some functions we use now for bpf prologue generation are > > > > going to be deprecated, so reworking the current code not > > > > to use them. > > > > > > > > We need to replace following functions/struct: > > > > bpf_program__set_prep > > > > bpf_program__nth_fd > > > > struct bpf_prog_prep_result > > > > > > > > Current code uses bpf_program__set_prep to hook perf callback > > > > before the program is loaded and provide new instructions with > > > > the prologue. > > > > > > > > We workaround this by using objects's 'unloaded' programs instructions > > > > for that specific program and load new ebpf programs with prologue > > > > using separate bpf_prog_load calls. > > > > > > > > We keep new ebpf program instances descriptors in bpf programs > > > > private struct. > > > > > > > > Suggested-by: Andrii Nakryiko <andrii@kernel.org> > > > > Signed-off-by: Jiri Olsa <jolsa@kernel.org> > > > > --- > > > > tools/perf/util/bpf-loader.c | 122 +++++++++++++++++++++++++++++------ > > > > 1 file changed, 104 insertions(+), 18 deletions(-) > > > > > > > > > > [...] > > > > > > > errout: > > > > @@ -696,7 +718,7 @@ static int hook_load_preprocessor(struct bpf_program *prog) > > > > struct bpf_prog_priv *priv = program_priv(prog); > > > > struct perf_probe_event *pev; > > > > bool need_prologue = false; > > > > - int err, i; > > > > + int i; > > > > > > > > if (IS_ERR_OR_NULL(priv)) { > > > > pr_debug("Internal error when hook preprocessor\n"); > > > > @@ -727,6 +749,12 @@ static int hook_load_preprocessor(struct bpf_program *prog) > > > > return 0; > > > > } > > > > > > > > + /* > > > > + * Do not load programs that need prologue, because we need > > > > + * to add prologue first, check bpf_object__load_prologue. > > > > + */ > > > > + bpf_program__set_autoload(prog, false); > > > > > > if you set autoload to false, program instructions might be invalid in > > > the end. Libbpf doesn't apply some (all?) relocations to such > > > programs, doesn't resolve CO-RE, etc, etc. You have to let > > > "prototypal" BPF program to be loaded before you can grab final > > > instructions. It's not great, but in your case it should work, right? > > > > hum, do we care? it should all be done when the 'new' program with > > the prologue is loaded, right? > > > > I switched it off because the verifier failed to load the program > > without the prologue.. because in the originaal program there's no > > code to grab the arguments that the rest of the code depends on, > > so the verifier sees invalid access > > > > > > > > > + > > > > priv->need_prologue = true; > > > > priv->insns_buf = malloc(sizeof(struct bpf_insn) * BPF_MAXINSNS); > > > > if (!priv->insns_buf) { > > > > @@ -734,6 +762,13 @@ static int hook_load_preprocessor(struct bpf_program *prog) > > > > return -ENOMEM; > > > > } > > > > > > > > > > [...] > > > > > > > + /* > > > > + * For each program that needs prologue we do following: > > > > + * > > > > + * - take its current instructions and use them > > > > + * to generate the new code with prologue > > > > + * > > > > + * - load new instructions with bpf_prog_load > > > > + * and keep the fd in proglogue_fds > > > > + * > > > > + * - new fd will be used bpf__foreach_event > > > > + * to connect this program with perf evsel > > > > + */ > > > > + orig_insns = bpf_program__insns(prog); > > > > + orig_insns_cnt = bpf_program__insn_cnt(prog); > > > > + > > > > + pev = &priv->pev; > > > > + for (i = 0; i < pev->ntevs; i++) { > > > > + err = preproc_gen_prologue(prog, i, orig_insns, > > > > + orig_insns_cnt, &res); > > > > + if (err) > > > > + return err; > > > > + > > > > + fd = bpf_prog_load(bpf_program__get_type(prog), > > > > > > nit: bpf_program__type() is preferred (we are deprecating/discouraging > > > "get_" prefixed getters in libbpf 1.0) > > > > ok, will change > > hum, I can't see bpf_program__type.. what do I miss? nah I was on top of perf/core.. I see it now ;-) jirka
On Fri, Feb 18, 2022 at 1:01 AM Jiri Olsa <olsajiri@gmail.com> wrote: > > On Thu, Feb 17, 2022 at 01:53:16PM -0800, Andrii Nakryiko wrote: > > On Thu, Feb 17, 2022 at 5:19 AM Jiri Olsa <jolsa@kernel.org> wrote: > > > > > > Some functions we use now for bpf prologue generation are > > > going to be deprecated, so reworking the current code not > > > to use them. > > > > > > We need to replace following functions/struct: > > > bpf_program__set_prep > > > bpf_program__nth_fd > > > struct bpf_prog_prep_result > > > > > > Current code uses bpf_program__set_prep to hook perf callback > > > before the program is loaded and provide new instructions with > > > the prologue. > > > > > > We workaround this by using objects's 'unloaded' programs instructions > > > for that specific program and load new ebpf programs with prologue > > > using separate bpf_prog_load calls. > > > > > > We keep new ebpf program instances descriptors in bpf programs > > > private struct. > > > > > > Suggested-by: Andrii Nakryiko <andrii@kernel.org> > > > Signed-off-by: Jiri Olsa <jolsa@kernel.org> > > > --- > > > tools/perf/util/bpf-loader.c | 122 +++++++++++++++++++++++++++++------ > > > 1 file changed, 104 insertions(+), 18 deletions(-) > > > > > > > [...] > > > > > errout: > > > @@ -696,7 +718,7 @@ static int hook_load_preprocessor(struct bpf_program *prog) > > > struct bpf_prog_priv *priv = program_priv(prog); > > > struct perf_probe_event *pev; > > > bool need_prologue = false; > > > - int err, i; > > > + int i; > > > > > > if (IS_ERR_OR_NULL(priv)) { > > > pr_debug("Internal error when hook preprocessor\n"); > > > @@ -727,6 +749,12 @@ static int hook_load_preprocessor(struct bpf_program *prog) > > > return 0; > > > } > > > > > > + /* > > > + * Do not load programs that need prologue, because we need > > > + * to add prologue first, check bpf_object__load_prologue. > > > + */ > > > + bpf_program__set_autoload(prog, false); > > > > if you set autoload to false, program instructions might be invalid in > > the end. Libbpf doesn't apply some (all?) relocations to such > > programs, doesn't resolve CO-RE, etc, etc. You have to let > > "prototypal" BPF program to be loaded before you can grab final > > instructions. It's not great, but in your case it should work, right? > > hum, do we care? it should all be done when the 'new' program with > the prologue is loaded, right? yeah, you should care. If there is any BPF map involved, it is properly resolved to correct FD (which is put into ldimm64 instruction in BPF program code) during the load. If program is not autoloaded, this is skipped. Same for any global variable or subprog call (if it's not always inlined). So you very much should care for any non-trivial program. > > I switched it off because the verifier failed to load the program > without the prologue.. because in the original program there's no > code to grab the arguments that the rest of the code depends on, > so the verifier sees invalid access Do you have an example of C code and corresponding BPF instructions before/after prologue generation? Just curious to see in details how this is done. > > > > > > + > > > priv->need_prologue = true; > > > priv->insns_buf = malloc(sizeof(struct bpf_insn) * BPF_MAXINSNS); > > > if (!priv->insns_buf) { > > > @@ -734,6 +762,13 @@ static int hook_load_preprocessor(struct bpf_program *prog) > > > return -ENOMEM; > > > } > > > > > > > [...] > > > > > + /* > > > + * For each program that needs prologue we do following: > > > + * > > > + * - take its current instructions and use them > > > + * to generate the new code with prologue > > > + * > > > + * - load new instructions with bpf_prog_load > > > + * and keep the fd in proglogue_fds > > > + * > > > + * - new fd will be used bpf__foreach_event > > > + * to connect this program with perf evsel > > > + */ > > > + orig_insns = bpf_program__insns(prog); > > > + orig_insns_cnt = bpf_program__insn_cnt(prog); > > > + > > > + pev = &priv->pev; > > > + for (i = 0; i < pev->ntevs; i++) { > > > + err = preproc_gen_prologue(prog, i, orig_insns, > > > + orig_insns_cnt, &res); > > > + if (err) > > > + return err; > > > + > > > + fd = bpf_prog_load(bpf_program__get_type(prog), > > > > nit: bpf_program__type() is preferred (we are deprecating/discouraging > > "get_" prefixed getters in libbpf 1.0) > > ok, will change It's been added in v0.7, yeah. > > > > > > + bpf_program__name(prog), "GPL", > > > > would it make sense to give each clone a distinct name? > > AFAICS the original code uses same prog name for instances, > so I'd rather keep it that way > sure, np > thanks, > jirka > > > > > > + res.new_insn_ptr, > > > + res.new_insn_cnt, NULL); > > > + if (fd < 0) { > > > + char bf[128]; > > > + > > > > [...]
On Fri, Feb 18, 2022 at 11:55:16AM -0800, Andrii Nakryiko wrote: > On Fri, Feb 18, 2022 at 1:01 AM Jiri Olsa <olsajiri@gmail.com> wrote: > > > > On Thu, Feb 17, 2022 at 01:53:16PM -0800, Andrii Nakryiko wrote: > > > On Thu, Feb 17, 2022 at 5:19 AM Jiri Olsa <jolsa@kernel.org> wrote: > > > > > > > > Some functions we use now for bpf prologue generation are > > > > going to be deprecated, so reworking the current code not > > > > to use them. > > > > > > > > We need to replace following functions/struct: > > > > bpf_program__set_prep > > > > bpf_program__nth_fd > > > > struct bpf_prog_prep_result > > > > > > > > Current code uses bpf_program__set_prep to hook perf callback > > > > before the program is loaded and provide new instructions with > > > > the prologue. > > > > > > > > We workaround this by using objects's 'unloaded' programs instructions > > > > for that specific program and load new ebpf programs with prologue > > > > using separate bpf_prog_load calls. > > > > > > > > We keep new ebpf program instances descriptors in bpf programs > > > > private struct. > > > > > > > > Suggested-by: Andrii Nakryiko <andrii@kernel.org> > > > > Signed-off-by: Jiri Olsa <jolsa@kernel.org> > > > > --- > > > > tools/perf/util/bpf-loader.c | 122 +++++++++++++++++++++++++++++------ > > > > 1 file changed, 104 insertions(+), 18 deletions(-) > > > > > > > > > > [...] > > > > > > > errout: > > > > @@ -696,7 +718,7 @@ static int hook_load_preprocessor(struct bpf_program *prog) > > > > struct bpf_prog_priv *priv = program_priv(prog); > > > > struct perf_probe_event *pev; > > > > bool need_prologue = false; > > > > - int err, i; > > > > + int i; > > > > > > > > if (IS_ERR_OR_NULL(priv)) { > > > > pr_debug("Internal error when hook preprocessor\n"); > > > > @@ -727,6 +749,12 @@ static int hook_load_preprocessor(struct bpf_program *prog) > > > > return 0; > > > > } > > > > > > > > + /* > > > > + * Do not load programs that need prologue, because we need > > > > + * to add prologue first, check bpf_object__load_prologue. > > > > + */ > > > > + bpf_program__set_autoload(prog, false); > > > > > > if you set autoload to false, program instructions might be invalid in > > > the end. Libbpf doesn't apply some (all?) relocations to such > > > programs, doesn't resolve CO-RE, etc, etc. You have to let > > > "prototypal" BPF program to be loaded before you can grab final > > > instructions. It's not great, but in your case it should work, right? > > > > hum, do we care? it should all be done when the 'new' program with > > the prologue is loaded, right? > > yeah, you should care. If there is any BPF map involved, it is > properly resolved to correct FD (which is put into ldimm64 instruction > in BPF program code) during the load. If program is not autoloaded, > this is skipped. Same for any global variable or subprog call (if it's > not always inlined). So you very much should care for any non-trivial > program. ah too bad.. all that is in the load path, ok > > > > > I switched it off because the verifier failed to load the program > > without the prologue.. because in the original program there's no > > code to grab the arguments that the rest of the code depends on, > > so the verifier sees invalid access > > Do you have an example of C code and corresponding BPF instructions > before/after prologue generation? Just curious to see in details how > this is done. so with following example: SEC("func=do_sched_setscheduler param->sched_priority@user") int bpf_func__setscheduler(void *ctx, int err, int param) { char fmt[] = "prio: %ld"; bpf_trace_printk(fmt, sizeof(fmt), param); return 1; } perf will attach the code to do_sched_setscheduler function, and read 'param->sched_priority' into 'param' argument so the resulting clang object expects 'param' to be in R3 0000000000000000 <bpf_func__setscheduler>: 0: b7 01 00 00 64 00 00 00 r1 = 100 1: 6b 1a f8 ff 00 00 00 00 *(u16 *)(r10 - 8) = r1 2: 18 01 00 00 70 72 69 6f 00 00 00 00 3a 20 25 6c r1 = 77926701655 4: 7b 1a f0 ff 00 00 00 00 *(u64 *)(r10 - 16) = r1 5: bf a1 00 00 00 00 00 00 r1 = r10 6: 07 01 00 00 f0 ff ff ff r1 += -16 7: b7 02 00 00 0a 00 00 00 r2 = 10 8: 85 00 00 00 06 00 00 00 call 6 9: b7 00 00 00 01 00 00 00 r0 = 1 10: 95 00 00 00 00 00 00 00 exit and R3 is loaded in the prologue code (first 15 instructions) and it also sets 'err' (R2) with the result of the reading: 0: (bf) r6 = r1 1: (79) r3 = *(u64 *)(r6 +96) 2: (bf) r7 = r10 3: (07) r7 += -8 4: (7b) *(u64 *)(r10 -8) = r3 5: (b7) r2 = 8 6: (bf) r1 = r7 7: (85) call bpf_probe_read_user#-60848 8: (55) if r0 != 0x0 goto pc+2 9: (61) r3 = *(u32 *)(r10 -8) 10: (05) goto pc+3 11: (b7) r2 = 1 12: (b7) r3 = 0 13: (05) goto pc+1 14: (b7) r2 = 0 15: (bf) r1 = r6 16: (b7) r1 = 100 17: (6b) *(u16 *)(r10 -8) = r1 18: (18) r1 = 0x6c25203a6f697270 20: (7b) *(u64 *)(r10 -16) = r1 21: (bf) r1 = r10 22: (07) r1 += -16 23: (b7) r2 = 10 24: (85) call bpf_trace_printk#-54848 25: (b7) r0 = 1 26: (95) exit I'm still scratching my head how to workaround this.. we do want maps and all the other updates to the code, but verifier won't let it pass without the prologue code jirka
On Sun, Feb 20, 2022 at 10:43:42AM -0800, Yonghong Song wrote: > > > On 2/20/22 5:44 AM, Jiri Olsa wrote: > > On Fri, Feb 18, 2022 at 11:55:16AM -0800, Andrii Nakryiko wrote: > > > On Fri, Feb 18, 2022 at 1:01 AM Jiri Olsa <olsajiri@gmail.com> wrote: > > > > > > > > On Thu, Feb 17, 2022 at 01:53:16PM -0800, Andrii Nakryiko wrote: > > > > > On Thu, Feb 17, 2022 at 5:19 AM Jiri Olsa <jolsa@kernel.org> wrote: > > > > > > > > > > > > Some functions we use now for bpf prologue generation are > > > > > > going to be deprecated, so reworking the current code not > > > > > > to use them. > > > > > > > > > > > > We need to replace following functions/struct: > > > > > > bpf_program__set_prep > > > > > > bpf_program__nth_fd > > > > > > struct bpf_prog_prep_result > > > > > > > > > > > > Current code uses bpf_program__set_prep to hook perf callback > > > > > > before the program is loaded and provide new instructions with > > > > > > the prologue. > > > > > > > > > > > > We workaround this by using objects's 'unloaded' programs instructions > > > > > > for that specific program and load new ebpf programs with prologue > > > > > > using separate bpf_prog_load calls. > > > > > > > > > > > > We keep new ebpf program instances descriptors in bpf programs > > > > > > private struct. > > > > > > > > > > > > Suggested-by: Andrii Nakryiko <andrii@kernel.org> > > > > > > Signed-off-by: Jiri Olsa <jolsa@kernel.org> > > > > > > --- > > > > > > tools/perf/util/bpf-loader.c | 122 +++++++++++++++++++++++++++++------ > > > > > > 1 file changed, 104 insertions(+), 18 deletions(-) > > > > > > > > > > > > > > > > [...] > > > > > > > > > > > errout: > > > > > > @@ -696,7 +718,7 @@ static int hook_load_preprocessor(struct bpf_program *prog) > > > > > > struct bpf_prog_priv *priv = program_priv(prog); > > > > > > struct perf_probe_event *pev; > > > > > > bool need_prologue = false; > > > > > > - int err, i; > > > > > > + int i; > > > > > > > > > > > > if (IS_ERR_OR_NULL(priv)) { > > > > > > pr_debug("Internal error when hook preprocessor\n"); > > > > > > @@ -727,6 +749,12 @@ static int hook_load_preprocessor(struct bpf_program *prog) > > > > > > return 0; > > > > > > } > > > > > > > > > > > > + /* > > > > > > + * Do not load programs that need prologue, because we need > > > > > > + * to add prologue first, check bpf_object__load_prologue. > > > > > > + */ > > > > > > + bpf_program__set_autoload(prog, false); > > > > > > > > > > if you set autoload to false, program instructions might be invalid in > > > > > the end. Libbpf doesn't apply some (all?) relocations to such > > > > > programs, doesn't resolve CO-RE, etc, etc. You have to let > > > > > "prototypal" BPF program to be loaded before you can grab final > > > > > instructions. It's not great, but in your case it should work, right? > > > > > > > > hum, do we care? it should all be done when the 'new' program with > > > > the prologue is loaded, right? > > > > > > yeah, you should care. If there is any BPF map involved, it is > > > properly resolved to correct FD (which is put into ldimm64 instruction > > > in BPF program code) during the load. If program is not autoloaded, > > > this is skipped. Same for any global variable or subprog call (if it's > > > not always inlined). So you very much should care for any non-trivial > > > program. > > > > ah too bad.. all that is in the load path, ok > > > > > > > > > > > > > I switched it off because the verifier failed to load the program > > > > without the prologue.. because in the original program there's no > > > > code to grab the arguments that the rest of the code depends on, > > > > so the verifier sees invalid access > > > > > > Do you have an example of C code and corresponding BPF instructions > > > before/after prologue generation? Just curious to see in details how > > > this is done. > > > > so with following example: > > > > SEC("func=do_sched_setscheduler param->sched_priority@user") > > int bpf_func__setscheduler(void *ctx, int err, int param) > > { > > char fmt[] = "prio: %ld"; > > bpf_trace_printk(fmt, sizeof(fmt), param); > > return 1; > > } > > > > perf will attach the code to do_sched_setscheduler function, > > and read 'param->sched_priority' into 'param' argument > > > > so the resulting clang object expects 'param' to be in R3 > > > > 0000000000000000 <bpf_func__setscheduler>: > > 0: b7 01 00 00 64 00 00 00 r1 = 100 > > 1: 6b 1a f8 ff 00 00 00 00 *(u16 *)(r10 - 8) = r1 > > 2: 18 01 00 00 70 72 69 6f 00 00 00 00 3a 20 25 6c r1 = 77926701655 > > 4: 7b 1a f0 ff 00 00 00 00 *(u64 *)(r10 - 16) = r1 > > 5: bf a1 00 00 00 00 00 00 r1 = r10 > > 6: 07 01 00 00 f0 ff ff ff r1 += -16 > > 7: b7 02 00 00 0a 00 00 00 r2 = 10 > > 8: 85 00 00 00 06 00 00 00 call 6 > > 9: b7 00 00 00 01 00 00 00 r0 = 1 > > 10: 95 00 00 00 00 00 00 00 exit > > > > and R3 is loaded in the prologue code (first 15 instructions) > > and it also sets 'err' (R2) with the result of the reading: > > > > 0: (bf) r6 = r1 > > 1: (79) r3 = *(u64 *)(r6 +96) > > 2: (bf) r7 = r10 > > 3: (07) r7 += -8 > > 4: (7b) *(u64 *)(r10 -8) = r3 > > 5: (b7) r2 = 8 > > 6: (bf) r1 = r7 > > 7: (85) call bpf_probe_read_user#-60848 > > 8: (55) if r0 != 0x0 goto pc+2 > > 9: (61) r3 = *(u32 *)(r10 -8) > > 10: (05) goto pc+3 > > 11: (b7) r2 = 1 > > 12: (b7) r3 = 0 > > 13: (05) goto pc+1 > > 14: (b7) r2 = 0 > > 15: (bf) r1 = r6 > > > > 16: (b7) r1 = 100 > > 17: (6b) *(u16 *)(r10 -8) = r1 > > 18: (18) r1 = 0x6c25203a6f697270 > > 20: (7b) *(u64 *)(r10 -16) = r1 > > 21: (bf) r1 = r10 > > 22: (07) r1 += -16 > > 23: (b7) r2 = 10 > > 24: (85) call bpf_trace_printk#-54848 > > 25: (b7) r0 = 1 > > 26: (95) exit > > Just curious. Is the prologue code generated through C code or through > asm code? Is it possible prologue code can be generated through C it's C code in perf generating bpf instructions: https://git.kernel.org/pub/scm/linux/kernel/git/acme/linux.git/tree/tools/perf/util/bpf-prologue.c?h=perf/core > code with similar mechanism like BPF_PROG macro? Or this is already > an API which cannot be changed? do you mean to have some stub like: int bpf_func__setscheduler_stub(void *ctx) { return bpf_func__setscheduler(ctx, 0, 0) } int bpf_func__setscheduler(void *ctx, int err, int param) { char fmt[] = "prio: %ld"; bpf_trace_printk(fmt, sizeof(fmt), param); return 1; } to make verifier happy then we'd need instructions for bpf_func__setscheduler it looks like subprogram instructions are appended and we should be able to locate bpf_func__setscheduler start in instructions returned in bpf_program__insns ? anyway does not look nice ;-) jirka
On Sun, Feb 20, 2022 at 5:44 AM Jiri Olsa <olsajiri@gmail.com> wrote: > > On Fri, Feb 18, 2022 at 11:55:16AM -0800, Andrii Nakryiko wrote: > > On Fri, Feb 18, 2022 at 1:01 AM Jiri Olsa <olsajiri@gmail.com> wrote: > > > > > > On Thu, Feb 17, 2022 at 01:53:16PM -0800, Andrii Nakryiko wrote: > > > > On Thu, Feb 17, 2022 at 5:19 AM Jiri Olsa <jolsa@kernel.org> wrote: > > > > > > > > > > Some functions we use now for bpf prologue generation are > > > > > going to be deprecated, so reworking the current code not > > > > > to use them. > > > > > > > > > > We need to replace following functions/struct: > > > > > bpf_program__set_prep > > > > > bpf_program__nth_fd > > > > > struct bpf_prog_prep_result > > > > > > > > > > Current code uses bpf_program__set_prep to hook perf callback > > > > > before the program is loaded and provide new instructions with > > > > > the prologue. > > > > > > > > > > We workaround this by using objects's 'unloaded' programs instructions > > > > > for that specific program and load new ebpf programs with prologue > > > > > using separate bpf_prog_load calls. > > > > > > > > > > We keep new ebpf program instances descriptors in bpf programs > > > > > private struct. > > > > > > > > > > Suggested-by: Andrii Nakryiko <andrii@kernel.org> > > > > > Signed-off-by: Jiri Olsa <jolsa@kernel.org> > > > > > --- > > > > > tools/perf/util/bpf-loader.c | 122 +++++++++++++++++++++++++++++------ > > > > > 1 file changed, 104 insertions(+), 18 deletions(-) > > > > > > > > > > > > > [...] > > > > > > > > > errout: > > > > > @@ -696,7 +718,7 @@ static int hook_load_preprocessor(struct bpf_program *prog) > > > > > struct bpf_prog_priv *priv = program_priv(prog); > > > > > struct perf_probe_event *pev; > > > > > bool need_prologue = false; > > > > > - int err, i; > > > > > + int i; > > > > > > > > > > if (IS_ERR_OR_NULL(priv)) { > > > > > pr_debug("Internal error when hook preprocessor\n"); > > > > > @@ -727,6 +749,12 @@ static int hook_load_preprocessor(struct bpf_program *prog) > > > > > return 0; > > > > > } > > > > > > > > > > + /* > > > > > + * Do not load programs that need prologue, because we need > > > > > + * to add prologue first, check bpf_object__load_prologue. > > > > > + */ > > > > > + bpf_program__set_autoload(prog, false); > > > > > > > > if you set autoload to false, program instructions might be invalid in > > > > the end. Libbpf doesn't apply some (all?) relocations to such > > > > programs, doesn't resolve CO-RE, etc, etc. You have to let > > > > "prototypal" BPF program to be loaded before you can grab final > > > > instructions. It's not great, but in your case it should work, right? > > > > > > hum, do we care? it should all be done when the 'new' program with > > > the prologue is loaded, right? > > > > yeah, you should care. If there is any BPF map involved, it is > > properly resolved to correct FD (which is put into ldimm64 instruction > > in BPF program code) during the load. If program is not autoloaded, > > this is skipped. Same for any global variable or subprog call (if it's > > not always inlined). So you very much should care for any non-trivial > > program. > > ah too bad.. all that is in the load path, ok > > > > > > > > > I switched it off because the verifier failed to load the program > > > without the prologue.. because in the original program there's no > > > code to grab the arguments that the rest of the code depends on, > > > so the verifier sees invalid access > > > > Do you have an example of C code and corresponding BPF instructions > > before/after prologue generation? Just curious to see in details how > > this is done. > > so with following example: > > SEC("func=do_sched_setscheduler param->sched_priority@user") > int bpf_func__setscheduler(void *ctx, int err, int param) > { > char fmt[] = "prio: %ld"; > bpf_trace_printk(fmt, sizeof(fmt), param); > return 1; > } > > perf will attach the code to do_sched_setscheduler function, > and read 'param->sched_priority' into 'param' argument > > so the resulting clang object expects 'param' to be in R3 > > 0000000000000000 <bpf_func__setscheduler>: > 0: b7 01 00 00 64 00 00 00 r1 = 100 > 1: 6b 1a f8 ff 00 00 00 00 *(u16 *)(r10 - 8) = r1 > 2: 18 01 00 00 70 72 69 6f 00 00 00 00 3a 20 25 6c r1 = 77926701655 > 4: 7b 1a f0 ff 00 00 00 00 *(u64 *)(r10 - 16) = r1 > 5: bf a1 00 00 00 00 00 00 r1 = r10 > 6: 07 01 00 00 f0 ff ff ff r1 += -16 > 7: b7 02 00 00 0a 00 00 00 r2 = 10 > 8: 85 00 00 00 06 00 00 00 call 6 > 9: b7 00 00 00 01 00 00 00 r0 = 1 > 10: 95 00 00 00 00 00 00 00 exit > > and R3 is loaded in the prologue code (first 15 instructions) > and it also sets 'err' (R2) with the result of the reading: > > 0: (bf) r6 = r1 > 1: (79) r3 = *(u64 *)(r6 +96) > 2: (bf) r7 = r10 > 3: (07) r7 += -8 > 4: (7b) *(u64 *)(r10 -8) = r3 > 5: (b7) r2 = 8 > 6: (bf) r1 = r7 > 7: (85) call bpf_probe_read_user#-60848 > 8: (55) if r0 != 0x0 goto pc+2 > 9: (61) r3 = *(u32 *)(r10 -8) > 10: (05) goto pc+3 > 11: (b7) r2 = 1 > 12: (b7) r3 = 0 > 13: (05) goto pc+1 > 14: (b7) r2 = 0 > 15: (bf) r1 = r6 > > 16: (b7) r1 = 100 > 17: (6b) *(u16 *)(r10 -8) = r1 > 18: (18) r1 = 0x6c25203a6f697270 > 20: (7b) *(u64 *)(r10 -16) = r1 > 21: (bf) r1 = r10 > 22: (07) r1 += -16 > 23: (b7) r2 = 10 > 24: (85) call bpf_trace_printk#-54848 > 25: (b7) r0 = 1 > 26: (95) exit > > > I'm still scratching my head how to workaround this.. we do want maps > and all the other updates to the code, but verifier won't let it pass > without the prologue code ugh, perf cornered itself into supporting this crazy scheme and now there is no good solution. I'm still questioning the value of supporting this going forward. Is there an evidence that anyone is using this functionality at all? Is it worth it trying to carry it on just because we have some example that exercises this feature? Anyways, one way to solve this is to add bpf_program__set_insns() that could be called from prog_init_fn callback (which I just realized hasn't landed yet, I'll send v4 today) to prepend a simple preamble like this: r1 = 0; r2 = 0; r3 = 0; f4 = 0; r5 = 0; /* how many input arguments we support? */ This will make all input arguments initialized, libbpf will be able to adjust all the relocations and stuff. Once this "prototype program" is loaded, perf can grab final instructions and replace first X instructions with desired preamble. But... ugliness and horror, yeah :( > > jirka
On Wed, Feb 23, 2022 at 02:29:56PM -0800, Andrii Nakryiko wrote: SNIP > > and R3 is loaded in the prologue code (first 15 instructions) > > and it also sets 'err' (R2) with the result of the reading: > > > > 0: (bf) r6 = r1 > > 1: (79) r3 = *(u64 *)(r6 +96) > > 2: (bf) r7 = r10 > > 3: (07) r7 += -8 > > 4: (7b) *(u64 *)(r10 -8) = r3 > > 5: (b7) r2 = 8 > > 6: (bf) r1 = r7 > > 7: (85) call bpf_probe_read_user#-60848 > > 8: (55) if r0 != 0x0 goto pc+2 > > 9: (61) r3 = *(u32 *)(r10 -8) > > 10: (05) goto pc+3 > > 11: (b7) r2 = 1 > > 12: (b7) r3 = 0 > > 13: (05) goto pc+1 > > 14: (b7) r2 = 0 > > 15: (bf) r1 = r6 > > > > 16: (b7) r1 = 100 > > 17: (6b) *(u16 *)(r10 -8) = r1 > > 18: (18) r1 = 0x6c25203a6f697270 > > 20: (7b) *(u64 *)(r10 -16) = r1 > > 21: (bf) r1 = r10 > > 22: (07) r1 += -16 > > 23: (b7) r2 = 10 > > 24: (85) call bpf_trace_printk#-54848 > > 25: (b7) r0 = 1 > > 26: (95) exit > > > > > > I'm still scratching my head how to workaround this.. we do want maps > > and all the other updates to the code, but verifier won't let it pass > > without the prologue code > > ugh, perf cornered itself into supporting this crazy scheme and now well, it just used the interface that was provided at the time > there is no good solution. I'm still questioning the value of > supporting this going forward. Is there an evidence that anyone is > using this functionality at all? Is it worth it trying to carry it on > just because we have some example that exercises this feature? yea we discussed this again and I think we can somehow mark this feature in perf as deprecated and remove it after some time, because even with the workaround below it'll be pita ;-) or people will come and scream and we will find some other solution I already sent the rest of the changes (prog/map priv) separately and will send some RFC for the deprecation thanks, jirka > > Anyways, one way to solve this is to add bpf_program__set_insns() that > could be called from prog_init_fn callback (which I just realized > hasn't landed yet, I'll send v4 today) to prepend a simple preamble > like this: > > r1 = 0; > r2 = 0; > r3 = 0; > f4 = 0; > r5 = 0; /* how many input arguments we support? */ > > This will make all input arguments initialized, libbpf will be able to > adjust all the relocations and stuff. Once this "prototype program" is > loaded, perf can grab final instructions and replace first X > instructions with desired preamble. > > But... ugliness and horror, yeah :( > > > > > > jirka
Em Fri, Feb 25, 2022 at 01:14:49PM +0100, Jiri Olsa escreveu: > On Wed, Feb 23, 2022 at 02:29:56PM -0800, Andrii Nakryiko wrote: > > SNIP > > > > and R3 is loaded in the prologue code (first 15 instructions) > > > and it also sets 'err' (R2) with the result of the reading: > > > > > > 0: (bf) r6 = r1 > > > 1: (79) r3 = *(u64 *)(r6 +96) > > > 2: (bf) r7 = r10 > > > 3: (07) r7 += -8 > > > 4: (7b) *(u64 *)(r10 -8) = r3 > > > 5: (b7) r2 = 8 > > > 6: (bf) r1 = r7 > > > 7: (85) call bpf_probe_read_user#-60848 > > > 8: (55) if r0 != 0x0 goto pc+2 > > > 9: (61) r3 = *(u32 *)(r10 -8) > > > 10: (05) goto pc+3 > > > 11: (b7) r2 = 1 > > > 12: (b7) r3 = 0 > > > 13: (05) goto pc+1 > > > 14: (b7) r2 = 0 > > > 15: (bf) r1 = r6 > > > > > > 16: (b7) r1 = 100 > > > 17: (6b) *(u16 *)(r10 -8) = r1 > > > 18: (18) r1 = 0x6c25203a6f697270 > > > 20: (7b) *(u64 *)(r10 -16) = r1 > > > 21: (bf) r1 = r10 > > > 22: (07) r1 += -16 > > > 23: (b7) r2 = 10 > > > 24: (85) call bpf_trace_printk#-54848 > > > 25: (b7) r0 = 1 > > > 26: (95) exit > > > > > > > > > I'm still scratching my head how to workaround this.. we do want maps > > > and all the other updates to the code, but verifier won't let it pass > > > without the prologue code > > > > ugh, perf cornered itself into supporting this crazy scheme and now > well, it just used the interface that was provided at the time At the time it was where experimentation was done with tooling for eBPF, Wangnan tried to provide a compact way to give access to parameters. The problem now is for libbpf to remove something that is used and that was documented to some extent in the perf tools examples so there _may_ be some usage of it, we just can't know. Its like Linux removing some syscall that is "crazy" and wait for somebody to complain of the breakage caused when they update to a new version. > > there is no good solution. I'm still questioning the value of > > supporting this going forward. Is there an evidence that anyone is > > using this functionality at all? Is it worth it trying to carry it on > > just because we have some example that exercises this feature? > yea we discussed this again and I think we can somehow mark this > feature in perf as deprecated and remove it after some time, > because even with the workaround below it'll be pita ;-) > > or people will come and scream and we will find some other solution :-\ if you have some "ugly" way to keep the feature, can't we go with it? > I already sent the rest of the changes (prog/map priv) separately > and will send some RFC for the deprecation I'll look at it now. Thanks for your work on this, Jiri. - Araldo > thanks, > jirka > > > > > Anyways, one way to solve this is to add bpf_program__set_insns() that > > could be called from prog_init_fn callback (which I just realized > > hasn't landed yet, I'll send v4 today) to prepend a simple preamble > > like this: > > > > r1 = 0; > > r2 = 0; > > r3 = 0; > > f4 = 0; > > r5 = 0; /* how many input arguments we support? */ > > > > This will make all input arguments initialized, libbpf will be able to > > adjust all the relocations and stuff. Once this "prototype program" is > > loaded, perf can grab final instructions and replace first X > > instructions with desired preamble. > > > > But... ugliness and horror, yeah :( > > > > > > > > > > jirka
diff --git a/tools/perf/util/bpf-loader.c b/tools/perf/util/bpf-loader.c index 6f87729817ad..03f917002c00 100644 --- a/tools/perf/util/bpf-loader.c +++ b/tools/perf/util/bpf-loader.c @@ -48,6 +48,7 @@ struct bpf_prog_priv { struct bpf_insn *insns_buf; int nr_types; int *type_mapping; + int *proglogue_fds; }; struct bpf_perf_object { @@ -55,6 +56,11 @@ struct bpf_perf_object { struct bpf_object *obj; }; +struct bpf_preproc_result { + struct bpf_insn *new_insn_ptr; + int new_insn_cnt; +}; + static LIST_HEAD(bpf_objects_list); static struct hashmap *bpf_program_hash; static struct hashmap *bpf_map_hash; @@ -176,14 +182,31 @@ struct bpf_object *bpf__prepare_load(const char *filename, bool source) return obj; } +static void close_prologue_programs(struct bpf_prog_priv *priv) +{ + struct perf_probe_event *pev; + int i, fd; + + if (!priv->need_prologue) + return; + pev = &priv->pev; + for (i = 0; i < pev->ntevs; i++) { + fd = close(priv->proglogue_fds[i]); + if (fd != -1) + close(fd); + } +} + static void clear_prog_priv(const struct bpf_program *prog __maybe_unused, void *_priv) { struct bpf_prog_priv *priv = _priv; + close_prologue_programs(priv); cleanup_perf_probe_events(&priv->pev, 1); zfree(&priv->insns_buf); + zfree(&priv->proglogue_fds); zfree(&priv->type_mapping); zfree(&priv->sys_name); zfree(&priv->evt_name); @@ -539,8 +562,8 @@ static int bpf__prepare_probe(void) static int preproc_gen_prologue(struct bpf_program *prog, int n, - struct bpf_insn *orig_insns, int orig_insns_cnt, - struct bpf_prog_prep_result *res) + const struct bpf_insn *orig_insns, int orig_insns_cnt, + struct bpf_preproc_result *res) { struct bpf_prog_priv *priv = program_priv(prog); struct probe_trace_event *tev; @@ -588,7 +611,6 @@ preproc_gen_prologue(struct bpf_program *prog, int n, res->new_insn_ptr = buf; res->new_insn_cnt = prologue_cnt + orig_insns_cnt; - res->pfd = NULL; return 0; errout: @@ -696,7 +718,7 @@ static int hook_load_preprocessor(struct bpf_program *prog) struct bpf_prog_priv *priv = program_priv(prog); struct perf_probe_event *pev; bool need_prologue = false; - int err, i; + int i; if (IS_ERR_OR_NULL(priv)) { pr_debug("Internal error when hook preprocessor\n"); @@ -727,6 +749,12 @@ static int hook_load_preprocessor(struct bpf_program *prog) return 0; } + /* + * Do not load programs that need prologue, because we need + * to add prologue first, check bpf_object__load_prologue. + */ + bpf_program__set_autoload(prog, false); + priv->need_prologue = true; priv->insns_buf = malloc(sizeof(struct bpf_insn) * BPF_MAXINSNS); if (!priv->insns_buf) { @@ -734,6 +762,13 @@ static int hook_load_preprocessor(struct bpf_program *prog) return -ENOMEM; } + priv->proglogue_fds = malloc(sizeof(int) * pev->ntevs); + if (!priv->proglogue_fds) { + pr_debug("Not enough memory: alloc prologue fds failed\n"); + return -ENOMEM; + } + memset(priv->proglogue_fds, -1, sizeof(int) * pev->ntevs); + priv->type_mapping = malloc(sizeof(int) * pev->ntevs); if (!priv->type_mapping) { pr_debug("Not enough memory: alloc type_mapping failed\n"); @@ -742,13 +777,7 @@ static int hook_load_preprocessor(struct bpf_program *prog) memset(priv->type_mapping, -1, sizeof(int) * pev->ntevs); - err = map_prologue(pev, priv->type_mapping, &priv->nr_types); - if (err) - return err; - - err = bpf_program__set_prep(prog, priv->nr_types, - preproc_gen_prologue); - return err; + return map_prologue(pev, priv->type_mapping, &priv->nr_types); } int bpf__probe(struct bpf_object *obj) @@ -855,6 +884,66 @@ int bpf__unprobe(struct bpf_object *obj) return ret; } +static int bpf_object__load_prologue(struct bpf_object *obj) +{ + const struct bpf_insn *orig_insns; + struct bpf_preproc_result res; + struct perf_probe_event *pev; + struct bpf_program *prog; + int orig_insns_cnt; + + bpf_object__for_each_program(prog, obj) { + struct bpf_prog_priv *priv = program_priv(prog); + int err, i, fd; + + if (IS_ERR_OR_NULL(priv)) { + pr_debug("bpf: failed to get private field\n"); + return -BPF_LOADER_ERRNO__INTERNAL; + } + + if (!priv->need_prologue) + continue; + + /* + * For each program that needs prologue we do following: + * + * - take its current instructions and use them + * to generate the new code with prologue + * + * - load new instructions with bpf_prog_load + * and keep the fd in proglogue_fds + * + * - new fd will be used bpf__foreach_event + * to connect this program with perf evsel + */ + orig_insns = bpf_program__insns(prog); + orig_insns_cnt = bpf_program__insn_cnt(prog); + + pev = &priv->pev; + for (i = 0; i < pev->ntevs; i++) { + err = preproc_gen_prologue(prog, i, orig_insns, + orig_insns_cnt, &res); + if (err) + return err; + + fd = bpf_prog_load(bpf_program__get_type(prog), + bpf_program__name(prog), "GPL", + res.new_insn_ptr, + res.new_insn_cnt, NULL); + if (fd < 0) { + char bf[128]; + + libbpf_strerror(-errno, bf, sizeof(bf)); + pr_debug("bpf: load objects with prologue failed: err=%d: (%s)\n", + -errno, bf); + return -errno; + } + priv->proglogue_fds[i] = fd; + } + } + return 0; +} + int bpf__load(struct bpf_object *obj) { int err; @@ -866,7 +955,7 @@ int bpf__load(struct bpf_object *obj) pr_debug("bpf: load objects failed: err=%d: (%s)\n", err, bf); return err; } - return 0; + return bpf_object__load_prologue(obj); } int bpf__foreach_event(struct bpf_object *obj, @@ -901,13 +990,10 @@ int bpf__foreach_event(struct bpf_object *obj, for (i = 0; i < pev->ntevs; i++) { tev = &pev->tevs[i]; - if (priv->need_prologue) { - int type = priv->type_mapping[i]; - - fd = bpf_program__nth_fd(prog, type); - } else { + if (priv->need_prologue) + fd = priv->proglogue_fds[i]; + else fd = bpf_program__fd(prog); - } if (fd < 0) { pr_debug("bpf: failed to get file descriptor\n");
Some functions we use now for bpf prologue generation are going to be deprecated, so reworking the current code not to use them. We need to replace following functions/struct: bpf_program__set_prep bpf_program__nth_fd struct bpf_prog_prep_result Current code uses bpf_program__set_prep to hook perf callback before the program is loaded and provide new instructions with the prologue. We workaround this by using objects's 'unloaded' programs instructions for that specific program and load new ebpf programs with prologue using separate bpf_prog_load calls. We keep new ebpf program instances descriptors in bpf programs private struct. Suggested-by: Andrii Nakryiko <andrii@kernel.org> Signed-off-by: Jiri Olsa <jolsa@kernel.org> --- tools/perf/util/bpf-loader.c | 122 +++++++++++++++++++++++++++++------ 1 file changed, 104 insertions(+), 18 deletions(-)