Netdev Archive on lore.kernel.org
help / color / mirror / Atom feed
From: Yonghong Song <yhs@fb.com>
To: Andrii Nakryiko <andriin@fb.com>, <bpf@vger.kernel.org>,
	<netdev@vger.kernel.org>, <ast@fb.com>, <daniel@iogearbox.net>
Cc: <andrii.nakryiko@gmail.com>, <kernel-team@fb.com>
Subject: Re: [PATCH v2 bpf-next 0/5] Add support for type-based and enum value-based CO-RE relocations
Date: Wed, 19 Aug 2020 09:24:57 -0700	[thread overview]
Message-ID: <e2cd666b-4b97-141f-6925-fc00d8b4aa5f@fb.com> (raw)
In-Reply-To: <20200819052849.336700-1-andriin@fb.com>



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@fb.com>

      parent reply	other threads:[~2020-08-19 16:25 UTC|newest]

Thread overview: 12+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-08-19  5:28 Andrii Nakryiko
2020-08-19  5:28 ` [PATCH v2 bpf-next 1/5] libbpf: implement type-based CO-RE relocations support Andrii Nakryiko
2020-08-19  5:28 ` [PATCH v2 bpf-next 2/5] selftests/bpf: test TYPE_EXISTS and TYPE_SIZE CO-RE relocations Andrii Nakryiko
2020-08-19 16:30   ` Yonghong Song
2020-08-19 19:29     ` Andrii Nakryiko
2020-08-19  5:28 ` [PATCH v2 bpf-next 3/5] selftests/bpf: add CO-RE relo test for TYPE_ID_LOCAL/TYPE_ID_TARGET Andrii Nakryiko
2020-08-19 16:43   ` Yonghong Song
2020-08-19 19:30     ` Andrii Nakryiko
2020-08-19  5:28 ` [PATCH v2 bpf-next 4/5] libbpf: implement enum value-based CO-RE relocations Andrii Nakryiko
2020-08-19  5:28 ` [PATCH v2 bpf-next 5/5] selftests/bpf: add tests for ENUMVAL_EXISTS/ENUMVAL_VALUE relocations Andrii Nakryiko
2020-08-19 16:44   ` Yonghong Song
2020-08-19 16:24 ` Yonghong Song [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=e2cd666b-4b97-141f-6925-fc00d8b4aa5f@fb.com \
    --to=yhs@fb.com \
    --cc=andrii.nakryiko@gmail.com \
    --cc=andriin@fb.com \
    --cc=ast@fb.com \
    --cc=bpf@vger.kernel.org \
    --cc=daniel@iogearbox.net \
    --cc=kernel-team@fb.com \
    --cc=netdev@vger.kernel.org \
    --subject='Re: [PATCH v2 bpf-next 0/5] Add support for type-based and enum value-based CO-RE relocations' \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).