Re: [PATCH bpf-next 1/5] bpf: add support for forced LINK_DETACH command

On Jul 30, 2020, at 12:03 PM, Andrii Nakryiko [off-list ref] wrote:

On Thu, Jul 30, 2020 at 10:43 AM Song Liu [off-list ref] wrote:

quoted

quoted

On Jul 29, 2020, at 4:05 PM, Andrii Nakryiko [off-list ref] wrote:

Add LINK_DETACH command to force-detach bpf_link without destroying it. It has
the same behavior as auto-detaching of bpf_link due to cgroup dying for
bpf_cgroup_link or net_device being destroyed for bpf_xdp_link. In such case,
bpf_link is still a valid kernel object, but is defuncts and doesn't hold BPF
program attached to corresponding BPF hook. This functionality allows users
with enough access rights to manually force-detach attached bpf_link without
killing respective owner process.

This patch implements LINK_DETACH for cgroup, xdp, and netns links, mostly
re-using existing link release handling code.

Signed-off-by: Andrii Nakryiko <redacted>

The code looks good to me. My only question is, do we need both
bpf_link_ops->detach and bpf_link_ops->release?

I think so. release() is mandatory for final clean up, after the last
FD was closed, so every type of bpf_link has to implement this.
detach() is optional, though, and potentially can do different things
than release(). It just so happens right now that three bpf_linkl
types can re-use release as-is (with minimal change to netns release
specifically for detach use case). So I think having two is better and
more flexible.

I see. Thanks for the explanation. 

Acked-by: Song Liu <redacted>