Re: [PATCH v3 RFC 4/4] net: dsa: hsr: Provide generic HSR ksz_hsr_{join|leave} functions

[Vladimir Oltean <olteanv@gmail.com>]

On Tue, Sep 05, 2023 at 03:47:44PM +0200, Lukasz Majewski wrote:
> > > Moreover, for me it seems more natural, that we only allow full HSR
> > > support for 2 ports or none. Please be aware, that HSR supposed to
> > > support only 2 ports, and having only one working is not
> > > recommended by vendor.  
> > 
> > And where is it enforced that full HSR offload is only applied to 2
> > ports or none?
> 
> In the ksz_jsr_join() at ksz_common.c -> we exit from this function
> when !partner.

And what about 4 or 6 ports being placed as members of 2 or 3 HSR devices?
How does the !partner condition help there?

> > Results:
> > With KSZ9477 offloading support added: RX: 100 Mbps TX: 98 Mbps
> > With no offloading                     RX: 63 Mbps  TX: 63 Mbps
> > 
> > What was the setup for the "no offloading" case?
> > 
> 
> I used two boards. I've used the same kernel with and without HSR
> offloading patches.
> 
> Cables were connected to port1 and port2 on each board. Non HSR network
> was connected to port3.
> 
> > (a) kernel did not contain the offloading patch set
> > 
> 
> Yes.

Ok, then it would be good to check that your patch set does not break
something that used to work (software HSR - easiest to check with a
second pair of ports, but if not possible, it can also be emulated by
returning -EOPNOTSUPP in .port_hsr_join on an artificial other condition).

> > (b) the testing was on hsr1, in the situation below:
> > 
> > ip link add name hsr0 type hsr slave1 lan1 slave2 lan2 supervision 45
> > version 1 # offloaded ip link add name hsr1 type hsr slave1 lan3
> > slave2 lan4 supervision 45 version 1 # unoffloaded
> 
> I did not setup two hsr devices on the same board. I've used two boards
> with hsr0 setup on each.

Ok, but can you?

> > (d) in some other way (please detail)
> > 
> > I was specifically asking about situation (b).
> 
> I did not tested the hsr0, hsr1 as my (real) use case is with KSZ9477
> having only 3 ports available for connection (port 1,2,3).

ksz_chip_data :: port_cnt is set to 7 for KSZ9477. I guess that the
limitation of having only 3 user ports for testing is specific to your
board, and not to the entire switch as may be seen on other boards,
correct?

It doesn't mean that the "single HSR device" restriction shouldn't be
enforced.