netdev - RE: [RFC net-next 2/5] net: phylink: add separate pcs operations structure

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:   Tue, 17 Mar 2020 15:48:50 +0000
From:   Jose Abreu <Jose.Abreu@...opsys.com>
To:     Russell King <rmk+kernel@...linux.org.uk>,
        Andrew Lunn <andrew@...n.ch>,
        Florian Fainelli <f.fainelli@...il.com>,
        Heiner Kallweit <hkallweit1@...il.com>
CC:     "David S. Miller" <davem@...emloft.net>,
        "netdev@...r.kernel.org" <netdev@...r.kernel.org>
Subject: RE: [RFC net-next 2/5] net: phylink: add separate pcs operations
 structure

From: Russell King <rmk+kernel@...linux.org.uk>
Date: Mar/17/2020, 14:52:51 (UTC+00:00)

> -static void phylink_mac_an_restart(struct phylink *pl)
> +static void phylink_mac_pcs_an_restart(struct phylink *pl)
>  {
>  	if (pl->link_config.an_enabled &&
> -	    phy_interface_mode_is_8023z(pl->link_config.interface))
> -		pl->mac_ops->mac_an_restart(pl->config);
> +	    phy_interface_mode_is_8023z(pl->link_config.interface)) {

Please consider removing this condition and just rely on an_enabled field. 
I have USXGMII support for Clause 73 Autoneg so this won't work with 
that.

Overall, looks nice but I don't see a mechanism to restart AutoNeg in case 
of failure. i.e. following PHYLIB implementation you should add a call to 
restart_aneg in the state machine. This was just a quick look so I may 
have missed this.

---
Thanks,
Jose Miguel Abreu