From 672631c85abd954eda572121698bec94b779e291 Mon Sep 17 00:00:00 2001 From: "rtenneti@chromium.org" Date: Sat, 16 Aug 2014 06:11:45 +0000 Subject: [PATCH] Land Recent QUIC Changes. Slight improvement to QUIC version negotation debug logging. Merge internal change: 73319950 https://codereview.chromium.org/477813003/ Improve QUIC's SentEntropyManagerTest and removing a test from QuicConnectionTest that tests no code in QuicConnection. Merge internal change: 73298415 https://codereview.chromium.org/479463002/ quic_session.h doesn't need to include util/gtl/linked_hash_map.h. Merge internal change: 73256287 https://codereview.chromium.org/477863003/ Fix comment for QUIC handshake message tag COPT. Merge internal change: 73256108 https://codereview.chromium.org/474133002/ Not used in production. Add InRecovery() method to QUIC send algorithm interface, and pass slow start bool to bandwidth recorder. We still want to generate bandwidth estimates while in slow start, and include this information in the STK proto. We also need to know when we are in recovery mode so we can avoid calculating estimates. Merge internal change: 73239536 https://codereview.chromium.org/479443002/ Add a flag to enable QUIC's BBR congestion control by default instead of Cubic in order to test and gather logs on internal servers. Merge internal change: 73146298 https://codereview.chromium.org/477863002/ Add a QuicSustainedBandwidthEstimator class. Not yet used. Followup change list in progress (not ready for review, but in case you want to see intended usage) which does the following: 1) Use this in the QuicSentPacketManager. 2) Detect substantial differences between subsequent sustained bandwidth estimates. 3) Send estimates to client in server config update. internal change list: 72831442 Merge internal change: 73132177 https://codereview.chromium.org/477053002/ Re-add the QUIC DCHECK to ensure packets are always sent in the order of increasing sequence number. Merge internal change: 73128051 https://codereview.chromium.org/469383004/ Change to optimize QUIC's RetransmittableFrames::HasCryptoHandshake by storing it once when the crypto frame is added to RetransmittableFrames. Merge internal change: 73125935 https://codereview.chromium.org/467263003/ Add an Update method on QuicAlarm which contains a granularity. Also change the idle network timeout to be 1 second shorter on the client and 1 second longer on the server. Change saves ~2% of CPU according to an EndToEndTest profile. Merge internal change: 73125905 https://codereview.chromium.org/471313002/ R=rch@chromium.org Review URL: https://codereview.chromium.org/476023002 Cr-Commit-Position: refs/heads/master@{#290106} git-svn-id: svn://svn.chromium.org/chrome/trunk/src@290106 0039d316-1c4b-4281-b951-d872f2087c98 --- net/base/linked_hash_map.h | 22 +++ net/net.gypi | 3 + net/quic/congestion_control/pacing_sender.cc | 4 + net/quic/congestion_control/pacing_sender.h | 1 + .../send_algorithm_interface.h | 3 + .../congestion_control/tcp_cubic_sender.cc | 7 +- .../congestion_control/tcp_cubic_sender.h | 2 +- net/quic/crypto/crypto_protocol.h | 4 +- net/quic/quic_alarm.cc | 13 ++ net/quic/quic_alarm.h | 5 + net/quic/quic_alarm_test.cc | 19 +++ net/quic/quic_connection.cc | 48 ++++--- net/quic/quic_connection_helper_test.cc | 45 +++++++ net/quic/quic_connection_test.cc | 47 +------ net/quic/quic_flags.cc | 3 + net/quic/quic_flags.h | 1 + net/quic/quic_packet_creator.cc | 19 +-- net/quic/quic_protocol.cc | 16 +-- net/quic/quic_protocol.h | 5 +- net/quic/quic_sent_entropy_manager.cc | 8 +- net/quic/quic_sent_entropy_manager.h | 6 + net/quic/quic_sent_entropy_manager_test.cc | 32 +++++ net/quic/quic_session.h | 1 - net/quic/quic_session_test.cc | 5 +- net/quic/quic_sustained_bandwidth_recorder.cc | 62 +++++++++ net/quic/quic_sustained_bandwidth_recorder.h | 86 ++++++++++++ .../quic_sustained_bandwidth_recorder_test.cc | 125 ++++++++++++++++++ net/quic/test_tools/quic_connection_peer.cc | 10 +- net/quic/test_tools/quic_connection_peer.h | 7 +- net/quic/test_tools/quic_test_utils.h | 1 + .../quic/quic_epoll_connection_helper_test.cc | 36 +++++ 31 files changed, 537 insertions(+), 109 deletions(-) create mode 100644 net/quic/quic_sustained_bandwidth_recorder.cc create mode 100644 net/quic/quic_sustained_bandwidth_recorder.h create mode 100644 net/quic/quic_sustained_bandwidth_recorder_test.cc diff --git a/net/base/linked_hash_map.h b/net/base/linked_hash_map.h index 7948647df059..b024ca1d5361 100644 --- a/net/base/linked_hash_map.h +++ b/net/base/linked_hash_map.h @@ -79,6 +79,28 @@ class linked_hash_map { return list_.rend(); } + // Front and back accessors common to many stl containers. + + // Returns the earliest-inserted element + const value_type& front() const { + return list_.front(); + } + + // Returns the earliest-inserted element. + value_type& front() { + return list_.front(); + } + + // Returns the most-recently-inserted element. + const value_type& back() const { + return list_.back(); + } + + // Returns the most-recently-inserted element. + value_type& back() { + return list_.back(); + } + // Clears the map of all values. void clear() { map_.clear(); diff --git a/net/net.gypi b/net/net.gypi index b85f57bea9ac..ce064b2d16b4 100644 --- a/net/net.gypi +++ b/net/net.gypi @@ -925,6 +925,8 @@ 'quic/quic_stream_factory.h', 'quic/quic_stream_sequencer.cc', 'quic/quic_stream_sequencer.h', + 'quic/quic_sustained_bandwidth_recorder.cc', + 'quic/quic_sustained_bandwidth_recorder.h', 'quic/quic_time.cc', 'quic/quic_time.h', 'quic/quic_types.cc', @@ -1549,6 +1551,7 @@ 'quic/quic_socket_address_coder_test.cc', 'quic/quic_stream_factory_test.cc', 'quic/quic_stream_sequencer_test.cc', + 'quic/quic_sustained_bandwidth_recorder_test.cc', 'quic/quic_time_test.cc', 'quic/quic_unacked_packet_map_test.cc', 'quic/quic_utils_chromium_test.cc', diff --git a/net/quic/congestion_control/pacing_sender.cc b/net/quic/congestion_control/pacing_sender.cc index a1008399862e..cc4f983a809a 100644 --- a/net/quic/congestion_control/pacing_sender.cc +++ b/net/quic/congestion_control/pacing_sender.cc @@ -170,6 +170,10 @@ bool PacingSender::InSlowStart() const { return sender_->InSlowStart(); } +bool PacingSender::InRecovery() const { + return sender_->InRecovery(); +} + QuicByteCount PacingSender::GetSlowStartThreshold() const { return sender_->GetSlowStartThreshold(); } diff --git a/net/quic/congestion_control/pacing_sender.h b/net/quic/congestion_control/pacing_sender.h index be46df91d98d..3fef6f89aff0 100644 --- a/net/quic/congestion_control/pacing_sender.h +++ b/net/quic/congestion_control/pacing_sender.h @@ -59,6 +59,7 @@ class NET_EXPORT_PRIVATE PacingSender : public SendAlgorithmInterface { virtual QuicTime::Delta RetransmissionDelay() const OVERRIDE; virtual QuicByteCount GetCongestionWindow() const OVERRIDE; virtual bool InSlowStart() const OVERRIDE; + virtual bool InRecovery() const OVERRIDE; virtual QuicByteCount GetSlowStartThreshold() const OVERRIDE; virtual CongestionControlType GetCongestionControlType() const OVERRIDE; diff --git a/net/quic/congestion_control/send_algorithm_interface.h b/net/quic/congestion_control/send_algorithm_interface.h index 9173afe4a739..5d1f6df3532f 100644 --- a/net/quic/congestion_control/send_algorithm_interface.h +++ b/net/quic/congestion_control/send_algorithm_interface.h @@ -97,6 +97,9 @@ class NET_EXPORT_PRIVATE SendAlgorithmInterface { // BandwidthEstimate is expected to be too low. virtual bool InSlowStart() const = 0; + // Whether the send algorithm is currently in recovery. + virtual bool InRecovery() const = 0; + // Returns the size of the slow start congestion window in bytes, // aka ssthresh. Some send algorithms do not define a slow start // threshold and will return 0. diff --git a/net/quic/congestion_control/tcp_cubic_sender.cc b/net/quic/congestion_control/tcp_cubic_sender.cc index d96770466788..a4b42b4cc1b3 100644 --- a/net/quic/congestion_control/tcp_cubic_sender.cc +++ b/net/quic/congestion_control/tcp_cubic_sender.cc @@ -170,11 +170,8 @@ bool TcpCubicSender::OnPacketSent(QuicTime /*sent_time*/, } prr_out_ += bytes; - if (largest_sent_sequence_number_ < sequence_number) { - // TODO(rch): Ensure that packets are really sent in order. - // DCHECK_LT(largest_sent_sequence_number_, sequence_number); - largest_sent_sequence_number_ = sequence_number; - } + DCHECK_LT(largest_sent_sequence_number_, sequence_number); + largest_sent_sequence_number_ = sequence_number; hybrid_slow_start_.OnPacketSent(sequence_number); return true; } diff --git a/net/quic/congestion_control/tcp_cubic_sender.h b/net/quic/congestion_control/tcp_cubic_sender.h index 28f830222443..c11620c13cbe 100644 --- a/net/quic/congestion_control/tcp_cubic_sender.h +++ b/net/quic/congestion_control/tcp_cubic_sender.h @@ -62,6 +62,7 @@ class NET_EXPORT_PRIVATE TcpCubicSender : public SendAlgorithmInterface { virtual QuicTime::Delta RetransmissionDelay() const OVERRIDE; virtual QuicByteCount GetCongestionWindow() const OVERRIDE; virtual bool InSlowStart() const OVERRIDE; + virtual bool InRecovery() const OVERRIDE; virtual QuicByteCount GetSlowStartThreshold() const OVERRIDE; virtual CongestionControlType GetCongestionControlType() const OVERRIDE; // End implementation of SendAlgorithmInterface. @@ -80,7 +81,6 @@ class NET_EXPORT_PRIVATE TcpCubicSender : public SendAlgorithmInterface { void MaybeIncreaseCwnd(QuicPacketSequenceNumber acked_sequence_number, QuicByteCount bytes_in_flight); bool IsCwndLimited(QuicByteCount bytes_in_flight) const; - bool InRecovery() const; // Methods for isolating PRR from the rest of TCP Cubic. void PrrOnPacketLost(QuicByteCount bytes_in_flight); void PrrOnPacketAcked(QuicByteCount acked_bytes); diff --git a/net/quic/crypto/crypto_protocol.h b/net/quic/crypto/crypto_protocol.h index 0a9c579d1d90..30f92827b6cb 100644 --- a/net/quic/crypto/crypto_protocol.h +++ b/net/quic/crypto/crypto_protocol.h @@ -51,7 +51,7 @@ const QuicTag kSRBF = TAG('S', 'R', 'B', 'F'); // Socket receive buffer const QuicTag kQBIC = TAG('Q', 'B', 'I', 'C'); // TCP cubic const QuicTag kTSTP = TAG('T', 'S', 'T', 'P'); // Timestamp -// Congestion control options +// Connection options (COPT) values const QuicTag kTBBR = TAG('T', 'B', 'B', 'R'); // Reduced Buffer Bloat TCP const QuicTag kRENO = TAG('R', 'E', 'N', 'O'); // Reno Congestion Control const QuicTag kIW10 = TAG('I', 'W', '1', '0'); // Force ICWND to 10 @@ -81,7 +81,7 @@ const QuicTag kAEAD = TAG('A', 'E', 'A', 'D'); // Authenticated // encryption algorithms const QuicTag kCGST = TAG('C', 'G', 'S', 'T'); // Congestion control // feedback types -const QuicTag kCOPT = TAG('C', 'O', 'P', 'T'); // Congestion control options +const QuicTag kCOPT = TAG('C', 'O', 'P', 'T'); // Connection options // kLOSS was 'L', 'O', 'S', 'S', but was changed from a tag vector to a tag. const QuicTag kLOSS = TAG('L', 'O', 'S', 'A'); // Loss detection algorithms const QuicTag kICSL = TAG('I', 'C', 'S', 'L'); // Idle connection state diff --git a/net/quic/quic_alarm.cc b/net/quic/quic_alarm.cc index e1d4a16ae9c1..b5aca8caa175 100644 --- a/net/quic/quic_alarm.cc +++ b/net/quic/quic_alarm.cc @@ -27,6 +27,19 @@ void QuicAlarm::Cancel() { CancelImpl(); } +void QuicAlarm::Update(QuicTime deadline, QuicTime::Delta granularity) { + if (!deadline.IsInitialized()) { + Cancel(); + return; + } + if (std::abs(deadline.Subtract(deadline_).ToMicroseconds()) < + granularity.ToMicroseconds()) { + return; + } + Cancel(); + Set(deadline); +} + bool QuicAlarm::IsSet() const { return deadline_.IsInitialized(); } diff --git a/net/quic/quic_alarm.h b/net/quic/quic_alarm.h index bee51577395c..5df4fe051603 100644 --- a/net/quic/quic_alarm.h +++ b/net/quic/quic_alarm.h @@ -42,6 +42,11 @@ class NET_EXPORT_PRIVATE QuicAlarm { // delegates OnAlarm method will not be called. void Cancel(); + // Cancels and sets the alarm if the |deadline| is farther from the current + // deadline than |granularity|, and otherwise does nothing. If |deadline| is + // not initialized, the alarm is cancelled. + void Update(QuicTime deadline, QuicTime::Delta granularity); + bool IsSet() const; QuicTime deadline() const { return deadline_; } diff --git a/net/quic/quic_alarm_test.cc b/net/quic/quic_alarm_test.cc index cc5028b8457f..434bb38fe06d 100644 --- a/net/quic/quic_alarm_test.cc +++ b/net/quic/quic_alarm_test.cc @@ -88,6 +88,25 @@ TEST_F(QuicAlarmTest, Cancel) { EXPECT_EQ(QuicTime::Zero(), alarm_.deadline()); } +TEST_F(QuicAlarmTest, Update) { + QuicTime deadline = QuicTime::Zero().Add(QuicTime::Delta::FromSeconds(7)); + alarm_.Set(deadline); + QuicTime new_deadline = QuicTime::Zero().Add(QuicTime::Delta::FromSeconds(8)); + alarm_.Update(new_deadline, QuicTime::Delta::Zero()); + EXPECT_TRUE(alarm_.IsSet()); + EXPECT_TRUE(alarm_.scheduled()); + EXPECT_EQ(new_deadline, alarm_.deadline()); +} + +TEST_F(QuicAlarmTest, UpdateWithZero) { + QuicTime deadline = QuicTime::Zero().Add(QuicTime::Delta::FromSeconds(7)); + alarm_.Set(deadline); + alarm_.Update(QuicTime::Zero(), QuicTime::Delta::Zero()); + EXPECT_FALSE(alarm_.IsSet()); + EXPECT_FALSE(alarm_.scheduled()); + EXPECT_EQ(QuicTime::Zero(), alarm_.deadline()); +} + TEST_F(QuicAlarmTest, Fire) { QuicTime deadline = QuicTime::Zero().Add(QuicTime::Delta::FromSeconds(7)); alarm_.Set(deadline); diff --git a/net/quic/quic_connection.cc b/net/quic/quic_connection.cc index a4305f62c71b..c08706d85073 100644 --- a/net/quic/quic_connection.cc +++ b/net/quic/quic_connection.cc @@ -228,7 +228,8 @@ QuicConnection::QuicConnection(QuicConnectionId connection_id, time_of_last_sent_new_packet_(clock_->ApproximateNow()), sequence_number_of_last_sent_packet_(0), sent_packet_manager_( - is_server, clock_, &stats_, kCubic, + is_server, clock_, &stats_, + FLAGS_quic_use_bbr_congestion_control ? kBBR : kCubic, FLAGS_quic_use_time_loss_detection ? kTime : kNack), version_negotiation_state_(START_NEGOTIATION), is_server_(is_server), @@ -408,7 +409,8 @@ void QuicConnection::OnVersionNegotiationPacket( return; } - DVLOG(1) << ENDPOINT << "negotiating version " << version(); + DVLOG(1) << ENDPOINT + << "Negotiated version: " << QuicVersionToString(version()); server_supported_versions_ = packet.versions; version_negotiation_state_ = NEGOTIATION_IN_PROGRESS; RetransmitUnackedPackets(ALL_PACKETS); @@ -569,12 +571,10 @@ void QuicConnection::ProcessAckFrame(const QuicAckFrame& incoming_ack) { // Always reset the retransmission alarm when an ack comes in, since we now // have a better estimate of the current rtt than when it was set. - retransmission_alarm_->Cancel(); QuicTime retransmission_time = sent_packet_manager_.GetRetransmissionTime(); - if (retransmission_time != QuicTime::Zero()) { - retransmission_alarm_->Set(retransmission_time); - } + retransmission_alarm_->Update(retransmission_time, + QuicTime::Delta::FromMilliseconds(1)); } void QuicConnection::ProcessStopWaitingFrame( @@ -970,6 +970,8 @@ void QuicConnection::SendVersionNegotiationPacket() { visitor_->OnWriteBlocked(); return; } + DVLOG(1) << ENDPOINT << "Sending version negotiation packet: {" + << QuicVersionVectorToString(framer_.supported_versions()) << "}"; scoped_ptr version_packet( packet_generator_.SerializeVersionNegotiationPacket( framer_.supported_versions())); @@ -1278,11 +1280,9 @@ void QuicConnection::RetransmitUnackedPackets( void QuicConnection::NeuterUnencryptedPackets() { sent_packet_manager_.NeuterUnencryptedPackets(); // This may have changed the retransmission timer, so re-arm it. - retransmission_alarm_->Cancel(); QuicTime retransmission_time = sent_packet_manager_.GetRetransmissionTime(); - if (retransmission_time != QuicTime::Zero()) { - retransmission_alarm_->Set(retransmission_time); - } + retransmission_alarm_->Update(retransmission_time, + QuicTime::Delta::FromMilliseconds(1)); } bool QuicConnection::ShouldGeneratePacket( @@ -1304,20 +1304,21 @@ bool QuicConnection::CanWrite(HasRetransmittableData retransmittable) { return false; } - send_alarm_->Cancel(); QuicTime now = clock_->Now(); QuicTime::Delta delay = sent_packet_manager_.TimeUntilSend( now, retransmittable); if (delay.IsInfinite()) { + send_alarm_->Cancel(); return false; } // If the scheduler requires a delay, then we can not send this packet now. if (!delay.IsZero()) { - send_alarm_->Set(now.Add(delay)); + send_alarm_->Update(now.Add(delay), QuicTime::Delta::FromMilliseconds(1)); DVLOG(1) << "Delaying sending."; return false; } + send_alarm_->Cancel(); return true; } @@ -1511,11 +1512,9 @@ bool QuicConnection::OnPacketSent(WriteResult result) { transmission_type, retransmittable); if (reset_retransmission_alarm || !retransmission_alarm_->IsSet()) { - retransmission_alarm_->Cancel(); QuicTime retransmission_time = sent_packet_manager_.GetRetransmissionTime(); - if (retransmission_time != QuicTime::Zero()) { - retransmission_alarm_->Set(retransmission_time); - } + retransmission_alarm_->Update(retransmission_time, + QuicTime::Delta::FromMilliseconds(1)); } stats_.bytes_sent += result.bytes_written; @@ -1632,7 +1631,7 @@ void QuicConnection::OnRetransmissionTimeout() { // and nothing waiting to be sent. if (!HasQueuedData() && !retransmission_alarm_->IsSet()) { QuicTime rto_timeout = sent_packet_manager_.GetRetransmissionTime(); - if (rto_timeout != QuicTime::Zero()) { + if (rto_timeout.IsInitialized()) { retransmission_alarm_->Set(rto_timeout); } } @@ -1882,6 +1881,14 @@ bool QuicConnection::CanWriteStreamData() { } void QuicConnection::SetIdleNetworkTimeout(QuicTime::Delta timeout) { + // Adjust the idle timeout on client and server to prevent clients from + // sending requests to servers which have already closed the connection. + if (is_server_) { + timeout = timeout.Add(QuicTime::Delta::FromSeconds(1)); + } else if (timeout > QuicTime::Delta::FromSeconds(1)) { + timeout = timeout.Subtract(QuicTime::Delta::FromSeconds(1)); + } + if (timeout < idle_network_timeout_) { idle_network_timeout_ = timeout; CheckForTimeout(); @@ -1944,7 +1951,7 @@ bool QuicConnection::CheckForTimeout() { } timeout_alarm_->Cancel(); - timeout_alarm_->Set(clock_->ApproximateNow().Add(timeout)); + timeout_alarm_->Set(now.Add(timeout)); return false; } @@ -1953,13 +1960,14 @@ void QuicConnection::SetPingAlarm() { // Only clients send pings. return; } - ping_alarm_->Cancel(); if (!visitor_->HasOpenDataStreams()) { + ping_alarm_->Cancel(); // Don't send a ping unless there are open streams. return; } QuicTime::Delta ping_timeout = QuicTime::Delta::FromSeconds(kPingTimeoutSecs); - ping_alarm_->Set(clock_->ApproximateNow().Add(ping_timeout)); + ping_alarm_->Update(clock_->ApproximateNow().Add(ping_timeout), + QuicTime::Delta::FromSeconds(1)); } QuicConnection::ScopedPacketBundler::ScopedPacketBundler( diff --git a/net/quic/quic_connection_helper_test.cc b/net/quic/quic_connection_helper_test.cc index 2e677221a84d..638ca461c3ca 100644 --- a/net/quic/quic_connection_helper_test.cc +++ b/net/quic/quic_connection_helper_test.cc @@ -141,6 +141,51 @@ TEST_F(QuicConnectionHelperTest, CreateAlarmAndResetEarlier) { EXPECT_FALSE(delegate->fired()); } +TEST_F(QuicConnectionHelperTest, CreateAlarmAndUpdate) { + TestDelegate* delegate = new TestDelegate(); + scoped_ptr alarm(helper_.CreateAlarm(delegate)); + + const QuicClock* clock = helper_.GetClock(); + QuicTime start = clock->Now(); + QuicTime::Delta delta = QuicTime::Delta::FromMicroseconds(1); + alarm->Set(clock->Now().Add(delta)); + QuicTime::Delta new_delta = QuicTime::Delta::FromMicroseconds(3); + alarm->Update(clock->Now().Add(new_delta), + QuicTime::Delta::FromMicroseconds(1)); + + // The alarm task should still be posted. + ASSERT_EQ(1u, runner_->GetPostedTasks().size()); + EXPECT_EQ(base::TimeDelta::FromMicroseconds(delta.ToMicroseconds()), + runner_->GetPostedTasks()[0].delay); + + runner_->RunNextTask(); + EXPECT_EQ(QuicTime::Zero().Add(delta), clock->Now()); + EXPECT_FALSE(delegate->fired()); + + // Move the alarm forward 1us and ensure it doesn't move forward. + alarm->Update(clock->Now().Add(new_delta), + QuicTime::Delta::FromMicroseconds(2)); + + ASSERT_EQ(1u, runner_->GetPostedTasks().size()); + EXPECT_EQ( + base::TimeDelta::FromMicroseconds( + new_delta.Subtract(delta).ToMicroseconds()), + runner_->GetPostedTasks()[0].delay); + runner_->RunNextTask(); + EXPECT_EQ(start.Add(new_delta), clock->Now()); + EXPECT_TRUE(delegate->fired()); + + // Set the alarm via an update call. + new_delta = QuicTime::Delta::FromMicroseconds(5); + alarm->Update(clock->Now().Add(new_delta), + QuicTime::Delta::FromMicroseconds(1)); + EXPECT_TRUE(alarm->IsSet()); + + // Update it with an uninitialized time and ensure it's cancelled. + alarm->Update(QuicTime::Zero(), QuicTime::Delta::FromMicroseconds(1)); + EXPECT_FALSE(alarm->IsSet()); +} + } // namespace } // namespace test } // namespace net diff --git a/net/quic/quic_connection_test.cc b/net/quic/quic_connection_test.cc index 13f97bf436c9..db62ef4b9ee2 100644 --- a/net/quic/quic_connection_test.cc +++ b/net/quic/quic_connection_test.cc @@ -904,15 +904,12 @@ class QuicConnectionTest : public ::testing::TestWithParam { frame.least_unacked = least_unacked; return frame; } + // Explicitly nack a packet. void NackPacket(QuicPacketSequenceNumber missing, QuicAckFrame* frame) { frame->missing_packets.insert(missing); frame->entropy_hash ^= - QuicConnectionPeer::GetSentEntropyHash(&connection_, missing); - if (missing > 1) { - frame->entropy_hash ^= - QuicConnectionPeer::GetSentEntropyHash(&connection_, missing - 1); - } + QuicConnectionPeer::PacketEntropy(&connection_, missing); } // Undo nacking a packet within the frame. @@ -920,11 +917,7 @@ class QuicConnectionTest : public ::testing::TestWithParam { EXPECT_THAT(frame->missing_packets, Contains(arrived)); frame->missing_packets.erase(arrived); frame->entropy_hash ^= - QuicConnectionPeer::GetSentEntropyHash(&connection_, arrived); - if (arrived > 1) { - frame->entropy_hash ^= - QuicConnectionPeer::GetSentEntropyHash(&connection_, arrived - 1); - } + QuicConnectionPeer::PacketEntropy(&connection_, arrived); } void TriggerConnectionClose() { @@ -2639,7 +2632,10 @@ TEST_P(QuicConnectionTest, PingAfterSend) { EXPECT_CALL(*send_algorithm_, OnCongestionEvent(true, _, _, _)); ProcessAckPacket(&frame); EXPECT_TRUE(connection_.GetPingAlarm()->IsSet()); - EXPECT_EQ(clock_.ApproximateNow().Add(QuicTime::Delta::FromSeconds(15)), + // The ping timer is set slightly less than 15 seconds in the future, because + // of the 1s ping timer alarm granularity. + EXPECT_EQ(clock_.ApproximateNow().Add(QuicTime::Delta::FromSeconds(15)) + .Subtract(QuicTime::Delta::FromMilliseconds(5)), connection_.GetPingAlarm()->deadline()); writer_->Reset(); @@ -3246,35 +3242,6 @@ TEST_P(QuicConnectionTest, EntropyCalculationForTruncatedAck) { } } -TEST_P(QuicConnectionTest, CheckSentEntropyHash) { - peer_creator_.set_sequence_number(1); - SequenceNumberSet missing_packets; - QuicPacketEntropyHash entropy_hash = 0; - QuicPacketSequenceNumber max_sequence_number = 51; - for (QuicPacketSequenceNumber i = 1; i <= max_sequence_number; ++i) { - bool is_missing = i % 10 != 0; - bool entropy_flag = (i & (i - 1)) != 0; - QuicPacketEntropyHash packet_entropy_hash = 0; - if (entropy_flag) { - packet_entropy_hash = 1 << (i % 8); - } - QuicPacket* packet = ConstructDataPacket(i, 0, entropy_flag); - connection_.SendPacket( - ENCRYPTION_NONE, i, packet, packet_entropy_hash, - HAS_RETRANSMITTABLE_DATA); - - if (is_missing) { - missing_packets.insert(i); - continue; - } - - entropy_hash ^= packet_entropy_hash; - } - EXPECT_TRUE(QuicConnectionPeer::IsValidEntropy( - &connection_, max_sequence_number, missing_packets, entropy_hash)) - << ""; -} - TEST_P(QuicConnectionTest, ServerSendsVersionNegotiationPacket) { connection_.SetSupportedVersions(QuicSupportedVersions()); framer_.set_version_for_tests(QUIC_VERSION_UNSUPPORTED); diff --git a/net/quic/quic_flags.cc b/net/quic/quic_flags.cc index affa27129c3b..73e492eb420f 100644 --- a/net/quic/quic_flags.cc +++ b/net/quic/quic_flags.cc @@ -38,3 +38,6 @@ bool FLAGS_enable_quic_fec = false; // If true, a QUIC connection with too many unfinished streams will be closed. bool FLAGS_close_quic_connection_unfinished_streams = false; + +// When true, defaults to BBR congestion control instead of Cubic. +bool FLAGS_quic_use_bbr_congestion_control = false; diff --git a/net/quic/quic_flags.h b/net/quic/quic_flags.h index c68b82241351..fc32ddee4782 100644 --- a/net/quic/quic_flags.h +++ b/net/quic/quic_flags.h @@ -15,5 +15,6 @@ NET_EXPORT_PRIVATE extern bool FLAGS_use_early_return_when_verifying_chlo; NET_EXPORT_PRIVATE extern bool FLAGS_send_quic_crypto_reject_reason; NET_EXPORT_PRIVATE extern bool FLAGS_enable_quic_fec; NET_EXPORT_PRIVATE extern bool FLAGS_close_quic_connection_unfinished_streams; +NET_EXPORT_PRIVATE extern bool FLAGS_quic_use_bbr_congestion_control; #endif // NET_QUIC_QUIC_FLAGS_H_ diff --git a/net/quic/quic_packet_creator.cc b/net/quic/quic_packet_creator.cc index 4ad38bb72955..1c3a1a82725d 100644 --- a/net/quic/quic_packet_creator.cc +++ b/net/quic/quic_packet_creator.cc @@ -502,25 +502,16 @@ bool QuicPacketCreator::AddFrame(const QuicFrame& frame, } void QuicPacketCreator::MaybeAddPadding() { - if (BytesFree() == 0) { - // Don't pad full packets. + if (queued_retransmittable_frames_.get() == NULL) { return; } - - // If any of the frames in the current packet are on the crypto stream - // then they contain handshake messagses, and we should pad them. - bool is_handshake = false; - for (size_t i = 0; i < queued_frames_.size(); ++i) { - if (queued_frames_[i].type == STREAM_FRAME && - queued_frames_[i].stream_frame->stream_id == kCryptoStreamId) { - is_handshake = true; - break; - } + if (!queued_retransmittable_frames_->HasCryptoHandshake()) { + return; } - if (!is_handshake) { + if (BytesFree() == 0) { + // Don't pad full packets. return; } - QuicPaddingFrame padding; bool success = AddFrame(QuicFrame(&padding), false); DCHECK(success); diff --git a/net/quic/quic_protocol.cc b/net/quic/quic_protocol.cc index 419820a8c1ce..66925ba1a15e 100644 --- a/net/quic/quic_protocol.cc +++ b/net/quic/quic_protocol.cc @@ -622,7 +622,8 @@ StringPiece QuicPacket::Plaintext() const { } RetransmittableFrames::RetransmittableFrames() - : encryption_level_(NUM_ENCRYPTION_LEVELS) { + : encryption_level_(NUM_ENCRYPTION_LEVELS), + has_crypto_handshake_(NOT_HANDSHAKE) { } RetransmittableFrames::~RetransmittableFrames() { @@ -677,6 +678,9 @@ const QuicFrame& RetransmittableFrames::AddStreamFrame( stream_frame->data.Append(const_cast(stream_data_.back()->data()), stream_data_.back()->size()); frames_.push_back(QuicFrame(stream_frame)); + if (stream_frame->stream_id == kCryptoStreamId) { + has_crypto_handshake_ = IS_HANDSHAKE; + } return frames_.back(); } @@ -687,16 +691,6 @@ const QuicFrame& RetransmittableFrames::AddNonStreamFrame( return frames_.back(); } -IsHandshake RetransmittableFrames::HasCryptoHandshake() const { - for (size_t i = 0; i < frames().size(); ++i) { - if (frames()[i].type == STREAM_FRAME && - frames()[i].stream_frame->stream_id == kCryptoStreamId) { - return IS_HANDSHAKE; - } - } - return NOT_HANDSHAKE; -} - void RetransmittableFrames::set_encryption_level(EncryptionLevel level) { encryption_level_ = level; } diff --git a/net/quic/quic_protocol.h b/net/quic/quic_protocol.h index 6b29331e3b6a..58b6e7e788ec 100644 --- a/net/quic/quic_protocol.h +++ b/net/quic/quic_protocol.h @@ -999,7 +999,9 @@ class NET_EXPORT_PRIVATE RetransmittableFrames { const QuicFrame& AddNonStreamFrame(const QuicFrame& frame); const QuicFrames& frames() const { return frames_; } - IsHandshake HasCryptoHandshake() const; + IsHandshake HasCryptoHandshake() const { + return has_crypto_handshake_; + } void set_encryption_level(EncryptionLevel level); EncryptionLevel encryption_level() const { @@ -1009,6 +1011,7 @@ class NET_EXPORT_PRIVATE RetransmittableFrames { private: QuicFrames frames_; EncryptionLevel encryption_level_; + IsHandshake has_crypto_handshake_; // Data referenced by the StringPiece of a QuicStreamFrame. std::vector stream_data_; diff --git a/net/quic/quic_sent_entropy_manager.cc b/net/quic/quic_sent_entropy_manager.cc index 0f33e2d56e9b..9472a880a52e 100644 --- a/net/quic/quic_sent_entropy_manager.cc +++ b/net/quic/quic_sent_entropy_manager.cc @@ -21,8 +21,12 @@ QuicSentEntropyManager::~QuicSentEntropyManager() {} void QuicSentEntropyManager::RecordPacketEntropyHash( QuicPacketSequenceNumber sequence_number, QuicPacketEntropyHash entropy_hash) { - // TODO(satyamshekhar): Check this logic again when/if we enable packet - // reordering. + if (!packets_entropy_.empty()) { + // Ensure packets always are recorded in order. + // Every packet's entropy is recorded, even if it's not sent, so there + // are not sequence number gaps. + DCHECK_LT(packets_entropy_.back().first, sequence_number); + } packets_entropy_hash_ ^= entropy_hash; packets_entropy_.insert( make_pair(sequence_number, diff --git a/net/quic/quic_sent_entropy_manager.h b/net/quic/quic_sent_entropy_manager.h index c89d97730de0..e7730d569e5b 100644 --- a/net/quic/quic_sent_entropy_manager.h +++ b/net/quic/quic_sent_entropy_manager.h @@ -14,6 +14,10 @@ namespace net { +namespace test { +class QuicConnectionPeer; +} // namespace test + // Records all sent packets by a connection to track the cumulative entropy of // sent packets. It is used by the connection to validate an ack // frame sent by the peer as a preventive measure against the optimistic ack @@ -41,6 +45,8 @@ class NET_EXPORT_PRIVATE QuicSentEntropyManager { void ClearEntropyBefore(QuicPacketSequenceNumber sequence_number); private: + friend class test::QuicConnectionPeer; + typedef linked_hash_map > SentEntropyMap; diff --git a/net/quic/quic_sent_entropy_manager_test.cc b/net/quic/quic_sent_entropy_manager_test.cc index e4e9847b516d..3648bf3b7495 100644 --- a/net/quic/quic_sent_entropy_manager_test.cc +++ b/net/quic/quic_sent_entropy_manager_test.cc @@ -68,6 +68,38 @@ TEST_F(QuicSentEntropyManagerTest, IsValidEntropy) { entropy_hash)); } +TEST_F(QuicSentEntropyManagerTest, ClearEntropiesBefore) { + QuicPacketEntropyHash entropies[10] = + {12, 1, 33, 3, 32, 100, 28, 42, 22, 255}; + + for (size_t i = 0; i < 10; ++i) { + entropy_manager_.RecordPacketEntropyHash(i + 1, entropies[i]); + } + + // Discard the first 5 entropies and ensure IsValidEntropy and EntropyHash + // still return correct results. + entropy_manager_.ClearEntropyBefore(5); + + SequenceNumberSet missing_packets; + missing_packets.insert(7); + missing_packets.insert(8); + + QuicPacketEntropyHash entropy_hash = 0; + for (size_t i = 0; i < 10; ++i) { + if (missing_packets.find(i + 1) == missing_packets.end()) { + entropy_hash ^= entropies[i]; + } + } + EXPECT_TRUE(entropy_manager_.IsValidEntropy(10, missing_packets, + entropy_hash)); + + entropy_hash = 0; + for (size_t i = 0; i < 10; ++i) { + entropy_hash ^= entropies[i]; + } + EXPECT_EQ(entropy_hash, entropy_manager_.EntropyHash(10)); +} + } // namespace } // namespace test } // namespace net diff --git a/net/quic/quic_session.h b/net/quic/quic_session.h index 5dfa7dea95f2..8cd663d01ef8 100644 --- a/net/quic/quic_session.h +++ b/net/quic/quic_session.h @@ -12,7 +12,6 @@ #include "base/compiler_specific.h" #include "base/containers/hash_tables.h" #include "net/base/ip_endpoint.h" -#include "net/base/linked_hash_map.h" #include "net/quic/quic_connection.h" #include "net/quic/quic_crypto_stream.h" #include "net/quic/quic_data_stream.h" diff --git a/net/quic/quic_session_test.cc b/net/quic/quic_session_test.cc index b604a7ffee1f..787f6242f243 100644 --- a/net/quic/quic_session_test.cc +++ b/net/quic/quic_session_test.cc @@ -578,11 +578,12 @@ TEST_P(QuicSessionTest, DoNotSendGoAwayTwice) { } TEST_P(QuicSessionTest, IncreasedTimeoutAfterCryptoHandshake) { - EXPECT_EQ(kDefaultInitialTimeoutSecs, + // Add 1 to the connection timeout on the server side. + EXPECT_EQ(kDefaultInitialTimeoutSecs + 1, QuicConnectionPeer::GetNetworkTimeout(connection_).ToSeconds()); CryptoHandshakeMessage msg; session_.GetCryptoStream()->OnHandshakeMessage(msg); - EXPECT_EQ(kDefaultTimeoutSecs, + EXPECT_EQ(kDefaultTimeoutSecs + 1, QuicConnectionPeer::GetNetworkTimeout(connection_).ToSeconds()); } diff --git a/net/quic/quic_sustained_bandwidth_recorder.cc b/net/quic/quic_sustained_bandwidth_recorder.cc new file mode 100644 index 000000000000..458bbf1a1814 --- /dev/null +++ b/net/quic/quic_sustained_bandwidth_recorder.cc @@ -0,0 +1,62 @@ +// Copyright 2014 The Chromium Authors. All rights reserved. +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +#include "net/quic/quic_sustained_bandwidth_recorder.h" + +#include "base/logging.h" +#include "net/quic/quic_bandwidth.h" +#include "net/quic/quic_time.h" + +namespace net { + +QuicSustainedBandwidthRecorder::QuicSustainedBandwidthRecorder() + : has_estimate_(false), + is_recording_(false), + bandwidth_estimate_recorded_during_slow_start_(false), + bandwidth_estimate_(QuicBandwidth::Zero()), + max_bandwidth_estimate_(QuicBandwidth::Zero()), + max_bandwidth_timestamp_(0), + start_time_(QuicTime::Zero()) {} + +void QuicSustainedBandwidthRecorder::RecordEstimate(bool is_reliable_estimate, + bool in_slow_start, + QuicBandwidth bandwidth, + QuicTime estimate_time, + QuicWallTime wall_time, + QuicTime::Delta srtt) { + if (!is_reliable_estimate) { + is_recording_ = false; + DVLOG(1) << "Stopped recording due to unreliable estimate at: " + << estimate_time.ToDebuggingValue(); + return; + } + + if (!is_recording_) { + // This is the first estimate of a new recording period. + start_time_ = estimate_time; + is_recording_ = true; + DVLOG(1) << "Started recording at: " << start_time_.ToDebuggingValue(); + return; + } + + // If we have been recording for at least 3 * srtt, then record the latest + // bandwidth estimate as a valid sustained bandwidth estimate. + if (estimate_time.Subtract(start_time_) >= srtt.Multiply(3)) { + has_estimate_ = true; + bandwidth_estimate_recorded_during_slow_start_ = in_slow_start; + bandwidth_estimate_ = bandwidth; + DVLOG(1) << "New sustained bandwidth estimate (KBytes/s): " + << bandwidth_estimate_.ToKBytesPerSecond(); + } + + // Check for an increase in max bandwidth. + if (bandwidth > max_bandwidth_estimate_) { + max_bandwidth_estimate_ = bandwidth; + max_bandwidth_timestamp_ = wall_time.ToUNIXSeconds(); + DVLOG(1) << "New max bandwidth estimate (KBytes/s): " + << max_bandwidth_estimate_.ToKBytesPerSecond(); + } +} + +} // namespace net diff --git a/net/quic/quic_sustained_bandwidth_recorder.h b/net/quic/quic_sustained_bandwidth_recorder.h new file mode 100644 index 000000000000..0794a0719a8f --- /dev/null +++ b/net/quic/quic_sustained_bandwidth_recorder.h @@ -0,0 +1,86 @@ +// Copyright 2014 The Chromium Authors. All rights reserved. +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +#ifndef NET_QUIC_QUIC_SUSTAINED_BANDWIDTH_RECORDER_H_ +#define NET_QUIC_QUIC_SUSTAINED_BANDWIDTH_RECORDER_H_ + +#include "base/logging.h" +#include "net/quic/quic_bandwidth.h" +#include "net/quic/quic_time.h" + +namespace net { + +// This class keeps track of a sustained bandwidth estimate to ultimately send +// to the client in a server config update message. A sustained bandwidth +// estimate is only marked as valid if the QuicSustainedBandwidthRecorder has +// been given uninterrupted reliable estimates over a certain period of time. +class NET_EXPORT_PRIVATE QuicSustainedBandwidthRecorder { + public: + QuicSustainedBandwidthRecorder(); + + // As long as |is_reliable_estimate| is consistently true, multiple calls to + // this method over a 3 * srtt period results in storage of a valid sustained + // bandwidth estimate. + // |time_now| is used as a max bandwidth timestamp if needed. + void RecordEstimate(bool is_reliable_estimate, + bool in_slow_start, + QuicBandwidth bandwidth, + QuicTime estimate_time, + QuicWallTime wall_time, + QuicTime::Delta srtt); + + bool HasEstimate() const { + return has_estimate_; + } + + QuicBandwidth BandwidthEstimate() const { + DCHECK(has_estimate_); + return bandwidth_estimate_; + } + + QuicBandwidth MaxBandwidthEstimate() const { + DCHECK(has_estimate_); + return max_bandwidth_estimate_; + } + + int32 MaxBandwidthTimestamp() const { + DCHECK(has_estimate_); + return max_bandwidth_timestamp_; + } + + bool EstimateRecordedDuringSlowStart() const { + DCHECK(has_estimate_); + return bandwidth_estimate_recorded_during_slow_start_; + } + + private: + // True if we have been able to calculate sustained bandwidth, over at least + // one recording period (3 * rtt). + bool has_estimate_; + + // True if the last call to RecordEstimate had a reliable estimate. + bool is_recording_; + + // True if the current sustained bandwidth estimate was generated while in + // slow start. + bool bandwidth_estimate_recorded_during_slow_start_; + + // The latest sustained bandwidth estimate. + QuicBandwidth bandwidth_estimate_; + + // The maximum sustained bandwidth seen over the lifetime of the connection. + QuicBandwidth max_bandwidth_estimate_; + + // Timestamp indicating when the max_bandwidth_estimate_ was seen. + int32 max_bandwidth_timestamp_; + + // Timestamp marking the beginning of the latest recording period. + QuicTime start_time_; + + DISALLOW_COPY_AND_ASSIGN(QuicSustainedBandwidthRecorder); +}; + +} // namespace net + +#endif // NET_QUIC_QUIC_SUSTAINED_BANDWIDTH_RECORDER_H_ diff --git a/net/quic/quic_sustained_bandwidth_recorder_test.cc b/net/quic/quic_sustained_bandwidth_recorder_test.cc new file mode 100644 index 000000000000..2d24858afcfe --- /dev/null +++ b/net/quic/quic_sustained_bandwidth_recorder_test.cc @@ -0,0 +1,125 @@ +// Copyright 2014 The Chromium Authors. All rights reserved. +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +#include "net/quic/quic_sustained_bandwidth_recorder.h" + +#include "net/quic/quic_bandwidth.h" +#include "net/quic/quic_time.h" +#include "testing/gtest/include/gtest/gtest.h" + +namespace net { +namespace test { +namespace { + +TEST(QuicSustainedBandwidthRecorderTest, BandwidthEstimates) { + QuicSustainedBandwidthRecorder recorder; + EXPECT_FALSE(recorder.HasEstimate()); + + QuicTime estimate_time = QuicTime::Zero(); + QuicWallTime wall_time = QuicWallTime::Zero(); + QuicTime::Delta srtt = QuicTime::Delta::FromMilliseconds(150); + const int kBandwidthBitsPerSecond = 12345678; + QuicBandwidth bandwidth = + QuicBandwidth::FromBitsPerSecond(kBandwidthBitsPerSecond); + + // This triggers recording, but should not yield a valid estimate yet. + recorder.RecordEstimate(true, false, bandwidth, estimate_time, wall_time, + srtt); + EXPECT_FALSE(recorder.HasEstimate()); + + // Send a second reading, again this should not result in a valid estimate, + // as not enough time has passed. + estimate_time = estimate_time.Add(srtt); + recorder.RecordEstimate(true, false, bandwidth, estimate_time, wall_time, + srtt); + EXPECT_FALSE(recorder.HasEstimate()); + + // Now 3 * kSRTT has elapsed since first recording, expect a valid estimate. + estimate_time = estimate_time.Add(srtt); + estimate_time = estimate_time.Add(srtt); + recorder.RecordEstimate(true, false, bandwidth, estimate_time, wall_time, + srtt); + EXPECT_TRUE(recorder.HasEstimate()); + EXPECT_EQ(recorder.BandwidthEstimate(), bandwidth); + EXPECT_EQ(recorder.BandwidthEstimate(), recorder.MaxBandwidthEstimate()); + + // Resetting, and sending a different estimate will only change output after + // a further 3 * kSRTT has passed. + QuicBandwidth second_bandwidth = + QuicBandwidth::FromBitsPerSecond(2 * kBandwidthBitsPerSecond); + // Reset the recorder by passing in an unreliable measurement. + recorder.RecordEstimate(false, false, second_bandwidth, estimate_time, + wall_time, srtt); + recorder.RecordEstimate(true, false, second_bandwidth, estimate_time, + wall_time, srtt); + EXPECT_EQ(recorder.BandwidthEstimate(), bandwidth); + + estimate_time = estimate_time.Add(srtt.Multiply(3)); + const int32 kSeconds = 556677; + QuicWallTime second_bandwidth_wall_time = + QuicWallTime::FromUNIXSeconds(kSeconds); + recorder.RecordEstimate(true, false, second_bandwidth, estimate_time, + second_bandwidth_wall_time, srtt); + EXPECT_EQ(recorder.BandwidthEstimate(), second_bandwidth); + EXPECT_EQ(recorder.BandwidthEstimate(), recorder.MaxBandwidthEstimate()); + EXPECT_EQ(recorder.MaxBandwidthTimestamp(), kSeconds); + + // Reset again, this time recording a lower bandwidth than before. + QuicBandwidth third_bandwidth = + QuicBandwidth::FromBitsPerSecond(0.5 * kBandwidthBitsPerSecond); + // Reset the recorder by passing in an unreliable measurement. + recorder.RecordEstimate(false, false, third_bandwidth, estimate_time, + wall_time, srtt); + recorder.RecordEstimate(true, false, third_bandwidth, estimate_time, + wall_time, srtt); + EXPECT_EQ(recorder.BandwidthEstimate(), second_bandwidth); + + estimate_time = estimate_time.Add(srtt.Multiply(3)); + recorder.RecordEstimate(true, false, third_bandwidth, estimate_time, + wall_time, srtt); + EXPECT_EQ(recorder.BandwidthEstimate(), third_bandwidth); + + // Max bandwidth should not have changed. + EXPECT_LT(third_bandwidth, second_bandwidth); + EXPECT_EQ(recorder.MaxBandwidthEstimate(), second_bandwidth); + EXPECT_EQ(recorder.MaxBandwidthTimestamp(), kSeconds); +} + +TEST(QuicSustainedBandwidthRecorderTest, SlowStart) { + // Verify that slow start status is correctly recorded. + QuicSustainedBandwidthRecorder recorder; + EXPECT_FALSE(recorder.HasEstimate()); + + QuicTime estimate_time = QuicTime::Zero(); + QuicWallTime wall_time = QuicWallTime::Zero(); + QuicTime::Delta srtt = QuicTime::Delta::FromMilliseconds(150); + const int kBandwidthBitsPerSecond = 12345678; + QuicBandwidth bandwidth = + QuicBandwidth::FromBitsPerSecond(kBandwidthBitsPerSecond); + + bool in_slow_start = true; + + // This triggers recording, but should not yield a valid estimate yet. + recorder.RecordEstimate(true, in_slow_start, bandwidth, estimate_time, + wall_time, srtt); + + // Now 3 * kSRTT has elapsed since first recording, expect a valid estimate. + estimate_time = estimate_time.Add(srtt.Multiply(3)); + recorder.RecordEstimate(true, in_slow_start, bandwidth, estimate_time, + wall_time, srtt); + EXPECT_TRUE(recorder.HasEstimate()); + EXPECT_TRUE(recorder.EstimateRecordedDuringSlowStart()); + + // Now send another estimate, this time not in slow start. + estimate_time = estimate_time.Add(srtt.Multiply(3)); + in_slow_start = false; + recorder.RecordEstimate(true, in_slow_start, bandwidth, estimate_time, + wall_time, srtt); + EXPECT_TRUE(recorder.HasEstimate()); + EXPECT_FALSE(recorder.EstimateRecordedDuringSlowStart()); +} + +} // namespace +} // namespace test +} // namespace net diff --git a/net/quic/test_tools/quic_connection_peer.cc b/net/quic/test_tools/quic_connection_peer.cc index 6847c1d91f89..d1aa8a5cca98 100644 --- a/net/quic/test_tools/quic_connection_peer.cc +++ b/net/quic/test_tools/quic_connection_peer.cc @@ -111,13 +111,11 @@ QuicPacketEntropyHash QuicConnectionPeer::GetSentEntropyHash( } // static -bool QuicConnectionPeer::IsValidEntropy( +QuicPacketEntropyHash QuicConnectionPeer::PacketEntropy( QuicConnection* connection, - QuicPacketSequenceNumber largest_observed, - const SequenceNumberSet& missing_packets, - QuicPacketEntropyHash entropy_hash) { - return connection->sent_entropy_manager_.IsValidEntropy( - largest_observed, missing_packets, entropy_hash); + QuicPacketSequenceNumber sequence_number) { + return + connection->sent_entropy_manager_.packets_entropy_[sequence_number].first; } // static diff --git a/net/quic/test_tools/quic_connection_peer.h b/net/quic/test_tools/quic_connection_peer.h index f6a0bf9da566..c9c4ee7169bb 100644 --- a/net/quic/test_tools/quic_connection_peer.h +++ b/net/quic/test_tools/quic_connection_peer.h @@ -73,10 +73,9 @@ class QuicConnectionPeer { QuicConnection* connection, QuicPacketSequenceNumber sequence_number); - static bool IsValidEntropy(QuicConnection* connection, - QuicPacketSequenceNumber largest_observed, - const SequenceNumberSet& missing_packets, - QuicPacketEntropyHash entropy_hash); + static QuicPacketEntropyHash PacketEntropy( + QuicConnection* connection, + QuicPacketSequenceNumber sequence_number); static QuicPacketEntropyHash ReceivedEntropyHash( QuicConnection* connection, diff --git a/net/quic/test_tools/quic_test_utils.h b/net/quic/test_tools/quic_test_utils.h index ce6877ef505c..ca2df7e7a5b3 100644 --- a/net/quic/test_tools/quic_test_utils.h +++ b/net/quic/test_tools/quic_test_utils.h @@ -462,6 +462,7 @@ class MockSendAlgorithm : public SendAlgorithmInterface { MOCK_CONST_METHOD0(RetransmissionDelay, QuicTime::Delta(void)); MOCK_CONST_METHOD0(GetCongestionWindow, QuicByteCount()); MOCK_CONST_METHOD0(InSlowStart, bool()); + MOCK_CONST_METHOD0(InRecovery, bool()); MOCK_CONST_METHOD0(GetSlowStartThreshold, QuicByteCount()); MOCK_CONST_METHOD0(GetCongestionControlType, CongestionControlType()); diff --git a/net/tools/quic/quic_epoll_connection_helper_test.cc b/net/tools/quic/quic_epoll_connection_helper_test.cc index 0f977f4fb305..708bdd4b6b06 100644 --- a/net/tools/quic/quic_epoll_connection_helper_test.cc +++ b/net/tools/quic/quic_epoll_connection_helper_test.cc @@ -103,6 +103,42 @@ TEST_F(QuicEpollConnectionHelperTest, CreateAlarmAndReset) { EXPECT_TRUE(delegate->fired()); } +TEST_F(QuicEpollConnectionHelperTest, CreateAlarmAndUpdate) { + TestDelegate* delegate = new TestDelegate(); + scoped_ptr alarm(helper_.CreateAlarm(delegate)); + + const QuicClock* clock = helper_.GetClock(); + QuicTime start = clock->Now(); + QuicTime::Delta delta = QuicTime::Delta::FromMicroseconds(1); + alarm->Set(clock->Now().Add(delta)); + QuicTime::Delta new_delta = QuicTime::Delta::FromMicroseconds(3); + alarm->Update(clock->Now().Add(new_delta), + QuicTime::Delta::FromMicroseconds(1)); + + epoll_server_.AdvanceByExactlyAndCallCallbacks(delta.ToMicroseconds()); + EXPECT_EQ(start.Add(delta), clock->Now()); + EXPECT_FALSE(delegate->fired()); + + // Move the alarm forward 1us and ensure it doesn't move forward. + alarm->Update(clock->Now().Add(new_delta), + QuicTime::Delta::FromMicroseconds(2)); + + epoll_server_.AdvanceByExactlyAndCallCallbacks( + new_delta.Subtract(delta).ToMicroseconds()); + EXPECT_EQ(start.Add(new_delta), clock->Now()); + EXPECT_TRUE(delegate->fired()); + + // Set the alarm via an update call. + new_delta = QuicTime::Delta::FromMicroseconds(5); + alarm->Update(clock->Now().Add(new_delta), + QuicTime::Delta::FromMicroseconds(1)); + EXPECT_TRUE(alarm->IsSet()); + + // Update it with an uninitialized time and ensure it's cancelled. + alarm->Update(QuicTime::Zero(), QuicTime::Delta::FromMicroseconds(1)); + EXPECT_FALSE(alarm->IsSet()); +} + } // namespace } // namespace test } // namespace tools