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

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: <52114397-62cf-4112-9d5b-73098f25fe20@lunn.ch>
Date:   Mon, 6 Nov 2023 22:26:34 +0100
From:   Andrew Lunn <andrew@...n.ch>
To:     Klaus Kudielka <klaus.kudielka@...il.com>
Cc:     Russell King <linux@...linux.org.uk>,
        Heiner Kallweit <hkallweit1@...il.com>,
        "David S. Miller" <davem@...emloft.net>,
        Eric Dumazet <edumazet@...gle.com>,
        Jakub Kicinski <kuba@...nel.org>,
        Paolo Abeni <pabeni@...hat.com>, netdev@...r.kernel.org,
        linux-kernel@...r.kernel.org
Subject: Re: [PATCH net] net: phylink: initialize carrier state at creation

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@...r.kernel.org

There is more about this in:

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

Reviewed-by: Andrew Lunn <andrew@...n.ch>

    Andrew