netdev - Re: [PATCH v4 net-next 5/8] net: phylink: explicitly configure in-band autoneg for on-board PHYs

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: <Y3eYk1TJnLN+r86a@shell.armlinux.org.uk>
Date:   Fri, 18 Nov 2022 14:37:07 +0000
From:   "Russell King (Oracle)" <linux@...linux.org.uk>
To:     Vladimir Oltean <vladimir.oltean@....com>
Cc:     netdev@...r.kernel.org, "David S. Miller" <davem@...emloft.net>,
        Eric Dumazet <edumazet@...gle.com>,
        Jakub Kicinski <kuba@...nel.org>,
        Paolo Abeni <pabeni@...hat.com>,
        Heiner Kallweit <hkallweit1@...il.com>,
        Andrew Lunn <andrew@...n.ch>,
        Florian Fainelli <f.fainelli@...il.com>,
        UNGLinuxDriver@...rochip.com,
        bcm-kernel-feedback-list@...adcom.com,
        Madalin Bucur <madalin.bucur@....nxp.com>,
        Camelia Groza <camelia.groza@....com>,
        Claudiu Manoil <claudiu.manoil@....com>,
        Ioana Ciornei <ioana.ciornei@....com>,
        Maxim Kochetkov <fido_max@...ox.ru>,
        Sean Anderson <sean.anderson@...o.com>,
        Antoine Tenart <atenart@...nel.org>,
        Michael Walle <michael@...le.cc>,
        Raag Jadav <raagjadav@...il.com>,
        Siddharth Vadapalli <s-vadapalli@...com>,
        Ong Boon Leong <boon.leong.ong@...el.com>,
        Colin Foster <colin.foster@...advantage.com>,
        Marek Behun <marek.behun@....cz>
Subject: Re: [PATCH v4 net-next 5/8] net: phylink: explicitly configure
 in-band autoneg for on-board PHYs

On Fri, Nov 18, 2022 at 01:25:20PM +0200, Vladimir Oltean wrote:
> On Fri, Nov 18, 2022 at 10:09:35AM +0000, Russell King (Oracle) wrote:
> > On Fri, Nov 18, 2022 at 02:01:21AM +0200, Vladimir Oltean wrote:
> > > +	if (pl->config->sync_an_inband && !phy_on_sfp(phy)) {
> > 
> > Hmm, this phy_on_sfp() is new to me, and looking at the git history, I
> > really don't think this does what it claims to do. This returns the
> > status of phydev->is_on_sfp_module, which is set by this code:
> > 
> >         phydev->phy_link_change = phy_link_change;
> >         if (dev) {
> >                 phydev->attached_dev = dev;
> >                 dev->phydev = phydev;
> > 
> >                 if (phydev->sfp_bus_attached)
> >                         dev->sfp_bus = phydev->sfp_bus;
> >                 else if (dev->sfp_bus)
> >                         phydev->is_on_sfp_module = true;
> >         }
> > 
> > ... which is very wrong. "dev" here is the net_device, and a net_device
> > will have its sfp_bus member set when there is a SFP cage present,
> > which may be behind a off-SFP PHY.
> > 
> > This means that when a PHY is attached by the network driver in their
> > ndo_open, if there is a SFP bus on the interface (such as on the
> > Macchiatobin board), the above will set is_on_sfp_module true for the
> > on-board PHY even though it is not in the SFP module.
> > 
> > Essentially, commit b834489bcecc is incorrect, and needs to be fixed
> > before use is made of phy_on_sfp() outside of the broadcom driver.
> 
> IIUC, you're saying that if there is an SFP cage after an on-board PHY
> X (presumably set using phy_sfp_attach()), then PHY X will be declared
> as having phydev->is_on_sfp_module = true despite being on-board?

Having looked more deeply, I don't think it's the problem I thought it
was, so you're all good with using phy_on_sfp(). Sorry for the noise.

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 40Mbps down 10Mbps up. Decent connectivity at last!