netdev - Re: [PATCH net-next 2/2] net: dsa: mv88e6xxx: Set link down when changing speed

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: <20200327214854.GS25745@shell.armlinux.org.uk>
Date:   Fri, 27 Mar 2020 21:48:54 +0000
From:   Russell King - ARM Linux admin <linux@...linux.org.uk>
To:     Andrew Lunn <andrew@...n.ch>
Cc:     David Miller <davem@...emloft.net>,
        netdev <netdev@...r.kernel.org>,
        Vivien Didelot <vivien.didelot@...il.com>
Subject: Re: [PATCH net-next 2/2] net: dsa: mv88e6xxx: Set link down when
 changing speed

On Fri, Mar 27, 2020 at 10:26:08PM +0100, Andrew Lunn wrote:
> > > Hi Russell
> > > 
> > > So the problem here is that CPU and DSA ports should default to up and
> > > at their fastest speed. During setup, the driver is setting the CPU
> > > port to 1G and up. Later on, phylink finds the fixed-link node in DT,
> > > and then sets the port to 100Mbps as requested.
> > > 
> > > How do you suggest fixing this? If we find a fixed-link, configure it
> > > first down and then up?
> > 
> > I think this is another example of DSA fighting phylink in terms of
> > what's expected.
> > 
> > The only suggestion I've come up so far with is to avoid calling
> > mv88e6xxx_port_setup_mac() with forced-link-up in
> > mv88e6xxx_setup_port() if we have phylink attached.
> 
> Hi Russell
> 
> Yes, that might work. But it is a solution specific to mv88e6xxx. I
> guess other switches could have a similar issue.
> 
> Is it really that bad to add the link down as i proposed? Do we even
> have a guarantee the port is down before phylink starts configuring
> it, for all switch drivers?

That's partly why I made the suggestion, as mac_config() could end
up being called with potential changes to the configuration with
the link already up.  If that can happen, then it's not just about
the link being down before mac_link_up() is called...

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line in suburbia: sync at 10.2Mbps down 587kbps up