linux-kernel - Re: [PATCH v2 net-next] net: dsa: Allow only up to two HSR HW offloaded ports for KSZ9477

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: <20240619171057.766c657b@wsk>
Date: Wed, 19 Jun 2024 17:10:57 +0200
From: Lukasz Majewski <lukma@...x.de>
To: Vladimir Oltean <olteanv@...il.com>
Cc: Jakub Kicinski <kuba@...nel.org>, netdev@...r.kernel.org, Paolo Abeni
 <pabeni@...hat.com>, Eric Dumazet <edumazet@...gle.com>, "David S. Miller"
 <davem@...emloft.net>, Oleksij Rempel <o.rempel@...gutronix.de>,
 Tristram.Ha@...rochip.com, Sebastian Andrzej Siewior
 <bigeasy@...utronix.de>, Simon Horman <horms@...nel.org>, Dan Carpenter
 <dan.carpenter@...aro.org>, "Ricardo B. Marliere" <ricardo@...liere.net>,
 Casper Andersson <casper.casan@...il.com>, linux-kernel@...r.kernel.org,
 Woojung Huh <woojung.huh@...rochip.com>, UNGLinuxDriver@...rochip.com,
 Andrew Lunn <andrew@...n.ch>
Subject: Re: [PATCH v2 net-next] net: dsa: Allow only up to two HSR HW
 offloaded ports for KSZ9477

Hi Vladimir,

> On Wed, Jun 19, 2024 at 03:42:48PM +0200, Lukasz Majewski wrote:
> > The KSZ9477 allows HSR in-HW offloading for any of two selected
> > ports. This patch adds check if one tries to use more than two
> > ports with HSR offloading enabled.
> > 
> > The problem is with RedBox configuration (HSR-SAN) - when
> > configuring: ip link add name hsr0 type hsr slave1 lan1 slave2 lan2
> > interlink lan3 \ supervision 45 version 1
> > 
> > The lan1 (port0) and lan2 (port1) are correctly configured as
> > ports, which can use HSR offloading on ksz9477.
> > 
> > However, when we do already have two bits set in hsr_ports, we need
> > to return (-ENOTSUPP), so the interlink port (lan3) would be used
> > with  
> 
> EOPNOTSUPP
> 
> > SW based HSR RedBox support.
> > 
> > Otherwise, I do see some strange network behavior, as some HSR
> > frames are visible on non-HSR network and vice versa.
> > 
> > This causes the switch connected to interlink port (lan3) to drop
> > frames and no communication is possible.
> > 
> > Moreover, conceptually - the interlink (i.e. HSR-SAN port -
> > lan3/port2) shall be only supported in software as it is also
> > possible to use ksz9477 with only SW based HSR (i.e. port0/1 ->
> > hsr0 with offloading, port2 -> HSR-SAN/interlink, port4/5 -> hsr1
> > with SW based HSR).
> > 
> > Fixes: 5055cccfc2d1 ("net: hsr: Provide RedBox support (HSR-SAN)")
> > Signed-off-by: Lukasz Majewski <lukma@...x.de>
> > 
> > ---
> > Changes for v2:
> > - Add more verbose description with Fixes: tag
> > - Check the condition earlier and remove extra check if SoC is
> > ksz9477
> > - Add comment in the source code file
> > ---
> >  drivers/net/dsa/microchip/ksz_common.c | 7 +++++++
> >  1 file changed, 7 insertions(+)
> > 
> > diff --git a/drivers/net/dsa/microchip/ksz_common.c
> > b/drivers/net/dsa/microchip/ksz_common.c index
> > 2818e24e2a51..72bb419e34b0 100644 ---
> > a/drivers/net/dsa/microchip/ksz_common.c +++
> > b/drivers/net/dsa/microchip/ksz_common.c @@ -3906,6 +3906,13 @@
> > static int ksz_hsr_join(struct dsa_switch *ds, int port, struct
> > net_device *hsr, return -EOPNOTSUPP; }
> >  
> > +	/* KSZ9477 can only perform HSR offloading for up to two
> > ports */
> > +	if (hweight8(dev->hsr_ports) >= 2) {
> > +		NL_SET_ERR_MSG_MOD(extack,
> > +				   "Cannot offload more than two
> > ports - use software HSR");
> > +		return -EOPNOTSUPP;
> > +	}
> > +
> >  	/* Self MAC address filtering, to avoid frames traversing
> >  	 * the HSR ring more than once.
> >  	 */
> > -- 
> > 2.20.1
> >   
> 
> How do you know you are rejecting the offloading of the interlink
> port, and not of one of the ring ports?

It seems like iproute2 is providing the correct ordering (and assures
that lan3/port2 is called as a third one - please see below).

> Basing this off of calling
> order is fragile,

In the accepted form - yes - the interlink port is called as a third
one, so it is refused to configure it in the same way as ports, which
are supporting HSR in HW (i.e. offloading).

> and does not actually reflect the hardware
> limitation.

root@...a5d3-xplained-sd:~# ip link add name hsr0 type hsr slave1 lan1
interlink lan3 slave2 lan2 supervision 45 version 1
ksz-switch spi1.0
lan1: entered promiscuous mode ksz-switch spi1.0 lan2: entered
promiscuous mode ksz-switch spi1.0 lan3: entered promiscuous mode
port: 2
Warning: ksz_switch: Cannot offload more than two ports - using
software HSR.

Gives the same output as:
root@...a5d3-xplained-sd:~# ip link add name hsr0 type hsr slave1 lan1
slave2 lan2 interlink lan3 supervision 45 version 1
ksz-switch spi1.0
lan1: entered promiscuous mode ksz-switch spi1.0 lan2: entered
promiscuous mode ksz-switch spi1.0 lan3: entered promiscuous mode
port: 2

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

Content of type "application/pgp-signature" skipped