| Message ID | 20210324022211.1718762-7-revest@chromium.org (mailing list archive) |
|---|---|
| State | Changes Requested |
| Delegated to: | BPF |
| Headers | show |
| Series | Add a snprintf eBPF helper | expand |
On Tue, Mar 23, 2021 at 7:23 PM Florent Revest <revest@chromium.org> wrote: > > This exercises most of the format specifiers when things go well. > > Signed-off-by: Florent Revest <revest@chromium.org> > --- Looks good. Please add a no-argument test case as well. Acked-by: Andrii Nakryiko <andrii@kernel.org> > .../selftests/bpf/prog_tests/snprintf.c | 65 +++++++++++++++++++ > .../selftests/bpf/progs/test_snprintf.c | 59 +++++++++++++++++ > 2 files changed, 124 insertions(+) > create mode 100644 tools/testing/selftests/bpf/prog_tests/snprintf.c > create mode 100644 tools/testing/selftests/bpf/progs/test_snprintf.c > [...] > + > +SEC("raw_tp/sys_enter") > +int handler(const void *ctx) > +{ > + /* Convenient values to pretty-print */ > + const __u8 ex_ipv4[] = {127, 0, 0, 1}; > + const __u8 ex_ipv6[] = {0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 1}; > + const char str1[] = "str1"; > + const char longstr[] = "longstr"; > + extern const void schedule __ksym; oh, fancy. I'd move it out of this function into global space, though, to make it more apparent. I almost missed that it's a special one. > + > + /* Integer types */ > + num_ret = BPF_SNPRINTF(num_out, sizeof(num_out), > + "%d %u %x %li %llu %lX", > + -8, 9, 150, -424242, 1337, 0xDABBAD00); > + /* IP addresses */ > + ip_ret = BPF_SNPRINTF(ip_out, sizeof(ip_out), "%pi4 %pI6", > + &ex_ipv4, &ex_ipv6); > + /* Symbol lookup formatting */ > + sym_ret = BPF_SNPRINTF(sym_out, sizeof(sym_out), "%ps %pS %pB", > + &schedule, &schedule, &schedule); > + /* Kernel pointers */ > + addr_ret = BPF_SNPRINTF(addr_out, sizeof(addr_out), "%pK %px %p", > + 0, 0xFFFF00000ADD4E55, 0xFFFF00000ADD4E55); > + /* Strings embedding */ > + str_ret = BPF_SNPRINTF(str_out, sizeof(str_out), "%s %+05s", > + str1, longstr); > + /* Overflow */ > + over_ret = BPF_SNPRINTF(over_out, sizeof(over_out), "%%overflow"); > + > + return 0; > +} > + > +char _license[] SEC("license") = "GPL"; > -- > 2.31.0.291.g576ba9dcdaf-goog >
On Sat, Mar 27, 2021 at 12:05 AM Andrii Nakryiko <andrii.nakryiko@gmail.com> wrote: > > On Tue, Mar 23, 2021 at 7:23 PM Florent Revest <revest@chromium.org> wrote: > > > > This exercises most of the format specifiers when things go well. > > > > Signed-off-by: Florent Revest <revest@chromium.org> > > --- > > Looks good. Please add a no-argument test case as well. Agreed > Acked-by: Andrii Nakryiko <andrii@kernel.org> > > > .../selftests/bpf/prog_tests/snprintf.c | 65 +++++++++++++++++++ > > .../selftests/bpf/progs/test_snprintf.c | 59 +++++++++++++++++ > > 2 files changed, 124 insertions(+) > > create mode 100644 tools/testing/selftests/bpf/prog_tests/snprintf.c > > create mode 100644 tools/testing/selftests/bpf/progs/test_snprintf.c > > > > [...] > > > + > > +SEC("raw_tp/sys_enter") > > +int handler(const void *ctx) > > +{ > > + /* Convenient values to pretty-print */ > > + const __u8 ex_ipv4[] = {127, 0, 0, 1}; > > + const __u8 ex_ipv6[] = {0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 1}; > > + const char str1[] = "str1"; > > + const char longstr[] = "longstr"; > > + extern const void schedule __ksym; > > oh, fancy. I'd move it out of this function into global space, though, > to make it more apparent. I almost missed that it's a special one. Just schedule? Alright.
diff --git a/tools/testing/selftests/bpf/prog_tests/snprintf.c b/tools/testing/selftests/bpf/prog_tests/snprintf.c new file mode 100644 index 000000000000..948a05e6b2cb --- /dev/null +++ b/tools/testing/selftests/bpf/prog_tests/snprintf.c @@ -0,0 +1,65 @@ +// SPDX-License-Identifier: GPL-2.0 +/* Copyright (c) 2021 Google LLC. */ + +#include <test_progs.h> +#include "test_snprintf.skel.h" + +#define EXP_NUM_OUT "-8 9 96 -424242 1337 DABBAD00" +#define EXP_NUM_RET sizeof(EXP_NUM_OUT) + +#define EXP_IP_OUT "127.000.000.001 0000:0000:0000:0000:0000:0000:0000:0001" +#define EXP_IP_RET sizeof(EXP_IP_OUT) + +/* The third specifier, %pB, depends on compiler inlining so don't check it */ +#define EXP_SYM_OUT "schedule schedule+0x0/" +#define MIN_SYM_RET sizeof(EXP_SYM_OUT) + +/* The third specifier, %p, is a hashed pointer which changes on every reboot */ +#define EXP_ADDR_OUT "0000000000000000 ffff00000add4e55 " +#define EXP_ADDR_RET sizeof(EXP_ADDR_OUT "unknownhashedptr") + +#define EXP_STR_OUT "str1 longstr" +#define EXP_STR_RET sizeof(EXP_STR_OUT) + +#define EXP_OVER_OUT "%over" +#define EXP_OVER_RET 10 + +void test_snprintf(void) +{ + char exp_addr_out[] = EXP_ADDR_OUT; + char exp_sym_out[] = EXP_SYM_OUT; + struct test_snprintf *skel; + + skel = test_snprintf__open_and_load(); + if (!ASSERT_OK_PTR(skel, "skel_open")) + return; + + if (!ASSERT_OK(test_snprintf__attach(skel), "skel_attach")) + goto cleanup; + + /* trigger tracepoint */ + usleep(1); + + ASSERT_STREQ(skel->bss->num_out, EXP_NUM_OUT, "num_out"); + ASSERT_EQ(skel->bss->num_ret, EXP_NUM_RET, "num_ret"); + + ASSERT_STREQ(skel->bss->ip_out, EXP_IP_OUT, "ip_out"); + ASSERT_EQ(skel->bss->ip_ret, EXP_IP_RET, "ip_ret"); + + ASSERT_OK(memcmp(skel->bss->sym_out, exp_sym_out, + sizeof(exp_sym_out) - 1), "sym_out"); + ASSERT_LT(MIN_SYM_RET, skel->bss->sym_ret, "sym_ret"); + + ASSERT_OK(memcmp(skel->bss->addr_out, exp_addr_out, + sizeof(exp_addr_out) - 1), "addr_out"); + ASSERT_EQ(skel->bss->addr_ret, EXP_ADDR_RET, "addr_ret"); + + ASSERT_STREQ(skel->bss->str_out, EXP_STR_OUT, "str_out"); + ASSERT_EQ(skel->bss->str_ret, EXP_STR_RET, "str_ret"); + + ASSERT_STREQ(skel->bss->over_out, EXP_OVER_OUT, "over_out"); + ASSERT_EQ(skel->bss->over_ret, EXP_OVER_RET, "over_ret"); + +cleanup: + test_snprintf__destroy(skel); +} diff --git a/tools/testing/selftests/bpf/progs/test_snprintf.c b/tools/testing/selftests/bpf/progs/test_snprintf.c new file mode 100644 index 000000000000..e18709055fad --- /dev/null +++ b/tools/testing/selftests/bpf/progs/test_snprintf.c @@ -0,0 +1,59 @@ +// SPDX-License-Identifier: GPL-2.0 +/* Copyright (c) 2021 Google LLC. */ + +#include <linux/bpf.h> +#include <bpf/bpf_endian.h> +#include <bpf/bpf_helpers.h> +#include <bpf/bpf_tracing.h> + +char num_out[64] = {}; +long num_ret = 0; + +char ip_out[64] = {}; +long ip_ret = 0; + +char sym_out[64] = {}; +long sym_ret = 0; + +char addr_out[64] = {}; +long addr_ret = 0; + +char str_out[64] = {}; +long str_ret = 0; + +char over_out[6] = {}; +long over_ret = 0; + +SEC("raw_tp/sys_enter") +int handler(const void *ctx) +{ + /* Convenient values to pretty-print */ + const __u8 ex_ipv4[] = {127, 0, 0, 1}; + const __u8 ex_ipv6[] = {0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 1}; + const char str1[] = "str1"; + const char longstr[] = "longstr"; + extern const void schedule __ksym; + + /* Integer types */ + num_ret = BPF_SNPRINTF(num_out, sizeof(num_out), + "%d %u %x %li %llu %lX", + -8, 9, 150, -424242, 1337, 0xDABBAD00); + /* IP addresses */ + ip_ret = BPF_SNPRINTF(ip_out, sizeof(ip_out), "%pi4 %pI6", + &ex_ipv4, &ex_ipv6); + /* Symbol lookup formatting */ + sym_ret = BPF_SNPRINTF(sym_out, sizeof(sym_out), "%ps %pS %pB", + &schedule, &schedule, &schedule); + /* Kernel pointers */ + addr_ret = BPF_SNPRINTF(addr_out, sizeof(addr_out), "%pK %px %p", + 0, 0xFFFF00000ADD4E55, 0xFFFF00000ADD4E55); + /* Strings embedding */ + str_ret = BPF_SNPRINTF(str_out, sizeof(str_out), "%s %+05s", + str1, longstr); + /* Overflow */ + over_ret = BPF_SNPRINTF(over_out, sizeof(over_out), "%%overflow"); + + return 0; +} + +char _license[] SEC("license") = "GPL";
This exercises most of the format specifiers when things go well. Signed-off-by: Florent Revest <revest@chromium.org> --- .../selftests/bpf/prog_tests/snprintf.c | 65 +++++++++++++++++++ .../selftests/bpf/progs/test_snprintf.c | 59 +++++++++++++++++ 2 files changed, 124 insertions(+) create mode 100644 tools/testing/selftests/bpf/prog_tests/snprintf.c create mode 100644 tools/testing/selftests/bpf/progs/test_snprintf.c