On Sat, Sep 24, 2022 at 6:18 AM Hou Tao <houtao@xxxxxxxxxxxxxxx> wrote: > > From: Hou Tao <houtao1@xxxxxxxxxx> > > Userspace application uses bpf syscall to lookup or update bpf map. It > passes a pointer of fixed-size buffer to kernel to represent the map > key. To support map with variable-length key, introduce bpf_dynptr_user > to allow userspace to pass a pointer of bpf_dynptr_user to specify the > address and the length of key buffer. And in order to represent dynptr > from userspace, adding a new dynptr type: BPF_DYNPTR_TYPE_USER. Because > BPF_DYNPTR_TYPE_USER-typed dynptr is not available from bpf program, so > no verifier update is needed. > > Add dynptr_key_off in bpf_map to distinguish map with fixed-size key > from map with variable-length. dynptr_key_off is less than zero for > fixed-size key and can only be zero for dynptr key. > > For dynptr-key map, key btf type is bpf_dynptr and key size is 16, so > use the lower 32-bits of map_extra to specify the maximum size of dynptr > key. > > Signed-off-by: Hou Tao <houtao1@xxxxxxxxxx> > --- This is a great feature and you've put lots of high-quality work into this! Looking forward to have qp-trie BPF map available. Apart from your discussion with Alexie about locking and memory allocation/reused, I have questions about this dynptr from user-space interface. Let's discuss it in this patch to not interfere. I'm trying to understand why there should be so many new concepts and interfaces just to allow variable-sized keys. Can you elaborate on that? Like why do we even need BPF_DYNPTR_TYPE_USER? Why user can't just pass a void * (casted to u64) pointer and size of the memory pointed to it, and kernel will just copy necessary amount of data into kvmalloc'ed temporary region? It also seems like you want to allow key (and maybe value as well, not sure) to be a custom user-defined type where some of the fields are struct bpf_dynptr. I think it's a big overcomplication, tbh. I'd say it's enough to just say that entire key has to be described by a single bpf_dynptr. Then we can have bpf_map_lookup_elem_dynptr(map, key_dynptr, flags) new helper to provide variable-sized key for lookup. I think it would keep it much simpler. But if I'm missing something, it would be good to understand that. Thanks! > include/linux/bpf.h | 8 +++ > include/uapi/linux/bpf.h | 6 ++ > kernel/bpf/map_in_map.c | 3 + > kernel/bpf/syscall.c | 121 +++++++++++++++++++++++++++------ > tools/include/uapi/linux/bpf.h | 6 ++ > 5 files changed, 125 insertions(+), 19 deletions(-) > [...]
