Hi Baruch, Geert, Could you share these findings with bpf and netdev people, please? On Fri, May 03, 2019 at 02:16:04PM +0200, Geert Uytterhoeven wrote:
Hi Baruch, On Fri, May 3, 2019 at 1:52 PM Baruch Siach <baruch@xxxxxxxxxx> wrote:On Fri, May 03 2019, Geert Uytterhoeven wrote:On Fri, May 3, 2019 at 6:06 AM Baruch Siach <baruch@xxxxxxxxxx> wrote:strace 5.0 fails to build for m86k/5208 with the Buildroot generated toolchain: In file included from bpf_attr_check.c:6:0: static_assert.h:20:25: error: static assertion failed: "bpf_prog_info_struct.nr_jited_ksyms offset mismatch" # define static_assert _Static_assert ^ bpf_attr_check.c:913:2: note: in expansion of macro ‘static_assert’ static_assert(offsetof(struct bpf_prog_info_struct, nr_jited_ksyms) == offsetof(struct bpf_prog_info, nr_jited_ksyms), ^~~~~~~~~~~~~ The direct cause is a difference in the hole after the gpl_compatible field. Here is pahole output for the kernel struct (from v4.19): struct bpf_prog_info { ... __u32 ifindex; /* 80 4 */ __u32 gpl_compatible:1; /* 84: 0 4 */ /* XXX 15 bits hole, try to pack */ /* Bitfield combined with next fields */ __u64 netns_dev; /* 86 8 */I guess that should be "__aligned_u64 netns_dev;", to not rely on implicit alignment.Thanks. I can confirm that this minimal change fixes strace build: diff --git a/include/uapi/linux/bpf.h b/include/uapi/linux/bpf.h index 929c8e537a14..709d4dddc229 100644 --- a/include/uapi/linux/bpf.h +++ b/include/uapi/linux/bpf.h @@ -2869,7 +2869,7 @@ struct bpf_prog_info { char name[BPF_OBJ_NAME_LEN]; __u32 ifindex; __u32 gpl_compatible:1; - __u64 netns_dev; + __aligned_u64 netns_dev; __u64 netns_ino; __u32 nr_jited_ksyms; __u32 nr_jited_func_lens; Won't that break ABI compatibility for affected architectures?Yes it will. Or it may have been unusable without the fix. I don't know for sure.And this is for the strace struct: struct bpf_prog_info_struct { ... uint32_t ifindex; /* 80 4 */ uint32_t gpl_compatible:1; /* 84: 0 4 */ /* XXX 31 bits hole, try to pack */How come the uint64_t below is 8-byte aligned, not 2-byte aligned? Does strace use a special definition of uint64_t?I guess this is because of the netns_dev field definition in struct bpf_prog_info_struct at bpf_attr.h: struct bpf_prog_info_struct { ... uint32_t gpl_compatible:1; /* * The kernel UAPI is broken by Linux commit * v4.16-rc1~123^2~227^2~5^2~2 . */ uint64_t ATTRIBUTE_ALIGNED(8) netns_dev; /* skip check */Oh, the bug was even documented, with its cause ;-) That's commit 675fc275a3a2d905 ("bpf: offload: report device information for offloaded programs"). Partially fixed by commit 36f9814a494a874d ("bpf: fix uapi hole for 32 bit compat applications"), which left architectures with 16-bit alignment broken...
The offending commit seems to be the merge commit v4.18-rc1~114 that replaced "__u32 :32;" from the fix commit v4.17~4^2^2 with "__u32 gpl_compatible:1;" from earlier commit v4.18-rc1~114^2~376^2~6.
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@xxxxxxxxxxxxxx
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
--
Strace-devel mailing list
Strace-devel@xxxxxxxxxxxxxxx
https://lists.strace.io/mailman/listinfo/strace-devel
-- ldv
Attachment:
signature.asc
Description: PGP signature
