[DTrace-devel] [PATCH] Add dt_bpf_map_lookup() function to retrieve values from BPF maps

[Eugene Loh]

Some context here would be good.  E.g., why is this being introduced?  
Maybe that's problematic to answer since the patch that will use this 
will depend on a lot of leg work that must first be set up, but a 
unified patch set would be nice.  I won't insist on it, though.  So, not 
ideal, but so be it.

But maybe at least say why the libbpf/bpf.c function 
bpf_map_lookup_elem() is not being used?  Could it be?  Are you trying 
to wean us off of libbpf?  But then that should be its own patch?

But at the very least (looking ahead to the next patch), the movement of 
dt_bpf_map_[lookup|update] in "Implement a 'state' BPF map to 
communicate tracing session state" should be moved into this patch 
rather than introducing the new function here in the "wrong" place only 
to fix its location in the next patch.  Just get the placement right the 
first time.  As it is, the function being introduced here is backed out 
in the next patch.  Just put dt_bpf_map_lookup in the right place here, 
moving dt_bpf_map_update while you're at it.  Those gymnastics do not 
need to be in the next patch.

On 08/27/2020 11:54 AM, Kris Van Hees wrote:
> Signed-off-by: Kris Van Hees <kris.van.hees at oracle.com>
> ---
>   libdtrace/dt_bpf.c | 15 +++++++++++++++
>   libdtrace/dt_bpf.h |  1 +
>   2 files changed, 16 insertions(+)
>
> diff --git a/libdtrace/dt_bpf.c b/libdtrace/dt_bpf.c
> index d6d05dc8..0531a37e 100644
> --- a/libdtrace/dt_bpf.c
> +++ b/libdtrace/dt_bpf.c
> @@ -185,6 +185,21 @@ dt_bpf_gmap_create(dtrace_hdl_t *dtp)
>   	return 0;
>   }
>   
> +/*
> + * Load the value for the given key in the map referenced by the given fd.
> + */
> +int dt_bpf_map_lookup(int fd, const void *key, void *val)
> +{
> +	union bpf_attr attr;
> +
> +	memset(&attr, 0, sizeof(attr));
> +	attr.map_fd = fd;
> +	attr.key = (uint64_t)(unsigned long)key;
> +	attr.value = (uint64_t)(unsigned long)val;
> +
> +	return bpf(BPF_MAP_LOOKUP_ELEM, &attr);
> +}
> +
>   /*
>    * Store the (key, value) pair in the map referenced by the given fd.
>    */
> diff --git a/libdtrace/dt_bpf.h b/libdtrace/dt_bpf.h
> index c4a4a99c..e87d4653 100644
> --- a/libdtrace/dt_bpf.h
> +++ b/libdtrace/dt_bpf.h
> @@ -23,6 +23,7 @@ extern int perf_event_open(struct perf_event_attr *attr, pid_t pid, int cpu,
>   extern int bpf(enum bpf_cmd cmd, union bpf_attr *attr);
>   
>   extern int dt_bpf_gmap_create(dtrace_hdl_t *);
> +extern int dt_bpf_map_lookup(int fd, const void *key, void *val);
>   extern int dt_bpf_map_update(int fd, const void *key, const void *val);
>   extern int dt_bpf_load_progs(dtrace_hdl_t *, uint_t);
>