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

[Martin KaFai Lau <kafai@fb.com>]

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;
> +}