On Sun, Mar 27, 2022 at 3:03 PM Quentin Monnet <quentin@xxxxxxxxxxxxx> wrote:
>
> 2022-03-26 10:08 UTC+0100 ~ Dmitry Dolgov <9erthalion6@xxxxxxxxx>
> >> On Sat, Mar 26, 2022 at 01:38:36AM +0000, Quentin Monnet wrote:
> >> 2022-03-09 17:31 UTC+0100 ~ Dmitrii Dolgov <9erthalion6@xxxxxxxxx>
> >>> Commit 82e6b1eee6a8 ("bpf: Allow to specify user-provided bpf_cookie for
> >>> BPF perf links") introduced the concept of user specified bpf_cookie,
> >>> which could be accessed by BPF programs using bpf_get_attach_cookie().
> >>> For troubleshooting purposes it is convenient to expose bpf_cookie via
> >>> bpftool as well, so there is no need to meddle with the target BPF
> >>> program itself.
> >>>
> >>> [...]
> >>>
> >>> diff --git a/tools/bpf/bpftool/pids.c b/tools/bpf/bpftool/pids.c
> >>> index 7c384d10e95f..bb6c969a114a 100644
> >>> --- a/tools/bpf/bpftool/pids.c
> >>> +++ b/tools/bpf/bpftool/pids.c
> >>> @@ -78,6 +78,8 @@ static void add_ref(struct hashmap *map, struct pid_iter_entry *e)
> >>> ref->pid = e->pid;
> >>> memcpy(ref->comm, e->comm, sizeof(ref->comm));
> >>> refs->ref_cnt = 1;
> >>> + refs->has_bpf_cookie = e->has_bpf_cookie;
> >>> + refs->bpf_cookie = e->bpf_cookie;
> >>>
> >>> err = hashmap__append(map, u32_as_hash_field(e->id), refs);
> >>> if (err)
> >>> @@ -205,6 +207,9 @@ void emit_obj_refs_json(struct hashmap *map, __u32 id,
> >>> if (refs->ref_cnt == 0)
> >>> break;
> >>>
> >>> + if (refs->has_bpf_cookie)
> >>> + jsonw_lluint_field(json_writer, "bpf_cookie", refs->bpf_cookie);
> >>> +
> >>
> >> Thinking again about this patch, shouldn't the JSON output for the
> >> cookie(s) be an array if we expect to have several cookies for
> >> multi-attach links in the future?
> >
> > Interesting point. My impression is that this could be done together
> > with the other changes about making multi-attach links possible (I
> > didn't miss anything, it's not yet implemented, right?). On the other
> > hand I'm planning to prepare few more patches in similar direction -- so
> > if everyone agrees it has to be extended to an array now, I can tackle
> > this as well.
>
> Correct, it's not implemented yet for multi-attach links. My concern
> here is to avoid changing the JSON structure in the future (to avoid
> breaking changes for tools that would process the JSON). If we know
> we're likely to have several cookies in the future, it may be worth
> using an array “from start” (since no version has been tagged yet after
> you added support for the cookie).
The problem with multi-cookie links (like KPROBE_MULTI that was merged
a week ago) is that cookies by themselves are not that helpful. Also,
internally we change their order to be sorted according to resolved
kernel function addresses. So just an array of cookies are not enough,
but asking kernel to preserve all the addresses just for the sake of
reporting them in bpftool seems to much.
So in general, with multi-cookie links, it's not clear what you should
report at all.
>
> Quentin
