netdev - Re: [PATCH v3 net-next 04/16] sfc_ef100: skeleton EF100 PF driver

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]

Message-ID: <aa134db3-a860-534c-9ee2-d68cded37061@solarflare.com>
Date:   Tue, 21 Jul 2020 15:48:00 +0100
From:   Edward Cree <ecree@...arflare.com>
To:     <linux-net-drivers@...arflare.com>, <davem@...emloft.net>
CC:     kernel test robot <lkp@...el.com>, <kbuild-all@...ts.01.org>,
        <netdev@...r.kernel.org>
Subject: Re: [PATCH v3 net-next 04/16] sfc_ef100: skeleton EF100 PF driver

On 16/07/2020 18:39, kernel test robot wrote:
> [auto build test ERROR on net-next/master]
...
> config: mips-allyesconfig (attached as .config)
...
>    mips-linux-ld: drivers/net/ethernet/sfc/ef100_nic.o: in function `efx_mcdi_sensor_event':
>>> ef100_nic.c:(.text.efx_mcdi_sensor_event+0x0): multiple definition of `efx_mcdi_sensor_event'; drivers/net/ethernet/sfc/mcdi_mon.o:mcdi_mon.c:(.text.efx_mcdi_sensor_event+0x0): first defined here

Well, this holes us below the waterline.

When the sfc team was originally developing this driver, we
 didn't anticipate this problem (in fact we tested a build with
 both drivers 'y' and it apparently worked.  It doesn't now, I
 can reproduce this problem locally by just setting CONFIG_SFC=y
 CONFIG_SFC_EF100=y in my normal .config, no specific need for
 mips-allyesconfig).  So we leaned pretty heavily on the 'use
 the linker to select NIC-specific functions in calls from
 common code' trick.
Some of these can be replaced with function pointers in struct
 efx_nic_type (like this sensor-event handler above).  But:

>    mips-linux-ld: drivers/net/ethernet/sfc/ef100_rx.o: in function `__efx_rx_packet':
>>> ef100_rx.c:(.text.__efx_rx_packet+0x0): multiple definition of `__efx_rx_packet'; drivers/net/ethernet/sfc/rx.o:rx.c:(.text.__efx_rx_packet+0x0): first defined here
>    mips-linux-ld: drivers/net/ethernet/sfc/ef100_tx.o: in function `efx_enqueue_skb':
>>> ef100_tx.c:(.text.efx_enqueue_skb+0x0): multiple definition of `efx_enqueue_skb'; drivers/net/ethernet/sfc/tx.o:tx.c:(.text.efx_enqueue_skb+0x0): first defined here

These two functions are right on the data path, where we really
 don't want indirect calls and retpoline overhead.

I wondered if there were a way to deploy INDIRECT_CALLABLE, but
 I don't see how to make it deal with all the cases:
* both 'y': both symbols reachable from the common code, so a
  straightforward use of INDIRECT_CALLABLE to speed them up.
* both 'm': each time the common is linked, only the symbol
  from the current module is reachable.  The current link trick
  handles this.
* one 'y' and the other 'm': from the built-in link, only the
  y-module's symbol is reachable, but from the module, both are.
  (Also, I get a lot of "drivers/net/ethernet/sfc/efx_common.o:
  (__param+0x8): undefined reference to `__this_module'" and I
  don't really understand why.)
And while in principle this should be fixable with a lot of
 #if IS_REACHABLE() and #ifdef MODULE... the common code is only
 built once AIUI, which is why I had to move stuff like
 efx_driver_name in the first place!  We would need a different
 (e.g.) efx_common.o to link into each of a built-in and a
 modular driver.

Aaaaargh; does anyone have any bright ideas?

-ed