On Thu, May 09, 2024 at 05:29:41PM -0700, Kui-Feng Lee wrote:
> Not only a user space program can detach a struct_ops link, the subsystem
> managing a link can also detach the link. This patch adds a kfunc to
> simulate detaching a link by the subsystem managing it and makes sure user
> space programs get notified through epoll.
>
> Signed-off-by: Kui-Feng Lee <thinker.li@xxxxxxxxx>
> ---
> .../selftests/bpf/bpf_testmod/bpf_testmod.c | 42 ++++++++++++
> .../bpf/bpf_testmod/bpf_testmod_kfunc.h | 1 +
> .../bpf/prog_tests/test_struct_ops_module.c | 67 +++++++++++++++++++
> .../selftests/bpf/progs/struct_ops_detach.c | 7 ++
> 4 files changed, 117 insertions(+)
>
> diff --git a/tools/testing/selftests/bpf/bpf_testmod/bpf_testmod.c b/tools/testing/selftests/bpf/bpf_testmod/bpf_testmod.c
> index 1150e758e630..1f347eed6c18 100644
> --- a/tools/testing/selftests/bpf/bpf_testmod/bpf_testmod.c
> +++ b/tools/testing/selftests/bpf/bpf_testmod/bpf_testmod.c
> @@ -741,6 +741,38 @@ __bpf_kfunc int bpf_kfunc_call_kernel_getpeername(struct addr_args *args)
> return err;
> }
>
> +static DEFINE_SPINLOCK(detach_lock);
> +static struct bpf_link *link_to_detach;
> +
> +__bpf_kfunc int bpf_dummy_do_link_detach(void)
> +{
> + struct bpf_link *link;
> + int ret = -ENOENT;
> +
> + /* A subsystem must ensure that a link is valid when detaching the
> + * link. In order to achieve that, the subsystem may need to obtain
> + * a lock to safeguard a table that holds the pointer to the link
> + * being detached. However, the subsystem cannot invoke
> + * link->ops->detach() while holding the lock because other tasks
> + * may be in the process of unregistering, which could lead to
> + * acquiring the same lock and causing a deadlock. This is why
> + * bpf_link_inc_not_zero() is used to maintain the link's validity.
> + */
> + spin_lock(&detach_lock);
> + link = link_to_detach;
> + /* Make sure the link is still valid by increasing its refcnt */
> + if (link && IS_ERR(bpf_link_inc_not_zero(link)))
> + link = NULL;
> + spin_unlock(&detach_lock);
> +
I know it probably doesn't matter in this example, but where would you set
link_to_detach to NULL if reg and unreg can be called multiple times?
> + if (link) {
> + ret = link->ops->detach(link);
> + bpf_link_put(link);
> + }
> +
> + return ret;
> +}
[...]
