Re: [PATCH bpf-next v6] bpftool: Add bpf_cookie to link output

[Quentin Monnet <quentin@isovalent.com>]

2022-03-09 17:31 UTC+0100 ~ Dmitrii Dolgov <9erthalion6@gmail.com>
> 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.
> 
> Implemented using the pid iterator BPF program to actually fetch
> bpf_cookies, which allows constraining code changes only to bpftool.
> 
> $ bpftool link
> 1: type 7  prog 5
>         bpf_cookie 123
>         pids bootstrap(81)
> 
> Signed-off-by: Dmitrii Dolgov <9erthalion6@gmail.com>
> Acked-by: Yonghong Song <yhs@fb.com>
> ---
> Changes in v6:
>     - Remove unnecessary initialization of fields in pid_iter_entry
>     - Changing bpf_cookie_set to has_bpf_cookie
>     - Small code cleanup (casting bpf_cookie when needed, removing
>       __always_inline, etc.)
> 
> Changes in v5:
>     - Remove unneeded cookie assigns
> 
> Changes in v4:
>     - Fetch cookies only for bpf_perf_link
>     - Signal about bpf_cookie via the flag, instead of deducing it from
>       the object and link type
>     - Reset pid_iter_entry to avoid invalid indirect read from stack
> 
> Changes in v3:
>     - Use pid iterator to fetch bpf_cookie
> 
> Changes in v2:
>     - Display bpf_cookie in bpftool link command instead perf
> 
> Previous discussion: https://lore.kernel.org/bpf/20220225152802.20957-1-9erthalion6@gmail.com/
> 
>  tools/bpf/bpftool/main.h                  |  2 ++
>  tools/bpf/bpftool/pids.c                  |  8 ++++++++
>  tools/bpf/bpftool/skeleton/pid_iter.bpf.c | 22 ++++++++++++++++++++++
>  tools/bpf/bpftool/skeleton/pid_iter.h     |  2 ++
>  4 files changed, 34 insertions(+)
> 
> diff --git a/tools/bpf/bpftool/main.h b/tools/bpf/bpftool/main.h
> index 0c3840596b5a..3574bef7d4ce 100644
> --- a/tools/bpf/bpftool/main.h
> +++ b/tools/bpf/bpftool/main.h
> @@ -114,6 +114,8 @@ struct obj_ref {
>  struct obj_refs {
>  	int ref_cnt;
>  	struct obj_ref *refs;
> +	bool has_bpf_cookie;
> +	__u64 bpf_cookie;
>  };
>  
>  struct btf;
> 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?

Quentin