netdev - Re: [BUG] mv88e6xxx: tx regression in v5.3

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]

Date:   Thu, 12 Dec 2019 14:14:48 +0100
From:   Andrew Lunn <andrew@...n.ch>
To:     Baruch Siach <baruch@...s.co.il>
Cc:     Vivien Didelot <vivien.didelot@...il.com>, netdev@...r.kernel.org,
        Denis Odintsov <d.odintsov@...viangames.com>,
        Hubert Feurstein <h.feurstein@...il.com>
Subject: Re: [BUG] mv88e6xxx: tx regression in v5.3

> As you guessed, mv88e6xxx_mac_config() exits early because 
> mv88e6xxx_phy_is_internal() returns true for port number 2, and 'mode' is 
> MLO_AN_PHY. What is the right MAC/PHY setup flow in this case?

So this goes back to

commit d700ec4118f9d5e88db8f678e7342f28c93037b9
Author: Marek Vasut <marex@...x.de>
Date:   Wed Sep 12 00:15:24 2018 +0200

    net: dsa: mv88e6xxx: Make sure to configure ports with external PHYs
    
    The MV88E6xxx can have external PHYs attached to certain ports and those
    PHYs could even be on different MDIO bus than the one within the switch.
    This patch makes sure that ports with such PHYs are configured correctly
    according to the information provided by the PHY.

@@ -709,13 +717,17 @@ static void mv88e6xxx_mac_config(struct dsa_switch *ds, int port,
        struct mv88e6xxx_chip *chip = ds->priv;
        int speed, duplex, link, pause, err;
 
-       if (mode == MLO_AN_PHY)
+       if ((mode == MLO_AN_PHY) && mv88e6xxx_phy_is_internal(ds, port))
                return;

The idea being, that the MAC has direct knowledge of the PHY
configuration because it is internal. There is no need to configure
the MAC, it does it itself.

This assumption seems wrong for the switch you have.

I think it is just a optimisation. So we can probably remove this phy
internal test.

And
        } else if (!mv88e6xxx_phy_is_internal(ds, port)) {

also needs to change.

It would be interesting to know if the MAC is completely wrongly
configured, or it is just a subset of parameters.

	    Andrew