diff options
35 files changed, 482 insertions, 322 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); diff --git a/net/tools/quic/end_to_end_test.cc b/net/tools/quic/end_to_end_test.cc index 7305127..43684c4 100644 --- a/net/tools/quic/end_to_end_test.cc +++ b/net/tools/quic/end_to_end_test.cc @@ -534,14 +534,13 @@ class WrongAddressWriter : public QuicPacketWriter { self_address_ = IPEndPoint(ip, 0); } - virtual int WritePacket(const char* buffer, size_t buf_len, - const IPAddressNumber& real_self_address, - const IPEndPoint& peer_address, - QuicBlockedWriterInterface* blocked_writer, - int* error) OVERRIDE { + virtual WriteResult WritePacket( + const char* buffer, size_t buf_len, + const IPAddressNumber& real_self_address, + const IPEndPoint& peer_address, + QuicBlockedWriterInterface* blocked_writer) OVERRIDE { return QuicSocketUtils::WritePacket(fd_, buffer, buf_len, - self_address_.address(), peer_address, - error); + self_address_.address(), peer_address); } IPEndPoint self_address_; diff --git a/net/tools/quic/quic_dispatcher.cc b/net/tools/quic/quic_dispatcher.cc index 8550ef0..b22b9dc 100644 --- a/net/tools/quic/quic_dispatcher.cc +++ b/net/tools/quic/quic_dispatcher.cc @@ -53,25 +53,22 @@ QuicDispatcher::~QuicDispatcher() { STLDeleteElements(&closed_session_list_); } -int QuicDispatcher::WritePacket(const char* buffer, size_t buf_len, - const IPAddressNumber& self_address, - const IPEndPoint& peer_address, - QuicBlockedWriterInterface* writer, - int* error) { +WriteResult QuicDispatcher::WritePacket(const char* buffer, size_t buf_len, + const IPAddressNumber& self_address, + const IPEndPoint& peer_address, + QuicBlockedWriterInterface* writer) { if (write_blocked_) { write_blocked_list_.insert(make_pair(writer, true)); - *error = EAGAIN; - return -1; + return WriteResult(WRITE_STATUS_BLOCKED, EAGAIN); } - int rc = QuicSocketUtils::WritePacket(fd_, buffer, buf_len, - self_address, peer_address, - error); - if (rc == -1 && (*error == EWOULDBLOCK || *error == EAGAIN)) { + WriteResult result = QuicSocketUtils::WritePacket(fd_, buffer, buf_len, + self_address, peer_address); + if (result.status == WRITE_STATUS_BLOCKED) { write_blocked_list_.insert(make_pair(writer, true)); write_blocked_ = true; } - return rc; + return result; } void QuicDispatcher::ProcessPacket(const IPEndPoint& server_address, diff --git a/net/tools/quic/quic_dispatcher.h b/net/tools/quic/quic_dispatcher.h index 465601c..584965c 100644 --- a/net/tools/quic/quic_dispatcher.h +++ b/net/tools/quic/quic_dispatcher.h @@ -60,11 +60,11 @@ class QuicDispatcher : public QuicPacketWriter, public QuicSessionOwner { virtual ~QuicDispatcher(); // QuicPacketWriter - virtual int WritePacket(const char* buffer, size_t buf_len, - const IPAddressNumber& self_address, - const IPEndPoint& peer_address, - QuicBlockedWriterInterface* writer, - int* error) OVERRIDE; + virtual WriteResult WritePacket( + const char* buffer, size_t buf_len, + const IPAddressNumber& self_address, + const IPEndPoint& peer_address, + QuicBlockedWriterInterface* writer) OVERRIDE; virtual void ProcessPacket(const IPEndPoint& server_address, const IPEndPoint& client_address, diff --git a/net/tools/quic/quic_epoll_connection_helper.cc b/net/tools/quic/quic_epoll_connection_helper.cc index 9c33847..fc68b8f 100644 --- a/net/tools/quic/quic_epoll_connection_helper.cc +++ b/net/tools/quic/quic_epoll_connection_helper.cc @@ -98,13 +98,11 @@ QuicRandom* QuicEpollConnectionHelper::GetRandomGenerator() { return random_generator_; } -int QuicEpollConnectionHelper::WritePacketToWire( - const QuicEncryptedPacket& packet, - int* error) { +WriteResult QuicEpollConnectionHelper::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()); } // If we have a writer, delgate the write to it. @@ -112,14 +110,12 @@ int QuicEpollConnectionHelper::WritePacketToWire( return writer_->WritePacket(packet.data(), packet.length(), connection_->self_address().address(), connection_->peer_address(), - connection_, - error); + connection_); } else { return QuicSocketUtils::WritePacket( fd_, packet.data(), packet.length(), connection_->self_address().address(), - connection_->peer_address(), - error); + connection_->peer_address()); } } diff --git a/net/tools/quic/quic_epoll_connection_helper.h b/net/tools/quic/quic_epoll_connection_helper.h index 5bd526f..a7e6f01 100644 --- a/net/tools/quic/quic_epoll_connection_helper.h +++ b/net/tools/quic/quic_epoll_connection_helper.h @@ -43,8 +43,8 @@ class QuicEpollConnectionHelper : public QuicConnectionHelperInterface { 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/tools/quic/quic_epoll_connection_helper_test.cc b/net/tools/quic/quic_epoll_connection_helper_test.cc index 154c393..0e5cd48 100644 --- a/net/tools/quic/quic_epoll_connection_helper_test.cc +++ b/net/tools/quic/quic_epoll_connection_helper_test.cc @@ -39,15 +39,14 @@ class TestConnectionHelper : public QuicEpollConnectionHelper { : QuicEpollConnectionHelper(fd, eps) { } - virtual int WritePacketToWire(const QuicEncryptedPacket& packet, - int* error) OVERRIDE { + virtual WriteResult WritePacketToWire( + const QuicEncryptedPacket& packet) OVERRIDE { QuicFramer framer(QuicVersionMax(), QuicTime::Zero(), true); FramerVisitorCapturingFrames visitor; framer.set_visitor(&visitor); EXPECT_TRUE(framer.ProcessPacket(packet)); header_ = *visitor.header(); - *error = 0; - return packet.length(); + return WriteResult(WRITE_STATUS_OK, packet.length()); } QuicPacketHeader* header() { return &header_; } diff --git a/net/tools/quic/quic_packet_writer.h b/net/tools/quic/quic_packet_writer.h index b10e852..2d70e09 100644 --- a/net/tools/quic/quic_packet_writer.h +++ b/net/tools/quic/quic_packet_writer.h @@ -6,10 +6,12 @@ #define NET_TOOLS_QUIC_QUIC_PACKET_WRITER_H_ #include "net/base/ip_endpoint.h" +#include "net/quic/quic_protocol.h" namespace net { class QuicBlockedWriterInterface; +struct WriteResult; namespace tools { @@ -20,11 +22,11 @@ class QuicPacketWriter { public: virtual ~QuicPacketWriter() {} - virtual int WritePacket(const char* buffer, size_t buf_len, - const net::IPAddressNumber& self_address, - const net::IPEndPoint& peer_address, - QuicBlockedWriterInterface* blocked_writer, - int* error) = 0; + virtual WriteResult WritePacket( + const char* buffer, size_t buf_len, + const net::IPAddressNumber& self_address, + const net::IPEndPoint& peer_address, + QuicBlockedWriterInterface* blocked_writer) = 0; }; } // namespace tools diff --git a/net/tools/quic/quic_socket_utils.cc b/net/tools/quic/quic_socket_utils.cc index 6535d57..87071a6 100644 --- a/net/tools/quic/quic_socket_utils.cc +++ b/net/tools/quic/quic_socket_utils.cc @@ -12,6 +12,7 @@ #include <string> #include "base/logging.h" +#include "net/quic/quic_protocol.h" #ifndef SO_RXQ_OVFL #define SO_RXQ_OVFL 40 @@ -50,7 +51,7 @@ IPAddressNumber QuicSocketUtils::GetAddressFromMsghdr(struct msghdr *hdr) { // static bool QuicSocketUtils::GetOverflowFromMsghdr(struct msghdr *hdr, - int *dropped_packets) { + int *dropped_packets) { if (hdr->msg_controllen > 0) { struct cmsghdr *cmsg; for (cmsg = CMSG_FIRSTHDR(hdr); @@ -79,9 +80,9 @@ int QuicSocketUtils::SetGetAddressInfo(int fd, int address_family) { // static int QuicSocketUtils::ReadPacket(int fd, char* buffer, size_t buf_len, - int* dropped_packets, - IPAddressNumber* self_address, - IPEndPoint* peer_address) { + int* dropped_packets, + IPAddressNumber* self_address, + IPEndPoint* peer_address) { CHECK(peer_address != NULL); const int kSpaceForOverflowAndIp = CMSG_SPACE(sizeof(int)) + CMSG_SPACE(sizeof(in6_pktinfo)); @@ -135,10 +136,11 @@ int QuicSocketUtils::ReadPacket(int fd, char* buffer, size_t buf_len, } // static -int QuicSocketUtils::WritePacket(int fd, const char* buffer, size_t buf_len, - const IPAddressNumber& self_address, - const IPEndPoint& peer_address, - int* error) { +WriteResult QuicSocketUtils::WritePacket(int fd, + const char* buffer, + size_t buf_len, + const IPAddressNumber& self_address, + const IPEndPoint& peer_address) { sockaddr_storage raw_address; socklen_t address_len = sizeof(raw_address); CHECK(peer_address.ToSockAddr( @@ -190,8 +192,11 @@ int QuicSocketUtils::WritePacket(int fd, const char* buffer, size_t buf_len, } int rc = sendmsg(fd, &hdr, 0); - *error = (rc >= 0) ? 0 : errno; - return rc; + if (rc >= 0) { + return WriteResult(WRITE_STATUS_OK, rc); + } + return WriteResult((errno == EAGAIN || errno == EWOULDBLOCK) ? + WRITE_STATUS_BLOCKED : WRITE_STATUS_ERROR, errno); } } // namespace tools diff --git a/net/tools/quic/quic_socket_utils.h b/net/tools/quic/quic_socket_utils.h index bfacc6c..8f0feff 100644 --- a/net/tools/quic/quic_socket_utils.h +++ b/net/tools/quic/quic_socket_utils.h @@ -12,6 +12,7 @@ #include <string> #include "net/base/ip_endpoint.h" +#include "net/quic/quic_protocol.h" namespace net { namespace tools { @@ -45,12 +46,13 @@ class QuicSocketUtils { IPAddressNumber* self_address, IPEndPoint* peer_address); - // Writes buf_len to the socket. If writing is successful returns the number - // of bytes written otherwise returns -1 and sets error to errno. - static int WritePacket(int fd, const char* buffer, size_t buf_len, - const IPAddressNumber& self_address, - const IPEndPoint& peer_address, - int* error); + // Writes buf_len to the socket. If writing is successful, sets the result's + // status to WRITE_STATUS_OK and sets bytes_written. Otherwise sets the + // result's status to WRITE_STATUS_BLOCKED or WRITE_STATUS_ERROR and sets + // error_code to errno. + static WriteResult WritePacket(int fd, const char* buffer, size_t buf_len, + const IPAddressNumber& self_address, + const IPEndPoint& peer_address); }; } // namespace tools diff --git a/net/tools/quic/quic_time_wait_list_manager.cc b/net/tools/quic/quic_time_wait_list_manager.cc index a2fc4f8..345ddb2 100644 --- a/net/tools/quic/quic_time_wait_list_manager.cc +++ b/net/tools/quic/quic_time_wait_list_manager.cc @@ -263,22 +263,19 @@ void QuicTimeWaitListManager::SendOrQueuePacket(QueuedPacket* packet) { void QuicTimeWaitListManager::WriteToWire(QueuedPacket* queued_packet) { DCHECK(!is_write_blocked_); - int error; - int rc = writer_->WritePacket(queued_packet->packet()->data(), - queued_packet->packet()->length(), - queued_packet->server_address().address(), - queued_packet->client_address(), - this, - &error); - - if (rc == -1) { - if (error == EAGAIN || error == EWOULDBLOCK) { - is_write_blocked_ = true; - } else { - LOG(WARNING) << "Received unknown error while sending reset packet to " - << queued_packet->client_address().ToString() << ": " - << strerror(error); - } + WriteResult result = writer_->WritePacket( + queued_packet->packet()->data(), + queued_packet->packet()->length(), + queued_packet->server_address().address(), + queued_packet->client_address(), + this); + + if (result.status == WRITE_STATUS_BLOCKED) { + is_write_blocked_ = true; + } else if (result.status == WRITE_STATUS_ERROR) { + LOG(WARNING) << "Received unknown error while sending reset packet to " + << queued_packet->client_address().ToString() << ": " + << strerror(result.error_code); } } diff --git a/net/tools/quic/quic_time_wait_list_manager_test.cc b/net/tools/quic/quic_time_wait_list_manager_test.cc index 8f59687..64abb61 100644 --- a/net/tools/quic/quic_time_wait_list_manager_test.cc +++ b/net/tools/quic/quic_time_wait_list_manager_test.cc @@ -195,11 +195,10 @@ TEST_F(QuicTimeWaitListManagerTest, SendPublicReset) { EXPECT_CALL(writer_, WritePacket(_, _, server_address_.address(), client_address_, - &time_wait_list_manager_, - _)) + &time_wait_list_manager_)) .With(Args<0, 1>(PublicResetPacketEq(guid_, kRandomSequenceNumber))) - .WillOnce(Return(packet->length())); + .WillOnce(Return(WriteResult(WRITE_STATUS_OK, packet->length()))); ProcessPacket(guid_, *packet); } @@ -210,7 +209,7 @@ TEST_F(QuicTimeWaitListManagerTest, DropInvalidPacket) { QuicEncryptedPacket packet(buffer, arraysize(buffer)); ProcessPacket(guid_, packet); // Will get called for a valid packet since received packet count = 1 (2 ^ 0). - EXPECT_CALL(writer_, WritePacket(_, _, _, _, _, _)).Times(0); + EXPECT_CALL(writer_, WritePacket(_, _, _, _, _)).Times(0); } TEST_F(QuicTimeWaitListManagerTest, DropPublicResetPacket) { @@ -225,7 +224,7 @@ TEST_F(QuicTimeWaitListManagerTest, DropPublicResetPacket) { QuicFramer::BuildPublicResetPacket(packet)); ProcessPacket(guid_, *public_reset_packet); // Will get called for a data packet since received packet count = 1 (2 ^ 0). - EXPECT_CALL(writer_, WritePacket(_, _, _, _, _, _)) + EXPECT_CALL(writer_, WritePacket(_, _, _, _, _)) .Times(0); } @@ -235,8 +234,8 @@ TEST_F(QuicTimeWaitListManagerTest, SendPublicResetWithExponentialBackOff) { scoped_ptr<QuicEncryptedPacket> packet( ConstructEncryptedPacket(guid_, sequence_number)); if ((sequence_number & (sequence_number - 1)) == 0) { - EXPECT_CALL(writer_, WritePacket(_, _, _, _, _, _)) - .WillOnce(Return(1)); + EXPECT_CALL(writer_, WritePacket(_, _, _, _, _)) + .WillOnce(Return(WriteResult(WRITE_STATUS_OK, 1))); } ProcessPacket(guid_, *packet); // Send public reset with exponential back off. @@ -294,11 +293,10 @@ TEST_F(QuicTimeWaitListManagerTest, SendQueuedPackets) { EXPECT_CALL(writer_, WritePacket(_, _, server_address_.address(), client_address_, - &time_wait_list_manager_, - _)) + &time_wait_list_manager_)) .With(Args<0, 1>(PublicResetPacketEq(guid, sequence_number))) - .WillOnce(Return(packet->length())); + .WillOnce(Return(WriteResult(WRITE_STATUS_OK, packet->length()))); ProcessPacket(guid, *packet); EXPECT_FALSE(time_wait_list_manager_.is_write_blocked()); @@ -306,11 +304,10 @@ TEST_F(QuicTimeWaitListManagerTest, SendQueuedPackets) { EXPECT_CALL(writer_, WritePacket(_, _, server_address_.address(), client_address_, - &time_wait_list_manager_, - _)) + &time_wait_list_manager_)) .With(Args<0, 1>(PublicResetPacketEq(guid, sequence_number))) - .WillOnce(DoAll(SetArgPointee<5>(EAGAIN), Return(-1))); + .WillOnce(Return(WriteResult(WRITE_STATUS_BLOCKED, EAGAIN))); ProcessPacket(guid, *packet); // 3rd packet. No public reset should be sent; ProcessPacket(guid, *packet); @@ -323,7 +320,7 @@ TEST_F(QuicTimeWaitListManagerTest, SendQueuedPackets) { QuicPacketSequenceNumber other_sequence_number = 23423; scoped_ptr<QuicEncryptedPacket> other_packet( ConstructEncryptedPacket(other_guid, other_sequence_number)); - EXPECT_CALL(writer_, WritePacket(_, _, _, _, _, _)) + EXPECT_CALL(writer_, WritePacket(_, _, _, _, _)) .Times(0); ProcessPacket(other_guid, *other_packet); @@ -331,19 +328,18 @@ TEST_F(QuicTimeWaitListManagerTest, SendQueuedPackets) { EXPECT_CALL(writer_, WritePacket(_, _, server_address_.address(), client_address_, - &time_wait_list_manager_, - _)) + &time_wait_list_manager_)) .With(Args<0, 1>(PublicResetPacketEq(guid, sequence_number))) - .WillOnce(Return(packet->length())); + .WillOnce(Return(WriteResult(WRITE_STATUS_OK, packet->length()))); EXPECT_CALL(writer_, WritePacket(_, _, server_address_.address(), client_address_, - &time_wait_list_manager_, - _)) + &time_wait_list_manager_)) .With(Args<0, 1>(PublicResetPacketEq(other_guid, other_sequence_number))) - .WillOnce(Return(other_packet->length())); + .WillOnce(Return(WriteResult(WRITE_STATUS_OK, + other_packet->length()))); time_wait_list_manager_.OnCanWrite(); EXPECT_FALSE(time_wait_list_manager_.is_write_blocked()); } @@ -357,7 +353,8 @@ TEST_F(QuicTimeWaitListManagerTest, MakeSureFramerUsesCorrectVersion) { packet.reset(ConstructEncryptedPacket(guid_, kRandomSequenceNumber)); // Reset packet should be written, using the minimum quic version. - EXPECT_CALL(writer_, WritePacket(_, _, _, _, _, _)).Times(1); + EXPECT_CALL(writer_, WritePacket(_, _, _, _, _)).Times(1) + .WillOnce(Return(WriteResult(WRITE_STATUS_OK, 0))); ProcessPacket(guid_, *packet); EXPECT_EQ(time_wait_list_manager_.version(), QuicVersionMin()); @@ -369,7 +366,8 @@ TEST_F(QuicTimeWaitListManagerTest, MakeSureFramerUsesCorrectVersion) { packet.reset(ConstructEncryptedPacket(guid_, kRandomSequenceNumber)); // Reset packet should be written, using the maximum quic version. - EXPECT_CALL(writer_, WritePacket(_, _, _, _, _, _)).Times(1); + EXPECT_CALL(writer_, WritePacket(_, _, _, _, _)).Times(1) + .WillOnce(Return(WriteResult(WRITE_STATUS_OK, 0))); ProcessPacket(guid_, *packet); EXPECT_EQ(time_wait_list_manager_.version(), QuicVersionMax()); } diff --git a/net/tools/quic/test_tools/quic_test_utils.h b/net/tools/quic/test_tools/quic_test_utils.h index dffa255..2710a56 100644 --- a/net/tools/quic/test_tools/quic_test_utils.h +++ b/net/tools/quic/test_tools/quic_test_utils.h @@ -125,12 +125,12 @@ class MockPacketWriter : public QuicPacketWriter { MockPacketWriter(); virtual ~MockPacketWriter(); - MOCK_METHOD6(WritePacket, int(const char* buffer, - size_t buf_len, - const IPAddressNumber& self_address, - const IPEndPoint& peer_address, - QuicBlockedWriterInterface* blocked_writer, - int* error)); + MOCK_METHOD5(WritePacket, + WriteResult(const char* buffer, + size_t buf_len, + const IPAddressNumber& self_address, + const IPEndPoint& peer_address, + QuicBlockedWriterInterface* blocked_writer)); }; } // namespace test |