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

[Yonghong Song]

On 4/28/20 6:32 PM, Martin KaFai Lau wrote:

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 <y...@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()?

Yes, we can do it.

+               ret = -EINVAL;
+               goto out_unlock;
+       }
+
+       link->prog = new_prog;

Does the old link->prog need a bpf_prog_put()?

The old_prog is replaced in caller link_update (syscall.c):
static int link_update(union bpf_attr *attr)
{
        struct bpf_prog *old_prog = NULL, *new_prog;
        struct bpf_link *link;
        u32 flags;
        int ret;
...
        if (link->ops == &bpf_iter_link_lops) {
                ret = bpf_iter_link_replace(link, old_prog, new_prog);
                goto out_put_progs;
        }
        ret = -EINVAL;

out_put_progs:
        if (old_prog)
                bpf_prog_put(old_prog);
...

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