netdev - Re: [PATCH] net: dsa: microchip: look for phy-mode in port nodes

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 for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20200716100743.GA3275@laureti-dev>
Date:   Thu, 16 Jul 2020 12:07:43 +0200
From:   Helmut Grohne <helmut.grohne@...enta.de>
To:     Andrew Lunn <andrew@...n.ch>
CC:     Nicolas Ferre <nicolas.ferre@...rochip.com>,
        Alexandre Belloni <alexandre.belloni@...tlin.com>,
        Ludovic Desroches <ludovic.desroches@...rochip.com>,
        Woojung Huh <woojung.huh@...rochip.com>,
        Microchip Linux Driver Support <UNGLinuxDriver@...rochip.com>,
        Vivien Didelot <vivien.didelot@...il.com>,
        Florian Fainelli <f.fainelli@...il.com>,
        "David S. Miller" <davem@...emloft.net>,
        Jakub Kicinski <kuba@...nel.org>,
        "Rob Herring" <robh+dt@...nel.org>,
        "devicetree@...r.kernel.org" <devicetree@...r.kernel.org>,
        "netdev@...r.kernel.org" <netdev@...r.kernel.org>
Subject: Re: [PATCH] net: dsa: microchip: look for phy-mode in port nodes

On Thu, Jul 16, 2020 at 09:00:00AM +0200, Helmut Grohne wrote:
> I've prepared a patch based one the one-CPU-port assumption. It really
> becomes way simpler that way. I'd like to give it a little more testing
> before sending it.

I'm sorry, but it is not that simple. Testing revealed a fatal flaw.

At the time ksz_switch_register is called, dev->cpu_port is not
necessarily initialized. For ksz8795, it is initialized during detect
and that is fine. For ksz9477, it is initialized in
ksz9477_config_cpu_port, which comes much too late. The ksz9477 driver
actually handles the case where we choose a CPU port and selects it
using dsa_is_cpu_port (which derives this information from the device
tree during dsa_register_switch).

So in ksz_switch_register, I have no good way of knowing which port will
end up being the CPU port. Options include:
 * Replicating the logic from ksz9477_config_cpu_port. I expect that
   doing this would make the driver harder to maintain.
 * Move the cpu_port computation from ksz9477_config_cpu_port to
   ksz9477_switch_detect. I don't think this would work, because we
   presently call detect before dsa_register_switch, which parses the
   device tree. During detect, dsa_is_cpu_port is not usable.
 * Add a function to ksz_common.c that looks up the phy mode for a port
   from the device tree on demand. That way, ksz9477_config_cpu_port
   could call into the common code instead of reading a ksz_device
   property. It would defer parsing the device tree though and it could
   issue the warning multiple times.
 * Stick with my previous patch that stores the phy mode per-port.

Given the above, the last option seems least bad to me. Do you agree?

Helmut