Re: BPF_MISC+BPF_COP and BPF_COPX

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

Hello Alex,

Let's keep freebsd-net CCed in case there is any interest.

Alexander Nasonov <alnsn%yandex.ru@localhost> wrote:
> 
> I like the idea but I have some questions:
> 
> 1. Why do you need access to scratch memory? Is your goal to keep a
>   state for C code between cop calls? In this case, you can use
>   the standard void* pointer technique. You'd need to define special
>   getter/setter cop functions and they will be slower than getting/setting
>   scratch memory but it's more important to keep a very clear separation
>   between C code and bpf code. You'd also need to pass void* to
>   bpf_filter(9), of course.

Yes, I may want to keep the state in the memory store or pass the arguments
through it, since the accumulator might not be enough.  If you prefer getter
and setter to perform a boundary check and enforce uint32_t type, I am fine
with that.  Although BPF_MEMWORDS constant and words storing 32-bit values
stayed since 80s.. it is not going to change.

However, abusing void * is wrong.  Once bpf_filter(9) is adjusted to take
an opaque struct bpf_ctx *, the memory store ought to be moved into it.

> 2. Why do you set X to 0 and 0xffffffff? For the out-of-range access,
>    all other bpf code aborts filter program with code 0. I think it's
>    better to keep X unchanged.

The rationale behind it is that we might want to handle out-of-range in the
byte-code without breaking the flow.  This can be a valid case for BPF_COPX.
Just BPF_RET 0 seems a limitation, but I do not have a strong feeling here.

> 3. Can cop "patch" itself or other cop functions at runtime? It'd be
>    much safe if an array of cop functions was read-only. As an
>    additional benefit when using bpfjit, you could generate direct calls
>    for BPF_COP (but not for BPF_COPX) in bpfjit_generate_code().

Yes, I think coprocessor functions should be read-only once set.  And the
direct call for BPF_COP when using BPF JIT is very much preferred!

> 
> Alex

-- 
Mindaugas