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

[Lukasz Majewski]

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@xxxxxxx>
> > 
> > ---
> > 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@sama5d3-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@sama5d3-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@xxxxxxx

Attachment: pgpxjLU12MCHb.pgp
Description: OpenPGP digital signature