Re: [PATCH bpf-next v1 06/19] bpf: support bpf tracing/iter programs for BPF_LINK_UPDATE

Martin KaFai Lau
2020-04-29T01:32:57Z

On Mon, Apr 27, 2020 at 01:12:41PM -0700, Yonghong Song wrote:
> Added BPF_LINK_UPDATE support for tracing/iter programs.
> This way, a file based bpf iterator, which holds a reference
> to the link, can have its bpf program updated without
> creating new files.
>
> Signed-off-by: Yonghong Song <yhs@fb.com>
> ---
> include/linux/bpf.h | 2 ++
> kernel/bpf/bpf_iter.c | 29 +++++++++++++++++++++++++++++
> kernel/bpf/syscall.c | 5 +++++
> 3 files changed, 36 insertions(+)
>
> diff --git a/include/linux/bpf.h b/include/linux/bpf.h
> index 60ecb73d8f6d..4fc39d9b5cd0 100644
> --- a/include/linux/bpf.h
> +++ b/include/linux/bpf.h
> @@ -1131,6 +1131,8 @@ struct bpf_prog *bpf_iter_get_prog(struct seq_file *seq, u32 priv_data_size,
> u64 *session_id, u64 *seq_num, bool is_last);
> int bpf_iter_run_prog(struct bpf_prog *prog, void *ctx);
> int bpf_iter_link_attach(const union bpf_attr *attr, struct bpf_prog *prog);
> +int bpf_iter_link_replace(struct bpf_link *link, struct bpf_prog *old_prog,
> + struct bpf_prog *new_prog);
>
> int bpf_percpu_hash_copy(struct bpf_map *map, void *key, void *value);
> int bpf_percpu_array_copy(struct bpf_map *map, void *key, void *value);
> diff --git a/kernel/bpf/bpf_iter.c b/kernel/bpf/bpf_iter.c
> index 9532e7bcb8e1..fc1ce5ee5c3f 100644
> --- a/kernel/bpf/bpf_iter.c
> +++ b/kernel/bpf/bpf_iter.c
> @@ -23,6 +23,9 @@ static struct list_head targets;
> static struct mutex targets_mutex;
> static bool bpf_iter_inited = false;
>
> +/* protect bpf_iter_link.link->prog upddate */
> +static struct mutex bpf_iter_mutex;
> +
> int bpf_iter_reg_target(struct bpf_iter_reg *reg_info)
> {
> struct bpf_iter_target_info *tinfo;
> @@ -33,6 +36,7 @@ int bpf_iter_reg_target(struct bpf_iter_reg *reg_info)
> if (!bpf_iter_inited) {
> INIT_LIST_HEAD(&targets);
> mutex_init(&targets_mutex);
> + mutex_init(&bpf_iter_mutex);
> bpf_iter_inited = true;
> }
>
> @@ -121,3 +125,28 @@ int bpf_iter_link_attach(const union bpf_attr *attr, struct bpf_prog *prog)
> kfree(link);
> return err;
> }
> +
> +int bpf_iter_link_replace(struct bpf_link *link, struct bpf_prog *old_prog,
> + struct bpf_prog *new_prog)
> +{
> + int ret = 0;
> +
> + mutex_lock(&bpf_iter_mutex);
> + if (old_prog && link->prog != old_prog) {
> + ret = -EPERM;
> + goto out_unlock;
> + }
> +
> + if (link->prog->type != new_prog->type ||
> + link->prog->expected_attach_type != new_prog->expected_attach_type ||
> + strcmp(link->prog->aux->attach_func_name, new_prog->aux->attach_func_name)) {
Can attach_btf_id be compared instead of strcmp()?

> + ret = -EINVAL;
> + goto out_unlock;
> + }
> +
> + link->prog = new_prog;
Does the old link->prog need a bpf_prog_put()?

> +
> +out_unlock:
> + mutex_unlock(&bpf_iter_mutex);
> + return ret;
> +}