On Thu, Feb 17, 2022 at 5:19 AM Jiri Olsa <jolsa@xxxxxxxxxx> 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@xxxxxxxxxx>
> Signed-off-by: Jiri Olsa <jolsa@xxxxxxxxxx>
> ---
> 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];
> +
[...]
