netdev - Re: [RFC net-next 0/2] net: dsa: bcm_sf2: Utilize PHYLINK for all ports

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: <174b5e64-a250-e1b2-43b9-474b915ddc22@gmail.com>
Date:   Tue, 26 Jul 2022 09:14:17 -0700
From:   Florian Fainelli <f.fainelli@...il.com>
To:     Vladimir Oltean <olteanv@...il.com>
Cc:     netdev@...r.kernel.org, Andrew Lunn <andrew@...n.ch>,
        Vivien Didelot <vivien.didelot@...il.com>,
        "David S. Miller" <davem@...emloft.net>,
        Eric Dumazet <edumazet@...gle.com>,
        Jakub Kicinski <kuba@...nel.org>,
        Paolo Abeni <pabeni@...hat.com>,
        open list <linux-kernel@...r.kernel.org>
Subject: Re: [RFC net-next 0/2] net: dsa: bcm_sf2: Utilize PHYLINK for all
 ports

On 7/26/22 04:23, Vladimir Oltean wrote:
> Hi Florian,
> 
> On Mon, Jul 25, 2022 at 02:49:40PM -0700, Florian Fainelli wrote:
>> Hi all,
>>
>> Although this should work for all devices, since most DTBs on the
>> platforms where bcm_sf2 is use do not populate a 'fixed-link' property
>> for their CPU ports, but rely on the Ethernet controller DT node doing
>> that, we will not be registering a 'fixed-link' instance for CPU ports.
>>
>> This still works because the switch matches the configuration of the
>> Ethernet controller, but on BCM4908 where we want to force 2GBits/sec,
>> that I cannot test, not so sure.
>>
>> So as of now, this series does not produce register for register
>> compatile changes.
> 
> My understanding of this change set is that it stops overriding the link
> status of IMP ports from dsa_switch_ops :: setup (unconditionally) and
> it moves it to phylink_mac_link_up(). But the latter may not be called
> when you lack a fixed-link, and yet the IMP port(s) still work(s).
> 
> This begs the natural question, is overriding the link status ever needed?

It was until we started to unconditionally reset the switch using the "external" reset method as opposed to the "internal" reset method which turned out not to be functional:

https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=eee87e4377a4b86dc2eea0ade162b0dc33f40576

At any rate (no pun intended), 4908 will want a 2GBit/sec IMP port to be set-up and we have no way to do that other than by forcing that setting, either through the bcm_sf2_imp_setup() method or via a hack to the mac_link_up() callback. This is kind of orthogonal in the sense that there is no "official" support for speed 2000 mbits/sec anyway in the emulated SW PHY, PHYLINK or anywhere in between but if we want to fully transition over to PHYLINK to configure all ports, which is absolutely the goal, we will need to find a solution one way or another.

I would prefer if also we sort of "transferred" the 'fixed-link' parameters from the DSA Ethernet controller attached to the CPU port onto the PHYLINK instance of the CPU port in the switch as they ought to be strictly identical otherwise it just won't work. This would ensure that we continue to force the link and it would make me sleep better a night to know that the IMP port is operating strictly the same way it was. My script compares register values before/after for the registers that are static and this was flagged as a difference.
-- 
Florian