| 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
| ||
|
Message-ID: <YmJbzay/OiSAxYWF@shell.armlinux.org.uk>
Date: Fri, 22 Apr 2022 08:39:57 +0100
From: "Russell King (Oracle)" <linux@...linux.org.uk>
To: Florian Fainelli <f.fainelli@...il.com>
Cc: Andrew Lunn <andrew@...n.ch>,
"David S. Miller" <davem@...emloft.net>,
Jakub Kicinski <kuba@...nel.org>, netdev@...r.kernel.org,
Vivien Didelot <vivien.didelot@...il.com>,
Vladimir Oltean <olteanv@...il.com>
Subject: Re: [PATCH net-next 5/5] net: dsa: b53: mark as non-legacy
On Thu, Apr 21, 2022 at 03:31:26PM -0700, Florian Fainelli wrote:
> Hi Russell,
>
> On 2/22/22 02:16, Russell King (Oracle) wrote:
> > The B53 driver does not make use of the speed, duplex, pause or
> > advertisement in its phylink_mac_config() implementation, so it can be
> > marked as a non-legacy driver.
> >
> > Signed-off-by: Russell King (Oracle) <rmk+kernel@...linux.org.uk>
> > ---
> > drivers/net/dsa/b53/b53_common.c | 6 ++++++
> > 1 file changed, 6 insertions(+)
> >
> > diff --git a/drivers/net/dsa/b53/b53_common.c b/drivers/net/dsa/b53/b53_common.c
> > index 50a372dc32ae..83bf30349c26 100644
> > --- a/drivers/net/dsa/b53/b53_common.c
> > +++ b/drivers/net/dsa/b53/b53_common.c
> > @@ -1346,6 +1346,12 @@ static void b53_phylink_get_caps(struct dsa_switch *ds, int port,
> > /* Get the implementation specific capabilities */
> > if (dev->ops->phylink_get_caps)
> > dev->ops->phylink_get_caps(dev, port, config);
> > +
> > + /* This driver does not make use of the speed, duplex, pause or the
> > + * advertisement in its mac_config, so it is safe to mark this driver
> > + * as non-legacy.
> > + */
> > + config->legacy_pre_march2020 = false;
>
> This patch appears to cause a regression for me, I am not sure why I did not
> notice it back when I tested it but I suspect it had to do with me testing
> only with a copper module and not with a fiber module.
>
> Now that I tested it again, the SFP port (port 5 in my set-up) link up
> interrupt does not fire up when setting config->legacy_pre_march2020 to
> false.
>
> Here is a working log with phylink debugging enabled:
>
> # udhcpc -i sfp
> udhcpc: started, v1.35.0
> [ 49.479637] bgmac-enet 18024000.ethernet eth2: Link is Up - 1Gbps/Full -
> flow control off
> [ 49.488139] Generic PHY fixed-0:02: PHY state change UP -> RUNNING
> [ 49.488256] b53-srab-switch 18036000.ethernet-switch sfp: configuring for
> inband/1000base-x link mode
> [ 49.504062] b53-srab-switch 18036000.ethernet-switch sfp: major config
> 1000base-x
> [ 49.511800] b53-srab-switch 18036000.ethernet-switch sfp:
> phylink_mac_config: mode=inband/1000base-x/Unknown/Unknown
> adv=0000000,00000201
> [ 49.527504] b53-srab-switch 18036000.ethernet-switch sfp: mac link down
> [ 49.535044] sfp sfp: SM: enter present:down:down event dev_up
> [ 49.541006] sfp sfp: tx disable 1 -> 0
> [ 49.544897] sfp sfp: SM: exit present:up:wait
> [ 49.549509] IPv6: ADDRCONF(NETDEV_CHANGE): eth2: link becomes ready
> udhcpc: broadcasting discover
> [ 49.595185] sfp sfp: SM: enter present:up:wait event timeout
> [ 49.601064] sfp sfp: SM: exit present:up:link_up
> [ 52.388917] b53-srab-switch 18036000.ethernet-switch sfp: mac link up
> [ 52.396513] b53-srab-switch 18036000.ethernet-switch sfp: Link is Up -
> 1Gbps/Full - flow control rx/tx
> [ 52.406145] IPv6: ADDRCONF(NETDEV_CHANGE): sfp: link becomes ready
> udhcpc: broadcasting discover
> udhcpc: broadcasting select for 192.168.3.156, server 192.168.3.1
> udhcpc: lease of 192.168.3.156 obtained from 192.168.3.1, lease time 600
> deleting routers
> adding dns 192.168.1.1
>
> and one that is not working with phylink debugging enabled:
>
> # udhcpc -i sfp
> udhcpc: started, v1.35.0
> [ 27.863529] bgmac-enet 18024000.ethernet eth2: Link is Up - 1Gbps/Full -
> flow control off
> [ 27.872021] Generic PHY fixed-0:02: PHY state change UP -> RUNNING
> [ 27.872120] b53-srab-switch 18036000.ethernet-switch sfp: configuring for
> inband/1000base-x link mode
> [ 27.887952] b53-srab-switch 18036000.ethernet-switch sfp: major config
> 1000base-x
> [ 27.895689] b53-srab-switch 18036000.ethernet-switch sfp:
> phylink_mac_config: mode=inband/1000base-x/Unknown/Unknown
> adv=0000000,00000201
> [ 27.895802] b53-srab-switch 18036000.ethernet-switch sfp: mac link down
> [ 27.911945] sfp sfp: SM: enter present:down:down event dev_up
> [ 27.923947] sfp sfp: tx disable 1 -> 0
> [ 27.927835] sfp sfp: SM: exit present:up:wait
> [ 27.932442] IPv6: ADDRCONF(NETDEV_CHANGE): eth2: link becomes ready
> udhcpc: broadcasting discover
> [ 27.978181] sfp sfp: SM: enter present:up:wait event timeout
> [ 27.984056] sfp sfp: SM: exit present:up:link_up
> [ 30.686440] b53-srab-switch 18036000.ethernet-switch sfp: mac link up
> udhcpc: broadcasting discover
> udhcpc: broadcasting discover
>
> The mac side appears to be UP but not no carrier is set to the sfp network
> device. Do you have any idea why that would happen?
Oh, it's because setting that flag means we're wanting the PCS methods
rather than the legacy MAC methods for an_restart and getting the PCS
link state - so the patch in question was submitted too early (it
should have been _after_ the conversion to PCS.)
If we get the patch reverted in net-next, and then convert b53 to use
PCS support, we'll then be putting the patch back, so I wonder if it
would just make sense to apply the PCS conversion patch, possibly
adding a comment in the commit message pointing out that this fixes
the b53 legacy_pre_march2020 patch. Thoughts?
--
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 40Mbps down 10Mbps up. Decent connectivity at last!
Powered by blists - more mailing lists