On Sat, Aug 27, 2016 at 12:40:04PM +0100, Jakub Kicinski wrote: > On Fri, 26 Aug 2016 16:29:05 -0700, Alexei Starovoitov wrote: > > On Fri, Aug 26, 2016 at 07:06:06PM +0100, Jakub Kicinski wrote: > > > Advanced JIT compilers and translators may want to use > > > eBPF verifier as a base for parsers or to perform custom > > > checks and validations. > > > > > > Add ability for external users to invoke the verifier > > > and provide callbacks to be invoked for every intruction > > > checked. For now only add most basic callback for > > > per-instruction pre-interpretation checks is added. More > > > advanced users may also like to have per-instruction post > > > callback and state comparison callback. > > > > > > Signed-off-by: Jakub Kicinski <jakub.kicin...@netronome.com> > > > > I like the apporach. Making verifier into 'bytecode parser' > > that JITs can reuse is a good design choice. > > The only thing I would suggest is to tweak the verifier to > > avoid in-place state recording. Then I think patch 8 for > > clone/unclone of the program won't be needed, since verifier > > will be read-only from bytecode point of view and patch 9 > > also will be slightly cleaner. > > I think there are very few places in verifier that do this > > state keeping inside insn. It was bugging me for some time. > > Good time to clean that up. > > Unless I misunderstand the patches 7,8,9... > > Agreed, I think the verifier only modifies the program to > store pointer types in imm field. I will try to come up > a way around this, any suggestions? Perhaps state_equal()
probably array_of_insn_aux_data[num_insns] should do it. Unlike reg_state that is forked on branches, this array is only one. > logic could be modified to downgrade pointers to UNKONWNs > when it detects other state had incompatible pointer type. > > > There is also small concern for patches 5 and 6 that add > > more register state information. Potentially that extra > > state can prevent states_equal() to recognize equivalent > > states. Only patch 9 uses that info, right? > > 5 and 6 recognize more constant loads, those can only > upgrade some UNKNOWN_VALUEs to CONST_IMMs. So yes, if the > verifier hits the CONST first and then tries with UNKNOWN > it will have to reverify the path. > > > Another question is do you need all state walking that > > verifier does or single linear pass through insns > > would have worked? > > Looks like you're only using CONST_IMM and PTR_TO_CTX > > state, right? > > I think I need all the parsing. Right now I mostly need > the verification to check that exit codes are specific > CONST_IMMs. Clang quite happily does this: > > r0 <- 0 > if (...) > r0 <- 1 > exit I see. Indeed then you'd need the verifier to walk all paths to make sure constant return values. If you only need yes/no check then such info can probably be collected unconditionally during initial program load. Like prog->cb_access flag.
