On Fri, Feb 28, 2014 at 4:45 AM, Daniel Borkmann <dborkman@xxxxxxxxxx> wrote:...
Did you also test that seccomp-BPF still works out?
yes. Have a prototype, but it needs a bit more cleanup.
We should keep naming consistent (so either extended BPF or BPF64),
so maybe bpf_ext_enable ? I'd presume rather {bpf,sk_filter}*_ext
agree. we need consistent naming for both (old and new).
I'll try all-out rename of bpf_*() into sk_filter64_*() and sk_filter_ext_*()
to see which one looks better.
I'm kinda leaning to sk_filter64, since it's easier to quickly spot
the difference
and more descriptive.
as in 'struct bpf_insn' the immediate value is 32 bit, so for 64 bit
comparisons, you'd still need to load to immediate values, right?
there is no insn that use 64-bit immediate, since 64-bit immediates
are extremely rare. grep x86-64 asm code for movabsq will return very few.
llvm or gcc can easily construct any constant by combination of mov,
shifts and ors.
bpf64 comparisons are all 64-bit right now. So far I didn't see a need to do
32-bit comparison, since old bpf is all unsigned, mapping 32->64 of
Jxx is painless.
Notice I added signed comparison, since real life programs cannot do
without them.
I also kept the spirit of old bpf having > and >= only. (no < and <=)
that made llvm/gcc backends a bit harder to do, since no real cpu has
such limitations.
I'm still contemplating do add < and <= (both signed and unsigned), which is
interesting trade-off: number of instructions vs complexity of compiler
After all your changes, we will still have the bpf_jit_enable knob
in tact, right?
Yes.
In this diff the workflow is the following:
old filter comes through sk_attach_filter() or sk_unattached_filter_create()
if (bpf64) {
convert to new
sk_chk_filter() - check old bpf
use new interpreter
} else {
sk_chk_filter() - check old bpf
if (bpf_jit_enable)
use old jit
else
use old interpreter
}
soon I'll add bpf64 jit and will reuse the same bpf_jit_enable knob for it.
then add new/old inband demux into sk_attach_filter(),
so that workflow will become:
a filter comes through sk_attach_filter() or sk_unattached_filter_create()
if (new filter prog) {
sk_chk_filter64() - check new bpf
if (bpf_jit_enable)
use new jit
else
use new interpreter
} else {
if (bpf64_enable) {
convert to new
sk_chk_filter() - check old bpf
if (bpf_jit_enable)
use new jit
else
use new interpreter
} else {
sk_chk_filter()
if (bpf_jit_enable)
use old jit
else
use old interpreter
}
}
eventually bpf64_enable can be made default,
the last 'else' can be retired and 'bpf64_enable' removed along with
old interpreter.
bpf_jit_enable knob will stay for foreseeable future.
Why would that need to be exported as a symbol?
the performance numbers I mentioned are from bpf_bench that is done
as kernel module, so I used this for debugging from it.
also to see what execution traces I get while running trinity bpf fuzzer :)
I would actually like to avoid having this pr_info* inside the kernel.
Couldn't this be done e.g. through systemtap script that could e.g. be
placed under tools/net/ or inside the documentation file?
like the idea!
Will drop it from the diff and eventually will move it to tools/net.