| 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: <d3e4e88b64f8a5c5f5b16563f670e61325c891db.camel@ew.tq-group.com>
Date: Wed, 27 Mar 2024 10:42:41 +0100
From: Matthias Schiffer <matthias.schiffer@...tq-group.com>
To: Lukasz Majewski <lukma@...x.de>
Cc: Andrew Lunn <andrew@...n.ch>, Florian Fainelli <f.fainelli@...il.com>,
Vladimir Oltean <olteanv@...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,
linux-kernel@...r.kernel.org, linux@...tq-group.com, Michael Krummsdorf
<michael.krummsdorf@...group.com>
Subject: Re: [PATCH] net: dsa: mv88e6xxx: fix usable ports on 88e6020
On Tue, 2024-03-26 at 16:52 +0100, Lukasz Majewski wrote:
> Hi Matthias,
>
> > On Tue, 2024-03-26 at 14:34 +0100, Lukasz Majewski wrote:
> > > Hi Matthias,
> > >
> > > > From: Michael Krummsdorf <michael.krummsdorf@...group.com>
> > > >
> > > > The switch has 4 ports with 2 internal PHYs, but ports are
> > > > numbered up to 6, with ports 0, 1, 5 and 6 being usable.
> > > >
> > > > Fixes: 71d94a432a15 ("net: dsa: mv88e6xxx: add support for
> > > > MV88E6020 switch") Signed-off-by: Michael Krummsdorf
> > > > <michael.krummsdorf@...group.com> Signed-off-by: Matthias Schiffer
> > > > <matthias.schiffer@...tq-group.com> ---
> > > >
> > > > I was unfortunately too busy to notice the issue when the patch
> > > > this Fixes was resubmitted in my name. It would have been better
> > > > to change my From into a Based-on-patch-by or similar when
> > > > modifying it
> > >
> > > The "discussion" about this work was lasting at least a few months
> > > with several iterations and changing the design decisions ...
> > >
> > > > - and
> > > > the final version obviously wasn't even tested on an 88E6020...
> > >
> > >
> > > Can you share on which kernel version have you tested the patch that
> > > you claim that testing was omitted?
> >
> > Hi Lukasz,
> >
> > we are currently testing with commit 71d94a432a15 backported onto a
> > recent Linux 6.1.y. At least on this kernel version, the driver will
> > reject a Device Tree configuration that uses the ports 5 and 6
> > (rightfully so, as num_ports is set to 4), leaving only the
> > internal-PHY ports 0 and 1, and none of the *MII ports that are
> > likely to be used as CPU ports.
> >
> > So if the accepted version worked fine for you, your configuration
> > possibly only used the first two ports,
>
> And then the penny has dropped....
>
>
> switch@10 {
> compatible = "marvell,mv88e6250";
> reg = <0x10>;
>
> ports {
> #address-cells = <1>;
> #size-cells = <0>;
>
> port@0 {
> reg = <0>;
> label = "lan1";
> };
>
> port@1 {
> reg = <1>;
> label = "lan2";
> };
>
> port@6 {
> reg = <6>;
> label = "cpu";
> phy-mode = "rmii";
> ethernet = <&fec2>;
>
> fixed-link {
> speed = <100>;
> full-duplex;
> };
> };
> };
> };
>
>
>
> > or newer kernels somehow
> > ignore num_ports when determining if a port number is valid.
> >
> > We should be able to repeat our test on a newer kernel next week if
> > needed.
>
> I think that considering the above snippet - customer was using only
> port 0 and 1.
Hmm, I don't think that's right - the DTS has port 6 as CPU port, which should make the whole probe
of the switch fail with num_ports == 4 (`reg >= ds->num_ports` check in dsa_switch_parse_ports_of())
- that's what we saw on kernel 6.1.y with our board for CPU on port 5.
Best regards,
Matthias
> >
> > >
> > > >
> > > >
> > > > drivers/net/dsa/mv88e6xxx/chip.c | 6 +++++-
> > > > 1 file changed, 5 insertions(+), 1 deletion(-)
> > > >
> > > > diff --git a/drivers/net/dsa/mv88e6xxx/chip.c
> > > > b/drivers/net/dsa/mv88e6xxx/chip.c index
> > > > 9ed1821184ece..c95787cb90867 100644 ---
> > > > a/drivers/net/dsa/mv88e6xxx/chip.c +++
> > > > b/drivers/net/dsa/mv88e6xxx/chip.c @@ -5503,8 +5503,12 @@ static
> > > > const struct mv88e6xxx_info mv88e6xxx_table[] = { .family =
> > > > MV88E6XXX_FAMILY_6250, .name = "Marvell 88E6020",
> > > > .num_databases = 64,
> > > > - .num_ports = 4,
> > > > + /* Ports 2-4 are not routed to pins
> > > > + * => usable ports 0, 1, 5, 6
> > > > + */
> > > > + .num_ports = 7,
> > > > .num_internal_phys = 2,
> > > > + .invalid_port_mask = BIT(2) | BIT(3) | BIT(4),
> > > > .max_vid = 4095,
> > > > .port_base_addr = 0x8,
> > > > .phy_base_addr = 0x0,
> > >
> > >
> > >
> > >
> > > Best regards,
> > >
> > > Lukasz Majewski
> > >
> > > --
> > >
> > > DENX Software Engineering GmbH, Managing Director: Erika Unter
> > > HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell,
> > > Germany Phone: (+49)-8142-66989-59 Fax: (+49)-8142-66989-80 Email:
> > > lukma@...x.de Achtung externe E-Mail: Öffnen Sie Anhänge und Links
> > > nur, wenn Sie wissen, dass diese aus einer sicheren Quelle stammen
> > > und sicher sind. Leiten Sie die E-Mail im Zweifelsfall zur Prüfung
> > > an den IT-Helpdesk weiter. Attention external email: Open
> > > attachments and links only if you know that they are from a secure
> > > source and are safe. In doubt forward the email to the IT-Helpdesk
> > > to check it.
> > >
> > >
> >
>
>
>
>
> Best regards,
>
> Lukasz Majewski
>
> --
>
> DENX Software Engineering GmbH, Managing Director: Erika Unter
> HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
> Phone: (+49)-8142-66989-59 Fax: (+49)-8142-66989-80 Email: lukma@...x.de
> Achtung externe E-Mail: Öffnen Sie Anhänge und Links nur, wenn Sie wissen, dass diese aus einer sicheren Quelle stammen und sicher sind. Leiten Sie die E-Mail im Zweifelsfall zur Prüfung an den IT-Helpdesk weiter.
> Attention external email: Open attachments and links only if you know that they are from a secure source and are safe. In doubt forward the email to the IT-Helpdesk to check it.
>
>
Powered by blists - more mailing lists