netdev - Re: [net-next 01/12] i40e: replace switch-statement to speed-up retpoline-enabled builds

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <14dd60c5-0fb7-1fc5-9bf6-4cb1510a1abc@iogearbox.net>
Date:   Mon, 6 May 2019 10:43:43 +0200
From:   Daniel Borkmann <daniel@...earbox.net>
To:     David Laight <David.Laight@...LAB.COM>,
        'Josh Elsasser' <jelsasser@...neta.com>,
        Jeff Kirsher <jeffrey.t.kirsher@...el.com>,
        Björn Töpel <bjorn.topel@...el.com>
Cc:     David Miller <davem@...emloft.net>,
        "netdev@...r.kernel.org" <netdev@...r.kernel.org>,
        "nhorman@...hat.com" <nhorman@...hat.com>,
        "sassmann@...hat.com" <sassmann@...hat.com>,
        Andrew Bowers <andrewx.bowers@...el.com>
Subject: Re: [net-next 01/12] i40e: replace switch-statement to speed-up
 retpoline-enabled builds

On 04/30/2019 12:42 PM, David Laight wrote:
> From: Josh Elsasser
>> Sent: 29 April 2019 21:02
>> On Apr 29, 2019, at 12:16 PM, Jeff Kirsher <jeffrey.t.kirsher@...el.com> wrote:
>>
>>> From: Björn Töpel <bjorn.topel@...el.com>
>>>
>>> GCC will generate jump tables for switch-statements with more than 5
>>> case statements. An entry into the jump table is an indirect call,
>>> which means that for CONFIG_RETPOLINE builds, this is rather
>>> expensive.
>>>
>>> This commit replaces the switch-statement that acts on the XDP program
>>> result with an if-clause.
>>
>> Apologies for the noise, but is this patch still required after the
>> recent threshold bump[0] and later removal[1] of switch-case jump
>> table generation when building with CONFIG_RETPOLINE?
>>
>> [0]: https://lore.kernel.org/patchwork/patch/1044863/
>> [1]: https://lore.kernel.org/patchwork/patch/1054472/
>>
>> If nothing else the commit message no longer seems accurate.
> 
> Looking at those two patches, the second one seems wrong:
> 
>    # Additionally, avoid generating expensive indirect jumps which
>    # are subject to retpolines for small number of switch cases.
>    # clang turns off jump table generation by default when under
> -  # retpoline builds, however, gcc does not for x86.
> -  KBUILD_CFLAGS += $(call cc-option,--param=case-values-threshold=20)
> +  # retpoline builds, however, gcc does not for x86. This has
> +  # only been fixed starting from gcc stable version 8.4.0 and
> +  # onwards, but not for older ones. See gcc bug #86952.
> +  ifndef CONFIG_CC_IS_CLANG
> +    KBUILD_CFLAGS += $(call cc-option,-fno-jump-tables)
> +  endif
> 
> If -fno-jump-tables isn't supported then --param=case-values-threshold=20
> needs to be set (if supported).

Nope, not really, -fno-jump-tables support predates the latter, and
both are supported for gcc versions the kernel cares about.