Re: Need a way to modify the section name for a read program object

[Andrii Nakryiko <andrii.nakryiko@gmail.com>]

On Tue, Feb 4, 2020 at 11:27 AM Toke Høiland-Jørgensen <toke@redhat.com> wrote:
>
> Andrii Nakryiko <andriin@fb.com> writes:
>
> > On 2/4/20 2:19 AM, Toke Høiland-Jørgensen wrote:
> >> "Eelco Chaudron" <echaudro@redhat.com> writes:
> >>
> >>> Hi All,
> >>>
> >>> I'm trying to write an xdpdump like utility and have some missing part
> >>> in libbpf to change the fentry/FUNCTION section name before loading the
> >>> trace program.
> >>>
> >>> In short, I have an eBPF program that has a section name like
> >>> "fentry/FUNCTION" where FUNCTION needs to be replaced by the name of the
> >>> XDP program loaded in the interfaces its start function.
> >>>
> >>> The code for loading the ftrace part is something like:
> >>>
> >>>     open_opts.attach_prog_fd = bpf_prog_get_fd_by_id(info.id);
> >>>     trace_obj = bpf_object__open_file("xdpdump_bpf.o", &open_opts);
> >>>
> >>>     trace_prog_fentry = bpf_object__find_program_by_title(trace_obj,
> >>> "fentry/FUNCTION");
> >>>
> >>>     /* Here I need to replace the trace_prog_fentry->section_name =
> >>> "fentry/<INTERFACE PROG NAME> */
> >>>
> >>>     bpf_object__load(trace_obj);
> >>>     trace_link_fentry = bpf_program__attach_trace(trace_prog_fentry);
> >>>
> >>>
> >>> See the above, I would like to change the section_name but there is no
> >>> API to do this, and of course, the struct bpf_program is
> >>> implementation-specific.
> >>>
> >>> Any idea how I would work around this, or what extension to libbpf can
> >>> be suggested to support this?
> >>
> >> I think what's missing is a way for the caller to set the attach_btf_id.
> >> Currently, libbpf always tries to discover this based on the section
> >> name (see libbpf_find_attach_btf_id()). I think the right way to let the
> >> caller specify this is not to change the section name, though, but just
> >> to expose a way to explicitly set the btf_id (which the caller can then
> >> go find on its own).
> >
> > Yes, I agree, section name should be treated as an immutable identifier
> > and a (overrideable) hint to libbpf.
> >
> >>
> >> Not sure if it would be better with a new open_opt (to mirror
> >> attach_prog_fd), or just a setter (bpf_program__set_attach_btf_id()?).
> >> Or maybe both? Andrii, WDYT?
> >
> > open_opts is definitely wrong way to do this, because open_opts apply to
> > all BPF programs, while this should be per-program.
>
> Yes, of course; silly me :)
>
> > I'm also not sure having API that allows to specify BTF type ID is the
> > best, probably better to let libbpf perform the search by name. So I'd
> > say something like this:
> >
> > int bpf_program__set_attach_target(int attach_prog_fd, const char
> > *attach_func_name)
> >
> > This should handle customizing all the tp_btf/fentry/fexit/freplace BPF
> > programs we have.
>
> Right, that makes sense; I think that would cover it (apart from your
> function signature missing a struct bpf_program argument).

great! and, ha-ha, too object-oriented thinking ;)

>
> > We might add extra attach_target_ops for future extensibility, if we
> > anticipate that we'll need more knobs in the future, I haven't thought
> > too much about that.
>
> Good question, me neither. Will see if I can think of anything...
>
> -Toke
>