diff mbox

[net] tcp: fix incorrect ca_state in tail loss probe

Message ID 1381598187-9681-1-git-send-email-ycheng@google.com
State Accepted, archived
Delegated to: David Miller
Headers show

Commit Message

Yuchung Cheng Oct. 12, 2013, 5:16 p.m. UTC 
On receiving an ACK that covers the loss probe sequence, TLP
immediately sets the congestion state to Open, even though some packets
are not recovered and retransmisssion are on the way.  The later ACks
may trigger a WARN_ON check in step D of tcp_fastretrans_alert(), e.g.,
https://bugzilla.redhat.com/show_bug.cgi?id=989251

The fix is to follow the similar procedure in recovery by calling
tcp_try_keep_open(). The sender switches to Open state if no packets
are retransmissted. Otherwise it goes to Disorder and let subsequent
ACKs move the state to Recovery or Open.

Reported-By: Michael Sterrett <michael@sterretts.net>
Tested-By: Dormando <dormando@rydia.net>
Signed-off-by: Yuchung Cheng <ycheng@google.com>
---
 net/ipv4/tcp_input.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

Neal Cardwell Oct. 14, 2013, 5:55 a.m. UTC | #1 
On Sat, Oct 12, 2013 at 1:16 PM, Yuchung Cheng <ycheng@google.com> wrote:
> On receiving an ACK that covers the loss probe sequence, TLP
> immediately sets the congestion state to Open, even though some packets
> are not recovered and retransmisssion are on the way.  The later ACks
> may trigger a WARN_ON check in step D of tcp_fastretrans_alert(), e.g.,
> https://bugzilla.redhat.com/show_bug.cgi?id=989251
>
> The fix is to follow the similar procedure in recovery by calling
> tcp_try_keep_open(). The sender switches to Open state if no packets
> are retransmissted. Otherwise it goes to Disorder and let subsequent
> ACKs move the state to Recovery or Open.
>
> Reported-By: Michael Sterrett <michael@sterretts.net>
> Tested-By: Dormando <dormando@rydia.net>
> Signed-off-by: Yuchung Cheng <ycheng@google.com>

Acked-by: Neal Cardwell <ncardwell@google.com>

Nice catch!

neal
--
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
Nandita Dukkipati Oct. 18, 2013, 5:29 p.m. UTC | #2 
On Sun, Oct 13, 2013 at 10:55 PM, Neal Cardwell <ncardwell@google.com> wrote:
>
> On Sat, Oct 12, 2013 at 1:16 PM, Yuchung Cheng <ycheng@google.com> wrote:
> > On receiving an ACK that covers the loss probe sequence, TLP
> > immediately sets the congestion state to Open, even though some packets
> > are not recovered and retransmisssion are on the way.  The later ACks
> > may trigger a WARN_ON check in step D of tcp_fastretrans_alert(), e.g.,
> > https://bugzilla.redhat.com/show_bug.cgi?id=989251
> >
> > The fix is to follow the similar procedure in recovery by calling
> > tcp_try_keep_open(). The sender switches to Open state if no packets
> > are retransmissted. Otherwise it goes to Disorder and let subsequent
> > ACKs move the state to Recovery or Open.
> >
> > Reported-By: Michael Sterrett <michael@sterretts.net>
> > Tested-By: Dormando <dormando@rydia.net>
> > Signed-off-by: Yuchung Cheng <ycheng@google.com>
>
> Acked-by: Neal Cardwell <ncardwell@google.com>

Acked-by: Nandita Dukkipati <nanditad@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
diff mbox

Patch

diff --git a/net/ipv4/tcp_input.c b/net/ipv4/tcp_input.c
index 113dc5f..53974c7 100644
--- a/net/ipv4/tcp_input.c
+++ b/net/ipv4/tcp_input.c
@@ -3291,7 +3291,7 @@  static void tcp_process_tlp_ack(struct sock *sk, u32 ack, int flag)
 			tcp_init_cwnd_reduction(sk, true);
 			tcp_set_ca_state(sk, TCP_CA_CWR);
 			tcp_end_cwnd_reduction(sk);
-			tcp_set_ca_state(sk, TCP_CA_Open);
+			tcp_try_keep_open(sk);
 			NET_INC_STATS_BH(sock_net(sk),
 					 LINUX_MIB_TCPLOSSPROBERECOVERY);
 		}