On Tue, May 3, 2022 at 10:15 AM Maxim Mikityanskiy maximmi@nvidia.com wrote:
Before this commit, the BPF verifier required ARG_PTR_TO_MEM arguments to be followed by ARG_CONST_SIZE holding the size of the memory region. The helpers had to check that size in runtime.
There are cases where the size expected by a helper is a compile-time constant. Checking it in runtime is an unnecessary overhead and waste of BPF registers.
This commit allows helpers to accept ARG_PTR_TO_MEM arguments without the corresponding ARG_CONST_SIZE, given that they define the memory region size in struct bpf_func_proto.
I think it's much less confusing and cleaner to have ARG_PTR_TO_FIXED_SIZE_MEM (or whatever similar name), instead of adding special casing to ARG_PTR_TO_MEM.
Signed-off-by: Maxim Mikityanskiy maximmi@nvidia.com Reviewed-by: Tariq Toukan tariqt@nvidia.com
include/linux/bpf.h | 10 ++++++++++ kernel/bpf/verifier.c | 26 +++++++++++++++----------- 2 files changed, 25 insertions(+), 11 deletions(-)
diff --git a/include/linux/bpf.h b/include/linux/bpf.h index be94833d390a..255ae3652225 100644 --- a/include/linux/bpf.h +++ b/include/linux/bpf.h @@ -514,6 +514,16 @@ struct bpf_func_proto { }; u32 *arg_btf_id[5]; };
union {struct {size_t arg1_size;size_t arg2_size;size_t arg3_size;size_t arg4_size;size_t arg5_size;};size_t arg_size[5];};
We have almost 250 instances of struct bpf_func_proto variables:
$ rg 'const struct bpf_func_proto.* = {' | wc -l 244
You are adding 40 bytes to it just to use it for 1-2 special prototypes. It is quite expensive, IMHO, to increase vmlinux image size by 10KB just for this.
Should we reuse arg_btf_id union (same argument won't be PTR_TO_BTF_ID and PTR_TO_FIXED_SIZE_MEM, right)? Or alternatively special-case those few prototypes in verifier code directly when trying to fetch memory size?
int *ret_btf_id; /* return value btf_id */ bool (*allowed)(const struct bpf_prog *prog);};
[...]