[PATCH net-next 29/30] net: dsa: introduce preferred_default_local_cpu_port and use on MT7530

[Arınç ÜNAL]

On 26.05.2023 20:17, Vladimir Oltean wrote:
> On Mon, May 22, 2023 at 03:15:31PM +0300, arinc9.unal at gmail.com wrote:
>> From: Vladimir Oltean <olteanv at gmail.com>
>>
>> When multiple CPU ports are being used, the numerically smallest CPU port
>> becomes the port all user ports become affine to. This may not be the best
>> choice for all switches as there may be a numerically greater CPU port with
>> more bandwidth than the numerically smallest one.
>>
>> Such switches are MT7530 and MT7531BE, which the MT7530 DSA subdriver
>> controls. Port 5 of these switches has got RGMII whilst port 6 has got
>> either TRGMII or SGMII.
>>
>> Therefore, introduce the preferred_default_local_cpu_port operation to the
>> DSA subsystem and use it on the MT7530 DSA subdriver to prefer port 6 as
>> the default CPU port.
>>
>> To prove the benefit of this operation, I (Arınç) have done a bidirectional
>> speed test between two DSA user ports on the MT7531BE switch using iperf3.
>> The user ports are 1 Gbps full duplex and on different networks so the SoC
>> MAC would have to do 2 Gbps TX and 2 Gbps RX to deliver full speed.
> 
> I think the real argument would sound like this:
> 
> Since the introduction of the OF bindings, DSA has always had a policy
> that in case multiple CPU ports are present in the device tree, the
> numerically first one is always chosen.
> 
> The MT7530 switch family has 2 CPU ports, 5 and 6, where port 6 is
> preferable because it has higher bandwidth.
> 
> The MT7530 driver developers had 3 options:
> - to modify DSA when the driver was introduced, such as to prefer the
>    better port
> - to declare both CPU ports in device trees as CPU ports, and live with
>    the sub-optimal performance resulting from not preferring the better
>    port
> - to declare just port 6 in the device tree as a CPU port
> 
> Of course they chose the path of least resistance (3rd option), kicking
> the can down the road. The hardware description in the device tree is
> supposed to be stable - developers are not supposed to adopt the
> strategy of piecemeal hardware description, where the device tree is
> updated in lockstep with the features that the kernel currently supports.
> 
> Now, as a result of the fact that they did that, any attempts to modify
> the device tree and describe both CPU ports as CPU ports would make DSA
> change its default selection from port 6 to 5, effectively resulting in
> a performance degradation visible to users as can be seen below vvvvv
> 
>>
>> Without preferring port 6:
>>
>> [ ID][Role] Interval           Transfer     Bitrate         Retr
>> [  5][TX-C]   0.00-20.00  sec   374 MBytes   157 Mbits/sec  734    sender
>> [  5][TX-C]   0.00-20.00  sec   373 MBytes   156 Mbits/sec    receiver
>> [  7][RX-C]   0.00-20.00  sec  1.81 GBytes   778 Mbits/sec    0    sender
>> [  7][RX-C]   0.00-20.00  sec  1.81 GBytes   777 Mbits/sec    receiver
>>
>> With preferring port 6:
>>
>> [ ID][Role] Interval           Transfer     Bitrate         Retr
>> [  5][TX-C]   0.00-20.00  sec  1.99 GBytes   856 Mbits/sec  273    sender
>> [  5][TX-C]   0.00-20.00  sec  1.99 GBytes   855 Mbits/sec    receiver
>> [  7][RX-C]   0.00-20.00  sec  1.72 GBytes   737 Mbits/sec   15    sender
>> [  7][RX-C]   0.00-20.00  sec  1.71 GBytes   736 Mbits/sec    receiver
>>
>> Using one port for WAN and the other ports for LAN is a very popular use
>> case which is what this test emulates.
> 
> As such, this change proposes that we retroactively modify stable
> kernels to keep the mt7530 driver preferring port 6 even with device
> trees where the hardware is more fully described.
> 
> Fixes: b8f126a8d543 ("net-next: dsa: add dsa support for Mediatek MT7530 switch")
> 
>>
>> This doesn't affect the remaining switches, MT7531AE and the switch on the
>> MT7988 SoC. Both CPU ports of the MT7531AE switch have got SGMII and there
>> is only one CPU port on the switch on the MT7988 SoC.
>>
>> Signed-off-by: Vladimir Oltean <olteanv at gmail.com>
>> Signed-off-by: Arınç ÜNAL <arinc.unal at arinc9.com>
>> ---
> 
> See the difference in intent?

Yeah, nicely put.

Arınç