Re: [PATCH net-next v2 2/4] cgroup: bpf: Add BPF_MAP_TYPE_CGROUP_ARRAY

From: Martin KaFai Lau
Date: Thu Jun 23 2016 - 17:26:42 EST


On Thu, Jun 23, 2016 at 11:42:31AM +0200, Daniel Borkmann wrote:
> Hi Martin,
>
> [ sorry to jump late in here, on pto currently ]
Thanks for reviewing.

> Could you describe a bit more with regards to pinning maps and how this
> should interact with cgroups? The two specialized array maps we have (tail
> calls, perf events) have fairly complicated semantics for when to clean up
> map slots (see commits c9da161c6517ba1, 3b1efb196eee45b2f0c4).
>
> How is this managed with cgroups? Once a cgroup fd is placed into a map and
> the user removes the cgroup, will this be prevented due to 'being busy', or
> will the cgroup live further as long as a program is running with a cgroup
> map entry (but the cgroup itself is not visible from user space in any way
> anymore)?
Having a cgroup ptr stored in the bpf_map will not stop the user from
removing the cgroup (by rmdir /mnt/cgroup2/tc/test_cgrp).

The cgroup ptr stored in the bpf_map holds a refcnt which answer the
second part.

The situation is similar to the netfilter usecase in
commit 38c4597e4bf ("netfilter: implement xt_cgroup cgroup2 path match")

>
> I presume it's a valid use case to pin a cgroup map, put fds into it and
> remove the pinned file expecting to continue to match on it, right? So
> lifetime is really until last prog using a cgroup map somewhere gets removed
> (even if not accessible from user space anymore, meaning no prog has fd and
> pinned file was removed).
Yes.

We are still hatching out how to set this up in production. However, the
situation is similar to removing the pinned file.
We probably will not use tc and pin a bpf_map to do that. Instead,
one process will setup eveything (e.g. create the cgroup, pouplate the
cgroup map, load the bpf to egress) and then go away.

I don't think we need a prog fd to remove the bpf prog.

>
> I assume that using struct file here doesn't make sense (commit e03e7ee34fdd1c3)
> either, right?
No. I don't think so. We eventually need a cgroup from the 'struct file'.