netdev - Re: Heads up: phylink changes for next merge window

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: <CA+h21ho=siWbp9B=sC5P-Z5B2YEtmnjxnZLzwSwfcVHBkO6rKA@mail.gmail.com>
Date:   Thu, 13 Feb 2020 17:57:36 +0200
From:   Vladimir Oltean <olteanv@...il.com>
To:     Russell King - ARM Linux admin <linux@...linux.org.uk>
Cc:     netdev <netdev@...r.kernel.org>,
        Florian Fainelli <f.fainelli@...il.com>,
        Sean Wang <sean.wang@...iatek.com>,
        John Crispin <john@...ozen.org>,
        Mark Lee <Mark-MC.Lee@...iatek.com>,
        Giuseppe Cavallaro <peppe.cavallaro@...com>,
        Alexandre Torgue <alexandre.torgue@...com>,
        Jose Abreu <joabreu@...opsys.com>,
        Radhey Shyam Pandey <radhey.shyam.pandey@...inx.com>,
        Nicolas Ferre <nicolas.ferre@...rochip.com>,
        Thomas Petazzoni <thomas.petazzoni@...tlin.com>,
        Ioana Ciornei <ioana.ciornei@....com>
Subject: Re: Heads up: phylink changes for next merge window

Hi Russell,

On Thu, 13 Feb 2020 at 16:46, Russell King - ARM Linux admin
<linux@...linux.org.uk> wrote:
>
> [Recipient list updated; removed addresses that bounce, added Ioana
> Ciornei for dpaa2 and DSA issue mentioned below.]
>
> On Thu, Feb 13, 2020 at 01:38:31PM +0000, Russell King - ARM Linux admin wrote:
> > Hi,
>
> I should also have pointed out that with mv88e6xxx, the patch
> "net: mv88e6xxx: use resolved link config in mac_link_up()" "fixes" by
> side-effect an issue that Andrew has mentioned, where inter-DSA ports
> get configured down to 10baseHD speed.  This is by no means a true fix
> for that problem - which is way deeper than this series can address.
> The reason it fixes it is because we no longer set the speed/duplex
> in mac_config() but set it in mac_link_up() - but mac_link_up() is
> never called for CPU and DSA ports.
>
> However, I think there may be another side-effect of that - any fixed
> link declaration in DT may not be respected after this patch.
>
> I believe the root of this goes back to this commit:
>
>   commit 0e27921816ad99f78140e0c61ddf2bc515cc7e22
>   Author: Ioana Ciornei <ioana.ciornei@....com>
>   Date:   Tue May 28 20:38:16 2019 +0300
>
>   net: dsa: Use PHYLINK for the CPU/DSA ports
>
> and, in the case of no fixed-link declaration, phylink has no idea what
> the link parameters should be (and hence the initial bug, where
> mac_config gets called with speed=0 duplex=0, which gets interpreted as
> 10baseHD.)  Moreover, as far as phylink is concerned, these links never
> come up. Essentially, this commit was not fully tested with inter-DSA
> links, and probably was never tested with phylink debugging enabled.
>
> There is currently no fix for this, and it is not an easy problem to
> resolve, irrespective of the patches I'm proposing.
>
> --
> 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

Correct me if I'm wrong, but if the lack of fixed-link specifier for
CPU and DSA ports was not a problem before, but has suddenly started
to become a problem with that patch, then simply reverting to the old
"legacy" logic from dsa_port_link_register_of() should restore the
behavior for those device tree blobs that don't specify an explicit
fixed-link?
In the longer term, can't we just require fixed-link specifiers for
non-netdev ports on new boards, keep the adjust_link code in DSA for
the legacy blobs (which works through some sort of magic), and be done
with it?

Regards,
-Vladimir