netdev - Re: [PATCH net-next 4/4] net: dsa: remove phylink

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] [thread-next>] [day] [month] [year] [list]

Message-ID: <20221104133247.4cfzt4wcm6oei563@skbuf>
Date:   Fri, 4 Nov 2022 13:32:48 +0000
From:   Vladimir Oltean <vladimir.oltean@....com>
To:     "Russell King (Oracle)" <linux@...linux.org.uk>
CC:     Florian Fainelli <f.fainelli@...il.com>,
        "netdev@...r.kernel.org" <netdev@...r.kernel.org>,
        Claudiu Manoil <claudiu.manoil@....com>,
        Alexandre Belloni <alexandre.belloni@...tlin.com>,
        "UNGLinuxDriver@...rochip.com" <UNGLinuxDriver@...rochip.com>,
        Heiner Kallweit <hkallweit1@...il.com>,
        Sean Anderson <sean.anderson@...o.com>,
        Colin Foster <colin.foster@...advantage.com>,
        Andrew Lunn <andrew@...n.ch>,
        Vivien Didelot <vivien.didelot@...il.com>,
        "David S. Miller" <davem@...emloft.net>,
        Eric Dumazet <edumazet@...gle.com>,
        Jakub Kicinski <kuba@...nel.org>,
        Paolo Abeni <pabeni@...hat.com>
Subject: Re: [PATCH net-next 4/4] net: dsa: remove phylink_validate() method

On Fri, Nov 04, 2022 at 11:35:08AM +0000, Russell King (Oracle) wrote:
> On Fri, Nov 04, 2022 at 11:24:44AM +0000, Russell King (Oracle) wrote:
> > There is one remaining issue that needs to be properly addressed,
> > which is the bcm_sf2 driver, which is basically buggy. The recent
> > kernel build bot reports reminded me of this.
> > 
> > I've tried talking to Florian about it, and didn't make much progress,
> > so I'm carrying a patch in my tree which at least makes what is
> > provided to phylink correct.
> > 
> > See
> > http://git.armlinux.org.uk/cgit/linux-arm.git/commit/?h=net-queue&id=63d77c1f9db167fd74994860a4a899df5c957aab
> > and all the FIXME comments in there.
> > 
> > This driver really needs to be fixed before we kill DSA's
> > phylink_validate method (although doing so doesn't change anything
> > in mainline, but will remove my reminder that bcm_sf2 is still
> > technically broken.)
> 
> Here's the corrected patch, along with a bit more commentry about the
> problems that I had kicking around in another commit.

The inconsistencies in the sf2 driver seem valid - I don't know why/if
the hardware doesn't support flow control on MoCA, internal ports and
(some but not all?!) RGMII modes. I hope Florian can make some clarifications.

However, I don't exactly understand your choice of fixing this
inconsistency (by providing a phylink_validate method). Why don't you
simply set MAC_ASYM_PAUSE | MAC_SYM_PAUSE in config->mac_capabilities
from within bcm_sf2_sw_get_caps(), only if we know this is an xMII port
(and not for MoCA and internal PHYs)? Then, phylink_generic_validate()
would know to exclude the "pause" link modes, right?

In any case, it looks like coming up with a resolution for DSA's
phylink_validate is out of scope for this patch set, and that I should
just drop patch 4/4 for now and resend the first 3.