On Wed, Feb 21, 2024 at 11:17 AM Dave Thaler <dthaler1968@xxxxxxxxxxxxxx> wrote: > > -BPF_CALL 0x8 0x0 call helper function by address BPF_JMP | BPF_K only, see `Helper functions`_ > +BPF_CALL 0x8 0x0 call_by_address(imm) BPF_JMP | BPF_K only > +BPF_CALL 0x8 0x0 call_by_address(dst) BPF_JMP | BPF_X only ... > +* call_by_address(value) means to call a helper function by the address specified by 'value' (see `Helper functions`_ for details) Sorry, we're not going to take this path in the kernel verifier. I understand that you went with this semantics in PREVAIL verifier, but this is user space and I suspect once PREVAIL folks realize that it's not that useful you will change that. User space has a luxury to change. The kernel doesn't and we won't be able to change such things in the standard either. Essentially what you're proposing is to treat callx dst_reg as calling any of the existing helpers by a number. Let's look at the first ~6: id = 1 void *bpf_map_lookup_elem(struct bpf_map *map, const void *key) id = 2 long bpf_map_update_elem(struct bpf_map *map, const void *key, const void *value, u64 flags) ... id = 6 long bpf_trace_printk(const char *fmt, u32 fmt_size, ...) They have almost nothing in common. In C that would be an indirect call of "long (*fn)(...)" just call anything and hope it works. This is not useful in practice. Also commit log is wrong: > Only src=0 is currently listed for callx. Neither clang nor gcc > use src=1 or src=2, and both use exactly the same semantics for > src=0 which was agreed between them (Yonghong and Jose). this is not at all what gcc and clang are doing. They emit "callx dst_reg" when they need to compile a normal indirect call which address is in dst_reg. It's the real address of the function and not a helper ID. Hence these two: > +BPF_CALL 0x8 0x0 call_by_address(imm) BPF_JMP | BPF_K only > +BPF_CALL 0x8 0x0 call_by_address(dst) BPF_JMP | BPF_X only are not correct. call imm is a call of helper with a given ID. callx dst_reg is a call of a function by its real address. This is _prelminary_ definition of callx dst_reg from compiler pov, but there is no implementation of it in the kernel, so it's way too early to hard code such semantics in the standard. pw-bot: cr
