linux-kernel - Re: [PATCH v3 net-next 05/10] net: dsa: microchip: add DSA support for microchip lan937x

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: <YQ6pc6EZRLftmRh3@lunn.ch>
Date:   Sat, 7 Aug 2021 17:40:35 +0200
From:   Andrew Lunn <andrew@...n.ch>
To:     Vladimir Oltean <olteanv@...il.com>
Cc:     "Russell King (Oracle)" <linux@...linux.org.uk>,
        Prasanna Vengateshan <prasanna.vengateshan@...rochip.com>,
        netdev@...r.kernel.org, robh+dt@...nel.org,
        UNGLinuxDriver@...rochip.com, Woojung.Huh@...rochip.com,
        hkallweit1@...il.com, davem@...emloft.net, kuba@...nel.org,
        linux-kernel@...r.kernel.org, vivien.didelot@...il.com,
        f.fainelli@...il.com, devicetree@...r.kernel.org
Subject: Re: [PATCH v3 net-next 05/10] net: dsa: microchip: add DSA support
 for microchip lan937x

> I am not even clear what is the expected canonical behavior for a MAC
> driver. It parses rx-internal-delay-ps and tx-internal-delay-ps, and
> then what?

So best practices are based around a MAC-PHY link. phy-mode is passed
to the PHY, and the MAC does not act upon it. MAC rx-internal-delay-ps
and tx-internal-delay-ps can be used to fine tune the link. You can
use them to add and sometimes subtract small amounts of delay.

> It treats all "rgmii*" phy-mode strings identically? Or is it an
> error to have "rgmii-rxid" for phy-mode and non-zero
> rx-internal-delay-ps?

I would say the first is correct, the second statement is false. You
should always be able to fine tune the link, independent of the PHY
mode.

We also have to consider the case when the PHY is not actually able to
implement the delay. It hopefully returns -EOPNOTSUPP for anything
other than "rgmii". You can then put the full 2ns delay into
tx-internal-delay-ps nd rx-internal-delay-ps.

And lastly there is one MAC driver which mostly ignores these best
practices because the vendor crap tree always did the delay in the
MAC. It correctly masks the phy-mode, so the PHY does not add delays.

For MAC-MAC and fixed link best practices are very fuzzily defined.
It is not something we have much of in the kernel. We might also want
to narrow the discussion down to MACs within a switch. MACs within in
NIC should probably follow the best practices for a MAC-PHY link, even
if it is actually a switch on the other end.

I also agree with Russell that mv88e6xxx is probably broken for a
MAC-PHY link. It is known to work for a Marvell DSA in MAC-MAC link,
we have boards doing that.

It seems like a switch MAC should parse rx-internal-delay-ps and
tx-internal-delay-ps and apply them independent of the phy-mode. That
keeps it consistent with MAC-PHY. And if there is a PHY connected,
pass the phy-mode on unmasked.

So that we don't break mv88e6xxx, for a CPU or DSA port, we probably
should continue to locally implement the delay, with the assumption
there is no PHY, it is a MAC-MAC link. We probably want to patch
mv88e6xxx to do nothing for user ports.

I suspect it is a 50/50 roll of a dice what rx and tx actually
mean. Is it from the perspective of the MAC or the PHY? Luckily,
rgmii-rxid and rgmii-txid don't appear in DT very often.

   Andrew