On Mon, Jul 20, 2020 at 02:54:53PM -0500, YiFei Zhu wrote: > From: YiFei Zhu <zhuyifei@xxxxxxxxxx> > > This change comes in several parts: > > One, the restriction that the CGROUP_STORAGE map can only be used > by one program is removed. This results in the removal of the field > 'aux' in struct bpf_cgroup_storage_map, and removal of relevant > code associated with the field, and removal of now-noop functions > bpf_free_cgroup_storage and bpf_cgroup_storage_release. > > Second, because there could be multiple attach types to the same > cgroup, the attach type is completely ignored on comparison in > the map key. Newly added keys have it zeroed. The only value in > the key that still matters is the cgroup inode. bpftool map dump > will also show an attach type of zero. 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. [ ... ] > diff --git a/kernel/bpf/local_storage.c b/kernel/bpf/local_storage.c > index 51bd5a8cb01b..0b94b4ba99ba 100644 > --- a/kernel/bpf/local_storage.c > +++ b/kernel/bpf/local_storage.c > @@ -9,6 +9,8 @@ > #include <linux/slab.h> > #include <uapi/linux/btf.h> > > +#include "../cgroup/cgroup-internal.h" Why? Others LGTM.
