netdev - Re: [PATCH net-next] net: phylink: require supported

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]

Date: Tue, 21 Nov 2023 15:29:23 +0100
From: Andrew Lunn <andrew@...n.ch>
To: Greg Ungerer <gerg@...nel.org>
Cc: "Russell King (Oracle)" <rmk+kernel@...linux.org.uk>,
	Heiner Kallweit <hkallweit1@...il.com>,
	"David S. Miller" <davem@...emloft.net>,
	Eric Dumazet <edumazet@...gle.com>,
	Jakub Kicinski <kuba@...nel.org>, Paolo Abeni <pabeni@...hat.com>,
	netdev@...r.kernel.org
Subject: Re: [PATCH net-next] net: phylink: require supported_interfaces to
 be filled

> The 6350 looks to be similar to the 6352 in many respects, though it lacks
> a SERDES interface, but it otherwise mostly seems compatible.

Not having the SERDES is important. Without that SERDES, the bit about
Port 4 in mv88e6352_phylink_get_caps() is
incorrect. mv88e61852_phylink_get_caps() looks reasonable for this
hardware.

> Using the 6352
> phylink_get_caps function instead of the 6185 one fixes this:
> 
> --- a/drivers/net/dsa/mv88e6xxx/chip.c
> +++ b/drivers/net/dsa/mv88e6xxx/chip.c
> @@ -5418,7 +5418,7 @@ static const struct mv88e6xxx_ops mv88e6350_ops = {
>         .set_max_frame_size = mv88e6185_g1_set_max_frame_size,
>         .stu_getnext = mv88e6352_g1_stu_getnext,
>         .stu_loadpurge = mv88e6352_g1_stu_loadpurge,
> -       .phylink_get_caps = mv88e6185_phylink_get_caps,
> +       .phylink_get_caps = mv88e6352_phylink_get_caps,
>  };
> 
>  static const struct mv88e6xxx_ops mv88e6351_ops = {
> 
> 
> The story doesn't quite end here though. With this fix in place support
> for the 6350 is then again broken by commit b92143d4420f ("net: dsa:
> mv88e6xxx: add infrastructure for phylink_pcs"). This results in a dump
> on boot up:

PCS is approximately another name of a SERDES. Since there is no
SERDES, you don't don't want any of the pcs ops filled in.

Russell knows this code much better than i do. Let see what he says.

	Andrew