netdev - Fwd: Re: [PATCH net-next] net: phy: initialise phydev speed and duplex sanely

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]

Date:   Fri, 22 Nov 2019 16:09:29 +0000
From:   Russell King - ARM Linux admin <linux@...linux.org.uk>
To:     netdev@...r.kernel.org
Subject: Fwd: Re: [PATCH net-next] net: phy: initialise phydev speed and
 duplex sanely

Unfortunately, Andrew dropped the 'g' from the netdev email address,
and Zach's email address doesn't seem to work.

Forwarding this to netdev (with appropriate threading) for archival
purposes.

----- Forwarded message from Russell King - ARM Linux admin <linux@...linux.org.uk> -----

Date: Fri, 22 Nov 2019 16:03:23 +0000
From: Russell King - ARM Linux admin <linux@...linux.org.uk>
To: Andrew Lunn <andrew@...n.ch>
Cc: zach.brown@...com, Florian Fainelli <f.fainelli@...il.com>, Heiner Kallweit
	<hkallweit1@...il.com>, "David S. Miller" <davem@...emloft.net>,
	netdev@...r.kernel.or
Subject: Re: [PATCH net-next] net: phy: initialise phydev speed and duplex
	sanely

On Fri, Nov 22, 2019 at 04:02:01PM +0000, Russell King - ARM Linux admin wrote:
> On Fri, Nov 22, 2019 at 04:51:24PM +0100, Andrew Lunn wrote:
> > On Fri, Nov 22, 2019 at 03:23:23PM +0000, Russell King wrote:
> > > When a phydev is created, the speed and duplex are set to zero and
> > > -1 respectively, rather than using the predefined SPEED_UNKNOWN and
> > > DUPLEX_UNKNOWN constants.
> > > 
> > > There is a window at initialisation time where we may report link
> > > down using the 0/-1 values.  Tidy this up and use the predefined
> > > constants, so debug doesn't complain with:
> > > 
> > > "Unsupported (update phy-core.c)/Unsupported (update phy-core.c)"
> > > 
> > > when the speed and duplex settings are printed.
> > > 
> > > Signed-off-by: Russell King <rmk+kernel@...linux.org.uk>
> > > ---
> > >  drivers/net/phy/phy_device.c | 4 ++--
> > >  1 file changed, 2 insertions(+), 2 deletions(-)
> > > 
> > > diff --git a/drivers/net/phy/phy_device.c b/drivers/net/phy/phy_device.c
> > > index 232ad33b1159..8186aad4ef90 100644
> > > --- a/drivers/net/phy/phy_device.c
> > > +++ b/drivers/net/phy/phy_device.c
> > > @@ -597,8 +597,8 @@ struct phy_device *phy_device_create(struct mii_bus *bus, int addr, int phy_id,
> > >  	mdiodev->device_free = phy_mdio_device_free;
> > >  	mdiodev->device_remove = phy_mdio_device_remove;
> > >  
> > > -	dev->speed = 0;
> > > -	dev->duplex = -1;
> > > +	dev->speed = SPEED_UNKNOWN;
> > > +	dev->duplex = DUPLEX_UNKNOWN;
> > >  	dev->pause = 0;
> > >  	dev->asym_pause = 0;
> > >  	dev->link = 0;
> > 
> > Hi Russell, Zach
> > 
> > Does phy_led_trigger_change_speed() need to change? It has
> > 
> >         if (phy->speed == 0)
> >                 return;
> 
> I'm not sure what that's trying to achieve.
> 
> From what I can see, phy_speed_to_led_trigger() looks up the speed in
> the table of triggers, which is populated from the PHYs supported
> speeds, which will never contain zero or SPEED_UNKNOWN.
> 
> Note that genphy will return SPEED_UNKNOWN if autoneg_complete is
> false (see genphy_read_status()).  However, in that case, ->link
> will be false, just like it is at initialisation time, and hence
> we won't reach that statement (we'll go off to
> phy_led_trigger_no_link()).
> 
> So I think the test is entirely unnecessary.

... unless there's a buggy phylib driver, in which case we shouldn't
be working around it in this code (as it would affect network drivers
as well) but should be fixing the broken phylib driver.

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line in suburbia: sync at 12.1Mbps down 622kbps up
According to speedtest.net: 11.9Mbps down 500kbps up

----- End forwarded message -----

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line in suburbia: sync at 12.1Mbps down 622kbps up
According to speedtest.net: 11.9Mbps down 500kbps up