netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH net] tcp: fix incorrect ca_state in tail loss probe
@ 2013-10-12 17:16 Yuchung Cheng
  2013-10-14  5:55 ` Neal Cardwell
  0 siblings, 1 reply; 3+ messages in thread
From: Yuchung Cheng @ 2013-10-12 17:16 UTC (permalink / raw)
  To: davem, ncardwell, nanditad
  Cc: netdev, michael, jwboyer, sesse, dormando, Yuchung Cheng

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(-)

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);
 		}
-- 
1.8.4

^ permalink raw reply related	[flat|nested] 3+ messages in thread

* Re: [PATCH net] tcp: fix incorrect ca_state in tail loss probe
  2013-10-12 17:16 [PATCH net] tcp: fix incorrect ca_state in tail loss probe Yuchung Cheng
@ 2013-10-14  5:55 ` Neal Cardwell
  2013-10-18 17:29   ` Nandita Dukkipati
  0 siblings, 1 reply; 3+ messages in thread
From: Neal Cardwell @ 2013-10-14  5:55 UTC (permalink / raw)
  To: Yuchung Cheng
  Cc: David Miller, Nandita Dukkipati, Netdev, michael, jwboyer,
	Steinar H. Gunderson, dormando

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

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH net] tcp: fix incorrect ca_state in tail loss probe
  2013-10-14  5:55 ` Neal Cardwell
@ 2013-10-18 17:29   ` Nandita Dukkipati
  0 siblings, 0 replies; 3+ messages in thread
From: Nandita Dukkipati @ 2013-10-18 17:29 UTC (permalink / raw)
  To: Neal Cardwell
  Cc: Yuchung Cheng, David Miller, Netdev, michael, jwboyer,
	Steinar H. Gunderson, dormando

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>

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2013-10-18 17:29 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2013-10-12 17:16 [PATCH net] tcp: fix incorrect ca_state in tail loss probe Yuchung Cheng
2013-10-14  5:55 ` Neal Cardwell
2013-10-18 17:29   ` Nandita Dukkipati

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).