Re: BPF memstore and bpf_validate_ext()

[Alexander Nasonov <alnsn%yandex.ru@localhost>]

Mindaugas Rasiukevicius wrote:
> Alexander Nasonov <alnsn%yandex.ru@localhost> wrote:
> > Are you saying that we want to pass some values to bpf programs that
> > don't use cop?
> 
> Including the BPF programs which do not use COP, yes.
> 
> > > Isn't it only i386?  On amd64 or RISC archs it would mean just an extra
> > > fetch, right?  I am not sure what is (or would be) the current register
> > > allocation in the latest bpfjit snapshot.  Still, having memstore as a
> > > cache ought to win way more compared to the extra fetches or push/pops.
> > 
> > You can win even more if you switch to manually crafted assembly or
> > build a sophisticated JIT language tailored for NPF. You don't have
> > to use bpfjit at all.
> 
> Well, sljit is what we've got.  I would think it can do nearly as good
> as manual crafting if we cover the main cases based on Pareto principle.

All your recent changes to adapt bpfjit for npf show that you're hitting
sljit limitation all the time. Your expectations of sljit are probably
too high.

Originally, there were three arguments (buf, wirelen and buflen)
passed to a generated function. You wanted to add two more: ctx
and an auxiliary pointer. However, sljit doesn't support functions
with more than three arguments. So, you packed the arguments into
a struct:

struct bpf_args {
        const struct mbuf *     pkt;
        size_t                  wirelen;
        size_t                  buflen;
        uint32_t                mem[BPF_MEMWORDS];
        void *                  arg;
};

But you also added non-argument mem[BPF_MEMWORDS] array. If you
wanted external memory, you'd better passed mem array separately.
But you were hitting three arguments limit of sljit when calling
a copfunc.

When I later was implementing copfunc calls in bpfjit, I din't know
how you gonna use external memory and I felt that it wasn't necessary
(and I still do), so I changed your structs.

I moved non-argument "mem" from the bpf_args argument pack:

struct bpf_args {
        const struct mbuf *     pkt;
        size_t                  wirelen;
        size_t                  buflen;
        void *                  arg;
};

to bpf_state struct:

struct bpf_state {
        uint32_t        mem[BPF_MEMWORDS];
        uint32_t        regA;
};

and I passed bpf_state to copfunc.

By not making memory external I avoided doing a lot of changes in
bpfjit to adapt to a new execution model.

Now you want to have *both* external memory and copfuncs for a faster
execution of your code without considering other uses of bpfjit.

To be honest, I'm not very intersted in implementing external memory
in bpfjit. Quite the opposite, after all these discussions I want
to improve my index check optimization and make filters like 'host xxx'
run a bit faster. Needless to say that this optimization is the most
effective if bpf memory isn't visible from outside.

This discussion is going to nowhere. At this point, someone from
outside should review our opinions and help us to make a decision.

Alex

> > > Right, but this is for the coprocessors.  I am fine with it (as we
> > > talked privately, I was considering to propose a very similar API), but
> > > it seems that bpf_set_initwords() already *efficiently* covers all my
> > > cases.  With this change, I will actually be able to remove NPF_COP_L3!
> > 
> > I think I know what are you getting at.
> > 
> > In your case BPF_COP is always the first instruction (or in the
> > first linear block). Because it's the first and it's a function
> > call, it can be moved outside of bpf program and inlined.
> > 
> > If this is the case, you don't need BPF_COP at all.
> 
> For this particular case - yes, correct.
> 
> > Except of course you may run out or memwords one day and you want
> > to have BPF_COP as a fallback.
> 
> I still need BPF_COP for other tasks (e.g. NPF_COP_TABLE).  Running out
> of words is unlikely, but COP can certainly be used to handle that too.
> 
> -- 
> Mindaugas
> 

--