> On Jul 30, 2020, at 12:03 PM, Andrii Nakryiko <andrii.nakry...@gmail.com> 
> wrote:
> 
> On Thu, Jul 30, 2020 at 10:43 AM Song Liu <songliubrav...@fb.com> wrote:
>> 
>> 
>> 
>>> On Jul 29, 2020, at 4:05 PM, Andrii Nakryiko <andr...@fb.com> 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 <andr...@fb.com>
>> 
>> 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 <songliubrav...@fb.com>

Reply via email to