Patch "tcp: update skb->skb_mstamp more carefully" has been added to the 4.9-stable tree

This is a note to let you know that I've just added the patch titled

    tcp: update skb->skb_mstamp more carefully

to the 4.9-stable tree which can be found at:
http://www.kernel.org/git/?p=linux/kernel/git/stable/stable-queue.git;a=summary

The filename of the patch is:
tcp-update-skb-skb_mstamp-more-carefully.patch
and it can be found in the queue-4.9 subdirectory.

If you, or anyone else, feels it should not be added to the stable tree, please let <[email protected]> know about it.

From: Eric Dumazet <[email protected]>
Date: Wed, 13 Sep 2017 20:30:39 -0700
Subject: tcp: update skb->skb_mstamp more carefully

From: Eric Dumazet <[email protected]>

[ Upstream commit 8c72c65b426b47b3c166a8fef0d8927fe5e8a28d ]

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

---
 net/ipv4/tcp_output.c |   19 ++++++++++++-------
 1 file changed, 12 insertions(+), 7 deletions(-)

@@ -914,6 +914,7 @@ static int tcp_transmit_skb(struct sock
 	struct tcp_skb_cb *tcb;
 	struct tcp_out_options opts;
 	unsigned int tcp_options_size, tcp_header_size;
+	struct sk_buff *oskb = NULL;
 	struct tcp_md5sig_key *md5;
 	struct tcphdr *th;
 	int err;

@@ -922,11 +923,11 @@ static int tcp_transmit_skb(struct sock
 	tp = tcp_sk(sk);
 
 	if (clone_it) {
-		skb_mstamp_get(&skb->skb_mstamp);
 		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

@@ -934,6 +935,7 @@ static int tcp_transmit_skb(struct sock
 		if (unlikely(!skb))
 			return -ENOBUFS;
 	}
+	skb_mstamp_get(&skb->skb_mstamp);
 
 	inet = inet_sk(sk);
 	tcb = TCP_SKB_CB(skb);

@@ -1035,12 +1037,14 @@ static int tcp_transmit_skb(struct sock
 
 	err = icsk->icsk_af_ops->queue_xmit(sk, skb, &inet->cork.fl);
 
-	if (likely(err <= 0))
-		return err;
-
-	tcp_enter_cwr(sk);
+	if (unlikely(err > 0)) {
+		tcp_enter_cwr(sk);
+		err = net_xmit_eval(err);
+	}
+	if (!err && oskb)
+		skb_mstamp_get(&oskb->skb_mstamp);
 
-	return net_xmit_eval(err);
+	return err;
 }
 
 /* This routine just queues the buffer for sending.

@@ -2709,10 +2713,11 @@ int __tcp_retransmit_skb(struct sock *sk
 		     skb_headroom(skb) >= 0xFFFF)) {
 		struct sk_buff *nskb;
 
-		skb_mstamp_get(&skb->skb_mstamp);
 		nskb = __pskb_copy(skb, MAX_TCP_HEADER, GFP_ATOMIC);
 		err = nskb ? tcp_transmit_skb(sk, nskb, 0, GFP_ATOMIC) :
 			     -ENOBUFS;
+		if (!err)
+			skb_mstamp_get(&skb->skb_mstamp);
 	} else {
 		err = tcp_transmit_skb(sk, skb, 1, GFP_ATOMIC);
 	}

Patches currently in stable-queue which might be from [email protected] are

queue-4.9/tcp-update-skb-skb_mstamp-more-carefully.patch