On Tue, Jul 13, 2021 at 11:34 AM Lorenzo Fontana
<fontanalorenz@xxxxxxxxx> wrote:
>
> This allows consumers of libbpf to iterate trough the insns
> of a program without loading it first directly after the ELF parsing.
>
> Being able to do that is useful to create tooling that can show
> the structure of a BPF program using libbpf without having to
> parse the ELF separately.
>
So I wonder how useful is getting raw BPF instructions before libbpf
processed them and resolved map references, subprogram calls, etc?
You'll have lots of zeroes or meaningless constants in ldimm64
instructions, etc. I always felt that being able to get instructions
after libbpf processed them is more useful. The problem is that
currently libbpf frees prog->insns after successful bpf_program__load.
There is one extra (advanced) scenario where having those instructions
preserved after load would be really nice -- cloning BPF program (I
had use case for fentry/fexit). So the question is whether we should
just leave those prog->insns around until the object is closed or not?
And if we do, should bpftool dump instructions before or after load?
Let's see what folks think.
> Usage:
> struct bpf_insn *insn;
> insn = bpf_program__insns(prog);
>
> Signed-off-by: Lorenzo Fontana <fontanalorenz@xxxxxxxxx>
> ---
> tools/lib/bpf/libbpf.c | 5 +++++
> tools/lib/bpf/libbpf.h | 1 +
> 2 files changed, 6 insertions(+)
>
> diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
> index 1e04ce724240..67d51531f6b6 100644
> --- a/tools/lib/bpf/libbpf.c
> +++ b/tools/lib/bpf/libbpf.c
> @@ -8866,6 +8866,11 @@ void *bpf_program__priv(const struct bpf_program *prog)
> return prog ? prog->priv : libbpf_err_ptr(-EINVAL);
> }
>
> +struct bpf_insn *bpf_program__insns(const struct bpf_program *prog)
Definitely needs to be const, we don't want anyone accidentally to
modify it (though it's C and they can always force they way, but
that's a separate issue)
> +{
> + return prog ? prog->insns : libbpf_err_ptr(-EINVAL);
> +}
> +
> void bpf_program__set_ifindex(struct bpf_program *prog, __u32 ifindex)
> {
> prog->prog_ifindex = ifindex;
> diff --git a/tools/lib/bpf/libbpf.h b/tools/lib/bpf/libbpf.h
> index 6e61342ba56c..e4a1c98ae6d9 100644
> --- a/tools/lib/bpf/libbpf.h
> +++ b/tools/lib/bpf/libbpf.h
> @@ -195,6 +195,7 @@ typedef void (*bpf_program_clear_priv_t)(struct bpf_program *, void *);
> LIBBPF_API int bpf_program__set_priv(struct bpf_program *prog, void *priv,
> bpf_program_clear_priv_t clear_priv);
>
> +LIBBPF_API struct bpf_insn *bpf_program__insns(const struct bpf_program *prog);
BTW, I find bpf_program__size() is very ambiguous (is it number of
bytes or number of instructions)? I'd also add bpf_program__insn_cnt()
in the same patch.
And as Quentin mentioned, you need to update libbpf.map (and please
keep everything alphabetically sorted).
> LIBBPF_API void *bpf_program__priv(const struct bpf_program *prog);
> LIBBPF_API void bpf_program__set_ifindex(struct bpf_program *prog,
> __u32 ifindex);
> --
> 2.32.0
>
