include/linux/build_bug.h:78:41: error: static assertion failed: "struct bpf_fib_lookup size check"

include/linux/build_bug.h:78:41: error: static assertion failed: "struct bpf_fib_lookup size check"

[Naresh Kamboju]

The arm footbridge_defconfig failed with gcc-13 and gcc-8 on Linux next
starting from next-20240328..next-20240402.

arm:
  build:
    * gcc-8-footbridge_defconfig - Failed
    * gcc-13-footbridge_defconfig - Failed

Reported-by: Linux Kernel Functional Testing <lkft@linaro.org>

In file included from include/linux/bitfield.h:10,
                 from arch/arm/include/asm/ptrace.h:13,
                 from arch/arm/include/asm/processor.h:14,
                 from include/linux/prefetch.h:15,
                 from arch/arm/include/asm/atomic.h:12,
                 from include/linux/atomic.h:7,
                 from net/core/filter.c:20:
include/linux/build_bug.h:78:41: error: static assertion failed:
"struct bpf_fib_lookup size check"
   78 | #define __static_assert(expr, msg, ...) _Static_assert(expr, msg)
      |                                         ^~~~~~~~~~~~~~

Steps to reproduce:
------
# tuxmake --runtime podman --target-arch arm --toolchain gcc-13
--kconfig footbridge_defconfig

Links:
 - https://qa-reports.linaro.org/lkft/linux-next-master/build/next-20240402/testrun/23264362/suite/build/test/gcc-13-footbridge_defconfig/details/
 - https://storage.tuxsuite.com/public/linaro/lkft/builds/2eWtBPKv1yM8gfZJC8GkEkxN2j8/

--
Linaro LKFT
https://lkft.linaro.org

Re: include/linux/build_bug.h:78:41: error: static assertion failed: "struct bpf_fib_lookup size check"

[Anton Protopopov]

On Wed, Apr 3, 2024 at 10:03 AM Naresh Kamboju
<naresh.kamboju@linaro.org> wrote:
>
> The arm footbridge_defconfig failed with gcc-13 and gcc-8 on Linux next
> starting from next-20240328..next-20240402.
>
> arm:
>   build:
>     * gcc-8-footbridge_defconfig - Failed
>     * gcc-13-footbridge_defconfig - Failed
>
> Reported-by: Linux Kernel Functional Testing <lkft@linaro.org>
>
> In file included from include/linux/bitfield.h:10,
>                  from arch/arm/include/asm/ptrace.h:13,
>                  from arch/arm/include/asm/processor.h:14,
>                  from include/linux/prefetch.h:15,
>                  from arch/arm/include/asm/atomic.h:12,
>                  from include/linux/atomic.h:7,
>                  from net/core/filter.c:20:
> include/linux/build_bug.h:78:41: error: static assertion failed:
> "struct bpf_fib_lookup size check"
>    78 | #define __static_assert(expr, msg, ...) _Static_assert(expr, msg)
>       |                                         ^~~~~~~~~~~~~~

Thanks, I will take a look today

> Steps to reproduce:
> ------
> # tuxmake --runtime podman --target-arch arm --toolchain gcc-13
> --kconfig footbridge_defconfig
>
> Links:
>  - https://qa-reports.linaro.org/lkft/linux-next-master/build/next-20240402/testrun/23264362/suite/build/test/gcc-13-footbridge_defconfig/details/
>  - https://storage.tuxsuite.com/public/linaro/lkft/builds/2eWtBPKv1yM8gfZJC8GkEkxN2j8/
>
> --
> Linaro LKFT
> https://lkft.linaro.org

Re: include/linux/build_bug.h:78:41: error: static assertion failed: "struct bpf_fib_lookup size check"

[Arnd Bergmann]

On Wed, Apr 3, 2024, at 10:10, Anton Protopopov wrote:
> On Wed, Apr 3, 2024 at 10:03 AM Naresh Kamboju
> <naresh.kamboju@linaro.org> wrote:
>>
>> The arm footbridge_defconfig failed with gcc-13 and gcc-8 on Linux next
>> starting from next-20240328..next-20240402.
>>
>> arm:
>>   build:
>>     * gcc-8-footbridge_defconfig - Failed
>>     * gcc-13-footbridge_defconfig - Failed
>>
>> Reported-by: Linux Kernel Functional Testing <lkft@linaro.org>
>>
>> In file included from include/linux/bitfield.h:10,
>>                  from arch/arm/include/asm/ptrace.h:13,
>>                  from arch/arm/include/asm/processor.h:14,
>>                  from include/linux/prefetch.h:15,
>>                  from arch/arm/include/asm/atomic.h:12,
>>                  from include/linux/atomic.h:7,
>>                  from net/core/filter.c:20:
>> include/linux/build_bug.h:78:41: error: static assertion failed:
>> "struct bpf_fib_lookup size check"
>>    78 | #define __static_assert(expr, msg, ...) _Static_assert(expr, msg)
>>       |                                         ^~~~~~~~~~~~~~
>
> Thanks, I will take a look today

The problem is CONFIG_AEABI=n, which changes the alignment
of sub-word struct members. I had assumed that AEABI is enabled
by default for everything already, but it looks like footbridge
and a couple of other defconfigs still have it turned off:

$ git grep -l  CONFIG_ARCH_MULTI_V7.is.not arch/arm/configs/* | xargs git grep -L AEABI
arch/arm/configs/assabet_defconfig
arch/arm/configs/collie_defconfig
arch/arm/configs/footbridge_defconfig
arch/arm/configs/h3600_defconfig
arch/arm/configs/jornada720_defconfig
arch/arm/configs/neponset_defconfig
arch/arm/configs/netwinder_defconfig
arch/arm/configs/rpc_defconfig
arch/arm/configs/spear3xx_defconfig
arch/arm/configs/spear6xx_defconfig
arch/arm/configs/spitz_defconfig

Russell still has machines with an OABI toolchain, but I'm not
aware of anyone else relying on it. It does cause other
problems as well, so I already turned it off a long time ago
for my randconfig testing.

We should probably make it the default for everything, except
whichever defconfig Russell uses:

--- a/arch/arm/Kconfig
+++ b/arch/arm/Kconfig
@@ -1159,7 +1159,7 @@ config ARM_PATCH_IDIV
 config AEABI
        bool "Use the ARM EABI to compile the kernel" if !CPU_V7 && \
                !CPU_V7M && !CPU_V6 && !CPU_V6K && !CC_IS_CLANG && !COMPILE_TEST
-       default CPU_V7 || CPU_V7M || CPU_V6 || CPU_V6K || CC_IS_CLANG || COMPILE_TEST
+       default y
        help
          This option allows for the kernel to be compiled using the latest
          ARM ABI (aka EABI).  This is only useful if you are using a user

Or we could go one step further and make it 'depends on
EXPERT', short of removing it entirely.

     Arnd

Re: include/linux/build_bug.h:78:41: error: static assertion failed: "struct bpf_fib_lookup size check"

[Alexander Lobakin]

From: Arnd Bergmann <arnd@arndb.de>
Date: Wed, 03 Apr 2024 10:45:36 +0200

> On Wed, Apr 3, 2024, at 10:10, Anton Protopopov wrote:
>> On Wed, Apr 3, 2024 at 10:03 AM Naresh Kamboju
>> <naresh.kamboju@linaro.org> wrote:
>>>
>>> The arm footbridge_defconfig failed with gcc-13 and gcc-8 on Linux next
>>> starting from next-20240328..next-20240402.
>>>
>>> arm:
>>>   build:
>>>     * gcc-8-footbridge_defconfig - Failed
>>>     * gcc-13-footbridge_defconfig - Failed
>>>
>>> Reported-by: Linux Kernel Functional Testing <lkft@linaro.org>
>>>
>>> In file included from include/linux/bitfield.h:10,
>>>                  from arch/arm/include/asm/ptrace.h:13,
>>>                  from arch/arm/include/asm/processor.h:14,
>>>                  from include/linux/prefetch.h:15,
>>>                  from arch/arm/include/asm/atomic.h:12,
>>>                  from include/linux/atomic.h:7,
>>>                  from net/core/filter.c:20:
>>> include/linux/build_bug.h:78:41: error: static assertion failed:
>>> "struct bpf_fib_lookup size check"
>>>    78 | #define __static_assert(expr, msg, ...) _Static_assert(expr, msg)
>>>       |                                         ^~~~~~~~~~~~~~
>>
>> Thanks, I will take a look today

Naresh,

Could you please remove that static_assert() and dump bpf_bif_lookup
layout from pahole?

Anton unionized { smac, dmac } with __u32 mark. On x86_64, the offset of
smac was 52 (aligned to 4) already, so I don't really get what AEABI
does here. IIRC it aligns every structure to 8 bytes?

Maybe we could just add __attribute__((__packed__))
__attribute__((__aligned__(4))) to that anonymous union at the end.

> 
> The problem is CONFIG_AEABI=n, which changes the alignment
> of sub-word struct members. I had assumed that AEABI is enabled
> by default for everything already, but it looks like footbridge
> and a couple of other defconfigs still have it turned off:
> 
> $ git grep -l  CONFIG_ARCH_MULTI_V7.is.not arch/arm/configs/* | xargs git grep -L AEABI
> arch/arm/configs/assabet_defconfig
> arch/arm/configs/collie_defconfig
> arch/arm/configs/footbridge_defconfig
> arch/arm/configs/h3600_defconfig
> arch/arm/configs/jornada720_defconfig
> arch/arm/configs/neponset_defconfig
> arch/arm/configs/netwinder_defconfig
> arch/arm/configs/rpc_defconfig
> arch/arm/configs/spear3xx_defconfig
> arch/arm/configs/spear6xx_defconfig
> arch/arm/configs/spitz_defconfig
> 
> Russell still has machines with an OABI toolchain, but I'm not
> aware of anyone else relying on it. It does cause other
> problems as well, so I already turned it off a long time ago
> for my randconfig testing.
> 
> We should probably make it the default for everything, except
> whichever defconfig Russell uses:
> 
> --- a/arch/arm/Kconfig
> +++ b/arch/arm/Kconfig
> @@ -1159,7 +1159,7 @@ config ARM_PATCH_IDIV
>  config AEABI
>         bool "Use the ARM EABI to compile the kernel" if !CPU_V7 && \
>                 !CPU_V7M && !CPU_V6 && !CPU_V6K && !CC_IS_CLANG && !COMPILE_TEST
> -       default CPU_V7 || CPU_V7M || CPU_V6 || CPU_V6K || CC_IS_CLANG || COMPILE_TEST
> +       default y
>         help
>           This option allows for the kernel to be compiled using the latest
>           ARM ABI (aka EABI).  This is only useful if you are using a user
> 
> Or we could go one step further and make it 'depends on
> EXPERT', short of removing it entirely.>
>      Arnd

Thanks,
Olek

Re: include/linux/build_bug.h:78:41: error: static assertion failed: "struct bpf_fib_lookup size check"

[Anton Protopopov]

On Wed, Apr 3, 2024 at 11:39 AM Alexander Lobakin
<aleksander.lobakin@intel.com> wrote:
>
> From: Arnd Bergmann <arnd@arndb.de>
> Date: Wed, 03 Apr 2024 10:45:36 +0200
>
> > On Wed, Apr 3, 2024, at 10:10, Anton Protopopov wrote:
> >> On Wed, Apr 3, 2024 at 10:03 AM Naresh Kamboju
> >> <naresh.kamboju@linaro.org> wrote:
> >>>
> >>> The arm footbridge_defconfig failed with gcc-13 and gcc-8 on Linux next
> >>> starting from next-20240328..next-20240402.
> >>>
> >>> arm:
> >>>   build:
> >>>     * gcc-8-footbridge_defconfig - Failed
> >>>     * gcc-13-footbridge_defconfig - Failed
> >>>
> >>> Reported-by: Linux Kernel Functional Testing <lkft@linaro.org>
> >>>
> >>> In file included from include/linux/bitfield.h:10,
> >>>                  from arch/arm/include/asm/ptrace.h:13,
> >>>                  from arch/arm/include/asm/processor.h:14,
> >>>                  from include/linux/prefetch.h:15,
> >>>                  from arch/arm/include/asm/atomic.h:12,
> >>>                  from include/linux/atomic.h:7,
> >>>                  from net/core/filter.c:20:
> >>> include/linux/build_bug.h:78:41: error: static assertion failed:
> >>> "struct bpf_fib_lookup size check"
> >>>    78 | #define __static_assert(expr, msg, ...) _Static_assert(expr, msg)
> >>>       |                                         ^~~~~~~~~~~~~~
> >>
> >> Thanks, I will take a look today
>
> Naresh,
>
> Could you please remove that static_assert() and dump bpf_bif_lookup
> layout from pahole?
>
> Anton unionized { smac, dmac } with __u32 mark. On x86_64, the offset of
> smac was 52 (aligned to 4) already, so I don't really get what AEABI
> does here. IIRC it aligns every structure to 8 bytes?
>
> Maybe we could just add __attribute__((__packed__))
> __attribute__((__aligned__(4))) to that anonymous union at the end.

Yeah, I am sending a patch for this right now. Better not to depend on
compiler options

Re: include/linux/build_bug.h:78:41: error: static assertion failed: "struct bpf_fib_lookup size check"

[Anton Protopopov]

On Wed, Apr 3, 2024 at 11:57 AM Anton Protopopov <aspsk@isovalent.com> wrote:
>
> On Wed, Apr 3, 2024 at 11:39 AM Alexander Lobakin
> <aleksander.lobakin@intel.com> wrote:
> >
> > From: Arnd Bergmann <arnd@arndb.de>
> > Date: Wed, 03 Apr 2024 10:45:36 +0200
> >
> > > On Wed, Apr 3, 2024, at 10:10, Anton Protopopov wrote:
> > >> On Wed, Apr 3, 2024 at 10:03 AM Naresh Kamboju
> > >> <naresh.kamboju@linaro.org> wrote:
> > >>>
> > >>> The arm footbridge_defconfig failed with gcc-13 and gcc-8 on Linux next
> > >>> starting from next-20240328..next-20240402.
> > >>>
> > >>> arm:
> > >>>   build:
> > >>>     * gcc-8-footbridge_defconfig - Failed
> > >>>     * gcc-13-footbridge_defconfig - Failed
> > >>>
> > >>> Reported-by: Linux Kernel Functional Testing <lkft@linaro.org>
> > >>>
> > >>> In file included from include/linux/bitfield.h:10,
> > >>>                  from arch/arm/include/asm/ptrace.h:13,
> > >>>                  from arch/arm/include/asm/processor.h:14,
> > >>>                  from include/linux/prefetch.h:15,
> > >>>                  from arch/arm/include/asm/atomic.h:12,
> > >>>                  from include/linux/atomic.h:7,
> > >>>                  from net/core/filter.c:20:
> > >>> include/linux/build_bug.h:78:41: error: static assertion failed:
> > >>> "struct bpf_fib_lookup size check"
> > >>>    78 | #define __static_assert(expr, msg, ...) _Static_assert(expr, msg)
> > >>>       |                                         ^~~~~~~~~~~~~~
> > >>
> > >> Thanks, I will take a look today
> >
> > Naresh,
> >
> > Could you please remove that static_assert() and dump bpf_bif_lookup
> > layout from pahole?
> >
> > Anton unionized { smac, dmac } with __u32 mark. On x86_64, the offset of
> > smac was 52 (aligned to 4) already, so I don't really get what AEABI
> > does here. IIRC it aligns every structure to 8 bytes?
> >
> > Maybe we could just add __attribute__((__packed__))
> > __attribute__((__aligned__(4))) to that anonymous union at the end.
>
> Yeah, I am sending a patch for this right now. Better not to depend on
> compiler options

One __packed__ was not enough though. The problem was also with the
union of two __u16's which is padded to be 32 bits when AEABI=n and
the whole structure is packed (so total size is 66 in this case).

Re: include/linux/build_bug.h:78:41: error: static assertion failed: "struct bpf_fib_lookup size check"

[Alexander Lobakin]

From: Anton Protopopov <aspsk@isovalent.com>
Date: Wed, 3 Apr 2024 12:09:29 +0200

> On Wed, Apr 3, 2024 at 11:57 AM Anton Protopopov <aspsk@isovalent.com> wrote:
>>
>> On Wed, Apr 3, 2024 at 11:39 AM Alexander Lobakin
>> <aleksander.lobakin@intel.com> wrote:
>>>
>>> From: Arnd Bergmann <arnd@arndb.de>
>>> Date: Wed, 03 Apr 2024 10:45:36 +0200
>>>
>>>> On Wed, Apr 3, 2024, at 10:10, Anton Protopopov wrote:
>>>>> On Wed, Apr 3, 2024 at 10:03 AM Naresh Kamboju
>>>>> <naresh.kamboju@linaro.org> wrote:
>>>>>>
>>>>>> The arm footbridge_defconfig failed with gcc-13 and gcc-8 on Linux next
>>>>>> starting from next-20240328..next-20240402.
>>>>>>
>>>>>> arm:
>>>>>>   build:
>>>>>>     * gcc-8-footbridge_defconfig - Failed
>>>>>>     * gcc-13-footbridge_defconfig - Failed
>>>>>>
>>>>>> Reported-by: Linux Kernel Functional Testing <lkft@linaro.org>
>>>>>>
>>>>>> In file included from include/linux/bitfield.h:10,
>>>>>>                  from arch/arm/include/asm/ptrace.h:13,
>>>>>>                  from arch/arm/include/asm/processor.h:14,
>>>>>>                  from include/linux/prefetch.h:15,
>>>>>>                  from arch/arm/include/asm/atomic.h:12,
>>>>>>                  from include/linux/atomic.h:7,
>>>>>>                  from net/core/filter.c:20:
>>>>>> include/linux/build_bug.h:78:41: error: static assertion failed:
>>>>>> "struct bpf_fib_lookup size check"
>>>>>>    78 | #define __static_assert(expr, msg, ...) _Static_assert(expr, msg)
>>>>>>       |                                         ^~~~~~~~~~~~~~
>>>>>
>>>>> Thanks, I will take a look today
>>>
>>> Naresh,
>>>
>>> Could you please remove that static_assert() and dump bpf_bif_lookup
>>> layout from pahole?
>>>
>>> Anton unionized { smac, dmac } with __u32 mark. On x86_64, the offset of
>>> smac was 52 (aligned to 4) already, so I don't really get what AEABI
>>> does here. IIRC it aligns every structure to 8 bytes?
>>>
>>> Maybe we could just add __attribute__((__packed__))
>>> __attribute__((__aligned__(4))) to that anonymous union at the end.
>>
>> Yeah, I am sending a patch for this right now. Better not to depend on
>> compiler options
> 
> One __packed__ was not enough though. The problem was also with the
> union of two __u16's which is padded to be 32 bits when AEABI=n and
> the whole structure is packed (so total size is 66 in this case).

Hmm, on my setup it's 64 bytes. Since it's UAPI, it always must be of
the same size. There's probably a padding somewhere in the middle.

Also, don't forget to always set __aligned__(4 or 8) together with
__packed__, otherwise the compilers generate terrible code (they assume
the structure alignment is 1 and access to every field can be unaligned).

Thanks,
Olek

Re: include/linux/build_bug.h:78:41: error: static assertion failed: "struct bpf_fib_lookup size check"

[Arnd Bergmann]

On Wed, Apr 3, 2024, at 12:09, Anton Protopopov wrote:
> On Wed, Apr 3, 2024 at 11:57 AM Anton Protopopov <aspsk@isovalent.com> wrote:
 end.
>>
>> Yeah, I am sending a patch for this right now. Better not to depend on
>> compiler options
>
> One __packed__ was not enough though. The problem was also with the
> union of two __u16's which is padded to be 32 bits when AEABI=n and
> the whole structure is packed (so total size is 66 in this case).

The __packed attribute is easy to misunderstand, in this case you
would need to mark every internal union and struct as well, since
you otherwise run into one or both of these problems:

- an inner aggregate with explicit packing still requires
  32-bit alignment (and padding) for each member, even if the
  outer struct puts it at an unaligned position

- You get a compiler warning if an internal structure
  is follows the normal alignment constraints but is located
  at an unaligned offset, since that violates the alignment
  constraints of the C standard.

      Arnd