[bpf-next,v6,0/5] Allow storage of flexible metadata information for eBPF programs

Currently, if a user wants to store arbitrary metadata for an eBPF
program, for example, the program build commit hash or version, they
could store it in a map, and conveniently libbpf uses .data section to
populate an internal map. However, if the program does not actually
reference the map, then the map would be de-refcounted and freed.

This patch set introduces a new syscall BPF_PROG_BIND_MAP to add a map
to a program's used_maps, even if the program instructions does not
reference the map.

libbpf is extended to always BPF_PROG_BIND_MAP .rodata section so the
metadata is kept in place.
bpftool is also extended to print metadata in the 'bpftool prog' list.

The variable is considered metadata if it starts with the
magic 'bpf_metadata_' prefix; everything after the prefix is the
metadata name.

An example use of this would be BPF C file declaring:

  volatile const char bpf_metadata_commit_hash[] SEC(".rodata") = "abcdef123456";

and bpftool would emit:

  $ bpftool prog
  [...]
        metadata:
                commit_hash = "abcdef123456"

v6 changes:
* libbpf: drop FEAT_GLOBAL_DATA from probe_prog_bind_map (Andrii Nakryiko)
* bpftool: combine find_metadata_map_id & find_metadata;
  drops extra bpf_map_get_fd_by_id and bpf_map_get_fd_by_id (Andrii Nakryiko)
* bpftool: use strncmp instead of strstr (Andrii Nakryiko)
* bpftool: memset(map_info) and extra empty line (Andrii Nakryiko)

v5 changes:
* selftest: verify that prog holds rodata (Andrii Nakryiko)
* selftest: use volatile for metadata (Andrii Nakryiko)
* bpftool: use sizeof in BPF_METADATA_PREFIX_LEN (Andrii Nakryiko)
* bpftool: new find_metadata that does map lookup (Andrii Nakryiko)
* libbpf: don't generalize probe_create_global_data (Andrii Nakryiko)
* libbpf: use OPTS_VALID in bpf_prog_bind_map (Andrii Nakryiko)
* libbpf: keep LIBBPF_0.2.0 sorted (Andrii Nakryiko)

v4 changes:
* Don't return EEXIST from syscall if already bound (Andrii Nakryiko)
* Removed --metadata argument (Andrii Nakryiko)
* Removed custom .metadata section (Alexei Starovoitov)
* Addressed Andrii's suggestions about btf helpers and vsi (Andrii Nakryiko)
* Moved bpf_prog_find_metadata into bpftool (Alexei Starovoitov)

v3 changes:
* API changes for bpf_prog_find_metadata (Toke Høiland-Jørgensen)

v2 changes:
* Made struct bpf_prog_bind_opts in libbpf so flags is optional.
* Deduped probe_kern_global_data and probe_prog_bind_map into a common
  helper.
* Added comment regarding why EEXIST is ignored in libbpf bind map.
* Froze all LIBBPF_MAP_METADATA internal maps.
* Moved bpf_prog_bind_map into new LIBBPF_0.1.1 in libbpf.map.
* Added p_err() calls on error cases in bpftool show_prog_metadata.
* Reverse christmas tree coding style in bpftool show_prog_metadata.
* Made bpftool gen skeleton recognize .metadata as an internal map and
  generate datasec definition in skeleton.
* Added C test using skeleton to see asset that the metadata is what we
  expect and rebinding causes EEXIST.

v1 changes:
* Fixed a few missing unlocks, and missing close while iterating map fds.
* Move mutex initialization to right after prog aux allocation, and mutex
  destroy to right after prog aux free.
* s/ADD_MAP/BIND_MAP/
* Use mutex only instead of RCU to protect the used_map array & count.

Cc: YiFei Zhu <zhuyifei1999@gmail.com>

YiFei Zhu (5):
  bpf: Mutex protect used_maps array and count
  bpf: Add BPF_PROG_BIND_MAP syscall
  libbpf: Add BPF_PROG_BIND_MAP syscall and use it on .rodata section
  bpftool: support dumping metadata
  selftests/bpf: Test load and dump metadata with btftool and skel

 .../net/ethernet/netronome/nfp/bpf/offload.c  |  18 +-
 include/linux/bpf.h                           |   1 +
 include/uapi/linux/bpf.h                      |   7 +
 kernel/bpf/core.c                             |  15 +-
 kernel/bpf/syscall.c                          |  79 ++++++-
 net/core/dev.c                                |  11 +-
 tools/bpf/bpftool/json_writer.c               |   6 +
 tools/bpf/bpftool/json_writer.h               |   3 +
 tools/bpf/bpftool/prog.c                      | 199 ++++++++++++++++++
 tools/include/uapi/linux/bpf.h                |   7 +
 tools/lib/bpf/bpf.c                           |  16 ++
 tools/lib/bpf/bpf.h                           |   8 +
 tools/lib/bpf/libbpf.c                        |  69 ++++++
 tools/lib/bpf/libbpf.map                      |   1 +
 tools/testing/selftests/bpf/Makefile          |   3 +-
 .../selftests/bpf/prog_tests/metadata.c       | 141 +++++++++++++
 .../selftests/bpf/progs/metadata_unused.c     |  15 ++
 .../selftests/bpf/progs/metadata_used.c       |  15 ++
 .../selftests/bpf/test_bpftool_metadata.sh    |  82 ++++++++
 19 files changed, 678 insertions(+), 18 deletions(-)
 create mode 100644 tools/testing/selftests/bpf/prog_tests/metadata.c
 create mode 100644 tools/testing/selftests/bpf/progs/metadata_unused.c
 create mode 100644 tools/testing/selftests/bpf/progs/metadata_used.c
 create mode 100755 tools/testing/selftests/bpf/test_bpftool_metadata.sh

On Tue, Sep 15, 2020 at 4:47 PM Stanislav Fomichev <sdf@google.com> wrote:
>
> Currently, if a user wants to store arbitrary metadata for an eBPF
> program, for example, the program build commit hash or version, they
> could store it in a map, and conveniently libbpf uses .data section to
> populate an internal map. However, if the program does not actually
> reference the map, then the map would be de-refcounted and freed.
>
> This patch set introduces a new syscall BPF_PROG_BIND_MAP to add a map
> to a program's used_maps, even if the program instructions does not
> reference the map.
>
> libbpf is extended to always BPF_PROG_BIND_MAP .rodata section so the
> metadata is kept in place.
> bpftool is also extended to print metadata in the 'bpftool prog' list.
>
> The variable is considered metadata if it starts with the
> magic 'bpf_metadata_' prefix; everything after the prefix is the
> metadata name.
>
> An example use of this would be BPF C file declaring:
>
>   volatile const char bpf_metadata_commit_hash[] SEC(".rodata") = "abcdef123456";
>
> and bpftool would emit:
>
>   $ bpftool prog
>   [...]
>         metadata:
>                 commit_hash = "abcdef123456"
>

[...]

>
> Cc: YiFei Zhu <zhuyifei1999@gmail.com>
>
> YiFei Zhu (5):
>   bpf: Mutex protect used_maps array and count
>   bpf: Add BPF_PROG_BIND_MAP syscall
>   libbpf: Add BPF_PROG_BIND_MAP syscall and use it on .rodata section
>   bpftool: support dumping metadata
>   selftests/bpf: Test load and dump metadata with btftool and skel
>
>  .../net/ethernet/netronome/nfp/bpf/offload.c  |  18 +-
>  include/linux/bpf.h                           |   1 +
>  include/uapi/linux/bpf.h                      |   7 +
>  kernel/bpf/core.c                             |  15 +-
>  kernel/bpf/syscall.c                          |  79 ++++++-
>  net/core/dev.c                                |  11 +-
>  tools/bpf/bpftool/json_writer.c               |   6 +
>  tools/bpf/bpftool/json_writer.h               |   3 +
>  tools/bpf/bpftool/prog.c                      | 199 ++++++++++++++++++
>  tools/include/uapi/linux/bpf.h                |   7 +
>  tools/lib/bpf/bpf.c                           |  16 ++
>  tools/lib/bpf/bpf.h                           |   8 +
>  tools/lib/bpf/libbpf.c                        |  69 ++++++
>  tools/lib/bpf/libbpf.map                      |   1 +
>  tools/testing/selftests/bpf/Makefile          |   3 +-
>  .../selftests/bpf/prog_tests/metadata.c       | 141 +++++++++++++
>  .../selftests/bpf/progs/metadata_unused.c     |  15 ++
>  .../selftests/bpf/progs/metadata_used.c       |  15 ++
>  .../selftests/bpf/test_bpftool_metadata.sh    |  82 ++++++++
>  19 files changed, 678 insertions(+), 18 deletions(-)
>  create mode 100644 tools/testing/selftests/bpf/prog_tests/metadata.c
>  create mode 100644 tools/testing/selftests/bpf/progs/metadata_unused.c
>  create mode 100644 tools/testing/selftests/bpf/progs/metadata_used.c
>  create mode 100755 tools/testing/selftests/bpf/test_bpftool_metadata.sh
>
> --
> 2.28.0.618.gf4bc123cb7-goog
>

LGTM, thanks!

Acked-by: Andrii Nakryiko <andriin@fb.com>