Skip to content

Commit 840a3cb

Browse files
yuchungchengdavem330
yuchungcheng
authored and
davem330
committed
tcp: remove forward retransmit feature
Forward retransmit is an esoteric feature in RFC3517 (condition(3) in the NextSeg()). Basically if a packet is not considered lost by the current criteria (# of dupacks etc), but the congestion window has room for more packets, then retransmit this packet. However it actually conflicts with the rest of recovery design. For example, when reordering is detected we want to be conservative in retransmitting packets but forward-retransmit feature would break that to force more retransmission. Also the implementation is fairly complicated inside the retransmission logic inducing extra iterations in the write queue. With RACK losses are being detected timely and this heuristic is no longer necessary. There this patch removes the feature. Signed-off-by: Yuchung Cheng <ycheng@google.com> Signed-off-by: Neal Cardwell <ncardwell@google.com> Acked-by: Eric Dumazet <edumazet@google.com> Signed-off-by: David S. Miller <davem@davemloft.net>
1 parent 89fe18e commit 840a3cb

File tree

3 files changed

+3
-64
lines changed

3 files changed

+3
-64
lines changed

‎include/linux/tcp.h

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -307,7 +307,6 @@ struct tcp_sock {
307307
*/
308308

309309
int lost_cnt_hint;
310-
u32 retransmit_high; /* L-bits may be on up to this seqno */
311310

312311
u32 prior_ssthresh; /* ssthresh saved at recovery start */
313312
u32 high_seq; /* snd_nxt at onset of congestion */

‎net/ipv4/tcp_input.c

Lines changed: 0 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -916,10 +916,6 @@ static void tcp_verify_retransmit_hint(struct tcp_sock *tp, struct sk_buff *skb)
916916
before(TCP_SKB_CB(skb)->seq,
917917
TCP_SKB_CB(tp->retransmit_skb_hint)->seq))
918918
tp->retransmit_skb_hint = skb;
919-
920-
if (!tp->lost_out ||
921-
after(TCP_SKB_CB(skb)->end_seq, tp->retransmit_high))
922-
tp->retransmit_high = TCP_SKB_CB(skb)->end_seq;
923919
}
924920

925921
/* Sum the number of packets on the wire we have marked as lost.
@@ -1983,7 +1979,6 @@ void tcp_enter_loss(struct sock *sk)
19831979
TCP_SKB_CB(skb)->sacked &= ~TCPCB_SACKED_ACKED;
19841980
TCP_SKB_CB(skb)->sacked |= TCPCB_LOST;
19851981
tp->lost_out += tcp_skb_pcount(skb);
1986-
tp->retransmit_high = TCP_SKB_CB(skb)->end_seq;
19871982
}
19881983
}
19891984
tcp_verify_left_out(tp);

‎net/ipv4/tcp_output.c

Lines changed: 3 additions & 58 deletions
Original file line numberDiff line numberDiff line change
@@ -2831,36 +2831,6 @@ int tcp_retransmit_skb(struct sock *sk, struct sk_buff *skb, int segs)
28312831
return err;
28322832
}
28332833

2834-
/* Check if we forward retransmits are possible in the current
2835-
* window/congestion state.
2836-
*/
2837-
static bool tcp_can_forward_retransmit(struct sock *sk)
2838-
{
2839-
const struct inet_connection_sock *icsk = inet_csk(sk);
2840-
const struct tcp_sock *tp = tcp_sk(sk);
2841-
2842-
/* Forward retransmissions are possible only during Recovery. */
2843-
if (icsk->icsk_ca_state != TCP_CA_Recovery)
2844-
return false;
2845-
2846-
/* No forward retransmissions in Reno are possible. */
2847-
if (tcp_is_reno(tp))
2848-
return false;
2849-
2850-
/* Yeah, we have to make difficult choice between forward transmission
2851-
* and retransmission... Both ways have their merits...
2852-
*
2853-
* For now we do not retransmit anything, while we have some new
2854-
* segments to send. In the other cases, follow rule 3 for
2855-
* NextSeg() specified in RFC3517.
2856-
*/
2857-
2858-
if (tcp_may_send_now(sk))
2859-
return false;
2860-
2861-
return true;
2862-
}
2863-
28642834
/* This gets called after a retransmit timeout, and the initially
28652835
* retransmitted data is acknowledged. It tries to continue
28662836
* resending the rest of the retransmit queue, until either
@@ -2875,24 +2845,16 @@ void tcp_xmit_retransmit_queue(struct sock *sk)
28752845
struct tcp_sock *tp = tcp_sk(sk);
28762846
struct sk_buff *skb;
28772847
struct sk_buff *hole = NULL;
2878-
u32 max_segs, last_lost;
2848+
u32 max_segs;
28792849
int mib_idx;
2880-
int fwd_rexmitting = 0;
28812850

28822851
if (!tp->packets_out)
28832852
return;
28842853

2885-
if (!tp->lost_out)
2886-
tp->retransmit_high = tp->snd_una;
2887-
28882854
if (tp->retransmit_skb_hint) {
28892855
skb = tp->retransmit_skb_hint;
2890-
last_lost = TCP_SKB_CB(skb)->end_seq;
2891-
if (after(last_lost, tp->retransmit_high))
2892-
last_lost = tp->retransmit_high;
28932856
} else {
28942857
skb = tcp_write_queue_head(sk);
2895-
last_lost = tp->snd_una;
28962858
}
28972859

28982860
max_segs = tcp_tso_segs(sk, tcp_current_mss(sk));
@@ -2915,31 +2877,14 @@ void tcp_xmit_retransmit_queue(struct sock *sk)
29152877
*/
29162878
segs = min_t(int, segs, max_segs);
29172879

2918-
if (fwd_rexmitting) {
2919-
begin_fwd:
2920-
if (!before(TCP_SKB_CB(skb)->seq, tcp_highest_sack_seq(tp)))
2921-
break;
2922-
mib_idx = LINUX_MIB_TCPFORWARDRETRANS;
2923-
2924-
} else if (!before(TCP_SKB_CB(skb)->seq, tp->retransmit_high)) {
2925-
tp->retransmit_high = last_lost;
2926-
if (!tcp_can_forward_retransmit(sk))
2927-
break;
2928-
/* Backtrack if necessary to non-L'ed skb */
2929-
if (hole) {
2930-
skb = hole;
2931-
hole = NULL;
2932-
}
2933-
fwd_rexmitting = 1;
2934-
goto begin_fwd;
2935-
2880+
if (tp->retrans_out >= tp->lost_out) {
2881+
break;
29362882
} else if (!(sacked & TCPCB_LOST)) {
29372883
if (!hole && !(sacked & (TCPCB_SACKED_RETRANS|TCPCB_SACKED_ACKED)))
29382884
hole = skb;
29392885
continue;
29402886

29412887
} else {
2942-
last_lost = TCP_SKB_CB(skb)->end_seq;
29432888
if (icsk->icsk_ca_state != TCP_CA_Loss)
29442889
mib_idx = LINUX_MIB_TCPFASTRETRANS;
29452890
else

0 commit comments

Comments
 (0)