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

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]

Message-ID: <1312878988.2371.9.camel@edumazet-HP-Compaq-6005-Pro-SFF-PC>
Date:	Tue, 09 Aug 2011 10:36:28 +0200
From:	Eric Dumazet <eric.dumazet@...il.com>
To:	Hagen Paul Pfeifer <hagen@...u.net>
Cc:	David Miller <davem@...emloft.net>, rui314@...il.com,
	netdev@...r.kernel.org
Subject: Re: [PATCH] net: filter: Convert the BPF VM to threaded code

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



--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html