lkml.org 
[lkml]   [2017]   [Jun]   [30]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
SubjectRe: [PATCH v3 net-next 00/12] bpf: rewrite value tracking in verifier
From
Date
On 28/06/17 22:37, Alexei Starovoitov wrote:
> Increasing the limit is must have, since pruning suffered so much.
> Going from 53k to 76k is pretty substantial.
> What is the % increase for tests in selftests/ ?
When I tried to measure the test_verifier tests, they changed hardly at
all, only a couple of percent iirc. But that's with (a) only the
accepted progs get measured, since rejected don't print the #insns line
- and most of the tests in test_verifier are rejected; and (b) those
test progs are pretty small, usually with only a couple of jumps and
not much chance for pruning to occur. So it's really not a great test
case for pruning effectiveness.
I haven't measured the test_progs ones, because I *still* haven't gotten
around to actually setting up a BPF toolchain (it doesn't help that I'm
building everything on a test server that gets reimaged every night to
run our nightly tests...).
> I think we need to pin point exactly the reason.
> Saying we just track more data is not enough.
> We've tried v2 set on our load balancer and also saw ~20% increase.
> I don't remember the absolute numbers.
> These jumps don't make me comfortable with these extra tracking.
> Can you try to roll back ptr&const and full negative/positive tracking
> and see whether it gets back to what we had before?
The ptr&const bit shouldn't be relevant unless your programs are actually
doing that (i.e. ops on pointers other than +/-), which seems surprising.
But if you really are, then it's not too hard to roll it back - just
need to change how adjust_reg_min_max_vals() deals with EACCES.
For a version without full negative/positive tracking, just take the first
3 patches; some of the selftests will fail but hopefully your progs will
still be accepted. If not, we can try jbacik's patch (off-list response
to v2). I will followup this email with a patch to apply on top of the
first 3 that does that and rolls back ptr&const.
> If tnum is causing it that would be reasonable trade off to make,
> but if it's full neg/pos tracking that has no use today other than
> (the whole thing is cleaner) I would rather drop it then.
Well, the full neg/pos tracking was a result of needing to fix the bug I
found with patch #1, and not wanting to confuse myself with the min/max
range while doing my signed/unsigned tracking. But if we can make it
work with jbacik's approach of 'remember whether our current bounds came
from a signed or an unsigned compare', then we can drop or delay the
full neg/pos tracking unless/until pruning is sorted out.

-Ed

\
 
 \ /
  Last update: 2017-06-30 23:15    [W:0.062 / U:2.116 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site