Thread (14 messages) flat view 14 messages, 2 authors, 2020-09-11

Re: [PATCH bpf-next v4 3/5] libbpf: Add BPF_PROG_BIND_MAP syscall and use it on .metadata section

From: Andrii Nakryiko <hidden>
Date: 2020-09-10 19:41:32
Also in: bpf

On Wed, Sep 9, 2020 at 11:25 AM Stanislav Fomichev [off-list ref] wrote:
From: YiFei Zhu <redacted>

The patch adds a simple wrapper bpf_prog_bind_map around the syscall.
When the libbpf tries to load a program, it will probe the kernel for
the support of this syscall and unconditionally bind .rodata section
btw, you subject is out of sync, still mentions .metadata
to the program.

Cc: YiFei Zhu <redacted>
Signed-off-by: YiFei Zhu <redacted>
Please drop zhuyifei@google.com from CC list (it's unreachable), when
you submit a new version.
quoted hunk ↗ jump to hunk
Signed-off-by: Stanislav Fomichev <redacted>
---
 tools/lib/bpf/bpf.c      | 13 ++++++
 tools/lib/bpf/bpf.h      |  8 ++++
 tools/lib/bpf/libbpf.c   | 94 ++++++++++++++++++++++++++++++++--------
 tools/lib/bpf/libbpf.map |  1 +
 4 files changed, 98 insertions(+), 18 deletions(-)
diff --git a/tools/lib/bpf/bpf.c b/tools/lib/bpf/bpf.c
index 82b983ff6569..5f6c5676cc45 100644
--- a/tools/lib/bpf/bpf.c
+++ b/tools/lib/bpf/bpf.c
@@ -872,3 +872,16 @@ int bpf_enable_stats(enum bpf_stats_type type)

        return sys_bpf(BPF_ENABLE_STATS, &attr, sizeof(attr));
 }
+
+int bpf_prog_bind_map(int prog_fd, int map_fd,
+                     const struct bpf_prog_bind_opts *opts)
+{
+       union bpf_attr attr;
+
you forgot OPTS_VALID check here
+       memset(&attr, 0, sizeof(attr));
+       attr.prog_bind_map.prog_fd = prog_fd;
+       attr.prog_bind_map.map_fd = map_fd;
+       attr.prog_bind_map.flags = OPTS_GET(opts, flags, 0);
+
+       return sys_bpf(BPF_PROG_BIND_MAP, &attr, sizeof(attr));
+}
[...]
quoted hunk ↗ jump to hunk
-static int probe_kern_global_data(void)
+static void probe_create_global_data(int *prog, int *map,
+                                    struct bpf_insn *insns, size_t insns_cnt)
 {
        struct bpf_load_program_attr prg_attr;
        struct bpf_create_map_attr map_attr;
        char *cp, errmsg[STRERR_BUFSIZE];
-       struct bpf_insn insns[] = {
-               BPF_LD_MAP_VALUE(BPF_REG_1, 0, 16),
-               BPF_ST_MEM(BPF_DW, BPF_REG_1, 0, 42),
-               BPF_MOV64_IMM(BPF_REG_0, 0),
-               BPF_EXIT_INSN(),
-       };
-       int ret, map;
+       int err;

        memset(&map_attr, 0, sizeof(map_attr));
        map_attr.map_type = BPF_MAP_TYPE_ARRAY;
@@ -3748,26 +3749,40 @@ static int probe_kern_global_data(void)
        map_attr.value_size = 32;
        map_attr.max_entries = 1;

-       map = bpf_create_map_xattr(&map_attr);
-       if (map < 0) {
-               ret = -errno;
-               cp = libbpf_strerror_r(ret, errmsg, sizeof(errmsg));
+       *map = bpf_create_map_xattr(&map_attr);
+       if (*map < 0) {
+               err = errno;
+               cp = libbpf_strerror_r(err, errmsg, sizeof(errmsg));
                pr_warn("Error in %s():%s(%d). Couldn't create simple array map.\n",
-                       __func__, cp, -ret);
-               return ret;
+                       __func__, cp, -err);
+               return;
        }

-       insns[0].imm = map;
+       insns[0].imm = *map;
I think I already complained about this? You are assuming that
insns[0] is BPF_LD_MAP_VALUE, which is true only for one case out of
two already! It's just by luck that probe_prog_bind_map works because
the verifier ignores the exit code, apparently.

If this doesn't generalize well, don't generalize. But let's not do a
blind instruction rewrite, which will cause tons of confusion later.
quoted hunk ↗ jump to hunk
        memset(&prg_attr, 0, sizeof(prg_attr));
        prg_attr.prog_type = BPF_PROG_TYPE_SOCKET_FILTER;
        prg_attr.insns = insns;
-       prg_attr.insns_cnt = ARRAY_SIZE(insns);
+       prg_attr.insns_cnt = insns_cnt;
        prg_attr.license = "GPL";

-       ret = bpf_load_program_xattr(&prg_attr, NULL, 0);
+       *prog = bpf_load_program_xattr(&prg_attr, NULL, 0);
+}
+
+static int probe_kern_global_data(void)
+{
+       struct bpf_insn insns[] = {
+               BPF_LD_MAP_VALUE(BPF_REG_1, 0, 16),
+               BPF_ST_MEM(BPF_DW, BPF_REG_1, 0, 42),
+               BPF_MOV64_IMM(BPF_REG_0, 0),
+               BPF_EXIT_INSN(),
+       };
+       int prog = -1, map = -1;
+
+       probe_create_global_data(&prog, &map, insns, ARRAY_SIZE(insns));
+
        close(map);
-       return probe_fd(ret);
+       return probe_fd(prog);
 }

 static int probe_kern_btf(void)
@@ -3894,6 +3909,32 @@ static int probe_kern_probe_read_kernel(void)
        return probe_fd(bpf_load_program_xattr(&attr, NULL, 0));
 }

+static int probe_prog_bind_map(void)
+{
+       struct bpf_insn insns[] = {
+               BPF_MOV64_IMM(BPF_REG_0, 0),
+               BPF_EXIT_INSN(),
+       };
+       int prog = -1, map = -1, ret = 0;
+
+       if (!kernel_supports(FEAT_GLOBAL_DATA))
+               return 0;
+
+       probe_create_global_data(&prog, &map, insns, ARRAY_SIZE(insns));
+
+       if (map >= 0 && prog < 0) {
+               close(map);
+               return 0;
+       }
+
+       if (!bpf_prog_bind_map(prog, map, NULL))
+               ret = 1;
+
+       close(map);
+       close(prog);
+       return ret;
+}
+
[...]
quoted hunk ↗ jump to hunk
diff --git a/tools/lib/bpf/libbpf.map b/tools/lib/bpf/libbpf.map
index 92ceb48a5ca2..0b7830f4ff8b 100644
--- a/tools/lib/bpf/libbpf.map
+++ b/tools/lib/bpf/libbpf.map
@@ -308,4 +308,5 @@ LIBBPF_0.2.0 {
                perf_buffer__epoll_fd;
                perf_buffer__consume_buffer;
                xsk_socket__create_shared;
+               bpf_prog_bind_map;
please keep this list sorted
 } LIBBPF_0.1.0;
--
2.28.0.526.ge36021eeef-goog
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help