On Thu, Jul 30, 2020 at 10:43 AM Song Liu <songliubraving@xxxxxx> wrote: > > > > > On Jul 29, 2020, at 4:05 PM, Andrii Nakryiko <andriin@xxxxxx> 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 <andriin@xxxxxx> > > 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. > > Thanks, > Song > > [...]
