> On Jul 30, 2020, at 12:03 PM, Andrii Nakryiko <andrii.nakryiko@xxxxxxxxx> wrote: > > 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. I see. Thanks for the explanation. Acked-by: Song Liu <songliubraving@xxxxxx>