On Fri, Oct 11, 2019 at 5:40 PM Alexei Starovoitov <ast@xxxxxx> wrote:
>
> On 10/11/19 11:07 AM, Andrii Nakryiko wrote:
> > On Wed, Oct 9, 2019 at 9:17 PM Alexei Starovoitov <ast@xxxxxxxxxx> wrote:
> >>
> >> For raw tracepoint program types libbpf will try to find
> >> btf_id of raw tracepoint in vmlinux's BTF.
> >> It's a responsiblity of bpf program author to annotate the program
> >> with SEC("raw_tracepoint/name") where "name" is a valid raw tracepoint.
> >> If "name" is indeed a valid raw tracepoint then in-kernel BTF
> >> will have "btf_trace_##name" typedef that points to function
> >> prototype of that raw tracepoint. BTF description captures
> >> exact argument the kernel C code is passing into raw tracepoint.
> >> The kernel verifier will check the types while loading bpf program.
> >>
> >> libbpf keeps BTF type id in expected_attach_type, but since
> >> kernel ignores this attribute for tracing programs copy it
> >> into attach_btf_id attribute before loading.
> >>
> >> Signed-off-by: Alexei Starovoitov <ast@xxxxxxxxxx>
> >> ---
> >> tools/lib/bpf/bpf.c | 3 +++
> >> tools/lib/bpf/libbpf.c | 17 +++++++++++++++++
> >> 2 files changed, 20 insertions(+)
> >>
> >> diff --git a/tools/lib/bpf/bpf.c b/tools/lib/bpf/bpf.c
> >> index cbb933532981..79046067720f 100644
> >> --- a/tools/lib/bpf/bpf.c
> >> +++ b/tools/lib/bpf/bpf.c
> >> @@ -228,6 +228,9 @@ int bpf_load_program_xattr(const struct bpf_load_program_attr *load_attr,
> >> memset(&attr, 0, sizeof(attr));
> >> attr.prog_type = load_attr->prog_type;
> >> attr.expected_attach_type = load_attr->expected_attach_type;
> >> + if (attr.prog_type == BPF_PROG_TYPE_RAW_TRACEPOINT)
> >> + /* expected_attach_type is ignored for tracing progs */
> >> + attr.attach_btf_id = attr.expected_attach_type;
> >> attr.insn_cnt = (__u32)load_attr->insns_cnt;
> >> attr.insns = ptr_to_u64(load_attr->insns);
> >> attr.license = ptr_to_u64(load_attr->license);
> >> diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
> >> index a02cdedc4e3f..8bf30a67428c 100644
> >> --- a/tools/lib/bpf/libbpf.c
> >> +++ b/tools/lib/bpf/libbpf.c
> >> @@ -4586,6 +4586,23 @@ int libbpf_prog_type_by_name(const char *name, enum bpf_prog_type *prog_type,
> >> continue;
> >> *prog_type = section_names[i].prog_type;
> >> *expected_attach_type = section_names[i].expected_attach_type;
> >> + if (*prog_type == BPF_PROG_TYPE_RAW_TRACEPOINT) {
> >> + struct btf *btf = bpf_core_find_kernel_btf();
> >> + char raw_tp_btf_name[128] = "btf_trace_";
> >> + char *dst = raw_tp_btf_name + sizeof("btf_trace_") - 1;
> >> + int ret;
> >> +
> >> + if (IS_ERR(btf))
> >> + /* lack of kernel BTF is not a failure */
> >> + return 0;
> >> + /* prepend "btf_trace_" prefix per kernel convention */
> >> + strncat(dst, name + section_names[i].len,
> >> + sizeof(raw_tp_btf_name) - (dst - raw_tp_btf_name));
> >> + ret = btf__find_by_name(btf, raw_tp_btf_name);
> >> + if (ret > 0)
> >> + *expected_attach_type = ret;
> >
> > Well, actually, I realized after I gave Acked-by, so not yet :)
> >
> > This needs kernel feature probe of whether kernel supports specifying
> > attach_btf_id, otherwise on older kernels we'll stop successfully
> > loading valid program.
>
> The code above won't find anything on older kernels.
> The patch 1 of the series has to be there for proper btf to be
> generated by pahole.
> Before that happens expected_attach_type will stay zero
> and corresponding copy in attach_btf_id will be zero as well.
> I see no issues being compatible with older kernels.
indeed, this one is not an issue.
>
> > But even if kernel supports attach_btf_id, I think users still need to
> > opt in into specifying attach_btf_id by libbpf. Think about existing
> > raw_tp programs that are using bpf_probe_read() because they were not
> > created with this kernel feature in mind. They will suddenly stop
> > working without any of user's fault.
>
> This one is excellent catch.
> loop1.c should have caught it, since it has
> SEC("raw_tracepoint/kfree_skb")
> {
> int nested_loops(volatile struct pt_regs* ctx)
> .. = PT_REGS_RC(ctx);
>
> and verifier would have rejected it.
> But the way the test is written it's not using libbpf's autodetect
> of program type, so everything is passing.
