Re: [PATCH] net: filter: Convert the BPF VM to threaded code

[Eric Dumazet <eric.dumazet@gmail.com>]

Le mardi 09 août 2011 à 10:29 +0200, Hagen Paul Pfeifer a écrit :
> On Tue, 09 Aug 2011 07:00:27 +0200, Eric Dumazet wrote:
> 
> > I tried this idea since its already an enum and all enum values are
> > handled in the switch, but all gcc versions I used still generate the
> > useless compare and branch (always predicted by modern CPU, so harmless
> > anyway ?)
> 
> Don't think so, the list is rather long - the CPU may not have any
> indication. But strange that your gcc generate a conditional sequence of
> cmp and conditional jump instruction instead of a jump table! Look at
> commit 01f2f3f6ef4d076c, this is what I got after I refactored to an
> _dense_ enum list.

I am speaking of one single conditional branch, the one that is done for
every BPF instruction done in a filter. It's never taken, so must be
predicted by the cpu. A perf session could make sure it is  ;)

>  
> > (But this would need to use a larger kernel_sock_filter with not an u16
> > code, but the target address).
> 
> Not sure if the benefit is worth to try it. Can we avoid any cacheline
> misses? I don't think so.

We avoid some instructions (to load the address from the instruction
code), thats all, for each BPF instruction.

> 
> 
> > +		switch ((enum bpf_inst)fentry->code) {
> 
> That should not differ! Eric, sure that this do the trick?

It is the way to tell gcc to make its optimizations, if they exists on
enum switch (), as David suggested.

If I add a new BPF_S_ANC_NEW_INST value in the enum list, not handled in
the switch(), gcc complains correctly.

net/core/filter.c: In function ‘sk_run_filter’:
net/core/filter.c:130:3: warning: enumeration value ‘BPF_S_ANC_NEW_INST’
not handled in switch