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: <20190423134136.GF2677@nanopsycho.orion> Date: Tue, 23 Apr 2019 15:41:36 +0200 From: Jiri Pirko <jiri@...nulli.us> To: Andrew Lunn <andrew@...n.ch> Cc: Petr Machata <petrm@...lanox.com>, Roopa Prabhu <roopa@...ulusnetworks.com>, "netdev@...r.kernel.org" <netdev@...r.kernel.org>, Jiri Pirko <jiri@...lanox.com>, Ido Schimmel <idosch@...lanox.com>, "davem@...emloft.net" <davem@...emloft.net>, Tariq Toukan <tariqt@...lanox.com>, "jakub.kicinski@...ronome.com" <jakub.kicinski@...ronome.com>, "stephen@...workplumber.org" <stephen@...workplumber.org> Subject: Re: [RFC PATCH net-next 1/3] net: rtnetlink: Add link-down reason to RTNL messages Thu, Mar 28, 2019 at 08:51:34PM CET, andrew@...n.ch wrote: >On Thu, Mar 28, 2019 at 05:59:50PM +0000, Petr Machata wrote: > >Hi Petr > >> - PHY driver: dev->phydev->drv->get_link_down_reason >> Certain PHY drivers might want to have a custom handling for some >> PHY-specific insight. Something like: >> >> modified include/linux/phy.h >> @@ -636,4 +636,7 @@ struct phy_driver { >> struct ethtool_tunable *tuna, >> const void *data); >> int (*set_loopback)(struct phy_device *dev, bool enable); >> + int (*get_link_down_reason)(struct phy_device *dev, >> + enum link_down_reason_major *major, >> + u32 *minor); >> }; >> >> Return -ENODATA to indicate there's nothing to report. >> >> - Generic PHY >> It would be possible to add a function that e.g. consults the PHY >> state machine to figure out what went wrong. Phy as such may have to >> be extended to keep track of e.g. why the state machine is PHY_HALTED, >> or possibly other problems worthy of reporting. > >I would suggest changing the order of these two. Put the generic PHY >first. If it sees the driver has a OP to get more specific detail, it >would make the call into the driver. You don't violate any layering >that way, and the locking is done correctly. > >The PHY being in state HALTED probably means there is a hardware error >at the MDIO level. That does not happen very often at all. > >What you are more interested is state PHY_NOLINK, which indicates >there is no link. Maybe because auto-neg didn't complete, if it was >turned on, or if auto-neg is off and the link is forced, the peer is >using something different, or is not there at all. > >> - phylink >> I'm not sure how to generically plug in the phylink library, because >> it is a private property of a MAC driver. There are currently only >> three drivers using phylink (mvvp2, mvneta, DSA), but it looks as if >> the intention is that phylink is used much more widely. So maybe it >> would make sense to have an NDO like get_phylink and use that to call >> into the phylink library for some generic handling. > >We need to consider adding a phylink pointer to the netdev structure >soon, in the same way there is a phylib structure. > >> Speaking specifically, my patch set would only do the first step above, >> because neither mlxsw nor mlx5 use the PHY subsystem. > >Shame. With just mlx, you will get a coverage of ~0. If you did the >generic phylib work, you might get coverage of > 50% of the MAC >drivers. It then becomes something useful and really used. Yeah, that's what it is. But maybe someone would pick up the work and do the phylib implementation too. Let's see. > > Andrew
Powered by blists - more mailing lists