[PATCH net-next 0/2] tcp: minor tail loss probe improvements

[PATCH net-next 0/2] tcp: minor tail loss probe improvements

[Yuchung Cheng]

This patch series enhance the tail loss probe (TLP) to timeout
quicker if it fails to send a probe, and tries harder to send
a probe.

Yuchung Cheng (2):
  tcp: don't extend RTO on failed loss probe attempts
  tcp: TLP retransmits last if failed to send new packet

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

-- 
2.4.3.573.g4eafbef

[PATCH net-next 1/2] tcp: don't extend RTO on failed loss probe attempts

[Yuchung Cheng]

If TLP was unable to send a probe, it extended the RTO to
now + icsk_rto. But extending the RTO makes little sense
if no TLP probe went out. With this commit, instead of
extending the RTO we re-arm it relative to the transmit time
of the write queue head.

Signed-off-by: Yuchung Cheng <ycheng@google.com>
Signed-off-by: Neal Cardwell <ncardwell@google.com>
---
 net/ipv4/tcp_output.c | 13 ++++++-------
 1 file changed, 6 insertions(+), 7 deletions(-)

diff --git a/net/ipv4/tcp_output.c b/net/ipv4/tcp_output.c
index 7105784..deb944b 100644
--- a/net/ipv4/tcp_output.c
+++ b/net/ipv4/tcp_output.c
@@ -2275,13 +2275,12 @@ void tcp_send_loss_probe(struct sock *sk)
 		tp->tlp_high_seq = tp->snd_nxt;
 
 rearm_timer:
-	inet_csk_reset_xmit_timer(sk, ICSK_TIME_RETRANS,
-				  inet_csk(sk)->icsk_rto,
-				  TCP_RTO_MAX);
-
-	if (likely(!err))
-		NET_INC_STATS_BH(sock_net(sk),
-				 LINUX_MIB_TCPLOSSPROBES);
+	if (likely(!err)) {
+		/* Reset s.t. tcp_rearm_rto will restart timer from now */
+		inet_csk(sk)->icsk_pending = 0;
+		NET_INC_STATS_BH(sock_net(sk), LINUX_MIB_TCPLOSSPROBES);
+	}
+	tcp_rearm_rto(sk);
 }
 
 /* Push out any pending frames which were held back due to
-- 
2.4.3.573.g4eafbef

[PATCH net-next 2/2] tcp: TLP retransmits last if failed to send new packet

[Yuchung Cheng]

When TLP fails to send new packet because of receive window
limit or other reasons, it should fall back to retransmit
the last packet, instead of resorting to RTO.

Signed-off-by: Yuchung Cheng <ycheng@google.com>
Signed-off-by: Neal Cardwell <ncardwell@google.com>
Signed-off-by: Nanditad Dukkipati <nanditad@google.com>
---
 net/ipv4/tcp_output.c | 14 +++++++++-----
 1 file changed, 9 insertions(+), 5 deletions(-)

diff --git a/net/ipv4/tcp_output.c b/net/ipv4/tcp_output.c
index deb944b..4f238bb 100644
--- a/net/ipv4/tcp_output.c
+++ b/net/ipv4/tcp_output.c
@@ -2226,7 +2226,7 @@ static bool skb_still_in_host_queue(const struct sock *sk,
 	return false;
 }
 
-/* When probe timeout (PTO) fires, send a new segment if one exists, else
+/* When probe timeout (PTO) fires, try send a new segment if possible, else
  * retransmit the last segment.
  */
 void tcp_send_loss_probe(struct sock *sk)
@@ -2237,9 +2237,14 @@ void tcp_send_loss_probe(struct sock *sk)
 	int mss = tcp_current_mss(sk);
 	int err = -1;
 
-	if (tcp_send_head(sk)) {
+	skb = tcp_send_head(sk);
+	if (skb) {
 		err = tcp_write_xmit(sk, mss, TCP_NAGLE_OFF, 2, GFP_ATOMIC);
-		goto rearm_timer;
+		if (!err)
+			goto rearm_timer;
+		skb = tcp_write_queue_prev(sk, skb);
+	} else {
+		skb = tcp_write_queue_tail(sk);
 	}
 
 	/* At most one outstanding TLP retransmission. */
@@ -2247,7 +2252,6 @@ void tcp_send_loss_probe(struct sock *sk)
 		goto rearm_timer;
 
 	/* Retransmit last segment. */
-	skb = tcp_write_queue_tail(sk);
 	if (WARN_ON(!skb))
 		goto rearm_timer;
 
@@ -2262,7 +2266,7 @@ void tcp_send_loss_probe(struct sock *sk)
 		if (unlikely(tcp_fragment(sk, skb, (pcount - 1) * mss, mss,
 					  GFP_ATOMIC)))
 			goto rearm_timer;
-		skb = tcp_write_queue_tail(sk);
+		skb = tcp_write_queue_next(sk, skb);
 	}
 
 	if (WARN_ON(!skb || !tcp_skb_pcount(skb)))
-- 
2.4.3.573.g4eafbef

Re: [PATCH net-next 1/2] tcp: don't extend RTO on failed loss probe attempts

[Eric Dumazet]

On Fri, 2015-07-17 at 14:22 -0700, Yuchung Cheng wrote:
> If TLP was unable to send a probe, it extended the RTO to
> now + icsk_rto. But extending the RTO makes little sense
> if no TLP probe went out. With this commit, instead of
> extending the RTO we re-arm it relative to the transmit time
> of the write queue head.

But what was the reason the probe could not be sent ?

If it is local congestion or memory allocation error, it does make sense
to not add fuel to the fire.

Re: [PATCH net-next 1/2] tcp: don't extend RTO on failed loss probe attempts

[Yuchung Cheng]

On Fri, Jul 17, 2015 at 10:27 PM, Eric Dumazet <eric.dumazet@gmail.com> wrote:
>
> On Fri, 2015-07-17 at 14:22 -0700, Yuchung Cheng wrote:
> > If TLP was unable to send a probe, it extended the RTO to
> > now + icsk_rto. But extending the RTO makes little sense
> > if no TLP probe went out. With this commit, instead of
> > extending the RTO we re-arm it relative to the transmit time
> > of the write queue head.
>
> But what was the reason the probe could not be sent ?
>
> If it is local congestion or memory allocation error, it does make sense
> to not add fuel to the fire.
Good point. We can identify those so we don't attempt to
retransmit on these errors, but will retransmit on receive-window
limit. I'll re-spin the patch.

>
>
>