netdev - Re: call for bpf progs. Re: [PATCHv2 bpf-next 5/5] selftests: bpf: add test for sk

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 linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Fri, 27 Mar 2020 15:24:58 -0700
From:   Alexei Starovoitov <alexei.starovoitov@...il.com>
To:     Daniel Borkmann <daniel@...earbox.net>
Cc:     Joe Stringer <joe@...d.net.nz>, Lorenz Bauer <lmb@...udflare.com>,
        Yonghong Song <yhs@...com>, bpf <bpf@...r.kernel.org>,
        Networking <netdev@...r.kernel.org>,
        Alexei Starovoitov <ast@...nel.org>,
        Eric Dumazet <eric.dumazet@...il.com>,
        Martin Lau <kafai@...com>,
        john fastabend <john.fastabend@...il.com>
Subject: Re: call for bpf progs. Re: [PATCHv2 bpf-next 5/5] selftests: bpf:
 add test for sk_assign

On Fri, Mar 27, 2020 at 09:16:52PM +0100, Daniel Borkmann wrote:
> 
> Perhaps that would be more useful and always up to date than a copy of the
> code base that would get stale next day? In the end in this context kernel
> changes and/or llvm changes might be of interest to check whether anything
> potentially blows up, so having a self-contained packaging might be useful.
> Thoughts?

Right now we have zero cilium progs in selftest :)
so any number of progs is better than nothing.

> > Do we just parachute the ~11K LoC of Cilium datapath into the kernel
> > tree once per cycle? 

I don't think 11k progs updated every kernel release will catch
much more than basic reduced set.
selftests/bpf is not a substitute for cilium CI.
I would prefer .c tests to be developed once and frozen.
More tests can be added every 6 month or so.
I think copy-paste is ok-ish too, but would be much better
to think through about aspects of the code first.
It worked well for fb xdp/tc progs. I took some of them,
refactored them to selftest/Makefile and they were kept as-is for
the last 3 years. While real progs kept being actively changed.
For example: progs/test_l4lb.c is about 1/10th of the real deal here:
https://github.com/facebookincubator/katran/tree/master/katran/lib/bpf
In terms of lines code, number of includes and so on.
While hacking katran into selftest I tried to capture the _style_ of C code.
Like:
 bool get_packet_dst(struct real_definition **real,
                     struct packet_description *pckt,
                     struct vip_meta *vip_info,
                     bool is_ipv6)
I wouldn't have written the prototype this way.
Passing double pointer as a return value as a first argument is not my style :)
The style of nested 'if', etc. Those are the things that make clang
generate specific code patterns that I was trying to preserve in selftests.

Example 2: progs/strobemeta.h is about 1/4 of real thing.
Yet frozen it was super useful for the work on bounded loops.

Example 3: progs/test_get_stack_rawtp.c is one specific code pattern
that used in our profiling prog.
This one was immensely helpful to track down jmp32/alu32 bugs.
It's the one that we're still fixing (see John's jmp32 work).

and so on.
selftests/bpf/progs don't need to be full copy-paste. Ideally the capture
the essence of the progs in the short form.

clang bpf backend and verifier smartness keep changing. So having
frozen selftests is necessary to see the trend and capture bugs
in the backend and in the verifier.