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] [thread-next>] [day] [month] [year] [list]

Date:   Tue, 30 Apr 2019 10:42:58 +0000
From:   David Laight <David.Laight@...LAB.COM>
To:     '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

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).

	David

-
Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK
Registration No: 1397386 (Wales)