summaryrefslogtreecommitdiffstats
path: root/net/quic
diff options
context:
space:
mode:
authorrtenneti@chromium.org <rtenneti@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98>2013-10-09 23:01:09 +0000
committerrtenneti@chromium.org <rtenneti@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98>2013-10-09 23:01:09 +0000
commit994f02e416891be7d0c2c6aed465fa685bdc8347 (patch)
treeff18c11a3599818e2647367be388f214e6e9f94a /net/quic
parent514b1fc954d0bbe50addb402a94355c41aaf6c70 (diff)
downloadchromium_src-994f02e416891be7d0c2c6aed465fa685bdc8347.zip
chromium_src-994f02e416891be7d0c2c6aed465fa685bdc8347.tar.gz
chromium_src-994f02e416891be7d0c2c6aed465fa685bdc8347.tar.bz2
Land Recent QUIC changes.
Addressing comments in Jana's review of cr/52231261. Merge internal change: 53582401 Add a temporary legacy quic constructor Merge internal change: 53506960 QUIC: disable P-256 support on the server. The P-256 key generation is done with OpenSSL, which doesn't use the QuicRandom passed to DefaultConfig(). This is causing the generated server configs to be non-deterministic and breaking 0-RTT handshakes. Merge internal change: 53501783 Fix an LOG to use the correct condition in QuicReceivedPacketManager and change it to a DFATAL so in the future tests will prevent re-occurrence. Merge internal change: 53483753 Cleanup: Rename OnIncomingAck to OnPacketAcked, and remove unneeeded argument from SentPacketManager::OnIncomingAck. Merge internal change: 53483155 Fix a bug in QuicConnection/QuicConnectionHelper if the helper buffered the write (as is the case in chrome). In this case, the sent packet was not accounted for properly. Merge internal change: 53462749 Refactor to change WritePacket to return a WriteResult struct. Merge internal change: 53382221 Fixing a bug where the version negotiation packet would get dropped on the floor if the socket was write blocked at the time it was sent. The packet is now queued. Merge internal change: 53317846 Create a new QUIC_INVALID_CHANNEL_ID_SIGNATURE error to replace a usage of QUIC_INTERNAL_ERROR. Merge internal change: 53277933 Added a todo to merge internal CL 53267501 when chromium's version of OpenSSL has latest AEAD code. Didn't merge internal change: 53267501 R=rch@chromium.org Review URL: https://codereview.chromium.org/26385004 git-svn-id: svn://svn.chromium.org/chrome/trunk/src@227827 0039d316-1c4b-4281-b951-d872f2087c98
Diffstat (limited to 'net/quic')
-rw-r--r--net/quic/crypto/aes_128_gcm_12_decrypter.h2
-rw-r--r--net/quic/crypto/aes_128_gcm_12_encrypter.h2
-rw-r--r--net/quic/crypto/crypto_handshake.cc2
-rw-r--r--net/quic/crypto/crypto_server_config.cc29
-rw-r--r--net/quic/crypto/crypto_server_config.h5
-rw-r--r--net/quic/crypto/crypto_server_test.cc19
-rw-r--r--net/quic/quic_ack_notifier_manager.cc52
-rw-r--r--net/quic/quic_ack_notifier_manager.h9
-rw-r--r--net/quic/quic_connection.cc193
-rw-r--r--net/quic/quic_connection.h46
-rw-r--r--net/quic/quic_connection_helper.cc21
-rw-r--r--net/quic/quic_connection_helper.h4
-rw-r--r--net/quic/quic_connection_helper_test.cc13
-rw-r--r--net/quic/quic_connection_logger.cc10
-rw-r--r--net/quic/quic_connection_logger.h3
-rw-r--r--net/quic/quic_connection_test.cc76
-rw-r--r--net/quic/quic_protocol.h24
-rw-r--r--net/quic/quic_received_packet_manager.cc10
-rw-r--r--net/quic/quic_sent_packet_manager.cc28
-rw-r--r--net/quic/quic_sent_packet_manager.h9
-rw-r--r--net/quic/quic_sent_packet_manager_test.cc39
-rw-r--r--net/quic/quic_utils.cc1
-rw-r--r--net/quic/test_tools/quic_test_utils.h4
23 files changed, 383 insertions, 218 deletions
diff --git a/net/quic/crypto/aes_128_gcm_12_decrypter.h b/net/quic/crypto/aes_128_gcm_12_decrypter.h
index 5dc12c8..aba610f 100644
--- a/net/quic/crypto/aes_128_gcm_12_decrypter.h
+++ b/net/quic/crypto/aes_128_gcm_12_decrypter.h
@@ -60,6 +60,8 @@ class NET_EXPORT_PRIVATE Aes128Gcm12Decrypter : public QuicDecrypter {
unsigned char nonce_prefix_[4];
#if defined(USE_OPENSSL)
+ // TODO(rtenneti): when Chromium's version of OpenSSL has EVP_AEAD_CTX, merge
+ // internal CL 53267501.
ScopedEVPCipherCtx ctx_;
#endif
};
diff --git a/net/quic/crypto/aes_128_gcm_12_encrypter.h b/net/quic/crypto/aes_128_gcm_12_encrypter.h
index ca9a2b1..abea8ac2 100644
--- a/net/quic/crypto/aes_128_gcm_12_encrypter.h
+++ b/net/quic/crypto/aes_128_gcm_12_encrypter.h
@@ -63,6 +63,8 @@ class NET_EXPORT_PRIVATE Aes128Gcm12Encrypter : public QuicEncrypter {
unsigned char nonce_prefix_[4];
#if defined(USE_OPENSSL)
+ // TODO(rtenneti): when Chromium's version of OpenSSL has EVP_AEAD_CTX, merge
+ // internal CL 53267501.
ScopedEVPCipherCtx ctx_;
#endif
};
diff --git a/net/quic/crypto/crypto_handshake.cc b/net/quic/crypto/crypto_handshake.cc
index 395f4ae..f5c7f4d 100644
--- a/net/quic/crypto/crypto_handshake.cc
+++ b/net/quic/crypto/crypto_handshake.cc
@@ -752,7 +752,7 @@ QuicErrorCode QuicCryptoClientConfig::FillClientHello(
if (!channel_id_signer_->Sign(server_hostname, hkdf_input,
&key, &signature)) {
*error_details = "Channel ID signature failed";
- return QUIC_INTERNAL_ERROR;
+ return QUIC_INVALID_CHANNEL_ID_SIGNATURE;
}
cetv.SetStringPiece(kCIDK, key);
diff --git a/net/quic/crypto/crypto_server_config.cc b/net/quic/crypto/crypto_server_config.cc
index 89cea42..7c7d0ff 100644
--- a/net/quic/crypto/crypto_server_config.cc
+++ b/net/quic/crypto/crypto_server_config.cc
@@ -46,7 +46,8 @@ const char QuicCryptoServerConfig::TESTING[] = "secret string for testing";
QuicCryptoServerConfig::ConfigOptions::ConfigOptions()
: expiry_time(QuicWallTime::Zero()),
- channel_id_enabled(false) { }
+ channel_id_enabled(false),
+ p256(false) {}
QuicCryptoServerConfig::QuicCryptoServerConfig(
StringPiece source_address_token_secret,
@@ -97,9 +98,14 @@ QuicServerConfigProtobuf* QuicCryptoServerConfig::DefaultConfig(
Curve25519KeyExchange::New(curve25519_private_key));
StringPiece curve25519_public_value = curve25519->public_value();
- const string p256_private_key = P256KeyExchange::NewPrivateKey();
- scoped_ptr<P256KeyExchange> p256(P256KeyExchange::New(p256_private_key));
- StringPiece p256_public_value = p256->public_value();
+ string p256_private_key;
+ StringPiece p256_public_value;
+ scoped_ptr<P256KeyExchange> p256;
+ if (options.p256) {
+ p256_private_key = P256KeyExchange::NewPrivateKey();
+ p256.reset(P256KeyExchange::New(p256_private_key));
+ p256_public_value = p256->public_value();
+ }
string encoded_public_values;
// First three bytes encode the length of the public value.
@@ -115,7 +121,11 @@ QuicServerConfigProtobuf* QuicCryptoServerConfig::DefaultConfig(
p256_public_value.size());
msg.set_tag(kSCFG);
- msg.SetTaglist(kKEXS, kC255, kP256, 0);
+ if (options.p256) {
+ msg.SetTaglist(kKEXS, kC255, kP256, 0);
+ } else {
+ msg.SetTaglist(kKEXS, kC255, 0);
+ }
msg.SetTaglist(kAEAD, kAESG, 0);
msg.SetValue(kVERS, static_cast<uint16>(0));
msg.SetStringPiece(kPUBS, encoded_public_values);
@@ -158,9 +168,12 @@ QuicServerConfigProtobuf* QuicCryptoServerConfig::DefaultConfig(
QuicServerConfigProtobuf::PrivateKey* curve25519_key = config->add_key();
curve25519_key->set_tag(kC255);
curve25519_key->set_private_key(curve25519_private_key);
- QuicServerConfigProtobuf::PrivateKey* p256_key = config->add_key();
- p256_key->set_tag(kP256);
- p256_key->set_private_key(p256_private_key);
+
+ if (options.p256) {
+ QuicServerConfigProtobuf::PrivateKey* p256_key = config->add_key();
+ p256_key->set_tag(kP256);
+ p256_key->set_private_key(p256_private_key);
+ }
return config.release();
}
diff --git a/net/quic/crypto/crypto_server_config.h b/net/quic/crypto/crypto_server_config.h
index 4255d22..4551425 100644
--- a/net/quic/crypto/crypto_server_config.h
+++ b/net/quic/crypto/crypto_server_config.h
@@ -61,6 +61,11 @@ class NET_EXPORT_PRIVATE QuicCryptoServerConfig {
// orbit contains the kOrbitSize bytes of the orbit value for the server
// config. If |orbit| is empty then a random orbit is generated.
std::string orbit;
+ // p256 determines whether a P-256 public key will be included in the
+ // server config. Note that this breaks deterministic server-config
+ // generation since P-256 key generation doesn't use the QuicRandom given
+ // to DefaultConfig().
+ bool p256;
};
// |source_address_token_secret|: secret key material used for encrypting and
diff --git a/net/quic/crypto/crypto_server_test.cc b/net/quic/crypto/crypto_server_test.cc
index b2cdf82..348b31a 100644
--- a/net/quic/crypto/crypto_server_test.cc
+++ b/net/quic/crypto/crypto_server_test.cc
@@ -8,6 +8,7 @@
#include "net/quic/crypto/quic_random.h"
#include "net/quic/test_tools/crypto_test_utils.h"
#include "net/quic/test_tools/mock_clock.h"
+#include "net/quic/test_tools/mock_random.h"
#include "testing/gtest/include/gtest/gtest.h"
using base::StringPiece;
@@ -239,6 +240,24 @@ TEST_F(CryptoServerTest, ReplayProtection) {
ASSERT_EQ(kSHLO, out_.tag());
}
+TEST(CryptoServerConfigGenerationTest, Determinism) {
+ // Test that using a deterministic PRNG causes the server-config to be
+ // deterministic.
+
+ MockRandom rand_a, rand_b;
+ const QuicCryptoServerConfig::ConfigOptions options;
+ MockClock clock;
+
+ QuicCryptoServerConfig a(QuicCryptoServerConfig::TESTING, &rand_a);
+ QuicCryptoServerConfig b(QuicCryptoServerConfig::TESTING, &rand_b);
+ scoped_ptr<CryptoHandshakeMessage> scfg_a(
+ a.AddDefaultConfig(&rand_a, &clock, options));
+ scoped_ptr<CryptoHandshakeMessage> scfg_b(
+ b.AddDefaultConfig(&rand_b, &clock, options));
+
+ ASSERT_EQ(scfg_a->DebugString(), scfg_b->DebugString());
+}
+
class CryptoServerTestNoConfig : public CryptoServerTest {
public:
virtual void SetUp() {
diff --git a/net/quic/quic_ack_notifier_manager.cc b/net/quic/quic_ack_notifier_manager.cc
index 901d9d6..2221e30 100644
--- a/net/quic/quic_ack_notifier_manager.cc
+++ b/net/quic/quic_ack_notifier_manager.cc
@@ -22,40 +22,32 @@ AckNotifierManager::~AckNotifierManager() {
STLDeleteElements(&ack_notifiers_);
}
-void AckNotifierManager::OnIncomingAck(const SequenceNumberSet& acked_packets) {
- // Inform all the registered AckNotifiers of the new ACKs.
- for (SequenceNumberSet::const_iterator seq_it = acked_packets.begin();
- seq_it != acked_packets.end(); ++seq_it) {
- AckNotifierMap::iterator map_it = ack_notifier_map_.find(*seq_it);
- if (map_it == ack_notifier_map_.end()) {
- // No AckNotifier is interested in this sequence number.
- continue;
- }
-
- // One or more AckNotifiers are registered as interested in this sequence
- // number. Iterate through them and call OnAck on each.
- for (AckNotifierSet::iterator set_it = map_it->second.begin();
- set_it != map_it->second.end(); ++set_it) {
- (*set_it)->OnAck(*seq_it);
- }
-
- // Remove the sequence number from the map as we have notified all the
- // registered AckNotifiers, and we won't see it again.
- ack_notifier_map_.erase(map_it);
+void AckNotifierManager::OnPacketAcked(
+ QuicPacketSequenceNumber sequence_number) {
+ // Inform all the registered AckNotifiers of the new ACK.
+ AckNotifierMap::iterator map_it = ack_notifier_map_.find(sequence_number);
+ if (map_it == ack_notifier_map_.end()) {
+ // No AckNotifier is interested in this sequence number.
+ return;
}
- // Clear up any empty AckNotifiers
- AckNotifierSet::iterator it = ack_notifiers_.begin();
- while (it != ack_notifiers_.end()) {
- if ((*it)->IsEmpty()) {
- // The QuicAckNotifier has seen all the ACKs it was interested in, and
- // has triggered its callback. No more use for it.
- delete *it;
- ack_notifiers_.erase(it++);
- } else {
- ++it;
+ // One or more AckNotifiers are registered as interested in this sequence
+ // number. Iterate through them and call OnAck on each.
+ for (AckNotifierSet::iterator set_it = map_it->second.begin();
+ set_it != map_it->second.end(); ++set_it) {
+ QuicAckNotifier* ack_notifier = *set_it;
+ ack_notifier->OnAck(sequence_number);
+
+ // If this has resulted in an empty AckNotifer, erase it.
+ if (ack_notifier->IsEmpty()) {
+ delete ack_notifier;
+ ack_notifiers_.erase(ack_notifier);
}
}
+
+ // Remove the sequence number from the map as we have notified all the
+ // registered AckNotifiers, and we won't see it again.
+ ack_notifier_map_.erase(map_it);
}
void AckNotifierManager::UpdateSequenceNumber(
diff --git a/net/quic/quic_ack_notifier_manager.h b/net/quic/quic_ack_notifier_manager.h
index 7fe9a46..5ddf794 100644
--- a/net/quic/quic_ack_notifier_manager.h
+++ b/net/quic/quic_ack_notifier_manager.h
@@ -37,11 +37,10 @@ class NET_EXPORT_PRIVATE AckNotifierManager {
AckNotifierManager();
virtual ~AckNotifierManager();
- // Called when the connection receives a new AckFrame. For each packet
- // in |acked_packets|, if the packet sequence number exists in
- // ack_notifier_map_ then the corresponding AckNotifiers will have their OnAck
- // method called.
- void OnIncomingAck(const SequenceNumberSet& acked_packets);
+ // Called when the connection receives a new AckFrame. If |sequence_number|
+ // exists in ack_notifier_map_ then the corresponding AckNotifiers will have
+ // their OnAck method called.
+ void OnPacketAcked(QuicPacketSequenceNumber sequence_number);
// If a packet has been retransmitted with a new sequence number, then this
// will be called. It updates the mapping in ack_notifier_map_, and also
diff --git a/net/quic/quic_connection.cc b/net/quic/quic_connection.cc
index 59759f7..2073919 100644
--- a/net/quic/quic_connection.cc
+++ b/net/quic/quic_connection.cc
@@ -52,8 +52,9 @@ namespace {
// This will likely have to be tuned.
const QuicPacketSequenceNumber kMaxPacketGap = 5000;
-// We want to make sure if we get a large nack packet, we don't queue up too
-// many packets at once. 10 is a common default initial congestion window.
+// We want to make sure if we get a nack packet which triggers several
+// retransmissions, we don't queue up too many packets. 10 is TCP's default
+// initial congestion window(RFC 6928).
const size_t kMaxRetransmissionsPerAck = 10;
// TCP retransmits after 2 nacks. We allow for a third in case of out-of-order
@@ -215,6 +216,7 @@ QuicConnection::QuicConnection(QuicGuid guid,
guid_(guid),
peer_address_(address),
largest_seen_packet_with_ack_(0),
+ pending_version_negotiation_packet_(false),
write_blocked_(false),
ack_alarm_(helper->CreateAlarm(new AckAlarm(this))),
retransmission_alarm_(helper->CreateAlarm(new RetransmissionAlarm(this))),
@@ -504,11 +506,15 @@ bool QuicConnection::OnAckFrame(const QuicAckFrame& incoming_ack) {
}
void QuicConnection::ProcessAckFrame(const QuicAckFrame& incoming_ack) {
+ // Latch current least unacked sequence number. This allows us to reset the
+ // retransmission and FEC abandonment timers conditionally below.
+ QuicPacketSequenceNumber least_unacked_sent_before =
+ sent_packet_manager_.GetLeastUnackedSentPacket();
+
largest_seen_packet_with_ack_ = last_header_.packet_sequence_number;
- received_truncated_ack_ =
- incoming_ack.received_info.missing_packets.size() >=
- QuicFramer::GetMaxUnackedPackets(last_header_);
+ received_truncated_ack_ = incoming_ack.received_info.missing_packets.size() >=
+ QuicFramer::GetMaxUnackedPackets(last_header_);
received_packet_manager_.UpdatePacketInformationReceivedByPeer(incoming_ack);
received_packet_manager_.UpdatePacketInformationSentByPeer(incoming_ack);
@@ -519,35 +525,42 @@ void QuicConnection::ProcessAckFrame(const QuicAckFrame& incoming_ack) {
received_packet_manager_.least_packet_awaited_by_peer() - 1);
retransmitted_nacked_packet_count_ = 0;
- SequenceNumberSet acked_packets;
- sent_packet_manager_.OnIncomingAck(
- incoming_ack.received_info, received_truncated_ack_, &acked_packets);
- if (acked_packets.size() > 0) {
- // Reset the RTO timeout for each packet when an ack is received.
- if (retransmission_alarm_->IsSet()) {
- retransmission_alarm_->Cancel();
- // Only reschedule the timer if there are outstanding packets.
- if (sent_packet_manager_.HasUnackedPackets()) {
- QuicTime::Delta retransmission_delay =
- congestion_manager_.GetRetransmissionDelay(
- sent_packet_manager_.GetNumUnackedPackets(), 0);
- retransmission_alarm_->Set(clock_->ApproximateNow().Add(
- retransmission_delay));
- }
- }
- if (abandon_fec_alarm_->IsSet()) {
- abandon_fec_alarm_->Cancel();
- // Only reschedule the timer if there are outstanding fec packets.
- if (sent_packet_manager_.HasUnackedFecPackets()) {
- QuicTime::Delta retransmission_delay =
- congestion_manager_.GetRetransmissionDelay(
- sent_packet_manager_.GetNumUnackedPackets(), 0);
- abandon_fec_alarm_->Set(clock_->ApproximateNow().Add(
- retransmission_delay));
- }
- }
+ sent_packet_manager_.OnIncomingAck(incoming_ack.received_info,
+ received_truncated_ack_);
+
+ // Get the updated least unacked sequence number.
+ QuicPacketSequenceNumber least_unacked_sent_after =
+ sent_packet_manager_.GetLeastUnackedSentPacket();
+
+ // Used to set RTO and FEC alarms.
+ QuicTime::Delta retransmission_delay =
+ congestion_manager_.GetRetransmissionDelay(
+ sent_packet_manager_.GetNumUnackedPackets(), 0);
+
+ // If there are outstanding packets, and the least unacked sequence number
+ // has increased after processing this latest AckFrame, then reschedule the
+ // retransmission timer.
+ if (sent_packet_manager_.HasUnackedPackets() &&
+ least_unacked_sent_before < least_unacked_sent_after) {
+ DCHECK(retransmission_alarm_->IsSet());
+
+ retransmission_alarm_->Cancel();
+ retransmission_alarm_->Set(
+ clock_->ApproximateNow().Add(retransmission_delay));
consecutive_rto_count_ = 0;
+ } else if (!sent_packet_manager_.HasUnackedPackets()) {
+ retransmission_alarm_->Cancel();
}
+
+ // If there are outstanding FEC packets, and the least unacked sequence number
+ // has increased after processing this latest AckFrame, then reschedule the
+ // abandon FEC timer.
+ abandon_fec_alarm_->Cancel();
+ if (sent_packet_manager_.HasUnackedFecPackets() &&
+ least_unacked_sent_before < least_unacked_sent_after) {
+ abandon_fec_alarm_->Set(clock_->ApproximateNow().Add(retransmission_delay));
+ }
+
congestion_manager_.OnIncomingAckFrame(incoming_ack,
time_of_last_received_packet_);
}
@@ -740,7 +753,6 @@ void QuicConnection::OnPacketComplete() {
MaybeSendInResponseToPacket(send_ack_immediately,
last_packet_should_instigate_ack);
-
ClearLastFrames();
}
@@ -833,12 +845,25 @@ void QuicConnection::SendVersionNegotiationPacket() {
for (size_t i = 0; i < arraysize(kSupportedQuicVersions); ++i) {
supported_versions.push_back(kSupportedQuicVersions[i]);
}
- QuicEncryptedPacket* encrypted =
- packet_creator_.SerializeVersionNegotiationPacket(supported_versions);
+ scoped_ptr<QuicEncryptedPacket> version_packet(
+ packet_creator_.SerializeVersionNegotiationPacket(supported_versions));
// TODO(satyamshekhar): implement zero server state negotiation.
- int error;
- helper_->WritePacketToWire(*encrypted, &error);
- delete encrypted;
+ WriteResult result =
+ helper_->WritePacketToWire(*version_packet);
+ if (result.status == WRITE_STATUS_OK ||
+ (result.status == WRITE_STATUS_BLOCKED &&
+ helper_->IsWriteBlockedDataBuffered())) {
+ pending_version_negotiation_packet_ = false;
+ return;
+ }
+ if (result.status == WRITE_STATUS_ERROR) {
+ // We can't send an error as the socket is presumably borked.
+ CloseConnection(QUIC_PACKET_WRITE_ERROR, false);
+ }
+ if (result.status == WRITE_STATUS_BLOCKED) {
+ write_blocked_ = true;
+ }
+ pending_version_negotiation_packet_ = true;
}
QuicConsumedData QuicConnection::SendvStreamDataInner(
@@ -1049,6 +1074,10 @@ bool QuicConnection::ProcessValidatedPacket() {
bool QuicConnection::WriteQueuedPackets() {
DCHECK(!write_blocked_);
+ if (pending_version_negotiation_packet_) {
+ SendVersionNegotiationPacket();
+ }
+
QueuedPacketList::iterator packet_iterator = queued_packets_.begin();
while (!write_blocked_ && packet_iterator != queued_packets_.end()) {
if (WritePacket(packet_iterator->encryption_level,
@@ -1203,9 +1232,8 @@ void QuicConnection::SetupRetransmission(
return;
}
- // Do not set the retransmission alarm if we're already handling the
- // retransmission alarm because the retransmission alarm will be reset when
- // OnRetransmissionTimeout completes.
+ // Do not set the retransmission alarm if we're already handling one, since
+ // it will be reset when OnRetransmissionTimeout completes.
if (retransmission_alarm_->IsSet()) {
return;
}
@@ -1338,27 +1366,51 @@ bool QuicConnection::WritePacket(EncryptionLevel level,
<< packet->length() << " " << encrypted->length() << " "
<< " forced: " << (forced == FORCE ? "yes" : "no");
- int error;
- QuicTime now = clock_->Now();
- if (WritePacketToWire(sequence_number, level, *encrypted, &error) == -1) {
- if (helper_->IsWriteBlocked(error)) {
- // TODO(satyashekhar): It might be more efficient (fewer system calls), if
- // all connections share this variable i.e this becomes a part of
- // PacketWriterInterface.
- write_blocked_ = true;
- // If the socket buffers the the data, then the packet should not
- // be queued and sent again, which would result in an unnecessary
- // duplicate packet being sent.
- if (helper_->IsWriteBlockedDataBuffered()) {
- delete packet;
- return true;
- }
- return false;
+ DCHECK(pending_write_.get() == NULL);
+ pending_write_.reset(new PendingWrite(sequence_number, transmission_type,
+ retransmittable, level, packet));
+
+ WriteResult result = WritePacketToWire(sequence_number, level, *encrypted);
+ if (result.status == WRITE_STATUS_BLOCKED) {
+ // TODO(satyashekhar): It might be more efficient (fewer system calls), if
+ // all connections share this variable i.e this becomes a part of
+ // PacketWriterInterface.
+ write_blocked_ = true;
+ // If the socket buffers the the data, then the packet should not
+ // be queued and sent again, which would result in an unnecessary
+ // duplicate packet being sent. The helper must call OnPacketSent
+ // when the packet is actually sent.
+ if (helper_->IsWriteBlockedDataBuffered()) {
+ return true;
}
+ pending_write_.reset();
+ return false;
+ }
+
+ return OnPacketSent(result);
+}
+
+bool QuicConnection::OnPacketSent(WriteResult result) {
+ DCHECK_NE(WRITE_STATUS_BLOCKED, result.status);
+ if (pending_write_.get() == NULL) {
+ LOG(DFATAL) << "OnPacketSent called without a pending write.";
+ return false;
+ }
+
+ QuicPacketSequenceNumber sequence_number = pending_write_->sequence_number;
+ TransmissionType transmission_type = pending_write_->transmission_type;
+ HasRetransmittableData retransmittable = pending_write_->retransmittable;
+ EncryptionLevel level = pending_write_->level;
+ QuicPacket* packet = pending_write_->packet;
+ pending_write_.reset();
+
+ if (result.status == WRITE_STATUS_ERROR) {
// We can't send an error as the socket is presumably borked.
CloseConnection(QUIC_PACKET_WRITE_ERROR, false);
return false;
}
+
+ QuicTime now = clock_->Now();
if (transmission_type == NOT_RETRANSMISSION) {
time_of_last_sent_packet_ = now;
}
@@ -1384,12 +1436,12 @@ bool QuicConnection::WritePacket(EncryptionLevel level,
congestion_manager_.OnPacketSent(sequence_number, now, packet->length(),
transmission_type, retransmittable);
- stats_.bytes_sent += encrypted->length();
+ stats_.bytes_sent += result.bytes_written;
++stats_.packets_sent;
if (transmission_type == NACK_RETRANSMISSION ||
transmission_type == RTO_RETRANSMISSION) {
- stats_.bytes_retransmitted += encrypted->length();
+ stats_.bytes_retransmitted += result.bytes_written;
++stats_.packets_retransmitted;
}
@@ -1397,18 +1449,16 @@ bool QuicConnection::WritePacket(EncryptionLevel level,
return true;
}
-int QuicConnection::WritePacketToWire(QuicPacketSequenceNumber sequence_number,
- EncryptionLevel level,
- const QuicEncryptedPacket& packet,
- int* error) {
- int bytes_written = helper_->WritePacketToWire(packet, error);
+WriteResult QuicConnection::WritePacketToWire(
+ QuicPacketSequenceNumber sequence_number,
+ EncryptionLevel level,
+ const QuicEncryptedPacket& packet) {
+ WriteResult result = helper_->WritePacketToWire(packet);
if (debug_visitor_) {
- // WritePacketToWire returned -1, then |error| will be populated with
- // an error code, which we want to pass along to the visitor.
- debug_visitor_->OnPacketSent(sequence_number, level, packet,
- bytes_written == -1 ? *error : bytes_written);
+ // Pass the write result to the visitor.
+ debug_visitor_->OnPacketSent(sequence_number, level, packet, result);
}
- return bytes_written;
+ return result;
}
bool QuicConnection::OnSerializedPacket(
@@ -1506,8 +1556,6 @@ void QuicConnection::OnRetransmissionTimeout() {
return;
}
- // TODO(ianswett): When multiple RTO's fire, the subsequent RTO should send
- // the same data packet as the first RTO according to the TCP spec.
// TODO(ianswett): When an RTO fires, but the connection has not been
// established as forward secure, re-send the client hello first.
++stats_.rto_count;
@@ -1765,7 +1813,8 @@ void QuicConnection::Flush() {
}
bool QuicConnection::HasQueuedData() const {
- return !queued_packets_.empty() || packet_generator_.HasQueuedFrames();
+ return pending_version_negotiation_packet_ ||
+ !queued_packets_.empty() || packet_generator_.HasQueuedFrames();
}
void QuicConnection::SetIdleNetworkTimeout(QuicTime::Delta timeout) {
diff --git a/net/quic/quic_connection.h b/net/quic/quic_connection.h
index 2f20351..2eb7873 100644
--- a/net/quic/quic_connection.h
+++ b/net/quic/quic_connection.h
@@ -104,7 +104,7 @@ class NET_EXPORT_PRIVATE QuicConnectionDebugVisitorInterface
virtual void OnPacketSent(QuicPacketSequenceNumber sequence_number,
EncryptionLevel level,
const QuicEncryptedPacket& packet,
- int rv) = 0;
+ WriteResult result) = 0;
// Called when the contents of a packet have been retransmitted as
// a new packet.
@@ -171,10 +171,10 @@ class NET_EXPORT_PRIVATE QuicConnectionHelperInterface {
// Sends the packet out to the peer, possibly simulating packet
// loss if FLAGS_fake_packet_loss_percentage is set. If the write
- // succeeded, returns the number of bytes written. If the write
- // failed, returns -1 and the error code will be copied to |*error|.
- virtual int WritePacketToWire(const QuicEncryptedPacket& packet,
- int* error) = 0;
+ // succeeded, the result's status is WRITE_STATUS_OK and bytes_written is
+ // populated. If the write failed, the result's status is WRITE_STATUS_BLOCKED
+ // or WRITE_STATUS_ERROR and error_code will populated.
+ virtual WriteResult WritePacketToWire(const QuicEncryptedPacket& packet) = 0;
// Returns true if the helper buffers and subsequently rewrites data
// when an attempt to write results in the underlying socket becoming
@@ -274,6 +274,9 @@ class NET_EXPORT_PRIVATE QuicConnection
// writes to happen. Returns false if the socket has become blocked.
virtual bool OnCanWrite() OVERRIDE;
+ // Called when a packet has been finally sent to the network.
+ bool OnPacketSent(WriteResult result);
+
// If the socket is not blocked, this allows queued writes to happen. Returns
// false if the socket has become blocked.
bool WriteIfNotBlocked();
@@ -455,10 +458,9 @@ class NET_EXPORT_PRIVATE QuicConnection
HasRetransmittableData retransmittable,
Force force);
- int WritePacketToWire(QuicPacketSequenceNumber sequence_number,
- EncryptionLevel level,
- const QuicEncryptedPacket& packet,
- int* error);
+ WriteResult WritePacketToWire(QuicPacketSequenceNumber sequence_number,
+ EncryptionLevel level,
+ const QuicEncryptedPacket& packet);
// Make sure an ack we got from our peer is sane.
bool ValidateAckFrame(const QuicAckFrame& incoming_ack);
@@ -573,6 +575,25 @@ class NET_EXPORT_PRIVATE QuicConnection
bool for_fec;
};
+ struct PendingWrite {
+ PendingWrite(QuicPacketSequenceNumber sequence_number,
+ TransmissionType transmission_type,
+ HasRetransmittableData retransmittable,
+ EncryptionLevel level,
+ QuicPacket* packet)
+ : sequence_number(sequence_number),
+ transmission_type(transmission_type),
+ retransmittable(retransmittable),
+ level(level),
+ packet(packet) { }
+
+ QuicPacketSequenceNumber sequence_number;
+ TransmissionType transmission_type;
+ HasRetransmittableData retransmittable;
+ EncryptionLevel level;
+ QuicPacket* packet;
+ };
+
class RetransmissionTimeComparator {
public:
bool operator()(const RetransmissionTime& lhs,
@@ -700,11 +721,18 @@ class NET_EXPORT_PRIVATE QuicConnection
// sent with the INITIAL encryption and the CHLO message was lost.
std::deque<QuicEncryptedPacket*> undecryptable_packets_;
+ // When the version negotiation packet could not be sent because the socket
+ // was not writable, this is set to true.
+ bool pending_version_negotiation_packet_;
+
// When packets could not be sent because the socket was not writable,
// they are added to this list. All corresponding frames are in
// unacked_packets_ if they are to be retransmitted.
QueuedPacketList queued_packets_;
+ // Contains information about the current write in progress, if any.
+ scoped_ptr<PendingWrite> pending_write_;
+
// True when the socket becomes unwritable.
bool write_blocked_;
diff --git a/net/quic/quic_connection_helper.cc b/net/quic/quic_connection_helper.cc
index 703151b..1de5013 100644
--- a/net/quic/quic_connection_helper.cc
+++ b/net/quic/quic_connection_helper.cc
@@ -107,13 +107,11 @@ QuicRandom* QuicConnectionHelper::GetRandomGenerator() {
return random_generator_;
}
-int QuicConnectionHelper::WritePacketToWire(
- const QuicEncryptedPacket& packet,
- int* error) {
+WriteResult QuicConnectionHelper::WritePacketToWire(
+ const QuicEncryptedPacket& packet) {
if (connection_->ShouldSimulateLostPacket()) {
DLOG(INFO) << "Dropping packet due to fake packet loss.";
- *error = 0;
- return packet.length();
+ return WriteResult(WRITE_STATUS_OK, packet.length());
}
scoped_refptr<StringIOBuffer> buf(
@@ -123,16 +121,17 @@ int QuicConnectionHelper::WritePacketToWire(
packet.length(),
base::Bind(&QuicConnectionHelper::OnWriteComplete,
weak_factory_.GetWeakPtr()));
- if (rv >= 0) {
- *error = 0;
- } else {
+ WriteStatus status = WRITE_STATUS_OK;
+ if (rv < 0) {
if (rv != ERR_IO_PENDING) {
UMA_HISTOGRAM_SPARSE_SLOWLY("Net.QuicSession.WriteError", -rv);
+ status = WRITE_STATUS_ERROR;
+ } else {
+ status = WRITE_STATUS_BLOCKED;
}
- *error = rv;
- rv = -1;
}
- return rv;
+
+ return WriteResult(status, rv);
}
bool QuicConnectionHelper::IsWriteBlockedDataBuffered() {
diff --git a/net/quic/quic_connection_helper.h b/net/quic/quic_connection_helper.h
index 6667b95..e530176 100644
--- a/net/quic/quic_connection_helper.h
+++ b/net/quic/quic_connection_helper.h
@@ -45,8 +45,8 @@ class NET_EXPORT_PRIVATE QuicConnectionHelper
virtual void SetConnection(QuicConnection* connection) OVERRIDE;
virtual const QuicClock* GetClock() const OVERRIDE;
virtual QuicRandom* GetRandomGenerator() OVERRIDE;
- virtual int WritePacketToWire(const QuicEncryptedPacket& packet,
- int* error) OVERRIDE;
+ virtual WriteResult WritePacketToWire(
+ const QuicEncryptedPacket& packet) OVERRIDE;
virtual bool IsWriteBlockedDataBuffered() OVERRIDE;
virtual bool IsWriteBlocked(int error) OVERRIDE;
virtual QuicAlarm* CreateAlarm(QuicAlarm::Delegate* delegate) OVERRIDE;
diff --git a/net/quic/quic_connection_helper_test.cc b/net/quic/quic_connection_helper_test.cc
index e228d12..ffc6ee19 100644
--- a/net/quic/quic_connection_helper_test.cc
+++ b/net/quic/quic_connection_helper_test.cc
@@ -10,6 +10,7 @@
#include "net/quic/crypto/quic_decrypter.h"
#include "net/quic/crypto/quic_encrypter.h"
#include "net/quic/quic_connection.h"
+#include "net/quic/quic_protocol.h"
#include "net/quic/test_tools/mock_clock.h"
#include "net/quic/test_tools/quic_connection_peer.h"
#include "net/quic/test_tools/quic_test_utils.h"
@@ -409,9 +410,9 @@ TEST_F(QuicConnectionHelperTest, WritePacketToWire) {
Initialize();
int len = GetWrite(0)->length();
- int error = 0;
- EXPECT_EQ(len, helper_->WritePacketToWire(*GetWrite(0), &error));
- EXPECT_EQ(0, error);
+ WriteResult result = helper_->WritePacketToWire(*GetWrite(0));
+ EXPECT_EQ(len, result.bytes_written);
+ EXPECT_EQ(WRITE_STATUS_OK, result.status);
EXPECT_TRUE(AtEof());
}
@@ -420,9 +421,9 @@ TEST_F(QuicConnectionHelperTest, WritePacketToWireAsync) {
Initialize();
EXPECT_CALL(visitor_, OnCanWrite()).WillOnce(Return(true));
- int error = 0;
- EXPECT_EQ(-1, helper_->WritePacketToWire(*GetWrite(0), &error));
- EXPECT_EQ(ERR_IO_PENDING, error);
+ WriteResult result = helper_->WritePacketToWire(*GetWrite(0));
+ EXPECT_EQ(-1, result.bytes_written);
+ EXPECT_EQ(WRITE_STATUS_BLOCKED, result.status);
base::MessageLoop::current()->RunUntilIdle();
EXPECT_TRUE(AtEof());
}
diff --git a/net/quic/quic_connection_logger.cc b/net/quic/quic_connection_logger.cc
index 5195f43..25532e0 100644
--- a/net/quic/quic_connection_logger.cc
+++ b/net/quic/quic_connection_logger.cc
@@ -33,15 +33,15 @@ base::Value* NetLogQuicPacketSentCallback(
QuicPacketSequenceNumber sequence_number,
EncryptionLevel level,
size_t packet_size,
- int rv,
+ WriteResult result,
NetLog::LogLevel /* log_level */) {
base::DictionaryValue* dict = new base::DictionaryValue();
dict->SetInteger("encryption_level", level);
dict->SetString("packet_sequence_number",
base::Uint64ToString(sequence_number));
dict->SetInteger("size", packet_size);
- if (rv < 0) {
- dict->SetInteger("net_error", rv);
+ if (result.status != WRITE_STATUS_OK) {
+ dict->SetInteger("net_error", result.error_code);
}
return dict;
}
@@ -255,11 +255,11 @@ void QuicConnectionLogger::OnPacketSent(
QuicPacketSequenceNumber sequence_number,
EncryptionLevel level,
const QuicEncryptedPacket& packet,
- int rv) {
+ WriteResult result) {
net_log_.AddEvent(
NetLog::TYPE_QUIC_SESSION_PACKET_SENT,
base::Bind(&NetLogQuicPacketSentCallback, sequence_number, level,
- packet.length(), rv));
+ packet.length(), result));
}
void QuicConnectionLogger:: OnPacketRetransmitted(
diff --git a/net/quic/quic_connection_logger.h b/net/quic/quic_connection_logger.h
index d498b12..a7a8747 100644
--- a/net/quic/quic_connection_logger.h
+++ b/net/quic/quic_connection_logger.h
@@ -6,6 +6,7 @@
#define NET_QUIC_QUIC_CONNECTION_LOGGER_H_
#include "net/quic/quic_connection.h"
+#include "net/quic/quic_protocol.h"
namespace net {
@@ -28,7 +29,7 @@ class NET_EXPORT_PRIVATE QuicConnectionLogger
virtual void OnPacketSent(QuicPacketSequenceNumber sequence_number,
EncryptionLevel level,
const QuicEncryptedPacket& packet,
- int rv) OVERRIDE;
+ WriteResult result) OVERRIDE;
virtual void OnPacketRetransmitted(
QuicPacketSequenceNumber old_sequence_number,
QuicPacketSequenceNumber new_sequence_number) OVERRIDE;
diff --git a/net/quic/quic_connection_test.cc b/net/quic/quic_connection_test.cc
index 102bb33..f861b1b 100644
--- a/net/quic/quic_connection_test.cc
+++ b/net/quic/quic_connection_test.cc
@@ -243,7 +243,9 @@ class TestConnectionHelper : public QuicConnectionHelperInterface {
TestConnectionHelper(MockClock* clock, MockRandom* random_generator)
: clock_(clock),
random_generator_(random_generator),
+ last_packet_size_(0),
blocked_(false),
+ is_write_blocked_data_buffered_(false),
is_server_(true),
use_tagging_decrypter_(false),
packets_write_attempts_(0) {
@@ -261,8 +263,8 @@ class TestConnectionHelper : public QuicConnectionHelperInterface {
return random_generator_;
}
- virtual int WritePacketToWire(const QuicEncryptedPacket& packet,
- int* error) OVERRIDE {
+ virtual WriteResult WritePacketToWire(
+ const QuicEncryptedPacket& packet) OVERRIDE {
++packets_write_attempts_;
if (packet.length() >= sizeof(final_bytes_of_last_packet_)) {
@@ -293,16 +295,14 @@ class TestConnectionHelper : public QuicConnectionHelperInterface {
*visitor.version_negotiation_packet()));
}
if (blocked_) {
- *error = ERR_IO_PENDING;
- return -1;
+ return WriteResult(WRITE_STATUS_BLOCKED, -1);
}
- *error = 0;
last_packet_size_ = packet.length();
- return last_packet_size_;
+ return WriteResult(WRITE_STATUS_OK, last_packet_size_);
}
virtual bool IsWriteBlockedDataBuffered() OVERRIDE {
- return false;
+ return is_write_blocked_data_buffered_;
}
virtual bool IsWriteBlocked(int error) OVERRIDE {
@@ -335,6 +335,10 @@ class TestConnectionHelper : public QuicConnectionHelperInterface {
void set_blocked(bool blocked) { blocked_ = blocked; }
+ void set_is_write_blocked_data_buffered(bool buffered) {
+ is_write_blocked_data_buffered_ = buffered;
+ }
+
void set_is_server(bool is_server) { is_server_ = is_server; }
// final_bytes_of_last_packet_ returns the last four bytes of the previous
@@ -360,6 +364,7 @@ class TestConnectionHelper : public QuicConnectionHelperInterface {
scoped_ptr<QuicVersionNegotiationPacket> version_negotiation_packet_;
size_t last_packet_size_;
bool blocked_;
+ bool is_write_blocked_data_buffered_;
bool is_server_;
uint32 final_bytes_of_last_packet_;
bool use_tagging_decrypter_;
@@ -1716,6 +1721,18 @@ TEST_P(QuicConnectionTest, ResumptionAlarmThenWriteBlocked) {
EXPECT_TRUE(QuicConnectionPeer::IsWriteBlocked(&connection_));
}
+TEST_P(QuicConnectionTest, WriteBlockedThenSent) {
+ helper_->set_blocked(true);
+
+ helper_->set_is_write_blocked_data_buffered(true);
+ connection_.SendStreamData(1, "foo", 0, !kFin);
+ EXPECT_FALSE(connection_.GetRetransmissionAlarm()->IsSet());
+
+ EXPECT_CALL(*send_algorithm_, OnPacketSent(_, _, _, _, _)).Times(1);
+ connection_.OnPacketSent(WriteResult(WRITE_STATUS_OK, 0));
+ EXPECT_TRUE(connection_.GetRetransmissionAlarm()->IsSet());
+}
+
TEST_P(QuicConnectionTest, LimitPacketsPerNack) {
EXPECT_CALL(visitor_, OnSuccessfulVersionNegotiation(_));
EXPECT_CALL(*send_algorithm_, OnIncomingAck(12, _, _)).Times(1);
@@ -2729,6 +2746,49 @@ TEST_P(QuicConnectionTest, ServerSendsVersionNegotiationPacket) {
}
}
+TEST_P(QuicConnectionTest, ServerSendsVersionNegotiationPacketSocketBlocked) {
+ framer_.set_version_for_tests(QUIC_VERSION_UNSUPPORTED);
+
+ QuicPacketHeader header;
+ header.public_header.guid = guid_;
+ header.public_header.reset_flag = false;
+ header.public_header.version_flag = true;
+ header.entropy_flag = false;
+ header.fec_flag = false;
+ header.packet_sequence_number = 12;
+ header.fec_group = 0;
+
+ QuicFrames frames;
+ QuicFrame frame(&frame1_);
+ frames.push_back(frame);
+ scoped_ptr<QuicPacket> packet(
+ framer_.BuildUnsizedDataPacket(header, frames).packet);
+ scoped_ptr<QuicEncryptedPacket> encrypted(
+ framer_.EncryptPacket(ENCRYPTION_NONE, 12, *packet));
+
+ framer_.set_version(QuicVersionMax());
+ connection_.set_is_server(true);
+ helper_->set_blocked(true);
+ connection_.ProcessUdpPacket(IPEndPoint(), IPEndPoint(), *encrypted);
+ EXPECT_EQ(0u, helper_->last_packet_size());
+ EXPECT_TRUE(connection_.HasQueuedData());
+
+ helper_->set_blocked(false);
+ connection_.OnCanWrite();
+ EXPECT_TRUE(helper_->version_negotiation_packet() != NULL);
+
+ size_t num_versions = arraysize(kSupportedQuicVersions);
+ EXPECT_EQ(num_versions,
+ helper_->version_negotiation_packet()->versions.size());
+
+ // We expect all versions in kSupportedQuicVersions to be
+ // included in the packet.
+ for (size_t i = 0; i < num_versions; ++i) {
+ EXPECT_EQ(kSupportedQuicVersions[i],
+ helper_->version_negotiation_packet()->versions[i]);
+ }
+}
+
TEST_P(QuicConnectionTest, ClientHandlesVersionNegotiation) {
// Start out with some unsupported version.
QuicConnectionPeer::GetFramer(&connection_)->set_version_for_tests(
@@ -3130,7 +3190,7 @@ class MockQuicConnectionDebugVisitor
void(QuicPacketSequenceNumber,
EncryptionLevel,
const QuicEncryptedPacket&,
- int));
+ WriteResult));
MOCK_METHOD2(OnPacketRetransmitted,
void(QuicPacketSequenceNumber,
diff --git a/net/quic/quic_protocol.h b/net/quic/quic_protocol.h
index 8532a22..c42d1fe 100644
--- a/net/quic/quic_protocol.h
+++ b/net/quic/quic_protocol.h
@@ -377,6 +377,8 @@ enum QuicErrorCode {
QUIC_INVALID_CRYPTO_MESSAGE_TYPE = 33,
// A crypto message was received with an illegal parameter.
QUIC_INVALID_CRYPTO_MESSAGE_PARAMETER = 34,
+ // An invalid channel id signature was supplied.
+ QUIC_INVALID_CHANNEL_ID_SIGNATURE = 52,
// A crypto message was received with a mandatory parameter missing.
QUIC_CRYPTO_MESSAGE_PARAMETER_NOT_FOUND = 35,
// A crypto message was received with a parameter that has no overlap
@@ -405,7 +407,7 @@ enum QuicErrorCode {
QUIC_CRYPTO_SERVER_CONFIG_EXPIRED = 45,
// No error. Used as bound while iterating.
- QUIC_LAST_ERROR = 52,
+ QUIC_LAST_ERROR = 53,
};
struct NET_EXPORT_PRIVATE QuicPacketPublicHeader {
@@ -870,6 +872,26 @@ struct QuicConsumedData {
bool fin_consumed;
};
+enum WriteStatus {
+ WRITE_STATUS_OK,
+ WRITE_STATUS_BLOCKED,
+ WRITE_STATUS_ERROR,
+};
+
+// A struct used to return the result of write calls including either the number
+// of bytes written or the error code, depending upon the status.
+struct NET_EXPORT_PRIVATE WriteResult {
+ WriteResult(WriteStatus status, int bytes_written_or_error_code) :
+ status(status), bytes_written(bytes_written_or_error_code) {
+ }
+
+ WriteStatus status;
+ union {
+ int bytes_written; // only valid when status is OK
+ int error_code; // only valid when status is ERROR
+ };
+};
+
} // namespace net
#endif // NET_QUIC_QUIC_PROTOCOL_H_
diff --git a/net/quic/quic_received_packet_manager.cc b/net/quic/quic_received_packet_manager.cc
index 81868ac..1cd261a 100644
--- a/net/quic/quic_received_packet_manager.cc
+++ b/net/quic/quic_received_packet_manager.cc
@@ -98,11 +98,13 @@ QuicPacketEntropyHash QuicReceivedPacketManager::EntropyHash(
ReceivedEntropyMap::const_iterator it =
packets_entropy_.upper_bound(sequence_number);
// When this map is empty we should only query entropy for
- // |largest_received_sequence_number_|.
+ // received_info_.largest_observed, since no other entropy can be correctly
+ // calculated, because we're not storing the entropy for any prior packets.
// TODO(rtenneti): add support for LOG_IF_EVERY_N_SEC to chromium.
- // LOG_IF_EVERY_N_SEC(WARNING, it != packets_entropy_.end(), 10)
- LOG_IF(WARNING, it != packets_entropy_.end())
- << "largest_received: " << received_info_.largest_observed
+ // LOG_IF_EVERY_N_SEC(DFATAL, it == packets_entropy_.end(), 10)
+ LOG_IF(DFATAL, it == packets_entropy_.end())
+ << "EntropyHash may be unknown. largest_received: "
+ << received_info_.largest_observed
<< " sequence_number: " << sequence_number;
// TODO(satyamshekhar): Make this O(1).
diff --git a/net/quic/quic_sent_packet_manager.cc b/net/quic/quic_sent_packet_manager.cc
index a9f26b8..66eea78 100644
--- a/net/quic/quic_sent_packet_manager.cc
+++ b/net/quic/quic_sent_packet_manager.cc
@@ -131,15 +131,9 @@ void QuicSentPacketManager::OnRetransmittedPacket(
void QuicSentPacketManager::OnIncomingAck(
const ReceivedPacketInfo& received_info,
- bool is_truncated_ack,
- SequenceNumberSet* acked_packets) {
- HandleAckForSentPackets(received_info, is_truncated_ack, acked_packets);
- HandleAckForSentFecPackets(received_info, acked_packets);
-
- if (acked_packets->size() > 0) {
- // The AckNotifierManager should be informed of every ACKed sequence number.
- ack_notifier_manager_.OnIncomingAck(*acked_packets);
- }
+ bool is_truncated_ack) {
+ HandleAckForSentPackets(received_info, is_truncated_ack);
+ HandleAckForSentFecPackets(received_info);
}
void QuicSentPacketManager::DiscardUnackedPacket(
@@ -149,8 +143,7 @@ void QuicSentPacketManager::DiscardUnackedPacket(
void QuicSentPacketManager::HandleAckForSentPackets(
const ReceivedPacketInfo& received_info,
- bool is_truncated_ack,
- SequenceNumberSet* acked_packets) {
+ bool is_truncated_ack) {
// Go through the packets we have not received an ack for and see if this
// incoming_ack shows they've been seen by the peer.
UnackedPacketMap::iterator it = unacked_packets_.begin();
@@ -166,12 +159,11 @@ void QuicSentPacketManager::HandleAckForSentPackets(
DVLOG(1) << ENDPOINT <<"Got an ack for packet " << sequence_number;
// If data is associated with the most recent transmission of this
// packet, then inform the caller.
- QuicPacketSequenceNumber most_recent_transmission =
- GetMostRecentTransmission(sequence_number);
- if (HasRetransmittableFrames(most_recent_transmission)) {
- acked_packets->insert(most_recent_transmission);
- }
it = MarkPacketReceivedByPeer(sequence_number);
+
+ // The AckNotifierManager is informed of every ACKed sequence number.
+ ack_notifier_manager_.OnPacketAcked(sequence_number);
+
continue;
}
@@ -381,8 +373,7 @@ void QuicSentPacketManager::DiscardPacket(
}
void QuicSentPacketManager::HandleAckForSentFecPackets(
- const ReceivedPacketInfo& received_info,
- SequenceNumberSet* acked_packets) {
+ const ReceivedPacketInfo& received_info) {
UnackedFecPacketMap::iterator it = unacked_fec_packets_.begin();
while (it != unacked_fec_packets_.end()) {
QuicPacketSequenceNumber sequence_number = it->first;
@@ -392,7 +383,6 @@ void QuicSentPacketManager::HandleAckForSentFecPackets(
if (!IsAwaitingPacket(received_info, sequence_number)) {
DVLOG(1) << ENDPOINT << "Got an ack for fec packet: " << sequence_number;
- acked_packets->insert(sequence_number);
unacked_fec_packets_.erase(it++);
} else {
// TODO(rch): treat these packets more consistently. They should
diff --git a/net/quic/quic_sent_packet_manager.h b/net/quic/quic_sent_packet_manager.h
index 4987bcc..cf4edc9 100644
--- a/net/quic/quic_sent_packet_manager.h
+++ b/net/quic/quic_sent_packet_manager.h
@@ -79,8 +79,7 @@ class NET_EXPORT_PRIVATE QuicSentPacketManager {
// Processes the ReceivedPacketInfo data from the incoming ack.
void OnIncomingAck(const ReceivedPacketInfo& received_info,
- bool is_truncated_ack,
- SequenceNumberSet* acked_packets);
+ bool is_truncated_ack);
// Discards any information for the packet corresponding to |sequence_number|.
// If this packet has been retransmitted, information on those packets
@@ -187,12 +186,10 @@ class NET_EXPORT_PRIVATE QuicSentPacketManager {
// Process the incoming ack looking for newly ack'd data packets.
void HandleAckForSentPackets(const ReceivedPacketInfo& received_info,
- bool is_truncated_ack,
- SequenceNumberSet* acked_packets);
+ bool is_truncated_ack);
// Process the incoming ack looking for newly ack'd FEC packets.
- void HandleAckForSentFecPackets(const ReceivedPacketInfo& received_info,
- SequenceNumberSet* acked_packets);
+ void HandleAckForSentFecPackets(const ReceivedPacketInfo& received_info);
// Marks |sequence_number| as having been seen by the peer. Returns an
// iterator to the next remaining unacked packet.
diff --git a/net/quic/quic_sent_packet_manager_test.cc b/net/quic/quic_sent_packet_manager_test.cc
index bfd4d4a..201d9a1 100644
--- a/net/quic/quic_sent_packet_manager_test.cc
+++ b/net/quic/quic_sent_packet_manager_test.cc
@@ -170,14 +170,11 @@ TEST_P(QuicSentPacketManagerTest, RetransmitThenAck) {
ReceivedPacketInfo received_info;
received_info.largest_observed = 2;
received_info.missing_packets.insert(1);
- SequenceNumberSet acked;
- manager_.OnIncomingAck(received_info, false, &acked);
+ manager_.OnIncomingAck(received_info, false);
// No unacked packets remain.
VerifyUnackedPackets(NULL, 0);
VerifyRetransmittablePackets(NULL, 0);
- QuicPacketSequenceNumber expected_acked[] = { 2 };
- VerifyAckedPackets(expected_acked, arraysize(expected_acked), acked);
}
TEST_P(QuicSentPacketManagerTest, RetransmitThenAckBeforeSend) {
@@ -190,8 +187,7 @@ TEST_P(QuicSentPacketManagerTest, RetransmitThenAckBeforeSend) {
// Ack 1.
ReceivedPacketInfo received_info;
received_info.largest_observed = 1;
- SequenceNumberSet acked;
- manager_.OnIncomingAck(received_info, false, &acked);
+ manager_.OnIncomingAck(received_info, false);
// There should no longer be a pending retransmission.
EXPECT_FALSE(manager_.HasPendingRetransmissions());
@@ -199,8 +195,6 @@ TEST_P(QuicSentPacketManagerTest, RetransmitThenAckBeforeSend) {
// No unacked packets remain.
VerifyUnackedPackets(NULL, 0);
VerifyRetransmittablePackets(NULL, 0);
- QuicPacketSequenceNumber expected_acked[] = { 1 };
- VerifyAckedPackets(expected_acked, arraysize(expected_acked), acked);
}
TEST_P(QuicSentPacketManagerTest, RetransmitThenAckPrevious) {
@@ -217,16 +211,13 @@ TEST_P(QuicSentPacketManagerTest, RetransmitThenAckPrevious) {
ReceivedPacketInfo received_info;
received_info.largest_observed = 2;
received_info.missing_packets.insert(2);
- SequenceNumberSet acked;
EXPECT_CALL(helper_, OnPacketNacked(2, 1)).Times(1);
- manager_.OnIncomingAck(received_info, false, &acked);
+ manager_.OnIncomingAck(received_info, false);
// 2 remains unacked, but no packets have retransmittable data.
QuicPacketSequenceNumber unacked[] = { 2 };
VerifyUnackedPackets(unacked, arraysize(unacked));
VerifyRetransmittablePackets(NULL, 0);
- QuicPacketSequenceNumber expected_acked[] = { 2 };
- VerifyAckedPackets(expected_acked, arraysize(expected_acked), acked);
}
TEST_P(QuicSentPacketManagerTest, TruncatedAck) {
@@ -246,8 +237,7 @@ TEST_P(QuicSentPacketManagerTest, TruncatedAck) {
received_info.largest_observed = 2;
received_info.missing_packets.insert(1);
received_info.missing_packets.insert(2);
- SequenceNumberSet acked;
- manager_.OnIncomingAck(received_info, true, &acked);
+ manager_.OnIncomingAck(received_info, true);
// High water mark will be raised.
QuicPacketSequenceNumber unacked[] = { 2, 3, 4 };
@@ -270,9 +260,8 @@ TEST_P(QuicSentPacketManagerTest, SendDropAckRetransmitManyPackets) {
ReceivedPacketInfo received_info;
received_info.largest_observed = 3;
received_info.missing_packets.insert(2);
- SequenceNumberSet acked;
EXPECT_CALL(helper_, OnPacketNacked(2, 1)).Times(1);
- manager_.OnIncomingAck(received_info, false, &acked);
+ manager_.OnIncomingAck(received_info, false);
QuicPacketSequenceNumber unacked[] = { 2 };
VerifyUnackedPackets(unacked, arraysize(unacked));
@@ -289,10 +278,9 @@ TEST_P(QuicSentPacketManagerTest, SendDropAckRetransmitManyPackets) {
received_info.largest_observed = 5;
received_info.missing_packets.insert(2);
received_info.missing_packets.insert(4);
- SequenceNumberSet acked;
EXPECT_CALL(helper_, OnPacketNacked(2, 2)).Times(1);
EXPECT_CALL(helper_, OnPacketNacked(4, 1)).Times(1);
- manager_.OnIncomingAck(received_info, false, &acked);
+ manager_.OnIncomingAck(received_info, false);
QuicPacketSequenceNumber unacked[] = { 2, 4 };
VerifyUnackedPackets(unacked, arraysize(unacked));
@@ -310,11 +298,10 @@ TEST_P(QuicSentPacketManagerTest, SendDropAckRetransmitManyPackets) {
received_info.missing_packets.insert(2);
received_info.missing_packets.insert(4);
received_info.missing_packets.insert(6);
- SequenceNumberSet acked;
EXPECT_CALL(helper_, OnPacketNacked(2, 3)).Times(1);
EXPECT_CALL(helper_, OnPacketNacked(4, 2)).Times(1);
EXPECT_CALL(helper_, OnPacketNacked(6, 1)).Times(1);
- manager_.OnIncomingAck(received_info, false, &acked);
+ manager_.OnIncomingAck(received_info, false);
QuicPacketSequenceNumber unacked[] = { 2, 4, 6 };
VerifyUnackedPackets(unacked, arraysize(unacked));
@@ -335,12 +322,11 @@ TEST_P(QuicSentPacketManagerTest, SendDropAckRetransmitManyPackets) {
received_info.missing_packets.insert(6);
received_info.missing_packets.insert(8);
received_info.missing_packets.insert(9);
- SequenceNumberSet acked;
EXPECT_CALL(helper_, OnPacketNacked(4, 3)).Times(1);
EXPECT_CALL(helper_, OnPacketNacked(6, 2)).Times(1);
EXPECT_CALL(helper_, OnPacketNacked(8, 1)).Times(1);
EXPECT_CALL(helper_, OnPacketNacked(9, 1)).Times(1);
- manager_.OnIncomingAck(received_info, false, &acked);
+ manager_.OnIncomingAck(received_info, false);
QuicPacketSequenceNumber unacked[] = { 2, 4, 6, 8, 9 };
VerifyUnackedPackets(unacked, arraysize(unacked));
@@ -364,13 +350,12 @@ TEST_P(QuicSentPacketManagerTest, SendDropAckRetransmitManyPackets) {
received_info.missing_packets.insert(9);
received_info.missing_packets.insert(11);
received_info.missing_packets.insert(12);
- SequenceNumberSet acked;
EXPECT_CALL(helper_, OnPacketNacked(6, 3)).Times(1);
EXPECT_CALL(helper_, OnPacketNacked(8, 2)).Times(1);
EXPECT_CALL(helper_, OnPacketNacked(9, 2)).Times(1);
EXPECT_CALL(helper_, OnPacketNacked(11, 1)).Times(1);
EXPECT_CALL(helper_, OnPacketNacked(12, 1)).Times(1);
- manager_.OnIncomingAck(received_info, false, &acked);
+ manager_.OnIncomingAck(received_info, false);
QuicPacketSequenceNumber unacked[] = { 2, 4, 6, 8, 9, 11, 12 };
VerifyUnackedPackets(unacked, arraysize(unacked));
@@ -393,12 +378,11 @@ TEST_P(QuicSentPacketManagerTest, SendDropAckRetransmitManyPackets) {
received_info.missing_packets.insert(9);
received_info.missing_packets.insert(11);
received_info.missing_packets.insert(12);
- SequenceNumberSet acked;
EXPECT_CALL(helper_, OnPacketNacked(8, 3)).Times(1);
EXPECT_CALL(helper_, OnPacketNacked(9, 3)).Times(1);
EXPECT_CALL(helper_, OnPacketNacked(11, 2)).Times(1);
EXPECT_CALL(helper_, OnPacketNacked(12, 2)).Times(1);
- manager_.OnIncomingAck(received_info, true, &acked);
+ manager_.OnIncomingAck(received_info, true);
// Truncated ack raises the high water mark by clearing out 2, 4, and 6.
QuicPacketSequenceNumber unacked[] = { 8, 9, 11, 12, 14, 15, 16 };
@@ -462,8 +446,7 @@ TEST_P(QuicSentPacketManagerTest, GetLeastUnackedFecPacketAndDiscard) {
// Ack 2.
ReceivedPacketInfo received_info;
received_info.largest_observed = 2;
- SequenceNumberSet acked;
- manager_.OnIncomingAck(received_info, false, &acked);
+ manager_.OnIncomingAck(received_info, false);
EXPECT_EQ(3u, manager_.GetLeastUnackedFecPacket());
diff --git a/net/quic/quic_utils.cc b/net/quic/quic_utils.cc
index cfb3cb7..a5a0eec 100644
--- a/net/quic/quic_utils.cc
+++ b/net/quic/quic_utils.cc
@@ -188,6 +188,7 @@ const char* QuicUtils::ErrorToString(QuicErrorCode error) {
RETURN_STRING_LITERAL(QUIC_CRYPTO_DUPLICATE_TAG);
RETURN_STRING_LITERAL(QUIC_CRYPTO_ENCRYPTION_LEVEL_INCORRECT);
RETURN_STRING_LITERAL(QUIC_CRYPTO_SERVER_CONFIG_EXPIRED);
+ RETURN_STRING_LITERAL(QUIC_INVALID_CHANNEL_ID_SIGNATURE);
RETURN_STRING_LITERAL(QUIC_LAST_ERROR);
// Intentionally have no default case, so we'll break the build
// if we add errors and don't put them here.
diff --git a/net/quic/test_tools/quic_test_utils.h b/net/quic/test_tools/quic_test_utils.h
index ffb433e..fd29824 100644
--- a/net/quic/test_tools/quic_test_utils.h
+++ b/net/quic/test_tools/quic_test_utils.h
@@ -197,8 +197,8 @@ class MockHelper : public QuicConnectionHelperInterface {
const QuicClock* GetClock() const;
QuicRandom* GetRandomGenerator();
void AdvanceTime(QuicTime::Delta delta);
- MOCK_METHOD2(WritePacketToWire, int(const QuicEncryptedPacket& packet,
- int* error));
+ MOCK_METHOD1(WritePacketToWire,
+ WriteResult(const QuicEncryptedPacket& packet));
MOCK_METHOD0(IsWriteBlockedDataBuffered, bool());
MOCK_METHOD1(IsWriteBlocked, bool(int stream_id));
virtual QuicAlarm* CreateAlarm(QuicAlarm::Delegate* delegate);