| 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
| ||
|
Message-ID: <YXg1F7cGOEjd2a+c@shell.armlinux.org.uk>
Date: Tue, 26 Oct 2021 18:04:23 +0100
From: "Russell King (Oracle)" <linux@...linux.org.uk>
To: Nicolas Ferre <nicolas.ferre@...rochip.com>
Cc: Sean Anderson <sean.anderson@...o.com>, netdev@...r.kernel.org,
"David S . Miller" <davem@...emloft.net>,
Jakub Kicinski <kuba@...nel.org>,
Claudiu Beznea <claudiu.beznea@...rochip.com>,
Antoine Tenart <atenart@...nel.org>
Subject: Re: [PATCH v4] net: macb: Fix several edge cases in validate
On Tue, Oct 26, 2021 at 06:37:18PM +0200, Nicolas Ferre wrote:
> I like it as well. Thanks a lot for these enhancements.
So, would this work - have I understood all these capability flags
correctly? To aid this process, I've included the resulting code
below as well as the diff.
Should we only be setting have_10g if _all_ of MACB_CAPS_HIGH_SPEED,
MACB_CAPS_PCS, and MACB_CAPS_GIGABIT_MODE_AVAILABLE are set and we
have gem, or is what I have below sufficient (does it matter if
MACB_CAPS_PCS is clear?)
static void macb_validate(struct phylink_config *config,
unsigned long *supported,
struct phylink_link_state *state)
{
struct net_device *ndev = to_net_dev(config->dev);
__ETHTOOL_DECLARE_LINK_MODE_MASK(mask) = { 0, };
bool have_1g = false, have_10g = false;
struct macb *bp = netdev_priv(ndev);
if (macb_is_gem(bp) &&
(bp->caps & MACB_CAPS_GIGABIT_MODE_AVAILABLE)) {
if (bp->caps & MACB_CAPS_PCS)
have_1g = true;
if (bp->caps & MACB_CAPS_HIGH_SPEED)
have_10g = true;
}
switch (state->interface) {
case PHY_INTERFACE_MODE_NA:
case PHY_INTERFACE_MODE_MII:
case PHY_INTERFACE_MODE_RMII:
break;
case PHY_INTERFACE_MODE_GMII:
case PHY_INTERFACE_MODE_RGMII:
case PHY_INTERFACE_MODE_RGMII_ID:
case PHY_INTERFACE_MODE_RGMII_RXID:
case PHY_INTERFACE_MODE_RGMII_TXID:
case PHY_INTERFACE_MODE_SGMII:
if (!have_1g) {
linkmode_zero(supported);
return;
}
break;
case PHY_INTERFACE_MODE_10GBASER:
if (!have_10g) {
linkmode_zero(supported);
return;
}
break;
default:
linkmode_zero(supported);
return;
}
phylink_set_port_modes(mask);
phylink_set(mask, Autoneg);
phylink_set(mask, Asym_Pause);
switch (state->interface) {
case PHY_INTERFACE_MODE_NA:
case PHY_INTERFACE_MODE_10GBASER:
if (have_10g) {
phylink_set_10g_modes(mask);
phylink_set(mask, 10000baseKR_Full);
}
if (state->interface != PHY_INTERFACE_MODE_NA)
break;
fallthrough;
case PHY_INTERFACE_MODE_GMII:
case PHY_INTERFACE_MODE_RGMII:
case PHY_INTERFACE_MODE_RGMII_ID:
case PHY_INTERFACE_MODE_RGMII_RXID:
case PHY_INTERFACE_MODE_RGMII_TXID:
case PHY_INTERFACE_MODE_SGMII:
if (have_1g) {
phylink_set(mask, 1000baseT_Full);
phylink_set(mask, 1000baseX_Full);
if (!(bp->caps & MACB_CAPS_NO_GIGABIT_HALF))
phylink_set(mask, 1000baseT_Half);
}
fallthrough;
default:
phylink_set(mask, 10baseT_Half);
phylink_set(mask, 10baseT_Full);
phylink_set(mask, 100baseT_Half);
phylink_set(mask, 100baseT_Full);
break;
}
linkmode_and(supported, supported, mask);
linkmode_and(state->advertising, state->advertising, mask);
}
diff --git a/drivers/net/ethernet/cadence/macb_main.c b/drivers/net/ethernet/cadence/macb_main.c
index 309371abfe23..36fcd5563a71 100644
--- a/drivers/net/ethernet/cadence/macb_main.c
+++ b/drivers/net/ethernet/cadence/macb_main.c
@@ -512,30 +512,43 @@ static void macb_validate(struct phylink_config *config,
{
struct net_device *ndev = to_net_dev(config->dev);
__ETHTOOL_DECLARE_LINK_MODE_MASK(mask) = { 0, };
+ bool have_1g = false, have_10g = false;
struct macb *bp = netdev_priv(ndev);
- /* We only support MII, RMII, GMII, RGMII & SGMII. */
- if (state->interface != PHY_INTERFACE_MODE_NA &&
- state->interface != PHY_INTERFACE_MODE_MII &&
- state->interface != PHY_INTERFACE_MODE_RMII &&
- state->interface != PHY_INTERFACE_MODE_GMII &&
- state->interface != PHY_INTERFACE_MODE_SGMII &&
- state->interface != PHY_INTERFACE_MODE_10GBASER &&
- !phy_interface_mode_is_rgmii(state->interface)) {
- linkmode_zero(supported);
- return;
+ if (macb_is_gem(bp) &&
+ (bp->caps & MACB_CAPS_GIGABIT_MODE_AVAILABLE)) {
+ if (bp->caps & MACB_CAPS_PCS)
+ have_1g = true;
+ if (bp->caps & MACB_CAPS_HIGH_SPEED)
+ have_10g = true;
}
- if (!macb_is_gem(bp) &&
- (state->interface == PHY_INTERFACE_MODE_GMII ||
- phy_interface_mode_is_rgmii(state->interface))) {
- linkmode_zero(supported);
- return;
- }
+ switch (state->interface) {
+ case PHY_INTERFACE_MODE_NA:
+ case PHY_INTERFACE_MODE_MII:
+ case PHY_INTERFACE_MODE_RMII:
+ break;
+
+ case PHY_INTERFACE_MODE_GMII:
+ case PHY_INTERFACE_MODE_RGMII:
+ case PHY_INTERFACE_MODE_RGMII_ID:
+ case PHY_INTERFACE_MODE_RGMII_RXID:
+ case PHY_INTERFACE_MODE_RGMII_TXID:
+ case PHY_INTERFACE_MODE_SGMII:
+ if (!have_1g) {
+ linkmode_zero(supported);
+ return;
+ }
+ break;
+
+ case PHY_INTERFACE_MODE_10GBASER:
+ if (!have_10g) {
+ linkmode_zero(supported);
+ return;
+ }
+ break;
- if (state->interface == PHY_INTERFACE_MODE_10GBASER &&
- !(bp->caps & MACB_CAPS_HIGH_SPEED &&
- bp->caps & MACB_CAPS_PCS)) {
+ default:
linkmode_zero(supported);
return;
}
@@ -544,32 +557,40 @@ static void macb_validate(struct phylink_config *config,
phylink_set(mask, Autoneg);
phylink_set(mask, Asym_Pause);
- if (bp->caps & MACB_CAPS_GIGABIT_MODE_AVAILABLE &&
- (state->interface == PHY_INTERFACE_MODE_NA ||
- state->interface == PHY_INTERFACE_MODE_10GBASER)) {
- phylink_set_10g_modes(mask);
- phylink_set(mask, 10000baseKR_Full);
+ switch (state->interface) {
+ case PHY_INTERFACE_MODE_NA:
+ case PHY_INTERFACE_MODE_10GBASER:
+ if (have_10g) {
+ phylink_set_10g_modes(mask);
+ phylink_set(mask, 10000baseKR_Full);
+ }
if (state->interface != PHY_INTERFACE_MODE_NA)
- goto out;
- }
+ break;
+ fallthrough;
+
+ case PHY_INTERFACE_MODE_GMII:
+ case PHY_INTERFACE_MODE_RGMII:
+ case PHY_INTERFACE_MODE_RGMII_ID:
+ case PHY_INTERFACE_MODE_RGMII_RXID:
+ case PHY_INTERFACE_MODE_RGMII_TXID:
+ case PHY_INTERFACE_MODE_SGMII:
+ if (have_1g) {
+ phylink_set(mask, 1000baseT_Full);
+ phylink_set(mask, 1000baseX_Full);
+
+ if (!(bp->caps & MACB_CAPS_NO_GIGABIT_HALF))
+ phylink_set(mask, 1000baseT_Half);
+ }
+ fallthrough;
- phylink_set(mask, 10baseT_Half);
- phylink_set(mask, 10baseT_Full);
- phylink_set(mask, 100baseT_Half);
- phylink_set(mask, 100baseT_Full);
-
- if (bp->caps & MACB_CAPS_GIGABIT_MODE_AVAILABLE &&
- (state->interface == PHY_INTERFACE_MODE_NA ||
- state->interface == PHY_INTERFACE_MODE_GMII ||
- state->interface == PHY_INTERFACE_MODE_SGMII ||
- phy_interface_mode_is_rgmii(state->interface))) {
- phylink_set(mask, 1000baseT_Full);
- phylink_set(mask, 1000baseX_Full);
-
- if (!(bp->caps & MACB_CAPS_NO_GIGABIT_HALF))
- phylink_set(mask, 1000baseT_Half);
+ default:
+ phylink_set(mask, 10baseT_Half);
+ phylink_set(mask, 10baseT_Full);
+ phylink_set(mask, 100baseT_Half);
+ phylink_set(mask, 100baseT_Full);
+ break;
}
-out:
+
linkmode_and(supported, supported, mask);
linkmode_and(state->advertising, state->advertising, mask);
}
--
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 40Mbps down 10Mbps up. Decent connectivity at last!
Powered by blists - more mailing lists