Messages in this thread | | | Date | Sun, 4 Jun 2023 13:02:20 +0300 | Subject | Re: [PATCH net-next 29/30] net: dsa: introduce preferred_default_local_cpu_port and use on MT7530 | From | Arınç ÜNAL <> |
| |
On 26.05.2023 20:17, Vladimir Oltean wrote: > On Mon, May 22, 2023 at 03:15:31PM +0300, arinc9.unal@gmail.com wrote: >> From: Vladimir Oltean <olteanv@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@gmail.com> >> Signed-off-by: Arınç ÜNAL <arinc.unal@arinc9.com> >> --- > > See the difference in intent?
Yeah, nicely put.
Arınç
| |