public inbox for netdev@vger.kernel.org
 help / color / mirror / Atom feed
From: Yuchung Cheng <ycheng@google.com>
To: davem@davemloft.net
Cc: netdev@vger.kernel.org, edumazet@google.com,
	ncardwell@google.com, soheil@google.com, priyarjha@google.com,
	Yuchung Cheng <ycheng@google.com>
Subject: [PATCH net-next 6/8] tcp: separate loss marking and state update on RTO
Date: Wed, 16 May 2018 16:40:15 -0700	[thread overview]
Message-ID: <20180516234017.172775-7-ycheng@google.com> (raw)
In-Reply-To: <20180516234017.172775-1-ycheng@google.com>

Previously when TCP times out, it first updates cwnd and ssthresh,
marks packets lost, and then updates congestion state again. This
was fine because everything not yet delivered is marked lost,
so the inflight is always 0 and cwnd can be safely set to 1 to
retransmit one packet on timeout.

But the inflight may not always be 0 on timeout if TCP changes to
mark packets lost based on packet sent time. Therefore we must
first mark the packet lost, then set the cwnd based on the
(updated) inflight.

This is not a pure refactor. Congestion control may potentially
break if it uses (not yet updated) inflight to compute ssthresh.
Fortunately all existing congestion control modules does not do that.
Also it changes the inflight when CA_LOSS_EVENT is called, and only
westwood processes such an event but does not use inflight.

This change has two other minor side benefits:
1) consistent with Fast Recovery s.t. the inflight is updated
   first before tcp_enter_recovery flips state to CA_Recovery.

2) avoid intertwining loss marking with state update, making the
   code more readable.

Signed-off-by: Yuchung Cheng <ycheng@google.com>
Signed-off-by: Neal Cardwell <ncardwell@google.com>
Reviewed-by: Eric Dumazet <edumazet@google.com>
Reviewed-by: Soheil Hassas Yeganeh <soheil@google.com>
Reviewed-by: Priyaranjan Jha <priyarjha@google.com>
---
 net/ipv4/tcp_input.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/net/ipv4/tcp_input.c b/net/ipv4/tcp_input.c
index af32accda2a9..1ccc97b368c7 100644
--- a/net/ipv4/tcp_input.c
+++ b/net/ipv4/tcp_input.c
@@ -1955,6 +1955,8 @@ void tcp_enter_loss(struct sock *sk)
 	struct net *net = sock_net(sk);
 	bool new_recovery = icsk->icsk_ca_state < TCP_CA_Recovery;
 
+	tcp_timeout_mark_lost(sk);
+
 	/* Reduce ssthresh if it has not yet been made inside this window. */
 	if (icsk->icsk_ca_state <= TCP_CA_Disorder ||
 	    !after(tp->high_seq, tp->snd_una) ||
@@ -1969,8 +1971,6 @@ void tcp_enter_loss(struct sock *sk)
 	tp->snd_cwnd_cnt   = 0;
 	tp->snd_cwnd_stamp = tcp_jiffies32;
 
-	tcp_timeout_mark_lost(sk);
-
 	/* Timeout in disordered state after receiving substantial DUPACKs
 	 * suggests that the degree of reordering is over-estimated.
 	 */
-- 
2.17.0.441.gb46fe60e1d-goog

  parent reply	other threads:[~2018-05-16 23:40 UTC|newest]

Thread overview: 11+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-05-16 23:40 [PATCH net-next 0/8] tcp: default RACK loss recovery Yuchung Cheng
2018-05-16 23:40 ` [PATCH net-next 1/8] tcp: support DUPACK threshold in RACK Yuchung Cheng
2018-05-16 23:40 ` [PATCH net-next 2/8] tcp: disable RFC6675 loss detection Yuchung Cheng
2018-05-16 23:40 ` [PATCH net-next 3/8] tcp: simpler NewReno implementation Yuchung Cheng
2018-05-16 23:40 ` [PATCH net-next 4/8] tcp: account lost retransmit after timeout Yuchung Cheng
2018-05-16 23:40 ` [PATCH net-next 5/8] tcp: new helper tcp_timeout_mark_lost Yuchung Cheng
2018-05-16 23:40 ` Yuchung Cheng [this message]
2018-05-16 23:40 ` [PATCH net-next 7/8] tcp: new helper tcp_rack_skb_timeout Yuchung Cheng
2018-05-16 23:40 ` [PATCH net-next 8/8] tcp: don't mark recently sent packets lost on RTO Yuchung Cheng
2018-05-17 19:45 ` [PATCH net-next 0/8] tcp: default RACK loss recovery David Miller
2018-05-21 18:39   ` hiren panchasara

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20180516234017.172775-7-ycheng@google.com \
    --to=ycheng@google.com \
    --cc=davem@davemloft.net \
    --cc=edumazet@google.com \
    --cc=ncardwell@google.com \
    --cc=netdev@vger.kernel.org \
    --cc=priyarjha@google.com \
    --cc=soheil@google.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox