netdev - [PATCH net-next] net: phylink: Support disabling autonegotiation for PCS

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

Message-Id: <20210630174927.1077249-1-robert.hancock@calian.com>
Date:   Wed, 30 Jun 2021 11:49:27 -0600
From:   Robert Hancock <robert.hancock@...ian.com>
To:     linux@...linux.org.uk, andrew@...n.ch, hkallweit1@...il.com
Cc:     davem@...emloft.net, kuba@...nel.org, netdev@...r.kernel.org,
        Robert Hancock <robert.hancock@...ian.com>
Subject: [PATCH net-next] net: phylink: Support disabling autonegotiation for PCS

The auto-negotiation state in the PCS as set by
phylink_mii_c22_pcs_config was previously always enabled when the driver is
configured for in-band autonegotiation, even if autonegotiation was
disabled on the interface with ethtool. Update the code to set the
BMCR_ANENABLE bit based on the interface's autonegotiation enabled
state.

Update phylink_mii_c22_pcs_get_state to not check
autonegotiation-related fields when autonegotiation is disabled.

Update phylink_mac_pcs_get_state to initialize the state based on the
interface's configured speed, duplex and pause parameters rather than to
unknown when autonegotiation is disabled, before calling the driver's
pcs_get_state functions, as they are not likely to provide meaningful data
for these fields when autonegotiation is disabled. In this case the
driver is really just filling in the link state field.

Note that in cases where there is a downstream PHY connected, such as
with SGMII and a copper PHY, the configuration set by ethtool is handled by
phy_ethtool_ksettings_set and not propagated to the PCS. This is correct
since SGMII or 1000Base-X autonegotiation with the PCS should normally
still be used even if the copper side has disabled it.

Signed-off-by: Robert Hancock <robert.hancock@...ian.com>
---
 drivers/net/phy/phylink.c | 21 ++++++++++++++++-----
 1 file changed, 16 insertions(+), 5 deletions(-)

diff --git a/drivers/net/phy/phylink.c b/drivers/net/phy/phylink.c
index eb29ef53d971..4fc07d92f0c6 100644
--- a/drivers/net/phy/phylink.c
+++ b/drivers/net/phy/phylink.c
@@ -539,9 +539,15 @@ static void phylink_mac_pcs_get_state(struct phylink *pl,
 	linkmode_zero(state->lp_advertising);
 	state->interface = pl->link_config.interface;
 	state->an_enabled = pl->link_config.an_enabled;
-	state->speed = SPEED_UNKNOWN;
-	state->duplex = DUPLEX_UNKNOWN;
-	state->pause = MLO_PAUSE_NONE;
+	if  (state->an_enabled) {
+		state->speed = SPEED_UNKNOWN;
+		state->duplex = DUPLEX_UNKNOWN;
+		state->pause = MLO_PAUSE_NONE;
+	} else {
+		state->speed =  pl->link_config.speed;
+		state->duplex = pl->link_config.duplex;
+		state->pause = pl->link_config.pause;
+	}
 	state->an_complete = 0;
 	state->link = 1;

@@ -2422,7 +2428,10 @@ void phylink_mii_c22_pcs_get_state(struct mdio_device *pcs,

 	state->link = !!(bmsr & BMSR_LSTATUS);
 	state->an_complete = !!(bmsr & BMSR_ANEGCOMPLETE);
-	if (!state->link)
+	/* If there is no link or autonegotiation is disabled, the LP advertisement
+	 * data is not meaningful, so don't go any further.
+	 */
+	if (!state->link || !state->an_enabled)
 		return;

 	switch (state->interface) {
@@ -2545,7 +2554,9 @@ int phylink_mii_c22_pcs_config(struct mdio_device *pcs, unsigned int mode,
 	changed = ret > 0;

 	/* Ensure ISOLATE bit is disabled */
-	bmcr = mode == MLO_AN_INBAND ? BMCR_ANENABLE : 0;
+	bmcr = (mode == MLO_AN_INBAND &&
+		linkmode_test_bit(ETHTOOL_LINK_MODE_Autoneg_BIT, advertising)) ?
+		       BMCR_ANENABLE : 0;
 	ret = mdiobus_modify(pcs->bus, pcs->addr, MII_BMCR,
 			     BMCR_ANENABLE | BMCR_ISOLATE, bmcr);
 	if (ret < 0)
-- 
2.27.0