Re: Aw: Re: Re: Choose a default DSA CPU port

["Arınç ÜNAL" <arinc.unal@arinc9.com>]

On 26.02.2023 15:12, Frank Wunderlich wrote:
> Hi,
>> Gesendet: Samstag, 25. Februar 2023 um 20:56 Uhr
>> Von: "Arınç ÜNAL" <arinc.unal@arinc9.com>
> 
>> On 25.02.2023 19:11, Arınç ÜNAL wrote:
>>> On 25.02.2023 16:50, Frank Wunderlich wrote:
> 
>>>> f63959c7eec3151c30a2ee0d351827b62e742dcb is the first bad commit
>>>
>>> Thanks a lot for finding this. I can confirm reverting this fixes the
>>> low throughput on my Bananapi BPI-R2 as well.
> 
>> Just tested on an MT7621 Unielec U7621-06 board. MT7621 is not affected.
> 
> do you have full 1G (940 Mbit/s) on mt7621 device in 6.1??

Just tried 6.1 on MT7621. The result is similar. This SoC isn't capable 
of delivering 1 Gbps throughput anyway, unless hardware flow offloading 
is used, which I don't here.

$ iperf3 -c 192.168.2.1 -R
Connecting to host 192.168.2.1, port 5201
Reverse mode, remote host 192.168.2.1 is sending
[  5] local 192.168.2.2 port 42310 connected to 192.168.2.1 port 5201
[ ID] Interval           Transfer     Bitrate
[  5]   0.00-1.00   sec  88.6 MBytes   743 Mbits/sec
[  5]   1.00-2.00   sec  88.9 MBytes   745 Mbits/sec
[  5]   2.00-3.00   sec  90.2 MBytes   757 Mbits/sec
[  5]   3.00-4.00   sec  91.9 MBytes   771 Mbits/sec
[  5]   4.00-5.00   sec  92.0 MBytes   772 Mbits/sec
[  5]   5.00-6.00   sec  91.6 MBytes   768 Mbits/sec
[  5]   6.00-7.00   sec  91.9 MBytes   771 Mbits/sec
[  5]   7.00-8.00   sec  91.9 MBytes   771 Mbits/sec
[  5]   8.00-9.00   sec  91.8 MBytes   770 Mbits/sec
[  5]   9.00-10.00  sec  91.4 MBytes   767 Mbits/sec
- - - - - - - - - - - - - - - - - - - - - - - - -
[ ID] Interval           Transfer     Bitrate         Retr
[  5]   0.00-10.01  sec   911 MBytes   764 Mbits/sec    0             sender
[  5]   0.00-10.00  sec   910 MBytes   764 Mbits/sec 
receiver

> 
> if you look at the commit you see a special handling for mt7621
> 
> if (IS_ENABLED(CONFIG_SOC_MT7621)) {
> ...
> }else{
> //all others go there including mt7623, out (t)rgmii should be here (internally SPEED_100 afair, but higher clock for trgmii):
>                 case SPEED_1000:
>                         val |= MTK_QTX_SCH_MAX_RATE_EN |
>                                FIELD_PREP(MTK_QTX_SCH_MAX_RATE_MAN, 10) |
>                                FIELD_PREP(MTK_QTX_SCH_MAX_RATE_EXP, 5) |
>                                FIELD_PREP(MTK_QTX_SCH_MAX_RATE_WEIGHT, 10);
>                         break;
> }
> 
> but i do not understand the full code as it looks like it changes the full packet-handling ;)

Well, whatever it's doing, it doesn't hinder the performance on MT7621. ;P

> 
> imho reverting is good for test, but dropping the full change is not the right way...we should wait for felix here

Agreed, I did that to make sure nothing else on current linux-next 
affects the performance.

> 
> but back to topic...we have a patch from vladuimir which allows setting the preferred cpu-port...how do we handle mt7531 here correctly (which still sets port5 if defined and then break)?
> 
> https://git.kernel.org/pub/scm/linux/kernel/git/netdev/net-next.git/tree/drivers/net/dsa/mt7530.c#n2383
> 
> 	/* BPDU to CPU port */
> 	dsa_switch_for_each_cpu_port(cpu_dp, ds) {
> 		mt7530_rmw(priv, MT7531_CFC, MT7531_CPU_PMAP_MASK,
> 			   BIT(cpu_dp->index));
> 		break; //<<< should we drop this break only to set all "cpu-bits"? what happens then (flooding both ports with packets?)
> 	}
> 
> as dsa only handles only 1 cpu-port we want the real cpu-port (preferred | first). is this bit set also if the master is changed with your follow-up patch?

Honestly, I don't know. I'd like to leave this to you to figure out. You 
should be able to get to the bottom of this with some testing on an 
MT7531 switch. I haven't got access to one.

Arınç