From: liujian <redacted>
After the tcp socket go to ESTABLISHED stat, change IP address (server
side),
then the tcp socket will go tcp_probe_timer process.
[root@localhost net]# netstat -toe
Active Internet connections (w/o servers)
Proto Recv-Q Send-Q Local Address Foreign Address State User Inode Timer
tcp 0 1104 9.81.254:personal-agent 9.84.201.213:23597 ESTABLISHED root 12819 probe (4.36/0/7)
[root@localhost net]# cat /proc/net/tcp
sl local_address rem_address st tx_queue rx_queue tr tm->when retrnsmt uid timeout inode
3: B1FE5109:15B3 D5C95409:5C2D 01 00000495:00000000 04:0000005E 00000000 0 7 12819 2 ffff95cdcf45a000 20 4 1 10 -1
In my test case, tcp_write_queue_head(sk) and tcp_send_head(sk) is same
SKB.
And ((s32)(tcp_time_stamp(tp) - start_ts) >
jiffies_to_msecs(icsk->icsk_user_timeout))
always is false.
Here use keepalive_time_elapsed(tp) to do the compare as
tcp_keepalive_timer do.
Signed-off-by: liujian <redacted>
---
net/ipv4/tcp_timer.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
From: Eric Dumazet <hidden> Date: 2017-09-11 14:49:21
On Mon, 2017-09-11 at 14:27 +0800, liujian56@huawei.com wrote:
From: liujian <redacted>
After the tcp socket go to ESTABLISHED stat, change IP address (server
side),
then the tcp socket will go tcp_probe_timer process.
[root@localhost net]# netstat -toe
Active Internet connections (w/o servers)
Proto Recv-Q Send-Q Local Address Foreign Address State User Inode Timer
tcp 0 1104 9.81.254:personal-agent 9.84.201.213:23597 ESTABLISHED root 12819 probe (4.36/0/7)
[root@localhost net]# cat /proc/net/tcp
sl local_address rem_address st tx_queue rx_queue tr tm->when retrnsmt uid timeout inode
3: B1FE5109:15B3 D5C95409:5C2D 01 00000495:00000000 04:0000005E 00000000 0 7 12819 2 ffff95cdcf45a000 20 4 1 10 -1
In my test case, tcp_write_queue_head(sk) and tcp_send_head(sk) is same
SKB.
And ((s32)(tcp_time_stamp(tp) - start_ts) >
jiffies_to_msecs(icsk->icsk_user_timeout))
always is false.
Interesting.
Here use keepalive_time_elapsed(tp) to do the compare as
tcp_keepalive_timer do.
But zero window probe and TCP_USER_TIMEOUT can be used without
keepalives...
A packetdrill test would help, I will write one.
Thanks.
From: Eric Dumazet <hidden> Date: 2017-09-11 15:13:08
On Mon, 2017-09-11 at 07:49 -0700, Eric Dumazet wrote:
On Mon, 2017-09-11 at 14:27 +0800, liujian56@huawei.com wrote:
quoted
From: liujian <redacted>
After the tcp socket go to ESTABLISHED stat, change IP address (server
side),
then the tcp socket will go tcp_probe_timer process.
[root@localhost net]# netstat -toe
Active Internet connections (w/o servers)
Proto Recv-Q Send-Q Local Address Foreign Address State User Inode Timer
tcp 0 1104 9.81.254:personal-agent 9.84.201.213:23597 ESTABLISHED root 12819 probe (4.36/0/7)
[root@localhost net]# cat /proc/net/tcp
sl local_address rem_address st tx_queue rx_queue tr tm->when retrnsmt uid timeout inode
3: B1FE5109:15B3 D5C95409:5C2D 01 00000495:00000000 04:0000005E 00000000 0 7 12819 2 ffff95cdcf45a000 20 4 1 10 -1
In my test case, tcp_write_queue_head(sk) and tcp_send_head(sk) is same
SKB.
And ((s32)(tcp_time_stamp(tp) - start_ts) >
jiffies_to_msecs(icsk->icsk_user_timeout))
always is false.
Interesting.
quoted
Here use keepalive_time_elapsed(tp) to do the compare as
tcp_keepalive_timer do.
But zero window probe and TCP_USER_TIMEOUT can be used without
keepalives...
A packetdrill test would help, I will write one.
So existing code seems to work :
# cat window-probe-without-data-user-timeout.pkt
0 socket(..., SOCK_STREAM, IPPROTO_TCP) = 3
+0 setsockopt(3, SOL_SOCKET, SO_REUSEADDR, [1], 4) = 0
+0 bind(3, ..., ...) = 0
+0 listen(3, 1) = 0
+0 < S 0:0(0) win 0 <mss 1460,sackOK,nop,nop,nop,wscale 7>
+0 > S. 0:0(0) ack 1 <mss 1460,nop,nop,sackOK,nop,wscale 8>
// Client advertises a zero receive window, so we can't send.
+.1 < . 1:1(0) ack 1 win 0
+0 accept(3, ..., ...) = 4
+0 setsockopt(4, SOL_TCP, TCP_USER_TIMEOUT, [3000], 4) = 0
+0 write(4, ..., 2920) = 2920
// Window probes are scheduled just like RTOs.
+.3~+.31 > . 0:0(0) ack 1
+.6~+.62 > . 0:0(0) ack 1
+1.2~+1.24 > . 0:0(0) ack 1
+2.4~+2.48 > . 0:0(0) ack 1
# ./packetdrill window-probe-without-data-user_timeout.pkt
08:10:39.306137 IP 192.0.2.1.58149 > 192.168.79.31.8080: Flags [S], seq 0, win 0, options [mss 1460,sackOK,nop,nop,nop,wscale 7], length 0
08:10:39.306166 IP 192.168.79.31.8080 > 192.0.2.1.58149: Flags [S.], seq 3982794529, ack 1, win 29200, options [mss 1460,nop,nop,sackOK,nop,wscale 8], length 0
08:10:39.406296 IP 192.0.2.1.58149 > 192.168.79.31.8080: Flags [.], ack 1, win 0, length 0
08:10:39.716004 IP 192.168.79.31.8080 > 192.0.2.1.58149: Flags [.], ack 1, win 115, length 0
08:10:40.327133 IP 192.168.79.31.8080 > 192.0.2.1.58149: Flags [.], ack 1, win 115, length 0
08:10:41.540243 IP 192.168.79.31.8080 > 192.0.2.1.58149: Flags [.], ack 1, win 115, length 0
You can see we got only 3 probes, not 4.
This has nothing to do with the code patch you have changed.
How have you tested your patch exactly ?
[root@localhost ~]# time ./gtests/net/packetdrill/packetdrill test.pkt
test.pkt:50: runtime error in write call: Expected result -1 but got 24 with errno 2 (No such file or directory)
real 1m11.364s
user 0m0.028s
sys 0m0.106s
[root@localhost ~]# netstat -toen
Active Internet connections (w/o servers)
Proto Recv-Q Send-Q Local Address Foreign Address State User Inode Timer
tcp 0 504 192.168.0.1:8080 192.0.2.1:33993 ESTABLISHED 0 45453 probe (22.38/0/7)
since the script didn't wait for enough time, here only got 7 probes.
在 2017/9/11 23:22, Eric Dumazet 写道:
quoted
On Mon, 2017-09-11 at 08:13 -0700, Eric Dumazet wrote:
quoted
You can see we got only 3 probes, not 4.
Here is complete packetdrill test showing that code behaves as expected.
0 socket(..., SOCK_STREAM, IPPROTO_TCP) = 3
+0 setsockopt(3, SOL_SOCKET, SO_REUSEADDR, [1], 4) = 0
+0 bind(3, ..., ...) = 0
+0 listen(3, 1) = 0
+0 < S 0:0(0) win 0 <mss 1460,sackOK,nop,nop,nop,wscale 7>
+0 > S. 0:0(0) ack 1 <mss 1460,nop,nop,sackOK,nop,wscale 8>
// Client advertises a zero receive window, so we can't send.
+.1 < . 1:1(0) ack 1 win 0
+0 accept(3, ..., ...) = 4
+0 setsockopt(4, SOL_TCP, TCP_USER_TIMEOUT, [3000], 4) = 0
+0 write(4, ..., 2920) = 2920
// Window probes are scheduled just like RTOs.
+.3~+.31 > . 0:0(0) ack 1
+.6~+.62 > . 0:0(0) ack 1
+1.2~+1.24 > . 0:0(0) ack 1
// Peer opens its window too late !
+3 < . 1:1(0) ack 1 win 1000
+0 > R 1:1(0)
.
This has nothing to do with the code patch you have changed.
How have you tested your patch exactly ?
I tested the patch, it can work.
[root@localhost ~]# time ./gtests/net/packetdrill/packetdrill test.pkt
test.pkt:24: runtime error in write call: Expected result 24 but got -1 with errno 110 (Connection timed out)
real 0m5.356s
user 0m0.026s
sys 0m0.104s
[root@localhost ~]# ss -toenmi src :8080
State Recv-Q Send-Q Local Address:Port Peer Address:Port
ESTAB 0 48 192.168.0.1:8080 192.0.2.1:39559 timer:(persist,186ms,2) ino:37178 sk:6 <->
skmem:(r0,rb369280,t0,tb87040,f1792,w2304,o0,bl0) sack cubic wscale:7,7 rto:301 backoff:2 rtt:100.253/37.643 mss:1460 cwnd:10 bytes_acked:24 segs_out:5 segs_in:3 send 1.2Mbps lastsnd:976 lastrcv:4082 lastack:3982 pacing_rate 2.3Mbps rcv_space:29200
if change the TCP_USER_TIMEOUT to 30s, test result as below:
[root@localhost ~]# time ./gtests/net/packetdrill/packetdrill test.pkt
test.pkt:37: runtime error in write call: Expected result 24 but got -1 with errno 110 (Connection timed out)
real 0m44.362s
user 0m0.018s
sys 0m0.110s
ESTAB 0 360 192.168.0.1:8080 192.0.2.1:47577 timer:(persist,516ms,6) ino:18806 sk:5 <->
skmem:(r0,rb369280,t0,tb87040,f1792,w2304,o0,bl0) sack cubic wscale:7,7 rto:301 backoff:6 rtt:100.228/37.623 mss:10 bytes_acked:24 segs_out:22 segs_in:3 send 1.2Mbps lastsnd:2343 lastrcv:40450 lastack:40350 pacing_rate 2.3Mbps rcv_sp
lpaa23:~# ss -toenmi src :8080
State Recv-Q Send-Q Local Address:Port Peer
Address:Port
ESTAB 0 144 192.168.134.161:8080
192.0.2.1:51165 timer:(persist,8.262ms,5) ino:1
82083 sk:3 <->
skmem:(r0,rb359040,t0,tb46080,f1792,w2304,o0,bl0,d0) sack cubic
wscale:7,8 rto:301 backoff:5 rtt:100.127/37.576
mss:1460 rcvmss:536 advmss:1460 cwnd:10 bytes_acked:24 segs_out:12
segs_in:3 data_segs_out:12 send 1.2Mbps lastsnd:1370 l
astrcv:13348 lastack:13248 pacing_rate 2.3Mbps delivery_rate 116.7Kbps
app_limited busy:11346ms rcv_space:29200 notsent:1
44 minrtt:100.043userspace application still send data continuously
This is the typical RTO timer, not zero window probe.
with the script, it is not zero window; but the code enter tcp_probe_timer.
->tcp_sendmsg
-->tcp_push
---->__tcp_push_pending_frames
------>tcp_write_xmit
-------->tcp_transmit_skb (at this function, tcp_send_head(sk)'s timestamp was be refreshed)
---------->ip_queue_xmit (can not find route, return -EHOSTUNREACH)
-------->tcp_transmit_skb (send fail, do not advance send head)
------->tcp_check_probe_timer (reset ICSK_TIME_PROBE0 timer)
Here userspace application still send data continuously,
it triggered ICSK_TIME_PROBE0 timer and tcp_send_head(sk)'s timestamp refresh.
This has nothing to do with the code patch you have changed.
How have you tested your patch exactly ?
I tested the patch, it can work.
[root@localhost ~]# time ./gtests/net/packetdrill/packetdrill test.pkt
test.pkt:24: runtime error in write call: Expected result 24 but got -1 with errno 110 (Connection timed out)
real 0m5.356s
user 0m0.026s
sys 0m0.104s
[root@localhost ~]# ss -toenmi src :8080
State Recv-Q Send-Q Local Address:Port Peer Address:Port
ESTAB 0 48 192.168.0.1:8080 192.0.2.1:39559 timer:(persist,186ms,2) ino:37178 sk:6 <->
skmem:(r0,rb369280,t0,tb87040,f1792,w2304,o0,bl0) sack cubic wscale:7,7 rto:301 backoff:2 rtt:100.253/37.643 mss:1460 cwnd:10 bytes_acked:24 segs_out:5 segs_in:3 send 1.2Mbps lastsnd:976 lastrcv:4082 lastack:3982 pacing_rate 2.3Mbps rcv_space:29200
if change the TCP_USER_TIMEOUT to 30s, test result as below:
[root@localhost ~]# time ./gtests/net/packetdrill/packetdrill test.pkt
test.pkt:37: runtime error in write call: Expected result 24 but got -1 with errno 110 (Connection timed out)
real 0m44.362s
user 0m0.018s
sys 0m0.110s
ESTAB 0 360 192.168.0.1:8080 192.0.2.1:47577 timer:(persist,516ms,6) ino:18806 sk:5 <->
skmem:(r0,rb369280,t0,tb87040,f1792,w2304,o0,bl0) sack cubic wscale:7,7 rto:301 backoff:6 rtt:100.228/37.623 mss:10 bytes_acked:24 segs_out:22 segs_in:3 send 1.2Mbps lastsnd:2343 lastrcv:40450 lastack:40350 pacing_rate 2.3Mbps rcv_sp
quoted
lpaa23:~# ss -toenmi src :8080
State Recv-Q Send-Q Local Address:Port Peer
Address:Port
ESTAB 0 144 192.168.134.161:8080
192.0.2.1:51165 timer:(persist,8.262ms,5) ino:1
82083 sk:3 <->
skmem:(r0,rb359040,t0,tb46080,f1792,w2304,o0,bl0,d0) sack cubic
wscale:7,8 rto:301 backoff:5 rtt:100.127/37.576
mss:1460 rcvmss:536 advmss:1460 cwnd:10 bytes_acked:24 segs_out:12
segs_in:3 data_segs_out:12 send 1.2Mbps lastsnd:1370 l
astrcv:13348 lastack:13248 pacing_rate 2.3Mbps delivery_rate 116.7Kbps
app_limited busy:11346ms rcv_space:29200 notsent:1
44 minrtt:100.043userspace application still send data continuously
This is the typical RTO timer, not zero window probe.
with the script, it is not zero window; but the code enter tcp_probe_timer.
->tcp_sendmsg
-->tcp_push
---->__tcp_push_pending_frames
------>tcp_write_xmit
-------->tcp_transmit_skb (at this function, tcp_send_head(sk)'s timestamp was be refreshed)
---------->ip_queue_xmit (can not find route, return -EHOSTUNREACH)
-------->tcp_transmit_skb (send fail, do not advance send head)
------->tcp_check_probe_timer (reset ICSK_TIME_PROBE0 timer)
sorry, change to
->tcp_sendmsg
-->tcp_push
---->__tcp_push_pending_frames
------>tcp_write_xmit
-------->tcp_transmit_skb (at this function, tcp_send_head(sk)'s timestamp was be refreshed)
---------->ip_queue_xmit (can not find route, return -EHOSTUNREACH)
------>tcp_write_xmit (send fail, do not advance send head)
------>tcp_check_probe_timer (reset ICSK_TIME_PROBE0 timer)
Here userspace application still send data continuously,
it triggered ICSK_TIME_PROBE0 timer and tcp_send_head(sk)'s timestamp refresh.
This has nothing to do with the code patch you have changed.
How have you tested your patch exactly ?
I tested the patch, it can work.
[root@localhost ~]# time ./gtests/net/packetdrill/packetdrill test.pkt
test.pkt:24: runtime error in write call: Expected result 24 but got -1 with errno 110 (Connection timed out)
real 0m5.356s
user 0m0.026s
sys 0m0.104s
[root@localhost ~]# ss -toenmi src :8080
State Recv-Q Send-Q Local Address:Port Peer Address:Port
ESTAB 0 48 192.168.0.1:8080 192.0.2.1:39559 timer:(persist,186ms,2) ino:37178 sk:6 <->
skmem:(r0,rb369280,t0,tb87040,f1792,w2304,o0,bl0) sack cubic wscale:7,7 rto:301 backoff:2 rtt:100.253/37.643 mss:1460 cwnd:10 bytes_acked:24 segs_out:5 segs_in:3 send 1.2Mbps lastsnd:976 lastrcv:4082 lastack:3982 pacing_rate 2.3Mbps rcv_space:29200
if change the TCP_USER_TIMEOUT to 30s, test result as below:
[root@localhost ~]# time ./gtests/net/packetdrill/packetdrill test.pkt
test.pkt:37: runtime error in write call: Expected result 24 but got -1 with errno 110 (Connection timed out)
real 0m44.362s
user 0m0.018s
sys 0m0.110s
ESTAB 0 360 192.168.0.1:8080 192.0.2.1:47577 timer:(persist,516ms,6) ino:18806 sk:5 <->
skmem:(r0,rb369280,t0,tb87040,f1792,w2304,o0,bl0) sack cubic wscale:7,7 rto:301 backoff:6 rtt:100.228/37.623 mss:10 bytes_acked:24 segs_out:22 segs_in:3 send 1.2Mbps lastsnd:2343 lastrcv:40450 lastack:40350 pacing_rate 2.3Mbps rcv_sp
quoted
lpaa23:~# ss -toenmi src :8080
State Recv-Q Send-Q Local Address:Port Peer
Address:Port
ESTAB 0 144 192.168.134.161:8080
192.0.2.1:51165 timer:(persist,8.262ms,5) ino:1
82083 sk:3 <->
skmem:(r0,rb359040,t0,tb46080,f1792,w2304,o0,bl0,d0) sack cubic
wscale:7,8 rto:301 backoff:5 rtt:100.127/37.576
mss:1460 rcvmss:536 advmss:1460 cwnd:10 bytes_acked:24 segs_out:12
segs_in:3 data_segs_out:12 send 1.2Mbps lastsnd:1370 l
astrcv:13348 lastack:13248 pacing_rate 2.3Mbps delivery_rate 116.7Kbps
app_limited busy:11346ms rcv_space:29200 notsent:1
44 minrtt:100.043userspace application still send data continuously
This is the typical RTO timer, not zero window probe.
with the script, it is not zero window; but the code enter tcp_probe_timer.
->tcp_sendmsg
-->tcp_push
---->__tcp_push_pending_frames
------>tcp_write_xmit
-------->tcp_transmit_skb (at this function, tcp_send_head(sk)'s timestamp was be refreshed)
---------->ip_queue_xmit (can not find route, return -EHOSTUNREACH)
-------->tcp_transmit_skb (send fail, do not advance send head)
------->tcp_check_probe_timer (reset ICSK_TIME_PROBE0 timer)
sorry, change to
->tcp_sendmsg
-->tcp_push
---->__tcp_push_pending_frames
------>tcp_write_xmit
-------->tcp_transmit_skb (at this function, tcp_send_head(sk)'s timestamp was be refreshed)
---------->ip_queue_xmit (can not find route, return -EHOSTUNREACH)
------>tcp_write_xmit (send fail, do not advance send head)
------>tcp_check_probe_timer (reset ICSK_TIME_PROBE0 timer)
OK but :
1) keepalive_time_elapsed() returns jiffies, and you compare it to a ms
value. It will break on kernels with HZ=100
2) keepalive_time_elapsed() is reset every time a packet is received.
With following packetdrill your patch (even if we fix 1)) breaks again
# ./packetdrill --local_ip 192.168.102.64 user_timeout.pkt
# cat user_timeout.pkt
0 socket(..., SOCK_STREAM, IPPROTO_TCP) = 3
+0 setsockopt(3, SOL_SOCKET, SO_REUSEADDR, [1], 4) = 0
+0 bind(3, ..., ...) = 0
+0 listen(3, 1) = 0
+0 `ifconfig tun0 192.168.102.64/16; ip ro add 192.0.2.1 dev tun0`
+0 < S 0:0(0) win 0 <mss 1460,sackOK,nop,nop,nop,wscale 7>
+0 > S. 0:0(0) ack 1 <mss 1460,nop,nop,sackOK,nop,wscale 8>
+.1 < . 1:1(0) ack 1 win 65530
+0 accept(3, ..., ...) = 4
+0 setsockopt(4, SOL_TCP, TCP_USER_TIMEOUT, [3000], 4) = 0
+0 write(4, ..., 24) = 24
+0 > P. 1:25(24) ack 1 win 115
+.1 < . 1:1(0) ack 25 win 65530
//change the ipaddress
+1 `ifconfig tun0 192.168.0.10/16`
+1 write(4, ..., 24) = 24
+1 write(4, ..., 24) = 24
+1 write(4, ..., 24) = 24
+1 write(4, ..., 24) = 24
+0 `ifconfig tun0 192.168.102.64/16`
+0 < . 1:2(1) ack 25 win 65530
+0 `ifconfig tun0 192.168.0.10/16`
+3 write(4, ..., 24) = 24
+3 write(4, ..., 24) = 24
+3 write(4, ..., 24) = 24
+3 write(4, ..., 24) = 24
+3 write(4, ..., 24) = 24
+3 write(4, ..., 24) = 24
+3 write(4, ..., 24) = 24
+3 write(4, ..., 24) = 24
+3 write(4, ..., 24) = 24
+3 write(4, ..., 24) = 24
+3 write(4, ..., 24) = 24
+3 write(4, ..., 24) = 24
+3 write(4, ..., 24) = 24
+3 write(4, ..., 24) = 24
+3 write(4, ..., 24) = 24
+3 write(4, ..., 24) = 24
+3 write(4, ..., 24) = 24
+3 write(4, ..., 24) = 24
+3 write(4, ..., 24) = 24
+3 write(4, ..., 24) = 24
+3 write(4, ..., 24) = 24
+3 write(4, ..., 24) = 24
+0 `ifconfig tun0 192.168.0.1/16`
+0 < . 1:1(0) ack 1 win 1000
+0 write(4, ..., 24) = -1
I would instead try to keep a meaningful value in
tcp_send_head(sk)->skb_mstamp if the transmit failed.
Or more exactly change skb->skb_mstamp only on (successfully)
transmitted skb.
I will provide a patch ( fixing net/ipv4/tcp_output.c )
From: Eric Dumazet <hidden> Date: 2017-09-14 03:30:42
From: Eric Dumazet <redacted>
liujian reported a problem in TCP_USER_TIMEOUT processing with a patch
in tcp_probe_timer() :
https://www.spinics.net/lists/netdev/msg454496.html
After investigations, the root cause of the problem is that we update
skb->skb_mstamp of skbs in write queue, even if the attempt to send a
clone or copy of it failed. One reason being a routing problem.
This patch prevents this, solving liujian issue.
It also removes a potential RTT miscalculation, since
__tcp_retransmit_skb() is not OR-ing TCP_SKB_CB(skb)->sacked with
TCPCB_EVER_RETRANS if a failure happens, but skb->skb_mstamp has
been changed.
A future ACK would then lead to a very small RTT sample and min_rtt
would then be lowered to this too small value.
Tested:
# cat user_timeout.pkt
--local_ip=192.168.102.64
0 socket(..., SOCK_STREAM, IPPROTO_TCP) = 3
+0 setsockopt(3, SOL_SOCKET, SO_REUSEADDR, [1], 4) = 0
+0 bind(3, ..., ...) = 0
+0 listen(3, 1) = 0
+0 `ifconfig tun0 192.168.102.64/16; ip ro add 192.0.2.1 dev tun0`
+0 < S 0:0(0) win 0 <mss 1460>
+0 > S. 0:0(0) ack 1 <mss 1460>
+.1 < . 1:1(0) ack 1 win 65530
+0 accept(3, ..., ...) = 4
+0 setsockopt(4, SOL_TCP, TCP_USER_TIMEOUT, [3000], 4) = 0
+0 write(4, ..., 24) = 24
+0 > P. 1:25(24) ack 1 win 29200
+.1 < . 1:1(0) ack 25 win 65530
//change the ipaddress
+1 `ifconfig tun0 192.168.0.10/16`
+1 write(4, ..., 24) = 24
+1 write(4, ..., 24) = 24
+1 write(4, ..., 24) = 24
+1 write(4, ..., 24) = 24
+0 `ifconfig tun0 192.168.102.64/16`
+0 < . 1:2(1) ack 25 win 65530
+0 `ifconfig tun0 192.168.0.10/16`
+3 write(4, ..., 24) = -1
# ./packetdrill user_timeout.pkt
Signed-off-by: Eric Dumazet <redacted>
Reported-by: liujian <redacted>
---
net/ipv4/tcp_output.c | 19 ++++++++++++-------
1 file changed, 12 insertions(+), 7 deletions(-)
@@ -991,6 +991,7 @@ static int tcp_transmit_skb(struct sock *sk, struct sk_buff *skb, int clone_it,structtcp_skb_cb*tcb;structtcp_out_optionsopts;unsignedinttcp_options_size,tcp_header_size;+structsk_buff*oskb=NULL;structtcp_md5sig_key*md5;structtcphdr*th;interr;
@@ -998,12 +999,12 @@ static int tcp_transmit_skb(struct sock *sk, struct sk_buff *skb, int clone_it,BUG_ON(!skb||!tcp_skb_pcount(skb));tp=tcp_sk(sk);-skb->skb_mstamp=tp->tcp_mstamp;if(clone_it){TCP_SKB_CB(skb)->tx.in_flight=TCP_SKB_CB(skb)->end_seq-tp->snd_una;tcp_rate_skb_sent(sk,skb);+oskb=skb;if(unlikely(skb_cloned(skb)))skb=pskb_copy(skb,gfp_mask);else
@@ -1011,6 +1012,7 @@ static int tcp_transmit_skb(struct sock *sk, struct sk_buff *skb, int clone_it,if(unlikely(!skb))return-ENOBUFS;}+skb->skb_mstamp=tp->tcp_mstamp;inet=inet_sk(sk);tcb=TCP_SKB_CB(skb);
@@ -1122,12 +1124,14 @@ static int tcp_transmit_skb(struct sock *sk, struct sk_buff *skb, int clone_it,err=icsk->icsk_af_ops->queue_xmit(sk,skb,&inet->cork.fl);-if(likely(err<=0))-returnerr;--tcp_enter_cwr(sk);+if(unlikely(err>0)){+tcp_enter_cwr(sk);+err=net_xmit_eval(err);+}+if(!err&&oskb)+oskb->skb_mstamp=tp->tcp_mstamp;-returnnet_xmit_eval(err);+returnerr;}/* This routine just queues the buffer for sending.
@@ -2869,10 +2873,11 @@ int __tcp_retransmit_skb(struct sock *sk, struct sk_buff *skb, int segs)skb_headroom(skb)>=0xFFFF)){structsk_buff*nskb;-skb->skb_mstamp=tp->tcp_mstamp;nskb=__pskb_copy(skb,MAX_TCP_HEADER,GFP_ATOMIC);err=nskb?tcp_transmit_skb(sk,nskb,0,GFP_ATOMIC):-ENOBUFS;+if(!err)+skb->skb_mstamp=tp->tcp_mstamp;}else{err=tcp_transmit_skb(sk,skb,1,GFP_ATOMIC);}
From: Eric Dumazet <redacted>
liujian reported a problem in TCP_USER_TIMEOUT processing with a patch
in tcp_probe_timer() :
https://www.spinics.net/lists/netdev/msg454496.html
After investigations, the root cause of the problem is that we update
skb->skb_mstamp of skbs in write queue, even if the attempt to send a
clone or copy of it failed. One reason being a routing problem.
This patch prevents this, solving liujian issue.
It also removes a potential RTT miscalculation, since
__tcp_retransmit_skb() is not OR-ing TCP_SKB_CB(skb)->sacked with
TCPCB_EVER_RETRANS if a failure happens, but skb->skb_mstamp has
been changed.
A future ACK would then lead to a very small RTT sample and min_rtt
would then be lowered to this too small value.
On Wed, Sep 13, 2017 at 11:30 PM, Eric Dumazet [off-list ref] wrote:
From: Eric Dumazet <redacted>
liujian reported a problem in TCP_USER_TIMEOUT processing with a patch
in tcp_probe_timer() :
https://www.spinics.net/lists/netdev/msg454496.html
After investigations, the root cause of the problem is that we update
skb->skb_mstamp of skbs in write queue, even if the attempt to send a
clone or copy of it failed. One reason being a routing problem.
This patch prevents this, solving liujian issue.
It also removes a potential RTT miscalculation, since
__tcp_retransmit_skb() is not OR-ing TCP_SKB_CB(skb)->sacked with
TCPCB_EVER_RETRANS if a failure happens, but skb->skb_mstamp has
been changed.
A future ACK would then lead to a very small RTT sample and min_rtt
would then be lowered to this too small value.
On Thu, Sep 14, 2017 at 6:57 AM, Neal Cardwell [off-list ref] wrote:
On Wed, Sep 13, 2017 at 11:30 PM, Eric Dumazet [off-list ref] wrote:
quoted
From: Eric Dumazet <redacted>
liujian reported a problem in TCP_USER_TIMEOUT processing with a patch
in tcp_probe_timer() :
https://www.spinics.net/lists/netdev/msg454496.html
After investigations, the root cause of the problem is that we update
skb->skb_mstamp of skbs in write queue, even if the attempt to send a
clone or copy of it failed. One reason being a routing problem.
This patch prevents this, solving liujian issue.
It also removes a potential RTT miscalculation, since
__tcp_retransmit_skb() is not OR-ing TCP_SKB_CB(skb)->sacked with
TCPCB_EVER_RETRANS if a failure happens, but skb->skb_mstamp has
been changed.
A future ACK would then lead to a very small RTT sample and min_rtt
would then be lowered to this too small value.
On Thu, Sep 14, 2017 at 12:32 PM, Yuchung Cheng [off-list ref] wrote:
On Thu, Sep 14, 2017 at 6:57 AM, Neal Cardwell [off-list ref] wrote:
quoted
On Wed, Sep 13, 2017 at 11:30 PM, Eric Dumazet [off-list ref] wrote:
quoted
From: Eric Dumazet <redacted>
liujian reported a problem in TCP_USER_TIMEOUT processing with a patch
in tcp_probe_timer() :
https://www.spinics.net/lists/netdev/msg454496.html
After investigations, the root cause of the problem is that we update
skb->skb_mstamp of skbs in write queue, even if the attempt to send a
clone or copy of it failed. One reason being a routing problem.
This patch prevents this, solving liujian issue.
It also removes a potential RTT miscalculation, since
__tcp_retransmit_skb() is not OR-ing TCP_SKB_CB(skb)->sacked with
TCPCB_EVER_RETRANS if a failure happens, but skb->skb_mstamp has
been changed.
A future ACK would then lead to a very small RTT sample and min_rtt
would then be lowered to this too small value.
From: Eric Dumazet <redacted>
liujian reported a problem in TCP_USER_TIMEOUT processing with a patch
in tcp_probe_timer() :
https://www.spinics.net/lists/netdev/msg454496.html
After investigations, the root cause of the problem is that we update
skb->skb_mstamp of skbs in write queue, even if the attempt to send a
clone or copy of it failed. One reason being a routing problem.
This patch prevents this, solving liujian issue.
It also removes a potential RTT miscalculation, since
__tcp_retransmit_skb() is not OR-ing TCP_SKB_CB(skb)->sacked with
TCPCB_EVER_RETRANS if a failure happens, but skb->skb_mstamp has
been changed.
A future ACK would then lead to a very small RTT sample and min_rtt
would then be lowered to this too small value.
Tested:
...
Signed-off-by: Eric Dumazet <redacted>
Reported-by: liujian <redacted>
From: Eric Dumazet <redacted>
liujian reported a problem in TCP_USER_TIMEOUT processing with a patch
in tcp_probe_timer() :
https://www.spinics.net/lists/netdev/msg454496.html
After investigations, the root cause of the problem is that we update
skb->skb_mstamp of skbs in write queue, even if the attempt to send a
clone or copy of it failed. One reason being a routing problem.
This patch prevents this, solving liujian issue.
It also removes a potential RTT miscalculation, since
__tcp_retransmit_skb() is not OR-ing TCP_SKB_CB(skb)->sacked with
TCPCB_EVER_RETRANS if a failure happens, but skb->skb_mstamp has
been changed.
A future ACK would then lead to a very small RTT sample and min_rtt
would then be lowered to this too small value.
Tested:
...
quoted
Signed-off-by: Eric Dumazet <redacted>
Reported-by: liujian <redacted>
Applied and queued up for -stable, thanks.
Thanks David.
It seems I forgot to move the call to tcp_rate_skb_sent(sk, skb),
as this function depended on skb->skb_mstamp being up to date.
I will send a fix, sorry for not catching this earlier.
From: Eric Dumazet <hidden> Date: 2017-09-15 23:47:44
From: Eric Dumazet <edumazet@google.com>
Now skb->mstamp_skb is updated later, we also need to call
tcp_rate_skb_sent() after the update is done.
Fixes: 8c72c65b426b ("tcp: update skb->skb_mstamp more carefully")
Signed-off-by: Eric Dumazet <edumazet@google.com>
---
net/ipv4/tcp_output.c | 7 +++----
1 file changed, 3 insertions(+), 4 deletions(-)
On Fri, Sep 15, 2017 at 7:47 PM, Eric Dumazet [off-list ref] wrote:
From: Eric Dumazet <edumazet@google.com>
Now skb->mstamp_skb is updated later, we also need to call
tcp_rate_skb_sent() after the update is done.
Fixes: 8c72c65b426b ("tcp: update skb->skb_mstamp more carefully")
Signed-off-by: Eric Dumazet <edumazet@google.com>
From: Eric Dumazet <edumazet@google.com>
Now skb->mstamp_skb is updated later, we also need to call
tcp_rate_skb_sent() after the update is done.
Fixes: 8c72c65b426b ("tcp: update skb->skb_mstamp more carefully")
Signed-off-by: Eric Dumazet <edumazet@google.com>