ubpf icon indicating copy to clipboard operation
ubpf copied to clipboard

uBPF should support > 64 helper_id

Open Alan-Jowett opened this issue 3 years ago • 6 comments

https://github.com/iovisor/ubpf/blob/6492b7a9363ea1e32194a95b66617df0f7fd42de/vm/ubpf_vm.c#L27

The ebpf-for-windows uses a disjointed helper-id space, allowing for both global helper functions and program type specific helper functions, with the following mapping: 0x1-0xFFFF - Global helper functions usable by any program type 0x1000 - 0x1FFF - Program type specific helper functions

Alan-Jowett avatar May 12 '21 03:05 Alan-Jowett

Might be nice if we can set this via ubpf_create, since not every VM in the same process may have the same purpose (and thus might have varying needs for available helpers).

jpsamaroo avatar May 12 '21 15:05 jpsamaroo

I was leaning towards a callout. Something like:

int ubpf_set_helper_resolver(struct ubpf_vm *vm, void* resolver_context, uintptr_t (*resolver_function)(void* context, uint32_t helper_id));

The consumer of this library can then control the resolution of helper_id -> address of helper function.

Alan-Jowett avatar May 12 '21 15:05 Alan-Jowett

Isn't that just a more complicated form of ubpf_register: https://github.com/iovisor/ubpf/blob/c7c019c507c09c20243c5cbaf535d73a979aec23/vm/ubpf_vm.c#L85?

jpsamaroo avatar May 14 '21 15:05 jpsamaroo

Fair enough. We would then need some form of sparse storage for the helper-id -> address mapping. The issue is that we might need to store:

For an EBPF_PROGRAM_TYPE_XDP

--- Global ---
0x0001->bpf_map_lookup_elem
0x0002->bpf_map_update_elem
0x0003->bpf_map_delete_elem
--- Program specific ---
0x1001->bpf_xdp_redirect
0x1002->bpf_xdp_encap
0x1003->bpf_xdp_decap
0x1004->bpf_xdp_transpose

Or for EBPF_PROGRAM_TYPE_BIND

--- Global ---
0x0001->bpf_map_lookup_elem
0x0002->bpf_map_update_elem
0x0003->bpf_map_delete_elem
--- Program specific ---
0x1001->bpf_bind_redirect

Note: These are currently made up examples (we haven't added anything like this yet).

The current storage assumes helper_id is the position in the array, which won't work well with disjointed helper-id space.

We can either have the complexity of storing the mapping in ubpf or in the caller. In my case the caller needs to store it anyway, so duplicating it in ubpf doesn't make sense for my scenario, but if you think it is something that would be more generally usable, I am happy to add it.

Alan-Jowett avatar May 14 '21 15:05 Alan-Jowett

I think I'm in favor of having an optional callback to do this mapping at runtime/JIT time. I still think ubpf_create should accept an argument to allow specifying the initial size of ext_funcs, too.

jpsamaroo avatar May 18 '21 21:05 jpsamaroo

Doing this at JIT time is redundant. We can perform the translation on the byte code prior to JIT / interpret and update the eBPF byte code.

So, the only limitation then is if there are > 64 unique helper functions being called by the eBPF program, which seems unlikely.

Alan-Jowett avatar May 19 '21 17:05 Alan-Jowett