netdev - Re: [PATCH net-next] net: pcs: xpcs: depends on PHYLINK in Kconfig

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: <YrMkEp6EWDvd3GT/@shell.armlinux.org.uk>
Date:   Wed, 22 Jun 2022 15:15:46 +0100
From:   "Russell King (Oracle)" <linux@...linux.org.uk>
To:     Jakub Kicinski <kuba@...nel.org>
Cc:     Paolo Abeni <pabeni@...hat.com>, netdev@...r.kernel.org,
        Andrew Lunn <andrew@...n.ch>,
        Heiner Kallweit <hkallweit1@...il.com>,
        "David S. Miller" <davem@...emloft.net>,
        Eric Dumazet <edumazet@...gle.com>,
        Ong Boon Leong <boon.leong.ong@...el.com>
Subject: Re: [PATCH net-next] net: pcs: xpcs: depends on PHYLINK in Kconfig

On Tue, Jun 21, 2022 at 12:50:45PM -0700, Jakub Kicinski wrote:
> On Tue, 21 Jun 2022 09:55:35 +0200 Paolo Abeni wrote:
> > This is another attempt at fixing:
> > 
> > >> ERROR: modpost: "phylink_mii_c22_pcs_encode_advertisement" [drivers/net/pcs/pcs_xpcs.ko] undefined!
> > >> ERROR: modpost: "phylink_mii_c22_pcs_decode_state" [drivers/net/pcs/pcs_xpcs.ko] undefined!  
> > 
> > We can't select PHYLINK, or that will trigger a circular dependency
> > PHYLINK already selects PHYLIB, which in turn selects MDIO_DEVICE:
> > replacing the MDIO_DEVICE dependency with PHYLINK will pull all the
> > required configs.
> 
> We can't use depends with PHYLINK, AFAIU, because PHYLINK is not 
> a user-visible knob. Its always "select"ed and does not show up
> in {x,n,menu}config.

I'm not sure I understand the point you're making. You seem to be
saying we can't use "depend on PHYLINK" for this PCS driver, but
then you sent a patch doing exactly that.

As these PCS drivers are only usable if PHYLINK is already enabled,
there is a clear dependency between them and phylink. The drivers
that make use of xpcs are:

stmmac, which selects both PCS_XPCS and PHYLINK.
sja1105 (dsa driver), which selects PCS_XPCS. All DSA drivers depend
on NET_DSA, and NET_DSA selects PHYLINK.

So, for PCS_XPCS, PHYLINK will be enabled whenever PCS_XPCS is
selected. No other drivers in drivers/net appear to make use of
the XPCS driver (I couldn't find any other references to
xpcs_create()) so using "depends on PHYLINK" for it should be safe.

Moreover, the user-visible nature of PCS_XPCS doesn't add anything
to the kernel - two drivers require PCS_XPCS due to code references
to the xpcs code, these two select that symbol. Offering it to the
user just gives the user an extra knob to twiddle with no useful
result (other than more files to be built.)

It could be argued that it helps compile coverage, which I think is
the only reason to make PCS_XPCS visible... but then we get compile
coverage when stmmac or sja1105 are enabled.

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 40Mbps down 10Mbps up. Decent connectivity at last!