| 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
| ||
|
Date: Tue, 18 Sep 2018 12:58:17 +0200
From: Simon Horman <horms@...ge.net.au>
To: Andrew Lunn <andrew@...n.ch>
Cc: David Miller <davem@...emloft.net>,
netdev <netdev@...r.kernel.org>,
Florian Fainelli <f.fainelli@...il.com>
Subject: Re: [PATCH v3 net-next 07/12] net: ethernet: Add helper to remove a
supported link mode
On Mon, Sep 17, 2018 at 05:38:11PM +0200, Andrew Lunn wrote:
> On Mon, Sep 17, 2018 at 05:13:07PM +0200, Simon Horman wrote:
> > On Wed, Sep 12, 2018 at 01:53:14AM +0200, Andrew Lunn wrote:
> > > Some MAC hardware cannot support a subset of link modes. e.g. often
> > > 1Gbps Full duplex is supported, but Half duplex is not. Add a helper
> > > to remove such a link mode.
> > >
> > > Signed-off-by: Andrew Lunn <andrew@...n.ch>
> > > Reviewed-by: Florian Fainelli <f.fainelli@...il.com>
> > > ---
> > > drivers/net/ethernet/apm/xgene/xgene_enet_hw.c | 6 +++---
> > > drivers/net/ethernet/cadence/macb_main.c | 5 ++---
> > > drivers/net/ethernet/freescale/fec_main.c | 3 ++-
> > > drivers/net/ethernet/microchip/lan743x_main.c | 2 +-
> > > drivers/net/ethernet/renesas/ravb_main.c | 3 ++-
> > > .../net/ethernet/stmicro/stmmac/stmmac_main.c | 12 ++++++++----
> > > drivers/net/phy/phy_device.c | 18 ++++++++++++++++++
> > > drivers/net/usb/lan78xx.c | 2 +-
> > > include/linux/phy.h | 1 +
> > > 9 files changed, 38 insertions(+), 14 deletions(-)
> > >
> > > diff --git a/drivers/net/ethernet/apm/xgene/xgene_enet_hw.c b/drivers/net/ethernet/apm/xgene/xgene_enet_hw.c
> > > index 078a04dc1182..4831f9de5945 100644
> >
> > ...
> >
> > > diff --git a/drivers/net/ethernet/renesas/ravb_main.c b/drivers/net/ethernet/renesas/ravb_main.c
> > > index aff5516b781e..fb2a1125780d 100644
> > > --- a/drivers/net/ethernet/renesas/ravb_main.c
> > > +++ b/drivers/net/ethernet/renesas/ravb_main.c
> > > @@ -1074,7 +1074,8 @@ static int ravb_phy_init(struct net_device *ndev)
> > > }
> > >
> > > /* 10BASE is not supported */
> > > - phydev->supported &= ~PHY_10BT_FEATURES;
> > > + phy_remove_link_mode(phydev, ETHTOOL_LINK_MODE_10baseT_Half_BIT);
> > > + phy_remove_link_mode(phydev, ETHTOOL_LINK_MODE_10baseT_Full_BIT);
> > >
> > > phy_attached_info(phydev);
> > >
> >
> > ...
> >
> > > diff --git a/drivers/net/phy/phy_device.c b/drivers/net/phy/phy_device.c
> > > index db1172db1e7c..e9ca83a438b0 100644
> > > --- a/drivers/net/phy/phy_device.c
> > > +++ b/drivers/net/phy/phy_device.c
> > > @@ -1765,6 +1765,24 @@ int phy_set_max_speed(struct phy_device *phydev, u32 max_speed)
> > > }
> > > EXPORT_SYMBOL(phy_set_max_speed);
> > >
> > > +/**
> > > + * phy_remove_link_mode - Remove a supported link mode
> > > + * @phydev: phy_device structure to remove link mode from
> > > + * @link_mode: Link mode to be removed
> > > + *
> > > + * Description: Some MACs don't support all link modes which the PHY
> > > + * does. e.g. a 1G MAC often does not support 1000Half. Add a helper
> > > + * to remove a link mode.
> > > + */
> > > +void phy_remove_link_mode(struct phy_device *phydev, u32 link_mode)
> > > +{
> > > + WARN_ON(link_mode > 31);
> > > +
> > > + phydev->supported &= ~BIT(link_mode);
> > > + phydev->advertising = phydev->supported;
> > > +}
> > > +EXPORT_SYMBOL(phy_remove_link_mode);
> > > +
> > > static void of_set_phy_supported(struct phy_device *phydev)
> > > {
> > > struct device_node *node = phydev->mdio.dev.of_node;
> >
> > Hi Andrew,
> >
> > I believe that for the RAVB the overall effect of this change is that
> > 10-BaseT modes are no longer advertised (although both with and without
> > this patch they are not supported).
> >
> > Unfortunately on R-Car Gen3 M3-W (r8a7796) based Salvator-X board
> > I have observed that this results in the link no longer being negotiated
> > on one switch (the one I usually use) while it seemed fine on another.
>
> Hi Simon
>
> Thanks for testing this.
>
> Could you dump the PHY registers with and without this patch:
>
> $ mii-tool -vv eth0
>
> Once difference is that phy_remove_link_mode() does
> phydev->advertising = phydev->supported where as the old code does
> not. I though phylib would do this anyway, it does at some point in
> time, but i didn't check when. It could be you are actually
> advertising 10, even if you don't support it.
Hi Andrew,
here are the results. I ran them with the device connected to the switch
which doesn't allow successful link negotiation when this patch is present.
1. net-next: cf7d97e1e54d ("net: mdio: remove duplicated include from mdio_bus.c")
# mii-tool -vv eth0
Using SIOCGMIIPHY=0x8947
eth0: no link
registers for MII PHY 0:
1140 7949 0022 1622 0d81 c1e1 000f 0000
0000 0300 0000 0000 0000 0000 0000 3000
0000 0000 0000 0078 7002 0000 0000 0200
0000 0000 0000 0528 0000 0000 0000 0000
product info: vendor 00:08:85, model 34 rev 2
basic mode: autonegotiation enabled
basic status: no link
capabilities: 1000baseT-HD 1000baseT-FD 100baseTx-FD 100baseTx-HD 10baseT-FD 10baseT-HD
advertising: 100baseTx-FD 100baseTx-HD flow-control
link partner: 100baseTx-FD 100baseTx-HD 10baseT-FD 10baseT-HD
2. net-next with this patch reverted
# mii-tool -vv eth0
Using SIOCGMIIPHY=0x8947
eth0: negotiated 100baseTx-FD, link ok
registers for MII PHY 0:
1140 796d 0022 1622 0181 c1e1 000f 0000
0000 0300 3800 0000 0000 0000 0000 3000
0000 0000 0000 0c7e 54fe 0000 0000 0200
0000 0000 0000 0500 0000 0000 0000 0000
product info: vendor 00:08:85, model 34 rev 2
basic mode: autonegotiation enabled
basic status: autonegotiation complete, link ok
capabilities: 1000baseT-HD 1000baseT-FD 100baseTx-FD 100baseTx-HD 10baseT-FD 10baseT-HD
advertising: 100baseTx-FD 100baseTx-HD
link partner: 1000baseT-FD 100baseTx-FD 100baseTx-HD 10baseT-FD 10baseT-HD
3. net-next with the following modification:
diff --git a/drivers/net/phy/phy_device.c b/drivers/net/phy/phy_device.c
index af64a9320fb0..f531b615d80b 100644
--- a/drivers/net/phy/phy_device.c
+++ b/drivers/net/phy/phy_device.c
@@ -1779,7 +1779,6 @@ void phy_remove_link_mode(struct phy_device *phydev, u32 link_mode)
WARN_ON(link_mode > 31);
phydev->supported &= ~BIT(link_mode);
- phydev->advertising = phydev->supported;
}
EXPORT_SYMBOL(phy_remove_link_mode);
# mii-tool -vv eth0
Using SIOCGMIIPHY=0x8947
eth0: negotiated 100baseTx-FD, link ok
registers for MII PHY 0:
1140 796d 0022 1622 0181 c1e1 000f 0000
0000 0300 3800 0000 0000 0000 0000 3000
0000 0000 0000 087e 44fe 0000 0000 0200
0000 0000 0000 0500 0000 0000 0000 0000
product info: vendor 00:08:85, model 34 rev 2
basic mode: autonegotiation enabled
basic status: autonegotiation complete, link ok
capabilities: 1000baseT-HD 1000baseT-FD 100baseTx-FD 100baseTx-HD 10baseT-FD 10baseT-HD
advertising: 100baseTx-FD 100baseTx-HD
link partner: 1000baseT-FD 100baseTx-FD 100baseTx-HD 10baseT-FD 10baseT-HD
Powered by blists - more mailing lists