public inbox for netdev@vger.kernel.org 
 help / color / mirror / Atom feed
From: Neal Cardwell <ncardwell@google•com>
To: David Miller <davem@davemloft•net>
Cc: netdev@vger•kernel.org, Neal Cardwell <ncardwell@google•com>,
	Yuchung Cheng <ycheng@google•com>,
	Nandita Dukkipati <nanditad@google•com>
Subject: [PATCH net 2/3] tcp: enable xmit timer fix by having TLP use time when RTO should fire
Date: Mon, 31 Jul 2017 22:58:13 -0400	[thread overview]
Message-ID: <20170801025814.31206-3-ncardwell@google.com> (raw)
In-Reply-To: <20170801025814.31206-1-ncardwell@google.com>

Have tcp_schedule_loss_probe() base the TLP scheduling decision based
on when the RTO *should* fire. This is to enable the upcoming xmit
timer fix in this series, where tcp_schedule_loss_probe() cannot
assume that the last timer installed was an RTO timer (because we are
no longer doing the "rearm RTO, rearm RTO, rearm TLP" dance on every
ACK). So tcp_schedule_loss_probe() must independently figure out when
an RTO would want to fire.

In the new TLP implementation following in this series, we cannot
assume that icsk_timeout was set based on an RTO; after processing a
cumulative ACK the icsk_timeout we see can be from a previous TLP or
RTO. So we need to independently recalculate the RTO time (instead of
reading it out of icsk_timeout). Removing this dependency on the
nature of icsk_timeout makes things a little easier to reason about
anyway.

Note that the old and new code should be equivalent, since they are
both saying: "if the RTO is in the future, but at an earlier time than
the normal TLP time, then set the TLP timer to fire when the RTO would
have fired".

Fixes: 6ba8a3b19e76 ("tcp: Tail loss probe (TLP)")
Signed-off-by: Neal Cardwell <ncardwell@google•com>
Signed-off-by: Yuchung Cheng <ycheng@google•com>
Signed-off-by: Nandita Dukkipati <nanditad@google•com>
---
 net/ipv4/tcp_output.c | 12 ++++--------
 1 file changed, 4 insertions(+), 8 deletions(-)

diff --git a/net/ipv4/tcp_output.c b/net/ipv4/tcp_output.c
index 2f1588bf73da..0ae6b5d176c0 100644
--- a/net/ipv4/tcp_output.c
+++ b/net/ipv4/tcp_output.c
@@ -2377,8 +2377,8 @@ bool tcp_schedule_loss_probe(struct sock *sk)
 {
 	struct inet_connection_sock *icsk = inet_csk(sk);
 	struct tcp_sock *tp = tcp_sk(sk);
-	u32 timeout, tlp_time_stamp, rto_time_stamp;
 	u32 rtt = usecs_to_jiffies(tp->srtt_us >> 3);
+	u32 timeout, rto_delta_us;
 
 	/* No consecutive loss probes. */
 	if (WARN_ON(icsk->icsk_pending == ICSK_TIME_LOSS_PROBE)) {
@@ -2418,13 +2418,9 @@ bool tcp_schedule_loss_probe(struct sock *sk)
 	timeout = max_t(u32, timeout, msecs_to_jiffies(10));
 
 	/* If RTO is shorter, just schedule TLP in its place. */
-	tlp_time_stamp = tcp_jiffies32 + timeout;
-	rto_time_stamp = (u32)inet_csk(sk)->icsk_timeout;
-	if ((s32)(tlp_time_stamp - rto_time_stamp) > 0) {
-		s32 delta = rto_time_stamp - tcp_jiffies32;
-		if (delta > 0)
-			timeout = delta;
-	}
+	rto_delta_us = tcp_rto_delta_us(sk);  /* How far in future is RTO? */
+	if (rto_delta_us > 0)
+		timeout = min_t(u32, timeout, usecs_to_jiffies(rto_delta_us));
 
 	inet_csk_reset_xmit_timer(sk, ICSK_TIME_LOSS_PROBE, timeout,
 				  TCP_RTO_MAX);
-- 
2.14.0.rc0.400.g1c36432dff-goog

  parent reply	other threads:[~2017-08-01  2:58 UTC|newest]

Thread overview: 22+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-08-01  2:58 [PATCH net 0/3] tcp: fix xmit timer rearming to avoid stalls Neal Cardwell
2017-08-01  2:58 ` [PATCH net 1/3] tcp: introduce tcp_rto_delta_us() helper for xmit timer fix Neal Cardwell
2017-08-01  7:16   ` Eric Dumazet
2017-08-01  2:58 ` Neal Cardwell [this message]
2017-08-01  7:22   ` [PATCH net 2/3] tcp: enable xmit timer fix by having TLP use time when RTO should fire Eric Dumazet
2017-08-01 14:35     ` Neal Cardwell
2017-08-01 20:40       ` Eric Dumazet
2017-08-01  2:58 ` [PATCH net 3/3] tcp: fix xmit timer to only be reset if data ACKed/SACKed Neal Cardwell
2017-08-01  7:25   ` Eric Dumazet
2017-08-01 12:20   ` maowenan
2017-08-01 14:24     ` Neal Cardwell
2017-08-04  7:12   ` maowenan
2017-08-04 15:54     ` Neal Cardwell
2017-08-04  7:33   ` maowenan
2017-08-04 16:59     ` Neal Cardwell
2017-08-04 17:10       ` Willy Tarreau
2017-08-04 18:01         ` Neal Cardwell
2017-08-04 18:18           ` Willy Tarreau
2017-08-06  7:39             ` maowenan
2017-08-06  7:44               ` Willy Tarreau
2017-08-06 15:21                 ` Neal Cardwell
2017-08-06 15:24                 ` Neal Cardwell

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=20170801025814.31206-3-ncardwell@google.com \
    --to=ncardwell@google$(echo .)com \
    --cc=davem@davemloft$(echo .)net \
    --cc=nanditad@google$(echo .)com \
    --cc=netdev@vger$(echo .)kernel.org \
    --cc=ycheng@google$(echo .)com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html
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