On Wed, Jul 24, 2019 at 12:27:34PM -0700, Andrii Nakryiko wrote:
> This patch implements the core logic for BPF CO-RE offsets relocations.
> All the details are described in code comments.
>
> Signed-off-by: Andrii Nakryiko <andr...@fb.com>
> ---
> tools/lib/bpf/libbpf.c | 866 ++++++++++++++++++++++++++++++++++++++++-
> tools/lib/bpf/libbpf.h | 1 +
> 2 files changed, 861 insertions(+), 6 deletions(-)
>
> diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
> index 8741c39adb1c..86d87bf10d46 100644
> --- a/tools/lib/bpf/libbpf.c
> +++ b/tools/lib/bpf/libbpf.c
> @@ -38,6 +38,7 @@
> #include <sys/stat.h>
> #include <sys/types.h>
> #include <sys/vfs.h>
> +#include <sys/utsname.h>
> #include <tools/libc_compat.h>
> #include <libelf.h>
> #include <gelf.h>
> @@ -47,6 +48,7 @@
> #include "btf.h"
> #include "str_error.h"
> #include "libbpf_internal.h"
> +#include "hashmap.h"
>
> #ifndef EM_BPF
> #define EM_BPF 247
> @@ -1013,16 +1015,22 @@ static int bpf_object__init_user_maps(struct
> bpf_object *obj, bool strict)
> }
>
> static const struct btf_type *skip_mods_and_typedefs(const struct btf *btf,
> - __u32 id)
> + __u32 id,
> + __u32 *res_id)
I think it would be more readable to format it like:
static const struct btf_type *
skip_mods_and_typedefs(const struct btf *btf, __u32 id, __u32 *res_id)
> + } else if (class == BPF_ST && BPF_MODE(insn->code) == BPF_MEM) {
> + if (insn->imm != orig_off)
> + return -EINVAL;
> + insn->imm = new_off;
> + pr_debug("prog '%s': patched insn #%d (ST | MEM) imm %d ->
> %d\n",
> + bpf_program__title(prog, false),
> + insn_idx, orig_off, new_off);
I'm pretty sure llvm was not capable of emitting BPF_ST insn.
When did that change?
> +/*
> + * CO-RE relocate single instruction.
> + *
> + * The outline and important points of the algorithm:
> + * 1. For given local type, find corresponding candidate target types.
> + * Candidate type is a type with the same "essential" name, ignoring
> + * everything after last triple underscore (___). E.g., `sample`,
> + * `sample___flavor_one`, `sample___flavor_another_one`, are all
> candidates
> + * for each other. Names with triple underscore are referred to as
> + * "flavors" and are useful, among other things, to allow to
> + * specify/support incompatible variations of the same kernel struct,
> which
> + * might differ between different kernel versions and/or build
> + * configurations.
"flavors" is a convention of bpftool btf2c converter, right?
May be mention it here with pointer to the code?
> + pr_debug("prog '%s': relo #%d: insn_off=%d, [%d] (%s) + %s\n",
> + prog_name, relo_idx, relo->insn_off,
> + local_id, local_name, spec_str);
> +
> + err = bpf_core_spec_parse(local_btf, local_id, spec_str, &local_spec);
> + if (err) {
> + pr_warning("prog '%s': relo #%d: parsing [%d] (%s) + %s failed:
> %d\n",
> + prog_name, relo_idx, local_id, local_name, spec_str,
> + err);
> + return -EINVAL;
> + }
> + pr_debug("prog '%s': relo #%d: [%d] (%s) + %s is off %u, len %d,
> raw_len %d\n",
> + prog_name, relo_idx, local_id, local_name, spec_str,
> + local_spec.offset, local_spec.len, local_spec.raw_len);
one warn and two debug that print more or less the same info seems like
overkill.
> + for (i = 0, j = 0; i < cand_ids->len; i++) {
> + cand_id = cand_ids->data[j];
> + cand_type = btf__type_by_id(targ_btf, cand_id);
> + cand_name = btf__name_by_offset(targ_btf, cand_type->name_off);
> +
> + err = bpf_core_spec_match(&local_spec, targ_btf,
> + cand_id, &cand_spec);
> + if (err < 0) {
> + pr_warning("prog '%s': relo #%d: failed to match spec
> [%d] (%s) + %s to candidate #%d [%d] (%s): %d\n",
> + prog_name, relo_idx, local_id, local_name,
> + spec_str, i, cand_id, cand_name, err);
> + return err;
> + }
> + if (err == 0) {
> + pr_debug("prog '%s': relo #%d: candidate #%d [%d] (%s)
> doesn't match spec\n",
> + prog_name, relo_idx, i, cand_id, cand_name);
> + continue;
> + }
> +
> + pr_debug("prog '%s': relo #%d: candidate #%d ([%d] %s) is off
> %u, len %d, raw_len %d\n",
> + prog_name, relo_idx, i, cand_id, cand_name,
> + cand_spec.offset, cand_spec.len, cand_spec.raw_len);
have the same feeling about 3 printfs above.
