netdev - Re: [PATCH net] tcp_cubic: refactor code to perform a divide only when needed

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:   Mon, 30 Dec 2019 10:15:15 -0500
From:   Neal Cardwell <ncardwell@...gle.com>
To:     Eric Dumazet <edumazet@...gle.com>
Cc:     "David S . Miller" <davem@...emloft.net>,
        netdev <netdev@...r.kernel.org>,
        Eric Dumazet <eric.dumazet@...il.com>,
        Soheil Hassas Yeganeh <soheil@...gle.com>,
        Yuchung Cheng <ycheng@...gle.com>,
        Martin KaFai Lau <kafai@...com>
Subject: Re: [PATCH net] tcp_cubic: refactor code to perform a divide only
 when needed

On Mon, Dec 30, 2019 at 9:06 AM Eric Dumazet <edumazet@...gle.com> wrote:
>
> Neal Cardwell suggested to not change ca->delay_min
> and apply the ack delay cushion only when Hystart ACK train
> is still under consideration. This should avoid a 64bit
> divide unless needed.
>
> Tested:
>
> 40Gbit(mlx4) testbed (with sch_fq as packet scheduler)
>
> $ echo -n 'file tcp_cubic.c +p'  >/sys/kernel/debug/dynamic_debug/control
> $ nstat -n;for f in {1..10}; do ./super_netperf 1 -H lpaa24 -l -4000000; done;nstat|egrep "Hystart"
>   14815
>   16280
>   15293
>   15563
>   11574
>   15145
>   14789
>   18548
>   16972
>   12520
> TcpExtTCPHystartTrainDetect     10                 0.0
> TcpExtTCPHystartTrainCwnd       1396               0.0
> $ dmesg | tail -10
> [ 4873.951350] hystart_ack_train (116 > 93) delay_min 24 (+ ack_delay 69) cwnd 80
> [ 4875.155379] hystart_ack_train (55 > 50) delay_min 21 (+ ack_delay 29) cwnd 160
> [ 4876.333921] hystart_ack_train (69 > 62) delay_min 23 (+ ack_delay 39) cwnd 130
> [ 4877.519037] hystart_ack_train (69 > 60) delay_min 22 (+ ack_delay 38) cwnd 130
> [ 4878.701559] hystart_ack_train (87 > 63) delay_min 24 (+ ack_delay 39) cwnd 160
> [ 4879.844597] hystart_ack_train (93 > 50) delay_min 21 (+ ack_delay 29) cwnd 216
> [ 4880.956650] hystart_ack_train (74 > 67) delay_min 20 (+ ack_delay 47) cwnd 108
> [ 4882.098500] hystart_ack_train (61 > 57) delay_min 23 (+ ack_delay 34) cwnd 130
> [ 4883.262056] hystart_ack_train (72 > 67) delay_min 21 (+ ack_delay 46) cwnd 130
> [ 4884.418760] hystart_ack_train (74 > 67) delay_min 29 (+ ack_delay 38) cwnd 152
>
> 10Gbit(bnx2x) testbed (with sch_fq as packet scheduler)
>
> $ echo -n 'file tcp_cubic.c +p'  >/sys/kernel/debug/dynamic_debug/control
> $ nstat -n;for f in {1..10}; do ./super_netperf 1 -H lpk52 -l -4000000; done;nstat|egrep "Hystart"
>    7050
>    7065
>    7100
>    6900
>    7202
>    7263
>    7189
>    6869
>    7463
>    7034
> TcpExtTCPHystartTrainDetect     10                 0.0
> TcpExtTCPHystartTrainCwnd       3199               0.0
> $ dmesg | tail -10
> [  176.920012] hystart_ack_train (161 > 141) delay_min 83 (+ ack_delay 58) cwnd 264
> [  179.144645] hystart_ack_train (164 > 159) delay_min 120 (+ ack_delay 39) cwnd 444
> [  181.354527] hystart_ack_train (214 > 168) delay_min 125 (+ ack_delay 43) cwnd 436
> [  183.539565] hystart_ack_train (170 > 147) delay_min 96 (+ ack_delay 51) cwnd 326
> [  185.727309] hystart_ack_train (177 > 160) delay_min 61 (+ ack_delay 99) cwnd 128
> [  187.947142] hystart_ack_train (184 > 167) delay_min 123 (+ ack_delay 44) cwnd 367
> [  190.166680] hystart_ack_train (230 > 153) delay_min 116 (+ ack_delay 37) cwnd 444
> [  192.327285] hystart_ack_train (210 > 206) delay_min 86 (+ ack_delay 120) cwnd 152
> [  194.511392] hystart_ack_train (173 > 151) delay_min 94 (+ ack_delay 57) cwnd 239
> [  196.736023] hystart_ack_train (149 > 146) delay_min 105 (+ ack_delay 41) cwnd 399
>
> Fixes: 42f3a8aaae66 ("tcp_cubic: tweak Hystart detection for short RTT flows")
> Signed-off-by: Eric Dumazet <edumazet@...gle.com>
> Reported-by: Neal Cardwell <ncardwell@...gle.com>
> Link: https://www.spinics.net/lists/netdev/msg621886.html
> Link: https://www.spinics.net/lists/netdev/msg621797.html
> ---

Great!  Thanks, Eric.

Acked-by: Neal Cardwell <ncardwell@...gle.com>

neal