netdev - Re: [PATCH RFC 03/18] net: marvell: mvneta: Simplify EEE configuration

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: <Y+9sK/yN7JmQyTl0@shell.armlinux.org.uk>
Date:   Fri, 17 Feb 2023 11:59:39 +0000
From:   "Russell King (Oracle)" <linux@...linux.org.uk>
To:     Andrew Lunn <andrew@...n.ch>
Cc:     netdev <netdev@...r.kernel.org>,
        Florian Fainelli <f.fainelli@...il.com>,
        Vladimir Oltean <vladimir.oltean@....com>,
        Sean Wang <sean.wang@...iatek.com>,
        Landen Chao <Landen.Chao@...iatek.com>,
        DENG Qingfang <dqfext@...il.com>,
        Matthias Brugger <matthias.bgg@...il.com>,
        AngeloGioacchino Del Regno 
        <angelogioacchino.delregno@...labora.com>,
        Doug Berger <opendmb@...il.com>,
        Broadcom internal kernel review list 
        <bcm-kernel-feedback-list@...adcom.com>,
        Wei Fang <wei.fang@....com>,
        Shenwei Wang <shenwei.wang@....com>,
        Clark Wang <xiaoning.wang@....com>,
        NXP Linux Team <linux-imx@....com>,
        UNGLinuxDriver@...rochip.com, Byungho An <bh74.an@...sung.com>,
        Giuseppe Cavallaro <peppe.cavallaro@...com>,
        Alexandre Torgue <alexandre.torgue@...s.st.com>,
        Jose Abreu <joabreu@...opsys.com>,
        Maxime Coquelin <mcoquelin.stm32@...il.com>,
        Heiner Kallweit <hkallweit1@...il.com>,
        Woojung Huh <woojung.huh@...rochip.com>,
        Oleksij Rempel <linux@...pel-privat.de>
Subject: Re: [PATCH RFC 03/18] net: marvell: mvneta: Simplify EEE
 configuration

On Fri, Feb 17, 2023 at 04:42:15AM +0100, Andrew Lunn wrote:
> @@ -4221,10 +4218,8 @@ static void mvneta_mac_link_up(struct phylink_config *config,
>  
>  	mvneta_port_up(pp);
>  
> -	if (phy && pp->eee_enabled) {
> -		pp->eee_active = phy_init_eee(phy, false) >= 0;
> -		mvneta_set_eee(pp, pp->eee_active && pp->tx_lpi_enabled);
> -	}
> +	if (phy)
> +		mvneta_set_eee(pp, phy->eee_active && pp->tx_lpi_enabled);

Thinking about this a bit more, I'm not convinced this is properly safe.
What protects phy->eee_active from changing here? The phydev mutex won't
be held at this point.

As I mentioned in my reply to the cover letter about passing a flag to
mac_link_up() for EEE status, this would mean phylink could save the
EEE active status just like it does with the other phydev parameters
in phylink_phy_change() (which is called under the phydev mutex).

That ensures that we grab all the phylib state under the phydev mutex
and then use that state when bringing the MAC side up without needing
to worry about phydev mutexes.

It's way more invasive, as it requires the mac_link_up() method
signature to change, but I think it would be a cleaner approach
overall.

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 40Mbps down 10Mbps up. Decent connectivity at last!