netdev - Re: [PATCH net] net: mscc: ocelot: don't let phylink re-enable TX PAUSE on the NPI 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:   Wed, 12 Jan 2022 12:31:14 -0800
From:   Florian Fainelli <f.fainelli@...il.com>
To:     Vladimir Oltean <vladimir.oltean@....com>, netdev@...r.kernel.org
Cc:     "David S. Miller" <davem@...emloft.net>,
        Jakub Kicinski <kuba@...nel.org>, Andrew Lunn <andrew@...n.ch>,
        Vivien Didelot <vivien.didelot@...il.com>,
        Claudiu Manoil <claudiu.manoil@....com>,
        Alexandre Belloni <alexandre.belloni@...tlin.com>,
        UNGLinuxDriver@...rochip.com,
        Xiaoliang Yang <xiaoliang.yang_1@....com>
Subject: Re: [PATCH net] net: mscc: ocelot: don't let phylink re-enable TX
 PAUSE on the NPI port

On 1/12/22 12:21 PM, Vladimir Oltean wrote:
> Since commit b39648079db4 ("net: mscc: ocelot: disable flow control on
> NPI interface"), flow control should be disabled on the DSA CPU port
> when used in NPI mode.
> 
> However, the commit blamed in the Fixes: tag below broke this, because
> it allowed felix_phylink_mac_link_up() to overwrite SYS_PAUSE_CFG_PAUSE_ENA
> for the DSA CPU port.
> 
> This issue became noticeable since the device tree update from commit
> 8fcea7be5736 ("arm64: dts: ls1028a: mark internal links between Felix
> and ENETC as capable of flow control").
> 
> The solution is to check whether this is the currently configured NPI
> port from ocelot_phylink_mac_link_up(), and to not modify the statically
> disabled PAUSE frame transmission if it is.
> 
> When the port is configured for lossless mode as opposed to tail drop
> mode, but the link partner (DSA master) doesn't observe the transmitted
> PAUSE frames, the switch termination throughput is much worse, as can be
> seen below.
> 
> Before:
> 
> root@...ian:~# iperf3 -c 192.168.100.2
> Connecting to host 192.168.100.2, port 5201
> [  5] local 192.168.100.1 port 37504 connected to 192.168.100.2 port 5201
> [ ID] Interval           Transfer     Bitrate         Retr  Cwnd
> [  5]   0.00-1.00   sec  28.4 MBytes   238 Mbits/sec  357   22.6 KBytes
> [  5]   1.00-2.00   sec  33.6 MBytes   282 Mbits/sec  426   19.8 KBytes
> [  5]   2.00-3.00   sec  34.0 MBytes   285 Mbits/sec  343   21.2 KBytes
> [  5]   3.00-4.00   sec  32.9 MBytes   276 Mbits/sec  354   22.6 KBytes
> [  5]   4.00-5.00   sec  32.3 MBytes   271 Mbits/sec  297   18.4 KBytes
> ^C[  5]   5.00-5.06   sec  2.05 MBytes   270 Mbits/sec   45   19.8 KBytes
> - - - - - - - - - - - - - - - - - - - - - - - - -
> [ ID] Interval           Transfer     Bitrate         Retr
> [  5]   0.00-5.06   sec   163 MBytes   271 Mbits/sec  1822             sender
> [  5]   0.00-5.06   sec  0.00 Bytes  0.00 bits/sec                  receiver
> 
> After:
> 
> root@...ian:~# iperf3 -c 192.168.100.2
> Connecting to host 192.168.100.2, port 5201
> [  5] local 192.168.100.1 port 49470 connected to 192.168.100.2 port 5201
> [ ID] Interval           Transfer     Bitrate         Retr  Cwnd
> [  5]   0.00-1.00   sec   112 MBytes   941 Mbits/sec  259    143 KBytes
> [  5]   1.00-2.00   sec   110 MBytes   920 Mbits/sec  329    144 KBytes
> [  5]   2.00-3.00   sec   112 MBytes   936 Mbits/sec  255    144 KBytes
> [  5]   3.00-4.00   sec   110 MBytes   927 Mbits/sec  355    105 KBytes
> [  5]   4.00-5.00   sec   110 MBytes   926 Mbits/sec  350    156 KBytes
> [  5]   5.00-6.00   sec   110 MBytes   925 Mbits/sec  305    148 KBytes
> [  5]   6.00-7.00   sec   110 MBytes   924 Mbits/sec  320    143 KBytes
> [  5]   7.00-8.00   sec   110 MBytes   925 Mbits/sec  273   97.6 KBytes
> [  5]   8.00-9.00   sec   109 MBytes   913 Mbits/sec  299    141 KBytes
> [  5]   9.00-10.00  sec   110 MBytes   922 Mbits/sec  287    146 KBytes
> - - - - - - - - - - - - - - - - - - - - - - - - -
> [ ID] Interval           Transfer     Bitrate         Retr
> [  5]   0.00-10.00  sec  1.08 GBytes   926 Mbits/sec  3032             sender
> [  5]   0.00-10.00  sec  1.08 GBytes   925 Mbits/sec                  receiver

Still quite a lot of retries, do you know where they are coming from?

> 
> Fixes: de274be32cb2 ("net: dsa: felix: set TX flow control according to the phylink_mac_link_up resolution")
> Reported-by: Xiaoliang Yang <xiaoliang.yang_1@....com>
> Signed-off-by: Vladimir Oltean <vladimir.oltean@....com>

Reviewed-by: Florian Fainelli <f.fainelli@...il.com>
-- 
Florian