Message ID | 20200818223921.2911963-1-andriin@fb.com |
---|---|
Headers | show |
Series | Add support for type-based and enum value-based CO-RE relocations | expand |
On Tue, Aug 18, 2020 at 03:39:12PM -0700, Andrii Nakryiko wrote: > This patch set adds libbpf support to 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): > > LLVM patches adding these relocation in Clang: > - __builtin_btf_type_id() ([0], [1], [2]); > - __builtin_preserve_type_info(), __builtin_preserve_enum_value() ([3], [4]). I've applied patches 1-4, since they're somewhat indepedent of new features in 5+. What should be the process to land the rest? Land llvm first and add to bpf/README.rst that certain llvm commmits are necessary to build the tests? But CI will start failing. We can wait for that to be fixed, but I wonder is there way to detect new clang __builtins automatically in selftests and skip them if clang is too old?
On Tue, Aug 18, 2020 at 6:21 PM Alexei Starovoitov <alexei.starovoitov@gmail.com> wrote: > > On Tue, Aug 18, 2020 at 03:39:12PM -0700, Andrii Nakryiko wrote: > > This patch set adds libbpf support to 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): > > > > LLVM patches adding these relocation in Clang: > > - __builtin_btf_type_id() ([0], [1], [2]); > > - __builtin_preserve_type_info(), __builtin_preserve_enum_value() ([3], [4]). > > I've applied patches 1-4, since they're somewhat indepedent of new features in 5+. > What should be the process to land the rest? > Land llvm first and add to bpf/README.rst that certain llvm commmits are necessary > to build the tests? Clang patches landed about two weeks ago, so they are already in Clang nightly builds. libbpf CI should work fine as it uses clang-12 nightly builds. > But CI will start failing. We can wait for that to be fixed, > but I wonder is there way to detect new clang __builtins automatically in > selftests and skip them if clang is too old? There is a way to detect built-ins availability (__has_builtin macro, [0]) from C code. If we want to do it from Makefile, though, we'd need to do feature detection similar to how we did reallocarray and libbpf-elf-mmap detection I just removed in the other patch set :). Then we'll also need to somehow blacklist tests. Maintaining that would be a pain, honestly. So far selftests/bpf assumed the latest Clang, though, so do you think we should change that, or you were worried that patches hadn't landed yet?
On Tue, Aug 18, 2020 at 06:31:51PM -0700, Andrii Nakryiko wrote: > On Tue, Aug 18, 2020 at 6:21 PM Alexei Starovoitov > <alexei.starovoitov@gmail.com> wrote: > > > > On Tue, Aug 18, 2020 at 03:39:12PM -0700, Andrii Nakryiko wrote: > > > This patch set adds libbpf support to 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): > > > > > > LLVM patches adding these relocation in Clang: > > > - __builtin_btf_type_id() ([0], [1], [2]); > > > - __builtin_preserve_type_info(), __builtin_preserve_enum_value() ([3], [4]). > > > > I've applied patches 1-4, since they're somewhat indepedent of new features in 5+. > > What should be the process to land the rest? > > Land llvm first and add to bpf/README.rst that certain llvm commmits are necessary > > to build the tests? > > Clang patches landed about two weeks ago, so they are already in Clang > nightly builds. libbpf CI should work fine as it uses clang-12 nightly > builds. > > > > But CI will start failing. We can wait for that to be fixed, > > but I wonder is there way to detect new clang __builtins automatically in > > selftests and skip them if clang is too old? > > There is a way to detect built-ins availability (__has_builtin macro, > [0]) from C code. If we want to do it from Makefile, though, we'd need > to do feature detection similar to how we did reallocarray and > libbpf-elf-mmap detection I just removed in the other patch set :). > Then we'll also need to somehow blacklist tests. Maintaining that > would be a pain, honestly. So far selftests/bpf assumed the latest > Clang, though, so do you think we should change that, or you were > worried that patches hadn't landed yet? I was hoping that libbpf.h can have builtins unconditionally, but selftests can do feature detection automatically and mark them as 'skip'. People have been forever complaining about constant need to upgrade clang. In this case I think the feature is not fundamental enough (unlike the first set of builtins) to force adoption of new clang. If/when we start using these new builtins beyond selftests that would be a different story.
On Tue, Aug 18, 2020 at 6:37 PM Alexei Starovoitov <alexei.starovoitov@gmail.com> wrote: > > On Tue, Aug 18, 2020 at 06:31:51PM -0700, Andrii Nakryiko wrote: > > On Tue, Aug 18, 2020 at 6:21 PM Alexei Starovoitov > > <alexei.starovoitov@gmail.com> wrote: > > > > > > On Tue, Aug 18, 2020 at 03:39:12PM -0700, Andrii Nakryiko wrote: > > > > This patch set adds libbpf support to 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): > > > > > > > > LLVM patches adding these relocation in Clang: > > > > - __builtin_btf_type_id() ([0], [1], [2]); > > > > - __builtin_preserve_type_info(), __builtin_preserve_enum_value() ([3], [4]). > > > > > > I've applied patches 1-4, since they're somewhat indepedent of new features in 5+. > > > What should be the process to land the rest? > > > Land llvm first and add to bpf/README.rst that certain llvm commmits are necessary > > > to build the tests? > > > > Clang patches landed about two weeks ago, so they are already in Clang > > nightly builds. libbpf CI should work fine as it uses clang-12 nightly > > builds. > > > > > > > But CI will start failing. We can wait for that to be fixed, > > > but I wonder is there way to detect new clang __builtins automatically in > > > selftests and skip them if clang is too old? > > > > There is a way to detect built-ins availability (__has_builtin macro, > > [0]) from C code. If we want to do it from Makefile, though, we'd need > > to do feature detection similar to how we did reallocarray and > > libbpf-elf-mmap detection I just removed in the other patch set :). > > Then we'll also need to somehow blacklist tests. Maintaining that > > would be a pain, honestly. So far selftests/bpf assumed the latest > > Clang, though, so do you think we should change that, or you were > > worried that patches hadn't landed yet? > > I was hoping that libbpf.h can have builtins unconditionally, but selftests can > do feature detection automatically and mark them as 'skip'. > People have been forever complaining about constant need to upgrade clang. > In this case I think the feature is not fundamental enough (unlike the first > set of builtins) to force adoption of new clang. > If/when we start using these new builtins beyond selftests > that would be a different story. Ok, took some tinkering to do this for btf_type_id() tests, but everything works now as you described above. Posted v2.