Re: [PATCH v6 bpf-next 4/6] selftests/bpf: add bpf_snprintf_btf helper tests
From: Andrii Nakryiko <hidden>
Date: 2020-09-25 17:36:58
Also in:
bpf, linux-kselftest, lkml
On Thu, Sep 24, 2020 at 5:51 PM Alexei Starovoitov [off-list ref] wrote:
On Wed, Sep 23, 2020 at 06:46:26PM +0100, Alan Maguire wrote:quoted
+static int __strncmp(const void *m1, const void *m2, size_t len) +{ + const unsigned char *s1 = m1; + const unsigned char *s2 = m2; + int i, delta = 0; + +#pragma clang loop unroll(full)Shouldn't be needed? The verifier supports bounded loops.quoted
+ for (i = 0; i < len; i++) { + delta = s1[i] - s2[i]; + if (delta || s1[i] == 0 || s2[i] == 0) + break; + } + return delta; +} + +/* Use __builtin_btf_type_id to test snprintf_btf by type id instead of name */ +#if __has_builtin(__builtin_btf_type_id) +#define TEST_BTF_BY_ID(_str, _typestr, _ptr, _hflags) \ + do { \ + int _expected_ret = ret; \ + _ptr.type = 0; \ + _ptr.type_id = __builtin_btf_type_id(_typestr, 0); \The test is passing for me, but I don't understand why :) __builtin_btf_type_id(, 0); means btf_id of the bpf program. While bpf_snprintf_btf() is treating it as btf_id of vmlinux_btf. So it really should have been __builtin_btf_type_id(,1);
Better still to use bpf_core_type_id_kernel() macro from bpf_core_read.h.
quoted hunk ↗ jump to hunk
The following diff works:diff --git a/tools/testing/selftests/bpf/progs/netif_receive_skb.c b/tools/testing/selftests/bpf/progs/netif_receive_skb.c index b4f96f1f6830..bffa786e3b03 100644 --- a/tools/testing/selftests/bpf/progs/netif_receive_skb.c +++ b/tools/testing/selftests/bpf/progs/netif_receive_skb.c@@ -45,7 +45,7 @@ static int __strncmp(const void *m1, const void *m2, size_t len) do { \ int _expected_ret = ret; \ _ptr.type = 0; \ - _ptr.type_id = __builtin_btf_type_id(_typestr, 0); \ + _ptr.type_id = __builtin_btf_type_id(_typestr, 1); \ ret = bpf_snprintf_btf(_str, STRSIZE, &_ptr, \ sizeof(_ptr), _hflags); \ if (ret != _expected_ret) { \@@ -88,7 +88,7 @@ static int __strncmp(const void *m1, const void *m2, size_t len) ret = -EBADMSG; \ break; \ } \ - TEST_BTF_BY_ID(_str, #_type, _ptr, _hflags); \ + TEST_BTF_BY_ID(_str, _ptr, _ptr, _hflags); \But still makes me suspicious of the test. I haven't debugged further.