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

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]

Date:   Tue, 28 Feb 2023 12:54:58 +0300
From:   Arınç ÜNAL <arinc.unal@...nc9.com>
To:     Frank Wunderlich <frank-w@...lic-files.de>,
        Felix Fietkau <nbd@....name>
Cc:     Vladimir Oltean <olteanv@...il.com>,
        netdev <netdev@...r.kernel.org>, erkin.bozoglu@...ont.com,
        Andrew Lunn <andrew@...n.ch>,
        Florian Fainelli <f.fainelli@...il.com>,
        John Crispin <john@...ozen.org>,
        Mark Lee <Mark-MC.Lee@...iatek.com>,
        Lorenzo Bianconi <lorenzo@...nel.org>,
        Matthias Brugger <matthias.bgg@...il.com>,
        Landen Chao <Landen.Chao@...iatek.com>,
        Sean Wang <sean.wang@...iatek.com>,
        DENG Qingfang <dqfext@...il.com>
Subject: Re: Aw: Re: Re: Choose a default DSA CPU port

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@...nc9.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ç