| Message ID | 20231104125840.27914-1-klaus.kudielka@gmail.com |
|---|---|
| State | New |
| Headers | show |
| Series | leds: triggers: netdev: add a check, whether device is up | expand |
On Sat, Nov 04, 2023 at 01:58:40PM +0100, Klaus Kudielka wrote: > Some net devices do not report NO-CARRIER, if they haven't been brought > up. Hi Klaus We should probably fix the driver. What device is it? > In that case, the netdev trigger results in a wrong link state being > displayed. Fix this, by adding a check, whether the device is up. Is it wrong? Maybe the carrier really is up, even if the interface is admin down. Broadcast packets are being received by the hardware. Maybe there is a BMC sharing the link and it is active? It is not a clear cut wrong to me. And its a way to find broken drivers. We might want to discuss this some more. Andrew
On Sat, 2023-11-04 at 16:27 +0100, Klaus Kudielka wrote: > > phylink_start() is the first one that does netif_carrier_off() and thus > sets the NOCARRIER bit, but that only happens when bringing the device up. > > Before that, I would not know who cares about setting the NOCARRIER bit. A different, driver-specific solution could be like this (tested and working): --- a/drivers/net/ethernet/marvell/mvneta.c +++ b/drivers/net/ethernet/marvell/mvneta.c @@ -5690,6 +5690,7 @@ static int mvneta_probe(struct platform_device *pdev) /* 9676 == 9700 - 20 and rounding to 8 */ dev->max_mtu = 9676; + netif_carrier_off(dev); err = register_netdev(dev); if (err < 0) { dev_err(&pdev->dev, "failed to register\n"); Would that be the "correct" approach? Regards, Klaus
diff --git a/drivers/leds/trigger/ledtrig-netdev.c b/drivers/leds/trigger/ledtrig-netdev.c index e358e77e4b..bd5e21d0f0 100644 --- a/drivers/leds/trigger/ledtrig-netdev.c +++ b/drivers/leds/trigger/ledtrig-netdev.c @@ -195,7 +195,8 @@ static void get_device_state(struct led_netdev_data *trigger_data) { struct ethtool_link_ksettings cmd; - trigger_data->carrier_link_up = netif_carrier_ok(trigger_data->net_dev); + trigger_data->carrier_link_up = netif_running(trigger_data->net_dev) && + netif_carrier_ok(trigger_data->net_dev); if (!trigger_data->carrier_link_up) return;
Some net devices do not report NO-CARRIER, if they haven't been brought up. In that case, the netdev trigger results in a wrong link state being displayed. Fix this, by adding a check, whether the device is up. Signed-off-by: Klaus Kudielka <klaus.kudielka@gmail.com> --- drivers/leds/trigger/ledtrig-netdev.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)