On 10/9/20 7:42 PM, Andrii Nakryiko wrote:
On Fri, Oct 9, 2020 at 7:13 AM Daniel Borkmann <daniel@xxxxxxxxxxxxx> wrote:
[...]
static int percpu_array_map_btf_id; const struct bpf_map_ops percpu_array_map_ops = { .map_meta_equal = bpf_map_meta_equal, diff --git a/kernel/bpf/syscall.c b/kernel/bpf/syscall.c index 1110ecd7d1f3..519bf867f065 100644 --- a/kernel/bpf/syscall.c +++ b/kernel/bpf/syscall.c @@ -111,7 +111,8 @@ static struct bpf_map *find_and_alloc_map(union bpf_attr *attr) ops = bpf_map_types[type]; if (!ops) return ERR_PTR(-EINVAL); - + if (ops->map_swap_ops) + ops = ops->map_swap_ops(attr);I'm afraid that this can cause quite a lot of confusion down the road. Wouldn't designating -EOPNOTSUPP return code from map_gen_lookup() and not inlining in that case as if map_gen_lookup() wasn't even defined be a much smaller and more local (semantically) change that achieves exactly the same thing? Doesn't seem like switching from u32 to int for return value would be a big inconvenience for existing implementations of inlining callbacks, right?
I was originally thinking about it, but then decided not to take this path, for example the ops->map_gen_lookup() patching code has sanity checks for the u32 return code on whether we patched 0 or too many instructions, so if there is anything funky going on in one of the map_gen_lookup() that we'd get a negative code, for example, I don't want to just skip and not have the verifier bark loudly with "bpf verifier is misconfigured", also didn't want to make the logic inside fixup_bpf_calls() even more complex, so the patch here felt simpler & more straight forward to me. Thanks, Daniel
