| Submitter | Yuchung Cheng |
|---|---|
| Date | Dec. 6, 2012, 6:45 p.m. |
| Message ID | <1354819532-15676-1-git-send-email-ycheng@google.com> |
| Download | mbox | patch |
| Permalink | /patch/204300/ |
| State | Accepted |
| Delegated to: | David Miller |
| Headers | show |
Comments
On Thu, Dec 6, 2012 at 1:45 PM, Yuchung Cheng <ycheng@google.com> wrote: > If SYN-ACK partially acks SYN-data, the client retransmits the > remaining data by tcp_retransmit_skb(). This increments lost recovery > state variables like tp->retrans_out in Open state. If loss recovery > happens before the retransmission is acked, it triggers the WARN_ON > check in tcp_fastretrans_alert(). For example: the client sends > SYN-data, gets SYN-ACK acking only ISN, retransmits data, sends > another 4 data packets and get 3 dupacks. > > Since the retransmission is not caused by network drop it should not > update the recovery state variables. Further the server may return a > smaller MSS than the cached MSS used for SYN-data, so the retranmission > needs a loop. Otherwise some data will not be retransmitted until timeout > or other loss recovery events. > > Signed-off-by: Yuchung Cheng <ycheng@google.com> > --- Acked-by: Neal Cardwell <ncardwell@google.com> -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
From: Neal Cardwell <ncardwell@google.com> Date: Thu, 6 Dec 2012 14:50:58 -0500 > On Thu, Dec 6, 2012 at 1:45 PM, Yuchung Cheng <ycheng@google.com> wrote: >> If SYN-ACK partially acks SYN-data, the client retransmits the >> remaining data by tcp_retransmit_skb(). This increments lost recovery >> state variables like tp->retrans_out in Open state. If loss recovery >> happens before the retransmission is acked, it triggers the WARN_ON >> check in tcp_fastretrans_alert(). For example: the client sends >> SYN-data, gets SYN-ACK acking only ISN, retransmits data, sends >> another 4 data packets and get 3 dupacks. >> >> Since the retransmission is not caused by network drop it should not >> update the recovery state variables. Further the server may return a >> smaller MSS than the cached MSS used for SYN-data, so the retranmission >> needs a loop. Otherwise some data will not be retransmitted until timeout >> or other loss recovery events. >> >> Signed-off-by: Yuchung Cheng <ycheng@google.com> >> --- > > Acked-by: Neal Cardwell <ncardwell@google.com> Applied. -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Patch
diff --git a/include/net/tcp.h b/include/net/tcp.h index 3202bde..aed42c7 100644 --- a/include/net/tcp.h +++ b/include/net/tcp.h @@ -524,6 +524,7 @@ static inline __u32 cookie_v6_init_sequence(struct sock *sk, extern void __tcp_push_pending_frames(struct sock *sk, unsigned int cur_mss, int nonagle); extern bool tcp_may_send_now(struct sock *sk); +extern int __tcp_retransmit_skb(struct sock *, struct sk_buff *); extern int tcp_retransmit_skb(struct sock *, struct sk_buff *); extern void tcp_retransmit_timer(struct sock *sk); extern void tcp_xmit_retransmit_queue(struct sock *); diff --git a/net/ipv4/tcp_input.c b/net/ipv4/tcp_input.c index fc67831..a136925 100644 --- a/net/ipv4/tcp_input.c +++ b/net/ipv4/tcp_input.c @@ -5652,7 +5652,11 @@ static bool tcp_rcv_fastopen_synack(struct sock *sk, struct sk_buff *synack, tcp_fastopen_cache_set(sk, mss, cookie, syn_drop); if (data) { /* Retransmit unacked data in SYN */ - tcp_retransmit_skb(sk, data); + tcp_for_write_queue_from(data, sk) { + if (data == tcp_send_head(sk) || + __tcp_retransmit_skb(sk, data)) + break; + } tcp_rearm_rto(sk); return true; } diff --git a/net/ipv4/tcp_output.c b/net/ipv4/tcp_output.c index 8ac0855..5d45159 100644 --- a/net/ipv4/tcp_output.c +++ b/net/ipv4/tcp_output.c @@ -2309,12 +2309,11 @@ static void tcp_retrans_try_collapse(struct sock *sk, struct sk_buff *to, * state updates are done by the caller. Returns non-zero if an * error occurred which prevented the send. */ -int tcp_retransmit_skb(struct sock *sk, struct sk_buff *skb) +int __tcp_retransmit_skb(struct sock *sk, struct sk_buff *skb) { struct tcp_sock *tp = tcp_sk(sk); struct inet_connection_sock *icsk = inet_csk(sk); unsigned int cur_mss; - int err; /* Inconslusive MTU probe */ if (icsk->icsk_mtup.probe_size) { @@ -2387,11 +2386,17 @@ int tcp_retransmit_skb(struct sock *sk, struct sk_buff *skb) if (unlikely(NET_IP_ALIGN && ((unsigned long)skb->data & 3))) { struct sk_buff *nskb = __pskb_copy(skb, MAX_TCP_HEADER, GFP_ATOMIC); - err = nskb ? tcp_transmit_skb(sk, nskb, 0, GFP_ATOMIC) : - -ENOBUFS; + return nskb ? tcp_transmit_skb(sk, nskb, 0, GFP_ATOMIC) : + -ENOBUFS; } else { - err = tcp_transmit_skb(sk, skb, 1, GFP_ATOMIC); + return tcp_transmit_skb(sk, skb, 1, GFP_ATOMIC); } +} + +int tcp_retransmit_skb(struct sock *sk, struct sk_buff *skb) +{ + struct tcp_sock *tp = tcp_sk(sk); + int err = __tcp_retransmit_skb(sk, skb); if (err == 0) { /* Update global TCP statistics. */
If SYN-ACK partially acks SYN-data, the client retransmits the remaining data by tcp_retransmit_skb(). This increments lost recovery state variables like tp->retrans_out in Open state. If loss recovery happens before the retransmission is acked, it triggers the WARN_ON check in tcp_fastretrans_alert(). For example: the client sends SYN-data, gets SYN-ACK acking only ISN, retransmits data, sends another 4 data packets and get 3 dupacks. Since the retransmission is not caused by network drop it should not update the recovery state variables. Further the server may return a smaller MSS than the cached MSS used for SYN-data, so the retranmission needs a loop. Otherwise some data will not be retransmitted until timeout or other loss recovery events. Signed-off-by: Yuchung Cheng <ycheng@google.com> --- include/net/tcp.h | 1 + net/ipv4/tcp_input.c | 6 +++++- net/ipv4/tcp_output.c | 15 ++++++++++----- 3 files changed, 16 insertions(+), 6 deletions(-)