Re: [PATCH bpf-next 4/7] libbpf: wire up spec management and other arch-independent USDT logic

[Alan Maguire <alan.maguire@oracle.com>]

On Fri, 25 Mar 2022, Andrii Nakryiko wrote:

> Last part of architecture-agnostic user-space USDT handling logic is to
> set up BPF spec and, optionally, IP-to-ID maps from user-space.
> usdt_manager performs a compact spec ID allocation to utilize
> fixed-sized BPF maps as efficiently as possible. We also use hashmap to
> deduplicate USDT arg spec strings and map identical strings to single
> USDT spec, minimizing the necessary BPF map size. usdt_manager supports
> arbitrary sequences of attachment and detachment, both of the same USDT
> and multiple different USDTs and internally maintains a free list of
> unused spec IDs. bpf_link_usdt's logic is extended with proper setup and
> teardown of this spec ID free list and supporting BPF maps.
> 

It might be good to describe the relationship between a USDT specification
(spec) and the site specific targets that can be associated with it.  So 
the spec is the description of the provider + name + args, and the the 
target represents the potentially multiple sites associated with that 
spec.

Specs are stored in the spec array map, indexed by spec_id; targets are
stored in the ip_map, and these reference a spec id.  So from the BPF side 
we can use the bpf_cookie to look up the spec directly, or if cookies are
not supported on the BPF side, we can look up ip -> spec_id mapping in 
ip_map, and from there can look up the spec_id -> spec in the spec map.

Dumb question here: the spec id recycling is a lot of work; 
instead of maintaining this for the array map, couldn't we use a hashmap 
for spec ids with a monotonically-increasing next_spec_id value or
something similar?

> Signed-off-by: Andrii Nakryiko <andrii@kernel.org>

one suggestion below, but

Reviewed-by: Alan Maguire <alan.maguire@oracle.com>

> ---
>  tools/lib/bpf/usdt.c | 167 ++++++++++++++++++++++++++++++++++++++++++-
>  1 file changed, 166 insertions(+), 1 deletion(-)
> 
> diff --git a/tools/lib/bpf/usdt.c b/tools/lib/bpf/usdt.c
> index 86d5d8390eb1..22f5f56992f8 100644
> --- a/tools/lib/bpf/usdt.c
> +++ b/tools/lib/bpf/usdt.c

<snip>

>  		opts.ref_ctr_offset = target->sema_off;
> +		opts.bpf_cookie = man->has_bpf_cookie ? spec_id : 0;
>  		uprobe_link = bpf_program__attach_uprobe_opts(prog, pid, path,
>  							      target->rel_ip, &opts);
>  		err = libbpf_get_error(link);

should be uprobe_link I think.