netdev - Re: [PATCH net-next 1/2] net: phylink: explicitly configure in-band autoneg for PHYs that support it

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: <20210214103529.GT1463@shell.armlinux.org.uk>
Date:   Sun, 14 Feb 2021 10:35:29 +0000
From:   Russell King - ARM Linux admin <linux@...linux.org.uk>
To:     Vladimir Oltean <olteanv@...il.com>
Cc:     "David S. Miller" <davem@...emloft.net>,
        Jakub Kicinski <kuba@...nel.org>,
        Antoine Tenart <atenart@...nel.org>,
        Quentin Schulz <quentin.schulz@...tlin.com>,
        Michael Walle <michael@...le.cc>, netdev@...r.kernel.org,
        Heiner Kallweit <hkallweit1@...il.com>,
        Andrew Lunn <andrew@...n.ch>,
        Florian Fainelli <f.fainelli@...il.com>,
        Ioana Ciornei <ioana.ciornei@....com>,
        Maxim Kochetkov <fido_max@...ox.ru>,
        Bjarni Jonasson <bjarni.jonasson@...rochip.com>,
        Steen Hegelund <steen.hegelund@...rochip.com>,
        UNGLinuxDriver@...rochip.com
Subject: Re: [PATCH net-next 1/2] net: phylink: explicitly configure in-band
 autoneg for PHYs that support it

On Fri, Feb 12, 2021 at 07:23:40PM +0200, Vladimir Oltean wrote:
> +	ret = phy_config_inband_aneg(phy,
> +				     (pl->cur_link_an_mode == MLO_AN_INBAND));

Please use phylink_autoneg_inband(pl->cur_link_an_mode) here.

> +	if (ret && ret != -EOPNOTSUPP) {
> +		phylink_warn(pl, "failed to configure PHY in-band autoneg: %d\n",
> +			     ret);

Please use %pe and ERR_PTR(ret) so we can get a symbolic errno value.

As mentioned in this thread, we have at least one PHY which is unable
to provide the inband signalling in any mode (BCM84881). Currently,
phylink detects this PHY on a SFP (in phylink_phy_no_inband()) and
adjusts not to use inband mode. This would need to be addressed if we
are creating an alterative way to discover whether the PHY supports
inband mode or not.

Also, there needs to be consideration of PHYs that dynamically change
their interface type, and whether they support inband signalling.
For example, a PHY may support a mode where it dynamically selects
between 10GBASE-R, 5GBASE-R, 2500BASE-X and SGMII, where the SGMII
mode may have inband signalling enabled or disabled. This is not a
theoretical case; we have a PHY like that supported in the kernel and
boards use it. What would the semantics of your new call be for a PHY
that performs this?

Should we also have a phydev->inband tristate, taking values "unknown,
enabled, disabled" which the PHY driver is required to update in their
read_status callback if they dynamically change their interface type?
(Although then phylink will need to figure out how to deal with that.)

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