leds: netdev trigger - misleading link state indication at boot

leds: netdev trigger - misleading link state indication at boot

[Russell King - ARM Linux admin]

Hi,

I'm seeing some odd behaviour with the netdev trigger and some WiFi
interfaces.

When the WiFi interface has never been brought up (so is in an
operationally disabled state), if I bind a LED to the netdev
trigger, setting the device_name to the WiFi interface name and
enable the "link" property, the LED illuminates, indicating that
the WiFi device has link - but it's disabled.

If I up/down the WiFi interface, thereby returning it to the
original state, the link LED goes out.

I suspect ledtrig-netdev.c needs to check that the device is both
up (dev->flags & IFF_UP) and netif_carrier_ok(dev) both return true,
rather than just relying on netif_carrier_ok(). I don't think using
netif_running() is appropriate, as that will return true before
ndo_open() has been called to initialise the carrier state.

Agreed?

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

Re: leds: netdev trigger - misleading link state indication at boot

[Pavel Machek]

[-- Attachment #1: Type: text/plain, Size: 1010 bytes --]

Hi!

> I'm seeing some odd behaviour with the netdev trigger and some WiFi
> interfaces.
> 
> When the WiFi interface has never been brought up (so is in an
> operationally disabled state), if I bind a LED to the netdev
> trigger, setting the device_name to the WiFi interface name and
> enable the "link" property, the LED illuminates, indicating that
> the WiFi device has link - but it's disabled.
> 
> If I up/down the WiFi interface, thereby returning it to the
> original state, the link LED goes out.
> 
> I suspect ledtrig-netdev.c needs to check that the device is both
> up (dev->flags & IFF_UP) and netif_carrier_ok(dev) both return true,
> rather than just relying on netif_carrier_ok(). I don't think using
> netif_running() is appropriate, as that will return true before
> ndo_open() has been called to initialise the carrier state.
> 
> Agreed?

Seems reasonable to me. Will you do the patch?

Best regards,
								Pavel
-- 
http://www.livejournal.com/~pavelmachek

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 181 bytes --]