netdev - Re: [PATCH net-next] net: dsa: realtek: rtl8366rb: Configure ports properly

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] [day] [month] [year] [list]

Date:   Wed, 27 Jul 2022 17:07:56 +0300
From:   Vladimir Oltean <olteanv@...il.com>
To:     Linus Walleij <linus.walleij@...aro.org>
Cc:     Andrew Lunn <andrew@...n.ch>,
        Vivien Didelot <vivien.didelot@...il.com>,
        Florian Fainelli <f.fainelli@...il.com>,
        "David S . Miller" <davem@...emloft.net>,
        Jakub Kicinski <kuba@...nel.org>, netdev@...r.kernel.org,
        Alvin Šipraga <alsi@...g-olufsen.dk>
Subject: Re: [PATCH net-next] net: dsa: realtek: rtl8366rb: Configure ports
 properly

Hi Linus,

On Tue, Jul 26, 2022 at 04:20:18PM +0200, Linus Walleij wrote:
> > According to your phylink_get_caps() implementation, I see that all
> > ports are internal, so presumably the CPU ports too (and the user ports
> > are connected to internal PHYs).
> 
> Correct, if by internal you mean there is no external, discrete PHY
> component. They all route out to the physical sockets, maybe with
> some small analog filter inbetween.

Yes, I mean that if there are RJ-45 ports, they are all driven by PHYs
which are internal to the switch chip, and if there is no internal PHY,
those ports are also internal to the chip, like in the case of the CPU
port.

> > Is it just to act upon the phylink parameters rather than assuming the
> > CPU port is at gigabit? Can you actually set the CPU port at lower rates?
> 
> I think you can, actually. The Realtek vendor mess does support it.
> 
> Hm I should test to gear it down to 100Mbit and 10Mbit and see
> what happens.

So the change wasn't intended for the CPU port, this is good to know.

> > As for the internal PHY ports, do they need their link speed to be
> > forced at 10/100, or did those previously work at those lower speeds,
> > just left unforced?
> 
> They were left in "power-on"-state. Which I *guess* is
> autonegotiate. But haven't really tested.
> 
> It leaves me a bit uneasy since these registers are never explicit
> set up to autonegotiate. Maybe I should do a separate patch
> to just set them explicitly in autonegotiation mode?

I am confused as to what you are describing when you are using the
"autonegotiation" word. "Port" is too generic, every user port will have
a MAC and a PHY. The PHY is what deals with autonegotiation; also, the
PHY is what the DSA driver does *not* control (it uses either a dedicated
or a generic driver from drivers/net/phy).

Between the internal MAC and the internal PHY, what's going on isn't
called autonegotiation, but doesn't have a specific name either, as far
as I know. Rather, because the 2 components are part of the same die,
the hardware designers may have added logic that automatically adapts
the speed in the MAC according to the speed that the PHY negotiated for
(or was forced at).

Your change (or at least the way in which I understand it) essentially
always forces the internal MAC to the same speed as the PHY. This is not
wrong per se, but I'm not clear if it's necessary either, considering
that there might be hardware logic to do this automatically.

> I have a small 10MBit router, I will try to connect it and see what
> happens, if anything happens and can be detected.

You don't need a dedicated 10BASE-T device to test this, you can run
"ethtool -s eth0 advertise 0x2" on any gigabit-capable device, and this
will limit the advertisement in its PHY to just 10BASE-T.