Land Recent QUIC Changes.

Disable QuicFlowController for QUIC versions < QUIC_VERSION_17

Chrome Beta was happily advertising a flow control receive window of
length 0, under the assumption that because the version of QUIC being
spoken didn't support flow control, a zero length window advertisement
would have no adverse effects.

On the server end, the refactoring in internal change: 64733866 resulted
in the QUIC version not being checked when querying
QuicFlowController::IsBlocked. IsBlocked would look at the client's
advertised receive window, and conclude that no data could be sent. This
would never change as the flow control accounting is protected behind
version checks.

Ultimately this means that once a stream was marked as write blocked, it
would never resume writing:

https://codereview.chromium.org/242453002/diff/1/net/quic/quic_session.cc?context=&column_width=80
(line# 286) resulting in a connection timeout.

This CL explicitly disables the QuicFlowController when the negotiated
QUIC version is < QUIC_VERSION_17.

Merge internal change: 65137349

This internal change was LGTM'ed by rch. Wanted to port this change
to disable flow control.

https://codereview.chromium.org/242583002/


Downgrading rst stream codes which wouldn't be recognized by the peer.

chromium: fixed chromium's unit tests to use AdjustErrorForVersion.

Merge internal change: 64783797
https://codereview.chromium.org/242483002/


Added WriteResult::WriteResult method for internal use.

Merge internal change: 64780998
https://codereview.chromium.org/242293003/


QUIC: When sending a RST stream for flow control accounting purposes,
include a more descriptive error code.

Prompted by alyssar's comment in internal change: 61236803

Chromium specific code:
+ deleted unused ConstructRstPacket method from
QuicNetworkTransactionTest.
+ Changed ConstructRstPacket to use the new descriptive error code.

Merge internal change: 64774941
https://codereview.chromium.org/242093003/


Pull out flow control functionality from ReliableQuicStream into a new
class, QuicFlowController.

Without this refactoring, all the accounting of bytes
sent/buffered/consumed, comparisons of these with limits to decide if
blocked or not, will be duplicated in ReliableQuicStream and
QuicConnection. Putting all this in a new class means it's easier to
have more comprehensive testing, simplifies ReliableQuicStream, and will
make it much easier to add Connection level flow control (work in
progress internal change: 63944402).

Refactor of QUIC stream flow control. No behavior change, still
protected behind FLAGS_enable_quic_stream_flow_control. This flag is
currently disabled.

Merge internal change: 64733866
https://codereview.chromium.org/242453002/


Plumbs through delta_largest_observed from
QuicSentPackerManager::HandleAckForSentPacket, up to the
QuicAckNotifier. Eventually these values get to the
QuicFasterStatsGatherer which now performs a more accurate RT
calculation.

The important changes are in QuicSentPacketManager (pulling the delta
out of ReceivedInfo), and in QuicFasterStatsGatherer (doing the new
calculation of RT). The rest is plumbing and updating tests.

Improve accuracy of QUIC FasterStats RT calculation by using the
delta time included with delayed ACKs.

Merge internal change: 64721749
https://codereview.chromium.org/241783002/


Fixed a bug in QuicPacketCreator when FEC was used for unsupported
versions, and made packet_creator tests run across all supported
versions of QUIC.

Merge internal change: 64701677
https://codereview.chromium.org/241483004/


Fix to ensure hybrid slow start is reset when QUIC's retransmission
timer fires.

Merge internal change: 64698621
https://codereview.chromium.org/241563002/

R=rch@chromium.org

Review URL: https://codereview.chromium.org/242593002

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@264889 0039d316-1c4b-4281-b951-d872f2087c98

1 files changed, 46 insertions, 51 deletions

diff --git a/net/quic/congestion_control/hybrid_slow_start.cc b/net/quic/congestion_control/hybrid_slow_start.cc
index 9325ca4..284d1bc 100644
--- a/net/quic/congestion_control/hybrid_slow_start.cc
+++ b/net/quic/congestion_control/hybrid_slow_start.cc
@@ -17,9 +17,10 @@ namespace net {
 // tcp_cubic.c.
 const int64 kHybridStartLowWindow = 16;
 // Number of delay samples for detecting the increase of delay.
-const int kHybridStartMinSamples = 8;
+const uint32 kHybridStartMinSamples = 8;
 const int kHybridStartDelayFactorExp = 4;  // 2^4 = 16
 const int kHybridStartDelayMinThresholdUs = 2000;
+// TODO(ianswett): The original paper specifies 8, not 16ms.
 const int kHybridStartDelayMaxThresholdUs = 16000;
 
 HybridSlowStart::HybridSlowStart(const QuicClock* clock)
@@ -31,17 +32,15 @@ HybridSlowStart::HybridSlowStart(const QuicClock* clock)
       update_end_sequence_number_(true),
       sender_end_sequence_number_(0),
       end_sequence_number_(0),
-      last_time_(QuicTime::Zero()),
-      sample_count_(0),
-      current_rtt_(QuicTime::Delta::Zero()) {
+      last_close_ack_pair_time_(QuicTime::Zero()),
+      rtt_sample_count_(0),
+      current_min_rtt_(QuicTime::Delta::Zero()) {
 }
 
 void HybridSlowStart::OnPacketAcked(
     QuicPacketSequenceNumber acked_sequence_number, bool in_slow_start) {
-  if (in_slow_start) {
-    if (IsEndOfRound(acked_sequence_number)) {
-      Reset(sender_end_sequence_number_);
-    }
+  if (in_slow_start && IsEndOfRound(acked_sequence_number)) {
+    Reset(sender_end_sequence_number_);
   }
 
   if (sender_end_sequence_number_ == acked_sequence_number) {
@@ -63,28 +62,31 @@ void HybridSlowStart::OnPacketSent(QuicPacketSequenceNumber sequence_number,
 
 bool HybridSlowStart::ShouldExitSlowStart(const RttStats* rtt_stats,
                                           int64 congestion_window) {
+  // Hybrid start triggers when cwnd is larger than some threshold.
   if (congestion_window < kHybridStartLowWindow) {
     return false;
   }
-  if (!started()) {
+  if (!started_) {
     // Time to start the hybrid slow start.
     Reset(sender_end_sequence_number_);
   }
-  Update(rtt_stats->latest_rtt(), rtt_stats->min_rtt());
-  return Exit();
+  // TODO(ianswett): Merge UpdateAndMaybeExit into ShouldExitSlowStart in
+  // another refactor CL, since it's only used here.
+  return UpdateAndMaybeExit(rtt_stats->latest_rtt(), rtt_stats->min_rtt());
 }
 
 void HybridSlowStart::Restart() {
+  started_ = false;
   found_ack_train_ = false;
   found_delay_  = false;
 }
 
 void HybridSlowStart::Reset(QuicPacketSequenceNumber end_sequence_number) {
   DVLOG(1) << "Reset hybrid slow start @" << end_sequence_number;
-  round_start_ = last_time_ = clock_->ApproximateNow();
+  round_start_ = last_close_ack_pair_time_ = clock_->ApproximateNow();
   end_sequence_number_ = end_sequence_number;
-  current_rtt_ = QuicTime::Delta::Zero();
-  sample_count_ = 0;
+  current_min_rtt_ = QuicTime::Delta::Zero();
+  rtt_sample_count_ = 0;
   started_ = true;
 }
 
@@ -92,64 +94,57 @@ bool HybridSlowStart::IsEndOfRound(QuicPacketSequenceNumber ack) const {
   return end_sequence_number_ <= ack;
 }
 
-void HybridSlowStart::Update(QuicTime::Delta rtt, QuicTime::Delta delay_min) {
+bool HybridSlowStart::UpdateAndMaybeExit(QuicTime::Delta rtt,
+                                         QuicTime::Delta min_rtt) {
   // The original code doesn't invoke this until we hit 16 packet per burst.
-  // Since the code handles lower than 16 grecefully and I removed that
-  // limit.
+  // Since the code handles lower than 16 gracefully I removed that limit.
   if (found_ack_train_ || found_delay_) {
-    return;
+    return true;
   }
   QuicTime current_time = clock_->ApproximateNow();
 
   // First detection parameter - ack-train detection.
   // Since slow start burst out packets we can indirectly estimate the inter-
   // arrival time by looking at the arrival time of the ACKs if the ACKs are
-  // spread out more then half the minimum RTT packets are beeing spread out
+  // spread out more then half the minimum RTT packets are being spread out
   // more than the capacity.
-  // This first trigger will not come into play until we hit roughly 4.8 Mbit/s.
+  // This first trigger will not come into play until we hit roughly 9.6 Mbps
+  // with delayed acks (or 4.8Mbps without delayed acks)
   // TODO(pwestin): we need to make sure our pacing don't trigger this detector.
-  if (current_time.Subtract(last_time_).ToMicroseconds() <=
-      kHybridStartDelayMinThresholdUs) {
-    last_time_ = current_time;
-    if (current_time.Subtract(round_start_).ToMicroseconds() >=
-        (delay_min.ToMicroseconds() >> 1)) {
-      found_ack_train_ = true;
-    }
+  if (current_time.Subtract(last_close_ack_pair_time_).ToMicroseconds() <=
+          kHybridStartDelayMinThresholdUs) {
+    last_close_ack_pair_time_ = current_time;
+    found_ack_train_ = current_time.Subtract(round_start_).ToMicroseconds() >=
+        min_rtt.ToMicroseconds() >> 1;
   }
   // Second detection parameter - delay increase detection.
-  // Compare the minimum delay (current_rtt_) of the current
+  // Compare the minimum delay (current_min_rtt_) of the current
   // burst of packets relative to the minimum delay during the session.
   // Note: we only look at the first few(8) packets in each burst, since we
   // only want to compare the lowest RTT of the burst relative to previous
   // bursts.
-  sample_count_++;
-  if (sample_count_ <= kHybridStartMinSamples) {
-    if (current_rtt_.IsZero() || current_rtt_ > rtt) {
-      current_rtt_ = rtt;
+  rtt_sample_count_++;
+  if (rtt_sample_count_ <= kHybridStartMinSamples) {
+    if (current_min_rtt_.IsZero() || current_min_rtt_ > rtt) {
+      current_min_rtt_ = rtt;
     }
   }
   // We only need to check this once.
-  if (sample_count_ == kHybridStartMinSamples) {
-    int accepted_variance_us = delay_min.ToMicroseconds() >>
+  if (rtt_sample_count_ == kHybridStartMinSamples) {
+    // Divide min_rtt by 16 to get a rtt increase threshold for exiting.
+    int min_rtt_increase_threshold_us = min_rtt.ToMicroseconds() >>
         kHybridStartDelayFactorExp;
-    accepted_variance_us = min(accepted_variance_us,
-                               kHybridStartDelayMaxThresholdUs);
-    QuicTime::Delta accepted_variance = QuicTime::Delta::FromMicroseconds(
-        max(accepted_variance_us, kHybridStartDelayMinThresholdUs));
-
-    if (current_rtt_ > delay_min.Add(accepted_variance)) {
-      found_delay_ = true;
-    }
-  }
-}
-
-bool HybridSlowStart::Exit() {
-  // If either one of the two conditions are met we exit from slow start
-  // immediately.
-  if (found_ack_train_ || found_delay_) {
-    return true;
+    // Ensure the rtt threshold is never less than 2ms or more than 16ms.
+    min_rtt_increase_threshold_us = min(min_rtt_increase_threshold_us,
+                                        kHybridStartDelayMaxThresholdUs);
+    QuicTime::Delta min_rtt_increase_threshold =
+        QuicTime::Delta::FromMicroseconds(max(min_rtt_increase_threshold_us,
+                                              kHybridStartDelayMinThresholdUs));
+
+    found_delay_ = current_min_rtt_ > min_rtt.Add(min_rtt_increase_threshold);
   }
-  return false;
+  // If either of the two conditions are met we exit from slow start.
+  return found_ack_train_ || found_delay_;
 }
 
 }  // namespace net