* Re: strace for m68k bpf_prog_info mismatch [not found] ` <8736lv92ls.fsf@tarshish> @ 2019-05-03 12:16 ` Geert Uytterhoeven 2019-05-03 12:16 ` Geert Uytterhoeven 2019-05-21 22:00 ` Dmitry V. Levin 0 siblings, 2 replies; 4+ messages in thread From: Geert Uytterhoeven @ 2019-05-03 12:16 UTC (permalink / raw) To: Baruch Siach Cc: Dmitry V . Levin, strace-devel, linux-m68k, Daniel Borkmann, Linux Kernel Mailing List, Linux-Arch Hi Baruch, On Fri, May 3, 2019 at 1:52 PM Baruch Siach <baruch@tkos.co.il> wrote: > On Fri, May 03 2019, Geert Uytterhoeven wrote: > > On Fri, May 3, 2019 at 6:06 AM Baruch Siach <baruch@tkos.co.il> 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... Gr{oetje,eeting}s, Geert -- Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org 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 ^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: strace for m68k bpf_prog_info mismatch 2019-05-03 12:16 ` strace for m68k bpf_prog_info mismatch Geert Uytterhoeven @ 2019-05-03 12:16 ` Geert Uytterhoeven 2019-05-21 22:00 ` Dmitry V. Levin 1 sibling, 0 replies; 4+ messages in thread From: Geert Uytterhoeven @ 2019-05-03 12:16 UTC (permalink / raw) To: Baruch Siach Cc: Dmitry V . Levin, strace-devel, linux-m68k, Daniel Borkmann, Linux Kernel Mailing List, Linux-Arch Hi Baruch, On Fri, May 3, 2019 at 1:52 PM Baruch Siach <baruch@tkos.co.il> wrote: > On Fri, May 03 2019, Geert Uytterhoeven wrote: > > On Fri, May 3, 2019 at 6:06 AM Baruch Siach <baruch@tkos.co.il> 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... Gr{oetje,eeting}s, Geert -- Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org 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 ^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: strace for m68k bpf_prog_info mismatch 2019-05-03 12:16 ` strace for m68k bpf_prog_info mismatch Geert Uytterhoeven 2019-05-03 12:16 ` Geert Uytterhoeven @ 2019-05-21 22:00 ` Dmitry V. Levin 2019-05-21 22:00 ` Dmitry V. Levin 1 sibling, 1 reply; 4+ messages in thread From: Dmitry V. Levin @ 2019-05-21 22:00 UTC (permalink / raw) To: Baruch Siach, Geert Uytterhoeven Cc: Linux-Arch, Daniel Borkmann, Linux Kernel Mailing List, linux-m68k, strace-devel [-- Attachment #1: Type: text/plain, Size: 4381 bytes --] 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@tkos.co.il> wrote: > > On Fri, May 03 2019, Geert Uytterhoeven wrote: > > > On Fri, May 3, 2019 at 6:06 AM Baruch Siach <baruch@tkos.co.il> 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@linux-m68k.org > > 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@lists.strace.io > https://lists.strace.io/mailman/listinfo/strace-devel -- ldv [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 801 bytes --] ^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: strace for m68k bpf_prog_info mismatch 2019-05-21 22:00 ` Dmitry V. Levin @ 2019-05-21 22:00 ` Dmitry V. Levin 0 siblings, 0 replies; 4+ messages in thread From: Dmitry V. Levin @ 2019-05-21 22:00 UTC (permalink / raw) To: Baruch Siach, Geert Uytterhoeven Cc: Linux-Arch, Daniel Borkmann, Linux Kernel Mailing List, linux-m68k, strace-devel [-- Attachment #1: Type: text/plain, Size: 4381 bytes --] 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@tkos.co.il> wrote: > > On Fri, May 03 2019, Geert Uytterhoeven wrote: > > > On Fri, May 3, 2019 at 6:06 AM Baruch Siach <baruch@tkos.co.il> 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@linux-m68k.org > > 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@lists.strace.io > https://lists.strace.io/mailman/listinfo/strace-devel -- ldv [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 801 bytes --] ^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2019-05-21 22:00 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
[not found] <874l6c89nd.fsf@tarshish>
[not found] ` <CAMuHMdUT3ug+SCzrnA2eD=QyOLaHUGAe-ZrbWfDUWxTJ4CWEtQ@mail.gmail.com>
[not found] ` <8736lv92ls.fsf@tarshish>
2019-05-03 12:16 ` strace for m68k bpf_prog_info mismatch Geert Uytterhoeven
2019-05-03 12:16 ` Geert Uytterhoeven
2019-05-21 22:00 ` Dmitry V. Levin
2019-05-21 22:00 ` Dmitry V. Levin
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox