On Tue, Oct 18, 2022 at 11:08 AM Martin KaFai Lau <martin.lau@xxxxxxxxx> wrote:
>
> On 10/18/22 10:17 AM, Alexei Starovoitov wrote:
> > On Tue, Oct 18, 2022 at 10:08 AM <sdf@xxxxxxxxxx> wrote:
> >>>>
> >>>> '#define BPF_MAP_TYPE_CGROUP_STORAGE BPF_MAP_TYPE_CGRP_LOCAL_STORAGE /*
> >>>> depreciated by BPF_MAP_TYPE_CGRP_STORAGE */' in the uapi.
> >>>>
> >>>> The new cgroup storage uses a shorter name "cgrp", like
> >>>> BPF_MAP_TYPE_CGRP_STORAGE and bpf_cgrp_storage_get()?
> >>
> >>> This might work and the naming convention will be similar to
> >>> existing sk/inode/task storage.
> >>
> >> +1, CGRP_STORAGE sounds good!
> >
> > +1 from me as well.
> >
> > Something like this ?
> > diff --git a/include/uapi/linux/bpf.h b/include/uapi/linux/bpf.h
> > index 17f61338f8f8..13dcb2418847 100644
> > --- a/include/uapi/linux/bpf.h
> > +++ b/include/uapi/linux/bpf.h
> > @@ -922,7 +922,8 @@ enum bpf_map_type {
> > BPF_MAP_TYPE_CPUMAP,
> > BPF_MAP_TYPE_XSKMAP,
> > BPF_MAP_TYPE_SOCKHASH,
> > - BPF_MAP_TYPE_CGROUP_STORAGE,
> > + BPF_MAP_TYPE_CGROUP_STORAGE_DEPRECATED,
> > + BPF_MAP_TYPE_CGROUP_STORAGE = BPF_MAP_TYPE_CGROUP_STORAGE_DEPRECATED,
>
> +1
>
> > BPF_MAP_TYPE_REUSEPORT_SOCKARRAY,
> > BPF_MAP_TYPE_PERCPU_CGROUP_STORAGE,
> > BPF_MAP_TYPE_QUEUE,
> > @@ -935,6 +936,7 @@ enum bpf_map_type {
> > BPF_MAP_TYPE_TASK_STORAGE,
> > BPF_MAP_TYPE_BLOOM_FILTER,
> > BPF_MAP_TYPE_USER_RINGBUF,
> > + BPF_MAP_TYPE_CGRP_STORAGE,
> > };
> >
> > What are we going to do with BPF_MAP_TYPE_PERCPU_CGROUP_STORAGE ?
> > Probably should come up with a replacement as well?
>
> Yeah, need to come up with a percpu answer for it. The percpu usage has never
> come up on the sk storage and also the later task/inode storage. or the user is
> just getting by with an array like map's value.
>
> May be the bpf prog can call bpf_mem_alloc() to alloc the percpu memory in the
> future and then store it as the kptr in the BPF_MAP_TYPE_CGRP_STORAGE?
A percpu cgroup storage would be very beneficial for cgroup statistics
collection, things like the selftest in
tools/testing/selftests/bpf/progs/cgroup_hierarchical_stats.c
currently uses a percpu hashmap indexed by cgroup id, so using a
percpu cgroup storage instead would be a nice upgrade.
