Message ID | 20200219004236.2291125-1-yhs@fb.com |
---|---|
State | Accepted |
Delegated to: | BPF Maintainers |
Headers | show |
Series | [bpf-next] selftests/bpf: change llvm flag -mcpu=probe to -mcpu=v3 | expand |
On Tue, Feb 18, 2020 at 4:44 PM Yonghong Song <yhs@fb.com> wrote: > > The latest llvm supports cpu version v3, which is cpu version v1 > plus some additional 64bit jmp insns and 32bit jmp insn support. > > In selftests/bpf Makefile, the llvm flag -mcpu=probe did runtime > probe into the host system. Depending on compilation environments, > it is possible that runtime probe may fail, e.g., due to > memlock issue. This will cause generated code with cpu version v1. > This may cause confusion as the same compiler and the same C code > generates different byte codes in different environment. > > Let us change the llvm flag -mcpu=probe to -mcpu=v3 so the > generated code will be the same regardless of the compilation > environment. > > Signed-off-by: Yonghong Song <yhs@fb.com> > --- Acked-by: Andrii Nakryiko <andriin@fb.com> > tools/testing/selftests/bpf/Makefile | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > [...]
On 2/19/20 1:42 AM, Yonghong Song wrote: > The latest llvm supports cpu version v3, which is cpu version v1 > plus some additional 64bit jmp insns and 32bit jmp insn support. > > In selftests/bpf Makefile, the llvm flag -mcpu=probe did runtime > probe into the host system. Depending on compilation environments, > it is possible that runtime probe may fail, e.g., due to > memlock issue. This will cause generated code with cpu version v1. But those are tiny BPF progs that LLVM is probing. If memlock is not sufficient, should it try to bump the limit with the diff needed and only if that fails as well then it bails out to v1. > This may cause confusion as the same compiler and the same C code > generates different byte codes in different environment. > > Let us change the llvm flag -mcpu=probe to -mcpu=v3 so the > generated code will be the same regardless of the compilation > environment. > > Signed-off-by: Yonghong Song <yhs@fb.com>
On 2/19/20 8:56 AM, Daniel Borkmann wrote: > On 2/19/20 1:42 AM, Yonghong Song wrote: >> The latest llvm supports cpu version v3, which is cpu version v1 >> plus some additional 64bit jmp insns and 32bit jmp insn support. >> >> In selftests/bpf Makefile, the llvm flag -mcpu=probe did runtime >> probe into the host system. Depending on compilation environments, >> it is possible that runtime probe may fail, e.g., due to >> memlock issue. This will cause generated code with cpu version v1. > > But those are tiny BPF progs that LLVM is probing. If memlock is not > sufficient, should it try to bump the limit with the diff needed and > only if that fails as well then it bails out to v1. The selftest is also trying to test the latest kernel, so we want to keep cpu version as v3 always? There are cases e.g., compilation on a devserver and selftests running in a VM. The linux directory is shared between the host and the qemu. qemu runs latest kernel and devserver's old kernel may have small memlock or in unlikely case the old kernel may simply not supporting the cpu v3, so in such cases, maybe forcing cpu=v3 is better since this is what we intend to test? > >> This may cause confusion as the same compiler and the same C code >> generates different byte codes in different environment. >> >> Let us change the llvm flag -mcpu=probe to -mcpu=v3 so the >> generated code will be the same regardless of the compilation >> environment. >> >> Signed-off-by: Yonghong Song <yhs@fb.com>
On Wed, Feb 19, 2020 at 8:56 AM Daniel Borkmann <daniel@iogearbox.net> wrote: > > On 2/19/20 1:42 AM, Yonghong Song wrote: > > The latest llvm supports cpu version v3, which is cpu version v1 > > plus some additional 64bit jmp insns and 32bit jmp insn support. > > > > In selftests/bpf Makefile, the llvm flag -mcpu=probe did runtime > > probe into the host system. Depending on compilation environments, > > it is possible that runtime probe may fail, e.g., due to > > memlock issue. This will cause generated code with cpu version v1. > > But those are tiny BPF progs that LLVM is probing. If memlock is not > sufficient, should it try to bump the limit with the diff needed and > only if that fails as well then it bails out to v1. with hundred parallel clangs running and all stamping on the same rlimit I don't think bumping that limit can work. Also building on older kernel should still do v3, since build should produce selftest binaries for the same vmlinux as this kernel tree. We hit this issue with github/libbpf CI. The vm used to do the build was too old. So far we cannot build vmlinux out of latest tree, boot into it and only then build selftests inside. It's too complex for CI system. So we build vmlinux and build selftests in that CI's VM, and then boot into it and run selftests. Upgrading VM is an easy fix for now, but the issue will cause the problems later. So imo fixing selftests build to predictable -mcpu=v3 is the most sensible way.
On Wed, Feb 19, 2020 at 9:06 AM Alexei Starovoitov <alexei.starovoitov@gmail.com> wrote: > > On Wed, Feb 19, 2020 at 8:56 AM Daniel Borkmann <daniel@iogearbox.net> wrote: > > > > On 2/19/20 1:42 AM, Yonghong Song wrote: > > > The latest llvm supports cpu version v3, which is cpu version v1 > > > plus some additional 64bit jmp insns and 32bit jmp insn support. > > > > > > In selftests/bpf Makefile, the llvm flag -mcpu=probe did runtime > > > probe into the host system. Depending on compilation environments, > > > it is possible that runtime probe may fail, e.g., due to > > > memlock issue. This will cause generated code with cpu version v1. > > > > But those are tiny BPF progs that LLVM is probing. If memlock is not > > sufficient, should it try to bump the limit with the diff needed and > > only if that fails as well then it bails out to v1. > > with hundred parallel clangs running and all stamping on the same rlimit > I don't think bumping that limit can work. > Also building on older kernel should still do v3, since build should > produce selftest binaries for the same vmlinux as this kernel tree. > We hit this issue with github/libbpf CI. The vm used to do the build > was too old. So far we cannot build vmlinux out of latest tree, > boot into it and only then build selftests inside. It's too complex > for CI system. > So we build vmlinux and build selftests in that CI's VM, and then boot into it > and run selftests. > Upgrading VM is an easy fix for now, but the issue will cause the problems > later. So imo fixing selftests build to predictable -mcpu=v3 is the > most sensible way. Applied. Thanks
On 2/20/20 12:17 AM, Alexei Starovoitov wrote: > On Wed, Feb 19, 2020 at 9:06 AM Alexei Starovoitov > <alexei.starovoitov@gmail.com> wrote: >> On Wed, Feb 19, 2020 at 8:56 AM Daniel Borkmann <daniel@iogearbox.net> wrote: >>> On 2/19/20 1:42 AM, Yonghong Song wrote: >>>> The latest llvm supports cpu version v3, which is cpu version v1 >>>> plus some additional 64bit jmp insns and 32bit jmp insn support. >>>> >>>> In selftests/bpf Makefile, the llvm flag -mcpu=probe did runtime >>>> probe into the host system. Depending on compilation environments, >>>> it is possible that runtime probe may fail, e.g., due to >>>> memlock issue. This will cause generated code with cpu version v1. >>> >>> But those are tiny BPF progs that LLVM is probing. If memlock is not >>> sufficient, should it try to bump the limit with the diff needed and >>> only if that fails as well then it bails out to v1. >> >> with hundred parallel clangs running and all stamping on the same rlimit >> I don't think bumping that limit can work. Right, my main worry is that the default memlock limit is usually very low, so it would be quite ugly to have the probe fail and fall-back to v1 even though the underlying kernel would be totally fine to support v3 in general. Hard-coding v3 for selftests is okay; perhaps we need to resurrect the old CAP_IPC_LOCK patch or some different accounting, the memlock limit has never been working great from a usability pov. >> Also building on older kernel should still do v3, since build should >> produce selftest binaries for the same vmlinux as this kernel tree. >> We hit this issue with github/libbpf CI. The vm used to do the build >> was too old. So far we cannot build vmlinux out of latest tree, >> boot into it and only then build selftests inside. It's too complex >> for CI system. >> So we build vmlinux and build selftests in that CI's VM, and then boot into it >> and run selftests. >> Upgrading VM is an easy fix for now, but the issue will cause the problems >> later. So imo fixing selftests build to predictable -mcpu=v3 is the >> most sensible way. It would definitely be great to test all at some point, meaning, test run with v1, v2, v3 to ensure there are no regressions e.g. on verifier side for all of them. Thanks, Daniel
diff --git a/tools/testing/selftests/bpf/Makefile b/tools/testing/selftests/bpf/Makefile index 257a1aaaa37d..2a583196fa51 100644 --- a/tools/testing/selftests/bpf/Makefile +++ b/tools/testing/selftests/bpf/Makefile @@ -209,7 +209,7 @@ define CLANG_BPF_BUILD_RULE $(call msg,CLNG-LLC,$(TRUNNER_BINARY),$2) ($(CLANG) $3 -O2 -target bpf -emit-llvm \ -c $1 -o - || echo "BPF obj compilation failed") | \ - $(LLC) -mattr=dwarfris -march=bpf -mcpu=probe $4 -filetype=obj -o $2 + $(LLC) -mattr=dwarfris -march=bpf -mcpu=v3 $4 -filetype=obj -o $2 endef # Similar to CLANG_BPF_BUILD_RULE, but with disabled alu32 define CLANG_NOALU32_BPF_BUILD_RULE @@ -223,7 +223,7 @@ define CLANG_NATIVE_BPF_BUILD_RULE $(call msg,CLNG-BPF,$(TRUNNER_BINARY),$2) ($(CLANG) $3 -O2 -emit-llvm \ -c $1 -o - || echo "BPF obj compilation failed") | \ - $(LLC) -march=bpf -mcpu=probe $4 -filetype=obj -o $2 + $(LLC) -march=bpf -mcpu=v3 $4 -filetype=obj -o $2 endef # Build BPF object using GCC define GCC_BPF_BUILD_RULE
The latest llvm supports cpu version v3, which is cpu version v1 plus some additional 64bit jmp insns and 32bit jmp insn support. In selftests/bpf Makefile, the llvm flag -mcpu=probe did runtime probe into the host system. Depending on compilation environments, it is possible that runtime probe may fail, e.g., due to memlock issue. This will cause generated code with cpu version v1. This may cause confusion as the same compiler and the same C code generates different byte codes in different environment. Let us change the llvm flag -mcpu=probe to -mcpu=v3 so the generated code will be the same regardless of the compilation environment. Signed-off-by: Yonghong Song <yhs@fb.com> --- tools/testing/selftests/bpf/Makefile | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)