Re: BPF memstore and bpf_validate_ext()

[Mindaugas Rasiukevicius <rmind%netbsd.org@localhost>]

Alexander Nasonov <alnsn%yandex.ru@localhost> wrote:
> > The external memory store can be used as an argument (and the initial
> > values determined as proposed in this thread).  If you want, we can pass
> > it as a third argument, I just think it is a pointless indirection
> > level. Would even need extra wrapping i.e. more work in bpfjit, but if
> > you want that separated - fine.
> 
> I'll tell you more. Mixing mem with arguments will not only save
> me one register/one indirection level but it will allow to use
> memory addressing. It's very attractive solution from a performance
> perspective. But it's clearly a hack and I don't want to have hacks
> in public API.

We have discussed this before and I wrote quite long paragraph trying to
explain why I think such split does not really have real merits.  In a
nutshell: such separation does not solve any problem (does not reduce
complexity, does not help to honour the information hiding principle)
and the logical justification to have bpf_stack_t is a bit weak.

I am fine/neutral about adding a third uint32_t *mem argument.

> > That is great, but we are going circles here.  If a program just needs
> > some values stored in the memory store - why would you create a COP to
> > get few integers instead of simply letting the caller to pass them.  It
> > is the thing which actually has those numbers.
> 
> What if your program "just needs" something else? Does it mean we have
> to add a new feature to bpf? No. We have to stop somewhere. <...>

We should not stop because of the quantity.  We should consider whether it
*makes sense* to add such feature, looking at its virtues and implications.

I have already explained the benefits of the external memory store in this
thread (simple, straightforward way to pass values and have their cache).
Your main argument seems to be that the external memstore makes it more
difficult for bpfjit to optimise certain corner cases (which, as been
pointed out, are not common).

Given that BPF did not have JIT compiler for many years and even a very
simplistic JIT compilation is a huge win - I do not understand why do you
consider corner case optimisations as a higher value than the benefits
provided by the external memstore.

> BPF_COP
> is powerful enough for getting external data. It's a bit slower but I
> can give you SLJIT_FAST_CALL interface. If it's still slow for you,
> you should stop using sljit and consider other alternatives.

That is wonderful, but not the point..

> > > It's easy to
> > > suggest to have a flag but it's actually a lot or work. You need to
> > > write several lines of C code to generate a single instruction.
> > 
> > The flag would basically say "treat the memstore as internal i.e. just
> > do all the optimisations, because I assure there are no side effects".
> > It is a green light for what you said you already want to implement.
> > That would be one-liner check or I miss something?
> 
> Ok, with my recent change on github, it's indeed one-line change
> but having two different modes can introduce subtle bugs. If you
> forget to set the mode properly, you may have a situation when your
> memory is in a bad state only for packets of length 131 (or whatever)
> and it's consistent for all other lengths.
> I'm as a maintainer of bpfjit is responsible for a robust public
> interface.

Just keep the flag off by default?  Really, we have only few BPF users
in the NetBSD tree.  It is not rocket science to correctly set a flag.

-- 
Mindaugas