On Tue, Jul 21, 2020 at 1:05 PM Martin KaFai Lau <kafai@xxxxxx> wrote: > I quickly checked zero is actually BPF_CGROUP_INET_INGRESS instead > of UNSPEC for attach_type. It will be confusing on the syscall > side. e.g. map dumping with key.attach_type == BPF_CGROUP_INET_INGRESS > while the only cgroup program is actually attaching to BPF_CGROUP_INET_EGRESS. > > I don't have a clean way out for this. Adding a non-zero UNSPEC > to "enum bpf_attach_type" seems wrong also. > One possible way out is to allow the bpf_cgroup_storage_map > to have a smaller key size (i.e. allow both sizeof(cgroup_inode_id) > and the existing sizeof(struct bpf_cgroup_storage_key). That > will completely remove attach_type from the picture if the user > specified to use sizeof(cgroup_inode_id) as the key_size. > If sizeof(struct bpf_cgroup_storage_key) is specified, the attach_type > is still used in cmp(). > > The bpf_cgroup_storage_key_cmp() need to do cmp accordingly. > Same changes is needed to lookup_elem, delete_elem, check_btf, map_alloc...etc. ACK. Considering that the cgroup_inode_id is the first field of struct bpf_cgroup_storage_key, I can probably just use this fact and directly use the user-provided map key (after it's copied to kernel space) and cast the pointer to a __u64 - or are there any architectures where the first field is not at offset zero? If this is done then we can change all the kernel internal APIs to use __u64 cgroup_inode_id, ignoring the existence of the struct aside from the map creation (size checking and BTF checking). > > +#include "../cgroup/cgroup-internal.h" > Why? cgroup_mutex is declared in this header. YiFei Zhu
