Re: [PATCH v4 net-next 1/3] Extended BPF interpreter and converter

[Daniel Borkmann <dborkman@redhat.com>]

On 03/04/2014 06:09 PM, Alexei Starovoitov wrote:
> On Tue, Mar 4, 2014 at 1:59 AM, Daniel Borkmann <dborkman@redhat.com> wrote:
...
>> Hmm, so the case statement is about BPF_RET | BPF_A and BPF_RET | BPF_K
>> but BPF_RET | BPF_X is not mentioned. However, in BPF_SRC(fp->code)
>> selection you fall back to BPF_X if it doesn't equal BPF_K? Is that
>> correct? And, you probably also need to handle BPF_RET | BPF_X ?
>
> :) that design choice of original BPF always puzzled me.
> BPF_A macro only used in one insn: BPF_RET + BPF_A
> and all other insns use BPF_K and BPF_X
> and though comment in uapi/filter.h says "ret - BPF_K and BPF_X also apply"
> this is not true, since sk_chk_filter() only allows ret+a and ret+k
> libpcap is equally confused. It never generates ret+x, but has few
> places in the code
> that can recognize it. I guess that's an artifact of distant past.

Good point, ret+a and ret+k are main users anyway, though we could fix
that limitation actually. ;)

> epbf has only one RET insn that takes register R0 and returns it.
> That is similar to real CPU 'ret' insn and done to make epbf easier
> to generate from gcc/llvm point of view.
> ebpf jit converts 'ret' into 'leave; ret' on x86_64.
>
> so original bpf+k and bpf+a are converted into 'mov r0, [a or k]; ret r0'
>
> btw, if there is interest I can put ebpf testsuite into tools/net/

Yes, please. Would be great if you can place the test suite under:

   tools/testing/selftests/net/bpf/

I believe some stuff there could get its own folder e.g. "packet"
for PF_PACKET test cases etc, so that we can easily arrange them.