Re: [PATCH net] net: phylink: initialize carrier state at creation

From: Andrew Lunn
Date: Mon Nov 06 2023 - 16:26:56 EST


On Mon, Nov 06, 2023 at 07:05:06PM +0100, Klaus Kudielka wrote:
> Background: Turris Omnia (Armada 385); eth2 (mvneta) connected to SFP bus;
> SFP module is present, but no fiber connected, so definitely no carrier.
>
> After booting, eth2 is down, but netdev LED trigger surprisingly reports
> link active. Then, after "ip link set eth2 up", the link indicator goes
> away - as I would have expected it from the beginning.
>
> It turns out, that the default carrier state after netdev creation is
> "carrier ok". Some ethernet drivers explicitly call netif_carrier_off
> during probing, others (like mvneta) don't - which explains the current
> behaviour: only when the device is brought up, phylink_start calls
> netif_carrier_off.
>
> Fix this for all drivers, by calling netif_carrier_off in phylink_create.

I would actually say: Fix this for all drivers using phylink, by
calling...

You marked this patch for net, so it should be backported. Ideally you
should include a Fixes: tag, indicating when the problem was
introduced. That is bit hard in this case. Its been broken
forever. But adding LED support made this observable. So maybe a
Fixes: tag based on when the LED trigger was added?

You should also add:

Cc: stable@xxxxxxxxxxxxxxx

There is more about this in:

https://docs.kernel.org/process/stable-kernel-rules.html

Reviewed-by: Andrew Lunn <andrew@xxxxxxx>

Andrew