On Thu, Sep 24, 2020 at 5:51 PM Alexei Starovoitov
<alexei.starovoitov@xxxxxxxxx> wrote:
>
> On Wed, Sep 23, 2020 at 06:46:26PM +0100, Alan Maguire wrote:
> > +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.
>
> > + 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.
>
> 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.
