netdev - Re: [PATCH] leds: triggers: netdev: add a check, whether device is up

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: <9e9dad320a53864e6b5bd3ecaedd2dc3bc0ff0f1.camel@gmail.com>
Date: Sat, 04 Nov 2023 20:46:59 +0100
From: Klaus Kudielka <klaus.kudielka@...il.com>
To: "Russell King (Oracle)" <linux@...linux.org.uk>, Andrew Lunn
	 <andrew@...n.ch>
Cc: "David S . Miller" <davem@...emloft.net>, Jakub Kicinski
 <kuba@...nel.org>,  netdev <netdev@...r.kernel.org>
Subject: Re: [PATCH] leds: triggers: netdev: add a check, whether device is
 up

On Sat, 2023-11-04 at 17:42 +0000, Russell King (Oracle) wrote:
> On Sat, Nov 04, 2023 at 05:46:44PM +0100, Andrew Lunn wrote:
> > On Sat, Nov 04, 2023 at 05:32:19PM +0100, Klaus Kudielka wrote:
> > > 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?
> > 
> > Crossing emails.
> > 
> > Its a better approach. But it fixes just one driver. If we can do this
> > in phylink_create(), we fix it in a lot of drivers with a single
> > change...
> 
> ... and I think we should.
> 

So, the patch below also results in correct behaviour of the netdev trigger
with eth2 down, but I'm not so sure whether it really covers all desired
cases. Any advice?

--- a/drivers/net/phy/phylink.c
+++ b/drivers/net/phy/phylink.c
@@ -1616,6 +1616,7 @@ struct phylink *phylink_create(struct phylink_config *config,
        pl->config = config;
        if (config->type == PHYLINK_NETDEV) {
                pl->netdev = to_net_dev(config->dev);
+               netif_carrier_off(pl->netdev);
        } else if (config->type == PHYLINK_DEV) {
                pl->dev = config->dev;
        } else {


Best regards, Klaus