Re: [PATCH v2 bpf-next 7/7] selftests/bpf: Add unit tests for global functions

On Jan 9, 2020, at 2:09 PM, Alexei Starovoitov [off-list ref] wrote:

On Thu, Jan 09, 2020 at 09:27:26AM -0800, Song Liu wrote:

quoted

On Wed, Jan 8, 2020 at 10:39 PM Alexei Starovoitov [off-list ref] wrote:

quoted

test_global_func[12] - check 512 stack limit.
test_global_func[34] - check 8 frame call chain limit.
test_global_func5    - check that non-ctx pointer cannot be passed into
                      a function that expects context.
test_global_func6    - check that ctx pointer is unmodified.

Signed-off-by: Alexei Starovoitov <ast@kernel.org>

Acked-by: Song Liu <redacted>

quoted

---
.../bpf/prog_tests/test_global_funcs.c        | 81 +++++++++++++++++++
.../selftests/bpf/progs/test_global_func1.c   | 45 +++++++++++
.../selftests/bpf/progs/test_global_func2.c   |  4 +
.../selftests/bpf/progs/test_global_func3.c   | 65 +++++++++++++++
.../selftests/bpf/progs/test_global_func4.c   |  4 +
.../selftests/bpf/progs/test_global_func5.c   | 31 +++++++
.../selftests/bpf/progs/test_global_func6.c   | 31 +++++++
7 files changed, 261 insertions(+)
create mode 100644 tools/testing/selftests/bpf/prog_tests/test_global_funcs.c
create mode 100644 tools/testing/selftests/bpf/progs/test_global_func1.c
create mode 100644 tools/testing/selftests/bpf/progs/test_global_func2.c
create mode 100644 tools/testing/selftests/bpf/progs/test_global_func3.c
create mode 100644 tools/testing/selftests/bpf/progs/test_global_func4.c
create mode 100644 tools/testing/selftests/bpf/progs/test_global_func5.c
create mode 100644 tools/testing/selftests/bpf/progs/test_global_func6.c

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

@@ -0,0 +1,81 @@
+// SPDX-License-Identifier: GPL-2.0
+/* Copyright (c) 2020 Facebook */
+#include <test_progs.h>
+
+const char *err_str;
+bool found;
+
+static int libbpf_debug_print(enum libbpf_print_level level,
+                             const char *format, va_list args)
+{
+       char *log_buf;
+
+       if (level != LIBBPF_WARN ||
+           strcmp(format, "libbpf: \n%s\n")) {
+               vprintf(format, args);
+               return 0;
+       }
+
+       log_buf = va_arg(args, char *);
+       if (!log_buf)
+               goto out;
+       if (strstr(log_buf, err_str) == 0)
+               found = true;
+out:
+       printf(format, log_buf);
+       return 0;
+}

libbpf_debug_print() looks very useful. Maybe we can move it to some
header files?

I think it's hack that goes deep into libbpf internals that should be
discouraged. It's clearly very useful for selftests, but imo libbpf's log_buf
api should be redesigned instead. It's imo the worst part of the library.

Yeah, those global variables don't look good. 

Song