Re: [PATCH v4 bpf-next 17/22] libbpf: Introduce bpf_map__get_initial_value().

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

On Fri, May 7, 2021 at 8:49 PM Alexei Starovoitov
<alexei.starovoitov@gmail.com> wrote:
>
> From: Alexei Starovoitov <ast@kernel.org>
>
> Introduce bpf_map__get_initial_value() to read initial contents
> of rodata/bss maps. Note only mmaped maps qualify.
> Just as bpf_map__set_initial_value() works only for mmaped kconfig.
>
> Signed-off-by: Alexei Starovoitov <ast@kernel.org>
> ---
>  tools/lib/bpf/libbpf.c   | 10 ++++++++++
>  tools/lib/bpf/libbpf.h   |  2 ++
>  tools/lib/bpf/libbpf.map |  1 +
>  3 files changed, 13 insertions(+)
>
> diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
> index 24a659448782..f7cdbb0e1faf 100644
> --- a/tools/lib/bpf/libbpf.c
> +++ b/tools/lib/bpf/libbpf.c
> @@ -9763,6 +9763,16 @@ int bpf_map__set_initial_value(struct bpf_map *map,
>         return 0;
>  }
>
> +int bpf_map__get_initial_value(struct bpf_map *map,
> +                              const void **pdata, size_t *psize)

the general patterns (there are legacy exceptions) for getters is to
not have "get_" in the name. Please rename it to just
bpf_map__initial_value().

Also btf__get_raw_data() (note the legacy naming) follows a slightly
different pattern of returning NULL on error or valid pointer on
success. And then the size is returned as outer param. Unless there is
a good reason not to, let's be consistent with that pattern?

> +{
> +       if (!map->mmaped)
> +               return -EINVAL;
> +       *psize = map->def.value_size;
> +       *pdata = map->mmaped;
> +       return 0;
> +}
> +
>  bool bpf_map__is_offload_neutral(const struct bpf_map *map)
>  {
>         return map->def.type == BPF_MAP_TYPE_PERF_EVENT_ARRAY;
> diff --git a/tools/lib/bpf/libbpf.h b/tools/lib/bpf/libbpf.h
> index fb291b4529e8..f8976a30586f 100644
> --- a/tools/lib/bpf/libbpf.h
> +++ b/tools/lib/bpf/libbpf.h
> @@ -471,6 +471,8 @@ LIBBPF_API int bpf_map__set_priv(struct bpf_map *map, void *priv,
>  LIBBPF_API void *bpf_map__priv(const struct bpf_map *map);
>  LIBBPF_API int bpf_map__set_initial_value(struct bpf_map *map,
>                                           const void *data, size_t size);
> +LIBBPF_API int bpf_map__get_initial_value(struct bpf_map *map,
> +                                         const void **pdata, size_t *psize);
>  LIBBPF_API bool bpf_map__is_offload_neutral(const struct bpf_map *map);
>  LIBBPF_API bool bpf_map__is_internal(const struct bpf_map *map);
>  LIBBPF_API int bpf_map__set_pin_path(struct bpf_map *map, const char *path);
> diff --git a/tools/lib/bpf/libbpf.map b/tools/lib/bpf/libbpf.map
> index 889ee2f3611c..44285045ddf4 100644
> --- a/tools/lib/bpf/libbpf.map
> +++ b/tools/lib/bpf/libbpf.map
> @@ -360,6 +360,7 @@ LIBBPF_0.4.0 {
>                 bpf_linker__free;
>                 bpf_linker__new;
>                 bpf_map__inner_map;
> +               bpf_map__get_initial_value;

nit: g < i, and as bpf_map__initial_value will still go before inner_map getter

>                 bpf_object__gen_loader;
>                 bpf_object__set_kversion;
>  } LIBBPF_0.3.0;
> --
> 2.30.2
>