Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]


Groups > linux.kernel > #1716867

Re: [PATCH v3 net-next] bpf/verifier: track liveness for pruning

From Edward Cree <ecree@solarflare.com>
Newsgroups linux.kernel
Subject Re: [PATCH v3 net-next] bpf/verifier: track liveness for pruning
Date 2017-08-21 22:50 +0200
Message-ID <uh1O2-GL-7@gated-at.bofh.it> (permalink)
References (1 earlier) <ufQhX-4UA-21@gated-at.bofh.it> <ufQhX-4UA-19@gated-at.bofh.it> <ugZMe-7RS-15@gated-at.bofh.it> <ugZMe-7RS-13@gated-at.bofh.it> <uh1uI-Ac-53@gated-at.bofh.it>
Organization linux.* mail to news gateway

Show all headers | View raw


On 21/08/17 21:27, Daniel Borkmann wrote:
> On 08/21/2017 08:36 PM, Edward Cree wrote:
>> On 19/08/17 00:37, Alexei Starovoitov wrote:
> [...]
>> I'm tempted to just rip out env->varlen_map_value_access and always check
>>   the whole thing, because honestly I don't know what it was meant to do
>>   originally or how it can ever do any useful pruning.  While drastic, it
>>   does cause your test case to pass.
>
> Original intention from 484611357c19 ("bpf: allow access into map
> value arrays") was that it wouldn't potentially make pruning worse
> if PTR_TO_MAP_VALUE_ADJ was not used, meaning that we wouldn't need
> to take reg state's min_value and max_value into account for state
> checking; this was basically due to min_value / max_value is being
> adjusted/tracked on every alu/jmp ops for involved regs (e.g.
> adjust_reg_min_max_vals() and others that mangle them) even if we
> have the case that no actual dynamic map access is used throughout
> the program. To give an example on net tree, the bpf_lxc.o prog's
> section increases from 36,386 to 68,226 when env->varlen_map_value_access
> is always true, so it does have an effect. Did you do some checks
> on this on net-next?
I tested with the cilium progs and saw no change in insn count.  I
 suspect that for the normal case I already killed this optimisation
 when I did my unification patch, it was previously about ignoring
 min/max values on all regs (including scalars), whereas on net-next
 it only ignores them on map_value pointers; in practice this is
 useless because we tend to still have the offset scalar sitting in
 a register somewhere.  (Come to think of it, this may have been
 behind a large chunk of the #insn increase that my patches caused.)
Since we use umax_value in find_good_pkt_pointers() now (to check
 against MAX_PACKET_OFF and ensure our reg->range is really ok), we
 can't just stop caring about all min/max values just because we
 haven't done any variable map accesses.
I don't see a way around this.

-Ed

Back to linux.kernel | Previous | NextPrevious in thread | Next in thread | Find similar | Unroll thread


Thread

[PATCH v3 net-next] bpf/verifier: track liveness for pruning Edward Cree <ecree@solarflare.com> - 2017-08-15 21:40 +0200
  Re: [PATCH v3 net-next] bpf/verifier: track liveness for pruning Daniel Borkmann <daniel@iogearbox.net> - 2017-08-16 00:20 +0200
    Re: [PATCH v3 net-next] bpf/verifier: track liveness for pruning David Miller <davem@davemloft.net> - 2017-08-16 01:40 +0200
  Re: [PATCH v3 net-next] bpf/verifier: track liveness for pruning Edward Cree <ecree@solarflare.com> - 2017-08-18 16:20 +0200
    Re: [PATCH v3 net-next] bpf/verifier: track liveness for pruning Edward Cree <ecree@solarflare.com> - 2017-08-21 20:40 +0200
      Re: [PATCH v3 net-next] bpf/verifier: track liveness for pruning Daniel Borkmann <daniel@iogearbox.net> - 2017-08-21 22:30 +0200
        Re: [PATCH v3 net-next] bpf/verifier: track liveness for pruning Edward Cree <ecree@solarflare.com> - 2017-08-21 22:50 +0200
          Re: [PATCH v3 net-next] bpf/verifier: track liveness for pruning Daniel Borkmann <daniel@iogearbox.net> - 2017-08-21 23:10 +0200
    Re: [PATCH v3 net-next] bpf/verifier: track liveness for pruning Edward Cree <ecree@solarflare.com> - 2017-08-21 22:30 +0200

csiph-web