diff mbox series

[RFC,bpf-next,5/6] bpf: selftests: test_tunnel: Disable CO-RE relocations

Message ID 111a64c3e6ccda6b8a2826491715d4e8a645e384.1698431765.git.dxu@dxuuu.xyz (mailing list archive)
State New
Headers show
Series Add bpf_xdp_get_xfrm_state() kfunc | expand

Commit Message

Daniel Xu Oct. 27, 2023, 6:46 p.m. UTC
Switching to vmlinux.h definitions seems to make the verifier very
unhappy with bitfield accesses. The error is:

    ; md.u.md2.dir = direction;
    33: (69) r1 = *(u16 *)(r2 +11)
    misaligned stack access off (0x0; 0x0)+-64+11 size 2

It looks like disabling CO-RE relocations seem to make the error go
away.

Signed-off-by: Daniel Xu <dxu@dxuuu.xyz>
---
 tools/testing/selftests/bpf/progs/test_tunnel_kern.c | 1 +
 1 file changed, 1 insertion(+)

Comments

Andrii Nakryiko Oct. 27, 2023, 8:33 p.m. UTC | #1
On Fri, Oct 27, 2023 at 11:46 AM Daniel Xu <dxu@dxuuu.xyz> wrote:
>
> Switching to vmlinux.h definitions seems to make the verifier very
> unhappy with bitfield accesses. The error is:
>
>     ; md.u.md2.dir = direction;
>     33: (69) r1 = *(u16 *)(r2 +11)
>     misaligned stack access off (0x0; 0x0)+-64+11 size 2
>
> It looks like disabling CO-RE relocations seem to make the error go
> away.
>

for accessing bitfields libbpf provides
BPF_CORE_READ_BITFIELD_PROBED() and BPF_CORE_READ_BITFIELD() macros

> Signed-off-by: Daniel Xu <dxu@dxuuu.xyz>
> ---
>  tools/testing/selftests/bpf/progs/test_tunnel_kern.c | 1 +
>  1 file changed, 1 insertion(+)
>
> diff --git a/tools/testing/selftests/bpf/progs/test_tunnel_kern.c b/tools/testing/selftests/bpf/progs/test_tunnel_kern.c
> index 3065a716544d..ec7e04e012ae 100644
> --- a/tools/testing/selftests/bpf/progs/test_tunnel_kern.c
> +++ b/tools/testing/selftests/bpf/progs/test_tunnel_kern.c
> @@ -6,6 +6,7 @@
>   * modify it under the terms of version 2 of the GNU General Public
>   * License as published by the Free Software Foundation.
>   */
> +#define BPF_NO_PRESERVE_ACCESS_INDEX
>  #include "vmlinux.h"
>  #include <bpf/bpf_helpers.h>
>  #include <bpf/bpf_endian.h>
> --
> 2.42.0
>
Daniel Xu Oct. 29, 2023, 11:22 p.m. UTC | #2
On Fri, Oct 27, 2023 at 01:33:09PM -0700, Andrii Nakryiko wrote:
> On Fri, Oct 27, 2023 at 11:46 AM Daniel Xu <dxu@dxuuu.xyz> wrote:
> >
> > Switching to vmlinux.h definitions seems to make the verifier very
> > unhappy with bitfield accesses. The error is:
> >
> >     ; md.u.md2.dir = direction;
> >     33: (69) r1 = *(u16 *)(r2 +11)
> >     misaligned stack access off (0x0; 0x0)+-64+11 size 2
> >
> > It looks like disabling CO-RE relocations seem to make the error go
> > away.
> >
> 
> for accessing bitfields libbpf provides
> BPF_CORE_READ_BITFIELD_PROBED() and BPF_CORE_READ_BITFIELD() macros

In this case the code in question is:

        __u8 direction = 0;
        md.u.md2.dir = direction;

IOW the problem is assigning to bitfields, not reading from them.

Is that something that libbpf needs to support as well?

Thanks,
Daniel
Andrii Nakryiko Oct. 31, 2023, 6:25 a.m. UTC | #3
On Sun, Oct 29, 2023 at 4:22 PM Daniel Xu <dxu@dxuuu.xyz> wrote:
>
> On Fri, Oct 27, 2023 at 01:33:09PM -0700, Andrii Nakryiko wrote:
> > On Fri, Oct 27, 2023 at 11:46 AM Daniel Xu <dxu@dxuuu.xyz> wrote:
> > >
> > > Switching to vmlinux.h definitions seems to make the verifier very
> > > unhappy with bitfield accesses. The error is:
> > >
> > >     ; md.u.md2.dir = direction;
> > >     33: (69) r1 = *(u16 *)(r2 +11)
> > >     misaligned stack access off (0x0; 0x0)+-64+11 size 2
> > >
> > > It looks like disabling CO-RE relocations seem to make the error go
> > > away.
> > >
> >
> > for accessing bitfields libbpf provides
> > BPF_CORE_READ_BITFIELD_PROBED() and BPF_CORE_READ_BITFIELD() macros
>
> In this case the code in question is:
>
>         __u8 direction = 0;
>         md.u.md2.dir = direction;
>
> IOW the problem is assigning to bitfields, not reading from them.
>
> Is that something that libbpf needs to support as well?

Ah, I missed that this is a write into a struct. I think we can
support BPF_CORE_WRITE_BITFIELD() (not the PROBED version, though)
using all the same CO-RE relocations. It's probably a very niche case,
but BPF_CORE_READ_BITFIELD() is niche as well (though an absolute
necessity when the need does come up).

>
> Thanks,
> Daniel
diff mbox series

Patch

diff --git a/tools/testing/selftests/bpf/progs/test_tunnel_kern.c b/tools/testing/selftests/bpf/progs/test_tunnel_kern.c
index 3065a716544d..ec7e04e012ae 100644
--- a/tools/testing/selftests/bpf/progs/test_tunnel_kern.c
+++ b/tools/testing/selftests/bpf/progs/test_tunnel_kern.c
@@ -6,6 +6,7 @@ 
  * modify it under the terms of version 2 of the GNU General Public
  * License as published by the Free Software Foundation.
  */
+#define BPF_NO_PRESERVE_ACCESS_INDEX
 #include "vmlinux.h"
 #include <bpf/bpf_helpers.h>
 #include <bpf/bpf_endian.h>