lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening PHC | |
Open Source and information security mailing list archives
| ||
|
Date: Mon, 18 Sep 2017 15:31:32 +0200 From: Daniel Borkmann <daniel@...earbox.net> To: Shuah Khan <shuahkh@....samsung.com>, Alexei Starovoitov <alexei.starovoitov@...il.com>, Edward Cree <ecree@...arflare.com> CC: Shuah Khan <shuah@...nel.org>, Thomas Meyer <thomas@...3r.de>, linux-kernel@...r.kernel.org, linux-kselftest@...r.kernel.org, Networking <netdev@...r.kernel.org> Subject: Re: selftests/bpf doesn't compile On 09/16/2017 12:41 AM, Shuah Khan wrote: > On 09/15/2017 12:48 PM, Daniel Borkmann wrote: >> On 09/15/2017 08:23 PM, Daniel Borkmann wrote: >>> On 09/15/2017 08:07 PM, Alexei Starovoitov wrote: >>>> On Fri, Sep 15, 2017 at 05:58:40PM +0100, Edward Cree wrote: >>>>> On 15/09/17 17:02, Alexei Starovoitov wrote: >>>>>> On Thu, Sep 14, 2017 at 09:33:48AM -0600, Shuah Khan wrote: >>>>>>> Is bpf test intended to be run in kselftest run? The clang dependency might >>>>>>> not be met on majority of the systems. Is this a hard dependency?? >>>>>> It is a hard dependency and clang should be present on majority of the systems. >>>>> I think this is the wrong approach. Making kselftest hard-require clang doesn't >>>>> mean that the bpf tests will be run more often, it means that the rest of the >>>>> kselftests will be run less often. clang is quite big (when I tried to install >>>>> it on one of my test servers, I didn't have enough disk space & had to go on a >>>>> clear-out of unused packages), and most people aren't interested in the bpf >>>>> subsystem specifically; they would rather be able to skip those tests. >>>>> I feel that as long as they know they are skipping some tests (so e.g. they >>>>> won't consider it a sufficient test of a kselftest refactor), that's fine. >>>>> It's not even as though all of the bpf tests require clang; the (smaller) tests >>>>> written directly in raw eBPF instructions could still be run on such a system. >>>>> So I think we should attempt to run as much as possible but accept that clang >>>>> may not be available and have an option to skip some tests in that case. >>>> >>>> imo the value of selftests/bpf is twofold: >>>> 1. it helps bpf developers avoid regressions >>>> 2. as part of continuous integration it helps to catch bpf regressions >>>> that were somehow caused by changes in other parts of the kernel >>>> >>>> If a developer didn't bother to satisfy all bpf tests dependencies >>>> (which includes clang) and ran all tests before sending a patch, >>>> I don't want to see such patches. It just wastes maintainers time >>>> to review code and spot bugs that could have been caught by tests. >>>> Collectively we invested years of work into these tests and >>>> developers better take advantage of it by running all. >>> >>> +1 >>> >>>> If a CI server didn't satisfy all bpf test dependencies, >>>> I don't want such CI setup to be running and reporting results, >>>> since it will give false sense of test coverage. >>>> Test failures due to missing dependencies are hard failures. >>>> We cannot skip them. >>> >>> +1 >> >> Btw, on that note, the folks from zero-day bot run the BPF kselftests >> for a while now just fine and they do run them together with clang, >> so they have the full, proper coverage how it should be. It's not >> how it used to be in the early days, you can just go and install >> llvm/clang package on all the major distros today and you get the >> bpf target by default enabled already. >> >>>> I'd like generic XDP tests to be added to selftests/bpf which >>>> would mean that the latest iproute2 will become a hard dependency >>>> and bpf developers and CI host owners would need to upgrade >>>> their iproute2. >>>> The tests either pass or fail. Skipping them due to missing >>>> dependencies is the same as fail and in that sense I don't want >>>> to change selftests/bpf/Makefile to make it skip clang. >>> >>> I fully agree that for the BPF selftests it is very desirable >>> to not only test the verifier with couple of BPF insn snippets, >>> but to actually load and run programs that more closely resemble >>> real world programs. For more complex interactions these snippets >>> are just limited, think of tail calls, testing perf event output >>> helper, etc, which would all require to write these tests with >>> restricted C when we add them (unless we want to make writing >>> these tests a real pain ;) in which case no-one will bother to >>> write tests at all for them). Mid to long term I would definitely >>> like to see more programs in BPF selftests (e.g. moved over from >>> samples/bpf/) to increase the test coverage. > > As I said in my earlier email: > > Unless users choose to install clang, bpf will always fail run without > clang. So clang dependency is an issue for bpf test coverage in general. > That is your choice as to whether you want to increase the scope of > regression test coverage for bpf or not. > > I fully understand you have weigh that against ease of writing tests. > > We can leave things the way they are since: > > - You can't force users to install clang and run bpf test. Users might > opt for letting bpf test fail due to unmet dependency. > > - You have reasons to continue use clang and you have been using it for > a longtime. I'm definitely for leaving it as it currently is and having clang as hard dependency; there will be more BPF selftests over time that will require to compile BPF progs (through clang's BPF backend) written in restricted C, so just skipping these tests would give a false sense of coverage. clang is pretty much needed anyway for writing more complex programs, thus leaving requirements the way they are is the much better option. > I will try to see why make ksefltest fails on bpf even with my patch > series that addresses the source directory issue. All other tests build > and run. It is not an issue with bpf specifically, it is something that > has never been tested in this use-case. Great, thanks!
Powered by blists - more mailing lists