Re: [PATCH bpf-next v1 5/8] bpf/selftests: ksyms_btf to test typed ksyms

On Sat, Aug 22, 2020 at 12:27 AM Hao Luo [off-list ref] wrote:

On Fri, Aug 21, 2020 at 4:03 PM Andrii Nakryiko
[off-list ref] wrote:

quoted

On Thu, Aug 20, 2020 at 10:32 AM Yonghong Song [off-list ref] wrote:

quoted

On 8/19/20 3:40 PM, Hao Luo wrote:

quoted

Selftests for typed ksyms. Tests two types of ksyms: one is a struct,
the other is a plain int. This tests two paths in the kernel. Struct
ksyms will be converted into PTR_TO_BTF_ID by the verifier while int
typed ksyms will be converted into PTR_TO_MEM.

Signed-off-by: Hao Luo <redacted>
---
  .../selftests/bpf/prog_tests/ksyms_btf.c      | 77 +++++++++++++++++++
  .../selftests/bpf/progs/test_ksyms_btf.c      | 23 ++++++
  2 files changed, 100 insertions(+)
  create mode 100644 tools/testing/selftests/bpf/prog_tests/ksyms_btf.c
  create mode 100644 tools/testing/selftests/bpf/progs/test_ksyms_btf.c

diff --git a/tools/testing/selftests/bpf/prog_tests/ksyms_btf.c b/tools/testing/selftests/bpf/prog_tests/ksyms_btf.c
new file mode 100644
index 000000000000..1dad61ba7e99
--- /dev/null
+++ b/tools/testing/selftests/bpf/prog_tests/ksyms_btf.c

@@ -0,0 +1,77 @@
+// SPDX-License-Identifier: GPL-2.0
+/* Copyright (c) 2020 Google */
+
+#include <test_progs.h>
+#include <bpf/libbpf.h>
+#include <bpf/btf.h>
+#include "test_ksyms_btf.skel.h"
+
+static int duration;
+
+static __u64 kallsyms_find(const char *sym)
+{
+     char type, name[500];
+     __u64 addr, res = 0;
+     FILE *f;
+
+     f = fopen("/proc/kallsyms", "r");
+     if (CHECK(!f, "kallsyms_fopen", "failed to open: %d\n", errno))
+             return 0;

could you check whether libbpf API can provide this functionality for
you? As far as I know, libbpf does parse /proc/kallsyms.

No need to use libbpf's implementation. We already have
kallsyms_find() in prog_tests/ksyms.c and a combination of
load_kallsyms() + ksym_get_addr() in trace_helpers.c. It would be good
to switch to one implementation for both prog_tests/ksyms.c and this
one.

Ack. I can do some refactoring. The least thing that I can do is
moving kallsyms_find() to a header for both prog_tests/ksyms.c and
this test to use.

Please no extra headers. Just put kallsyms_find() in trace_helpers.c,
along other kallsyms-related functions.

quoted

quoted

quoted

diff --git a/tools/testing/selftests/bpf/progs/test_ksyms_btf.c b/tools/testing/selftests/bpf/progs/test_ksyms_btf.c
new file mode 100644
index 000000000000..e04e31117f84
--- /dev/null
+++ b/tools/testing/selftests/bpf/progs/test_ksyms_btf.c

@@ -0,0 +1,23 @@
+// SPDX-License-Identifier: GPL-2.0
+/* Copyright (c) 2020 Google */
+
+#include "vmlinux.h"
+
+#include <bpf/bpf_helpers.h>
+
+__u64 out__runqueues = -1;
+__u64 out__bpf_prog_active = -1;
+
+extern const struct rq runqueues __ksym; /* struct type global var. */
+extern const int bpf_prog_active __ksym; /* int type global var. */
+
+SEC("raw_tp/sys_enter")
+int handler(const void *ctx)
+{
+     out__runqueues = (__u64)&runqueues;
+     out__bpf_prog_active = (__u64)&bpf_prog_active;
+

You didn't test accessing any of the members of runqueues, because BTF
only has per-CPU variables, right? Adding global/static variables was
adding too much data to BTF or something like that, is that right?

Right. With some experiments, I found the address of a percpu variable
doesn't necessarily point to a valid structure. So it doesn't make
sense to dereference runqueues and access its members. However, right
now there are only percpu variables encoded in BTF, so I can't test
accessing members of general global/static variables unfortunately.

Hao