diff options
29 files changed, 88 insertions, 337 deletions
diff --git a/chrome/browser/io_thread.cc b/chrome/browser/io_thread.cc index 664a507..7726fcf 100644 --- a/chrome/browser/io_thread.cc +++ b/chrome/browser/io_thread.cc @@ -63,6 +63,7 @@ #include "net/proxy/proxy_config_service.h" #include "net/proxy/proxy_script_fetcher_impl.h" #include "net/proxy/proxy_service.h" +#include "net/quic/crypto/crypto_protocol.h" #include "net/quic/quic_protocol.h" #include "net/socket/tcp_client_socket.h" #include "net/spdy/spdy_session.h" @@ -1043,8 +1044,6 @@ void IOThread::InitializeNetworkSessionParamsFromGlobals( ¶ms->enable_websocket_over_spdy); globals.enable_quic.CopyToIfSet(¶ms->enable_quic); - globals.enable_quic_pacing.CopyToIfSet( - ¶ms->enable_quic_pacing); globals.enable_quic_time_based_loss_detection.CopyToIfSet( ¶ms->enable_quic_time_based_loss_detection); globals.enable_quic_port_selection.CopyToIfSet( @@ -1157,9 +1156,6 @@ void IOThread::ConfigureQuicGlobals( bool enable_quic = ShouldEnableQuic(command_line, quic_trial_group); globals->enable_quic.set(enable_quic); if (enable_quic) { - globals->enable_quic_pacing.set( - ShouldEnableQuicPacing(command_line, quic_trial_group, - quic_trial_params)); globals->enable_quic_time_based_loss_detection.set( ShouldEnableQuicTimeBasedLossDetection(command_line, quic_trial_group, quic_trial_params)); @@ -1167,6 +1163,10 @@ void IOThread::ConfigureQuicGlobals( ShouldEnableQuicPortSelection(command_line)); globals->quic_connection_options = GetQuicConnectionOptions(command_line, quic_trial_params); + if (ShouldEnableQuicPacing(command_line, quic_trial_group, + quic_trial_params)) { + globals->quic_connection_options.push_back(net::kPACE); + } } size_t max_packet_length = GetQuicMaxPacketLength(command_line, diff --git a/chrome/browser/io_thread.h b/chrome/browser/io_thread.h index b7cc3d3..8559217 100644 --- a/chrome/browser/io_thread.h +++ b/chrome/browser/io_thread.h @@ -185,7 +185,6 @@ class IOThread : public content::BrowserThreadDelegate { Optional<bool> enable_websocket_over_spdy; Optional<bool> enable_quic; - Optional<bool> enable_quic_pacing; Optional<bool> enable_quic_time_based_loss_detection; Optional<bool> enable_quic_port_selection; Optional<size_t> quic_max_packet_length; diff --git a/chrome/browser/io_thread_unittest.cc b/chrome/browser/io_thread_unittest.cc index e9f25d0..9cbf7e5 100644 --- a/chrome/browser/io_thread_unittest.cc +++ b/chrome/browser/io_thread_unittest.cc @@ -127,7 +127,6 @@ TEST_F(IOThreadTest, EnableQuicFromFieldTrialGroup) { net::HttpNetworkSession::Params params; InitializeNetworkSessionParams(¶ms); EXPECT_TRUE(params.enable_quic); - EXPECT_FALSE(params.enable_quic_pacing); EXPECT_FALSE(params.enable_quic_time_based_loss_detection); EXPECT_EQ(1350u, params.quic_max_packet_length); EXPECT_EQ(default_params.quic_supported_versions, @@ -153,7 +152,9 @@ TEST_F(IOThreadTest, EnablePacingFromCommandLine) { ConfigureQuicGlobals(); net::HttpNetworkSession::Params params; InitializeNetworkSessionParams(¶ms); - EXPECT_TRUE(params.enable_quic_pacing); + net::QuicTagVector options; + options.push_back(net::kPACE); + EXPECT_EQ(options, params.quic_connection_options); } TEST_F(IOThreadTest, EnablePacingFromFieldTrialGroup) { @@ -162,7 +163,9 @@ TEST_F(IOThreadTest, EnablePacingFromFieldTrialGroup) { ConfigureQuicGlobals(); net::HttpNetworkSession::Params params; InitializeNetworkSessionParams(¶ms); - EXPECT_TRUE(params.enable_quic_pacing); + net::QuicTagVector options; + options.push_back(net::kPACE); + EXPECT_EQ(options, params.quic_connection_options); } TEST_F(IOThreadTest, EnablePacingFromFieldTrialParams) { @@ -172,7 +175,9 @@ TEST_F(IOThreadTest, EnablePacingFromFieldTrialParams) { ConfigureQuicGlobals(); net::HttpNetworkSession::Params params; InitializeNetworkSessionParams(¶ms); - EXPECT_TRUE(params.enable_quic_pacing); + net::QuicTagVector options; + options.push_back(net::kPACE); + EXPECT_EQ(options, params.quic_connection_options); } TEST_F(IOThreadTest, EnableTimeBasedLossDetectionFromCommandLine) { diff --git a/net/http/http_network_session.cc b/net/http/http_network_session.cc index b48d660..cebbb1d 100644 --- a/net/http/http_network_session.cc +++ b/net/http/http_network_session.cc @@ -20,6 +20,7 @@ #include "net/quic/crypto/quic_random.h" #include "net/quic/quic_clock.h" #include "net/quic/quic_crypto_client_stream_factory.h" +#include "net/quic/quic_protocol.h" #include "net/quic/quic_stream_factory.h" #include "net/socket/client_socket_factory.h" #include "net/socket/client_socket_pool_manager_impl.h" @@ -87,7 +88,6 @@ HttpNetworkSession::Params::Params() enable_websocket_over_spdy(false), enable_quic(false), enable_quic_port_selection(true), - enable_quic_pacing(false), enable_quic_time_based_loss_detection(false), quic_clock(NULL), quic_random(NULL), @@ -129,7 +129,6 @@ HttpNetworkSession::HttpNetworkSession(const Params& params) params.quic_user_agent_id, params.quic_supported_versions, params.enable_quic_port_selection, - params.enable_quic_pacing, params.enable_quic_time_based_loss_detection, params.quic_connection_options), spdy_session_pool_(params.host_resolver, @@ -252,7 +251,7 @@ base::Value* HttpNetworkSession::QuicInfoToValue() const { dict->SetBoolean("enable_quic_port_selection", params_.enable_quic_port_selection); dict->SetBoolean("enable_quic_pacing", - params_.enable_quic_pacing); + ContainsQuicTag(params_.quic_connection_options, kPACE)); dict->SetBoolean("enable_quic_time_based_loss_detection", params_.enable_quic_time_based_loss_detection); dict->SetString("origin_to_force_quic_on", diff --git a/net/http/http_network_session.h b/net/http/http_network_session.h index 21a692d..f62232a 100644 --- a/net/http/http_network_session.h +++ b/net/http/http_network_session.h @@ -110,7 +110,6 @@ class NET_EXPORT HttpNetworkSession bool enable_quic; bool enable_quic_port_selection; - bool enable_quic_pacing; bool enable_quic_time_based_loss_detection; HostPortPair origin_to_force_quic_on; QuicClock* quic_clock; // Will be owned by QuicStreamFactory. diff --git a/net/quic/crypto/crypto_protocol.h b/net/quic/crypto/crypto_protocol.h index 878b8ee..0a9c579 100644 --- a/net/quic/crypto/crypto_protocol.h +++ b/net/quic/crypto/crypto_protocol.h @@ -49,13 +49,13 @@ const QuicTag kSRBF = TAG('S', 'R', 'B', 'F'); // Socket receive buffer // Congestion control feedback types const QuicTag kQBIC = TAG('Q', 'B', 'I', 'C'); // TCP cubic -const QuicTag kPACE = TAG('P', 'A', 'C', 'E'); // Paced TCP cubic const QuicTag kTSTP = TAG('T', 'S', 'T', 'P'); // Timestamp // Congestion control options 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 +const QuicTag kPACE = TAG('P', 'A', 'C', 'E'); // Paced TCP cubic // Loss detection algorithm types const QuicTag kNACK = TAG('N', 'A', 'C', 'K'); // TCP style nack counting diff --git a/net/quic/crypto/source_address_token.h b/net/quic/crypto/source_address_token.h index c719fef..a41a726 100644 --- a/net/quic/crypto/source_address_token.h +++ b/net/quic/crypto/source_address_token.h @@ -42,6 +42,23 @@ class CachedNetworkParameters { bandwidth_estimate_bytes_per_second_ = bandwidth_estimate_bytes_per_second; } + int32 max_bandwidth_estimate_bytes_per_second() const { + return max_bandwidth_estimate_bytes_per_second_; + } + void set_max_bandwidth_estimate_bytes_per_second( + int32 max_bandwidth_estimate_bytes_per_second) { + max_bandwidth_estimate_bytes_per_second_ = + max_bandwidth_estimate_bytes_per_second; + } + + int64 max_bandwidth_timestamp_seconds() const { + return max_bandwidth_timestamp_seconds_; + } + void set_max_bandwidth_timestamp_seconds( + int64 max_bandwidth_timestamp_seconds) { + max_bandwidth_timestamp_seconds_ = max_bandwidth_timestamp_seconds; + } + int32 min_rtt_ms() const { return min_rtt_ms_; } @@ -66,6 +83,11 @@ class CachedNetworkParameters { // The server can supply a bandwidth estimate (in bytes/s) which it may re-use // on receipt of a source-address token with this field set. int32 bandwidth_estimate_bytes_per_second_; + // The maximum bandwidth seen by the client, not necessarily the latest. + int32 max_bandwidth_estimate_bytes_per_second_; + // Timestamp (seconds since UNIX epoch) that indicates when the max bandwidth + // was seen by the server. + int64 max_bandwidth_timestamp_seconds_; // The min RTT seen on a previous connection can be used by the server to // inform initial connection parameters for new connections. int32 min_rtt_ms_; diff --git a/net/quic/quic_config.cc b/net/quic/quic_config.cc index c68eac9..4542a04 100644 --- a/net/quic/quic_config.cc +++ b/net/quic/quic_config.cc @@ -646,9 +646,6 @@ bool QuicConfig::negotiated() { void QuicConfig::SetDefaults() { QuicTagVector congestion_feedback; - if (FLAGS_enable_quic_pacing) { - congestion_feedback.push_back(kPACE); - } congestion_feedback.push_back(kQBIC); congestion_feedback_.set(congestion_feedback, kQBIC); idle_connection_state_lifetime_seconds_.set(kDefaultTimeoutSecs, @@ -665,15 +662,6 @@ void QuicConfig::SetDefaults() { SetInitialSessionFlowControlWindowToSend(kDefaultFlowControlSendWindow); } -void QuicConfig::EnablePacing(bool enable_pacing) { - QuicTagVector congestion_feedback; - if (enable_pacing) { - congestion_feedback.push_back(kPACE); - } - congestion_feedback.push_back(kQBIC); - congestion_feedback_.set(congestion_feedback, kQBIC); -} - void QuicConfig::ToHandshakeMessage(CryptoHandshakeMessage* out) const { congestion_feedback_.ToHandshakeMessage(out); idle_connection_state_lifetime_seconds_.ToHandshakeMessage(out); diff --git a/net/quic/quic_config.h b/net/quic/quic_config.h index 3c5f5cd..02c2b03 100644 --- a/net/quic/quic_config.h +++ b/net/quic/quic_config.h @@ -354,9 +354,6 @@ class NET_EXPORT_PRIVATE QuicConfig { // SetDefaults sets the members to sensible, default values. void SetDefaults(); - // Enabled pacing. - void EnablePacing(bool enable_pacing); - // ToHandshakeMessage serialises the settings in this object as a series of // tags /value pairs and adds them to |out|. void ToHandshakeMessage(CryptoHandshakeMessage* out) const; diff --git a/net/quic/quic_config_test.cc b/net/quic/quic_config_test.cc index 86f435f..5c302b0 100644 --- a/net/quic/quic_config_test.cc +++ b/net/quic/quic_config_test.cc @@ -88,9 +88,8 @@ TEST_F(QuicConfigTest, ToHandshakeMessageWithPacing) { const QuicTag* out; size_t out_len; EXPECT_EQ(QUIC_NO_ERROR, msg.GetTaglist(kCGST, &out, &out_len)); - EXPECT_EQ(2u, out_len); - EXPECT_EQ(kPACE, out[0]); - EXPECT_EQ(kQBIC, out[1]); + EXPECT_EQ(1u, out_len); + EXPECT_EQ(kQBIC, out[0]); } TEST_F(QuicConfigTest, ProcessClientHello) { diff --git a/net/quic/quic_connection.cc b/net/quic/quic_connection.cc index f40b34d..a4305f6 100644 --- a/net/quic/quic_connection.cc +++ b/net/quic/quic_connection.cc @@ -237,10 +237,13 @@ QuicConnection::QuicConnection(QuicConnectionId connection_id, peer_port_changed_(false), self_ip_changed_(false), self_port_changed_(false) { +#if 0 + // TODO(rtenneti): Should we enable this code in chromium? if (!is_server_) { // Pacing will be enabled if the client negotiates it. sent_packet_manager_.MaybeEnablePacing(); } +#endif DVLOG(1) << ENDPOINT << "Created connection with connection_id: " << connection_id; timeout_alarm_->Set(clock_->ApproximateNow().Add(idle_network_timeout_)); @@ -1698,6 +1701,9 @@ void QuicConnection::MaybeProcessUndecryptablePackets() { // never be able to be decrypted. if (encryption_level_ == ENCRYPTION_FORWARD_SECURE) { if (debug_visitor_.get() != NULL) { + // TODO(rtenneti): perhaps more efficient to pass the number of + // undecryptable packets as the argument to OnUndecryptablePacket so that + // we just need to call OnUndecryptablePacket once? for (size_t i = 0; i < undecryptable_packets_.size(); ++i) { debug_visitor_->OnUndecryptablePacket(); } diff --git a/net/quic/quic_connection.h b/net/quic/quic_connection.h index ee5955f..4d57208 100644 --- a/net/quic/quic_connection.h +++ b/net/quic/quic_connection.h @@ -137,7 +137,7 @@ class NET_EXPORT_PRIVATE QuicConnectionDebugVisitor const IPEndPoint& peer_address, const QuicEncryptedPacket& packet) {} - // Called when a packet is recived with a connection id that does not + // Called when a packet is received with a connection id that does not // match the ID of this connection. virtual void OnIncorrectConnectionId( QuicConnectionId connection_id) {} @@ -375,6 +375,7 @@ class NET_EXPORT_PRIVATE QuicConnection void set_visitor(QuicConnectionVisitorInterface* visitor) { visitor_ = visitor; } + // This method takes ownership of |debug_visitor|. void set_debug_visitor(QuicConnectionDebugVisitor* debug_visitor) { debug_visitor_.reset(debug_visitor); packet_generator_.set_debug_delegate(debug_visitor); diff --git a/net/quic/quic_connection_test.cc b/net/quic/quic_connection_test.cc index 7faafe7..13f97bf 100644 --- a/net/quic/quic_connection_test.cc +++ b/net/quic/quic_connection_test.cc @@ -57,7 +57,6 @@ const bool kEntropyFlag = true; const QuicPacketEntropyHash kTestEntropyHash = 76; const int kDefaultRetransmissionTimeMs = 500; -const int kMinRetransmissionTimeMs = 200; class TestReceiveAlgorithm : public ReceiveAlgorithmInterface { public: @@ -885,7 +884,7 @@ class QuicConnectionTest : public ::testing::TestWithParam<QuicVersion> { } QuicTime::Delta DefaultDelayedAckTime() { - return QuicTime::Delta::FromMilliseconds(kMinRetransmissionTimeMs/2); + return QuicTime::Delta::FromMilliseconds(kMaxDelayedAckTime); } // Initialize a frame acknowledging all packets up to largest_observed. @@ -3962,7 +3961,7 @@ TEST_P(QuicConnectionTest, Pacing) { writer_.get(), true, version()); TestConnection client(connection_id_, IPEndPoint(), helper_.get(), writer_.get(), false, version()); - EXPECT_TRUE(client.sent_packet_manager().using_pacing()); + EXPECT_FALSE(client.sent_packet_manager().using_pacing()); EXPECT_FALSE(server.sent_packet_manager().using_pacing()); } diff --git a/net/quic/quic_crypto_client_stream_test.cc b/net/quic/quic_crypto_client_stream_test.cc index 344dfe1..47b4ed6 100644 --- a/net/quic/quic_crypto_client_stream_test.cc +++ b/net/quic/quic_crypto_client_stream_test.cc @@ -93,8 +93,7 @@ TEST_F(QuicCryptoClientStreamTest, NegotiatedParameters) { CompleteCryptoHandshake(); const QuicConfig* config = session_->config(); - EXPECT_EQ(FLAGS_enable_quic_pacing ? kPACE : kQBIC, - config->congestion_feedback()); + EXPECT_EQ(kQBIC, config->congestion_feedback()); EXPECT_EQ(kDefaultTimeoutSecs, config->idle_connection_state_lifetime().ToSeconds()); EXPECT_EQ(kDefaultMaxStreamsPerConnection, diff --git a/net/quic/quic_crypto_server_stream.cc b/net/quic/quic_crypto_server_stream.cc index 8e07f54..df8f0d8 100644 --- a/net/quic/quic_crypto_server_stream.cc +++ b/net/quic/quic_crypto_server_stream.cc @@ -170,7 +170,7 @@ void QuicCryptoServerStream::SendServerConfigUpdate() { return; } - DVLOG(1) << "Server: Sending server config update (SCUP): " + DVLOG(1) << "Server: Sending server config update: " << server_config_update_message.DebugString(); const QuicData& data = server_config_update_message.GetSerialized(); WriteOrBufferData(string(data.data(), data.length()), false, NULL); diff --git a/net/quic/quic_crypto_stream.cc b/net/quic/quic_crypto_stream.cc index fe2d311..8aa8d3e 100644 --- a/net/quic/quic_crypto_stream.cc +++ b/net/quic/quic_crypto_stream.cc @@ -18,13 +18,12 @@ using base::StringPiece; namespace net { -#define ENDPOINT (is_server_ ? "Server: " : " Client: ") +#define ENDPOINT (session()->is_server() ? "Server: " : " Client: ") QuicCryptoStream::QuicCryptoStream(QuicSession* session) : ReliableQuicStream(kCryptoStreamId, session), encryption_established_(false), - handshake_confirmed_(false), - is_server_(session->is_server()) { + handshake_confirmed_(false) { crypto_framer_.set_visitor(this); if (version() <= QUIC_VERSION_20) { // Prior to QUIC_VERSION_21 the crypto stream is not subject to any flow diff --git a/net/quic/quic_crypto_stream.h b/net/quic/quic_crypto_stream.h index 4cce73c..dc8b227 100644 --- a/net/quic/quic_crypto_stream.h +++ b/net/quic/quic_crypto_stream.h @@ -73,8 +73,6 @@ class NET_EXPORT_PRIVATE QuicCryptoStream private: CryptoFramer crypto_framer_; - bool is_server_; - DISALLOW_COPY_AND_ASSIGN(QuicCryptoStream); }; diff --git a/net/quic/quic_dispatcher.h b/net/quic/quic_dispatcher.h index 684461d..2687e8b 100644 --- a/net/quic/quic_dispatcher.h +++ b/net/quic/quic_dispatcher.h @@ -70,7 +70,7 @@ class QuicDispatcher : public QuicBlockedWriterInterface, virtual ~QuicDispatcher(); - // Takes ownership of the packet writer + // Takes ownership of the packet writer. virtual void Initialize(QuicServerPacketWriter* writer); // Process the incoming packet by creating a new session, passing it to diff --git a/net/quic/quic_framer.cc b/net/quic/quic_framer.cc index 027392c..fe70501 100644 --- a/net/quic/quic_framer.cc +++ b/net/quic/quic_framer.cc @@ -975,16 +975,15 @@ QuicFramer::AckFrameInfo QuicFramer::GetAckFrameInfo( *iter == (last_missing + 1)) { ++cur_range_length; } else { - ack_info.nack_ranges[last_missing - cur_range_length] - = cur_range_length; + ack_info.nack_ranges[last_missing - cur_range_length] = + cur_range_length; cur_range_length = 0; } ack_info.max_delta = max(ack_info.max_delta, *iter - last_missing); last_missing = *iter; } // Include the last nack range. - ack_info.nack_ranges[last_missing - cur_range_length] = - cur_range_length; + ack_info.nack_ranges[last_missing - cur_range_length] = cur_range_length; // Include the range to the largest observed. ack_info.max_delta = max(ack_info.max_delta, frame.largest_observed - last_missing); @@ -1419,53 +1418,8 @@ bool QuicFramer::ProcessQuicCongestionFeedbackFrame( switch (frame->type) { case kTimestamp: { - CongestionFeedbackMessageTimestamp* timestamp = &frame->timestamp; - uint8 num_received_packets; - if (!reader_->ReadBytes(&num_received_packets, 1)) { - set_detailed_error("Unable to read num received packets."); - return false; - } - - if (num_received_packets > 0u) { - uint64 smallest_received; - if (!ProcessPacketSequenceNumber(PACKET_6BYTE_SEQUENCE_NUMBER, - &smallest_received)) { - set_detailed_error("Unable to read smallest received."); - return false; - } - - uint64 time_received_us; - if (!reader_->ReadUInt64(&time_received_us)) { - set_detailed_error("Unable to read time received."); - return false; - } - QuicTime time_received = creation_time_.Add( - QuicTime::Delta::FromMicroseconds(time_received_us)); - - timestamp->received_packet_times.insert( - make_pair(smallest_received, time_received)); - - for (uint8 i = 0; i < num_received_packets - 1; ++i) { - uint16 sequence_delta; - if (!reader_->ReadUInt16(&sequence_delta)) { - set_detailed_error( - "Unable to read sequence delta in received packets."); - return false; - } - - int32 time_delta_us; - if (!reader_->ReadBytes(&time_delta_us, sizeof(time_delta_us))) { - set_detailed_error( - "Unable to read time delta in received packets."); - return false; - } - QuicPacketSequenceNumber packet = smallest_received + sequence_delta; - timestamp->received_packet_times.insert( - make_pair(packet, time_received.Add( - QuicTime::Delta::FromMicroseconds(time_delta_us)))); - } - } - break; + set_detailed_error("Timestamp feedback not supported."); + return false; } case kTCP: { CongestionFeedbackMessageTCP* tcp = &frame->tcp; @@ -1788,16 +1742,7 @@ size_t QuicFramer::ComputeFrameLength( switch (congestion_feedback.type) { case kTimestamp: { - const CongestionFeedbackMessageTimestamp& timestamp = - congestion_feedback.timestamp; - len += 1; // Number received packets. - if (!timestamp.received_packet_times.empty()) { - len += PACKET_6BYTE_SEQUENCE_NUMBER; // Smallest received. - len += 8; // Time. - // 2 bytes per sequence number delta plus 4 bytes per delta time. - len += PACKET_6BYTE_SEQUENCE_NUMBER * - (timestamp.received_packet_times.size() - 1); - } + set_detailed_error("Timestamp feedback not supported."); break; } case kTCP: @@ -2094,7 +2039,8 @@ bool QuicFramer::AppendCongestionFeedbackFrame( switch (frame.type) { case kTimestamp: { - return AppendTimestampFrame(frame, writer); + // Timestamp feedback not supported. + return false; } case kTCP: { const CongestionFeedbackMessageTCP& tcp = frame.tcp; @@ -2113,53 +2059,6 @@ bool QuicFramer::AppendCongestionFeedbackFrame( return true; } -bool QuicFramer::AppendTimestampFrame( - const QuicCongestionFeedbackFrame& frame, - QuicDataWriter* writer) { - const CongestionFeedbackMessageTimestamp& timestamp = frame.timestamp; - DCHECK_GE(numeric_limits<uint8>::max(), - timestamp.received_packet_times.size()); - if (timestamp.received_packet_times.size() > numeric_limits<uint8>::max()) { - return false; - } - uint8 num_received_packets = timestamp.received_packet_times.size(); - if (!writer->WriteBytes(&num_received_packets, 1)) { - return false; - } - if (num_received_packets > 0) { - TimeMap::const_iterator it = timestamp.received_packet_times.begin(); - - QuicPacketSequenceNumber lowest_sequence = it->first; - if (!AppendPacketSequenceNumber(PACKET_6BYTE_SEQUENCE_NUMBER, - lowest_sequence, writer)) { - return false; - } - - QuicTime lowest_time = it->second; - if (!writer->WriteUInt64( - lowest_time.Subtract(creation_time_).ToMicroseconds())) { - return false; - } - - for (++it; it != timestamp.received_packet_times.end(); ++it) { - QuicPacketSequenceNumber sequence_delta = it->first - lowest_sequence; - DCHECK_GE(numeric_limits<uint16>::max(), sequence_delta); - if (sequence_delta > numeric_limits<uint16>::max()) { - return false; - } - if (!writer->WriteUInt16(static_cast<uint16>(sequence_delta))) { - return false; - } - - int32 time_delta_us = it->second.Subtract(lowest_time).ToMicroseconds(); - if (!writer->WriteBytes(&time_delta_us, sizeof(time_delta_us))) { - return false; - } - } - } - return true; -} - bool QuicFramer::AppendStopWaitingFrame( const QuicPacketHeader& header, const QuicStopWaitingFrame& frame, diff --git a/net/quic/quic_framer.h b/net/quic/quic_framer.h index dc94b86..a958786 100644 --- a/net/quic/quic_framer.h +++ b/net/quic/quic_framer.h @@ -463,8 +463,6 @@ class NET_EXPORT_PRIVATE QuicFramer { QuicDataWriter* builder); bool AppendCongestionFeedbackFrame(const QuicCongestionFeedbackFrame& frame, QuicDataWriter* builder); - bool AppendTimestampFrame(const QuicCongestionFeedbackFrame& frame, - QuicDataWriter* builder); bool AppendStopWaitingFrame(const QuicPacketHeader& header, const QuicStopWaitingFrame& frame, QuicDataWriter* builder); diff --git a/net/quic/quic_framer_test.cc b/net/quic/quic_framer_test.cc index 60aad46..d14e673 100644 --- a/net/quic/quic_framer_test.cc +++ b/net/quic/quic_framer_test.cc @@ -2044,96 +2044,6 @@ TEST_P(QuicFramerTest, CongestionFeedbackFrameTCP) { } } -TEST_P(QuicFramerTest, CongestionFeedbackFrameTimestamp) { - unsigned char packet[] = { - // public flags (8 byte connection_id) - 0x3C, - // connection_id - 0x10, 0x32, 0x54, 0x76, - 0x98, 0xBA, 0xDC, 0xFE, - // packet sequence number - 0xBC, 0x9A, 0x78, 0x56, - 0x34, 0x12, - // private flags - 0x00, - - // frame type (congestion feedback frame) - 0x20, - // congestion feedback type (timestamp) - 0x01, - // num received packets - 0x03, - // lowest sequence number - 0xBA, 0x9A, 0x78, 0x56, - 0x34, 0x12, - // receive time - 0x87, 0x96, 0xA5, 0xB4, - 0xC3, 0xD2, 0xE1, 0x07, - // sequence delta - 0x01, 0x00, - // time delta - 0x01, 0x00, 0x00, 0x00, - // sequence delta (skip one packet) - 0x03, 0x00, - // time delta - 0x02, 0x00, 0x00, 0x00, - }; - - QuicEncryptedPacket encrypted(AsChars(packet), arraysize(packet), false); - EXPECT_TRUE(framer_.ProcessPacket(encrypted)); - - EXPECT_EQ(QUIC_NO_ERROR, framer_.error()); - ASSERT_TRUE(visitor_.header_.get()); - EXPECT_TRUE(CheckDecryption(encrypted, !kIncludeVersion)); - - EXPECT_EQ(0u, visitor_.stream_frames_.size()); - ASSERT_EQ(1u, visitor_.congestion_feedback_frames_.size()); - const QuicCongestionFeedbackFrame& frame = - *visitor_.congestion_feedback_frames_[0]; - ASSERT_EQ(kTimestamp, frame.type); - ASSERT_EQ(3u, frame.timestamp.received_packet_times.size()); - TimeMap::const_iterator iter = - frame.timestamp.received_packet_times.begin(); - EXPECT_EQ(GG_UINT64_C(0x0123456789ABA), iter->first); - EXPECT_EQ(GG_INT64_C(0x07E1D2C3B4A59687), - iter->second.Subtract(start_).ToMicroseconds()); - ++iter; - EXPECT_EQ(GG_UINT64_C(0x0123456789ABB), iter->first); - EXPECT_EQ(GG_INT64_C(0x07E1D2C3B4A59688), - iter->second.Subtract(start_).ToMicroseconds()); - ++iter; - EXPECT_EQ(GG_UINT64_C(0x0123456789ABD), iter->first); - EXPECT_EQ(GG_INT64_C(0x07E1D2C3B4A59689), - iter->second.Subtract(start_).ToMicroseconds()); - - // Now test framing boundaries - for (size_t i = kQuicFrameTypeSize; i < 29; ++i) { - string expected_error; - if (i < 2) { - expected_error = "Unable to read congestion feedback type."; - } else if (i < 3) { - expected_error = "Unable to read num received packets."; - } else if (i < 9) { - expected_error = "Unable to read smallest received."; - } else if (i < 17) { - expected_error = "Unable to read time received."; - } else if (i < 19) { - expected_error = "Unable to read sequence delta in received packets."; - } else if (i < 23) { - expected_error = "Unable to read time delta in received packets."; - } else if (i < 25) { - expected_error = "Unable to read sequence delta in received packets."; - } else if (i < 29) { - expected_error = "Unable to read time delta in received packets."; - } - CheckProcessingFails( - packet, - i + GetPacketHeaderSize(PACKET_8BYTE_CONNECTION_ID, !kIncludeVersion, - PACKET_6BYTE_SEQUENCE_NUMBER, NOT_IN_FEC_GROUP), - expected_error, QUIC_INVALID_CONGESTION_FEEDBACK_DATA); - } -} - TEST_P(QuicFramerTest, CongestionFeedbackFrameInvalidFeedback) { unsigned char packet[] = { // public flags (8 byte connection_id) @@ -3428,75 +3338,6 @@ TEST_P(QuicFramerTest, BuildCongestionFeedbackFramePacketTCP) { AsChars(packet), arraysize(packet)); } -TEST_P(QuicFramerTest, BuildCongestionFeedbackFramePacketTimestamp) { - QuicPacketHeader header; - header.public_header.connection_id = GG_UINT64_C(0xFEDCBA9876543210); - header.public_header.reset_flag = false; - header.public_header.version_flag = false; - header.fec_flag = false; - header.entropy_flag = false; - header.packet_sequence_number = GG_UINT64_C(0x123456789ABC); - header.fec_group = 0; - - QuicCongestionFeedbackFrame frame; - frame.type = kTimestamp; - frame.timestamp.received_packet_times.insert( - make_pair(GG_UINT64_C(0x0123456789ABA), - start_.Add(QuicTime::Delta::FromMicroseconds( - GG_UINT64_C(0x07E1D2C3B4A59687))))); - frame.timestamp.received_packet_times.insert( - make_pair(GG_UINT64_C(0x0123456789ABB), - start_.Add(QuicTime::Delta::FromMicroseconds( - GG_UINT64_C(0x07E1D2C3B4A59688))))); - frame.timestamp.received_packet_times.insert( - make_pair(GG_UINT64_C(0x0123456789ABD), - start_.Add(QuicTime::Delta::FromMicroseconds( - GG_UINT64_C(0x07E1D2C3B4A59689))))); - QuicFrames frames; - frames.push_back(QuicFrame(&frame)); - - unsigned char packet[] = { - // public flags (8 byte connection_id) - 0x3C, - // connection_id - 0x10, 0x32, 0x54, 0x76, - 0x98, 0xBA, 0xDC, 0xFE, - // packet sequence number - 0xBC, 0x9A, 0x78, 0x56, - 0x34, 0x12, - // private flags - 0x00, - - // frame type (congestion feedback frame) - 0x20, - // congestion feedback type (timestamp) - 0x01, - // num received packets - 0x03, - // lowest sequence number - 0xBA, 0x9A, 0x78, 0x56, - 0x34, 0x12, - // receive time - 0x87, 0x96, 0xA5, 0xB4, - 0xC3, 0xD2, 0xE1, 0x07, - // sequence delta - 0x01, 0x00, - // time delta - 0x01, 0x00, 0x00, 0x00, - // sequence delta (skip one packet) - 0x03, 0x00, - // time delta - 0x02, 0x00, 0x00, 0x00, - }; - - scoped_ptr<QuicPacket> data(BuildDataPacket(header, frames)); - ASSERT_TRUE(data != NULL); - - test::CompareCharArraysWithHexError("constructed packet", - data->data(), data->length(), - AsChars(packet), arraysize(packet)); -} - TEST_P(QuicFramerTest, BuildStopWaitingPacket) { QuicPacketHeader header; header.public_header.connection_id = GG_UINT64_C(0xFEDCBA9876543210); diff --git a/net/quic/quic_protocol.h b/net/quic/quic_protocol.h index d91b55f..6b29331 100644 --- a/net/quic/quic_protocol.h +++ b/net/quic/quic_protocol.h @@ -70,7 +70,7 @@ const uint32 kDefaultFlowControlSendWindow = 16 * 1024; // 16 KB const size_t kMaxTcpCongestionWindow = 200; // Size of the socket receive buffer in bytes. -const QuicByteCount kDefaultSocketReceiveBuffer = 256000; +const QuicByteCount kDefaultSocketReceiveBuffer = 256 * 1024; // Don't allow a client to suggest an RTT longer than 15 seconds. const uint32 kMaxInitialRoundTripTimeUs = 15 * kNumMicrosPerSecond; @@ -104,6 +104,9 @@ const QuicStreamId kCryptoStreamId = 1; // Reserved ID for the headers stream. const QuicStreamId kHeadersStreamId = 3; +// Maximum delayed ack time, in ms. +const int kMaxDelayedAckTime = 25; + // This is the default network timeout a for connection till the crypto // handshake succeeds and the negotiated timeout from the handshake is received. const int64 kDefaultInitialTimeoutSecs = 120; // 2 mins. diff --git a/net/quic/quic_received_packet_manager.h b/net/quic/quic_received_packet_manager.h index b63d292..94cfbc6 100644 --- a/net/quic/quic_received_packet_manager.h +++ b/net/quic/quic_received_packet_manager.h @@ -114,7 +114,7 @@ class NET_EXPORT_PRIVATE QuicReceivedPacketManager : // Checks if we're still waiting for the packet with |sequence_number|. bool IsAwaitingPacket(QuicPacketSequenceNumber sequence_number); - // Update the |received_info| for an outgoing ack. + // Update the |ack_frame| for an outgoing ack. void UpdateReceivedPacketInfo(QuicAckFrame* ack_frame, QuicTime approximate_now); @@ -131,7 +131,7 @@ class NET_EXPORT_PRIVATE QuicReceivedPacketManager : virtual QuicPacketEntropyHash EntropyHash( QuicPacketSequenceNumber sequence_number) const OVERRIDE; - // Updates internal state based on |received_info|. + // Updates internal state based on |ack_frame|. void UpdatePacketInformationReceivedByPeer(const QuicAckFrame& ack_frame); // Updates internal state based on |stop_waiting|. void UpdatePacketInformationSentByPeer( diff --git a/net/quic/quic_sent_packet_manager.cc b/net/quic/quic_sent_packet_manager.cc index a0ce684..292afa6 100644 --- a/net/quic/quic_sent_packet_manager.cc +++ b/net/quic/quic_sent_packet_manager.cc @@ -49,8 +49,6 @@ static const size_t kNumMinRttSamplesAfterQuiescence = 2; // Number of unpaced packets to send after quiescence. static const size_t kInitialUnpacedBurst = 10; -// Use a 1 minute window for Recent Min RTT with BBR. - bool HasCryptoHandshake(const TransmissionInfo& transmission_info) { if (transmission_info.retransmittable_frames == NULL) { return false; @@ -115,9 +113,8 @@ void QuicSentPacketManager::SetFromConfig(const QuicConfig& config) { send_algorithm_.reset( SendAlgorithmInterface::Create(clock_, &rtt_stats_, kReno, stats_)); } - if (config.congestion_feedback() == kPACE || - (config.HasReceivedConnectionOptions() && - ContainsQuicTag(config.ReceivedConnectionOptions(), kPACE))) { + if (config.HasReceivedConnectionOptions() && + ContainsQuicTag(config.ReceivedConnectionOptions(), kPACE)) { MaybeEnablePacing(); } // TODO(ianswett): Remove the "HasReceivedLossDetection" branch once @@ -740,6 +737,8 @@ QuicTime::Delta QuicSentPacketManager::TimeUntilSend( now, unacked_packets_.bytes_in_flight(), retransmittable); } +// Uses a 25ms delayed ack timer. Also helps with better signaling +// in low-bandwidth (< ~384 kbps), where an ack is sent per packet. // Ensures that the Delayed Ack timer is always set to a value lesser // than the retransmission timer's minimum value (MinRTO). We want the // delayed ack to get back to the QUIC peer before the sender's @@ -753,7 +752,8 @@ QuicTime::Delta QuicSentPacketManager::TimeUntilSend( // any benefits, but if the delayed ack becomes a significant source // of (likely, tail) latency, then consider such a mechanism. const QuicTime::Delta QuicSentPacketManager::DelayedAckTime() const { - return QuicTime::Delta::FromMilliseconds(kMinRetransmissionTimeMs/2); + return QuicTime::Delta::FromMilliseconds(min(kMaxDelayedAckTime, + kMinRetransmissionTimeMs/2)); } const QuicTime QuicSentPacketManager::GetRetransmissionTime() const { @@ -805,7 +805,8 @@ const QuicTime::Delta QuicSentPacketManager::GetTailLossProbeDelay() const { QuicTime::Delta srtt = rtt_stats_.SmoothedRtt(); if (!unacked_packets_.HasMultipleInFlightPackets()) { return QuicTime::Delta::Max( - srtt.Multiply(1.5).Add(DelayedAckTime()), srtt.Multiply(2)); + srtt.Multiply(2), srtt.Multiply(1.5) + .Add(QuicTime::Delta::FromMilliseconds(kMinRetransmissionTimeMs/2))); } return QuicTime::Delta::FromMilliseconds( max(kMinTailLossProbeTimeoutMs, diff --git a/net/quic/quic_stream_factory.cc b/net/quic/quic_stream_factory.cc index ea30b2e..6ecb057 100644 --- a/net/quic/quic_stream_factory.cc +++ b/net/quic/quic_stream_factory.cc @@ -85,12 +85,10 @@ bool IsEcdsaSupported() { return true; } -QuicConfig InitializeQuicConfig(bool enable_pacing, - bool enable_time_based_loss_detection, +QuicConfig InitializeQuicConfig(bool enable_time_based_loss_detection, const QuicTagVector& connection_options) { QuicConfig config; config.SetDefaults(); - config.EnablePacing(enable_pacing); if (enable_time_based_loss_detection) config.SetLossDetectionToSend(kTIME); config.set_idle_connection_state_lifetime( @@ -462,7 +460,6 @@ QuicStreamFactory::QuicStreamFactory( const std::string& user_agent_id, const QuicVersionVector& supported_versions, bool enable_port_selection, - bool enable_pacing, bool enable_time_based_loss_detection, const QuicTagVector& connection_options) : require_confirmation_(true), @@ -474,8 +471,7 @@ QuicStreamFactory::QuicStreamFactory( random_generator_(random_generator), clock_(clock), max_packet_length_(max_packet_length), - config_(InitializeQuicConfig(enable_pacing, - enable_time_based_loss_detection, + config_(InitializeQuicConfig(enable_time_based_loss_detection, connection_options)), supported_versions_(supported_versions), enable_port_selection_(enable_port_selection), diff --git a/net/quic/quic_stream_factory.h b/net/quic/quic_stream_factory.h index 574bf6f..1d66fb5 100644 --- a/net/quic/quic_stream_factory.h +++ b/net/quic/quic_stream_factory.h @@ -102,7 +102,6 @@ class NET_EXPORT_PRIVATE QuicStreamFactory const std::string& user_agent_id, const QuicVersionVector& supported_versions, bool enable_port_selection, - bool enable_pacing, bool enable_time_based_loss_detection, const QuicTagVector& connection_options); virtual ~QuicStreamFactory(); diff --git a/net/quic/quic_stream_factory_test.cc b/net/quic/quic_stream_factory_test.cc index df66134..7c0b8af 100644 --- a/net/quic/quic_stream_factory_test.cc +++ b/net/quic/quic_stream_factory_test.cc @@ -100,8 +100,7 @@ class QuicStreamFactoryTest : public ::testing::TestWithParam<QuicVersion> { channel_id_service_.get(), &transport_security_state_, &crypto_client_stream_factory_, &random_generator_, clock_, kDefaultMaxPacketSize, std::string(), - SupportedVersions(GetParam()), true, true, true, - QuicTagVector()), + SupportedVersions(GetParam()), true, true, QuicTagVector()), host_port_pair_(kDefaultServerHostName, kDefaultServerPort), is_https_(false), privacy_mode_(PRIVACY_MODE_DISABLED) { diff --git a/net/quic/test_tools/quic_test_utils.h b/net/quic/test_tools/quic_test_utils.h index 350bc37..ce6877e 100644 --- a/net/quic/test_tools/quic_test_utils.h +++ b/net/quic/test_tools/quic_test_utils.h @@ -90,8 +90,8 @@ QuicConfig DefaultQuicConfig(); // Returns a version vector consisting of |version|. QuicVersionVector SupportedVersions(QuicVersion version); -// Testing convenience method to construct a QuicAckFrame with all packets -// from least_unacked to largest_observed acked. +// Testing convenience method to construct a QuicAckFrame with entropy_hash set +// to 0 and largest_observed from peer set to |largest_observed|. QuicAckFrame MakeAckFrame(QuicPacketSequenceNumber largest_observed); // Testing convenience method to construct a QuicAckFrame with |num_nack_ranges| diff --git a/net/tools/quic/end_to_end_test.cc b/net/tools/quic/end_to_end_test.cc index fe1bbd1..0c86d03 100644 --- a/net/tools/quic/end_to_end_test.cc +++ b/net/tools/quic/end_to_end_test.cc @@ -292,6 +292,11 @@ class EndToEndTest : public ::testing::TestWithParam<TestParams> { bool Initialize() { QuicTagVector copt; + if (GetParam().use_pacing) { + copt.push_back(kPACE); + } + server_config_.SetConnectionOptionsToSend(copt); + // TODO(nimia): Consider setting the congestion control algorithm for the // client as well according to the test parameter. copt.push_back(GetParam().congestion_control_tag); |