netdev - Re: [PATCH net] net: enetc: fix the netc-lib driver build dependency

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: <20250603204501.2lcszfoiy5svbw6s@skbuf>
Date: Tue, 3 Jun 2025 23:45:01 +0300
From: Vladimir Oltean <vladimir.oltean@....com>
To: Wei Fang <wei.fang@....com>
Cc: claudiu.manoil@....com, xiaoning.wang@....com, andrew+netdev@...n.ch,
	davem@...emloft.net, edumazet@...gle.com, kuba@...nel.org,
	pabeni@...hat.com, netdev@...r.kernel.org,
	linux-kernel@...r.kernel.org, imx@...ts.linux.dev, arnd@...nel.org
Subject: Re: [PATCH net] net: enetc: fix the netc-lib driver build dependency

On Tue, Jun 03, 2025 at 06:50:56PM +0800, Wei Fang wrote:
> The kernel robot reported the following errors when the netc-lib driver
> was compiled as a loadable module and the enetc-core driver was built-in.
> 
> ld.lld: error: undefined symbol: ntmp_init_cbdr
> referenced by enetc_cbdr.c:88 (drivers/net/ethernet/freescale/enetc/enetc_cbdr.c:88)
> ld.lld: error: undefined symbol: ntmp_free_cbdr
> referenced by enetc_cbdr.c:96 (drivers/net/ethernet/freescale/enetc/enetc_cbdr.c:96)
> 
> Simply changing "tristate" to "bool" can fix this issue, but take into
> account that the netc-lib driver needs to support being compiled as a
> loadable module. So we can solve this issue and support "tristate" by
> setting the default value.
> 
> Reported-by: Arnd Bergmann <arnd@...nel.org>
> Reported-by: kernel test robot <lkp@...el.com>
> Closes: https://lore.kernel.org/oe-kbuild-all/202505220734.x6TF6oHR-lkp@intel.com/
> Fixes: 4701073c3deb ("net: enetc: add initial netc-lib driver to support NTMP")
> Signed-off-by: Wei Fang <wei.fang@....com>
> ---
> Arnd Bergmann has posted a similar patch [1], but it has not been updated
> since the first version, perhaps he is busy with more important things.
> In order to fix the issue ASAP, I made this patch. And I added the
> Reported-by tag to give credit to Arnd Bergmann.
> [1] https://lore.kernel.org/imx/20250520161218.3581272-1-arnd@kernel.org/
> ---

Ok, so to summarize, you want nxp-netc-lib.ko to be separate from
fsl-enetc-core.ko, because when you upstream the switch driver (also a
consumer of ntmp.o), you want it to depend just on nxp-netc-lib.ko but
not on the full fsl-enetc-core.ko.

Does it practically matter, given the fact that the yet-to-be-upstreamed
switch is DSA, and needs the conduit interface driver to load anyway?

>  drivers/net/ethernet/freescale/enetc/Kconfig | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/drivers/net/ethernet/freescale/enetc/Kconfig b/drivers/net/ethernet/freescale/enetc/Kconfig
> index e917132d3714..06759bedb193 100644
> --- a/drivers/net/ethernet/freescale/enetc/Kconfig
> +++ b/drivers/net/ethernet/freescale/enetc/Kconfig
> @@ -17,6 +17,7 @@ config NXP_ENETC_PF_COMMON
>  
>  config NXP_NETC_LIB
>  	tristate
> +	default y if FSL_ENETC_CORE=y && NXP_ENETC4=m

So your logic here is: NXP_NETC_LIB has only one select/reverse dependency (NXP_ENETC4)
and FSL_ENETC_CORE has 3 (FSL_ENETC, NXP_ENETC4, FSL_ENETC_VF).

If the only reverse dependency of NXP_NETC_LIB, NXP_ENETC4, becomes m,
then NXP_NETC_LIB also becomes m, but in reality, FSL_ENETC_CORE, via
cbdr.o, still depends on symbols from NXP_NETC_LIB.

So you influence NXP_NETC_LIB to not become m when its only selecter is m,
instead stay y.

Won't this need to change, and become even more complicated when
NXP_NETC_LIB gains another selecter, the switch driver?

>  	help
>  	  This module provides common functionalities for both ENETC and NETC
>  	  Switch, such as NETC Table Management Protocol (NTMP) 2.0, common tc
> -- 
> 2.34.1
>

What about this interpretation? cbdr.o uses symbols from NXP_NETC_LIB,
so the Kconfig option controlling cbdr.o, aka FSL_ENETC_CORE, should
select NXP_NETC_LIB. This solves the problem in a way which is more
logical to me, and doesn't need to change when the switch is later added.

Then you can drop "select NXP_NETC_LIB" from NXP_ENETC4, because the
dependency will transfer transitively via FSL_ENETC_CORE.

diff --git a/drivers/net/ethernet/freescale/enetc/Kconfig b/drivers/net/ethernet/freescale/enetc/Kconfig
index 616ea22ceabc..ef31eea0fc50 100644
--- a/drivers/net/ethernet/freescale/enetc/Kconfig
+++ b/drivers/net/ethernet/freescale/enetc/Kconfig
@@ -1,6 +1,7 @@
 # SPDX-License-Identifier: GPL-2.0
 config FSL_ENETC_CORE
 	tristate
+	select NXP_NETC_LIB
 	help
 	  This module supports common functionality between the PF and VF
 	  drivers for the NXP ENETC controller.
@@ -47,7 +48,6 @@ config NXP_ENETC4
 	select FSL_ENETC_CORE
 	select FSL_ENETC_MDIO
 	select NXP_ENETC_PF_COMMON
-	select NXP_NETC_LIB
 	select PHYLINK
 	select DIMLIB
 	help