netdev - Re: [PATCH] net: phy: leds: Trigger leds only if PHY speed is known

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] [day] [month] [year] [list]

Message-ID: <YPGjnvB92ajEBKGJ@lunn.ch>
Date:   Fri, 16 Jul 2021 17:19:58 +0200
From:   Andrew Lunn <andrew@...n.ch>
To:     "Ivan T. Ivanov" <iivanov@...e.de>
Cc:     Heiner Kallweit <hkallweit1@...il.com>,
        Russell King <linux@...linux.org.uk>,
        "David S . Miller" <davem@...emloft.net>,
        Jakub Kicinski <kuba@...nel.org>, netdev@...r.kernel.org,
        linux-kernel@...r.kernel.org
Subject: Re: [PATCH] net: phy: leds: Trigger leds only if PHY speed is known

On Fri, Jul 16, 2021 at 05:11:42PM +0300, Ivan T. Ivanov wrote:
> This prevents "No phy led trigger registered for speed(-1)"
> alert message which is coused by phy_led_trigger_chage_speed()
> being called during attaching phy to net_device where phy device
> speed could be still unknown.

Hi Ivan

It seems odd that when attaching the PHY we have link, but not the
speed. What PHY is this?

> -	if (phy->speed == 0)
> +	if (phy->speed == 0 || phy->speed == SPEED_UNKNOWN)
>  		return;

This change makes sense. But i'm wondering if the original logic is
sound. We have link, but no speed information. So the LED trigger is
left in an indeterminate state. Rather than a plain return, maybe
phy_led_trigger_no_link(phy) should be called?

     Andrew