diff options
author | zhongyi <zhongyi@chromium.org> | 2015-11-30 21:51:30 -0800 |
---|---|---|
committer | Commit bot <commit-bot@chromium.org> | 2015-12-01 05:52:31 +0000 |
commit | b867702268009cc8d2be5c67559a1f10e55ee45d (patch) | |
tree | 6333bbb739ab9fc0cc7b114c57572c4519c0d7a5 | |
parent | 5d5df7d2076c1dce20d3550dd21589157909ed2c (diff) | |
download | chromium_src-b867702268009cc8d2be5c67559a1f10e55ee45d.zip chromium_src-b867702268009cc8d2be5c67559a1f10e55ee45d.tar.gz chromium_src-b867702268009cc8d2be5c67559a1f10e55ee45d.tar.bz2 |
Landing Recent QUIC changes until and including Mon Nov 23 17:03:24.
Set validate_client_hello_cb_ to nullptr after cancellation. Protected by default enabled --quic_set_client_hello_cb_nullptr
Merge internal change: 108506727
https://codereview.chromium.org/1481673002/
n/a (QUIC test server code)
SendReponse() now checks response :status to be valid http/2 response
status (integer) before proceeding;
Merge internal change: 108359314
https://codereview.chromium.org/1480793002/
n/a(add new unused interfaces to QuicInMemoryCache)
Add more members to QuicInMemoryCache to support server push. Currently
I added a multimap to keep the association between request and push
resources. And also added two public methods to add response which has
server push resources and to get these resources with request.
Merge internal change: 108138965
https://codereview.chromium.org/1479443003
Deprecate --quic_fix_fin_accounting
Merge internal change: 108509430
https://codereview.chromium.org/1478893002/
Pretty-print QUIC RREJ tag values as a list of enum strings instead of a blob of hex.
Merge internal change: 108500935
https://codereview.chromium.org/1479663002/
Move packet header validation checking into ProcessValidatedPacket. Only migrates connection if peer address change and this packet can be handled.
Merge internal change: 108420190
https://codereview.chromium.org/1477013002/
Combine OnSynStream and OnSynReply into a single OnHeaders method. No behavior change.
Also fixes comments that refer to SYN_STREAM and SYN_REPLY.
Merge internal change: 108368234
https://codereview.chromium.org/1476093002/
Reserve 2 entries in the frames held in QUIC's RetransmittableFrames. No functional change.
Saves 1% of CPU in the local loadtest in new and delete.
Merge internal change: 108356497
https://codereview.chromium.org/1475913002/
Add a new QUIC connection option, kACKD, allowing QUIC to ack less frequently. Flag protected by quic_ack_decimation.
Merge internal change: 108337600
https://codereview.chromium.org/1465223005/
Protected by --quic_connection_options_to_transport_stats.
Merge internal change: 108265760
https://codereview.chromium.org/1474943002/
Pass StringPiece by value rather than reference in QUIC NullDecrypter.
Merge internal change: 108252135
https://codereview.chromium.org/1473633003/
Deprecate --allow_many_available_streams
DELTA_BY_EXTENSION=cc=35,cfg=6
Merge internal change: 108244052
https://codereview.chromium.org/1472083004/
deprecating quic_implement_stop_reading
Merge internal change: 108241751
https://codereview.chromium.org/1473243004/
Refactor to break QUIC's ack queueing in to a separate method, no functional change.
Merge internal change: 108171697
https://codereview.chromium.org/1470393003/
renaming SPDY priorities for clarity
Merge internal change: 108141573
https://codereview.chromium.org/1474933002/
Refactor of QUIC ack code and minor change to current behavior by sending an ack immediately when 20 packets are received instead of setting the alarm.
This refactor will make it easier to change the acking policy in future
CLs via a connection option.
Merge internal change: 108134651
https://codereview.chromium.org/1481463002/
Isolate most FEC logic in QuicPacketCreator. No functional change expected.
QuicPacketGenerator:
Add OnFecGroupReset.
Move MaybeStartFecProtection, MaybeSendFecPacketAndCloseGroup,
fec_send_policy_, fec_timeout_, rtt_multiplier_for_fec_timeout_,
should_fec_protect_ to Creator.
Remove ResetFecGroup, ShouldSendFecPacket.
QuicPacketCreator:
Add OnResetFecGroup to Delegate. Add OnRttChange, GetFecTimeout.
Remove IsFecEnabled, IsFecProtected, fec_group_number().
Move StartFecProtectingPackets, StopFecProtectingPackets, ShouldSendFec,
ResetFecGroup from public to private.
Merge internal change: 108084786
https://codereview.chromium.org/1472263002/
Use public flags 0x40 bit as PUBLIC_FLAG_MULTIPATH, indicating whether a 1-byte QuicPathId is presented in packet header.
QuicFrame can write and process this flag and the path id.
Merge internal change: 108080231
https://codereview.chromium.org/1470113004/
Reset rtt estimates and TCP cubic congestion control on connection migrations which are not caused by NATs.
Merge internal change: 108078794
https://codereview.chromium.org/1473803002/
BUG=
Review URL: https://codereview.chromium.org/1485963002
Cr-Commit-Position: refs/heads/master@{#362359}
69 files changed, 1958 insertions, 875 deletions
diff --git a/net/net.gypi b/net/net.gypi index 969a61d..58261b7 100644 --- a/net/net.gypi +++ b/net/net.gypi @@ -1512,6 +1512,7 @@ 'quic/crypto/channel_id_test.cc', 'quic/crypto/common_cert_set_test.cc', 'quic/crypto/crypto_framer_test.cc', + 'quic/crypto/crypto_handshake_message_test.cc', 'quic/crypto/crypto_secret_boxer_test.cc', 'quic/crypto/crypto_server_test.cc', 'quic/crypto/crypto_utils_test.cc', @@ -1726,6 +1727,8 @@ 'tools/balsa/balsa_frame_test.cc', 'tools/balsa/balsa_headers_test.cc', 'tools/quic/quic_simple_client_test.cc', + 'tools/quic/test_tools/mock_quic_server_session_visitor.cc', + 'tools/quic/test_tools/mock_quic_server_session_visitor.h', 'tools/tld_cleanup/tld_cleanup_util_unittest.cc', 'udp/udp_socket_unittest.cc', 'url_request/certificate_report_sender_unittest.cc', @@ -1792,8 +1795,6 @@ 'tools/quic/test_tools/limited_mtu_test_writer.h', 'tools/quic/test_tools/mock_epoll_server.cc', 'tools/quic/test_tools/mock_epoll_server.h', - 'tools/quic/test_tools/mock_quic_server_session_visitor.cc', - 'tools/quic/test_tools/mock_quic_server_session_visitor.h', 'tools/quic/test_tools/mock_quic_time_wait_list_manager.cc', 'tools/quic/test_tools/mock_quic_time_wait_list_manager.h', 'tools/quic/test_tools/packet_dropping_test_writer.cc', diff --git a/net/quic/congestion_control/rtt_stats.cc b/net/quic/congestion_control/rtt_stats.cc index 02a5dfb..3bc08bf 100644 --- a/net/quic/congestion_control/rtt_stats.cc +++ b/net/quic/congestion_control/rtt_stats.cc @@ -127,4 +127,15 @@ void RttStats::UpdateRecentMinRtt(QuicTime::Delta rtt_sample, QuicTime now) { } } +void RttStats::OnConnectionMigration() { + latest_rtt_ = QuicTime::Delta::Zero(); + min_rtt_ = QuicTime::Delta::Zero(); + smoothed_rtt_ = QuicTime::Delta::Zero(); + mean_deviation_ = QuicTime::Delta::Zero(); + initial_rtt_us_ = kInitialRttMs * kNumMicrosPerMilli; + num_min_rtt_samples_remaining_ = 0; + recent_min_rtt_window_ = QuicTime::Delta::Infinite(); + recent_min_rtt_ = half_window_rtt_ = quarter_window_rtt_ = RttSample(); +} + } // namespace net diff --git a/net/quic/congestion_control/rtt_stats.h b/net/quic/congestion_control/rtt_stats.h index df1df773..0b888f3 100644 --- a/net/quic/congestion_control/rtt_stats.h +++ b/net/quic/congestion_control/rtt_stats.h @@ -39,7 +39,7 @@ class NET_EXPORT_PRIVATE RttStats { void SampleNewRecentMinRtt(uint32 num_samples); // Called when connection migrates and rtt measurement needs to be reset. - void OnConnectionMigration() {} + void OnConnectionMigration(); // Returns the EWMA smoothed RTT for the connection. // May return Zero if no valid updates have occurred. diff --git a/net/quic/congestion_control/rtt_stats_test.cc b/net/quic/congestion_control/rtt_stats_test.cc index 86c7973..931fcb0 100644 --- a/net/quic/congestion_control/rtt_stats_test.cc +++ b/net/quic/congestion_control/rtt_stats_test.cc @@ -243,5 +243,23 @@ TEST_F(RttStatsTest, UpdateRttWithBadSendDeltas) { } } +TEST_F(RttStatsTest, ResetAfterConnectionMigrations) { + rtt_stats_.UpdateRtt(QuicTime::Delta::FromMilliseconds(300), + QuicTime::Delta::FromMilliseconds(100), + QuicTime::Zero()); + EXPECT_EQ(QuicTime::Delta::FromMilliseconds(200), rtt_stats_.latest_rtt()); + EXPECT_EQ(QuicTime::Delta::FromMilliseconds(200), rtt_stats_.smoothed_rtt()); + EXPECT_EQ(QuicTime::Delta::FromMilliseconds(300), rtt_stats_.min_rtt()); + EXPECT_EQ(QuicTime::Delta::FromMilliseconds(300), + rtt_stats_.recent_min_rtt()); + + // Reset rtt stats on connection migrations. + rtt_stats_.OnConnectionMigration(); + EXPECT_EQ(QuicTime::Delta::Zero(), rtt_stats_.latest_rtt()); + EXPECT_EQ(QuicTime::Delta::Zero(), rtt_stats_.smoothed_rtt()); + EXPECT_EQ(QuicTime::Delta::Zero(), rtt_stats_.min_rtt()); + EXPECT_EQ(QuicTime::Delta::Zero(), rtt_stats_.recent_min_rtt()); +} + } // namespace test } // namespace net diff --git a/net/quic/congestion_control/tcp_cubic_bytes_sender.cc b/net/quic/congestion_control/tcp_cubic_bytes_sender.cc index 0dd8169..e2d9519 100644 --- a/net/quic/congestion_control/tcp_cubic_bytes_sender.cc +++ b/net/quic/congestion_control/tcp_cubic_bytes_sender.cc @@ -48,7 +48,11 @@ TcpCubicBytesSender::TcpCubicBytesSender( min4_mode_(false), max_congestion_window_(max_congestion_window * kDefaultTCPMSS), slowstart_threshold_(max_congestion_window * kDefaultTCPMSS), - last_cutback_exited_slowstart_(false) {} + last_cutback_exited_slowstart_(false), + initial_tcp_congestion_window_(initial_tcp_congestion_window * + kDefaultTCPMSS), + initial_max_tcp_congestion_window_(max_congestion_window * + kDefaultTCPMSS) {} TcpCubicBytesSender::~TcpCubicBytesSender() { } @@ -351,4 +355,18 @@ CongestionControlType TcpCubicBytesSender::GetCongestionControlType() const { return reno_ ? kRenoBytes : kCubicBytes; } +void TcpCubicBytesSender::OnConnectionMigration() { + hybrid_slow_start_.Restart(); + cubic_.Reset(); + prr_ = PrrSender(); + num_acked_packets_ = 0; + largest_sent_packet_number_ = 0; + largest_acked_packet_number_ = 0; + largest_sent_at_last_cutback_ = 0; + congestion_window_ = initial_tcp_congestion_window_; + max_congestion_window_ = initial_max_tcp_congestion_window_; + slowstart_threshold_ = initial_max_tcp_congestion_window_; + last_cutback_exited_slowstart_ = false; +} + } // namespace net diff --git a/net/quic/congestion_control/tcp_cubic_bytes_sender.h b/net/quic/congestion_control/tcp_cubic_bytes_sender.h index e340fcc..4113794 100644 --- a/net/quic/congestion_control/tcp_cubic_bytes_sender.h +++ b/net/quic/congestion_control/tcp_cubic_bytes_sender.h @@ -54,7 +54,7 @@ class NET_EXPORT_PRIVATE TcpCubicBytesSender : public SendAlgorithmInterface { QuicByteCount bytes, HasRetransmittableData is_retransmittable) override; void OnRetransmissionTimeout(bool packets_retransmitted) override; - void OnConnectionMigration() override {} + void OnConnectionMigration() override; QuicTime::Delta TimeUntilSend( QuicTime now, QuicByteCount bytes_in_flight, @@ -130,6 +130,14 @@ class NET_EXPORT_PRIVATE TcpCubicBytesSender : public SendAlgorithmInterface { // collection of slowstart_packets_lost. bool last_cutback_exited_slowstart_; + // Initial TCP congestion window in bytes. This variable can only be set when + // this algorithm is created. + const QuicByteCount initial_tcp_congestion_window_; + + // Initial maximum TCP congestion window in bytes. This variable can only be + // set when this algorithm is created. + const QuicByteCount initial_max_tcp_congestion_window_; + DISALLOW_COPY_AND_ASSIGN(TcpCubicBytesSender); }; diff --git a/net/quic/congestion_control/tcp_cubic_bytes_sender_test.cc b/net/quic/congestion_control/tcp_cubic_bytes_sender_test.cc index 70606dc..ef3230e 100644 --- a/net/quic/congestion_control/tcp_cubic_bytes_sender_test.cc +++ b/net/quic/congestion_control/tcp_cubic_bytes_sender_test.cc @@ -643,5 +643,36 @@ TEST_F(TcpCubicBytesSenderTest, PaceBelowCWND) { HAS_RETRANSMITTABLE_DATA).IsZero()); } +TEST_F(TcpCubicBytesSenderTest, ResetAfterConnectionMigration) { + // Starts from slow start. + sender_->SetNumEmulatedConnections(1); + const int kNumberOfAcks = 10; + for (int i = 0; i < kNumberOfAcks; ++i) { + // Send our full send window. + SendAvailableSendWindow(); + AckNPackets(2); + } + SendAvailableSendWindow(); + QuicByteCount expected_send_window = + kDefaultWindowTCP + (kDefaultTCPMSS * 2 * kNumberOfAcks); + EXPECT_EQ(expected_send_window, sender_->GetCongestionWindow()); + + // Loses a packet to exit slow start. + LoseNPackets(1); + + // We should now have fallen out of slow start with a reduced window. Slow + // start threshold is also updated. + expected_send_window *= kRenoBeta; + EXPECT_EQ(expected_send_window, sender_->GetCongestionWindow()); + EXPECT_EQ(expected_send_window, sender_->GetSlowStartThreshold()); + + // Resets cwnd and slow start threshold on connection migrations. + sender_->OnConnectionMigration(); + EXPECT_EQ(kDefaultWindowTCP, sender_->GetCongestionWindow()); + EXPECT_EQ(kMaxCongestionWindow * kDefaultTCPMSS, + sender_->GetSlowStartThreshold()); + EXPECT_FALSE(sender_->hybrid_slow_start().started()); +} + } // namespace test } // namespace net diff --git a/net/quic/congestion_control/tcp_cubic_sender.cc b/net/quic/congestion_control/tcp_cubic_sender.cc index 0881981..4d6111f2 100644 --- a/net/quic/congestion_control/tcp_cubic_sender.cc +++ b/net/quic/congestion_control/tcp_cubic_sender.cc @@ -48,7 +48,9 @@ TcpCubicSender::TcpCubicSender(const QuicClock* clock, min4_mode_(false), slowstart_threshold_(max_tcp_congestion_window), last_cutback_exited_slowstart_(false), - max_tcp_congestion_window_(max_tcp_congestion_window) {} + max_tcp_congestion_window_(max_tcp_congestion_window), + initial_tcp_congestion_window_(initial_tcp_congestion_window), + initial_max_tcp_congestion_window_(max_tcp_congestion_window) {} TcpCubicSender::~TcpCubicSender() { UMA_HISTOGRAM_COUNTS("Net.QuicSession.FinalTcpCwnd", congestion_window_); @@ -360,6 +362,20 @@ void TcpCubicSender::OnRetransmissionTimeout(bool packets_retransmitted) { congestion_window_ = min_congestion_window_; } +void TcpCubicSender::OnConnectionMigration() { + hybrid_slow_start_.Restart(); + cubic_.Reset(); + prr_ = PrrSender(); + congestion_window_count_ = 0; + largest_sent_packet_number_ = 0; + largest_acked_packet_number_ = 0; + largest_sent_at_last_cutback_ = 0; + congestion_window_ = initial_tcp_congestion_window_; + slowstart_threshold_ = initial_max_tcp_congestion_window_; + last_cutback_exited_slowstart_ = false; + max_tcp_congestion_window_ = initial_max_tcp_congestion_window_; +} + CongestionControlType TcpCubicSender::GetCongestionControlType() const { return reno_ ? kReno : kCubic; } diff --git a/net/quic/congestion_control/tcp_cubic_sender.h b/net/quic/congestion_control/tcp_cubic_sender.h index 723ab07..9969d40 100644 --- a/net/quic/congestion_control/tcp_cubic_sender.h +++ b/net/quic/congestion_control/tcp_cubic_sender.h @@ -56,7 +56,7 @@ class NET_EXPORT_PRIVATE TcpCubicSender : public SendAlgorithmInterface { QuicByteCount bytes, HasRetransmittableData is_retransmittable) override; void OnRetransmissionTimeout(bool packets_retransmitted) override; - void OnConnectionMigration() override {} + void OnConnectionMigration() override; QuicTime::Delta TimeUntilSend( QuicTime now, QuicByteCount bytes_in_flight, @@ -131,6 +131,14 @@ class NET_EXPORT_PRIVATE TcpCubicSender : public SendAlgorithmInterface { // Maximum number of outstanding packets for tcp. QuicPacketCount max_tcp_congestion_window_; + // Initial TCP congestion window. This variable can only be set when this + // algorithm is created. + const QuicPacketCount initial_tcp_congestion_window_; + + // Initial maximum TCP congestion window. This variable can only be set when + // this algorithm is created. + const QuicPacketCount initial_max_tcp_congestion_window_; + DISALLOW_COPY_AND_ASSIGN(TcpCubicSender); }; diff --git a/net/quic/congestion_control/tcp_cubic_sender_test.cc b/net/quic/congestion_control/tcp_cubic_sender_test.cc index 95fe69c..db8aff5a 100644 --- a/net/quic/congestion_control/tcp_cubic_sender_test.cc +++ b/net/quic/congestion_control/tcp_cubic_sender_test.cc @@ -799,5 +799,39 @@ TEST_F(TcpCubicSenderTest, PaceBelowCWND) { HAS_RETRANSMITTABLE_DATA).IsZero()); } +TEST_F(TcpCubicSenderTest, ResetAfterConnectionMigration) { + EXPECT_EQ(kDefaultWindowTCP, sender_->GetCongestionWindow()); + EXPECT_EQ(kMaxCongestionWindow, sender_->slowstart_threshold()); + + // Starts with slow start. + sender_->SetNumEmulatedConnections(1); + const int kNumberOfAcks = 10; + for (int i = 0; i < kNumberOfAcks; ++i) { + // Send our full send window. + SendAvailableSendWindow(); + AckNPackets(2); + } + SendAvailableSendWindow(); + QuicByteCount expected_send_window = + kDefaultWindowTCP + (kDefaultTCPMSS * 2 * kNumberOfAcks); + EXPECT_EQ(expected_send_window, sender_->GetCongestionWindow()); + + // Loses a packet to exit slow start. + LoseNPackets(1); + + // We should now have fallen out of slow start with a reduced window. Slow + // start threshold is also updated. + expected_send_window *= kRenoBeta; + EXPECT_EQ(expected_send_window, sender_->GetCongestionWindow()); + EXPECT_EQ(expected_send_window / kDefaultTCPMSS, + sender_->slowstart_threshold()); + + // Resets cwnd and slow start threshold on connection migrations. + sender_->OnConnectionMigration(); + EXPECT_EQ(kDefaultWindowTCP, sender_->GetCongestionWindow()); + EXPECT_EQ(kMaxCongestionWindow, sender_->slowstart_threshold()); + EXPECT_FALSE(sender_->hybrid_slow_start().started()); +} + } // namespace test } // namespace net diff --git a/net/quic/crypto/crypto_handshake_message.cc b/net/quic/crypto/crypto_handshake_message.cc index d45b6aa..fcbd4e0 100644 --- a/net/quic/crypto/crypto_handshake_message.cc +++ b/net/quic/crypto/crypto_handshake_message.cc @@ -8,6 +8,7 @@ #include "base/strings/string_number_conversions.h" #include "net/quic/crypto/crypto_framer.h" #include "net/quic/crypto/crypto_protocol.h" +#include "net/quic/crypto/crypto_utils.h" #include "net/quic/quic_socket_address_coder.h" #include "net/quic/quic_utils.h" @@ -268,6 +269,21 @@ string CryptoHandshakeMessage::DebugStringInternal(size_t indent) const { done = true; } break; + case kRREJ: + // uint32 lists + if (it->second.size() % sizeof(uint32) == 0) { + for (size_t j = 0; j < it->second.size(); j += sizeof(uint32)) { + uint32 value; + memcpy(&value, it->second.data() + j, sizeof(value)); + if (j > 0) { + ret += ","; + } + ret += CryptoUtils::HandshakeFailureReasonToString( + static_cast<HandshakeFailureReason>(value)); + } + done = true; + } + break; case kCADR: // IP address and port if (!it->second.empty()) { diff --git a/net/quic/crypto/crypto_handshake_message_test.cc b/net/quic/crypto/crypto_handshake_message_test.cc new file mode 100644 index 0000000..0acace1 --- /dev/null +++ b/net/quic/crypto/crypto_handshake_message_test.cc @@ -0,0 +1,44 @@ +// Copyright (c) 2013 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/crypto/crypto_handshake_message.h" + +#include "net/quic/crypto/crypto_handshake.h" +#include "net/quic/crypto/crypto_protocol.h" +#include "net/test/gtest_util.h" + +namespace net { +namespace test { +namespace { + +TEST(CryptoHandshakeMessageTest, DebugString) { + CryptoHandshakeMessage message; + message.set_tag(kSHLO); + EXPECT_EQ("SHLO<\n>", message.DebugString()); +} + +TEST(CryptoHandshakeMessageTest, DebugStringWithUintVector) { + CryptoHandshakeMessage message; + message.set_tag(kREJ); + std::vector<uint32> reasons = { + SOURCE_ADDRESS_TOKEN_DIFFERENT_IP_ADDRESS_FAILURE, + CLIENT_NONCE_NOT_UNIQUE_FAILURE}; + message.SetVector(kRREJ, reasons); + EXPECT_EQ( + "REJ <\n RREJ: " + "SOURCE_ADDRESS_TOKEN_DIFFERENT_IP_ADDRESS_FAILURE," + "CLIENT_NONCE_NOT_UNIQUE_FAILURE\n>", + message.DebugString()); +} + +TEST(CryptoHandshakeMessageTest, DebugStringWithTagVector) { + CryptoHandshakeMessage message; + message.set_tag(kCHLO); + message.SetTaglist(kCOPT, kTBBR, kPAD, kBYTE, 0); + EXPECT_EQ("CHLO<\n COPT: 'TBBR','PAD ','BYTE'\n>", message.DebugString()); +} + +} // namespace +} // namespace test +} // namespace net diff --git a/net/quic/crypto/crypto_protocol.h b/net/quic/crypto/crypto_protocol.h index 8412080..b2e3ff9 100644 --- a/net/quic/crypto/crypto_protocol.h +++ b/net/quic/crypto/crypto_protocol.h @@ -84,6 +84,7 @@ const QuicTag kMIN4 = TAG('M', 'I', 'N', '4'); // Min CWND of 4 packets, // with a min rate of 1 BDP. const QuicTag kTLPR = TAG('T', 'L', 'P', 'R'); // Tail loss probe delay of // 0.5RTT. +const QuicTag kACKD = TAG('A', 'C', 'K', 'D'); // Ack decimation style acking. // Optional support of truncated Connection IDs. If sent by a peer, the value // is the minimum number of bytes allowed for the connection ID sent to the diff --git a/net/quic/crypto/crypto_utils.cc b/net/quic/crypto/crypto_utils.cc index 67c1c38..8d5d4b4 100644 --- a/net/quic/crypto/crypto_utils.cc +++ b/net/quic/crypto/crypto_utils.cc @@ -235,4 +235,47 @@ QuicErrorCode CryptoUtils::ValidateClientHello( return QUIC_NO_ERROR; } +#define RETURN_STRING_LITERAL(x) \ + case x: \ + return #x + +// Returns the name of the HandshakeFailureReason as a char* +// static +const char* CryptoUtils::HandshakeFailureReasonToString( + HandshakeFailureReason reason) { + switch (reason) { + RETURN_STRING_LITERAL(HANDSHAKE_OK); + RETURN_STRING_LITERAL(CLIENT_NONCE_UNKNOWN_FAILURE); + RETURN_STRING_LITERAL(CLIENT_NONCE_INVALID_FAILURE); + RETURN_STRING_LITERAL(CLIENT_NONCE_NOT_UNIQUE_FAILURE); + RETURN_STRING_LITERAL(CLIENT_NONCE_INVALID_ORBIT_FAILURE); + RETURN_STRING_LITERAL(CLIENT_NONCE_INVALID_TIME_FAILURE); + RETURN_STRING_LITERAL(CLIENT_NONCE_STRIKE_REGISTER_TIMEOUT); + RETURN_STRING_LITERAL(CLIENT_NONCE_STRIKE_REGISTER_FAILURE); + + RETURN_STRING_LITERAL(SERVER_NONCE_DECRYPTION_FAILURE); + RETURN_STRING_LITERAL(SERVER_NONCE_INVALID_FAILURE); + RETURN_STRING_LITERAL(SERVER_NONCE_NOT_UNIQUE_FAILURE); + RETURN_STRING_LITERAL(SERVER_NONCE_INVALID_TIME_FAILURE); + RETURN_STRING_LITERAL(SERVER_NONCE_REQUIRED_FAILURE); + + RETURN_STRING_LITERAL(SERVER_CONFIG_INCHOATE_HELLO_FAILURE); + RETURN_STRING_LITERAL(SERVER_CONFIG_UNKNOWN_CONFIG_FAILURE); + + RETURN_STRING_LITERAL(SOURCE_ADDRESS_TOKEN_INVALID_FAILURE); + RETURN_STRING_LITERAL(SOURCE_ADDRESS_TOKEN_DECRYPTION_FAILURE); + RETURN_STRING_LITERAL(SOURCE_ADDRESS_TOKEN_PARSE_FAILURE); + RETURN_STRING_LITERAL(SOURCE_ADDRESS_TOKEN_DIFFERENT_IP_ADDRESS_FAILURE); + RETURN_STRING_LITERAL(SOURCE_ADDRESS_TOKEN_CLOCK_SKEW_FAILURE); + RETURN_STRING_LITERAL(SOURCE_ADDRESS_TOKEN_EXPIRED_FAILURE); + + RETURN_STRING_LITERAL(INVALID_EXPECTED_LEAF_CERTIFICATE); + RETURN_STRING_LITERAL(MAX_FAILURE_REASON); + } + // Return a default value so that we return this when |reason| doesn't match + // any HandshakeFailureReason.. This can happen when the message by the peer + // (attacker) has invalid reason. + return "INVALID_HANDSHAKE_FAILURE_REASON"; +} + } // namespace net diff --git a/net/quic/crypto/crypto_utils.h b/net/quic/crypto/crypto_utils.h index 6d40074..f9f85bb 100644 --- a/net/quic/crypto/crypto_utils.h +++ b/net/quic/crypto/crypto_utils.h @@ -97,6 +97,10 @@ class NET_EXPORT_PRIVATE CryptoUtils { const QuicVersionVector& supported_versions, std::string* error_details); + // Returns the name of the HandshakeFailureReason as a char* + static const char* HandshakeFailureReasonToString( + HandshakeFailureReason reason); + private: DISALLOW_COPY_AND_ASSIGN(CryptoUtils); }; diff --git a/net/quic/crypto/crypto_utils_test.cc b/net/quic/crypto/crypto_utils_test.cc index 1395cc3..0bfd649 100644 --- a/net/quic/crypto/crypto_utils_test.cc +++ b/net/quic/crypto/crypto_utils_test.cc @@ -128,6 +128,80 @@ TEST(CryptoUtilsTest, TestExportKeyingMaterial) { } } +TEST(CryptoUtilsTest, HandshakeFailureReasonToString) { + EXPECT_STREQ("HANDSHAKE_OK", + CryptoUtils::HandshakeFailureReasonToString(HANDSHAKE_OK)); + EXPECT_STREQ("CLIENT_NONCE_UNKNOWN_FAILURE", + CryptoUtils::HandshakeFailureReasonToString( + CLIENT_NONCE_UNKNOWN_FAILURE)); + EXPECT_STREQ("CLIENT_NONCE_INVALID_FAILURE", + CryptoUtils::HandshakeFailureReasonToString( + CLIENT_NONCE_INVALID_FAILURE)); + EXPECT_STREQ("CLIENT_NONCE_NOT_UNIQUE_FAILURE", + CryptoUtils::HandshakeFailureReasonToString( + CLIENT_NONCE_NOT_UNIQUE_FAILURE)); + EXPECT_STREQ("CLIENT_NONCE_INVALID_ORBIT_FAILURE", + CryptoUtils::HandshakeFailureReasonToString( + CLIENT_NONCE_INVALID_ORBIT_FAILURE)); + EXPECT_STREQ("CLIENT_NONCE_INVALID_TIME_FAILURE", + CryptoUtils::HandshakeFailureReasonToString( + CLIENT_NONCE_INVALID_TIME_FAILURE)); + EXPECT_STREQ("CLIENT_NONCE_STRIKE_REGISTER_TIMEOUT", + CryptoUtils::HandshakeFailureReasonToString( + CLIENT_NONCE_STRIKE_REGISTER_TIMEOUT)); + EXPECT_STREQ("CLIENT_NONCE_STRIKE_REGISTER_FAILURE", + CryptoUtils::HandshakeFailureReasonToString( + CLIENT_NONCE_STRIKE_REGISTER_FAILURE)); + EXPECT_STREQ("SERVER_NONCE_DECRYPTION_FAILURE", + CryptoUtils::HandshakeFailureReasonToString( + SERVER_NONCE_DECRYPTION_FAILURE)); + EXPECT_STREQ("SERVER_NONCE_INVALID_FAILURE", + CryptoUtils::HandshakeFailureReasonToString( + SERVER_NONCE_INVALID_FAILURE)); + EXPECT_STREQ("SERVER_NONCE_NOT_UNIQUE_FAILURE", + CryptoUtils::HandshakeFailureReasonToString( + SERVER_NONCE_NOT_UNIQUE_FAILURE)); + EXPECT_STREQ("SERVER_NONCE_INVALID_TIME_FAILURE", + CryptoUtils::HandshakeFailureReasonToString( + SERVER_NONCE_INVALID_TIME_FAILURE)); + EXPECT_STREQ("SERVER_NONCE_REQUIRED_FAILURE", + CryptoUtils::HandshakeFailureReasonToString( + SERVER_NONCE_REQUIRED_FAILURE)); + EXPECT_STREQ("SERVER_CONFIG_INCHOATE_HELLO_FAILURE", + CryptoUtils::HandshakeFailureReasonToString( + SERVER_CONFIG_INCHOATE_HELLO_FAILURE)); + EXPECT_STREQ("SERVER_CONFIG_UNKNOWN_CONFIG_FAILURE", + CryptoUtils::HandshakeFailureReasonToString( + SERVER_CONFIG_UNKNOWN_CONFIG_FAILURE)); + EXPECT_STREQ("SOURCE_ADDRESS_TOKEN_INVALID_FAILURE", + CryptoUtils::HandshakeFailureReasonToString( + SOURCE_ADDRESS_TOKEN_INVALID_FAILURE)); + EXPECT_STREQ("SOURCE_ADDRESS_TOKEN_DECRYPTION_FAILURE", + CryptoUtils::HandshakeFailureReasonToString( + SOURCE_ADDRESS_TOKEN_DECRYPTION_FAILURE)); + EXPECT_STREQ("SOURCE_ADDRESS_TOKEN_PARSE_FAILURE", + CryptoUtils::HandshakeFailureReasonToString( + SOURCE_ADDRESS_TOKEN_PARSE_FAILURE)); + EXPECT_STREQ("SOURCE_ADDRESS_TOKEN_DIFFERENT_IP_ADDRESS_FAILURE", + CryptoUtils::HandshakeFailureReasonToString( + SOURCE_ADDRESS_TOKEN_DIFFERENT_IP_ADDRESS_FAILURE)); + EXPECT_STREQ("SOURCE_ADDRESS_TOKEN_CLOCK_SKEW_FAILURE", + CryptoUtils::HandshakeFailureReasonToString( + SOURCE_ADDRESS_TOKEN_CLOCK_SKEW_FAILURE)); + EXPECT_STREQ("SOURCE_ADDRESS_TOKEN_EXPIRED_FAILURE", + CryptoUtils::HandshakeFailureReasonToString( + SOURCE_ADDRESS_TOKEN_EXPIRED_FAILURE)); + EXPECT_STREQ("INVALID_EXPECTED_LEAF_CERTIFICATE", + CryptoUtils::HandshakeFailureReasonToString( + INVALID_EXPECTED_LEAF_CERTIFICATE)); + EXPECT_STREQ("MAX_FAILURE_REASON", + CryptoUtils::HandshakeFailureReasonToString(MAX_FAILURE_REASON)); + EXPECT_STREQ( + "INVALID_HANDSHAKE_FAILURE_REASON", + CryptoUtils::HandshakeFailureReasonToString( + static_cast<HandshakeFailureReason>(MAX_FAILURE_REASON + 1))); +} + } // namespace } // namespace test } // namespace net diff --git a/net/quic/crypto/null_decrypter.cc b/net/quic/crypto/null_decrypter.cc index cad2f7e..4fb41a5 100644 --- a/net/quic/crypto/null_decrypter.cc +++ b/net/quic/crypto/null_decrypter.cc @@ -74,8 +74,8 @@ bool NullDecrypter::ReadHash(QuicDataReader* reader, uint128* hash) { return true; } -uint128 NullDecrypter::ComputeHash(const StringPiece& data1, - const StringPiece& data2) const { +uint128 NullDecrypter::ComputeHash(const StringPiece data1, + const StringPiece data2) const { uint128 correct_hash = QuicUtils::FNV1a_128_Hash_Two( data1.data(), data1.length(), data2.data(), data2.length()); uint128 mask(UINT64_C(0x0), UINT64_C(0xffffffff)); diff --git a/net/quic/crypto/null_decrypter.h b/net/quic/crypto/null_decrypter.h index 414d2a1..ac501e6 100644 --- a/net/quic/crypto/null_decrypter.h +++ b/net/quic/crypto/null_decrypter.h @@ -38,8 +38,8 @@ class NET_EXPORT_PRIVATE NullDecrypter : public QuicDecrypter { private: bool ReadHash(QuicDataReader* reader, uint128* hash); - uint128 ComputeHash(const base::StringPiece& data1, - const base::StringPiece& data2) const; + uint128 ComputeHash(const base::StringPiece data1, + const base::StringPiece data2) const; DISALLOW_COPY_AND_ASSIGN(NullDecrypter); }; diff --git a/net/quic/quic_connection.cc b/net/quic/quic_connection.cc index 16e7427..d981506 100644 --- a/net/quic/quic_connection.cc +++ b/net/quic/quic_connection.cc @@ -62,6 +62,15 @@ const size_t kMaxFecGroups = 2; // Maximum number of acks received before sending an ack in response. const QuicPacketCount kMaxPacketsReceivedBeforeAckSend = 20; +// Maximum number of retransmittable packets received before sending an ack. +const QuicPacketCount kDefaultRetransmittablePacketsBeforeAck = 2; +// Minimum number of packets received before ack decimation is enabled. +// This intends to avoid the beginning of slow start, when CWNDs may be +// rapidly increasing. +const QuicPacketCount kMinReceivedBeforeAckDecimation = 100; +// Wait for up to 10 retransmittable packets before sending an ack. +const QuicPacketCount kMaxRetransmittablePacketsBeforeAck = 10; + bool Near(QuicPacketNumber a, QuicPacketNumber b) { QuicPacketNumber delta = (a > b) ? a - b : b - a; return delta <= kMaxPacketGap; @@ -282,8 +291,10 @@ QuicConnection::QuicConnection(QuicConnectionId connection_id, silent_close_enabled_(false), received_packet_manager_(&stats_), ack_queued_(false), + num_retransmittable_packets_received_since_last_ack_sent_(0), num_packets_received_since_last_ack_sent_(0), stop_waiting_count_(0), + ack_decimation_enabled_(false), delay_setting_retransmission_alarm_(false), pending_retransmission_alarm_(false), ack_alarm_(helper->CreateAlarm(new AckAlarm(this))), @@ -395,6 +406,13 @@ void QuicConnection::SetFromConfig(const QuicConfig& config) { if (config.HasClientSentConnectionOption(kMTUL, perspective_)) { SetMtuDiscoveryTarget(kMtuDiscoveryTargetPacketSizeLow); } + if (debug_visitor_ != nullptr) { + debug_visitor_->OnSetFromConfig(config); + } + if (FLAGS_quic_ack_decimation && + config.HasClientSentConnectionOption(kACKD, perspective_)) { + ack_decimation_enabled_ = true; + } } void QuicConnection::OnSendConnectionState( @@ -624,65 +642,13 @@ bool QuicConnection::OnPacketHeader(const QuicPacketHeader& header) { debug_visitor_->OnPacketHeader(header); } - if (!ProcessValidatedPacket()) { - return false; - } - // Will be decremented below if we fall through to return true. ++stats_.packets_dropped; - if (!Near(header.packet_number, last_header_.packet_number)) { - DVLOG(1) << ENDPOINT << "Packet " << header.packet_number - << " out of bounds. Discarding"; - SendConnectionCloseWithDetails(QUIC_INVALID_PACKET_HEADER, - "packet number out of bounds"); + if (!ProcessValidatedPacket(header)) { return false; } - // If this packet has already been seen, or the sender has told us that it - // will not be retransmitted, then stop processing the packet. - if (!received_packet_manager_.IsAwaitingPacket(header.packet_number)) { - DVLOG(1) << ENDPOINT << "Packet " << header.packet_number - << " no longer being waited for. Discarding."; - if (debug_visitor_ != nullptr) { - debug_visitor_->OnDuplicatePacket(header.packet_number); - } - return false; - } - - if (version_negotiation_state_ != NEGOTIATED_VERSION) { - if (perspective_ == Perspective::IS_SERVER) { - if (!header.public_header.version_flag) { - DLOG(WARNING) << ENDPOINT << "Packet " << header.packet_number - << " without version flag before version negotiated."; - // Packets should have the version flag till version negotiation is - // done. - CloseConnection(QUIC_INVALID_VERSION, false); - return false; - } else { - DCHECK_EQ(1u, header.public_header.versions.size()); - DCHECK_EQ(header.public_header.versions[0], version()); - version_negotiation_state_ = NEGOTIATED_VERSION; - visitor_->OnSuccessfulVersionNegotiation(version()); - if (debug_visitor_ != nullptr) { - debug_visitor_->OnSuccessfulVersionNegotiation(version()); - } - } - } else { - DCHECK(!header.public_header.version_flag); - // If the client gets a packet without the version flag from the server - // it should stop sending version since the version negotiation is done. - packet_generator_.StopSendingVersion(); - version_negotiation_state_ = NEGOTIATED_VERSION; - visitor_->OnSuccessfulVersionNegotiation(version()); - if (debug_visitor_ != nullptr) { - debug_visitor_->OnSuccessfulVersionNegotiation(version()); - } - } - } - - DCHECK_EQ(NEGOTIATED_VERSION, version_negotiation_state_); - --stats_.packets_dropped; DVLOG(1) << ENDPOINT << "Received packet header: " << header; last_header_ = header; @@ -969,11 +935,10 @@ void QuicConnection::OnPacketComplete() { << " packet " << last_header_.packet_number << " for " << last_header_.public_header.connection_id; - ++num_packets_received_since_last_ack_sent_; - - // Call MaybeQueueAck() before recording the received packet, since we want - // to trigger an ack if the newly received packet was previously missing. - MaybeQueueAck(); + // An ack will be sent if a missing retransmittable packet was received; + const bool was_missing = + should_last_packet_instigate_acks_ && + received_packet_manager_.IsMissing(last_header_.packet_number); // Record received or revived packet to populate ack info correctly before // processing stream frames, since the processing may result in a response @@ -985,8 +950,8 @@ void QuicConnection::OnPacketComplete() { last_size_, last_header_, time_of_last_received_packet_); } - // Continue to process stop waiting frames later, because the packet needs - // to be considered 'received' before the entropy can be updated. + // Process stop waiting frames here, instead of inline, because the packet + // needs to be considered 'received' before the entropy can be updated. if (last_stop_waiting_frame_.least_unacked > 0) { ProcessStopWaitingFrame(last_stop_waiting_frame_); if (!connected_) { @@ -994,33 +959,59 @@ void QuicConnection::OnPacketComplete() { } } - // If there are new missing packets to report, send an ack immediately. - if (ShouldLastPacketInstigateAck() && - received_packet_manager_.HasNewMissingPackets()) { - ack_queued_ = true; - ack_alarm_->Cancel(); - } + MaybeQueueAck(was_missing); ClearLastFrames(); MaybeCloseIfTooManyOutstandingPackets(); MaybeProcessRevivedPacket(); } -void QuicConnection::MaybeQueueAck() { - // If the last packet is an ack, don't ack it. - if (!ShouldLastPacketInstigateAck()) { - return; +void QuicConnection::MaybeQueueAck(bool was_missing) { + ++num_packets_received_since_last_ack_sent_; + // Always send an ack every 20 packets in order to allow the peer to discard + // information from the SentPacketManager and provide an RTT measurement. + if (num_packets_received_since_last_ack_sent_ >= + kMaxPacketsReceivedBeforeAckSend) { + ack_queued_ = true; } - // If the incoming packet was missing, send an ack immediately. - ack_queued_ = received_packet_manager_.IsMissing(last_header_.packet_number); - if (!ack_queued_) { - if (ack_alarm_->IsSet()) { - ack_queued_ = true; + // Determine whether the newly received packet was missing before recording + // the received packet. + if (was_missing) { + ack_queued_ = true; + } + + if (should_last_packet_instigate_acks_ && !ack_queued_) { + ++num_retransmittable_packets_received_since_last_ack_sent_; + if (ack_decimation_enabled_ && + last_header_.packet_number > kMinReceivedBeforeAckDecimation) { + // Ack up to 10 packets at once. + if (num_retransmittable_packets_received_since_last_ack_sent_ >= + kMaxRetransmittablePacketsBeforeAck) { + ack_queued_ = true; + } else if (!ack_alarm_->IsSet()) { + // Wait the minimum of a quarter min_rtt and the delayed ack time. + QuicTime::Delta ack_delay = QuicTime::Delta::Min( + sent_packet_manager_.DelayedAckTime(), + sent_packet_manager_.GetRttStats()->min_rtt().Multiply(0.25)); + ack_alarm_->Set(clock_->ApproximateNow().Add(ack_delay)); + } } else { - ack_alarm_->Set( - clock_->ApproximateNow().Add(sent_packet_manager_.DelayedAckTime())); - DVLOG(1) << "Ack timer set; next packet or timer will trigger ACK."; + // Ack with a timer or every 2 packets by default. + if (num_retransmittable_packets_received_since_last_ack_sent_ >= + kDefaultRetransmittablePacketsBeforeAck) { + ack_queued_ = true; + } else if (!ack_alarm_->IsSet()) { + ack_alarm_->Set(clock_->ApproximateNow().Add( + sent_packet_manager_.DelayedAckTime())); + } + } + + // If there are new missing packets to report, send an ack immediately. + // TODO(ianswett): Consider allowing 1ms of reordering for the + // ack decimation experiment. + if (received_packet_manager_.HasNewMissingPackets()) { + ack_queued_ = true; } } @@ -1064,19 +1055,6 @@ void QuicConnection::PopulateStopWaitingFrame( stop_waiting->least_unacked - 1); } -bool QuicConnection::ShouldLastPacketInstigateAck() const { - if (should_last_packet_instigate_acks_) { - return true; - } - // Always send an ack every 20 packets in order to allow the peer to discard - // information from the SentPacketManager and provide an RTT measurement. - if (num_packets_received_since_last_ack_sent_ >= - kMaxPacketsReceivedBeforeAckSend) { - return true; - } - return false; -} - QuicPacketNumber QuicConnection::GetLeastUnacked() const { return sent_packet_manager_.GetLeastUnacked(); } @@ -1342,15 +1320,16 @@ void QuicConnection::WriteIfNotBlocked() { } } -bool QuicConnection::ProcessValidatedPacket() { +bool QuicConnection::ProcessValidatedPacket(const QuicPacketHeader& header) { if (self_ip_changed_ || self_port_changed_) { SendConnectionCloseWithDetails(QUIC_ERROR_MIGRATING_ADDRESS, "Self address migration is not supported."); return false; } + PeerAddressChangeType type = NO_CHANGE; if (peer_ip_changed_ || peer_port_changed_) { - PeerAddressChangeType type = DeterminePeerAddressChangeType(); + type = DeterminePeerAddressChangeType(); if (type != NO_CHANGE && type != UNKNOWN && (FLAGS_quic_disable_non_nat_address_migration && type != NAT_PORT_REBINDING && type != IPV4_SUBNET_REBINDING)) { @@ -1358,7 +1337,61 @@ bool QuicConnection::ProcessValidatedPacket() { "Invalid peer address migration."); return false; } + } + + if (!Near(header.packet_number, last_header_.packet_number)) { + DVLOG(1) << ENDPOINT << "Packet " << header.packet_number + << " out of bounds. Discarding"; + SendConnectionCloseWithDetails(QUIC_INVALID_PACKET_HEADER, + "packet number out of bounds"); + return false; + } + + // If this packet has already been seen, or the sender has told us that it + // will not be retransmitted, then stop processing the packet. + if (!received_packet_manager_.IsAwaitingPacket(header.packet_number)) { + DVLOG(1) << ENDPOINT << "Packet " << header.packet_number + << " no longer being waited for. Discarding."; + if (debug_visitor_ != nullptr) { + debug_visitor_->OnDuplicatePacket(header.packet_number); + } + return false; + } + + if (version_negotiation_state_ != NEGOTIATED_VERSION) { + if (perspective_ == Perspective::IS_SERVER) { + if (!header.public_header.version_flag) { + DLOG(WARNING) << ENDPOINT << "Packet " << header.packet_number + << " without version flag before version negotiated."; + // Packets should have the version flag till version negotiation is + // done. + CloseConnection(QUIC_INVALID_VERSION, false); + return false; + } else { + DCHECK_EQ(1u, header.public_header.versions.size()); + DCHECK_EQ(header.public_header.versions[0], version()); + version_negotiation_state_ = NEGOTIATED_VERSION; + visitor_->OnSuccessfulVersionNegotiation(version()); + if (debug_visitor_ != nullptr) { + debug_visitor_->OnSuccessfulVersionNegotiation(version()); + } + } + } else { + DCHECK(!header.public_header.version_flag); + // If the client gets a packet without the version flag from the server + // it should stop sending version since the version negotiation is done. + packet_generator_.StopSendingVersion(); + version_negotiation_state_ = NEGOTIATED_VERSION; + visitor_->OnSuccessfulVersionNegotiation(version()); + if (debug_visitor_ != nullptr) { + debug_visitor_->OnSuccessfulVersionNegotiation(version()); + } + } + } + + DCHECK_EQ(NEGOTIATED_VERSION, version_negotiation_state_); + if (peer_ip_changed_ || peer_port_changed_) { IPEndPoint old_peer_address = peer_address_; peer_address_ = IPEndPoint( peer_ip_changed_ ? migrating_peer_ip_ : peer_address_.address(), @@ -1369,6 +1402,7 @@ bool QuicConnection::ProcessValidatedPacket() { << peer_address_.ToString() << ", migrating connection."; visitor_->OnConnectionMigration(); + DCHECK_NE(type, NO_CHANGE); sent_packet_manager_.OnConnectionMigration(type); } @@ -1784,6 +1818,7 @@ void QuicConnection::SendAck() { ack_alarm_->Cancel(); ack_queued_ = false; stop_waiting_count_ = 0; + num_retransmittable_packets_received_since_last_ack_sent_ = 0; num_packets_received_since_last_ack_sent_ = 0; packet_generator_.SetShouldSendAck(true); diff --git a/net/quic/quic_connection.h b/net/quic/quic_connection.h index 488950e..146324b 100644 --- a/net/quic/quic_connection.h +++ b/net/quic/quic_connection.h @@ -242,6 +242,10 @@ class NET_EXPORT_PRIVATE QuicConnectionDebugVisitor virtual void OnResumeConnectionState( const CachedNetworkParameters& cached_network_params) {} + // Called when the connection parameters are set from the supplied + // |config|. + virtual void OnSetFromConfig(const QuicConfig& config) {} + // Called when RTT may have changed, including when an RTT is read from // the config. virtual void OnRttChanged(QuicTime::Delta rtt) const {} @@ -647,8 +651,9 @@ class NET_EXPORT_PRIVATE QuicConnection // Do any work which logically would be done in OnPacket but can not be // safely done until the packet is validated. Returns true if the packet - // can be handled, false otherwise. - virtual bool ProcessValidatedPacket(); + // can be handled, false otherwise. Also migrates the connection if the packet + // can be handled and peer address changes. + virtual bool ProcessValidatedPacket(const QuicPacketHeader& header); // Send a packet to the peer, and takes ownership of the packet if the packet // cannot be written immediately. @@ -736,17 +741,14 @@ class NET_EXPORT_PRIVATE QuicConnection void ProcessStopWaitingFrame(const QuicStopWaitingFrame& stop_waiting); - // Queues an ack or sets the ack alarm when an incoming packet arrives that - // should be acked. - void MaybeQueueAck(); - - // Checks if the last packet should instigate an ack. - bool ShouldLastPacketInstigateAck() const; - // Sends any packets which are a response to the last packet, including both // acks and pending writes if an ack opened the congestion window. void MaybeSendInResponseToPacket(); + // Queue an ack or set the ack alarm if needed. |was_missing| is true if + // the most recently received packet was formerly missing. + void MaybeQueueAck(bool was_missing); + // Gets the least unacked packet number, which is the next packet number // to be sent if there are no outstanding packets. QuicPacketNumber GetLeastUnacked() const; @@ -862,11 +864,15 @@ class NET_EXPORT_PRIVATE QuicConnection // Indicates whether an ack should be sent the next time we try to write. bool ack_queued_; - // Indicates how many consecutive packets have arrived without sending an ack. + // How many retransmittable packets have arrived without sending an ack. + QuicPacketCount num_retransmittable_packets_received_since_last_ack_sent_; + // How many consecutive packets have arrived without sending an ack. QuicPacketCount num_packets_received_since_last_ack_sent_; // Indicates how many consecutive times an ack has arrived which indicates // the peer needs to stop waiting for some packets. int stop_waiting_count_; + // When true, ack only every 10 packets as long as they arrive close together. + bool ack_decimation_enabled_; // Indicates the retransmit alarm is going to be set by the // ScopedRetransmitAlarmDelayer diff --git a/net/quic/quic_connection_test.cc b/net/quic/quic_connection_test.cc index 4e18afd..b81c354 100644 --- a/net/quic/quic_connection_test.cc +++ b/net/quic/quic_connection_test.cc @@ -1516,11 +1516,14 @@ TEST_P(QuicConnectionTest, 20AcksCausesAckSend) { // But an ack with no missing packets will not send an ack. QuicAckFrame frame = InitAckFrame(1); EXPECT_CALL(*send_algorithm_, OnCongestionEvent(true, _, _, _)); - for (int i = 0; i < 20; ++i) { - EXPECT_FALSE(ack_alarm->IsSet()); + for (int i = 0; i < 19; ++i) { ProcessAckPacket(&frame); + EXPECT_FALSE(ack_alarm->IsSet()); } - EXPECT_TRUE(ack_alarm->IsSet()); + EXPECT_EQ(1u, writer_->packets_write_attempts()); + // The 20th ack packet will cause an ack to be sent. + ProcessAckPacket(&frame); + EXPECT_EQ(2u, writer_->packets_write_attempts()); } TEST_P(QuicConnectionTest, LeastUnackedLower) { @@ -1837,7 +1840,7 @@ TEST_P(QuicConnectionTest, FECSending) { connection_.SendStreamDataWithStringWithFec(1, payload, 0, !kFin, nullptr); // Expect the FEC group to be closed after SendStreamDataWithString. EXPECT_FALSE(creator_->IsFecGroupOpen()); - EXPECT_FALSE(creator_->IsFecProtected()); + EXPECT_FALSE(QuicPacketCreatorPeer::IsFecProtected(creator_)); } TEST_P(QuicConnectionTest, FECQueueing) { @@ -1847,14 +1850,14 @@ TEST_P(QuicConnectionTest, FECQueueing) { connection_.version(), kIncludeVersion, PACKET_8BYTE_CONNECTION_ID, PACKET_1BYTE_PACKET_NUMBER, IN_FEC_GROUP, &payload_length); connection_.SetMaxPacketLength(length); - EXPECT_TRUE(creator_->IsFecEnabled()); + EXPECT_TRUE(QuicPacketCreatorPeer::IsFecEnabled(creator_)); EXPECT_EQ(0u, connection_.NumQueuedPackets()); BlockOnNextWrite(); const string payload(payload_length, 'a'); connection_.SendStreamDataWithStringWithFec(1, payload, 0, !kFin, nullptr); EXPECT_FALSE(creator_->IsFecGroupOpen()); - EXPECT_FALSE(creator_->IsFecProtected()); + EXPECT_FALSE(QuicPacketCreatorPeer::IsFecProtected(creator_)); if (generator_->fec_send_policy() == FEC_ALARM_TRIGGER) { // Expect the first data packet to be queued and not the FEC packet. EXPECT_EQ(1u, connection_.NumQueuedPackets()); @@ -1865,7 +1868,7 @@ TEST_P(QuicConnectionTest, FECQueueing) { } TEST_P(QuicConnectionTest, FECAlarmStoppedWhenFECPacketSent) { - EXPECT_TRUE(creator_->IsFecEnabled()); + EXPECT_TRUE(QuicPacketCreatorPeer::IsFecEnabled(creator_)); EXPECT_EQ(0u, QuicSentPacketManagerPeer::GetBytesInFlight(manager_)); EXPECT_FALSE(connection_.GetFecAlarm()->IsSet()); @@ -1896,7 +1899,7 @@ TEST_P(QuicConnectionTest, FECAlarmStoppedWhenFECPacketSent) { } TEST_P(QuicConnectionTest, FECAlarmStoppedOnConnectionClose) { - EXPECT_TRUE(creator_->IsFecEnabled()); + EXPECT_TRUE(QuicPacketCreatorPeer::IsFecEnabled(creator_)); EXPECT_FALSE(connection_.GetFecAlarm()->IsSet()); creator_->set_max_packets_per_fec_group(100); @@ -1914,7 +1917,7 @@ TEST_P(QuicConnectionTest, FECAlarmStoppedOnConnectionClose) { TEST_P(QuicConnectionTest, RemoveFECFromInflightOnRetransmissionTimeout) { EXPECT_CALL(visitor_, OnSuccessfulVersionNegotiation(_)); - EXPECT_TRUE(creator_->IsFecEnabled()); + EXPECT_TRUE(QuicPacketCreatorPeer::IsFecEnabled(creator_)); EXPECT_EQ(0u, QuicSentPacketManagerPeer::GetBytesInFlight(manager_)); EXPECT_FALSE(connection_.GetFecAlarm()->IsSet()); @@ -1976,7 +1979,7 @@ TEST_P(QuicConnectionTest, RemoveFECFromInflightOnRetransmissionTimeout) { } TEST_P(QuicConnectionTest, RemoveFECFromInflightOnLossRetransmission) { - EXPECT_TRUE(creator_->IsFecEnabled()); + EXPECT_TRUE(QuicPacketCreatorPeer::IsFecEnabled(creator_)); EXPECT_FALSE(connection_.GetFecAlarm()->IsSet()); // 1 FEC-protected data packet. FEC alarm should be set. @@ -2064,7 +2067,7 @@ TEST_P(QuicConnectionTest, FECRemainsInflightOnTLPOfEarlierData) { // retransmission alarm fires. // Turn on TLP for this test. QuicSentPacketManagerPeer::SetMaxTailLossProbes(manager_, 1); - EXPECT_TRUE(creator_->IsFecEnabled()); + EXPECT_TRUE(QuicPacketCreatorPeer::IsFecEnabled(creator_)); EXPECT_EQ(0u, QuicSentPacketManagerPeer::GetBytesInFlight(manager_)); EXPECT_FALSE(connection_.GetFecAlarm()->IsSet()); @@ -2110,7 +2113,7 @@ TEST_P(QuicConnectionTest, FECRemainsInflightOnTLPOfLaterData) { // sent for data packet 2 when the retransmission alarm fires. Turn on TLP for // this test. QuicSentPacketManagerPeer::SetMaxTailLossProbes(manager_, 1); - EXPECT_TRUE(creator_->IsFecEnabled()); + EXPECT_TRUE(QuicPacketCreatorPeer::IsFecEnabled(creator_)); EXPECT_EQ(0u, QuicSentPacketManagerPeer::GetBytesInFlight(manager_)); EXPECT_FALSE(connection_.GetFecAlarm()->IsSet()); @@ -2177,7 +2180,7 @@ TEST_P(QuicConnectionTest, FECRemainsInflightOnTLPOfLaterData) { TEST_P(QuicConnectionTest, NoTLPForFECPacket) { // Turn on TLP for this test. QuicSentPacketManagerPeer::SetMaxTailLossProbes(manager_, 1); - EXPECT_TRUE(creator_->IsFecEnabled()); + EXPECT_TRUE(QuicPacketCreatorPeer::IsFecEnabled(creator_)); EXPECT_CALL(visitor_, OnSuccessfulVersionNegotiation(_)); // Send 1 FEC-protected data packet. FEC alarm should be set. @@ -2287,7 +2290,7 @@ TEST_P(QuicConnectionTest, FramePackingCryptoThenNonCrypto) { } TEST_P(QuicConnectionTest, FramePackingFEC) { - EXPECT_TRUE(creator_->IsFecEnabled()); + EXPECT_TRUE(QuicPacketCreatorPeer::IsFecEnabled(creator_)); CongestionBlockWrites(); @@ -4334,6 +4337,56 @@ TEST_P(QuicConnectionTest, SendDelayedAck) { EXPECT_FALSE(connection_.GetAckAlarm()->IsSet()); } +TEST_P(QuicConnectionTest, SendDelayedAckDecimation) { + QuicConnectionPeer::EnableAckDecimation(&connection_); + + const size_t kMinRttMs = 40; + RttStats* rtt_stats = QuicSentPacketManagerPeer::GetRttStats(manager_); + rtt_stats->UpdateRtt(QuicTime::Delta::FromMilliseconds(kMinRttMs), + QuicTime::Delta::Zero(), QuicTime::Zero()); + // The ack time should be based on min_rtt/4, since it's less than the + // default delayed ack time. + QuicTime ack_time = clock_.ApproximateNow().Add( + QuicTime::Delta::FromMilliseconds(kMinRttMs / 4)); + EXPECT_CALL(visitor_, OnSuccessfulVersionNegotiation(_)); + EXPECT_FALSE(connection_.GetAckAlarm()->IsSet()); + const uint8 tag = 0x07; + connection_.SetDecrypter(ENCRYPTION_INITIAL, new StrictTaggingDecrypter(tag)); + framer_.SetEncrypter(ENCRYPTION_INITIAL, new TaggingEncrypter(tag)); + // Process a packet from the non-crypto stream. + frame1_.stream_id = 3; + + // Process all the initial packets in order so there aren't missing packets. + QuicPacketNumber kFirstDecimatedPacket = 101; + for (unsigned int i = 0; i < kFirstDecimatedPacket - 1; ++i) { + EXPECT_CALL(visitor_, OnStreamFrame(_)).Times(1); + ProcessDataPacketAtLevel(1 + i, 0, !kEntropyFlag, ENCRYPTION_INITIAL); + } + EXPECT_FALSE(connection_.GetAckAlarm()->IsSet()); + // The same as ProcessPacket(1) except that ENCRYPTION_INITIAL is used + // instead of ENCRYPTION_NONE. + EXPECT_CALL(visitor_, OnStreamFrame(_)).Times(1); + ProcessDataPacketAtLevel(kFirstDecimatedPacket, 0, !kEntropyFlag, + ENCRYPTION_INITIAL); + + // Check if delayed ack timer is running for the expected interval. + EXPECT_TRUE(connection_.GetAckAlarm()->IsSet()); + EXPECT_EQ(ack_time, connection_.GetAckAlarm()->deadline()); + + // The 10th received packet causes an ack to be sent. + for (int i = 0; i < 9; ++i) { + EXPECT_TRUE(connection_.GetAckAlarm()->IsSet()); + EXPECT_CALL(visitor_, OnStreamFrame(_)).Times(1); + ProcessDataPacketAtLevel(kFirstDecimatedPacket + 1 + i, 0, !kEntropyFlag, + ENCRYPTION_INITIAL); + } + // Check that ack is sent and that delayed ack alarm is reset. + EXPECT_EQ(2u, writer_->frame_count()); + EXPECT_FALSE(writer_->stop_waiting_frames().empty()); + EXPECT_FALSE(writer_->ack_frames().empty()); + EXPECT_FALSE(connection_.GetAckAlarm()->IsSet()); +} + TEST_P(QuicConnectionTest, SendDelayedAckOnHandshakeConfirmed) { EXPECT_CALL(visitor_, OnSuccessfulVersionNegotiation(_)); ProcessPacket(1); @@ -5285,7 +5338,7 @@ TEST_P(QuicConnectionTest, NetworkChangeVisitorConfigCallbackChangesFecState) { QuicSentPacketManagerPeer::GetNetworkChangeVisitor(manager_); EXPECT_TRUE(visitor); EXPECT_EQ(QuicTime::Delta::Zero(), - QuicPacketGeneratorPeer::GetFecTimeout(generator_)); + QuicPacketCreatorPeer::GetFecTimeout(creator_)); // Verify that sending a config with a new initial rtt changes fec timeout. // Create and process a config with a non-zero initial RTT. @@ -5294,7 +5347,7 @@ TEST_P(QuicConnectionTest, NetworkChangeVisitorConfigCallbackChangesFecState) { config.SetInitialRoundTripTimeUsToSend(300000); connection_.SetFromConfig(config); EXPECT_LT(QuicTime::Delta::Zero(), - QuicPacketGeneratorPeer::GetFecTimeout(generator_)); + QuicPacketCreatorPeer::GetFecTimeout(creator_)); } TEST_P(QuicConnectionTest, NetworkChangeVisitorRttCallbackChangesFecState) { @@ -5303,7 +5356,7 @@ TEST_P(QuicConnectionTest, NetworkChangeVisitorRttCallbackChangesFecState) { QuicSentPacketManagerPeer::GetNetworkChangeVisitor(manager_); EXPECT_TRUE(visitor); EXPECT_EQ(QuicTime::Delta::Zero(), - QuicPacketGeneratorPeer::GetFecTimeout(generator_)); + QuicPacketCreatorPeer::GetFecTimeout(creator_)); // Increase FEC timeout by increasing RTT. RttStats* rtt_stats = QuicSentPacketManagerPeer::GetRttStats(manager_); @@ -5311,7 +5364,7 @@ TEST_P(QuicConnectionTest, NetworkChangeVisitorRttCallbackChangesFecState) { QuicTime::Delta::Zero(), QuicTime::Zero()); visitor->OnRttChange(); EXPECT_LT(QuicTime::Delta::Zero(), - QuicPacketGeneratorPeer::GetFecTimeout(generator_)); + QuicPacketCreatorPeer::GetFecTimeout(creator_)); } TEST_P(QuicConnectionTest, OnPacketHeaderDebugVisitor) { @@ -5333,7 +5386,7 @@ TEST_P(QuicConnectionTest, Pacing) { EXPECT_FALSE(server.sent_packet_manager().using_pacing()); } -TEST_P(QuicConnectionTest, ControlFramesInstigateAcks) { +TEST_P(QuicConnectionTest, WindowUpdateInstigateAcks) { EXPECT_CALL(visitor_, OnSuccessfulVersionNegotiation(_)); // Send a WINDOW_UPDATE frame. @@ -5346,13 +5399,19 @@ TEST_P(QuicConnectionTest, ControlFramesInstigateAcks) { // Ensure that this has caused the ACK alarm to be set. QuicAlarm* ack_alarm = QuicConnectionPeer::GetAckAlarm(&connection_); EXPECT_TRUE(ack_alarm->IsSet()); +} - // Cancel alarm, and try again with BLOCKED frame. - ack_alarm->Cancel(); +TEST_P(QuicConnectionTest, BlockedFrameInstigateAcks) { + EXPECT_CALL(visitor_, OnSuccessfulVersionNegotiation(_)); + + // Send a BLOCKED frame. QuicBlockedFrame blocked; blocked.stream_id = 3; EXPECT_CALL(visitor_, OnBlockedFrame(_)); ProcessFramePacket(QuicFrame(&blocked)); + + // Ensure that this has caused the ACK alarm to be set. + QuicAlarm* ack_alarm = QuicConnectionPeer::GetAckAlarm(&connection_); EXPECT_TRUE(ack_alarm->IsSet()); } @@ -5396,11 +5455,11 @@ TEST_P(QuicConnectionTest, FecRTTMultiplierReceivedConnectionOption) { copt.push_back(kFRTT); QuicConfigPeer::SetReceivedConnectionOptions(&config, copt); float rtt_multiplier_for_fec_timeout = - generator_->rtt_multiplier_for_fec_timeout(); + QuicPacketCreatorPeer::GetRttMultiplierForFecTimeout(creator_); connection_.SetFromConfig(config); // New RTT multiplier is half of the old RTT multiplier. EXPECT_EQ(rtt_multiplier_for_fec_timeout, - generator_->rtt_multiplier_for_fec_timeout() * 2); + QuicPacketCreatorPeer::GetRttMultiplierForFecTimeout(creator_) * 2); } TEST_P(QuicConnectionTest, DoNotSendGoAwayTwice) { diff --git a/net/quic/quic_crypto_client_stream_test.cc b/net/quic/quic_crypto_client_stream_test.cc index d4b41d5..0126116 100644 --- a/net/quic/quic_crypto_client_stream_test.cc +++ b/net/quic/quic_crypto_client_stream_test.cc @@ -263,11 +263,11 @@ class QuicCryptoClientStreamStatelessTest : public ::testing::Test { CryptoTestUtils::ProofSourceForTesting()), server_id_(kServerHostname, kServerPort, PRIVACY_MODE_DISABLED) { TestQuicSpdyClientSession* client_session = nullptr; - CreateClientSessionForTest(server_id_, - /* supports_stateless_rejects= */ true, - QuicTime::Delta::FromSeconds(100000), &helper_, - &client_crypto_config_, &client_connection_, - &client_session); + CreateClientSessionForTest( + server_id_, + /* supports_stateless_rejects= */ true, + QuicTime::Delta::FromSeconds(100000), QuicSupportedVersions(), &helper_, + &client_crypto_config_, &client_connection_, &client_session); CHECK(client_session); client_session_.reset(client_session); } @@ -287,8 +287,9 @@ class QuicCryptoClientStreamStatelessTest : public ::testing::Test { void InitializeFakeStatelessRejectServer() { TestQuicSpdyServerSession* server_session = nullptr; CreateServerSessionForTest(server_id_, QuicTime::Delta::FromSeconds(100000), - &helper_, &server_crypto_config_, - &server_connection_, &server_session); + QuicSupportedVersions(), &helper_, + &server_crypto_config_, &server_connection_, + &server_session); CHECK(server_session); server_session_.reset(server_session); CryptoTestUtils::FakeServerOptions options; diff --git a/net/quic/quic_crypto_server_stream.cc b/net/quic/quic_crypto_server_stream.cc index 8e2e15a..e041724 100644 --- a/net/quic/quic_crypto_server_stream.cc +++ b/net/quic/quic_crypto_server_stream.cc @@ -52,6 +52,9 @@ void QuicCryptoServerStream::CancelOutstandingCallbacks() { // Detach from the validation callback. Calling this multiple times is safe. if (validate_client_hello_cb_ != nullptr) { validate_client_hello_cb_->Cancel(); + if (FLAGS_quic_set_client_hello_cb_nullptr) { + validate_client_hello_cb_ = nullptr; + } } } diff --git a/net/quic/quic_crypto_server_stream_test.cc b/net/quic/quic_crypto_server_stream_test.cc index a5bf553..ef53a58 100644 --- a/net/quic/quic_crypto_server_stream_test.cc +++ b/net/quic/quic_crypto_server_stream_test.cc @@ -74,7 +74,9 @@ class QuicCryptoServerStreamTest : public ::testing::TestWithParam<bool> { client_crypto_config_(CryptoTestUtils::ProofVerifierForTesting()) { FLAGS_enable_quic_stateless_reject_support = false; server_crypto_config_.set_strike_register_no_startup_period(); + } + void Initialize() { InitializeServer(); if (AsyncStrikeRegisterVerification()) { @@ -101,8 +103,9 @@ class QuicCryptoServerStreamTest : public ::testing::TestWithParam<bool> { helpers_.push_back(new MockConnectionHelper); CreateServerSessionForTest(server_id_, QuicTime::Delta::FromSeconds(100000), - helpers_.back(), &server_crypto_config_, - &server_connection_, &server_session); + supported_versions_, helpers_.back(), + &server_crypto_config_, &server_connection_, + &server_session); CHECK(server_session); server_session_.reset(server_session); CryptoTestUtils::FakeServerOptions options; @@ -127,6 +130,8 @@ class QuicCryptoServerStreamTest : public ::testing::TestWithParam<bool> { helpers_.push_back(new MockConnectionHelper); CreateClientSessionForTest(server_id_, supports_stateless_rejects, QuicTime::Delta::FromSeconds(100000), + supported_versions_, + helpers_.back(), &client_crypto_config_, &client_connection_, &client_session); CHECK(client_session); @@ -186,16 +191,21 @@ class QuicCryptoServerStreamTest : public ::testing::TestWithParam<bool> { scoped_ptr<QuicData> message_data_; CryptoTestUtils::FakeClientOptions client_options_; DelayedVerifyStrikeRegisterClient* strike_register_client_; + + // Which QUIC versions the client and server support. + QuicVersionVector supported_versions_ = QuicSupportedVersions(); }; INSTANTIATE_TEST_CASE_P(Tests, QuicCryptoServerStreamTest, testing::Bool()); TEST_P(QuicCryptoServerStreamTest, NotInitiallyConected) { + Initialize(); EXPECT_FALSE(server_stream()->encryption_established()); EXPECT_FALSE(server_stream()->handshake_confirmed()); } TEST_P(QuicCryptoServerStreamTest, NotInitiallySendingStatelessRejects) { + Initialize(); EXPECT_FALSE(server_stream()->UseStatelessRejectsIfPeerSupported()); EXPECT_FALSE(server_stream()->PeerSupportsStatelessRejects()); } @@ -205,6 +215,7 @@ TEST_P(QuicCryptoServerStreamTest, ConnectedAfterCHLO) { // test should send: // * One to get a source-address token and certificates. // * One to complete the handshake. + Initialize(); EXPECT_EQ(2, CompleteCryptoHandshake()); EXPECT_TRUE(server_stream()->encryption_established()); EXPECT_TRUE(server_stream()->handshake_confirmed()); @@ -213,7 +224,7 @@ TEST_P(QuicCryptoServerStreamTest, ConnectedAfterCHLO) { TEST_P(QuicCryptoServerStreamTest, StatelessRejectAfterCHLO) { ValueRestore<bool> old_flag(&FLAGS_enable_quic_stateless_reject_support, true); - InitializeServer(); + Initialize(); InitializeFakeClient(/* supports_stateless_rejects= */ true); AdvanceHandshakeWithFakeClient(); @@ -245,7 +256,7 @@ TEST_P(QuicCryptoServerStreamTest, StatelessRejectAfterCHLO) { TEST_P(QuicCryptoServerStreamTest, ConnectedAfterStatelessHandshake) { ValueRestore<bool> old_flag(&FLAGS_enable_quic_stateless_reject_support, true); - InitializeServer(); + Initialize(); InitializeFakeClient(/* supports_stateless_rejects= */ true); AdvanceHandshakeWithFakeClient(); @@ -292,7 +303,7 @@ TEST_P(QuicCryptoServerStreamTest, ConnectedAfterStatelessHandshake) { TEST_P(QuicCryptoServerStreamTest, NoStatelessRejectIfNoClientSupport) { ValueRestore<bool> old_flag(&FLAGS_enable_quic_stateless_reject_support, true); - InitializeServer(); + Initialize(); // The server is configured to use stateless rejects, but the client does not // support it. @@ -313,6 +324,7 @@ TEST_P(QuicCryptoServerStreamTest, NoStatelessRejectIfNoClientSupport) { } TEST_P(QuicCryptoServerStreamTest, ZeroRTT) { + Initialize(); InitializeFakeClient(/* supports_stateless_rejects= */ false); // Do a first handshake in order to prime the client config with the server's @@ -363,6 +375,7 @@ TEST_P(QuicCryptoServerStreamTest, ZeroRTT) { } TEST_P(QuicCryptoServerStreamTest, MessageAfterHandshake) { + Initialize(); CompleteCryptoHandshake(); EXPECT_CALL( *server_connection_, @@ -375,6 +388,8 @@ TEST_P(QuicCryptoServerStreamTest, MessageAfterHandshake) { } TEST_P(QuicCryptoServerStreamTest, BadMessageType) { + Initialize(); + message_.set_tag(kSHLO); ConstructHandshakeMessage(); EXPECT_CALL(*server_connection_, @@ -385,6 +400,8 @@ TEST_P(QuicCryptoServerStreamTest, BadMessageType) { } TEST_P(QuicCryptoServerStreamTest, ChannelID) { + Initialize(); + client_options_.channel_id_enabled = true; client_options_.channel_id_source_async = false; // CompleteCryptoHandshake verifies @@ -395,6 +412,8 @@ TEST_P(QuicCryptoServerStreamTest, ChannelID) { } TEST_P(QuicCryptoServerStreamTest, ChannelIDAsync) { + Initialize(); + client_options_.channel_id_enabled = true; client_options_.channel_id_source_async = true; // CompleteCryptoHandshake verifies @@ -406,11 +425,15 @@ TEST_P(QuicCryptoServerStreamTest, ChannelIDAsync) { TEST_P(QuicCryptoServerStreamTest, OnlySendSCUPAfterHandshakeComplete) { // An attempt to send a SCUP before completing handshake should fail. + Initialize(); + server_stream()->SendServerConfigUpdate(nullptr); EXPECT_EQ(0, server_stream()->NumServerConfigUpdateMessagesSent()); } TEST_P(QuicCryptoServerStreamTest, DoesPeerSupportStatelessRejects) { + Initialize(); + ConstructHandshakeMessage(); QuicConfig stateless_reject_config = DefaultQuicConfigStatelessRejects(); stateless_reject_config.ToHandshakeMessage(&message_); @@ -425,6 +448,8 @@ TEST_P(QuicCryptoServerStreamTest, DoesPeerSupportStatelessRejects) { } TEST_P(QuicCryptoServerStreamTest, TokenBindingNegotiated) { + Initialize(); + client_options_.token_binding_enabled = true; CompleteCryptoHandshake(); EXPECT_EQ( @@ -435,6 +460,8 @@ TEST_P(QuicCryptoServerStreamTest, TokenBindingNegotiated) { } TEST_P(QuicCryptoServerStreamTest, NoTokenBindingWithoutClientSupport) { + Initialize(); + CompleteCryptoHandshake(); EXPECT_EQ( 0u, server_stream()->crypto_negotiated_params().token_binding_key_param); @@ -442,6 +469,49 @@ TEST_P(QuicCryptoServerStreamTest, NoTokenBindingWithoutClientSupport) { EXPECT_TRUE(server_stream()->handshake_confirmed()); } +TEST_P(QuicCryptoServerStreamTest, CancelRPCBeforeVerificationCompletes) { + // Tests that the client can close the connection while the remote strike + // register verification RPC is still pending. + FLAGS_quic_set_client_hello_cb_nullptr = true; + + // Set version to QUIC_VERSION_25 as QUIC_VERSION_26 and later don't support + // asynchronous strike register RPCs. + supported_versions_ = {QUIC_VERSION_25}; + Initialize(); + if (!AsyncStrikeRegisterVerification()) { + return; + } + InitializeFakeClient(/* supports_stateless_rejects= */ false); + + // Do a first handshake in order to prime the client config with the server's + // information. + AdvanceHandshakeWithFakeClient(); + + // Now start another handshake, this time the server will attempt to verify + // the client's nonce with the strike registers. + InitializeFakeClient(/* supports_stateless_rejects= */ false); + InitializeServer(); + client_stream()->CryptoConnect(); + EXPECT_FALSE(client_stream()->handshake_confirmed()); + EXPECT_FALSE(server_stream()->handshake_confirmed()); + + // Advance the handshake. Expect that the server will be stuck waiting for + // client nonce verification to complete. + CryptoTestUtils::AdvanceHandshake(client_connection_, client_stream(), 0, + server_connection_, server_stream(), 0); + EXPECT_EQ(1, strike_register_client_->PendingVerifications()); + EXPECT_FALSE(client_stream()->handshake_confirmed()); + EXPECT_FALSE(server_stream()->handshake_confirmed()); + + // While waiting for the asynchronous verification to complete, the client + // decides to close the connection. + server_session_->connection()->CloseConnection(QUIC_NO_ERROR, + /*from_peer=*/true); + + // The outstanding nonce verification RPC now completes. + strike_register_client_->RunPendingVerifications(); +} + } // namespace } // namespace test diff --git a/net/quic/quic_crypto_stream.cc b/net/quic/quic_crypto_stream.cc index fdee373..84bdf8c 100644 --- a/net/quic/quic_crypto_stream.cc +++ b/net/quic/quic_crypto_stream.cc @@ -60,7 +60,7 @@ void QuicCryptoStream::OnDataAvailable() { } SpdyPriority QuicCryptoStream::Priority() const { - return net::kHighestPriority; // The smallest priority is also the highest + return net::kV3HighestPriority; } void QuicCryptoStream::SendHandshakeMessage( diff --git a/net/quic/quic_flags.cc b/net/quic/quic_flags.cc index 672a20b2..b63a0e5 100644 --- a/net/quic/quic_flags.cc +++ b/net/quic/quic_flags.cc @@ -43,9 +43,16 @@ int64 FLAGS_quic_time_wait_list_max_connections = 600000; // Enables server-side support for QUIC stateless rejects. bool FLAGS_enable_quic_stateless_reject_support = true; +// If ture, allow Ack Decimation to be used for QUIC when requested by the +// client connection option ACKD. +bool FLAGS_quic_ack_decimation = true; + // If true, flow controller may grow the receive window size if necessary. bool FLAGS_quic_auto_tune_receive_window = true; +// If true, record received connection options to TransportConnectionStats. +bool FLAGS_quic_connection_options_to_transport_stats = true; + // Limits QUIC's max CWND to 200 packets. bool FLAGS_quic_limit_max_cwnd = true; @@ -65,10 +72,6 @@ bool FLAGS_quic_packet_queue_use_interval_set = true; // If true, Cubic's epoch is shifted when the sender is application-limited. bool FLAGS_shift_quic_cubic_epoch_when_app_limited = true; -// If true, accounts for available (implicitly opened) streams under a separate -// quota from open streams, which is 10 times larger. -bool FLAGS_allow_many_available_streams = true; - // If true, QUIC will measure head of line (HOL) blocking due between // streams due to packet losses on the headers stream. The // measurements will be surfaced via UMA histogram @@ -78,19 +81,10 @@ bool FLAGS_quic_measure_headers_hol_blocking_time = true; // Disable QUIC's userspace pacing. bool FLAGS_quic_disable_pacing = false; -// If true, a FIN received on a stream with read_side_closed_ true will be -// recorded correctly. -bool FLAGS_quic_fix_fin_accounting = true; - // If true, Use QUIC's GeneralLossAlgorithm implementation instead of // TcpLossAlgorithm or TimeLossAlgorithm. bool FLAGS_quic_general_loss_algorithm = true; -// If true, ReliableQuicStream::StopReading (formerly CloseReadSide) causes -// incoming data to be ignored but the read side of the stream object is not -// closed. -bool FLAGS_quic_implement_stop_reading = true; - // Invoke the QuicAckListener directly, instead of going through the AckNotifier // and AckNotifierManager. bool FLAGS_quic_no_ack_notifier = true; @@ -113,6 +107,10 @@ bool FLAGS_quic_use_stream_sequencer_buffer = true; // If true, don't send QUIC packets if the send alarm is set. bool FLAGS_quic_respect_send_alarm = true; +// If ture, sets callback pointer to nullptr after calling Cancel() in +// QuicCryptoServerStream::CancelOutstandingCallbacks. +bool FLAGS_quic_set_client_hello_cb_nullptr = true; + // If treu, Only track a single retransmission in QUIC's TransmissionInfo // struct. bool FLAGS_quic_track_single_retransmission = true; diff --git a/net/quic/quic_flags.h b/net/quic/quic_flags.h index aeb1f75..8f4ee24 100644 --- a/net/quic/quic_flags.h +++ b/net/quic/quic_flags.h @@ -18,24 +18,24 @@ NET_EXPORT_PRIVATE extern bool FLAGS_quic_too_many_outstanding_packets; NET_EXPORT_PRIVATE extern int64 FLAGS_quic_time_wait_list_seconds; NET_EXPORT_PRIVATE extern int64 FLAGS_quic_time_wait_list_max_connections; NET_EXPORT_PRIVATE extern bool FLAGS_enable_quic_stateless_reject_support; +NET_EXPORT_PRIVATE extern bool FLAGS_quic_ack_decimation; NET_EXPORT_PRIVATE extern bool FLAGS_quic_auto_tune_receive_window; +NET_EXPORT_PRIVATE extern bool FLAGS_quic_connection_options_to_transport_stats; NET_EXPORT_PRIVATE extern bool FLAGS_quic_limit_max_cwnd; NET_EXPORT_PRIVATE extern bool FLAGS_quic_require_handshake_confirmation; NET_EXPORT_PRIVATE extern bool FLAGS_send_goaway_after_client_migration; NET_EXPORT_PRIVATE extern bool FLAGS_quic_packet_queue_use_interval_set; NET_EXPORT_PRIVATE extern bool FLAGS_shift_quic_cubic_epoch_when_app_limited; -NET_EXPORT_PRIVATE extern bool FLAGS_allow_many_available_streams; NET_EXPORT_PRIVATE extern bool FLAGS_quic_measure_headers_hol_blocking_time; NET_EXPORT_PRIVATE extern bool FLAGS_quic_disable_pacing; -NET_EXPORT_PRIVATE extern bool FLAGS_quic_fix_fin_accounting; NET_EXPORT_PRIVATE extern bool FLAGS_quic_general_loss_algorithm; -NET_EXPORT_PRIVATE extern bool FLAGS_quic_implement_stop_reading; NET_EXPORT_PRIVATE extern bool FLAGS_quic_no_ack_notifier; NET_EXPORT_PRIVATE extern bool FLAGS_quic_packet_creator_prefetch; NET_EXPORT_PRIVATE extern bool FLAGS_quic_disable_non_nat_address_migration; NET_EXPORT_PRIVATE extern bool FLAGS_quic_use_new_idle_timeout; NET_EXPORT_PRIVATE extern bool FLAGS_quic_use_stream_sequencer_buffer; NET_EXPORT_PRIVATE extern bool FLAGS_quic_respect_send_alarm; +NET_EXPORT_PRIVATE extern bool FLAGS_quic_set_client_hello_cb_nullptr; NET_EXPORT_PRIVATE extern bool FLAGS_quic_track_single_retransmission; NET_EXPORT_PRIVATE extern bool FLAGS_quic_batch_writes; #endif // NET_QUIC_QUIC_FLAGS_H_ diff --git a/net/quic/quic_framer.cc b/net/quic/quic_framer.cc index eceb6cc..d7849fb 100644 --- a/net/quic/quic_framer.cc +++ b/net/quic/quic_framer.cc @@ -703,6 +703,9 @@ bool QuicFramer::AppendPacketHeader(const QuicPacketHeader& header, if (header.public_header.version_flag) { public_flags |= PACKET_PUBLIC_FLAGS_VERSION; } + if (header.public_header.multipath_flag) { + public_flags |= PACKET_PUBLIC_FLAGS_MULTIPATH; + } public_flags |= GetSequenceNumberFlags(header.public_header.packet_number_length) @@ -755,6 +758,11 @@ bool QuicFramer::AppendPacketHeader(const QuicPacketHeader& header, << QuicUtils::TagToString(tag) << "'"; } + if (header.public_header.multipath_flag && + !writer->WriteUInt8(header.path_id)) { + return false; + } + if (!AppendPacketSequenceNumber(header.public_header.packet_number_length, header.packet_number, writer)) { return false; @@ -845,6 +853,8 @@ bool QuicFramer::ProcessPublicHeader(QuicDataReader* reader, return false; } + public_header->multipath_flag = + (public_flags & PACKET_PUBLIC_FLAGS_MULTIPATH) != 0; public_header->reset_flag = (public_flags & PACKET_PUBLIC_FLAGS_RST) != 0; public_header->version_flag = (public_flags & PACKET_PUBLIC_FLAGS_VERSION) != 0; @@ -1004,6 +1014,12 @@ QuicFramer::AckFrameInfo QuicFramer::GetAckFrameInfo( bool QuicFramer::ProcessUnauthenticatedHeader(QuicDataReader* encrypted_reader, QuicPacketHeader* header) { + if (header->public_header.multipath_flag && + !ProcessPathId(encrypted_reader, &header->path_id)) { + set_detailed_error("Unable to read path id."); + return RaiseError(QUIC_INVALID_PACKET_HEADER); + } + if (!ProcessPacketSequenceNumber(encrypted_reader, header->public_header.packet_number_length, &header->packet_number)) { @@ -1062,6 +1078,14 @@ bool QuicFramer::ProcessAuthenticatedHeader(QuicDataReader* reader, return true; } +bool QuicFramer::ProcessPathId(QuicDataReader* reader, QuicPathId* path_id) { + if (!reader->ReadBytes(path_id, 1)) { + return false; + } + + return true; +} + bool QuicFramer::ProcessPacketSequenceNumber( QuicDataReader* reader, QuicPacketNumberLength packet_number_length, diff --git a/net/quic/quic_framer.h b/net/quic/quic_framer.h index 62f2ece..2f1e15c 100644 --- a/net/quic/quic_framer.h +++ b/net/quic/quic_framer.h @@ -394,6 +394,7 @@ class NET_EXPORT_PRIVATE QuicFramer { bool ProcessAuthenticatedHeader(QuicDataReader* reader, QuicPacketHeader* header); + bool ProcessPathId(QuicDataReader* reader, QuicPathId* path_id); bool ProcessPacketSequenceNumber(QuicDataReader* reader, QuicPacketNumberLength packet_number_length, QuicPacketNumber* packet_number); diff --git a/net/quic/quic_framer_test.cc b/net/quic/quic_framer_test.cc index e621c0d..a26dc34 100644 --- a/net/quic/quic_framer_test.cc +++ b/net/quic/quic_framer_test.cc @@ -43,6 +43,7 @@ const QuicPacketNumber kMask = kEpoch - 1; // Use fields in which each byte is distinct to ensure that every byte is // framed correctly. The values are otherwise arbitrary. const QuicConnectionId kConnectionId = UINT64_C(0xFEDCBA9876543210); +const QuicPathId kPathId = 0x42; const QuicPacketNumber kPacketNumber = UINT64_C(0x123456789ABC); const QuicPacketNumber kLargestObserved = UINT64_C(0x0123456789ABF); const QuicPacketNumber kMissingPacket = UINT64_C(0x0123456789ABE); @@ -62,49 +63,69 @@ size_t GetMinStreamFrameSize() { return kQuicFrameTypeSize + kQuicMaxStreamIdSize + kQuicMaxStreamOffsetSize; } +// Index into the path id offset in the header (if present). +size_t GetPathIdOffset(QuicConnectionIdLength connection_id_length, + bool include_version) { + return kConnectionIdOffset + connection_id_length + + (include_version ? kQuicVersionSize : 0); +} + // Index into the packet number offset in the header. size_t GetPacketNumberOffset(QuicConnectionIdLength connection_id_length, - bool include_version) { + bool include_version, + bool include_path_id) { return kConnectionIdOffset + connection_id_length + - (include_version ? kQuicVersionSize : 0); + (include_version ? kQuicVersionSize : 0) + + (include_path_id ? kQuicPathIdSize : 0); } -size_t GetPacketNumberOffset(bool include_version) { - return GetPacketNumberOffset(PACKET_8BYTE_CONNECTION_ID, include_version); +size_t GetPacketNumberOffset(bool include_version, bool include_path_id) { + return GetPacketNumberOffset(PACKET_8BYTE_CONNECTION_ID, include_version, + include_path_id); } // Index into the private flags offset in the data packet header. size_t GetPrivateFlagsOffset(QuicConnectionIdLength connection_id_length, - bool include_version) { - return GetPacketNumberOffset(connection_id_length, include_version) + + bool include_version, + bool include_path_id) { + return GetPacketNumberOffset(connection_id_length, include_version, + include_path_id) + PACKET_6BYTE_PACKET_NUMBER; } -size_t GetPrivateFlagsOffset(bool include_version) { - return GetPrivateFlagsOffset(PACKET_8BYTE_CONNECTION_ID, include_version); +size_t GetPrivateFlagsOffset(bool include_version, bool include_path_id) { + return GetPrivateFlagsOffset(PACKET_8BYTE_CONNECTION_ID, include_version, + include_path_id); } size_t GetPrivateFlagsOffset(bool include_version, + bool include_path_id, QuicPacketNumberLength packet_number_length) { - return GetPacketNumberOffset(PACKET_8BYTE_CONNECTION_ID, include_version) + + return GetPacketNumberOffset(PACKET_8BYTE_CONNECTION_ID, include_version, + include_path_id) + packet_number_length; } // Index into the fec group offset in the header. size_t GetFecGroupOffset(QuicConnectionIdLength connection_id_length, - bool include_version) { - return GetPrivateFlagsOffset(connection_id_length, include_version) + + bool include_version, + bool include_path_id) { + return GetPrivateFlagsOffset(connection_id_length, include_version, + include_path_id) + kPrivateFlagsSize; } -size_t GetFecGroupOffset(bool include_version) { - return GetPrivateFlagsOffset(PACKET_8BYTE_CONNECTION_ID, include_version) + +size_t GetFecGroupOffset(bool include_version, bool include_path_id) { + return GetPrivateFlagsOffset(PACKET_8BYTE_CONNECTION_ID, include_version, + include_path_id) + kPrivateFlagsSize; } size_t GetFecGroupOffset(bool include_version, + bool include_path_id, QuicPacketNumberLength packet_number_length) { - return GetPrivateFlagsOffset(include_version, packet_number_length) + + return GetPrivateFlagsOffset(include_version, include_path_id, + packet_number_length) + kPrivateFlagsSize; } @@ -445,7 +466,8 @@ class QuicFramerTest : public ::testing::TestWithParam<QuicVersion> { CheckProcessingFails( packet, i + GetPacketHeaderSize(PACKET_8BYTE_CONNECTION_ID, include_version, - PACKET_6BYTE_PACKET_NUMBER, NOT_IN_FEC_GROUP), + !kIncludePathId, PACKET_6BYTE_PACKET_NUMBER, + NOT_IN_FEC_GROUP), expected_error, QUIC_INVALID_STREAM_DATA); } } @@ -608,10 +630,10 @@ TEST_P(QuicFramerTest, LargePacket) { }; memset(packet + GetPacketHeaderSize( - PACKET_8BYTE_CONNECTION_ID, !kIncludeVersion, + PACKET_8BYTE_CONNECTION_ID, !kIncludeVersion, !kIncludePathId, PACKET_6BYTE_PACKET_NUMBER, NOT_IN_FEC_GROUP), 0, kMaxPacketSize - GetPacketHeaderSize( - PACKET_8BYTE_CONNECTION_ID, !kIncludeVersion, + PACKET_8BYTE_CONNECTION_ID, !kIncludeVersion, !kIncludePathId, PACKET_6BYTE_PACKET_NUMBER, NOT_IN_FEC_GROUP) + 1); // clang-format on @@ -644,6 +666,7 @@ TEST_P(QuicFramerTest, PacketHeader) { EXPECT_EQ(QUIC_MISSING_PAYLOAD, framer_.error()); ASSERT_TRUE(visitor_.header_.get()); EXPECT_EQ(kConnectionId, visitor_.header_->public_header.connection_id); + EXPECT_FALSE(visitor_.header_->public_header.multipath_flag); EXPECT_FALSE(visitor_.header_->public_header.reset_flag); EXPECT_FALSE(visitor_.header_->public_header.version_flag); EXPECT_FALSE(visitor_.header_->fec_flag); @@ -656,16 +679,17 @@ TEST_P(QuicFramerTest, PacketHeader) { // Now test framing boundaries. for (size_t i = 0; i < GetPacketHeaderSize(PACKET_8BYTE_CONNECTION_ID, !kIncludeVersion, - PACKET_6BYTE_PACKET_NUMBER, NOT_IN_FEC_GROUP); + !kIncludePathId, PACKET_6BYTE_PACKET_NUMBER, + NOT_IN_FEC_GROUP); ++i) { string expected_error; if (i < kConnectionIdOffset) { expected_error = "Unable to read public flags."; - } else if (i < GetPacketNumberOffset(!kIncludeVersion)) { + } else if (i < GetPacketNumberOffset(!kIncludeVersion, !kIncludePathId)) { expected_error = "Unable to read ConnectionId."; - } else if (i < GetPrivateFlagsOffset(!kIncludeVersion)) { + } else if (i < GetPrivateFlagsOffset(!kIncludeVersion, !kIncludePathId)) { expected_error = "Unable to read packet number."; - } else if (i < GetFecGroupOffset(!kIncludeVersion)) { + } else if (i < GetFecGroupOffset(!kIncludeVersion, !kIncludePathId)) { expected_error = "Unable to read private flags."; } else { expected_error = "Unable to read first fec protected packet offset."; @@ -695,6 +719,7 @@ TEST_P(QuicFramerTest, PacketHeaderWith4ByteConnectionId) { EXPECT_EQ(QUIC_MISSING_PAYLOAD, framer_.error()); ASSERT_TRUE(visitor_.header_.get()); EXPECT_EQ(kConnectionId, visitor_.header_->public_header.connection_id); + EXPECT_FALSE(visitor_.header_->public_header.multipath_flag); EXPECT_FALSE(visitor_.header_->public_header.reset_flag); EXPECT_FALSE(visitor_.header_->public_header.version_flag); EXPECT_FALSE(visitor_.header_->fec_flag); @@ -707,19 +732,20 @@ TEST_P(QuicFramerTest, PacketHeaderWith4ByteConnectionId) { // Now test framing boundaries. for (size_t i = 0; i < GetPacketHeaderSize(PACKET_4BYTE_CONNECTION_ID, !kIncludeVersion, - PACKET_6BYTE_PACKET_NUMBER, NOT_IN_FEC_GROUP); + !kIncludePathId, PACKET_6BYTE_PACKET_NUMBER, + NOT_IN_FEC_GROUP); ++i) { string expected_error; if (i < kConnectionIdOffset) { expected_error = "Unable to read public flags."; } else if (i < GetPacketNumberOffset(PACKET_4BYTE_CONNECTION_ID, - !kIncludeVersion)) { + !kIncludeVersion, !kIncludePathId)) { expected_error = "Unable to read ConnectionId."; } else if (i < GetPrivateFlagsOffset(PACKET_4BYTE_CONNECTION_ID, - !kIncludeVersion)) { + !kIncludeVersion, !kIncludePathId)) { expected_error = "Unable to read packet number."; } else if (i < GetFecGroupOffset(PACKET_4BYTE_CONNECTION_ID, - !kIncludeVersion)) { + !kIncludeVersion, !kIncludePathId)) { expected_error = "Unable to read private flags."; } else { expected_error = "Unable to read first fec protected packet offset."; @@ -749,6 +775,7 @@ TEST_P(QuicFramerTest, PacketHeader1ByteConnectionId) { EXPECT_EQ(QUIC_MISSING_PAYLOAD, framer_.error()); ASSERT_TRUE(visitor_.header_.get()); EXPECT_EQ(kConnectionId, visitor_.header_->public_header.connection_id); + EXPECT_FALSE(visitor_.header_->public_header.multipath_flag); EXPECT_FALSE(visitor_.header_->public_header.reset_flag); EXPECT_FALSE(visitor_.header_->public_header.version_flag); EXPECT_FALSE(visitor_.header_->fec_flag); @@ -761,19 +788,20 @@ TEST_P(QuicFramerTest, PacketHeader1ByteConnectionId) { // Now test framing boundaries. for (size_t i = 0; i < GetPacketHeaderSize(PACKET_1BYTE_CONNECTION_ID, !kIncludeVersion, - PACKET_6BYTE_PACKET_NUMBER, NOT_IN_FEC_GROUP); + !kIncludePathId, PACKET_6BYTE_PACKET_NUMBER, + NOT_IN_FEC_GROUP); ++i) { string expected_error; if (i < kConnectionIdOffset) { expected_error = "Unable to read public flags."; } else if (i < GetPacketNumberOffset(PACKET_1BYTE_CONNECTION_ID, - !kIncludeVersion)) { + !kIncludeVersion, !kIncludePathId)) { expected_error = "Unable to read ConnectionId."; } else if (i < GetPrivateFlagsOffset(PACKET_1BYTE_CONNECTION_ID, - !kIncludeVersion)) { + !kIncludeVersion, !kIncludePathId)) { expected_error = "Unable to read packet number."; } else if (i < GetFecGroupOffset(PACKET_1BYTE_CONNECTION_ID, - !kIncludeVersion)) { + !kIncludeVersion, !kIncludePathId)) { expected_error = "Unable to read private flags."; } else { expected_error = "Unable to read first fec protected packet offset."; @@ -803,6 +831,7 @@ TEST_P(QuicFramerTest, PacketHeaderWith0ByteConnectionId) { EXPECT_EQ(QUIC_MISSING_PAYLOAD, framer_.error()); ASSERT_TRUE(visitor_.header_.get()); EXPECT_EQ(kConnectionId, visitor_.header_->public_header.connection_id); + EXPECT_FALSE(visitor_.header_->public_header.multipath_flag); EXPECT_FALSE(visitor_.header_->public_header.reset_flag); EXPECT_FALSE(visitor_.header_->public_header.version_flag); EXPECT_FALSE(visitor_.header_->fec_flag); @@ -815,19 +844,20 @@ TEST_P(QuicFramerTest, PacketHeaderWith0ByteConnectionId) { // Now test framing boundaries. for (size_t i = 0; i < GetPacketHeaderSize(PACKET_0BYTE_CONNECTION_ID, !kIncludeVersion, - PACKET_6BYTE_PACKET_NUMBER, NOT_IN_FEC_GROUP); + !kIncludePathId, PACKET_6BYTE_PACKET_NUMBER, + NOT_IN_FEC_GROUP); ++i) { string expected_error; if (i < kConnectionIdOffset) { expected_error = "Unable to read public flags."; } else if (i < GetPacketNumberOffset(PACKET_0BYTE_CONNECTION_ID, - !kIncludeVersion)) { + !kIncludeVersion, !kIncludePathId)) { expected_error = "Unable to read ConnectionId."; } else if (i < GetPrivateFlagsOffset(PACKET_0BYTE_CONNECTION_ID, - !kIncludeVersion)) { + !kIncludeVersion, !kIncludePathId)) { expected_error = "Unable to read packet number."; } else if (i < GetFecGroupOffset(PACKET_0BYTE_CONNECTION_ID, - !kIncludeVersion)) { + !kIncludeVersion, !kIncludePathId)) { expected_error = "Unable to read private flags."; } else { expected_error = "Unable to read first fec protected packet offset."; @@ -857,6 +887,7 @@ TEST_P(QuicFramerTest, PacketHeaderWithVersionFlag) { EXPECT_EQ(QUIC_MISSING_PAYLOAD, framer_.error()); ASSERT_TRUE(visitor_.header_.get()); EXPECT_EQ(kConnectionId, visitor_.header_->public_header.connection_id); + EXPECT_FALSE(visitor_.header_->public_header.multipath_flag); EXPECT_FALSE(visitor_.header_->public_header.reset_flag); EXPECT_TRUE(visitor_.header_->public_header.version_flag); EXPECT_EQ(GetParam(), visitor_.header_->public_header.versions[0]); @@ -870,18 +901,138 @@ TEST_P(QuicFramerTest, PacketHeaderWithVersionFlag) { // Now test framing boundaries. for (size_t i = 0; i < GetPacketHeaderSize(PACKET_8BYTE_CONNECTION_ID, kIncludeVersion, - PACKET_6BYTE_PACKET_NUMBER, NOT_IN_FEC_GROUP); + !kIncludePathId, PACKET_6BYTE_PACKET_NUMBER, + NOT_IN_FEC_GROUP); ++i) { string expected_error; if (i < kConnectionIdOffset) { expected_error = "Unable to read public flags."; } else if (i < kVersionOffset) { expected_error = "Unable to read ConnectionId."; - } else if (i < GetPacketNumberOffset(kIncludeVersion)) { + } else if (i < GetPacketNumberOffset(kIncludeVersion, !kIncludePathId)) { expected_error = "Unable to read protocol version."; - } else if (i < GetPrivateFlagsOffset(kIncludeVersion)) { + } else if (i < GetPrivateFlagsOffset(kIncludeVersion, !kIncludePathId)) { expected_error = "Unable to read packet number."; - } else if (i < GetFecGroupOffset(kIncludeVersion)) { + } else if (i < GetFecGroupOffset(kIncludeVersion, !kIncludePathId)) { + expected_error = "Unable to read private flags."; + } else { + expected_error = "Unable to read first fec protected packet offset."; + } + CheckProcessingFails(packet, i, expected_error, QUIC_INVALID_PACKET_HEADER); + } +} + +TEST_P(QuicFramerTest, PacketHeaderWithMultipathFlag) { + // clang-format off + unsigned char packet[] = { + // public flags (version) + 0x7C, + // connection_id + 0x10, 0x32, 0x54, 0x76, 0x98, 0xBA, 0xDC, 0xFE, + // path_id + 0x42, + // packet number + 0xBC, 0x9A, 0x78, 0x56, 0x34, 0x12, + // private flags + 0x00, + }; + // clang-format on + + QuicEncryptedPacket encrypted(AsChars(packet), arraysize(packet), false); + EXPECT_FALSE(framer_.ProcessPacket(encrypted)); + EXPECT_EQ(QUIC_MISSING_PAYLOAD, framer_.error()); + ASSERT_TRUE(visitor_.header_.get()); + EXPECT_EQ(kConnectionId, visitor_.header_->public_header.connection_id); + EXPECT_TRUE(visitor_.header_->public_header.multipath_flag); + EXPECT_FALSE(visitor_.header_->public_header.reset_flag); + EXPECT_FALSE(visitor_.header_->public_header.version_flag); + EXPECT_FALSE(visitor_.header_->fec_flag); + EXPECT_FALSE(visitor_.header_->entropy_flag); + EXPECT_EQ(0, visitor_.header_->entropy_hash); + EXPECT_EQ(kPathId, visitor_.header_->path_id); + EXPECT_EQ(kPacketNumber, visitor_.header_->packet_number); + EXPECT_EQ(NOT_IN_FEC_GROUP, visitor_.header_->is_in_fec_group); + EXPECT_EQ(0u, visitor_.header_->fec_group); + + // Now test framing boundaries. + for (size_t i = 0; + i < GetPacketHeaderSize(PACKET_8BYTE_CONNECTION_ID, !kIncludeVersion, + kIncludePathId, PACKET_6BYTE_PACKET_NUMBER, + NOT_IN_FEC_GROUP); + ++i) { + string expected_error; + if (i < kConnectionIdOffset) { + expected_error = "Unable to read public flags."; + } else if (i < + GetPathIdOffset(PACKET_8BYTE_CONNECTION_ID, !kIncludeVersion)) { + expected_error = "Unable to read ConnectionId."; + } else if (i < GetPacketNumberOffset(!kIncludeVersion, kIncludePathId)) { + expected_error = "Unable to read path id."; + } else if (i < GetPrivateFlagsOffset(!kIncludeVersion, kIncludePathId)) { + expected_error = "Unable to read packet number."; + } else if (i < GetFecGroupOffset(!kIncludeVersion, kIncludePathId)) { + expected_error = "Unable to read private flags."; + } else { + expected_error = "Unable to read first fec protected packet offset."; + } + CheckProcessingFails(packet, i, expected_error, QUIC_INVALID_PACKET_HEADER); + } +} + +TEST_P(QuicFramerTest, PacketHeaderWithBothVersionFlagAndMultipathFlag) { + // clang-format off + unsigned char packet[] = { + // public flags (version) + 0x7D, + // connection_id + 0x10, 0x32, 0x54, 0x76, 0x98, 0xBA, 0xDC, 0xFE, + // version tag + 'Q', '0', GetQuicVersionDigitTens(), GetQuicVersionDigitOnes(), + // path_id + 0x42, + // packet number + 0xBC, 0x9A, 0x78, 0x56, 0x34, 0x12, + // private flags + 0x00, + }; + // clang-format on + + QuicEncryptedPacket encrypted(AsChars(packet), arraysize(packet), false); + EXPECT_FALSE(framer_.ProcessPacket(encrypted)); + EXPECT_EQ(QUIC_MISSING_PAYLOAD, framer_.error()); + ASSERT_TRUE(visitor_.header_.get()); + EXPECT_EQ(kConnectionId, visitor_.header_->public_header.connection_id); + EXPECT_TRUE(visitor_.header_->public_header.multipath_flag); + EXPECT_FALSE(visitor_.header_->public_header.reset_flag); + EXPECT_TRUE(visitor_.header_->public_header.version_flag); + EXPECT_EQ(GetParam(), visitor_.header_->public_header.versions[0]); + EXPECT_FALSE(visitor_.header_->fec_flag); + EXPECT_FALSE(visitor_.header_->entropy_flag); + EXPECT_EQ(0, visitor_.header_->entropy_hash); + EXPECT_EQ(kPathId, visitor_.header_->path_id); + EXPECT_EQ(kPacketNumber, visitor_.header_->packet_number); + EXPECT_EQ(NOT_IN_FEC_GROUP, visitor_.header_->is_in_fec_group); + EXPECT_EQ(0u, visitor_.header_->fec_group); + + // Now test framing boundaries. + for (size_t i = 0; + i < GetPacketHeaderSize(PACKET_8BYTE_CONNECTION_ID, !kIncludeVersion, + kIncludePathId, PACKET_6BYTE_PACKET_NUMBER, + NOT_IN_FEC_GROUP); + ++i) { + string expected_error; + if (i < kConnectionIdOffset) { + expected_error = "Unable to read public flags."; + } else if (i < kVersionOffset) { + expected_error = "Unable to read ConnectionId."; + } else if (i < + GetPathIdOffset(PACKET_8BYTE_CONNECTION_ID, kIncludeVersion)) { + expected_error = "Unable to read protocol version."; + } else if (i < GetPacketNumberOffset(kIncludeVersion, kIncludePathId)) { + expected_error = "Unable to read path id."; + } else if (i < GetPrivateFlagsOffset(kIncludeVersion, kIncludePathId)) { + expected_error = "Unable to read packet number."; + } else if (i < GetFecGroupOffset(kIncludeVersion, kIncludePathId)) { expected_error = "Unable to read private flags."; } else { expected_error = "Unable to read first fec protected packet offset."; @@ -911,6 +1062,7 @@ TEST_P(QuicFramerTest, PacketHeaderWith4BytePacketNumber) { EXPECT_EQ(QUIC_MISSING_PAYLOAD, framer_.error()); ASSERT_TRUE(visitor_.header_.get()); EXPECT_EQ(kConnectionId, visitor_.header_->public_header.connection_id); + EXPECT_FALSE(visitor_.header_->public_header.multipath_flag); EXPECT_FALSE(visitor_.header_->public_header.reset_flag); EXPECT_FALSE(visitor_.header_->public_header.version_flag); EXPECT_FALSE(visitor_.header_->fec_flag); @@ -923,17 +1075,18 @@ TEST_P(QuicFramerTest, PacketHeaderWith4BytePacketNumber) { // Now test framing boundaries. for (size_t i = 0; i < GetPacketHeaderSize(PACKET_8BYTE_CONNECTION_ID, !kIncludeVersion, - PACKET_4BYTE_PACKET_NUMBER, NOT_IN_FEC_GROUP); + !kIncludePathId, PACKET_4BYTE_PACKET_NUMBER, + NOT_IN_FEC_GROUP); ++i) { string expected_error; if (i < kConnectionIdOffset) { expected_error = "Unable to read public flags."; - } else if (i < GetPacketNumberOffset(!kIncludeVersion)) { + } else if (i < GetPacketNumberOffset(!kIncludeVersion, !kIncludePathId)) { expected_error = "Unable to read ConnectionId."; - } else if (i < GetPrivateFlagsOffset(!kIncludeVersion, + } else if (i < GetPrivateFlagsOffset(!kIncludeVersion, !kIncludePathId, PACKET_4BYTE_PACKET_NUMBER)) { expected_error = "Unable to read packet number."; - } else if (i < GetFecGroupOffset(!kIncludeVersion, + } else if (i < GetFecGroupOffset(!kIncludeVersion, !kIncludePathId, PACKET_4BYTE_PACKET_NUMBER)) { expected_error = "Unable to read private flags."; } else { @@ -964,6 +1117,7 @@ TEST_P(QuicFramerTest, PacketHeaderWith2BytePacketNumber) { EXPECT_EQ(QUIC_MISSING_PAYLOAD, framer_.error()); ASSERT_TRUE(visitor_.header_.get()); EXPECT_EQ(kConnectionId, visitor_.header_->public_header.connection_id); + EXPECT_FALSE(visitor_.header_->public_header.multipath_flag); EXPECT_FALSE(visitor_.header_->public_header.reset_flag); EXPECT_FALSE(visitor_.header_->public_header.version_flag); EXPECT_FALSE(visitor_.header_->fec_flag); @@ -976,17 +1130,18 @@ TEST_P(QuicFramerTest, PacketHeaderWith2BytePacketNumber) { // Now test framing boundaries. for (size_t i = 0; i < GetPacketHeaderSize(PACKET_8BYTE_CONNECTION_ID, !kIncludeVersion, - PACKET_2BYTE_PACKET_NUMBER, NOT_IN_FEC_GROUP); + !kIncludePathId, PACKET_2BYTE_PACKET_NUMBER, + NOT_IN_FEC_GROUP); ++i) { string expected_error; if (i < kConnectionIdOffset) { expected_error = "Unable to read public flags."; - } else if (i < GetPacketNumberOffset(!kIncludeVersion)) { + } else if (i < GetPacketNumberOffset(!kIncludeVersion, !kIncludePathId)) { expected_error = "Unable to read ConnectionId."; - } else if (i < GetPrivateFlagsOffset(!kIncludeVersion, + } else if (i < GetPrivateFlagsOffset(!kIncludeVersion, !kIncludePathId, PACKET_2BYTE_PACKET_NUMBER)) { expected_error = "Unable to read packet number."; - } else if (i < GetFecGroupOffset(!kIncludeVersion, + } else if (i < GetFecGroupOffset(!kIncludeVersion, !kIncludePathId, PACKET_2BYTE_PACKET_NUMBER)) { expected_error = "Unable to read private flags."; } else { @@ -1017,6 +1172,7 @@ TEST_P(QuicFramerTest, PacketHeaderWith1BytePacketNumber) { EXPECT_EQ(QUIC_MISSING_PAYLOAD, framer_.error()); ASSERT_TRUE(visitor_.header_.get()); EXPECT_EQ(kConnectionId, visitor_.header_->public_header.connection_id); + EXPECT_FALSE(visitor_.header_->public_header.multipath_flag); EXPECT_FALSE(visitor_.header_->public_header.reset_flag); EXPECT_FALSE(visitor_.header_->public_header.version_flag); EXPECT_FALSE(visitor_.header_->fec_flag); @@ -1029,17 +1185,18 @@ TEST_P(QuicFramerTest, PacketHeaderWith1BytePacketNumber) { // Now test framing boundaries. for (size_t i = 0; i < GetPacketHeaderSize(PACKET_8BYTE_CONNECTION_ID, !kIncludeVersion, - PACKET_1BYTE_PACKET_NUMBER, NOT_IN_FEC_GROUP); + !kIncludePathId, PACKET_1BYTE_PACKET_NUMBER, + NOT_IN_FEC_GROUP); ++i) { string expected_error; if (i < kConnectionIdOffset) { expected_error = "Unable to read public flags."; - } else if (i < GetPacketNumberOffset(!kIncludeVersion)) { + } else if (i < GetPacketNumberOffset(!kIncludeVersion, !kIncludePathId)) { expected_error = "Unable to read ConnectionId."; - } else if (i < GetPrivateFlagsOffset(!kIncludeVersion, + } else if (i < GetPrivateFlagsOffset(!kIncludeVersion, !kIncludePathId, PACKET_1BYTE_PACKET_NUMBER)) { expected_error = "Unable to read packet number."; - } else if (i < GetFecGroupOffset(!kIncludeVersion, + } else if (i < GetFecGroupOffset(!kIncludeVersion, !kIncludePathId, PACKET_1BYTE_PACKET_NUMBER)) { expected_error = "Unable to read private flags."; } else { @@ -1080,7 +1237,7 @@ TEST_P(QuicFramerTest, InvalidPublicFlagWithMatchingVersions) { // clang-format off unsigned char packet[] = { // public flags (8 byte connection_id and version flag and an unknown flag) - 0x4D, + 0x8D, // connection_id 0x10, 0x32, 0x54, 0x76, 0x98, 0xBA, 0xDC, 0xFE, @@ -1221,7 +1378,8 @@ TEST_P(QuicFramerTest, PaddingFrame) { // A packet with no frames is not acceptable. CheckProcessingFails( packet, GetPacketHeaderSize(PACKET_8BYTE_CONNECTION_ID, !kIncludeVersion, - PACKET_6BYTE_PACKET_NUMBER, NOT_IN_FEC_GROUP), + !kIncludePathId, PACKET_6BYTE_PACKET_NUMBER, + NOT_IN_FEC_GROUP), "Packet has no frames.", QUIC_MISSING_PAYLOAD); } @@ -1761,7 +1919,8 @@ TEST_P(QuicFramerTest, AckFrameTwoTimestamp) { CheckProcessingFails( packet, i + GetPacketHeaderSize(PACKET_8BYTE_CONNECTION_ID, !kIncludeVersion, - PACKET_6BYTE_PACKET_NUMBER, NOT_IN_FEC_GROUP), + !kIncludePathId, PACKET_6BYTE_PACKET_NUMBER, + NOT_IN_FEC_GROUP), expected_error, QUIC_INVALID_ACK_DATA); } } @@ -1867,7 +2026,8 @@ TEST_P(QuicFramerTest, AckFrameOneTimestamp) { CheckProcessingFails( packet, i + GetPacketHeaderSize(PACKET_8BYTE_CONNECTION_ID, !kIncludeVersion, - PACKET_6BYTE_PACKET_NUMBER, NOT_IN_FEC_GROUP), + !kIncludePathId, PACKET_6BYTE_PACKET_NUMBER, + NOT_IN_FEC_GROUP), expected_error, QUIC_INVALID_ACK_DATA); } } @@ -1961,7 +2121,8 @@ TEST_P(QuicFramerTest, AckFrame) { CheckProcessingFails( packet, i + GetPacketHeaderSize(PACKET_8BYTE_CONNECTION_ID, !kIncludeVersion, - PACKET_6BYTE_PACKET_NUMBER, NOT_IN_FEC_GROUP), + !kIncludePathId, PACKET_6BYTE_PACKET_NUMBER, + NOT_IN_FEC_GROUP), expected_error, QUIC_INVALID_ACK_DATA); } } @@ -2065,7 +2226,8 @@ TEST_P(QuicFramerTest, AckFrameRevivedPackets) { CheckProcessingFails( packet, i + GetPacketHeaderSize(PACKET_8BYTE_CONNECTION_ID, !kIncludeVersion, - PACKET_6BYTE_PACKET_NUMBER, NOT_IN_FEC_GROUP), + !kIncludePathId, PACKET_6BYTE_PACKET_NUMBER, + NOT_IN_FEC_GROUP), expected_error, QUIC_INVALID_ACK_DATA); } } @@ -2240,7 +2402,8 @@ TEST_P(QuicFramerTest, StopWaitingFrame) { CheckProcessingFails( packet, i + GetPacketHeaderSize(PACKET_8BYTE_CONNECTION_ID, !kIncludeVersion, - PACKET_6BYTE_PACKET_NUMBER, NOT_IN_FEC_GROUP), + !kIncludePathId, PACKET_6BYTE_PACKET_NUMBER, + NOT_IN_FEC_GROUP), expected_error, QUIC_INVALID_STOP_WAITING_DATA); } } @@ -2300,7 +2463,8 @@ TEST_P(QuicFramerTest, RstStreamFrameQuic) { CheckProcessingFails( packet, i + GetPacketHeaderSize(PACKET_8BYTE_CONNECTION_ID, !kIncludeVersion, - PACKET_6BYTE_PACKET_NUMBER, NOT_IN_FEC_GROUP), + !kIncludePathId, PACKET_6BYTE_PACKET_NUMBER, + NOT_IN_FEC_GROUP), expected_error, QUIC_INVALID_RST_STREAM_DATA); } } @@ -2360,7 +2524,8 @@ TEST_P(QuicFramerTest, ConnectionCloseFrame) { CheckProcessingFails( packet, i + GetPacketHeaderSize(PACKET_8BYTE_CONNECTION_ID, !kIncludeVersion, - PACKET_6BYTE_PACKET_NUMBER, NOT_IN_FEC_GROUP), + !kIncludePathId, PACKET_6BYTE_PACKET_NUMBER, + NOT_IN_FEC_GROUP), expected_error, QUIC_INVALID_CONNECTION_CLOSE_DATA); } } @@ -2422,7 +2587,8 @@ TEST_P(QuicFramerTest, GoAwayFrame) { CheckProcessingFails( packet, i + GetPacketHeaderSize(PACKET_8BYTE_CONNECTION_ID, !kIncludeVersion, - PACKET_6BYTE_PACKET_NUMBER, NOT_IN_FEC_GROUP), + !kIncludePathId, PACKET_6BYTE_PACKET_NUMBER, + NOT_IN_FEC_GROUP), expected_error, QUIC_INVALID_GOAWAY_DATA); } } @@ -2474,7 +2640,8 @@ TEST_P(QuicFramerTest, WindowUpdateFrame) { CheckProcessingFails( packet, i + GetPacketHeaderSize(PACKET_8BYTE_CONNECTION_ID, !kIncludeVersion, - PACKET_6BYTE_PACKET_NUMBER, NOT_IN_FEC_GROUP), + !kIncludePathId, PACKET_6BYTE_PACKET_NUMBER, + NOT_IN_FEC_GROUP), expected_error, QUIC_INVALID_WINDOW_UPDATE_DATA); } } @@ -2517,7 +2684,8 @@ TEST_P(QuicFramerTest, BlockedFrame) { CheckProcessingFails( packet, i + GetPacketHeaderSize(PACKET_8BYTE_CONNECTION_ID, !kIncludeVersion, - PACKET_6BYTE_PACKET_NUMBER, NOT_IN_FEC_GROUP), + !kIncludePathId, PACKET_6BYTE_PACKET_NUMBER, + NOT_IN_FEC_GROUP), expected_error, QUIC_INVALID_BLOCKED_DATA); } } @@ -2834,9 +3002,9 @@ TEST_P(QuicFramerTest, BuildPaddingFramePacket) { }; // clang-format on - uint64 header_size = - GetPacketHeaderSize(PACKET_8BYTE_CONNECTION_ID, !kIncludeVersion, - PACKET_6BYTE_PACKET_NUMBER, NOT_IN_FEC_GROUP); + uint64 header_size = GetPacketHeaderSize( + PACKET_8BYTE_CONNECTION_ID, !kIncludeVersion, !kIncludePathId, + PACKET_6BYTE_PACKET_NUMBER, NOT_IN_FEC_GROUP); memset(packet + header_size + 1, 0x00, kMaxPacketSize - header_size - 1); scoped_ptr<QuicPacket> data(BuildDataPacket(header, frames)); @@ -2881,9 +3049,9 @@ TEST_P(QuicFramerTest, Build4ByteSequenceNumberPaddingFramePacket) { }; // clang-format on - uint64 header_size = - GetPacketHeaderSize(PACKET_8BYTE_CONNECTION_ID, !kIncludeVersion, - PACKET_4BYTE_PACKET_NUMBER, NOT_IN_FEC_GROUP); + uint64 header_size = GetPacketHeaderSize( + PACKET_8BYTE_CONNECTION_ID, !kIncludeVersion, !kIncludePathId, + PACKET_4BYTE_PACKET_NUMBER, NOT_IN_FEC_GROUP); memset(packet + header_size + 1, 0x00, kMaxPacketSize - header_size - 1); scoped_ptr<QuicPacket> data(BuildDataPacket(header, frames)); @@ -2928,9 +3096,9 @@ TEST_P(QuicFramerTest, Build2ByteSequenceNumberPaddingFramePacket) { }; // clang-format on - uint64 header_size = - GetPacketHeaderSize(PACKET_8BYTE_CONNECTION_ID, !kIncludeVersion, - PACKET_2BYTE_PACKET_NUMBER, NOT_IN_FEC_GROUP); + uint64 header_size = GetPacketHeaderSize( + PACKET_8BYTE_CONNECTION_ID, !kIncludeVersion, !kIncludePathId, + PACKET_2BYTE_PACKET_NUMBER, NOT_IN_FEC_GROUP); memset(packet + header_size + 1, 0x00, kMaxPacketSize - header_size - 1); scoped_ptr<QuicPacket> data(BuildDataPacket(header, frames)); @@ -2975,9 +3143,9 @@ TEST_P(QuicFramerTest, Build1ByteSequenceNumberPaddingFramePacket) { }; // clang-format on - uint64 header_size = - GetPacketHeaderSize(PACKET_8BYTE_CONNECTION_ID, !kIncludeVersion, - PACKET_1BYTE_PACKET_NUMBER, NOT_IN_FEC_GROUP); + uint64 header_size = GetPacketHeaderSize( + PACKET_8BYTE_CONNECTION_ID, !kIncludeVersion, !kIncludePathId, + PACKET_1BYTE_PACKET_NUMBER, NOT_IN_FEC_GROUP); memset(packet + header_size + 1, 0x00, kMaxPacketSize - header_size - 1); scoped_ptr<QuicPacket> data(BuildDataPacket(header, frames)); @@ -3137,6 +3305,119 @@ TEST_P(QuicFramerTest, BuildStreamFramePacketWithVersionFlag) { arraysize(packet)); } +TEST_P(QuicFramerTest, BuildStreamFramePacketWithMultipathFlag) { + QuicPacketHeader header; + header.public_header.connection_id = kConnectionId; + header.public_header.multipath_flag = true; + header.public_header.reset_flag = false; + header.public_header.version_flag = false; + header.fec_flag = false; + header.entropy_flag = true; + header.path_id = kPathId; + header.packet_number = kPacketNumber; + header.fec_group = 0; + + QuicStreamFrame stream_frame(kStreamId, true, kStreamOffset, + StringPiece("hello world!")); + + QuicFrames frames; + frames.push_back(QuicFrame(&stream_frame)); + + // clang-format off + unsigned char packet[] = { + // public flags (8 byte connection_id) + 0x7C, + // connection_id + 0x10, 0x32, 0x54, 0x76, + 0x98, 0xBA, 0xDC, 0xFE, + // path_id + 0x42, + // packet number + 0xBC, 0x9A, 0x78, 0x56, + 0x34, 0x12, + // private flags (entropy) + 0x01, + + // frame type (stream frame with fin and no length) + 0xDF, + // stream id + 0x04, 0x03, 0x02, 0x01, + // offset + 0x54, 0x76, 0x10, 0x32, + 0xDC, 0xFE, 0x98, 0xBA, + // data + 'h', 'e', 'l', 'l', + 'o', ' ', 'w', 'o', + 'r', 'l', 'd', '!', + }; + // clang-format on + + scoped_ptr<QuicPacket> data(BuildDataPacket(header, frames)); + ASSERT_TRUE(data != nullptr); + + test::CompareCharArraysWithHexError("constructed packet", data->data(), + data->length(), AsChars(packet), + arraysize(packet)); +} + +TEST_P(QuicFramerTest, BuildStreamFramePacketWithBothVersionAndMultipathFlag) { + QuicPacketHeader header; + header.public_header.connection_id = kConnectionId; + header.public_header.multipath_flag = true; + header.public_header.reset_flag = false; + header.public_header.version_flag = true; + header.fec_flag = false; + header.entropy_flag = true; + header.path_id = kPathId; + header.packet_number = kPacketNumber; + header.fec_group = 0; + + QuicStreamFrame stream_frame(kStreamId, true, kStreamOffset, + StringPiece("hello world!")); + + QuicFrames frames; + frames.push_back(QuicFrame(&stream_frame)); + + // clang-format off + unsigned char packet[] = { + // public flags (8 byte connection_id) + 0x7D, + // connection_id + 0x10, 0x32, 0x54, 0x76, + 0x98, 0xBA, 0xDC, 0xFE, + // version tag + 'Q', '0', GetQuicVersionDigitTens(), GetQuicVersionDigitOnes(), + // path_id + 0x42, + // packet number + 0xBC, 0x9A, 0x78, 0x56, + 0x34, 0x12, + // private flags (entropy) + 0x01, + + // frame type (stream frame with fin and no length) + 0xDF, + // stream id + 0x04, 0x03, 0x02, 0x01, + // offset + 0x54, 0x76, 0x10, 0x32, + 0xDC, 0xFE, 0x98, 0xBA, + // data + 'h', 'e', 'l', 'l', + 'o', ' ', 'w', 'o', + 'r', 'l', 'd', '!', + }; + // clang-format on + + QuicFramerPeer::SetPerspective(&framer_, Perspective::IS_CLIENT); + scoped_ptr<QuicPacket> data(BuildDataPacket(header, frames)); + ASSERT_TRUE(data != nullptr); + + test::CompareCharArraysWithHexError("constructed packet", data->data(), + data->length(), AsChars(packet), + arraysize(packet)); +} + TEST_P(QuicFramerTest, BuildVersionNegotiationPacket) { // clang-format off unsigned char packet[] = { diff --git a/net/quic/quic_headers_stream.cc b/net/quic/quic_headers_stream.cc index 0d93cf9..7d27580 100644 --- a/net/quic/quic_headers_stream.cc +++ b/net/quic/quic_headers_stream.cc @@ -131,11 +131,8 @@ class QuicHeadersStream::SpdyFramerVisitor if (!stream_->IsConnected()) { return; } - if (has_priority) { - stream_->OnSynStream(stream_id, priority, fin); - } else { - stream_->OnSynReply(stream_id, fin); - } + + stream_->OnHeaders(stream_id, has_priority, priority, fin); } void OnWindowUpdate(SpdyStreamId stream_id, int delta_window_size) override { @@ -276,30 +273,26 @@ void QuicHeadersStream::OnDataAvailable() { } SpdyPriority QuicHeadersStream::Priority() const { - return net::kHighestPriority; // The smallest priority is also the highest + return net::kV3HighestPriority; } -void QuicHeadersStream::OnSynStream(SpdyStreamId stream_id, - SpdyPriority priority, - bool fin) { - if (session()->perspective() == Perspective::IS_CLIENT) { - CloseConnectionWithDetails( - QUIC_INVALID_HEADERS_STREAM_DATA, - "SPDY SYN_STREAM frame received at the client"); - return; - } - DCHECK_EQ(kInvalidStreamId, stream_id_); - stream_id_ = stream_id; - fin_ = fin; - spdy_session_->OnStreamHeadersPriority(stream_id, priority); -} - -void QuicHeadersStream::OnSynReply(SpdyStreamId stream_id, bool fin) { - if (session()->perspective() == Perspective::IS_SERVER) { - CloseConnectionWithDetails( - QUIC_INVALID_HEADERS_STREAM_DATA, - "SPDY SYN_REPLY frame received at the server"); - return; +void QuicHeadersStream::OnHeaders(SpdyStreamId stream_id, + bool has_priority, + SpdyPriority priority, + bool fin) { + if (has_priority) { + if (session()->perspective() == Perspective::IS_CLIENT) { + CloseConnectionWithDetails(QUIC_INVALID_HEADERS_STREAM_DATA, + "Server must not send priorities."); + return; + } + spdy_session_->OnStreamHeadersPriority(stream_id, priority); + } else { + if (session()->perspective() == Perspective::IS_SERVER) { + CloseConnectionWithDetails(QUIC_INVALID_HEADERS_STREAM_DATA, + "Client must send priorities."); + return; + } } DCHECK_EQ(kInvalidStreamId, stream_id_); stream_id_ = stream_id; diff --git a/net/quic/quic_headers_stream.h b/net/quic/quic_headers_stream.h index ef1bfef..5114d34 100644 --- a/net/quic/quic_headers_stream.h +++ b/net/quic/quic_headers_stream.h @@ -16,19 +16,17 @@ namespace net { class QuicSpdySession; -// Headers in QUIC are sent as SPDY SYN_STREAM or SYN_REPLY frames -// over a reserved reliable stream with the id 3. Each endpoint (client -// and server) will allocate an instance of QuicHeadersStream to send -// and receive headers. +// Headers in QUIC are sent as HTTP/2 HEADERS frames over a reserved reliable +// stream with the id 3. Each endpoint (client and server) will allocate an +// instance of QuicHeadersStream to send and receive headers. class NET_EXPORT_PRIVATE QuicHeadersStream : public ReliableQuicStream { public: explicit QuicHeadersStream(QuicSpdySession* session); ~QuicHeadersStream() override; - // Writes |headers| for |stream_id| in a SYN_STREAM or SYN_REPLY - // frame to the peer. If |fin| is true, the fin flag will be set on - // the SPDY frame. Returns the size, in bytes, of the resulting - // SPDY frame. + // Writes |headers| for |stream_id| in an HTTP/2 HEADERS frame to the peer. + // If |fin| is true, the fin flag will be set on the HEADERS frame. Returns + // the size, in bytes, of the resulting HEADERS frame. size_t WriteHeaders(QuicStreamId stream_id, const SpdyHeaderBlock& headers, bool fin, @@ -36,8 +34,8 @@ class NET_EXPORT_PRIVATE QuicHeadersStream : public ReliableQuicStream { QuicAckListenerInterface* ack_listener); // Write |headers| for |promised_stream_id| on |original_stream_id| in a - // PUSH_PROMISE frame to peer. Return the size, in bytes, of the resulting - // SPDY frame. + // PUSH_PROMISE frame to peer. + // Return the size, in bytes, of the resulting PUSH_PROMISE frame. size_t WritePushPromise(QuicStreamId original_stream_id, QuicStreamId promised_stream_id, const SpdyHeaderBlock& headers, @@ -52,16 +50,14 @@ class NET_EXPORT_PRIVATE QuicHeadersStream : public ReliableQuicStream { // The following methods are called by the SimpleVisitor. - // Called when a SYN_STREAM frame has been received. - void OnSynStream(SpdyStreamId stream_id, - SpdyPriority priority, - bool fin); - - // Called when a SYN_REPLY frame been received. - void OnSynReply(SpdyStreamId stream_id, bool fin); + // Called when a HEADERS frame has been received. + void OnHeaders(SpdyStreamId stream_id, + bool has_priority, + SpdyPriority priority, + bool fin); // Called when a chunk of header data is available. This is called - // after OnSynStream, or OnSynReply. + // after OnHeaders. // |stream_id| The stream receiving the header data. // |header_data| A buffer containing the header data chunk received. // |len| The length of the header data buffer. A length of zero indicates diff --git a/net/quic/quic_http_stream_test.cc b/net/quic/quic_http_stream_test.cc index 6bc1af2..f89cbe6 100644 --- a/net/quic/quic_http_stream_test.cc +++ b/net/quic/quic_http_stream_test.cc @@ -823,7 +823,7 @@ TEST_P(QuicHttpStreamTest, Priority) { QuicReliableClientStream* reliable_stream = QuicHttpStreamPeer::GetQuicReliableClientStream(stream_.get()); DCHECK(reliable_stream); - DCHECK_EQ(kHighestPriority, reliable_stream->Priority()); + DCHECK_EQ(kV3HighestPriority, reliable_stream->Priority()); EXPECT_EQ(OK, stream_->SendRequest(headers_, &response_, callback_.callback())); @@ -875,12 +875,12 @@ TEST_P(QuicHttpStreamTest, CheckPriorityWithNoDelegate) { DCHECK(reliable_stream); QuicReliableClientStream::Delegate* delegate = reliable_stream->GetDelegate(); DCHECK(delegate); - DCHECK_EQ(kHighestPriority, reliable_stream->Priority()); + DCHECK_EQ(kV3HighestPriority, reliable_stream->Priority()); // Set Delegate to nullptr and make sure Priority returns highest // priority. reliable_stream->SetDelegate(nullptr); - DCHECK_EQ(kHighestPriority, reliable_stream->Priority()); + DCHECK_EQ(kV3HighestPriority, reliable_stream->Priority()); reliable_stream->SetDelegate(delegate); EXPECT_EQ(0, stream_->GetTotalSentBytes()); diff --git a/net/quic/quic_packet_creator.cc b/net/quic/quic_packet_creator.cc index d76488e..33d112d 100644 --- a/net/quic/quic_packet_creator.cc +++ b/net/quic/quic_packet_creator.cc @@ -30,6 +30,20 @@ static const size_t kDefaultMaxPacketsPerFecGroup = 10; // Lowest max packets in an FEC group. static const size_t kLowestMaxPacketsPerFecGroup = 2; +// We want to put some space between a protected packet and the FEC packet to +// avoid losing them both within the same loss episode. On the other hand, we +// expect to be able to recover from any loss in about an RTT. We resolve this +// tradeoff by sending an FEC packet atmost half an RTT, or equivalently, half +// the max number of in-flight packets, the first protected packet. Since we +// don't want to delay an FEC packet past half an RTT, we set the max FEC group +// size to be half the current congestion window. +const float kMaxPacketsInFlightMultiplierForFecGroupSize = 0.5; +const float kRttMultiplierForFecTimeout = 0.5; + +// Minimum timeout for FEC alarm, set to half the minimum Tail Loss Probe +// timeout of 10ms. +const int64 kMinFecTimeoutMs = 5u; + } // namespace // A QuicRandom wrapper that gets a bucket of entropy and distributes it @@ -78,6 +92,7 @@ QuicPacketCreator::QuicPacketCreator(QuicConnectionId connection_id, random_bool_source_(new QuicRandomBoolSource(random_generator)), packet_number_(0), should_fec_protect_(false), + fec_protect_(false), send_version_in_packet_(framer->perspective() == Perspective::IS_CLIENT), max_packet_length_(0), max_packets_per_fec_group_(kDefaultMaxPacketsPerFecGroup), @@ -85,7 +100,10 @@ QuicPacketCreator::QuicPacketCreator(QuicConnectionId connection_id, next_packet_number_length_(PACKET_1BYTE_PACKET_NUMBER), packet_number_length_(next_packet_number_length_), packet_size_(0), - needs_padding_(false) { + needs_padding_(false), + fec_send_policy_(FEC_ANY_TRIGGER), + fec_timeout_(QuicTime::Delta::Zero()), + rtt_multiplier_for_fec_timeout_(kRttMultiplierForFecTimeout) { SetMaxPacketLength(kDefaultMaxPacketSize); } @@ -131,13 +149,9 @@ void QuicPacketCreator::set_max_packets_per_fec_group( DCHECK_LT(0u, max_packets_per_fec_group_); } -QuicFecGroupNumber QuicPacketCreator::fec_group_number() { - return fec_group_ != nullptr ? fec_group_->FecGroupNumber() : 0; -} - bool QuicPacketCreator::ShouldSendFec(bool force_close) const { - DCHECK(!HasPendingFrames()); - return fec_group_.get() != nullptr && fec_group_->NumReceivedPackets() > 0 && + return !HasPendingFrames() && fec_group_.get() != nullptr && + fec_group_->NumReceivedPackets() > 0 && (force_close || fec_group_->NumReceivedPackets() >= max_packets_per_fec_group_); } @@ -156,7 +170,7 @@ bool QuicPacketCreator::IsFecGroupOpen() const { } void QuicPacketCreator::StartFecProtectingPackets() { - if (!IsFecEnabled()) { + if (max_packets_per_fec_group_ == 0) { LOG(DFATAL) << "Cannot start FEC protection when FEC is not enabled."; return; } @@ -169,8 +183,8 @@ void QuicPacketCreator::StartFecProtectingPackets() { LOG(DFATAL) << "Cannot start FEC protection with pending frames."; return; } - DCHECK(!should_fec_protect_); - should_fec_protect_ = true; + DCHECK(!fec_protect_); + fec_protect_ = true; } void QuicPacketCreator::StopFecProtectingPackets() { @@ -178,16 +192,8 @@ void QuicPacketCreator::StopFecProtectingPackets() { LOG(DFATAL) << "Cannot stop FEC protection with open FEC group."; return; } - DCHECK(should_fec_protect_); - should_fec_protect_ = false; -} - -bool QuicPacketCreator::IsFecProtected() const { - return should_fec_protect_; -} - -bool QuicPacketCreator::IsFecEnabled() const { - return max_packets_per_fec_group_ > 0; + DCHECK(fec_protect_); + fec_protect_ = false; } InFecGroup QuicPacketCreator::MaybeUpdateLengthsAndStartFec() { @@ -204,7 +210,7 @@ InFecGroup QuicPacketCreator::MaybeUpdateLengthsAndStartFec() { // Update packet number length only on packet and FEC group boundaries. packet_number_length_ = next_packet_number_length_; - if (!should_fec_protect_) { + if (!fec_protect_) { return NOT_IN_FEC_GROUP; } // Start a new FEC group since protection is on. Set the fec group number to @@ -267,9 +273,8 @@ bool QuicPacketCreator::HasRoomForStreamFrame(QuicStreamId id, // is_in_fec_group a parameter. Same as with all public methods in // QuicPacketCreator. return BytesFree() > - QuicFramer::GetMinStreamFrameSize(id, offset, true, - should_fec_protect_ ? IN_FEC_GROUP : - NOT_IN_FEC_GROUP); + QuicFramer::GetMinStreamFrameSize( + id, offset, true, fec_protect_ ? IN_FEC_GROUP : NOT_IN_FEC_GROUP); } // static @@ -280,7 +285,8 @@ size_t QuicPacketCreator::StreamFramePacketOverhead( QuicStreamOffset offset, InFecGroup is_in_fec_group) { return GetPacketHeaderSize(connection_id_length, include_version, - packet_number_length, is_in_fec_group) + + /*include_path_id=*/false, packet_number_length, + is_in_fec_group) + // Assumes this is a stream with a single lone packet. QuicFramer::GetMinStreamFrameSize(1u, offset, true, is_in_fec_group); } @@ -398,7 +404,7 @@ SerializedPacket QuicPacketCreator::ReserializeAllFrames( DCHECK(fec_group_.get() == nullptr); const QuicPacketNumberLength saved_length = packet_number_length_; const QuicPacketNumberLength saved_next_length = next_packet_number_length_; - const bool saved_should_fec_protect = should_fec_protect_; + const bool saved_should_fec_protect = fec_protect_; const bool needs_padding = needs_padding_; const EncryptionLevel default_encryption_level = encryption_level_; @@ -406,7 +412,7 @@ SerializedPacket QuicPacketCreator::ReserializeAllFrames( // and change the encryption level. packet_number_length_ = original_length; next_packet_number_length_ = original_length; - should_fec_protect_ = false; + fec_protect_ = false; encryption_level_ = frames.encryption_level(); needs_padding_ = frames.needs_padding(); @@ -415,7 +421,7 @@ SerializedPacket QuicPacketCreator::ReserializeAllFrames( SerializeAllFrames(frames.frames(), buffer, buffer_len); packet_number_length_ = saved_length; next_packet_number_length_ = saved_next_length; - should_fec_protect_ = saved_should_fec_protect; + fec_protect_ = saved_should_fec_protect; needs_padding_ = needs_padding; encryption_level_ = default_encryption_level; @@ -461,7 +467,7 @@ bool QuicPacketCreator::HasPendingRetransmittableFrames() const { size_t QuicPacketCreator::ExpansionOnNewFrame() const { // If packet is FEC protected, there's no expansion. - if (should_fec_protect_) { + if (fec_protect_) { return 0; } // If the last frame in the packet is a stream frame, then it will expand to @@ -486,8 +492,8 @@ size_t QuicPacketCreator::PacketSize() const { packet_number_length_ = next_packet_number_length_; } packet_size_ = GetPacketHeaderSize( - connection_id_length_, send_version_in_packet_, packet_number_length_, - should_fec_protect_ ? IN_FEC_GROUP : NOT_IN_FEC_GROUP); + connection_id_length_, send_version_in_packet_, /*include_path_id=*/false, + packet_number_length_, fec_protect_ ? IN_FEC_GROUP : NOT_IN_FEC_GROUP); return packet_size_; } @@ -522,7 +528,8 @@ SerializedPacket QuicPacketCreator::SerializePacket( } QuicPacketHeader header; // FillPacketHeader increments packet_number_. - FillPacketHeader(fec_group_number(), false, &header); + FillPacketHeader(fec_group_ != nullptr ? fec_group_->FecGroupNumber() : 0, + false, &header); MaybeAddPadding(); @@ -725,4 +732,61 @@ void QuicPacketCreator::MaybeAddPadding() { DCHECK(success); } +void QuicPacketCreator::MaybeStartFecProtection() { + if (max_packets_per_fec_group_ == 0 || fec_protect_) { + // Do not start FEC protection when FEC protection is not enabled or FEC + // protection is already on. + return; + } + DVLOG(1) << "Turning FEC protection ON"; + // Flush current open packet. + Flush(); + + StartFecProtectingPackets(); + DCHECK(fec_protect_); +} + +void QuicPacketCreator::MaybeSendFecPacketAndCloseGroup(bool force_send_fec, + bool is_fec_timeout) { + if (ShouldSendFec(force_send_fec)) { + if (fec_send_policy_ == FEC_ALARM_TRIGGER && !is_fec_timeout) { + ResetFecGroup(); + delegate_->OnResetFecGroup(); + } else { + // TODO(zhongyi): Change the default 64 alignas value (used the default + // value from CACHELINE_SIZE). + ALIGNAS(64) char seralized_fec_buffer[kMaxPacketSize]; + SerializedPacket serialized_fec = + SerializeFec(seralized_fec_buffer, kMaxPacketSize); + delegate_->OnSerializedPacket(&serialized_fec); + } + } + + if (!should_fec_protect_ && fec_protect_ && !IsFecGroupOpen()) { + StopFecProtectingPackets(); + } +} + +QuicTime::Delta QuicPacketCreator::GetFecTimeout( + QuicPacketNumber packet_number) { + // Do not set up FEC alarm for |packet_number| it is not the first packet in + // the current group. + if (fec_group_.get() != nullptr && + (packet_number == fec_group_->FecGroupNumber())) { + return QuicTime::Delta::Max( + fec_timeout_, QuicTime::Delta::FromMilliseconds(kMinFecTimeoutMs)); + } + return QuicTime::Delta::Infinite(); +} + +void QuicPacketCreator::OnCongestionWindowChange( + QuicPacketCount max_packets_in_flight) { + set_max_packets_per_fec_group(static_cast<size_t>( + kMaxPacketsInFlightMultiplierForFecGroupSize * max_packets_in_flight)); +} + +void QuicPacketCreator::OnRttChange(QuicTime::Delta rtt) { + fec_timeout_ = rtt.Multiply(rtt_multiplier_for_fec_timeout_); +} + } // namespace net diff --git a/net/quic/quic_packet_creator.h b/net/quic/quic_packet_creator.h index 5256fb9..7fb1d53 100644 --- a/net/quic/quic_packet_creator.h +++ b/net/quic/quic_packet_creator.h @@ -36,6 +36,8 @@ class NET_EXPORT_PRIVATE QuicPacketCreator { // Called when a packet is serialized. Delegate does not take the ownership // of |serialized_packet|. virtual void OnSerializedPacket(SerializedPacket* serialized_packet) = 0; + // Called when current FEC group is reset (closed). + virtual void OnResetFecGroup() = 0; }; // QuicRandom* required for packet entropy. @@ -46,28 +48,30 @@ class NET_EXPORT_PRIVATE QuicPacketCreator { ~QuicPacketCreator(); - // Turn on FEC protection for subsequently created packets. FEC should be - // enabled first (max_packets_per_fec_group should be non-zero) for FEC - // protection to start. - void StartFecProtectingPackets(); - - // Turn off FEC protection for subsequently created packets. If the creator - // has any open FEC group, call will fail. It is the caller's responsibility - // to flush out FEC packets in generation, and to verify with ShouldSendFec() - // that there is no open FEC group. - void StopFecProtectingPackets(); - // Checks if it's time to send an FEC packet. |force_close| forces this to // return true if an FEC group is open. bool ShouldSendFec(bool force_close) const; - // Resets (closes) the FEC group. This method should only be called on a - // packet boundary. - void ResetFecGroup(); + // Turn on FEC protection for subsequent packets. If no FEC group is currently + // open, this method flushes current open packet and then turns FEC on. + void MaybeStartFecProtection(); + + // If ShouldSendFec returns true, serializes currently constructed FEC packet + // and calls the delegate on the packet. Resets current FEC group if FEC + // protection policy is FEC_ALARM_TRIGGER but |is_fec_timeout| is false. + // Also tries to turn off FEC protection if should_fec_protect_ is false. + void MaybeSendFecPacketAndCloseGroup(bool force_send_fec, + bool is_fec_timeout); // Returns true if an FEC packet is under construction. bool IsFecGroupOpen() const; + // Called after sending |packet_number| to determine whether an FEC alarm + // should be set for sending out an FEC packet. Returns a positive and finite + // timeout if an FEC alarm should be set, and infinite if no alarm should be + // set. + QuicTime::Delta GetFecTimeout(QuicPacketNumber packet_number); + // Makes the framer not serialize the protocol version in sent packets. void StopSendingVersion(); @@ -128,17 +132,6 @@ class NET_EXPORT_PRIVATE QuicPacketCreator { // Returns true if there are retransmittable frames pending to be serialized. bool HasPendingRetransmittableFrames() const; - // TODO(jri): Remove this method. - // Returns whether FEC protection is currently enabled. Note: Enabled does not - // mean that an FEC group is currently active; i.e., IsFecProtected() may - // still return false. - bool IsFecEnabled() const; - - // Returns true if subsequent packets will be FEC protected. Note: True does - // not mean that an FEC packet is currently under construction; i.e., - // fec_group_.get() may still be nullptr, until MaybeStartFec() is called. - bool IsFecProtected() const; - // Returns the number of bytes which are available to be used by additional // frames in the packet. Since stream frames are slightly smaller when they // are the last frame in a packet, this method will return a different @@ -173,12 +166,6 @@ class NET_EXPORT_PRIVATE QuicPacketCreator { // to cause the packet to be padded. bool AddPaddedSavedFrame(const QuicFrame& frame, UniqueStreamBuffer buffer); - // Packetize FEC data. All frames must fit into a single packet. Also, sets - // the entropy hash of the serialized packet to a random bool and returns - // that value as a member of SerializedPacket. - // Fails if |buffer_len| isn't long enough for the encrypted packet. - SerializedPacket SerializeFec(char* buffer, size_t buffer_len); - // Creates a version negotiation packet which supports |supported_versions|. // Caller owns the created packet. Also, sets the entropy hash of the // serialized packet to a random bool and returns that value as a member of @@ -189,6 +176,12 @@ class NET_EXPORT_PRIVATE QuicPacketCreator { // Returns a dummy packet that is valid but contains no useful information. static SerializedPacket NoPacket(); + // Called when the congestion window has changed. + void OnCongestionWindowChange(QuicPacketCount max_packets_in_flight); + + // Called when the RTT may have changed. + void OnRttChange(QuicTime::Delta rtt); + // Sets the encryption level that will be applied to new packets. void set_encryption_level(EncryptionLevel level) { encryption_level_ = level; @@ -232,9 +225,22 @@ class NET_EXPORT_PRIVATE QuicPacketCreator { // To turn off FEC protection, use StopFecProtectingPackets(). void set_max_packets_per_fec_group(size_t max_packets_per_fec_group); - // Returns the current open FEC group's number. Returns 0 when FEC is - // disabled or no FEC group is open. - QuicFecGroupNumber fec_group_number(); + FecSendPolicy fec_send_policy() { return fec_send_policy_; } + + void set_fec_send_policy(FecSendPolicy fec_send_policy) { + fec_send_policy_ = fec_send_policy; + } + + void set_rtt_multiplier_for_fec_timeout( + float rtt_multiplier_for_fec_timeout) { + rtt_multiplier_for_fec_timeout_ = rtt_multiplier_for_fec_timeout; + } + + bool should_fec_protect() { return should_fec_protect_; } + + void set_should_fec_protect(bool should_fec_protect) { + should_fec_protect_ = should_fec_protect; + } private: friend class test::QuicPacketCreatorPeer; @@ -300,6 +306,27 @@ class NET_EXPORT_PRIVATE QuicPacketCreator { // Fails if |buffer_len| isn't long enough for the encrypted packet. SerializedPacket SerializePacket(char* encrypted_buffer, size_t buffer_len); + // Turn on FEC protection for subsequently created packets. FEC should be + // enabled first (max_packets_per_fec_group should be non-zero) for FEC + // protection to start. + void StartFecProtectingPackets(); + + // Turn off FEC protection for subsequently created packets. If the creator + // has any open FEC group, call will fail. It is the caller's responsibility + // to flush out FEC packets in generation, and to verify with ShouldSendFec() + // that there is no open FEC group. + void StopFecProtectingPackets(); + + // Resets (closes) the FEC group. This method should only be called on a + // packet boundary. + void ResetFecGroup(); + + // Packetize FEC data. All frames must fit into a single packet. Also, sets + // the entropy hash of the serialized packet to a random bool and returns + // that value as a member of SerializedPacket. + // Fails if |buffer_len| isn't long enough for the encrypted packet. + SerializedPacket SerializeFec(char* buffer, size_t buffer_len); + // Does not own this delegate. DelegateInterface* delegate_; QuicConnectionId connection_id_; @@ -307,8 +334,14 @@ class NET_EXPORT_PRIVATE QuicPacketCreator { QuicFramer* framer_; scoped_ptr<QuicRandomBoolSource> random_bool_source_; QuicPacketNumber packet_number_; - // If true, any created packets will be FEC protected. + // True when creator is requested to turn on FEC protection. False otherwise. + // There could be a time difference between should_fec_protect_ is true/false + // and FEC is actually turned on/off (e.g., The creator may have an open FEC + // group even if this variable is false). bool should_fec_protect_; + // If true, any created packets will be FEC protected. + // TODO(fayang): Combine should_fec_protect_ and fec_protect_ to one variable. + bool fec_protect_; scoped_ptr<QuicFecGroup> fec_group_; // Controls whether protocol version should be included while serializing the // packet. @@ -336,6 +369,14 @@ class NET_EXPORT_PRIVATE QuicPacketCreator { scoped_ptr<RetransmittableFrames> queued_retransmittable_frames_; // If true, the packet will be padded up to |max_packet_length_|. bool needs_padding_; + // FEC policy that specifies when to send FEC packet. + FecSendPolicy fec_send_policy_; + // Timeout used for FEC alarm. Can be set to zero initially or if the SRTT has + // not yet been set. + QuicTime::Delta fec_timeout_; + // The multiplication factor for FEC timeout based on RTT. + // TODO(rtenneti): Delete this code after the 0.25 RTT FEC experiment. + float rtt_multiplier_for_fec_timeout_; DISALLOW_COPY_AND_ASSIGN(QuicPacketCreator); }; diff --git a/net/quic/quic_packet_creator_test.cc b/net/quic/quic_packet_creator_test.cc index a39d832..5c65f2c 100644 --- a/net/quic/quic_packet_creator_test.cc +++ b/net/quic/quic_packet_creator_test.cc @@ -84,6 +84,7 @@ class MockDelegate : public QuicPacketCreator::DelegateInterface { ~MockDelegate() override {} MOCK_METHOD1(OnSerializedPacket, void(SerializedPacket* packet)); + MOCK_METHOD0(OnResetFecGroup, void()); private: DISALLOW_COPY_AND_ASSIGN(MockDelegate); @@ -152,7 +153,7 @@ class QuicPacketCreatorTest : public ::testing::TestWithParam<TestParams> { // the version. size_t GetPacketHeaderOverhead(InFecGroup is_in_fec_group) { return GetPacketHeaderSize( - creator_.connection_id_length(), kIncludeVersion, + creator_.connection_id_length(), kIncludeVersion, !kIncludePathId, QuicPacketCreatorPeer::NextPacketNumberLength(&creator_), is_in_fec_group); } @@ -174,8 +175,8 @@ class QuicPacketCreatorTest : public ::testing::TestWithParam<TestParams> { // Enables and turns on FEC protection. Returns true if FEC protection is on. bool SwitchFecProtectionOn(size_t max_packets_per_fec_group) { creator_.set_max_packets_per_fec_group(max_packets_per_fec_group); - creator_.StartFecProtectingPackets(); - return creator_.IsFecProtected(); + creator_.MaybeStartFecProtection(); + return QuicPacketCreatorPeer::IsFecProtected(&creator_); } QuicIOVector MakeIOVector(StringPiece s) { @@ -268,8 +269,12 @@ TEST_P(QuicPacketCreatorTest, SerializeWithFEC) { // Should return true since there are packets in the FEC group. ASSERT_TRUE(creator_.ShouldSendFec(/*force_close=*/true)); - serialized = creator_.SerializeFec(buffer, kMaxPacketSize); - ASSERT_EQ(2u, serialized.packet_number); + EXPECT_CALL(delegate_, OnSerializedPacket(_)) + .WillOnce(Invoke(this, &QuicPacketCreatorTest::SaveSerializedPacket)); + creator_.set_should_fec_protect(true); + creator_.MaybeSendFecPacketAndCloseGroup(/*force_send_fec=*/true, + /*is_fec_timeout=*/false); + ASSERT_EQ(2u, serialized_packet_.packet_number); { InSequence s; EXPECT_CALL(framer_visitor_, OnPacket()); @@ -280,8 +285,8 @@ TEST_P(QuicPacketCreatorTest, SerializeWithFEC) { EXPECT_CALL(framer_visitor_, OnFecData(_)); EXPECT_CALL(framer_visitor_, OnPacketComplete()); } - ProcessPacket(serialized.packet); - delete serialized.packet; + ProcessPacket(serialized_packet_.packet); + ClearSerializedPacket(&serialized_packet_); } TEST_P(QuicPacketCreatorTest, SerializeChangingSequenceNumberLength) { @@ -480,10 +485,14 @@ TEST_P(QuicPacketCreatorTest, SerializeWithFECChangingSequenceNumberLength) { ASSERT_TRUE(creator_.ShouldSendFec(/*force_close=*/true)); // Force generation of FEC packet. - char buffer[kMaxPacketSize]; - SerializedPacket serialized = creator_.SerializeFec(buffer, kMaxPacketSize); - EXPECT_EQ(PACKET_1BYTE_PACKET_NUMBER, serialized.packet_number_length); - ASSERT_EQ(3u, serialized.packet_number); + EXPECT_CALL(delegate_, OnSerializedPacket(_)) + .WillOnce(Invoke(this, &QuicPacketCreatorTest::SaveSerializedPacket)); + creator_.set_should_fec_protect(true); + creator_.MaybeSendFecPacketAndCloseGroup(/*force_send_fec=*/true, + /*is_fec_timeout=*/false); + EXPECT_EQ(PACKET_1BYTE_PACKET_NUMBER, + serialized_packet_.packet_number_length); + ASSERT_EQ(3u, serialized_packet_.packet_number); { InSequence s; @@ -495,11 +504,13 @@ TEST_P(QuicPacketCreatorTest, SerializeWithFECChangingSequenceNumberLength) { EXPECT_CALL(framer_visitor_, OnFecData(_)); EXPECT_CALL(framer_visitor_, OnPacketComplete()); } - ProcessPacket(serialized.packet); - ClearSerializedPacket(&serialized); + ProcessPacket(serialized_packet_.packet); + ClearSerializedPacket(&serialized_packet_); // Ensure the next FEC group starts using the new packet number length. - serialized = creator_.SerializeAllFrames(frames_, buffer, kMaxPacketSize); + char buffer[kMaxPacketSize]; + SerializedPacket serialized = + creator_.SerializeAllFrames(frames_, buffer, kMaxPacketSize); EXPECT_EQ(PACKET_4BYTE_PACKET_NUMBER, serialized.packet_number_length); delete frames_[0].ack_frame; ClearSerializedPacket(&serialized); @@ -624,18 +635,19 @@ TEST_P(QuicPacketCreatorTest, SerializeConnectionClose) { TEST_P(QuicPacketCreatorTest, SwitchFecOnOffWithNoGroup) { // Enable FEC protection. creator_.set_max_packets_per_fec_group(6); - EXPECT_TRUE(creator_.IsFecEnabled()); - EXPECT_FALSE(creator_.IsFecProtected()); + EXPECT_TRUE(QuicPacketCreatorPeer::IsFecEnabled(&creator_)); + EXPECT_FALSE(QuicPacketCreatorPeer::IsFecProtected(&creator_)); // Turn on FEC protection. - creator_.StartFecProtectingPackets(); - EXPECT_TRUE(creator_.IsFecProtected()); + creator_.MaybeStartFecProtection(); + EXPECT_TRUE(QuicPacketCreatorPeer::IsFecProtected(&creator_)); // We have no packets in the FEC group, so no FEC packet can be created. EXPECT_FALSE(creator_.ShouldSendFec(/*force_close=*/true)); // Since no packets are in FEC group yet, we should be able to turn FEC // off with no trouble. - creator_.StopFecProtectingPackets(); - EXPECT_FALSE(creator_.IsFecProtected()); + creator_.MaybeSendFecPacketAndCloseGroup(/*force_send_fec=*/true, + /*is_fec_timeout=*/false); + EXPECT_FALSE(QuicPacketCreatorPeer::IsFecProtected(&creator_)); } TEST_P(QuicPacketCreatorTest, SwitchFecOnOffWithGroupInProgress) { @@ -649,7 +661,7 @@ TEST_P(QuicPacketCreatorTest, SwitchFecOnOffWithGroupInProgress) { delete frames_[0].stream_frame; delete serialized.packet; - EXPECT_TRUE(creator_.IsFecProtected()); + EXPECT_TRUE(QuicPacketCreatorPeer::IsFecProtected(&creator_)); // We do not have enough packets in the FEC group to trigger an FEC packet. EXPECT_FALSE(creator_.ShouldSendFec(/*force_close=*/false)); // Should return true since there are packets in the FEC group. @@ -657,20 +669,20 @@ TEST_P(QuicPacketCreatorTest, SwitchFecOnOffWithGroupInProgress) { // Switching FEC off should not change creator state, since there is an // FEC packet under construction. - EXPECT_DFATAL(creator_.StopFecProtectingPackets(), + EXPECT_DFATAL(QuicPacketCreatorPeer::StopFecProtectingPackets(&creator_), "Cannot stop FEC protection with open FEC group."); - EXPECT_TRUE(creator_.IsFecProtected()); + EXPECT_TRUE(QuicPacketCreatorPeer::IsFecProtected(&creator_)); // Confirm that FEC packet is still under construction. EXPECT_TRUE(creator_.ShouldSendFec(/*force_close=*/true)); - serialized = creator_.SerializeFec(buffer, kMaxPacketSize); - delete serialized.packet; - + EXPECT_CALL(delegate_, OnSerializedPacket(_)) + .WillOnce(Invoke(this, &QuicPacketCreatorTest::ClearSerializedPacket)); // Switching FEC on/off should work now. - creator_.StopFecProtectingPackets(); - EXPECT_FALSE(creator_.IsFecProtected()); - creator_.StartFecProtectingPackets(); - EXPECT_TRUE(creator_.IsFecProtected()); + creator_.MaybeSendFecPacketAndCloseGroup(/*force_send_fec=*/true, + /*is_fec_timeout=*/false); + EXPECT_FALSE(QuicPacketCreatorPeer::IsFecProtected(&creator_)); + creator_.MaybeStartFecProtection(); + EXPECT_TRUE(QuicPacketCreatorPeer::IsFecProtected(&creator_)); } TEST_P(QuicPacketCreatorTest, SwitchFecOnWithStreamFrameQueued) { @@ -686,20 +698,17 @@ TEST_P(QuicPacketCreatorTest, SwitchFecOnWithStreamFrameQueued) { // Enable FEC protection, and send FEC packet every 6 packets. creator_.set_max_packets_per_fec_group(6); - EXPECT_TRUE(creator_.IsFecEnabled()); - EXPECT_DFATAL(creator_.StartFecProtectingPackets(), + EXPECT_TRUE(QuicPacketCreatorPeer::IsFecEnabled(&creator_)); + EXPECT_DFATAL(QuicPacketCreatorPeer::StartFecProtectingPackets(&creator_), "Cannot start FEC protection with pending frames."); - EXPECT_FALSE(creator_.IsFecProtected()); + EXPECT_FALSE(QuicPacketCreatorPeer::IsFecProtected(&creator_)); - // Serialize packet for transmission. + // Start FEC protection after current open packet is flushed. EXPECT_CALL(delegate_, OnSerializedPacket(_)) .WillOnce(Invoke(this, &QuicPacketCreatorTest::ClearSerializedPacket)); - creator_.Flush(); + creator_.MaybeStartFecProtection(); EXPECT_FALSE(creator_.HasPendingFrames()); - - // Since all pending frames have been serialized, turning FEC on should work. - creator_.StartFecProtectingPackets(); - EXPECT_TRUE(creator_.IsFecProtected()); + EXPECT_TRUE(QuicPacketCreatorPeer::IsFecProtected(&creator_)); } TEST_P(QuicPacketCreatorTest, ConsumeData) { @@ -1035,7 +1044,8 @@ TEST_P(QuicPacketCreatorTest, AddFrameAndFlush) { GetPacketHeaderSize( creator_.connection_id_length(), QuicPacketCreatorPeer::SendVersionInPacket(&creator_), - PACKET_1BYTE_PACKET_NUMBER, NOT_IN_FEC_GROUP), + /*include_path_id=*/false, PACKET_1BYTE_PACKET_NUMBER, + NOT_IN_FEC_GROUP), creator_.BytesFree()); // Add a variety of frame types and then a padding frame. @@ -1077,7 +1087,8 @@ TEST_P(QuicPacketCreatorTest, AddFrameAndFlush) { GetPacketHeaderSize( creator_.connection_id_length(), QuicPacketCreatorPeer::SendVersionInPacket(&creator_), - PACKET_1BYTE_PACKET_NUMBER, NOT_IN_FEC_GROUP), + /*include_path_id=*/false, PACKET_1BYTE_PACKET_NUMBER, + NOT_IN_FEC_GROUP), creator_.BytesFree()); } @@ -1200,23 +1211,29 @@ TEST_P(QuicPacketCreatorTest, ResetFecGroup) { creator_.SerializeAllFrames(frames_, buffer, kMaxPacketSize); delete serialized.packet; - EXPECT_TRUE(creator_.IsFecProtected()); + EXPECT_TRUE(QuicPacketCreatorPeer::IsFecProtected(&creator_)); EXPECT_TRUE(creator_.IsFecGroupOpen()); // We do not have enough packets in the FEC group to trigger an FEC packet. EXPECT_FALSE(creator_.ShouldSendFec(/*force_close=*/false)); // Should return true since there are packets in the FEC group. EXPECT_TRUE(creator_.ShouldSendFec(/*force_close=*/true)); - // Close the FEC Group. - creator_.ResetFecGroup(); - EXPECT_TRUE(creator_.IsFecProtected()); + // FEC group will be reset if FEC police is alarm trigger but FEC alarm does + // not fire. + EXPECT_CALL(delegate_, OnResetFecGroup()).Times(1); + creator_.set_fec_send_policy(FEC_ALARM_TRIGGER); + creator_.set_should_fec_protect(true); + creator_.MaybeSendFecPacketAndCloseGroup(/*force_send_fec=*/true, + /*is_fec_timeout=*/false); + EXPECT_TRUE(QuicPacketCreatorPeer::IsFecProtected(&creator_)); EXPECT_FALSE(creator_.IsFecGroupOpen()); // We do not have enough packets in the FEC group to trigger an FEC packet. EXPECT_FALSE(creator_.ShouldSendFec(/*force_close=*/false)); // Confirm that there is no FEC packet under construction. EXPECT_FALSE(creator_.ShouldSendFec(/*force_close=*/true)); - EXPECT_DFATAL(serialized = creator_.SerializeFec(buffer, kMaxPacketSize), + EXPECT_DFATAL(serialized = QuicPacketCreatorPeer::SerializeFec( + &creator_, buffer, kMaxPacketSize), "SerializeFEC called but no group or zero packets in group."); delete serialized.packet; @@ -1225,7 +1242,7 @@ TEST_P(QuicPacketCreatorTest, ResetFecGroup) { delete frames_[0].stream_frame; delete serialized.packet; - EXPECT_TRUE(creator_.IsFecProtected()); + EXPECT_TRUE(QuicPacketCreatorPeer::IsFecProtected(&creator_)); EXPECT_TRUE(creator_.IsFecGroupOpen()); // We do not have enough packets in the FEC group to trigger an FEC packet. EXPECT_FALSE(creator_.ShouldSendFec(/*force_close=*/false)); @@ -1238,12 +1255,19 @@ TEST_P(QuicPacketCreatorTest, ResetFecGroup) { // Should return true since there are packets in the FEC group. ASSERT_TRUE(creator_.ShouldSendFec(/*force_close=*/true)); - serialized = creator_.SerializeFec(buffer, kMaxPacketSize); - ASSERT_EQ(3u, serialized.packet_number); - delete serialized.packet; + // Change FEC policy, send FEC packet and close FEC group. + creator_.set_fec_send_policy(FEC_ANY_TRIGGER); + EXPECT_CALL(delegate_, OnSerializedPacket(_)) + .WillOnce(Invoke(this, &QuicPacketCreatorTest::SaveSerializedPacket)); + creator_.MaybeSendFecPacketAndCloseGroup(/*force_send_fec=*/true, + /*is_fec_timeout=*/false); + ASSERT_EQ(3u, serialized_packet_.packet_number); + ClearSerializedPacket(&serialized_packet_); } TEST_P(QuicPacketCreatorTest, ResetFecGroupWithQueuedFrames) { + // Enable FEC protection, and send FEC packet every 6 packets. + EXPECT_TRUE(SwitchFecProtectionOn(6)); // Add a stream frame to the creator. QuicFrame frame; QuicIOVector io_vector(MakeIOVector("test")); @@ -1253,7 +1277,7 @@ TEST_P(QuicPacketCreatorTest, ResetFecGroupWithQueuedFrames) { size_t consumed = frame.stream_frame->data.length(); EXPECT_EQ(4u, consumed); EXPECT_TRUE(creator_.HasPendingFrames()); - EXPECT_DFATAL(creator_.ResetFecGroup(), + EXPECT_DFATAL(QuicPacketCreatorPeer::ResetFecGroup(&creator_), "Cannot reset FEC group with pending frames."); EXPECT_CALL(delegate_, OnSerializedPacket(_)) @@ -1261,8 +1285,13 @@ TEST_P(QuicPacketCreatorTest, ResetFecGroupWithQueuedFrames) { creator_.Flush(); EXPECT_FALSE(creator_.HasPendingFrames()); - // Close the FEC Group. - creator_.ResetFecGroup(); + // FEC group will be reset if FEC police is alarm trigger but FEC alarm does + // not fire. + EXPECT_CALL(delegate_, OnResetFecGroup()).Times(1); + creator_.set_fec_send_policy(FEC_ALARM_TRIGGER); + creator_.set_should_fec_protect(true); + creator_.MaybeSendFecPacketAndCloseGroup(/*force_send_fec=*/true, + /*is_fec_timeout=*/false); EXPECT_FALSE(creator_.IsFecGroupOpen()); } diff --git a/net/quic/quic_packet_generator.cc b/net/quic/quic_packet_generator.cc index 37bb3fe..96caf7a 100644 --- a/net/quic/quic_packet_generator.cc +++ b/net/quic/quic_packet_generator.cc @@ -14,24 +14,6 @@ using base::StringPiece; namespace net { -namespace { - -// We want to put some space between a protected packet and the FEC packet to -// avoid losing them both within the same loss episode. On the other hand, we -// expect to be able to recover from any loss in about an RTT. We resolve this -// tradeoff by sending an FEC packet atmost half an RTT, or equivalently, half -// the max number of in-flight packets, the first protected packet. Since we -// don't want to delay an FEC packet past half an RTT, we set the max FEC group -// size to be half the current congestion window. -const float kMaxPacketsInFlightMultiplierForFecGroupSize = 0.5; -const float kRttMultiplierForFecTimeout = 0.5; - -// Minimum timeout for FEC alarm, set to half the minimum Tail Loss Probe -// timeout of 10ms. -const int64 kMinFecTimeoutMs = 5u; - -} // namespace - class QuicAckNotifier; QuicPacketGenerator::QuicPacketGenerator(QuicConnectionId connection_id, @@ -42,10 +24,6 @@ QuicPacketGenerator::QuicPacketGenerator(QuicConnectionId connection_id, debug_delegate_(nullptr), packet_creator_(connection_id, framer, random_generator, this), batch_mode_(false), - fec_timeout_(QuicTime::Delta::Zero()), - rtt_multiplier_for_fec_timeout_(kRttMultiplierForFecTimeout), - should_fec_protect_(false), - fec_send_policy_(FEC_ANY_TRIGGER), should_send_ack_(false), should_send_stop_waiting_(false), ack_queued_(false), @@ -91,13 +69,11 @@ QuicPacketGenerator::~QuicPacketGenerator() { void QuicPacketGenerator::OnCongestionWindowChange( QuicPacketCount max_packets_in_flight) { - packet_creator_.set_max_packets_per_fec_group( - static_cast<size_t>(kMaxPacketsInFlightMultiplierForFecGroupSize * - max_packets_in_flight)); + packet_creator_.OnCongestionWindowChange(max_packets_in_flight); } void QuicPacketGenerator::OnRttChange(QuicTime::Delta rtt) { - fec_timeout_ = rtt.Multiply(rtt_multiplier_for_fec_timeout_); + packet_creator_.OnRttChange(rtt); } void QuicPacketGenerator::SetShouldSendAck(bool also_send_stop_waiting) { @@ -143,7 +119,8 @@ QuicConsumedData QuicPacketGenerator::ConsumeData( } if (fec_protection == MUST_FEC_PROTECT) { - MaybeStartFecProtection(); + packet_creator_.set_should_fec_protect(true); + packet_creator_.MaybeStartFecProtection(); } if (!fin && (iov.total_length == 0)) { @@ -190,7 +167,7 @@ QuicConsumedData QuicPacketGenerator::ConsumeData( if (fec_protection == MUST_FEC_PROTECT) { // Turn off FEC protection when we're done writing protected data. DVLOG(1) << "Turning FEC protection OFF"; - should_fec_protect_ = false; + packet_creator_.set_should_fec_protect(false); } break; } @@ -203,7 +180,8 @@ QuicConsumedData QuicPacketGenerator::ConsumeData( // Try to close FEC group since we've either run out of data to send or we're // blocked. - MaybeSendFecPacketAndCloseGroup(/*force=*/false, /*is_fec_timeout=*/false); + packet_creator_.MaybeSendFecPacketAndCloseGroup(/*force_send_fec=*/false, + /*is_fec_timeout=*/false); DCHECK(InBatchMode() || !packet_creator_.HasPendingFrames()); return QuicConsumedData(total_bytes_consumed, fin_consumed); @@ -257,94 +235,23 @@ void QuicPacketGenerator::SendQueuedFrames(bool flush, bool is_fec_timeout) { if (flush || !InBatchMode()) { packet_creator_.Flush(); } - MaybeSendFecPacketAndCloseGroup(flush, is_fec_timeout); -} - -void QuicPacketGenerator::MaybeStartFecProtection() { - if (!packet_creator_.IsFecEnabled()) { - return; - } - DVLOG(1) << "Turning FEC protection ON"; - should_fec_protect_ = true; - if (packet_creator_.IsFecProtected()) { - // Only start creator's FEC protection if not already on. - return; - } - if (HasQueuedFrames()) { - // TODO(jri): This currently requires that the generator flush out any - // pending frames when FEC protection is turned on. If current packet can be - // converted to an FEC protected packet, do it. This will require the - // generator to check if the resulting expansion still allows the incoming - // frame to be added to the packet. - SendQueuedFrames(/*flush=*/true, /*is_fec_timeout=*/false); - } - packet_creator_.StartFecProtectingPackets(); - DCHECK(packet_creator_.IsFecProtected()); -} - -void QuicPacketGenerator::MaybeSendFecPacketAndCloseGroup(bool force, - bool is_fec_timeout) { - if (ShouldSendFecPacket(force)) { - // If we want to send FEC packet only when FEC alaram goes off and if it is - // not a FEC timeout then close the group and dont send FEC packet. - if (fec_send_policy_ == FEC_ALARM_TRIGGER && !is_fec_timeout) { - ResetFecGroup(); - } else { - // TODO(jri): SerializeFec can return a NULL packet, and this should cause - // an early return, with a call to delegate_->OnPacketGenerationError. - char buffer[kMaxPacketSize]; - SerializedPacket serialized_fec = - packet_creator_.SerializeFec(buffer, kMaxPacketSize); - DCHECK(serialized_fec.packet); - delegate_->OnSerializedPacket(serialized_fec); - } - } - - // Turn FEC protection off if creator's protection is on and the creator - // does not have an open FEC group. - // Note: We only wait until the frames queued in the creator are flushed; - // pending frames in the generator will not keep us from turning FEC off. - if (!should_fec_protect_ && packet_creator_.IsFecProtected() && - !packet_creator_.IsFecGroupOpen()) { - packet_creator_.StopFecProtectingPackets(); - DCHECK(!packet_creator_.IsFecProtected()); - } -} - -bool QuicPacketGenerator::ShouldSendFecPacket(bool force) { - return packet_creator_.IsFecProtected() && - !packet_creator_.HasPendingFrames() && - packet_creator_.ShouldSendFec(force); -} - -void QuicPacketGenerator::ResetFecGroup() { - DCHECK(packet_creator_.IsFecGroupOpen()); - packet_creator_.ResetFecGroup(); - delegate_->OnResetFecGroup(); + packet_creator_.MaybeSendFecPacketAndCloseGroup(flush, is_fec_timeout); } void QuicPacketGenerator::OnFecTimeout() { DCHECK(!InBatchMode()); - if (!ShouldSendFecPacket(true)) { + if (!packet_creator_.ShouldSendFec(true)) { LOG(DFATAL) << "No FEC packet to send on FEC timeout."; return; } // Flush out any pending frames in the generator and the creator, and then // send out FEC packet. SendQueuedFrames(/*flush=*/true, /*is_fec_timeout=*/true); - MaybeSendFecPacketAndCloseGroup(/*force=*/true, /*is_fec_timeout=*/true); } QuicTime::Delta QuicPacketGenerator::GetFecTimeout( QuicPacketNumber packet_number) { - // Do not set up FEC alarm for |packet_number| it is not the first packet in - // the current group. - if (packet_creator_.IsFecGroupOpen() && - (packet_number == packet_creator_.fec_group_number())) { - return QuicTime::Delta::Max( - fec_timeout_, QuicTime::Delta::FromMilliseconds(kMinFecTimeoutMs)); - } - return QuicTime::Delta::Infinite(); + return packet_creator_.GetFecTimeout(packet_number); } bool QuicPacketGenerator::InBatchMode() { @@ -442,7 +349,8 @@ void QuicPacketGenerator::SetMaxPacketLength(QuicByteCount length, bool force) { // FEC group. if (!packet_creator_.CanSetMaxPacketLength() && force) { SendQueuedFrames(/*flush=*/true, /*is_fec_timeout=*/false); - MaybeSendFecPacketAndCloseGroup(/*force=*/true, /*is_fec_timeout=*/false); + packet_creator_.MaybeSendFecPacketAndCloseGroup(/*force_send_fec=*/true, + /*is_fec_timeout=*/false); DCHECK(packet_creator_.CanSetMaxPacketLength()); } @@ -497,8 +405,9 @@ void QuicPacketGenerator::SetEncrypter(EncryptionLevel level, void QuicPacketGenerator::OnSerializedPacket( SerializedPacket* serialized_packet) { if (serialized_packet->packet == nullptr) { - LOG(DFATAL) << "Failed to SerializePacket. fec_policy:" << fec_send_policy_ - << " should_fec_protect_:" << should_fec_protect_; + LOG(DFATAL) << "Failed to SerializePacket. fec_policy:" << fec_send_policy() + << " should_fec_protect_:" + << packet_creator_.should_fec_protect(); delegate_->CloseConnection(QUIC_FAILED_TO_SERIALIZE_PACKET, false); return; } @@ -508,7 +417,8 @@ void QuicPacketGenerator::OnSerializedPacket( ack_listeners_.clear(); delegate_->OnSerializedPacket(*serialized_packet); - MaybeSendFecPacketAndCloseGroup(/*force=*/false, /*is_fec_timeout=*/false); + packet_creator_.MaybeSendFecPacketAndCloseGroup(/*force_send_fec=*/false, + /*is_fec_timeout=*/false); // Maximum packet size may be only enacted while no packet is currently being // constructed, so here we have a good opportunity to actually change it. @@ -521,4 +431,22 @@ void QuicPacketGenerator::OnSerializedPacket( stop_waiting_queued_ = false; } +void QuicPacketGenerator::OnResetFecGroup() { + delegate_->OnResetFecGroup(); +} + +void QuicPacketGenerator::set_rtt_multiplier_for_fec_timeout( + float rtt_multiplier_for_fec_timeout) { + packet_creator_.set_rtt_multiplier_for_fec_timeout( + rtt_multiplier_for_fec_timeout); +} + +FecSendPolicy QuicPacketGenerator::fec_send_policy() { + return packet_creator_.fec_send_policy(); +} + +void QuicPacketGenerator::set_fec_send_policy(FecSendPolicy fec_send_policy) { + packet_creator_.set_fec_send_policy(fec_send_policy); +} + } // namespace net diff --git a/net/quic/quic_packet_generator.h b/net/quic/quic_packet_generator.h index 194cb86..7b94a5b 100644 --- a/net/quic/quic_packet_generator.h +++ b/net/quic/quic_packet_generator.h @@ -186,6 +186,7 @@ class NET_EXPORT_PRIVATE QuicPacketGenerator // QuicPacketCreator::DelegateInterface. void OnSerializedPacket(SerializedPacket* serialized_packet) override; + void OnResetFecGroup() override; // Sets the encryption level that will be applied to new packets. void set_encryption_level(EncryptionLevel level); @@ -211,47 +212,14 @@ class NET_EXPORT_PRIVATE QuicPacketGenerator debug_delegate_ = debug_delegate; } - // TODO(rtenneti): Delete this code after the 0.25 RTT FEC experiment. - float rtt_multiplier_for_fec_timeout() { - return rtt_multiplier_for_fec_timeout_; - } - void set_rtt_multiplier_for_fec_timeout( - float rtt_multiplier_for_fec_timeout) { - rtt_multiplier_for_fec_timeout_ = rtt_multiplier_for_fec_timeout; - } + void set_rtt_multiplier_for_fec_timeout(float rtt_multiplier_for_fec_timeout); - FecSendPolicy fec_send_policy() { return fec_send_policy_; } - void set_fec_send_policy(FecSendPolicy fec_send_policy) { - fec_send_policy_ = fec_send_policy; - } + FecSendPolicy fec_send_policy(); + void set_fec_send_policy(FecSendPolicy fec_send_policy); private: friend class test::QuicPacketGeneratorPeer; - // Turn on FEC protection for subsequent packets in the generator. - // If no FEC group is currently open in the creator, this method flushes any - // queued frames in the generator and in the creator, and it then turns FEC on - // in the creator. This method may be called with an open FEC group in the - // creator, in which case, only the generator's state is altered. - void MaybeStartFecProtection(); - - // Serializes and calls the delegate on an FEC packet if one was under - // construction in the creator. When |force| is false, it relies on the - // creator being ready to send an FEC packet, otherwise FEC packet is sent - // as long as one is under construction in the creator. Also tries to turn - // off FEC protection in the creator if it's off in the generator. - // When |fec_send_policy_| is FEC_SEND_QUIESCENCE, then send FEC - // packet if |is_fec_timeout| is true otherwise close the FEC group. - void MaybeSendFecPacketAndCloseGroup(bool force, bool is_fec_timeout); - - // Returns true if an FEC packet should be generated based on |force| and - // current state of the generator and the creator. - bool ShouldSendFecPacket(bool force); - - // Resets (closes) the FEC group and calls the Delegate's OnResetFecGroup. - // Asserts that FEC group is open. - void ResetFecGroup(); - void SendQueuedFrames(bool flush, bool is_fec_timeout); // Test to see if we have pending ack, or control frames. @@ -278,21 +246,6 @@ class NET_EXPORT_PRIVATE QuicPacketGenerator // True if batch mode is currently enabled. bool batch_mode_; - // Timeout used for FEC alarm. Can be set to zero initially or if the SRTT has - // not yet been set. - QuicTime::Delta fec_timeout_; - - // The multiplication factor for FEC timeout based on RTT. - // TODO(rtenneti): Delete this code after the 0.25 RTT FEC experiment. - float rtt_multiplier_for_fec_timeout_; - - // True if FEC protection is on. The creator may have an open FEC group even - // if this variable is false. - bool should_fec_protect_; - - // FEC policy that specifies when to send FEC packet. - FecSendPolicy fec_send_policy_; - // Flags to indicate the need for just-in-time construction of a frame. bool should_send_ack_; bool should_send_stop_waiting_; diff --git a/net/quic/quic_packet_generator_test.cc b/net/quic/quic_packet_generator_test.cc index 2c9487a..0432619 100644 --- a/net/quic/quic_packet_generator_test.cc +++ b/net/quic/quic_packet_generator_test.cc @@ -499,7 +499,7 @@ TEST_P(QuicPacketGeneratorTest, ConsumeDataFecOnMaxGroupSize) { CheckPacketIsFec(2, 1); CheckPacketHasSingleStreamFrame(3); } - EXPECT_TRUE(creator_->IsFecProtected()); + EXPECT_TRUE(QuicPacketCreatorPeer::IsFecProtected(creator_)); // If FEC send policy is FEC_ANY_TRIGGER, then the FEC packet under // construction will be sent when one more packet is sent (since FEC group @@ -529,7 +529,7 @@ TEST_P(QuicPacketGeneratorTest, ConsumeDataFecOnMaxGroupSize) { CheckPacketHasSingleStreamFrame(4); CheckPacketIsFec(5, 4); } - EXPECT_FALSE(creator_->IsFecProtected()); + EXPECT_FALSE(QuicPacketCreatorPeer::IsFecProtected(creator_)); } TEST_P(QuicPacketGeneratorTest, ConsumeDataSendsFecOnTimeout) { @@ -545,7 +545,7 @@ TEST_P(QuicPacketGeneratorTest, ConsumeDataSendsFecOnTimeout) { EXPECT_EQ(1u, consumed.bytes_consumed); EXPECT_TRUE(consumed.fin_consumed); CheckPacketHasSingleStreamFrame(0); - EXPECT_TRUE(creator_->IsFecProtected()); + EXPECT_TRUE(QuicPacketCreatorPeer::IsFecProtected(creator_)); // Send more data with MAY_FEC_PROTECT. This packet should also be protected, // and FEC packet is not yet sent. @@ -555,14 +555,14 @@ TEST_P(QuicPacketGeneratorTest, ConsumeDataSendsFecOnTimeout) { nullptr); EXPECT_EQ(1u, consumed.bytes_consumed); CheckPacketHasSingleStreamFrame(1); - EXPECT_TRUE(creator_->IsFecProtected()); + EXPECT_TRUE(QuicPacketCreatorPeer::IsFecProtected(creator_)); // Calling OnFecTimeout should cause the FEC packet to be emitted. EXPECT_CALL(delegate_, OnSerializedPacket(_)) .WillOnce(Invoke(this, &QuicPacketGeneratorTest::SavePacket)); generator_.OnFecTimeout(); CheckPacketIsFec(2, 1); - EXPECT_FALSE(creator_->IsFecProtected()); + EXPECT_FALSE(QuicPacketCreatorPeer::IsFecProtected(creator_)); // Subsequent data is protected under the next FEC group. Send enough data to // create 2 more packets: one full and one partial. @@ -580,14 +580,14 @@ TEST_P(QuicPacketGeneratorTest, ConsumeDataSendsFecOnTimeout) { EXPECT_TRUE(consumed.fin_consumed); CheckPacketHasSingleStreamFrame(3); CheckPacketHasSingleStreamFrame(4); - EXPECT_TRUE(creator_->IsFecProtected()); + EXPECT_TRUE(QuicPacketCreatorPeer::IsFecProtected(creator_)); // Calling OnFecTimeout should cause the FEC packet to be emitted. EXPECT_CALL(delegate_, OnSerializedPacket(_)) .WillOnce(Invoke(this, &QuicPacketGeneratorTest::SavePacket)); generator_.OnFecTimeout(); CheckPacketIsFec(5, 4); - EXPECT_FALSE(creator_->IsFecProtected()); + EXPECT_FALSE(QuicPacketCreatorPeer::IsFecProtected(creator_)); } TEST_P(QuicPacketGeneratorTest, GetFecTimeoutFiniteOnlyOnFirstPacketInGroup) { @@ -612,7 +612,7 @@ TEST_P(QuicPacketGeneratorTest, GetFecTimeoutFiniteOnlyOnFirstPacketInGroup) { EXPECT_FALSE(generator_.HasQueuedFrames()); CheckPacketHasSingleStreamFrame(0); CheckPacketHasSingleStreamFrame(1); - EXPECT_TRUE(creator_->IsFecProtected()); + EXPECT_TRUE(QuicPacketCreatorPeer::IsFecProtected(creator_)); // GetFecTimeout returns finite timeout only for first packet in group. EXPECT_EQ(QuicTime::Delta::FromMilliseconds(kMinFecTimeoutMs), @@ -627,7 +627,7 @@ TEST_P(QuicPacketGeneratorTest, GetFecTimeoutFiniteOnlyOnFirstPacketInGroup) { consumed = generator_.ConsumeData(5, CreateData(1u), 0, true, MAY_FEC_PROTECT, nullptr); CheckPacketHasSingleStreamFrame(2); - EXPECT_TRUE(creator_->IsFecProtected()); + EXPECT_TRUE(QuicPacketCreatorPeer::IsFecProtected(creator_)); // GetFecTimeout returns finite timeout only for first packet in group. EXPECT_EQ(QuicTime::Delta::Infinite(), @@ -638,7 +638,7 @@ TEST_P(QuicPacketGeneratorTest, GetFecTimeoutFiniteOnlyOnFirstPacketInGroup) { .WillOnce(Invoke(this, &QuicPacketGeneratorTest::SavePacket)); generator_.OnFecTimeout(); CheckPacketIsFec(3, /*fec_group=*/1u); - EXPECT_FALSE(creator_->IsFecProtected()); + EXPECT_FALSE(QuicPacketCreatorPeer::IsFecProtected(creator_)); // Subsequent data is protected under the next FEC group. Send enough data to // create 2 more packets: one full and one partial. @@ -656,7 +656,7 @@ TEST_P(QuicPacketGeneratorTest, GetFecTimeoutFiniteOnlyOnFirstPacketInGroup) { EXPECT_TRUE(consumed.fin_consumed); CheckPacketHasSingleStreamFrame(4); CheckPacketHasSingleStreamFrame(5); - EXPECT_TRUE(creator_->IsFecProtected()); + EXPECT_TRUE(QuicPacketCreatorPeer::IsFecProtected(creator_)); // GetFecTimeout returns finite timeout for first packet in the new group. EXPECT_EQ(QuicTime::Delta::FromMilliseconds(kMinFecTimeoutMs), @@ -669,7 +669,7 @@ TEST_P(QuicPacketGeneratorTest, GetFecTimeoutFiniteOnlyOnFirstPacketInGroup) { .WillOnce(Invoke(this, &QuicPacketGeneratorTest::SavePacket)); generator_.OnFecTimeout(); CheckPacketIsFec(6, /*fec_group=*/5u); - EXPECT_FALSE(creator_->IsFecProtected()); + EXPECT_FALSE(QuicPacketCreatorPeer::IsFecProtected(creator_)); // Send more data with MAY_FEC_PROTECT. No FEC protection, so GetFecTimeout // returns infinite. @@ -678,7 +678,7 @@ TEST_P(QuicPacketGeneratorTest, GetFecTimeoutFiniteOnlyOnFirstPacketInGroup) { consumed = generator_.ConsumeData(9, CreateData(1u), 0, true, MAY_FEC_PROTECT, nullptr); CheckPacketHasSingleStreamFrame(7); - EXPECT_FALSE(creator_->IsFecProtected()); + EXPECT_FALSE(QuicPacketCreatorPeer::IsFecProtected(creator_)); EXPECT_EQ(QuicTime::Delta::Infinite(), generator_.GetFecTimeout(/*packet_number=*/8u)); } @@ -689,7 +689,7 @@ TEST_P(QuicPacketGeneratorTest, ConsumeData_FramesPreviouslyQueued) { size_t length = NullEncrypter().GetCiphertextSize(0) + GetPacketHeaderSize( - creator_->connection_id_length(), true, + creator_->connection_id_length(), true, /*include_path_id=*/false, QuicPacketCreatorPeer::NextPacketNumberLength(creator_), NOT_IN_FEC_GROUP) + // Add an extra 3 bytes for the payload and 1 byte so BytesFree is larger @@ -766,10 +766,10 @@ TEST_P(QuicPacketGeneratorTest, NoFecPacketSentWhenBatchEnds) { TEST_P(QuicPacketGeneratorTest, FecTimeoutOnRttChange) { EXPECT_EQ(QuicTime::Delta::Zero(), - QuicPacketGeneratorPeer::GetFecTimeout(&generator_)); + QuicPacketCreatorPeer::GetFecTimeout(creator_)); generator_.OnRttChange(QuicTime::Delta::FromMilliseconds(300)); EXPECT_EQ(QuicTime::Delta::FromMilliseconds(150), - QuicPacketGeneratorPeer::GetFecTimeout(&generator_)); + QuicPacketCreatorPeer::GetFecTimeout(creator_)); } TEST_P(QuicPacketGeneratorTest, FecGroupSizeOnCongestionWindowChange) { @@ -843,13 +843,13 @@ TEST_P(QuicPacketGeneratorTest, FecGroupSizeChangeWithOpenGroup) { CheckPacketIsFec(4, /*fec_group=*/1u); } EXPECT_FALSE(creator_->IsFecGroupOpen()); - EXPECT_FALSE(creator_->IsFecProtected()); + EXPECT_FALSE(QuicPacketCreatorPeer::IsFecProtected(creator_)); } TEST_P(QuicPacketGeneratorTest, SwitchFecOnOff) { delegate_.SetCanWriteAnything(); creator_->set_max_packets_per_fec_group(2); - EXPECT_FALSE(creator_->IsFecProtected()); + EXPECT_FALSE(QuicPacketCreatorPeer::IsFecProtected(creator_)); // Send one unprotected data packet. EXPECT_CALL(delegate_, OnSerializedPacket(_)) @@ -858,7 +858,7 @@ TEST_P(QuicPacketGeneratorTest, SwitchFecOnOff) { MAY_FEC_PROTECT, nullptr); EXPECT_EQ(1u, consumed.bytes_consumed); EXPECT_FALSE(generator_.HasQueuedFrames()); - EXPECT_FALSE(creator_->IsFecProtected()); + EXPECT_FALSE(QuicPacketCreatorPeer::IsFecProtected(creator_)); // Verify that one data packet was sent. PacketContents contents; contents.num_stream_frames = 1; @@ -918,7 +918,7 @@ TEST_P(QuicPacketGeneratorTest, SwitchFecOnOff) { nullptr); EXPECT_EQ(1u, consumed.bytes_consumed); EXPECT_FALSE(generator_.HasQueuedFrames()); - EXPECT_FALSE(creator_->IsFecProtected()); + EXPECT_FALSE(QuicPacketCreatorPeer::IsFecProtected(creator_)); // Verify that one unprotected data packet was sent. if (generator_.fec_send_policy() == FEC_ALARM_TRIGGER) { CheckPacketContains(contents, 5); @@ -943,7 +943,7 @@ TEST_P(QuicPacketGeneratorTest, SwitchFecOnWithPendingFrameInCreator) { // Queue protected data for sending. Should cause queued frames to be flushed. EXPECT_CALL(delegate_, OnSerializedPacket(_)) .WillOnce(Invoke(this, &QuicPacketGeneratorTest::SavePacket)); - EXPECT_FALSE(creator_->IsFecProtected()); + EXPECT_FALSE(QuicPacketCreatorPeer::IsFecProtected(creator_)); consumed = generator_.ConsumeData(7, CreateData(1u), 0, true, MUST_FEC_PROTECT, nullptr); EXPECT_EQ(1u, consumed.bytes_consumed); @@ -951,7 +951,7 @@ TEST_P(QuicPacketGeneratorTest, SwitchFecOnWithPendingFrameInCreator) { contents.num_stream_frames = 1; // Transmitted packet was not FEC protected. CheckPacketContains(contents, 0); - EXPECT_TRUE(creator_->IsFecProtected()); + EXPECT_TRUE(QuicPacketCreatorPeer::IsFecProtected(creator_)); EXPECT_TRUE(creator_->HasPendingFrames()); } @@ -972,7 +972,7 @@ TEST_P(QuicPacketGeneratorTest, SwitchFecOnWithPendingFramesInGenerator) { // Generator should have queued control frames, and creator should be empty. EXPECT_TRUE(generator_.HasQueuedFrames()); EXPECT_FALSE(creator_->HasPendingFrames()); - EXPECT_FALSE(creator_->IsFecProtected()); + EXPECT_FALSE(QuicPacketCreatorPeer::IsFecProtected(creator_)); // Queue protected data for sending. Should cause queued frames to be flushed. EXPECT_CALL(delegate_, OnSerializedPacket(_)) @@ -986,7 +986,7 @@ TEST_P(QuicPacketGeneratorTest, SwitchFecOnWithPendingFramesInGenerator) { CheckPacketContains(contents, 0); // FEC protection should be on in creator. - EXPECT_TRUE(creator_->IsFecProtected()); + EXPECT_TRUE(QuicPacketCreatorPeer::IsFecProtected(creator_)); } TEST_P(QuicPacketGeneratorTest, SwitchFecOnOffWithSubsequentFramesProtected) { @@ -994,7 +994,7 @@ TEST_P(QuicPacketGeneratorTest, SwitchFecOnOffWithSubsequentFramesProtected) { // Enable FEC. creator_->set_max_packets_per_fec_group(2); - EXPECT_FALSE(creator_->IsFecProtected()); + EXPECT_FALSE(QuicPacketCreatorPeer::IsFecProtected(creator_)); // Queue stream frame to be protected in creator. generator_.StartBatchOperations(); @@ -1003,7 +1003,7 @@ TEST_P(QuicPacketGeneratorTest, SwitchFecOnOffWithSubsequentFramesProtected) { EXPECT_EQ(1u, consumed.bytes_consumed); // Creator has a pending protected frame. EXPECT_TRUE(creator_->HasPendingFrames()); - EXPECT_TRUE(creator_->IsFecProtected()); + EXPECT_TRUE(QuicPacketCreatorPeer::IsFecProtected(creator_)); // Add enough unprotected data to exceed size of current packet, so that // current packet is sent. Both frames will be sent out in a single packet. @@ -1018,7 +1018,7 @@ TEST_P(QuicPacketGeneratorTest, SwitchFecOnOffWithSubsequentFramesProtected) { contents.fec_group = 1u; CheckPacketContains(contents, 0); // FEC protection should still be on in creator. - EXPECT_TRUE(creator_->IsFecProtected()); + EXPECT_TRUE(QuicPacketCreatorPeer::IsFecProtected(creator_)); } TEST_P(QuicPacketGeneratorTest, SwitchFecOnOffWithSubsequentPacketsProtected) { @@ -1026,7 +1026,7 @@ TEST_P(QuicPacketGeneratorTest, SwitchFecOnOffWithSubsequentPacketsProtected) { // Enable FEC. creator_->set_max_packets_per_fec_group(2); - EXPECT_FALSE(creator_->IsFecProtected()); + EXPECT_FALSE(QuicPacketCreatorPeer::IsFecProtected(creator_)); // Send first packet, FEC protected. EXPECT_CALL(delegate_, OnSerializedPacket(_)) @@ -1040,7 +1040,7 @@ TEST_P(QuicPacketGeneratorTest, SwitchFecOnOffWithSubsequentPacketsProtected) { CheckPacketContains(contents, 0); // FEC should still be on in creator. - EXPECT_TRUE(creator_->IsFecProtected()); + EXPECT_TRUE(QuicPacketCreatorPeer::IsFecProtected(creator_)); // Send unprotected data to cause second packet to be sent, which gets // protected because it happens to fall within an open FEC group. Data packet @@ -1067,7 +1067,7 @@ TEST_P(QuicPacketGeneratorTest, SwitchFecOnOffWithSubsequentPacketsProtected) { } // FEC protection should be off in creator. - EXPECT_FALSE(creator_->IsFecProtected()); + EXPECT_FALSE(QuicPacketCreatorPeer::IsFecProtected(creator_)); } TEST_P(QuicPacketGeneratorTest, SwitchFecOnOffThenOnWithCreatorProtectionOn) { @@ -1076,7 +1076,7 @@ TEST_P(QuicPacketGeneratorTest, SwitchFecOnOffThenOnWithCreatorProtectionOn) { // Enable FEC. creator_->set_max_packets_per_fec_group(2); - EXPECT_FALSE(creator_->IsFecProtected()); + EXPECT_FALSE(QuicPacketCreatorPeer::IsFecProtected(creator_)); // Queue one byte of FEC protected data. QuicConsumedData consumed = generator_.ConsumeData(5, CreateData(1u), 0, true, @@ -1096,7 +1096,7 @@ TEST_P(QuicPacketGeneratorTest, SwitchFecOnOffThenOnWithCreatorProtectionOn) { CheckPacketContains(contents, 0); // FEC group is still open in creator. - EXPECT_TRUE(creator_->IsFecProtected()); + EXPECT_TRUE(QuicPacketCreatorPeer::IsFecProtected(creator_)); // Add data that should be protected, large enough to cause second packet to // be sent. Data packet should be followed by FEC packet. @@ -1121,14 +1121,14 @@ TEST_P(QuicPacketGeneratorTest, SwitchFecOnOffThenOnWithCreatorProtectionOn) { } // FEC protection should remain on in creator. - EXPECT_TRUE(creator_->IsFecProtected()); + EXPECT_TRUE(QuicPacketCreatorPeer::IsFecProtected(creator_)); } TEST_P(QuicPacketGeneratorTest, ResetFecGroupNoTimeout) { delegate_.SetCanWriteAnything(); // Send FEC packet after 2 packets. creator_->set_max_packets_per_fec_group(2); - EXPECT_FALSE(creator_->IsFecProtected()); + EXPECT_FALSE(QuicPacketCreatorPeer::IsFecProtected(creator_)); // Send two packets so that when this data is consumed, two packets are sent // out. In FEC_TRIGGER_ANY, this will cause an FEC packet to be sent out and @@ -1169,7 +1169,7 @@ TEST_P(QuicPacketGeneratorTest, ResetFecGroupNoTimeout) { CheckPacketIsFec(2, 1); CheckPacketHasSingleStreamFrame(3); } - EXPECT_TRUE(creator_->IsFecProtected()); + EXPECT_TRUE(QuicPacketCreatorPeer::IsFecProtected(creator_)); // Do the same send (with MUST_FEC_PROTECT) on a different stream id. { @@ -1219,7 +1219,7 @@ TEST_P(QuicPacketGeneratorTest, ResetFecGroupNoTimeout) { // FEC_ANY_TRIGGER. CheckPacketIsFec(8, 7); } - EXPECT_FALSE(creator_->IsFecProtected()); + EXPECT_FALSE(QuicPacketCreatorPeer::IsFecProtected(creator_)); // Do the another send (with MAY_FEC_PROTECT) on a different stream id, which // should not produce an FEC packet because the last FEC group has been @@ -1247,7 +1247,7 @@ TEST_P(QuicPacketGeneratorTest, ResetFecGroupNoTimeout) { CheckPacketHasSingleStreamFrame(10); CheckPacketHasSingleStreamFrame(11); } - EXPECT_FALSE(creator_->IsFecProtected()); + EXPECT_FALSE(QuicPacketCreatorPeer::IsFecProtected(creator_)); } // 1. Create and send one packet with MUST_FEC_PROTECT. @@ -1256,7 +1256,7 @@ TEST_P(QuicPacketGeneratorTest, ResetFecGroupNoTimeout) { TEST_P(QuicPacketGeneratorTest, FecPacketSentOnFecTimeout) { delegate_.SetCanWriteAnything(); creator_->set_max_packets_per_fec_group(1000); - EXPECT_FALSE(creator_->IsFecProtected()); + EXPECT_FALSE(QuicPacketCreatorPeer::IsFecProtected(creator_)); for (int i = 1; i < 4; i = i + 2) { // Send data with MUST_FEC_PROTECT flag. No FEC packet is emitted, but the @@ -1268,14 +1268,14 @@ TEST_P(QuicPacketGeneratorTest, FecPacketSentOnFecTimeout) { EXPECT_EQ(1u, consumed.bytes_consumed); EXPECT_TRUE(consumed.fin_consumed); CheckPacketHasSingleStreamFrame(0); - EXPECT_TRUE(creator_->IsFecProtected()); + EXPECT_TRUE(QuicPacketCreatorPeer::IsFecProtected(creator_)); // Calling OnFecTimeout should cause the FEC packet to be emitted. EXPECT_CALL(delegate_, OnSerializedPacket(_)) .WillOnce(Invoke(this, &QuicPacketGeneratorTest::SavePacket)); generator_.OnFecTimeout(); CheckPacketIsFec(i, i); - EXPECT_FALSE(creator_->IsFecProtected()); + EXPECT_FALSE(QuicPacketCreatorPeer::IsFecProtected(creator_)); } } diff --git a/net/quic/quic_protocol.cc b/net/quic/quic_protocol.cc index a9ffd17..d99018f 100644 --- a/net/quic/quic_protocol.cc +++ b/net/quic/quic_protocol.cc @@ -17,18 +17,20 @@ using std::string; namespace net { size_t GetPacketHeaderSize(const QuicPacketHeader& header) { - return GetPacketHeaderSize(header.public_header.connection_id_length, - header.public_header.version_flag, - header.public_header.packet_number_length, - header.is_in_fec_group); + return GetPacketHeaderSize( + header.public_header.connection_id_length, + header.public_header.version_flag, header.public_header.multipath_flag, + header.public_header.packet_number_length, header.is_in_fec_group); } size_t GetPacketHeaderSize(QuicConnectionIdLength connection_id_length, bool include_version, + bool include_path_id, QuicPacketNumberLength packet_number_length, InFecGroup is_in_fec_group) { return kPublicFlagsSize + connection_id_length + - (include_version ? kQuicVersionSize : 0) + packet_number_length + + (include_version ? kQuicVersionSize : 0) + + (include_path_id ? kQuicPathIdSize : 0) + packet_number_length + kPrivateFlagsSize + (is_in_fec_group == IN_FEC_GROUP ? kFecGroupSize : 0); } @@ -37,7 +39,8 @@ size_t GetStartOfFecProtectedData(QuicConnectionIdLength connection_id_length, bool include_version, QuicPacketNumberLength packet_number_length) { return GetPacketHeaderSize(connection_id_length, include_version, - packet_number_length, IN_FEC_GROUP); + /*include_path_id=*/false, packet_number_length, + IN_FEC_GROUP); } size_t GetStartOfEncryptedData(QuicConnectionIdLength connection_id_length, @@ -45,13 +48,15 @@ size_t GetStartOfEncryptedData(QuicConnectionIdLength connection_id_length, QuicPacketNumberLength packet_number_length) { // Don't include the fec size, since encryption starts before private flags. return GetPacketHeaderSize(connection_id_length, include_version, - packet_number_length, NOT_IN_FEC_GROUP) - + /*include_path_id=*/false, packet_number_length, + NOT_IN_FEC_GROUP) - kPrivateFlagsSize; } QuicPacketPublicHeader::QuicPacketPublicHeader() : connection_id(0), connection_id_length(PACKET_8BYTE_CONNECTION_ID), + multipath_flag(false), reset_flag(false), version_flag(false), packet_number_length(PACKET_6BYTE_PACKET_NUMBER) {} @@ -60,6 +65,7 @@ QuicPacketPublicHeader::QuicPacketPublicHeader( const QuicPacketPublicHeader& other) : connection_id(other.connection_id), connection_id_length(other.connection_id_length), + multipath_flag(other.multipath_flag), reset_flag(other.reset_flag), version_flag(other.version_flag), packet_number_length(other.packet_number_length), @@ -68,7 +74,8 @@ QuicPacketPublicHeader::QuicPacketPublicHeader( QuicPacketPublicHeader::~QuicPacketPublicHeader() {} QuicPacketHeader::QuicPacketHeader() - : packet_number(0), + : path_id(0), + packet_number(0), fec_flag(false), entropy_flag(false), entropy_hash(0), @@ -77,6 +84,7 @@ QuicPacketHeader::QuicPacketHeader() QuicPacketHeader::QuicPacketHeader(const QuicPacketPublicHeader& header) : public_header(header), + path_id(0), packet_number(0), fec_flag(false), entropy_flag(false), @@ -207,6 +215,7 @@ ostream& operator<<(ostream& os, const QuicPacketHeader& header) { os << "{ connection_id: " << header.public_header.connection_id << ", connection_id_length:" << header.public_header.connection_id_length << ", packet_number_length:" << header.public_header.packet_number_length + << ", multipath_flag: " << header.public_header.multipath_flag << ", reset_flag: " << header.public_header.reset_flag << ", version_flag: " << header.public_header.version_flag; if (header.public_header.version_flag) { @@ -218,6 +227,7 @@ ostream& operator<<(ostream& os, const QuicPacketHeader& header) { os << ", fec_flag: " << header.fec_flag << ", entropy_flag: " << header.entropy_flag << ", entropy hash: " << static_cast<int>(header.entropy_hash) + << ", path_id: " << header.path_id << ", packet_number: " << header.packet_number << ", is_in_fec_group:" << header.is_in_fec_group << ", fec_group: " << header.fec_group << "}\n"; @@ -679,6 +689,9 @@ RetransmittableFrames::RetransmittableFrames(EncryptionLevel level) : encryption_level_(level), has_crypto_handshake_(NOT_HANDSHAKE), needs_padding_(false) { + // TODO(ianswett): Consider using an inlined vector instead, since this + // is very frequently a single frame. + frames_.reserve(2); } RetransmittableFrames::~RetransmittableFrames() { diff --git a/net/quic/quic_protocol.h b/net/quic/quic_protocol.h index 24b4e46..100a663 100644 --- a/net/quic/quic_protocol.h +++ b/net/quic/quic_protocol.h @@ -111,6 +111,8 @@ const size_t kDefaultMaxStreamsPerConnection = 100; const size_t kPublicFlagsSize = 1; // Number of bytes reserved for version number in the packet header. const size_t kQuicVersionSize = 4; +// Number of bytes reserved for path id in the packet header. +const size_t kQuicPathIdSize = 1; // Number of bytes reserved for private flags in the packet header. const size_t kPrivateFlagsSize = 1; // Number of bytes reserved for FEC group in the packet header. @@ -118,6 +120,8 @@ const size_t kFecGroupSize = 1; // Signifies that the QuicPacket will contain version of the protocol. const bool kIncludeVersion = true; +// Signifies that the QuicPacket will contain path id. +const bool kIncludePathId = true; // Index of the first byte in a QUIC packet which is used in hash calculation. const size_t kStartOfHashData = 0; @@ -312,8 +316,11 @@ enum QuicPacketPublicFlags { PACKET_PUBLIC_FLAGS_4BYTE_PACKET = PACKET_FLAGS_4BYTE_PACKET << 4, PACKET_PUBLIC_FLAGS_6BYTE_PACKET = PACKET_FLAGS_6BYTE_PACKET << 4, - // All bits set (bits 6 and 7 are not currently used): 00111111 - PACKET_PUBLIC_FLAGS_MAX = (1 << 6) - 1 + // Bit 6: Does the packet header contain a path id? + PACKET_PUBLIC_FLAGS_MULTIPATH = 1 << 6, + + // All bits set (bit7 are not currently used): 01111111 + PACKET_PUBLIC_FLAGS_MAX = (1 << 7) - 1, }; // The private flags are specified in one byte. @@ -408,6 +415,7 @@ NET_EXPORT_PRIVATE size_t GetPacketHeaderSize(const QuicPacketHeader& header); NET_EXPORT_PRIVATE size_t GetPacketHeaderSize(QuicConnectionIdLength connection_id_length, bool include_version, + bool include_path_id, QuicPacketNumberLength packet_number_length, InFecGroup is_in_fec_group); @@ -635,6 +643,7 @@ struct NET_EXPORT_PRIVATE QuicPacketPublicHeader { // public flags. QuicConnectionId connection_id; QuicConnectionIdLength connection_id_length; + bool multipath_flag; bool reset_flag; bool version_flag; QuicPacketNumberLength packet_number_length; @@ -653,6 +662,7 @@ struct NET_EXPORT_PRIVATE QuicPacketHeader { std::ostream& os, const QuicPacketHeader& s); QuicPacketPublicHeader public_header; + QuicPathId path_id; QuicPacketNumber packet_number; bool fec_flag; bool entropy_flag; diff --git a/net/quic/quic_reliable_client_stream.cc b/net/quic/quic_reliable_client_stream.cc index 232962d..93bcbb3 100644 --- a/net/quic/quic_reliable_client_stream.cc +++ b/net/quic/quic_reliable_client_stream.cc @@ -74,7 +74,7 @@ SpdyPriority QuicReliableClientStream::Priority() const { if (delegate_ && delegate_->HasSendHeadersComplete()) { return QuicSpdyStream::Priority(); } - return net::kHighestPriority; + return net::kV3HighestPriority; } int QuicReliableClientStream::WriteStreamData( diff --git a/net/quic/quic_session.cc b/net/quic/quic_session.cc index 0ef3169..fc8e42f 100644 --- a/net/quic/quic_session.cc +++ b/net/quic/quic_session.cc @@ -612,43 +612,21 @@ ReliableQuicStream* QuicSession::GetOrCreateDynamicStream( available_streams_.erase(stream_id); if (stream_id > largest_peer_created_stream_id_) { - if (FLAGS_allow_many_available_streams) { - // Check if the new number of available streams would cause the number of - // available streams to exceed the limit. Note that the peer can create - // only alternately-numbered streams. - size_t additional_available_streams = - (stream_id - largest_peer_created_stream_id_) / 2 - 1; - size_t new_num_available_streams = - GetNumAvailableStreams() + additional_available_streams; - if (new_num_available_streams > get_max_available_streams()) { - DVLOG(1) << "Failed to create a new incoming stream with id:" - << stream_id << ". There are already " - << GetNumAvailableStreams() - << " streams available, which would become " - << new_num_available_streams << ", which exceeds the limit " - << get_max_available_streams() << "."; - CloseConnection(QUIC_TOO_MANY_AVAILABLE_STREAMS); - return nullptr; - } - } else { - // Check if the number of streams that will be created (including - // available streams) would cause the number of open streams to exceed the - // limit. Note that the peer can create only alternately-numbered - // streams. - if ((stream_id - largest_peer_created_stream_id_) / 2 + - GetNumOpenStreams() > - get_max_open_streams()) { - DVLOG(1) << "Failed to create a new incoming stream with id:" - << stream_id << ". Already " << GetNumOpenStreams() - << " streams open, would exceed max " << get_max_open_streams() - << "."; - // We may already have sent a connection close due to multiple reset - // streams in the same packet. - if (connection()->connected()) { - connection()->SendConnectionClose(QUIC_TOO_MANY_OPEN_STREAMS); - } - return nullptr; - } + // Check if the new number of available streams would cause the number of + // available streams to exceed the limit. Note that the peer can create + // only alternately-numbered streams. + size_t additional_available_streams = + (stream_id - largest_peer_created_stream_id_) / 2 - 1; + size_t new_num_available_streams = + GetNumAvailableStreams() + additional_available_streams; + if (new_num_available_streams > get_max_available_streams()) { + DVLOG(1) << "Failed to create a new incoming stream with id:" << stream_id + << ". There are already " << GetNumAvailableStreams() + << " streams available, which would become " + << new_num_available_streams << ", which exceeds the limit " + << get_max_available_streams() << "."; + CloseConnection(QUIC_TOO_MANY_AVAILABLE_STREAMS); + return nullptr; } for (QuicStreamId id = largest_peer_created_stream_id_ + 2; id < stream_id; @@ -657,18 +635,16 @@ ReliableQuicStream* QuicSession::GetOrCreateDynamicStream( } largest_peer_created_stream_id_ = stream_id; } - if (FLAGS_allow_many_available_streams) { - // Check if the new number of open streams would cause the number of - // open streams to exceed the limit. - if (GetNumOpenStreams() >= get_max_open_streams()) { - if (connection()->version() <= QUIC_VERSION_27) { - CloseConnection(QUIC_TOO_MANY_OPEN_STREAMS); - } else { - // Refuse to open the stream. - SendRstStream(stream_id, QUIC_REFUSED_STREAM, 0); - } - return nullptr; + // Check if the new number of open streams would cause the number of + // open streams to exceed the limit. + if (GetNumOpenStreams() >= get_max_open_streams()) { + if (connection()->version() <= QUIC_VERSION_27) { + CloseConnection(QUIC_TOO_MANY_OPEN_STREAMS); + } else { + // Refuse to open the stream. + SendRstStream(stream_id, QUIC_REFUSED_STREAM, 0); } + return nullptr; } ReliableQuicStream* stream = CreateIncomingDynamicStream(stream_id); if (stream == nullptr) { @@ -711,14 +687,8 @@ bool QuicSession::IsClosedStream(QuicStreamId id) { } size_t QuicSession::GetNumOpenStreams() const { - if (FLAGS_allow_many_available_streams) { - return dynamic_stream_map_.size() - draining_streams_.size() + - locally_closed_streams_highest_offset_.size(); - } else { - return dynamic_stream_map_.size() + available_streams_.size() - - draining_streams_.size() + - locally_closed_streams_highest_offset_.size(); - } + return dynamic_stream_map_.size() - draining_streams_.size() + + locally_closed_streams_highest_offset_.size(); } size_t QuicSession::GetNumActiveStreams() const { diff --git a/net/quic/quic_session_test.cc b/net/quic/quic_session_test.cc index 8c57679..35473da 100644 --- a/net/quic/quic_session_test.cc +++ b/net/quic/quic_session_test.cc @@ -36,7 +36,6 @@ using std::set; using std::string; using std::vector; using testing::CreateFunctor; -using net::SpdyPriority; using testing::InSequence; using testing::Invoke; using testing::Return; @@ -47,7 +46,7 @@ namespace net { namespace test { namespace { -const SpdyPriority kHighestPriority = 0; +const SpdyPriority kHighestPriority = kV3HighestPriority; const SpdyPriority kSomeMiddlePriority = 3; class TestCryptoStream : public QuicCryptoStream { @@ -351,14 +350,9 @@ TEST_P(QuicSessionTestServer, TooManyAvailableStreams) { QuicStreamId stream_id2; EXPECT_NE(nullptr, session_.GetOrCreateDynamicStream(stream_id1)); // A stream ID which is too large to create. - if (FLAGS_allow_many_available_streams) { - stream_id2 = stream_id1 + 2 * session_.get_max_available_streams() + 4; - EXPECT_CALL(*connection_, - SendConnectionClose(QUIC_TOO_MANY_AVAILABLE_STREAMS)); - } else { - stream_id2 = stream_id1 + 2 * session_.get_max_open_streams(); - EXPECT_CALL(*connection_, SendConnectionClose(QUIC_TOO_MANY_OPEN_STREAMS)); - } + stream_id2 = stream_id1 + 2 * session_.get_max_available_streams() + 4; + EXPECT_CALL(*connection_, + SendConnectionClose(QUIC_TOO_MANY_AVAILABLE_STREAMS)); EXPECT_EQ(nullptr, session_.GetOrCreateDynamicStream(stream_id2)); } @@ -737,12 +731,8 @@ TEST_P(QuicSessionTestServer, MultipleRstStreamsCauseSingleConnectionClose) { // Process first invalid stream reset, resulting in the connection being // closed. - if (FLAGS_allow_many_available_streams) { - EXPECT_CALL(*connection_, - SendConnectionClose(QUIC_TOO_MANY_AVAILABLE_STREAMS)); - } else { - EXPECT_CALL(*connection_, SendConnectionClose(QUIC_TOO_MANY_OPEN_STREAMS)); - } + EXPECT_CALL(*connection_, + SendConnectionClose(QUIC_TOO_MANY_AVAILABLE_STREAMS)); const QuicStreamId kLargeInvalidStreamId = 99999999; QuicRstStreamFrame rst1(kLargeInvalidStreamId, QUIC_STREAM_NO_ERROR, 0); @@ -1174,9 +1164,7 @@ TEST_P(QuicSessionTestClient, RecordFinAfterReadSideClosed) { // Receive a stream data frame with FIN. QuicStreamFrame frame(stream_id, true, 0, StringPiece()); session_.OnStreamFrame(frame); - if (FLAGS_quic_fix_fin_accounting) { - EXPECT_TRUE(stream->fin_received()); - } + EXPECT_TRUE(stream->fin_received()); // Reset stream locally. EXPECT_CALL(*connection_, SendRstStream(stream->id(), _, _)); @@ -1189,11 +1177,10 @@ TEST_P(QuicSessionTestClient, RecordFinAfterReadSideClosed) { EXPECT_TRUE(QuicSessionPeer::IsStreamClosed(&session_, stream_id)); EXPECT_EQ(nullptr, QuicSessionPeer::dynamic_streams(&session_)[stream_id]); - // Verify that there is no entry for the stream in - // locally_closed_streams_highest_offset_. - EXPECT_EQ( - FLAGS_quic_fix_fin_accounting ? 0u : 1u, - QuicSessionPeer::GetLocallyClosedStreamsHighestOffset(&session_).size()); + // The stream is not waiting for the arrival of the peer's final offset as it + // was received with the FIN earlier. + EXPECT_EQ(0u, QuicSessionPeer::GetLocallyClosedStreamsHighestOffset(&session_) + .size()); } } // namespace diff --git a/net/quic/quic_spdy_stream_test.cc b/net/quic/quic_spdy_stream_test.cc index 4e7f307..2a7583c 100644 --- a/net/quic/quic_spdy_stream_test.cc +++ b/net/quic/quic_spdy_stream_test.cc @@ -18,8 +18,6 @@ using base::StringPiece; using std::min; using std::string; -using net::kHighestPriority; -using net::SpdyHeaderBlock; using testing::Return; using testing::StrictMock; using testing::_; @@ -122,13 +120,12 @@ TEST_P(QuicSpdyStreamTest, ProcessHeaders) { Initialize(kShouldProcessData); string headers = SpdyUtils::SerializeUncompressedHeaders(headers_); - stream_->OnStreamHeadersPriority(net::kHighestPriority); + stream_->OnStreamHeadersPriority(kV3HighestPriority); stream_->OnStreamHeaders(headers); EXPECT_EQ("", stream_->data()); EXPECT_EQ(headers, stream_->decompressed_headers()); stream_->OnStreamHeadersComplete(false, headers.size()); - EXPECT_EQ(net::kHighestPriority, stream_->Priority()); - EXPECT_EQ(net::kHighestPriority, stream_->Priority()); + EXPECT_EQ(kV3HighestPriority, stream_->Priority()); EXPECT_EQ("", stream_->data()); EXPECT_EQ(headers, stream_->decompressed_headers()); EXPECT_FALSE(stream_->IsDoneReading()); @@ -138,13 +135,12 @@ TEST_P(QuicSpdyStreamTest, ProcessHeadersWithFin) { Initialize(kShouldProcessData); string headers = SpdyUtils::SerializeUncompressedHeaders(headers_); - stream_->OnStreamHeadersPriority(net::kHighestPriority); + stream_->OnStreamHeadersPriority(kV3HighestPriority); stream_->OnStreamHeaders(headers); EXPECT_EQ("", stream_->data()); EXPECT_EQ(headers, stream_->decompressed_headers()); stream_->OnStreamHeadersComplete(true, headers.size()); - EXPECT_EQ(net::kHighestPriority, stream_->Priority()); - EXPECT_EQ(net::kHighestPriority, stream_->Priority()); + EXPECT_EQ(kV3HighestPriority, stream_->Priority()); EXPECT_EQ("", stream_->data()); EXPECT_EQ(headers, stream_->decompressed_headers()); EXPECT_FALSE(stream_->IsDoneReading()); diff --git a/net/quic/quic_stream_sequencer.cc b/net/quic/quic_stream_sequencer.cc index b3f17dd..34c0d25 100644 --- a/net/quic/quic_stream_sequencer.cc +++ b/net/quic/quic_stream_sequencer.cc @@ -84,7 +84,7 @@ void QuicStreamSequencer::OnStreamFrame(const QuicStreamFrame& frame) { } if (byte_offset == buffered_frames_->BytesConsumed()) { - if (FLAGS_quic_implement_stop_reading && ignore_read_data_) { + if (ignore_read_data_) { FlushBufferedFrames(); } else { stream_->OnDataAvailable(); @@ -117,7 +117,7 @@ bool QuicStreamSequencer::MaybeCloseStream() { // This will cause the stream to consume the FIN. // Technically it's an error if |num_bytes_consumed| isn't exactly // equal to |close_offset|, but error handling seems silly at this point. - if (FLAGS_quic_implement_stop_reading && ignore_read_data_) { + if (ignore_read_data_) { // The sequencer is discarding stream data and must notify the stream on // receipt of a FIN because the consumer won't. stream_->OnFinRead(); diff --git a/net/quic/quic_stream_sequencer_test.cc b/net/quic/quic_stream_sequencer_test.cc index 95dfff9..83233dc 100644 --- a/net/quic/quic_stream_sequencer_test.cc +++ b/net/quic/quic_stream_sequencer_test.cc @@ -49,7 +49,7 @@ class MockStream : public ReliableQuicStream { void(QuicErrorCode error, const string& details)); MOCK_METHOD1(Reset, void(QuicRstStreamErrorCode error)); MOCK_METHOD0(OnCanWrite, void()); - SpdyPriority Priority() const override { return net::kHighestPriority; } + SpdyPriority Priority() const override { return kV3HighestPriority; } virtual bool IsFlowControlEnabled() const { return true; } }; diff --git a/net/quic/quic_time.h b/net/quic/quic_time.h index 5637837..9851ca5 100644 --- a/net/quic/quic_time.h +++ b/net/quic/quic_time.h @@ -85,11 +85,16 @@ class NET_EXPORT_PRIVATE QuicTime { return Delta(time_offset_ * d); } - // Returns the later delta of time1 and time2. + // Returns the larger delta of time1 and time2. static inline Delta Max(Delta delta1, Delta delta2) { return delta1 < delta2 ? delta2 : delta1; } + // Returns the smaller delta of time1 and time2. + static inline Delta Min(Delta delta1, Delta delta2) { + return delta1 < delta2 ? delta1 : delta2; + } + inline bool IsZero() const { return time_offset_ == 0; } inline bool IsInfinite() const { diff --git a/net/quic/quic_write_blocked_list.h b/net/quic/quic_write_blocked_list.h index f68c7e7..da91c08 100644 --- a/net/quic/quic_write_blocked_list.h +++ b/net/quic/quic_write_blocked_list.h @@ -101,14 +101,14 @@ class NET_EXPORT_PRIVATE QuicWriteBlockedList { // Headers and crypto streams are special cased to always resume first. void AddStream(QuicStreamId stream_id, SpdyPriority priority) { if (stream_id == kCryptoStreamId) { - DCHECK_EQ(net::kHighestPriority, priority); + DCHECK_EQ(kV3HighestPriority, priority); // TODO(avd) Add DCHECK(!crypto_stream_blocked_) crypto_stream_blocked_ = true; return; } if (stream_id == kHeadersStreamId) { - DCHECK_EQ(net::kHighestPriority, priority); + DCHECK_EQ(kV3HighestPriority, priority); // TODO(avd) Add DCHECK(!headers_stream_blocked_); headers_stream_blocked_ = true; return; @@ -135,11 +135,11 @@ class NET_EXPORT_PRIVATE QuicWriteBlockedList { // batch writes for this priority level. We will allow this stream to write // until it has written kBatchWriteSize bytes, it has no more data to write, // or a higher priority stream preempts. - QuicStreamId batch_write_stream_id_[kLowestPriority + 1]; + QuicStreamId batch_write_stream_id_[kV3LowestPriority + 1]; // Set to kBatchWriteSize when we set a new batch_write_stream_id_ for a given // priority. This is decremented with each write the stream does until it is // done with its batch write. - int32 bytes_left_for_batch_write_[kLowestPriority + 1]; + int32 bytes_left_for_batch_write_[kV3LowestPriority + 1]; // Tracks the last priority popped for UpdateBytesForStream. SpdyPriority last_priority_popped_; diff --git a/net/quic/quic_write_blocked_list_test.cc b/net/quic/quic_write_blocked_list_test.cc index 827f971..46e9c88 100644 --- a/net/quic/quic_write_blocked_list_test.cc +++ b/net/quic/quic_write_blocked_list_test.cc @@ -7,11 +7,8 @@ #include "net/quic/test_tools/quic_test_utils.h" #include "testing/gtest/include/gtest/gtest.h" -using net::kLowestPriority; -using net::kHighestPriority; - -using net::kLowestPriority; -using net::kHighestPriority; +using net::kV3LowestPriority; +using net::kV3HighestPriority; namespace net { namespace test { @@ -22,11 +19,11 @@ TEST(QuicWriteBlockedListTest, PriorityOrder) { // Mark streams blocked in roughly reverse priority order, and // verify that streams are sorted. - write_blocked_list.AddStream(40, net::kLowestPriority); - write_blocked_list.AddStream(23, net::kHighestPriority); - write_blocked_list.AddStream(17, net::kHighestPriority); - write_blocked_list.AddStream(kHeadersStreamId, net::kHighestPriority); - write_blocked_list.AddStream(kCryptoStreamId, net::kHighestPriority); + write_blocked_list.AddStream(40, kV3LowestPriority); + write_blocked_list.AddStream(23, kV3HighestPriority); + write_blocked_list.AddStream(17, kV3HighestPriority); + write_blocked_list.AddStream(kHeadersStreamId, kV3HighestPriority); + write_blocked_list.AddStream(kCryptoStreamId, kV3HighestPriority); EXPECT_EQ(5u, write_blocked_list.NumBlockedStreams()); EXPECT_TRUE(write_blocked_list.HasWriteBlockedCryptoOrHeadersStream()); @@ -48,7 +45,7 @@ TEST(QuicWriteBlockedListTest, PriorityOrder) { TEST(QuicWriteBlockedListTest, CryptoStream) { QuicWriteBlockedList write_blocked_list; - write_blocked_list.AddStream(kCryptoStreamId, net::kHighestPriority); + write_blocked_list.AddStream(kCryptoStreamId, kV3HighestPriority); EXPECT_EQ(1u, write_blocked_list.NumBlockedStreams()); EXPECT_TRUE(write_blocked_list.HasWriteBlockedCryptoOrHeadersStream()); @@ -59,7 +56,7 @@ TEST(QuicWriteBlockedListTest, CryptoStream) { TEST(QuicWriteBlockedListTest, HeadersStream) { QuicWriteBlockedList write_blocked_list; - write_blocked_list.AddStream(kHeadersStreamId, net::kHighestPriority); + write_blocked_list.AddStream(kHeadersStreamId, kV3HighestPriority); EXPECT_EQ(1u, write_blocked_list.NumBlockedStreams()); EXPECT_TRUE(write_blocked_list.HasWriteBlockedCryptoOrHeadersStream()); @@ -70,8 +67,8 @@ TEST(QuicWriteBlockedListTest, HeadersStream) { TEST(QuicWriteBlockedListTest, VerifyHeadersStream) { QuicWriteBlockedList write_blocked_list; - write_blocked_list.AddStream(5, net::kHighestPriority); - write_blocked_list.AddStream(kHeadersStreamId, net::kHighestPriority); + write_blocked_list.AddStream(5, kV3HighestPriority); + write_blocked_list.AddStream(kHeadersStreamId, kV3HighestPriority); EXPECT_EQ(2u, write_blocked_list.NumBlockedStreams()); EXPECT_TRUE(write_blocked_list.HasWriteBlockedCryptoOrHeadersStream()); @@ -92,9 +89,9 @@ TEST(QuicWriteBlockedListTest, NoDuplicateEntries) { // Try to add a stream to the write blocked list multiple times at the same // priority. const QuicStreamId kBlockedId = kClientDataStreamId1; - write_blocked_list.AddStream(kBlockedId, net::kHighestPriority); - write_blocked_list.AddStream(kBlockedId, net::kHighestPriority); - write_blocked_list.AddStream(kBlockedId, net::kHighestPriority); + write_blocked_list.AddStream(kBlockedId, kV3HighestPriority); + write_blocked_list.AddStream(kBlockedId, kV3HighestPriority); + write_blocked_list.AddStream(kBlockedId, kV3HighestPriority); // This should only result in one blocked stream being added. EXPECT_EQ(1u, write_blocked_list.NumBlockedStreams()); @@ -113,49 +110,49 @@ TEST(QuicWriteBlockedListTest, BatchingWrites) { const QuicStreamId id1 = kClientDataStreamId1; const QuicStreamId id2 = kClientDataStreamId2; - write_blocked_list.AddStream(id1, net::kLowestPriority); - write_blocked_list.AddStream(id2, net::kLowestPriority); + write_blocked_list.AddStream(id1, kV3LowestPriority); + write_blocked_list.AddStream(id2, kV3LowestPriority); EXPECT_EQ(2u, write_blocked_list.NumBlockedStreams()); // The first stream we push back should stay at the front until 16k is // written. EXPECT_EQ(id1, write_blocked_list.PopFront()); - write_blocked_list.AddStream(id1, net::kHighestPriority); + write_blocked_list.AddStream(id1, net::kV3HighestPriority); write_blocked_list.UpdateBytesForStream(id1, 15999); - write_blocked_list.AddStream(id1, net::kLowestPriority); + write_blocked_list.AddStream(id1, kV3LowestPriority); EXPECT_EQ(2u, write_blocked_list.NumBlockedStreams()); EXPECT_EQ(id1, write_blocked_list.PopFront()); // Once 16k is written the first stream will cede to the next. - write_blocked_list.AddStream(id1, net::kHighestPriority); + write_blocked_list.AddStream(id1, net::kV3HighestPriority); write_blocked_list.UpdateBytesForStream(id1, 1); - write_blocked_list.AddStream(id1, net::kLowestPriority); + write_blocked_list.AddStream(id1, kV3LowestPriority); EXPECT_EQ(2u, write_blocked_list.NumBlockedStreams()); EXPECT_EQ(id2, write_blocked_list.PopFront()); // Set the new stream to have written all but one byte. - write_blocked_list.AddStream(id2, net::kHighestPriority); + write_blocked_list.AddStream(id2, net::kV3HighestPriority); write_blocked_list.UpdateBytesForStream(id2, 15999); - write_blocked_list.AddStream(id2, net::kLowestPriority); + write_blocked_list.AddStream(id2, kV3LowestPriority); EXPECT_EQ(2u, write_blocked_list.NumBlockedStreams()); // Ensure higher priority streams are popped first. const QuicStreamId id3 = kClientDataStreamId2 + 2; - write_blocked_list.AddStream(id3, net::kHighestPriority); + write_blocked_list.AddStream(id3, kV3HighestPriority); EXPECT_EQ(id3, write_blocked_list.PopFront()); // Higher priority streams will always be popped first, even if using their // byte quota write_blocked_list.UpdateBytesForStream(id3, 20000); - write_blocked_list.AddStream(id3, net::kHighestPriority); + write_blocked_list.AddStream(id3, kV3HighestPriority); EXPECT_EQ(id3, write_blocked_list.PopFront()); // Once the higher priority stream is out of the way, id2 will resume its 16k // write, with only 1 byte remaining of its guaranteed write allocation. EXPECT_EQ(id2, write_blocked_list.PopFront()); - write_blocked_list.AddStream(id2, net::kHighestPriority); + write_blocked_list.AddStream(id2, net::kV3HighestPriority); write_blocked_list.UpdateBytesForStream(id2, 1); - write_blocked_list.AddStream(id2, net::kLowestPriority); + write_blocked_list.AddStream(id2, kV3LowestPriority); EXPECT_EQ(2u, write_blocked_list.NumBlockedStreams()); EXPECT_EQ(id1, write_blocked_list.PopFront()); } diff --git a/net/quic/reliable_quic_stream.cc b/net/quic/reliable_quic_stream.cc index 8389c0f..daf0915 100644 --- a/net/quic/reliable_quic_stream.cc +++ b/net/quic/reliable_quic_stream.cc @@ -91,15 +91,6 @@ void ReliableQuicStream::SetFromConfig() { void ReliableQuicStream::OnStreamFrame(const QuicStreamFrame& frame) { DCHECK_EQ(frame.stream_id, id_); - bool flag_value = FLAGS_quic_fix_fin_accounting; - if (!flag_value) { - if (read_side_closed_) { - DVLOG(1) << ENDPOINT << "Ignoring frame " << frame.stream_id; - // The subclass does not want to read data: blackhole the data. - return; - } - } - if (frame.fin) { fin_received_ = true; if (fin_sent_) { @@ -107,12 +98,10 @@ void ReliableQuicStream::OnStreamFrame(const QuicStreamFrame& frame) { } } - if (flag_value) { - if (read_side_closed_) { - DVLOG(1) << ENDPOINT << "Ignoring data in frame " << frame.stream_id; - // The subclass does not want to read data: blackhole the data. - return; - } + if (read_side_closed_) { + DVLOG(1) << ENDPOINT << "Ignoring data in frame " << frame.stream_id; + // The subclass does not want to read data: blackhole the data. + return; } // This count includes duplicate data received. @@ -389,10 +378,6 @@ QuicVersion ReliableQuicStream::version() const { } void ReliableQuicStream::StopReading() { - if (!FLAGS_quic_implement_stop_reading) { - CloseReadSide(); - return; - } DVLOG(1) << ENDPOINT << "Stop reading from stream " << id(); sequencer_.StopReading(); } diff --git a/net/quic/reliable_quic_stream_test.cc b/net/quic/reliable_quic_stream_test.cc index 03442b3..0fca992 100644 --- a/net/quic/reliable_quic_stream_test.cc +++ b/net/quic/reliable_quic_stream_test.cc @@ -60,7 +60,7 @@ class TestStream : public ReliableQuicStream { return should_process_data_ ? data_len : 0; } - SpdyPriority Priority() const override { return net::kHighestPriority; } + SpdyPriority Priority() const override { return net::kV3HighestPriority; } using ReliableQuicStream::WriteOrBufferData; using ReliableQuicStream::CloseWriteSide; @@ -612,10 +612,6 @@ TEST_F(ReliableQuicStreamTest, // Verify that after the consumer calls StopReading(), the stream still sends // flow control updates. TEST_F(ReliableQuicStreamTest, StopReadingSendsFlowControl) { - if (!FLAGS_quic_implement_stop_reading) { - return; - } - Initialize(kShouldProcessData); stream_->StopReading(); @@ -759,13 +755,8 @@ TEST_F(ReliableQuicStreamTest, EarlyResponseFinHandling) { // Receive remaining data and FIN for the request. QuicStreamFrame frame2(stream_->id(), true, 0, StringPiece("End")); stream_->OnStreamFrame(frame2); - if (FLAGS_quic_fix_fin_accounting) { - EXPECT_TRUE(stream_->fin_received()); - EXPECT_TRUE(stream_->HasFinalReceivedByteOffset()); - } else { - EXPECT_FALSE(stream_->fin_received()); - EXPECT_FALSE(stream_->HasFinalReceivedByteOffset()); - } + EXPECT_TRUE(stream_->fin_received()); + EXPECT_TRUE(stream_->HasFinalReceivedByteOffset()); } } // namespace diff --git a/net/quic/test_tools/quic_connection_peer.cc b/net/quic/test_tools/quic_connection_peer.cc index 15653ce..c2aad97 100644 --- a/net/quic/test_tools/quic_connection_peer.cc +++ b/net/quic/test_tools/quic_connection_peer.cc @@ -262,5 +262,10 @@ void QuicConnectionPeer::SetNextMtuProbeAt(QuicConnection* connection, connection->next_mtu_probe_at_ = number; } +// static +void QuicConnectionPeer::EnableAckDecimation(QuicConnection* connection) { + connection->ack_decimation_enabled_ = true; +} + } // namespace test } // namespace net diff --git a/net/quic/test_tools/quic_connection_peer.h b/net/quic/test_tools/quic_connection_peer.h index ff29cf1..6cabf65 100644 --- a/net/quic/test_tools/quic_connection_peer.h +++ b/net/quic/test_tools/quic_connection_peer.h @@ -121,6 +121,7 @@ class QuicConnectionPeer { QuicPacketCount packets); static void SetNextMtuProbeAt(QuicConnection* connection, QuicPacketNumber number); + static void EnableAckDecimation(QuicConnection* connection); private: DISALLOW_COPY_AND_ASSIGN(QuicConnectionPeer); diff --git a/net/quic/test_tools/quic_packet_creator_peer.cc b/net/quic/test_tools/quic_packet_creator_peer.cc index 82ae990..0b00d00 100644 --- a/net/quic/test_tools/quic_packet_creator_peer.cc +++ b/net/quic/test_tools/quic_packet_creator_peer.cc @@ -73,5 +73,51 @@ size_t QuicPacketCreatorPeer::CreateStreamFrame(QuicPacketCreator* creator, buffer); } +// static +bool QuicPacketCreatorPeer::IsFecProtected(QuicPacketCreator* creator) { + return creator->fec_protect_; +} + +// static +bool QuicPacketCreatorPeer::IsFecEnabled(QuicPacketCreator* creator) { + return creator->max_packets_per_fec_group_ > 0; +} + +// static +void QuicPacketCreatorPeer::StartFecProtectingPackets( + QuicPacketCreator* creator) { + creator->StartFecProtectingPackets(); +} + +// static +void QuicPacketCreatorPeer::StopFecProtectingPackets( + QuicPacketCreator* creator) { + creator->StopFecProtectingPackets(); +} + +// static +SerializedPacket QuicPacketCreatorPeer::SerializeFec(QuicPacketCreator* creator, + char* buffer, + size_t buffer_len) { + return creator->SerializeFec(buffer, buffer_len); +} + +// static +void QuicPacketCreatorPeer::ResetFecGroup(QuicPacketCreator* creator) { + creator->ResetFecGroup(); +} + +// static +QuicTime::Delta QuicPacketCreatorPeer::GetFecTimeout( + QuicPacketCreator* creator) { + return creator->fec_timeout_; +} + +// static +float QuicPacketCreatorPeer::GetRttMultiplierForFecTimeout( + QuicPacketCreator* creator) { + return creator->rtt_multiplier_for_fec_timeout_; +} + } // namespace test } // namespace net diff --git a/net/quic/test_tools/quic_packet_creator_peer.h b/net/quic/test_tools/quic_packet_creator_peer.h index 72d29d9..fdaaa03 100644 --- a/net/quic/test_tools/quic_packet_creator_peer.h +++ b/net/quic/test_tools/quic_packet_creator_peer.h @@ -41,6 +41,17 @@ class QuicPacketCreatorPeer { bool fin, QuicFrame* frame, UniqueStreamBuffer* buffer); + static bool IsFecProtected(QuicPacketCreator* creator); + static bool IsFecEnabled(QuicPacketCreator* creator); + static void StartFecProtectingPackets(QuicPacketCreator* creator); + static void StopFecProtectingPackets(QuicPacketCreator* creator); + static SerializedPacket SerializeFec(QuicPacketCreator* creator, + char* buffer, + size_t buffer_len); + static void ResetFecGroup(QuicPacketCreator* creator); + static QuicTime::Delta GetFecTimeout(QuicPacketCreator* creator); + // TODO(rtenneti): Delete this code after the 0.25 RTT FEC experiment. + static float GetRttMultiplierForFecTimeout(QuicPacketCreator* creator); private: DISALLOW_COPY_AND_ASSIGN(QuicPacketCreatorPeer); diff --git a/net/quic/test_tools/quic_packet_generator_peer.cc b/net/quic/test_tools/quic_packet_generator_peer.cc index c6ef402..ad0693a 100644 --- a/net/quic/test_tools/quic_packet_generator_peer.cc +++ b/net/quic/test_tools/quic_packet_generator_peer.cc @@ -16,11 +16,5 @@ QuicPacketCreator* QuicPacketGeneratorPeer::GetPacketCreator( return &generator->packet_creator_; } -// static -QuicTime::Delta QuicPacketGeneratorPeer::GetFecTimeout( - QuicPacketGenerator* generator) { - return generator->fec_timeout_; -} - } // namespace test } // namespace net diff --git a/net/quic/test_tools/quic_packet_generator_peer.h b/net/quic/test_tools/quic_packet_generator_peer.h index fba8dbe..0762d1f 100644 --- a/net/quic/test_tools/quic_packet_generator_peer.h +++ b/net/quic/test_tools/quic_packet_generator_peer.h @@ -17,7 +17,6 @@ namespace test { class QuicPacketGeneratorPeer { public: static QuicPacketCreator* GetPacketCreator(QuicPacketGenerator* generator); - static QuicTime::Delta GetFecTimeout(QuicPacketGenerator* generator); private: DISALLOW_COPY_AND_ASSIGN(QuicPacketGeneratorPeer); diff --git a/net/quic/test_tools/quic_test_utils.cc b/net/quic/test_tools/quic_test_utils.cc index 380a487a..f46419b 100644 --- a/net/quic/test_tools/quic_test_utils.cc +++ b/net/quic/test_tools/quic_test_utils.cc @@ -329,15 +329,15 @@ TestQuicSpdyServerSession::TestQuicSpdyServerSession( QuicConnection* connection, const QuicConfig& config, const QuicCryptoServerConfig* crypto_config) - : QuicSpdySession(connection, config) { - crypto_stream_.reset(new QuicCryptoServerStream(crypto_config, this)); + : QuicServerSession(config, connection, &visitor_, crypto_config) { Initialize(); } TestQuicSpdyServerSession::~TestQuicSpdyServerSession() {} QuicCryptoServerStream* TestQuicSpdyServerSession::GetCryptoStream() { - return crypto_stream_.get(); + return static_cast<QuicCryptoServerStream*>( + QuicServerSession::GetCryptoStream()); } TestQuicSpdyClientSession::TestQuicSpdyClientSession( @@ -655,7 +655,8 @@ size_t GetPacketLengthForOneStream(QuicVersion version, NullEncrypter().GetCiphertextSize( QuicFramer::GetMinAckFrameSize(PACKET_1BYTE_PACKET_NUMBER)) + GetPacketHeaderSize(connection_id_length, include_version, - packet_number_length, is_in_fec_group); + /*include_path_id=*/false, packet_number_length, + is_in_fec_group); if (stream_length < ack_length) { *payload_length = 1 + ack_length - stream_length; } @@ -756,6 +757,7 @@ MockQuicConnectionDebugVisitor::~MockQuicConnectionDebugVisitor() {} void CreateClientSessionForTest(QuicServerId server_id, bool supports_stateless_rejects, QuicTime::Delta connection_start_time, + QuicVersionVector supported_versions, MockConnectionHelper* helper, QuicCryptoClientConfig* crypto_client_config, PacketSavingConnection** client_connection, @@ -770,8 +772,8 @@ void CreateClientSessionForTest(QuicServerId server_id, QuicConfig config = supports_stateless_rejects ? DefaultQuicConfigStatelessRejects() : DefaultQuicConfig(); - *client_connection = - new PacketSavingConnection(helper, Perspective::IS_CLIENT); + *client_connection = new PacketSavingConnection( + helper, Perspective::IS_CLIENT, supported_versions); *client_session = new TestQuicSpdyClientSession( *client_connection, config, server_id, crypto_client_config); (*client_connection)->AdvanceTime(connection_start_time); @@ -779,6 +781,7 @@ void CreateClientSessionForTest(QuicServerId server_id, void CreateServerSessionForTest(QuicServerId server_id, QuicTime::Delta connection_start_time, + QuicVersionVector supported_versions, MockConnectionHelper* helper, QuicCryptoServerConfig* server_crypto_config, PacketSavingConnection** server_connection, @@ -790,8 +793,8 @@ void CreateServerSessionForTest(QuicServerId server_id, << "Connections must start at non-zero times, otherwise the " << "strike-register will be unhappy."; - *server_connection = - new PacketSavingConnection(helper, Perspective::IS_SERVER); + *server_connection = new PacketSavingConnection( + helper, Perspective::IS_SERVER, supported_versions); *server_session = new TestQuicSpdyServerSession( *server_connection, DefaultQuicConfig(), server_crypto_config); diff --git a/net/quic/test_tools/quic_test_utils.h b/net/quic/test_tools/quic_test_utils.h index 8a0189f..d78916a 100644 --- a/net/quic/test_tools/quic_test_utils.h +++ b/net/quic/test_tools/quic_test_utils.h @@ -26,6 +26,7 @@ #include "net/tools/quic/quic_dispatcher.h" #include "net/tools/quic/quic_per_connection_packet_writer.h" #include "net/tools/quic/quic_server_session.h" +#include "net/tools/quic/test_tools/mock_quic_server_session_visitor.h" #include "testing/gmock/include/gmock/gmock.h" namespace net { @@ -459,7 +460,7 @@ class MockQuicSpdySession : public QuicSpdySession { DISALLOW_COPY_AND_ASSIGN(MockQuicSpdySession); }; -class TestQuicSpdyServerSession : public QuicSpdySession { +class TestQuicSpdyServerSession : public tools::QuicServerSession { public: TestQuicSpdyServerSession(QuicConnection* connection, const QuicConfig& config, @@ -472,7 +473,7 @@ class TestQuicSpdyServerSession : public QuicSpdySession { QuicCryptoServerStream* GetCryptoStream() override; private: - scoped_ptr<QuicCryptoServerStream> crypto_stream_; + tools::test::MockQuicServerSessionVisitor visitor_; DISALLOW_COPY_AND_ASSIGN(TestQuicSpdyServerSession); }; @@ -738,6 +739,7 @@ class MockQuicConnectionDebugVisitor : public QuicConnectionDebugVisitor { // Needed for strike-register nonce verification. The client // connection_start_time should be synchronized witht the server // start time, otherwise nonce verification will fail. +// supported_versions: Set of QUIC versions this client supports. // helper: Pointer to the MockConnectionHelper to use for the session. // crypto_client_config: Pointer to the crypto client config. // client_connection: Pointer reference for newly created @@ -748,6 +750,7 @@ class MockQuicConnectionDebugVisitor : public QuicConnectionDebugVisitor { void CreateClientSessionForTest(QuicServerId server_id, bool supports_stateless_rejects, QuicTime::Delta connection_start_time, + QuicVersionVector supported_versions, MockConnectionHelper* helper, QuicCryptoClientConfig* crypto_client_config, PacketSavingConnection** client_connection, @@ -760,6 +763,7 @@ void CreateClientSessionForTest(QuicServerId server_id, // Needed for strike-register nonce verification. The server // connection_start_time should be synchronized witht the client // start time, otherwise nonce verification will fail. +// supported_versions: Set of QUIC versions this server supports. // helper: Pointer to the MockConnectionHelper to use for the session. // crypto_server_config: Pointer to the crypto server config. // server_connection: Pointer reference for newly created @@ -769,6 +773,7 @@ void CreateClientSessionForTest(QuicServerId server_id, // session. The new object will be owned by the caller. void CreateServerSessionForTest(QuicServerId server_id, QuicTime::Delta connection_start_time, + QuicVersionVector supported_versions, MockConnectionHelper* helper, QuicCryptoServerConfig* crypto_server_config, PacketSavingConnection** server_connection, diff --git a/net/spdy/spdy_protocol.h b/net/spdy/spdy_protocol.h index 75b1e83..2992711 100644 --- a/net/spdy/spdy_protocol.h +++ b/net/spdy/spdy_protocol.h @@ -419,6 +419,12 @@ enum SpdyGoAwayStatus { // number between 0 and 3. typedef uint8 SpdyPriority; +// Lowest and Highest here refer to SPDY priorities as described in + +// https://www.chromium.org/spdy/spdy-protocol/spdy-protocol-draft3-1#TOC-2.3.3-Stream-priority +const SpdyPriority kV3HighestPriority = 0; +const SpdyPriority kV3LowestPriority = 7; + typedef uint64 SpdyPingId; typedef std::string SpdyProtocolId; diff --git a/net/spdy/write_blocked_list.h b/net/spdy/write_blocked_list.h index 7622dc2..47b19db 100644 --- a/net/spdy/write_blocked_list.h +++ b/net/spdy/write_blocked_list.h @@ -18,9 +18,6 @@ namespace test { class WriteBlockedListPeer; } // namespace test -const int kHighestPriority = 0; -const int kLowestPriority = 7; - template <typename IdType> class WriteBlockedList { public: @@ -31,26 +28,26 @@ class WriteBlockedList { WriteBlockedList() {} static SpdyPriority ClampPriority(SpdyPriority priority) { - if (priority < kHighestPriority) { + if (priority < kV3HighestPriority) { LOG(DFATAL) << "Invalid priority: " << static_cast<int>(priority); - return kHighestPriority; + return kV3HighestPriority; } - if (priority > kLowestPriority) { + if (priority > kV3LowestPriority) { LOG(DFATAL) << "Invalid priority: " << static_cast<int>(priority); - return kLowestPriority; + return kV3LowestPriority; } return priority; } // Returns the priority of the highest priority list with sessions on it. SpdyPriority GetHighestPriorityWriteBlockedList() const { - for (SpdyPriority i = 0; i <= kLowestPriority; ++i) { + for (SpdyPriority i = 0; i <= kV3LowestPriority; ++i) { if (write_blocked_lists_[i].size() > 0) { return i; } } LOG(DFATAL) << "No blocked streams"; - return kHighestPriority; + return kV3HighestPriority; } IdType PopFront(SpdyPriority priority) { @@ -64,7 +61,7 @@ class WriteBlockedList { bool HasWriteBlockedStreamsGreaterThanPriority(SpdyPriority priority) const { priority = ClampPriority(priority); - for (SpdyPriority i = kHighestPriority; i < priority; ++i) { + for (SpdyPriority i = kV3HighestPriority; i < priority; ++i) { if (!write_blocked_lists_[i].empty()) { return true; } @@ -73,7 +70,7 @@ class WriteBlockedList { } bool HasWriteBlockedStreams() const { - for (SpdyPriority i = kHighestPriority; i <= kLowestPriority; ++i) { + for (SpdyPriority i = kV3HighestPriority; i <= kV3LowestPriority; ++i) { if (!write_blocked_lists_[i].empty()) { return true; } @@ -138,7 +135,7 @@ class WriteBlockedList { size_t NumBlockedStreams() const { size_t num_blocked_streams = 0; - for (SpdyPriority i = kHighestPriority; i <= kLowestPriority; ++i) { + for (SpdyPriority i = kV3HighestPriority; i <= kV3LowestPriority; ++i) { num_blocked_streams += write_blocked_lists_[i].size(); } return num_blocked_streams; @@ -186,7 +183,7 @@ class WriteBlockedList { } } } - BlockedList write_blocked_lists_[kLowestPriority + 1]; + BlockedList write_blocked_lists_[kV3LowestPriority + 1]; StreamToPriorityMap stream_to_priority_; }; diff --git a/net/tools/quic/end_to_end_test.cc b/net/tools/quic/end_to_end_test.cc index 3cd4ff0..b0de2f2 100644 --- a/net/tools/quic/end_to_end_test.cc +++ b/net/tools/quic/end_to_end_test.cc @@ -2059,17 +2059,10 @@ TEST_P(EndToEndTest, EarlyResponseFinRecording) { EXPECT_TRUE(it != map.end()); QuicServerSession* server_session = it->second; - // Verify that the stream is not pending the arrival of the peer's final - // offset. - if (FLAGS_quic_fix_fin_accounting) { - EXPECT_EQ(0u, QuicSessionPeer::GetLocallyClosedStreamsHighestOffset( - server_session) - .size()); - } else { - EXPECT_EQ(1u, QuicSessionPeer::GetLocallyClosedStreamsHighestOffset( - server_session) - .size()); - } + // The stream is not waiting for the arrival of the peer's final offset. + EXPECT_EQ( + 0u, QuicSessionPeer::GetLocallyClosedStreamsHighestOffset(server_session) + .size()); server_thread_->Resume(); } @@ -2117,23 +2110,13 @@ TEST_P(EndToEndTest, LargePostEarlyResponse) { GenerateBody(&body, kBodySize); client_->SendData(body, true); - if (FLAGS_quic_implement_stop_reading) { - // Run the client to let any buffered data be sent. - // (This is OK despite already waiting for a response.) - client_->WaitForResponse(); - // There should be no buffered data to write in the client's stream. - ReliableQuicStream* stream = - client_->client()->session()->GetStream(kClientDataStreamId1); - EXPECT_FALSE(stream != nullptr && stream->HasBufferedData()); - } else { - // Run the client for 0.1 second to let any buffered data be sent. - // Must have a timeout, as the stream will not close and cause a return. - // (This is OK despite already waiting for a response.) - client_->WaitForResponseForMs(100); - // There will be buffered data to write in the client's stream. - ReliableQuicStream* stream = client_->client()->session()->GetStream(5); - EXPECT_TRUE(stream != nullptr && stream->HasBufferedData()); - } + // Run the client to let any buffered data be sent. + // (This is OK despite already waiting for a response.) + client_->WaitForResponse(); + // There should be no buffered data to write in the client's stream. + ReliableQuicStream* stream = + client_->client()->session()->GetStream(kClientDataStreamId1); + EXPECT_FALSE(stream != nullptr && stream->HasBufferedData()); } } // namespace diff --git a/net/tools/quic/quic_in_memory_cache.cc b/net/tools/quic/quic_in_memory_cache.cc index eed6c78..0671099 100644 --- a/net/tools/quic/quic_in_memory_cache.cc +++ b/net/tools/quic/quic_in_memory_cache.cc @@ -9,6 +9,7 @@ #include "base/stl_util.h" #include "base/strings/string_number_conversions.h" #include "base/strings/string_util.h" +#include "base/strings/stringprintf.h" #include "net/http/http_response_headers.h" #include "net/http/http_util.h" #include "net/spdy/spdy_http_utils.h" @@ -21,6 +22,16 @@ using std::string; namespace net { namespace tools { +QuicInMemoryCache::ServerPushInfo::ServerPushInfo( + GURL request_url, + const SpdyHeaderBlock& headers, + net::SpdyPriority priority, + string body) + : request_url(request_url), + headers(headers), + priority(priority), + body(body) {} + QuicInMemoryCache::Response::Response() : response_type_(REGULAR_RESPONSE) {} QuicInMemoryCache::Response::~Response() {} @@ -43,6 +54,8 @@ const QuicInMemoryCache::Response* QuicInMemoryCache::GetResponse( return it->second; } +typedef QuicInMemoryCache::ServerPushInfo ServerPushInfo; + void QuicInMemoryCache::AddSimpleResponse(StringPiece host, StringPiece path, int response_code, @@ -54,6 +67,16 @@ void QuicInMemoryCache::AddSimpleResponse(StringPiece host, AddResponse(host, path, response_headers, body); } +void QuicInMemoryCache::AddSimpleResponseWithServerPushResources( + StringPiece host, + StringPiece path, + int response_code, + StringPiece body, + list<ServerPushInfo> push_resources) { + AddSimpleResponse(host, path, response_code, body); + MaybeAddServerPushResources(host, path, push_resources); +} + void QuicInMemoryCache::AddDefaultResponse(Response* response) { default_response_.reset(response); } @@ -76,6 +99,7 @@ QuicInMemoryCache::QuicInMemoryCache() {} void QuicInMemoryCache::ResetForTests() { STLDeleteValues(&responses_); + server_push_resources_.clear(); } void QuicInMemoryCache::InitializeFromDirectory(const string& cache_directory) { @@ -149,18 +173,26 @@ void QuicInMemoryCache::InitializeFromDirectory(const string& cache_directory) { } } +list<ServerPushInfo> QuicInMemoryCache::GetServerPushResources( + string request_url) { + list<ServerPushInfo> resources; + auto resource_range = server_push_resources_.equal_range(request_url); + for (auto it = resource_range.first; it != resource_range.second; ++it) { + resources.push_back(it->second); + } + return resources; +} + QuicInMemoryCache::~QuicInMemoryCache() { STLDeleteValues(&responses_); } -void QuicInMemoryCache::AddResponseImpl( - StringPiece host, - StringPiece path, - SpecialResponseType response_type, - const SpdyHeaderBlock& response_headers, - StringPiece response_body) { +void QuicInMemoryCache::AddResponseImpl(StringPiece host, + StringPiece path, + SpecialResponseType response_type, + const SpdyHeaderBlock& response_headers, + StringPiece response_body) { string key = GetKey(host, path); - VLOG(1) << "Adding response for: " << key; if (ContainsKey(responses_, key)) { LOG(DFATAL) << "Response for '" << key << "' already exists!"; return; @@ -176,5 +208,42 @@ string QuicInMemoryCache::GetKey(StringPiece host, StringPiece path) const { return host.as_string() + path.as_string(); } +void QuicInMemoryCache::MaybeAddServerPushResources( + StringPiece request_host, + StringPiece request_path, + list<ServerPushInfo> push_resources) { + string request_url = request_host.as_string() + request_path.as_string(); + + for (const auto& push_resource : push_resources) { + if (PushResourceExistsInCache(request_url, push_resource)) { + continue; + } + + DVLOG(1) << "Add request-resource association."; + server_push_resources_.insert(std::make_pair(request_url, push_resource)); + string path = push_resource.request_url.path(); + if (responses_.find(GetKey(request_host, path)) == responses_.end()) { + // Add a server push response to responses map, if it is not in the map. + SpdyHeaderBlock headers = push_resource.headers; + StringPiece body = push_resource.body; + DVLOG(1) << "Add response for push resource " << body; + AddResponse(request_host, path, headers, body); + } + } +} + +bool QuicInMemoryCache::PushResourceExistsInCache(string original_request_url, + ServerPushInfo resource) { + auto resource_range = + server_push_resources_.equal_range(original_request_url); + for (auto it = resource_range.first; it != resource_range.second; ++it) { + ServerPushInfo push_resource = it->second; + if (push_resource.request_url.spec() == resource.request_url.spec()) { + return true; + } + } + return false; +} + } // namespace tools } // namespace net diff --git a/net/tools/quic/quic_in_memory_cache.h b/net/tools/quic/quic_in_memory_cache.h index 5aee73a..5ceaf52 100644 --- a/net/tools/quic/quic_in_memory_cache.h +++ b/net/tools/quic/quic_in_memory_cache.h @@ -5,12 +5,19 @@ #ifndef NET_TOOLS_QUIC_QUIC_IN_MEMORY_CACHE_H_ #define NET_TOOLS_QUIC_QUIC_IN_MEMORY_CACHE_H_ +#include <map> #include <string> #include "base/containers/hash_tables.h" #include "base/memory/singleton.h" #include "base/strings/string_piece.h" +#include "net/quic/spdy_utils.h" #include "net/spdy/spdy_framer.h" +#include "url/gurl.h" + +using base::StringPiece; +using std::string; +using std::list; namespace base { @@ -32,6 +39,19 @@ class QuicServer; // `wget -p --save_headers <url>` class QuicInMemoryCache { public: + // A ServerPushInfo contains path of the push request and everything needed in + // comprising a response for the push request. + struct ServerPushInfo { + ServerPushInfo(GURL request_url, + const net::SpdyHeaderBlock& headers, + net::SpdyPriority priority, + string body); + GURL request_url; + net::SpdyHeaderBlock headers; + net::SpdyPriority priority; + string body; + }; + enum SpecialResponseType { REGULAR_RESPONSE, // Send the headers and body like a server should. CLOSE_CONNECTION, // Close the connection (sending the close packet). @@ -61,7 +81,7 @@ class QuicInMemoryCache { private: SpecialResponseType response_type_; SpdyHeaderBlock headers_; - std::string body_; + string body_; DISALLOW_COPY_AND_ASSIGN(Response); }; @@ -81,6 +101,17 @@ class QuicInMemoryCache { int response_code, base::StringPiece body); + // Add a simple response to the cache as AddSimpleResponse() does, and add + // some server push resources(resource path, corresponding response status and + // path) associated with it. + // Push resource implicitly come from the same host. + void AddSimpleResponseWithServerPushResources( + StringPiece host, + StringPiece path, + int response_code, + StringPiece body, + list<ServerPushInfo> push_resources); + // Add a response to the cache. void AddResponse(base::StringPiece host, base::StringPiece path, @@ -97,10 +128,13 @@ class QuicInMemoryCache { void AddDefaultResponse(Response* response); // |cache_cirectory| can be generated using `wget -p --save-headers <url>`. - void InitializeFromDirectory(const std::string& cache_directory); + void InitializeFromDirectory(const string& cache_directory); + + // Find all the server push resources associated with |request_url|. + list<ServerPushInfo> GetServerPushResources(string request_url); private: - typedef base::hash_map<std::string, Response*> ResponseMap; + typedef base::hash_map<string, Response*> ResponseMap; friend struct base::DefaultSingletonTraits<QuicInMemoryCache>; friend class test::QuicInMemoryCachePeer; @@ -116,7 +150,18 @@ class QuicInMemoryCache { const SpdyHeaderBlock& response_headers, base::StringPiece response_body); - std::string GetKey(base::StringPiece host, base::StringPiece path) const; + string GetKey(base::StringPiece host, base::StringPiece path) const; + + // Add some server push urls with given responses for specified + // request if these push resources are not associated with this request yet. + void MaybeAddServerPushResources(StringPiece request_host, + StringPiece request_path, + list<ServerPushInfo> push_resources); + + // Check if push resource(push_host/push_path) associated with given request + // url already exists in server push map. + bool PushResourceExistsInCache(string original_request_url, + ServerPushInfo resource); // Cached responses. ResponseMap responses_; @@ -124,6 +169,9 @@ class QuicInMemoryCache { // The default response for cache misses, if set. scoped_ptr<Response> default_response_; + // A map from request URL to associated server push responses (if any). + std::multimap<string, ServerPushInfo> server_push_resources_; + DISALLOW_COPY_AND_ASSIGN(QuicInMemoryCache); }; diff --git a/net/tools/quic/quic_in_memory_cache_test.cc b/net/tools/quic/quic_in_memory_cache_test.cc index 3e02bcf..1f5e8b6 100644 --- a/net/tools/quic/quic_in_memory_cache_test.cc +++ b/net/tools/quic/quic_in_memory_cache_test.cc @@ -10,6 +10,7 @@ #include "base/stl_util.h" #include "base/strings/string_number_conversions.h" #include "base/strings/string_piece.h" +#include "base/strings/stringprintf.h" #include "net/spdy/spdy_framer.h" #include "net/tools/balsa/balsa_headers.h" #include "net/tools/quic/quic_in_memory_cache.h" @@ -18,12 +19,18 @@ using base::IntToString; using base::StringPiece; +using net::SpdyHeaderBlock; using std::string; namespace net { namespace tools { namespace test { +namespace { +typedef QuicInMemoryCache::Response Response; +typedef QuicInMemoryCache::ServerPushInfo ServerPushInfo; +}; // namespace + class QuicInMemoryCacheTest : public ::testing::Test { protected: QuicInMemoryCacheTest() { @@ -132,6 +139,84 @@ TEST_F(QuicInMemoryCacheTest, DefaultResponse) { EXPECT_EQ("200", response->headers().find(":status")->second); } +TEST_F(QuicInMemoryCacheTest, AddSimpleResponseWithServerPushResources) { + string request_host = "www.foo.com"; + string response_body("hello response"); + const size_t kNumResources = 5; + int NumResources = 5; + list<QuicInMemoryCache::ServerPushInfo> push_resources; + string scheme = "http"; + for (int i = 0; i < NumResources; ++i) { + string path = "/server_push_src" + base::IntToString(i); + string url = scheme + "://" + request_host + path; + GURL resource_url(url); + string body = "This is server push response body for " + path; + SpdyHeaderBlock response_headers; + response_headers[":version"] = "HTTP/1.1"; + response_headers[":status"] = "200"; + response_headers["content-length"] = base::UintToString(body.size()); + push_resources.push_back( + ServerPushInfo(resource_url, response_headers, i, body)); + } + + QuicInMemoryCache* cache = QuicInMemoryCache::GetInstance(); + cache->AddSimpleResponseWithServerPushResources( + request_host, "/", 200, response_body, push_resources); + string request_url = request_host + "/"; + std::list<ServerPushInfo> resources = + cache->GetServerPushResources(request_url); + ASSERT_EQ(kNumResources, resources.size()); + for (const auto& push_resource : push_resources) { + ServerPushInfo resource = resources.front(); + EXPECT_EQ(resource.request_url.spec(), push_resource.request_url.spec()); + EXPECT_EQ(resource.priority, push_resource.priority); + resources.pop_front(); + } +} + +TEST_F(QuicInMemoryCacheTest, GetServerPushResourcesAndPushResponses) { + string request_host = "www.foo.com"; + string response_body("hello response"); + const size_t kNumResources = 4; + int NumResources = 4; + string scheme = "http"; + string push_response_status[kNumResources] = {"200", "200", "301", "404"}; + list<QuicInMemoryCache::ServerPushInfo> push_resources; + for (int i = 0; i < NumResources; ++i) { + string path = "/server_push_src" + base::IntToString(i); + string url = scheme + "://" + request_host + path; + GURL resource_url(url); + string body = "This is server push response body for " + path; + SpdyHeaderBlock response_headers; + response_headers[":version"] = "HTTP/1.1"; + response_headers[":status"] = push_response_status[i]; + response_headers["content-length"] = base::UintToString(body.size()); + push_resources.push_back( + ServerPushInfo(resource_url, response_headers, i, body)); + } + QuicInMemoryCache* cache = QuicInMemoryCache::GetInstance(); + cache->AddSimpleResponseWithServerPushResources( + request_host, "/", 200, response_body, push_resources); + string request_url = request_host + "/"; + std::list<ServerPushInfo> resources = + cache->GetServerPushResources(request_url); + ASSERT_EQ(kNumResources, resources.size()); + int i = 0; + for (const auto& push_resource : push_resources) { + GURL url = resources.front().request_url; + string host = url.host(); + string path = url.path(); + const QuicInMemoryCache::Response* response = + cache->GetResponse(host, path); + ASSERT_TRUE(response); + ASSERT_TRUE(ContainsKey(response->headers(), ":status")); + EXPECT_EQ(push_response_status[i++], + response->headers().find(":status")->second); + EXPECT_EQ(push_resource.body, response->body()); + resources.pop_front(); + } +} + } // namespace test } // namespace tools } // namespace net diff --git a/net/tools/quic/quic_server_session_test.cc b/net/tools/quic/quic_server_session_test.cc index b447ff8..8f8fa51 100644 --- a/net/tools/quic/quic_server_session_test.cc +++ b/net/tools/quic/quic_server_session_test.cc @@ -260,19 +260,13 @@ TEST_P(QuicServerSessionTest, MaxAvailableStreams) { // Establish available streams up to the server's limit. const int kLimitingStreamId = - FLAGS_allow_many_available_streams - ? kClientDataStreamId1 + (kAvailableStreamLimit)*2 + 2 - : kClientDataStreamId1 + (session_->get_max_open_streams() - 1) * 2; + kClientDataStreamId1 + (kAvailableStreamLimit)*2 + 2; EXPECT_TRUE(QuicServerSessionPeer::GetOrCreateDynamicStream( session_.get(), kLimitingStreamId)); // A further available stream will result in connection close. - if (FLAGS_allow_many_available_streams) { - EXPECT_CALL(*connection_, - SendConnectionClose(QUIC_TOO_MANY_AVAILABLE_STREAMS)); - } else { - EXPECT_CALL(*connection_, SendConnectionClose(QUIC_TOO_MANY_OPEN_STREAMS)); - } + EXPECT_CALL(*connection_, + SendConnectionClose(QUIC_TOO_MANY_AVAILABLE_STREAMS)); // This forces stream kLimitingStreamId + 2 to become available, which // violates the quota. EXPECT_FALSE(QuicServerSessionPeer::GetOrCreateDynamicStream( @@ -368,7 +362,7 @@ TEST_P(QuicServerSessionTest, BandwidthEstimates) { max_bandwidth_estimate_timestamp); // Queue up some pending data. session_->MarkConnectionLevelWriteBlocked(kCryptoStreamId, - net::kHighestPriority); + net::kV3HighestPriority); EXPECT_TRUE(session_->HasDataToWrite()); // There will be no update sent yet - not enough time has passed. diff --git a/net/tools/quic/quic_spdy_server_stream.cc b/net/tools/quic/quic_spdy_server_stream.cc index aa1c713..f10d040 100644 --- a/net/tools/quic/quic_spdy_server_stream.cc +++ b/net/tools/quic/quic_spdy_server_stream.cc @@ -152,6 +152,33 @@ void QuicSpdyServerStream::SendResponse() { return; } + // Examing response status, if it was not pure integer as typical h2 response + // status, send error response. + string request_url = request_headers_[":scheme"].as_string() + "://" + + request_headers_[":authority"].as_string() + + request_headers_[":path"].as_string(); + int response_code; + SpdyHeaderBlock response_headers = response->headers(); + if (!base::StringToInt(response_headers[":status"], &response_code)) { + DVLOG(1) << "Illegal (non-integer) response :status from cache: " + << response_headers[":status"].as_string() << " for request " + << request_url; + SendErrorResponse(); + return; + } + + if (id() % 2 == 0) { + // A server initiated stream is only used for a server push response, + // and only 200 and 30X response codes are supported for server push. + // This behavior mirrors the HTTP/2 implementation. + bool is_redirection = response_code / 100 == 3; + if (response_code != 200 && !is_redirection) { + LOG(WARNING) << "Response to server push request " << request_url + << " result in response code " << response_code; + Reset(QUIC_STREAM_CANCELLED); + return; + } + } DVLOG(1) << "Sending response for stream " << id(); SendHeadersAndBody(response->headers(), response->body()); } @@ -170,8 +197,6 @@ void QuicSpdyServerStream::SendHeadersAndBody( // This server only supports SPDY and HTTP, and neither handles bidirectional // streaming. if (!reading_stopped()) { - // If FLAGS_quic_implement_stop_reading is false, - // behaves as ReliableQuicStream::CloseReadSide(). StopReading(); } diff --git a/net/tools/quic/quic_spdy_server_stream_test.cc b/net/tools/quic/quic_spdy_server_stream_test.cc index 3ff6fe7..4798fc5 100644 --- a/net/tools/quic/quic_spdy_server_stream_test.cc +++ b/net/tools/quic/quic_spdy_server_stream_test.cc @@ -108,13 +108,6 @@ class QuicSpdyServerStreamTest : public ::testing::TestWithParam<QuicVersion> { session_.ActivateStream(stream_); QuicInMemoryCachePeer::ResetForTests(); - - string host = ""; - string path = "/foo"; - SpdyHeaderBlock response_headers; - StringPiece body("Yum"); - QuicInMemoryCache::GetInstance()->AddResponse(host, path, response_headers, - body); } ~QuicSpdyServerStreamTest() override { @@ -217,29 +210,89 @@ TEST_P(QuicSpdyServerStreamTest, TestFramingExtraData) { EXPECT_EQ("POST", StreamHeadersValue(":method")); } -TEST_P(QuicSpdyServerStreamTest, TestSendResponse) { +TEST_P(QuicSpdyServerStreamTest, SendResponseWithIllegalResponseStatus) { + // Send a illegal response with response status not supported by HTTP/2. SpdyHeaderBlock* request_headers = stream_->mutable_headers(); - (*request_headers)[":path"] = "/foo"; + (*request_headers)[":path"] = "/bar"; (*request_headers)[":authority"] = ""; (*request_headers)[":version"] = "HTTP/1.1"; (*request_headers)[":method"] = "GET"; response_headers_[":version"] = "HTTP/1.1"; + // HTTP/2 only supports integer responsecode, so "200 OK" is illegal. response_headers_[":status"] = "200 OK"; - response_headers_["content-length"] = "3"; + response_headers_["content-length"] = "5"; + string body = "Yummm"; + QuicInMemoryCache::GetInstance()->AddResponse("", "/bar", response_headers_, + body); + stream_->set_fin_received(true); InSequence s; EXPECT_CALL(session_, WritevData(kHeadersStreamId, _, 0, false, _, nullptr)); - EXPECT_CALL(session_, WritevData(_, _, _, _, _, _)).Times(1). - WillOnce(Return(QuicConsumedData(3, true))); + EXPECT_CALL(session_, WritevData(_, _, _, _, _, _)) + .Times(1) + .WillOnce(Return(QuicConsumedData( + strlen(QuicSpdyServerStream::kErrorResponseBody), true))); QuicSpdyServerStreamPeer::SendResponse(stream_); - if (!FLAGS_quic_implement_stop_reading) { - EXPECT_TRUE(ReliableQuicStreamPeer::read_side_closed(stream_)); - } else { - EXPECT_FALSE(ReliableQuicStreamPeer::read_side_closed(stream_)); - } + EXPECT_FALSE(ReliableQuicStreamPeer::read_side_closed(stream_)); + EXPECT_TRUE(stream_->reading_stopped()); + EXPECT_TRUE(stream_->write_side_closed()); +} + +TEST_P(QuicSpdyServerStreamTest, SendPushResponseWith404Response) { + // Create a new promised stream with even id(). + QuicSpdyServerStreamPeer* promised_stream = + new QuicSpdyServerStreamPeer(2, &session_); + session_.ActivateStream(promised_stream); + + // Send a push response with response status 404, which will be regarded as + // invalid server push response. + SpdyHeaderBlock* request_headers = promised_stream->mutable_headers(); + (*request_headers)[":path"] = "/bar"; + (*request_headers)[":authority"] = ""; + (*request_headers)[":version"] = "HTTP/1.1"; + (*request_headers)[":method"] = "GET"; + + response_headers_[":version"] = "HTTP/1.1"; + response_headers_[":status"] = "404"; + response_headers_["content-length"] = "8"; + string body = "NotFound"; + QuicInMemoryCache::GetInstance()->AddResponse("", "/bar", response_headers_, + body); + + InSequence s; + EXPECT_CALL(session_, + SendRstStream(promised_stream->id(), QUIC_STREAM_CANCELLED, 0)); + + QuicSpdyServerStreamPeer::SendResponse(promised_stream); +} + +TEST_P(QuicSpdyServerStreamTest, SendResponseWithValidHeaders) { + // Add a request and response with valid headers. + SpdyHeaderBlock* request_headers = stream_->mutable_headers(); + (*request_headers)[":path"] = "/bar"; + (*request_headers)[":authority"] = ""; + (*request_headers)[":version"] = "HTTP/1.1"; + (*request_headers)[":method"] = "GET"; + + response_headers_[":version"] = "HTTP/1.1"; + response_headers_[":status"] = "200"; + response_headers_["content-length"] = "5"; + string body = "Yummm"; + QuicInMemoryCache::GetInstance()->AddResponse("", "/bar", response_headers_, + body); + stream_->set_fin_received(true); + + InSequence s; + EXPECT_CALL(session_, WritevData(kHeadersStreamId, _, 0, false, _, nullptr)); + EXPECT_CALL(session_, WritevData(_, _, _, _, _, _)) + .Times(1) + .WillOnce(Return(QuicConsumedData(body.length(), true))); + + QuicSpdyServerStreamPeer::SendResponse(stream_); + EXPECT_FALSE(ReliableQuicStreamPeer::read_side_closed(stream_)); EXPECT_TRUE(stream_->reading_stopped()); EXPECT_TRUE(stream_->write_side_closed()); } @@ -258,11 +311,7 @@ TEST_P(QuicSpdyServerStreamTest, TestSendErrorResponse) { WillOnce(Return(QuicConsumedData(3, true))); QuicSpdyServerStreamPeer::SendErrorResponse(stream_); - if (!FLAGS_quic_implement_stop_reading) { - EXPECT_TRUE(ReliableQuicStreamPeer::read_side_closed(stream_)); - } else { - EXPECT_FALSE(ReliableQuicStreamPeer::read_side_closed(stream_)); - } + EXPECT_FALSE(ReliableQuicStreamPeer::read_side_closed(stream_)); EXPECT_TRUE(stream_->reading_stopped()); EXPECT_TRUE(stream_->write_side_closed()); } |