linux-kernel - Re: [PATCH net-next] net: phy: at803x: remove at803x_aneg

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]

Message-ID: <20210318151712.7hmdaufxylyl33em@skbuf>
Date:   Thu, 18 Mar 2021 17:17:12 +0200
From:   Vladimir Oltean <olteanv@...il.com>
To:     Heiner Kallweit <hkallweit1@...il.com>
Cc:     Michael Walle <michael@...le.cc>, netdev@...r.kernel.org,
        linux-kernel@...r.kernel.org, Andrew Lunn <andrew@...n.ch>,
        Russell King <linux@...linux.org.uk>,
        "David S . Miller" <davem@...emloft.net>,
        Jakub Kicinski <kuba@...nel.org>
Subject: Re: [PATCH net-next] net: phy: at803x: remove at803x_aneg_done()

On Thu, Mar 18, 2021 at 03:54:00PM +0100, Heiner Kallweit wrote:
> On 18.03.2021 15:23, Michael Walle wrote:
> > at803x_aneg_done() is pretty much dead code since the patch series
> > "net: phy: improve and simplify phylib state machine" [1]. Remove it.
> > 
> 
> Well, it's not dead, it's resting .. There are few places where
> phy_aneg_done() is used. So you would need to explain:
> - why these users can't be used with this PHY driver
> - or why the aneg_done callback isn't needed here and the
>   genphy_aneg_done() fallback is sufficient

The piece of code that Michael is removing keeps the aneg reporting as
"not done" even when the copper-side link was reported as up, but the
in-band autoneg has not finished.

That was the _intended_ behavior when that code was introduced, and you
have said about it:
https://www.spinics.net/lists/stable/msg389193.html

| That's not nice from the PHY:
| It signals "link up", and if the system asks the PHY for link details,
| then it sheepishly says "well, link is *almost* up".

If the specification of phy_aneg_done behavior does not include in-band
autoneg (and it doesn't), then this piece of code does not belong here.

The fact that we can no longer trigger this code from phylib is yet
another reason why it fails at its intended (and wrong) purpose and
should be removed.