Message-ID: <e2cd666b-4b97-141f-6925-fc00d8b4aa5f@fb.com>
Date:   Wed, 19 Aug 2020 09:24:57 -0700
From:   Yonghong Song <yhs@...com>
To:     Andrii Nakryiko <andriin@...com>, <bpf@...r.kernel.org>,
        <netdev@...r.kernel.org>, <ast@...com>, <daniel@...earbox.net>
CC:     <andrii.nakryiko@...il.com>, <kernel-team@...com>
Subject: Re: [PATCH v2 bpf-next 0/5] Add support for type-based and enum
 value-based CO-RE relocations



On 8/18/20 10:28 PM, Andrii Nakryiko wrote:
> This patch set adds libbpf support for two new classes of CO-RE relocations:
> type-based (TYPE_EXISTS/TYPE_SIZE/TYPE_ID_LOCAL/TYPE_ID_TARGET) and enum
> value-vased (ENUMVAL_EXISTS/ENUMVAL_VALUE):
>    - TYPE_EXISTS allows to detect presence in kernel BTF of a locally-recorded
>      BTF type. Useful for feature detection (new functionality often comes with
>      new internal kernel types), as well as handling type renames and bigger
>      refactorings.
>    - TYPE_SIZE allows to get the real size (in bytes) of a specified kernel
>      type. Useful for dumping internal structure as-is through perfbuf or
>      ringbuf.
>    - TYPE_ID_LOCAL/TYPE_ID_TARGET allow to capture BTF type ID of a BTF type in
>      program's BTF or kernel BTF, respectively. These could be used for
>      high-performance and space-efficient generic data dumping/logging by
>      relying on small and cheap BTF type ID as a data layout descriptor, for
>      post-processing on user-space side.
>    - ENUMVAL_EXISTS can be used for detecting the presence of enumerator value
>      in kernel's enum type. Most direct application is to detect BPF helper
>      support in kernel.
>    - ENUMVAL_VALUE allows to relocate real integer value of kernel enumerator
>      value, which is subject to change (e.g., always a potential issue for
>      internal, non-UAPI, kernel enums).
> 
> I've indicated potential applications for these relocations, but relocations
> themselves are generic and unassuming and are designed to work correctly even
> in unintended applications. Furthermore, relocated values become constants,
> known to the verifier and could and would be used for dead branch code
> detection and elimination. This makes them ideal to do all sorts of feature
> detection and guarding functionality that's not available on some older (but
> still supported by BPF program) kernels, while having to compile and maintain
> one unified source code.
> 
> Selftests are added for all the new features. Selftests utilizing new Clang
> built-ins are designed such that they will compile with older Clangs and will
> be skipped during test runs. So this shouldn't cause any build and test
> failures on systems with slightly outdated Clang compiler.
> 
> LLVM patches adding these relocation in Clang:
>    - __builtin_btf_type_id() ([0], [1], [2]);
>    - __builtin_preserve_type_info(), __builtin_preserve_enum_value() ([3], [4]).
> 
>    [0] https://urldefense.proofpoint.com/v2/url?u=https-3A__reviews.llvm.org_D74572&d=DwIFAg&c=5VD0RTtNlTh3ycd41b3MUw&r=DA8e1B5r073vIqRrFz7MRA&m=Sr14A5U1sClCcGVCk0rEi3mKHUVfuqMPeiM1_clUDhA&s=aOQT0NmQAfxLsMwCsIJjEkiZHZEe_Gu3KbH_KzcvJwg&e=
>    [1] https://urldefense.proofpoint.com/v2/url?u=https-3A__reviews.llvm.org_D74668&d=DwIFAg&c=5VD0RTtNlTh3ycd41b3MUw&r=DA8e1B5r073vIqRrFz7MRA&m=Sr14A5U1sClCcGVCk0rEi3mKHUVfuqMPeiM1_clUDhA&s=0Dal6DRCFzut_D-NAd72XZSra5qiBLDZD8E2YVQbIsQ&e=
>    [2] https://urldefense.proofpoint.com/v2/url?u=https-3A__reviews.llvm.org_D85174&d=DwIFAg&c=5VD0RTtNlTh3ycd41b3MUw&r=DA8e1B5r073vIqRrFz7MRA&m=Sr14A5U1sClCcGVCk0rEi3mKHUVfuqMPeiM1_clUDhA&s=1gWWaCVLCsmvdZalFuQNxINYvv1We4yRQ2VRyreJe3A&e=
>    [3] https://urldefense.proofpoint.com/v2/url?u=https-3A__reviews.llvm.org_D83878&d=DwIFAg&c=5VD0RTtNlTh3ycd41b3MUw&r=DA8e1B5r073vIqRrFz7MRA&m=Sr14A5U1sClCcGVCk0rEi3mKHUVfuqMPeiM1_clUDhA&s=HvZx6wUsq8kazfavPBRGWFH6zEDYjfkrpvvsC2UgWjI&e=
>    [4] https://urldefense.proofpoint.com/v2/url?u=https-3A__reviews.llvm.org_D83242&d=DwIFAg&c=5VD0RTtNlTh3ycd41b3MUw&r=DA8e1B5r073vIqRrFz7MRA&m=Sr14A5U1sClCcGVCk0rEi3mKHUVfuqMPeiM1_clUDhA&s=Y_Dg-Tl0lQOtbD36XQjZ20P2T7exTymmzqklOqC-USI&e=
> 
> v1->v2:
>    - selftests detect built-in support and are skipped if not found (Alexei).
> 
> Andrii Nakryiko (5):
>    libbpf: implement type-based CO-RE relocations support
>    selftests/bpf: test TYPE_EXISTS and TYPE_SIZE CO-RE relocations
>    selftests/bpf: add CO-RE relo test for TYPE_ID_LOCAL/TYPE_ID_TARGET
>    libbpf: implement enum value-based CO-RE relocations
>    selftests/bpf: add tests for ENUMVAL_EXISTS/ENUMVAL_VALUE relocations
> 
>   tools/lib/bpf/bpf_core_read.h                 |  80 +++-
>   tools/lib/bpf/libbpf.c                        | 376 ++++++++++++++++--
>   tools/lib/bpf/libbpf_internal.h               |   6 +
>   .../selftests/bpf/prog_tests/core_reloc.c     | 349 ++++++++++++++--
>   .../bpf/progs/btf__core_reloc_enumval.c       |   3 +
>   .../progs/btf__core_reloc_enumval___diff.c    |   3 +
>   .../btf__core_reloc_enumval___err_missing.c   |   3 +
>   .../btf__core_reloc_enumval___val3_missing.c  |   3 +
>   .../bpf/progs/btf__core_reloc_type_based.c    |   3 +
>   ...btf__core_reloc_type_based___all_missing.c |   3 +
>   .../btf__core_reloc_type_based___diff_sz.c    |   3 +
>   ...f__core_reloc_type_based___fn_wrong_args.c |   3 +
>   .../btf__core_reloc_type_based___incompat.c   |   3 +
>   .../bpf/progs/btf__core_reloc_type_id.c       |   3 +
>   ...tf__core_reloc_type_id___missing_targets.c |   3 +
>   .../selftests/bpf/progs/core_reloc_types.h    | 327 ++++++++++++++-
>   .../bpf/progs/test_core_reloc_enumval.c       |  73 ++++
>   .../bpf/progs/test_core_reloc_kernel.c        |   2 +
>   .../bpf/progs/test_core_reloc_type_based.c    | 111 ++++++
>   .../bpf/progs/test_core_reloc_type_id.c       | 107 +++++
>   20 files changed, 1408 insertions(+), 56 deletions(-)
>   create mode 100644 tools/testing/selftests/bpf/progs/btf__core_reloc_enumval.c
>   create mode 100644 tools/testing/selftests/bpf/progs/btf__core_reloc_enumval___diff.c
>   create mode 100644 tools/testing/selftests/bpf/progs/btf__core_reloc_enumval___err_missing.c
>   create mode 100644 tools/testing/selftests/bpf/progs/btf__core_reloc_enumval___val3_missing.c
>   create mode 100644 tools/testing/selftests/bpf/progs/btf__core_reloc_type_based.c
>   create mode 100644 tools/testing/selftests/bpf/progs/btf__core_reloc_type_based___all_missing.c
>   create mode 100644 tools/testing/selftests/bpf/progs/btf__core_reloc_type_based___diff_sz.c
>   create mode 100644 tools/testing/selftests/bpf/progs/btf__core_reloc_type_based___fn_wrong_args.c
>   create mode 100644 tools/testing/selftests/bpf/progs/btf__core_reloc_type_based___incompat.c
>   create mode 100644 tools/testing/selftests/bpf/progs/btf__core_reloc_type_id.c
>   create mode 100644 tools/testing/selftests/bpf/progs/btf__core_reloc_type_id___missing_targets.c
>   create mode 100644 tools/testing/selftests/bpf/progs/test_core_reloc_enumval.c
>   create mode 100644 tools/testing/selftests/bpf/progs/test_core_reloc_type_based.c
>   create mode 100644 tools/testing/selftests/bpf/progs/test_core_reloc_type_id.c
> 

Thanks for implementing this new btf relocations in libbpf.
Hopefully people will find them useful and start to use them once clang 
12 becomes widely available.

I do have a few nits when I apply the patch to my local bpf-next and
one suggestion for change. With that,
Acked-by: Yonghong Song <yhs@...com>

Powered by blists - more mailing lists