diff options
author | rtenneti@chromium.org <rtenneti@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98> | 2014-05-13 07:44:22 +0000 |
---|---|---|
committer | rtenneti@chromium.org <rtenneti@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98> | 2014-05-13 07:44:22 +0000 |
commit | 6d9ca3b790e9cc01e1ae0b39730c27423e2e7d7f (patch) | |
tree | e34edb10a3166cfdf5f4e0f71e8e8e754b801ffe /net | |
parent | 4c900827947228c8642a594d682817444cd4a559 (diff) | |
download | chromium_src-6d9ca3b790e9cc01e1ae0b39730c27423e2e7d7f.zip chromium_src-6d9ca3b790e9cc01e1ae0b39730c27423e2e7d7f.tar.gz chromium_src-6d9ca3b790e9cc01e1ae0b39730c27423e2e7d7f.tar.bz2 |
Land Recent QUIC Changes.
Fix a flaky EndToEndTest where packet loss is possible with a large
buffer that may overrun.
Merge internal change: 66821654
https://codereview.chromium.org/280383003/
QUIC change to invoke SetNotPending for retransmissions in
MarkForRetransmission and a minor cleanup in UnackedPacketMap.
Merge internal change: 66729074
https://codereview.chromium.org/278823005/
Remove unused IsHandshake argument to QuicConnection::OnCanWrite.
Merge internal change: 66652078
https://codereview.chromium.org/286563003/
Revert of Fix a bug where if a crypto packet is spuriously retransmitted
and thenneutered, it remains in missing_packets until the end of the
connection.
Reverted chromium cl: https://codereview.chromium.org/270213002/
Rollback of Merge internal change: 66262890.
*** Reason for rollback ***
This appears to have caused a major QUIC loadtest regression on
internal server.
Additionally, it's wrong in some other subtle ways, so rolling it back
and trying a new approach.
*** Original change description ***
Fix a bug where if a crypto packet is spuriously retransmitted and then
neutered, it remains in missing_packets until the end of the
connection.
***
Merge internal change: 66615668
https://codereview.chromium.org/287583002/
Revert of Minor QUIC cleanup to combine QuicUnackedPacketMap's
RemovePacket and NeuterPacket into RemoveOrNeuterPacket.
Rollback of merge internal change: 66306887
Rollback of Chromium's CL https://codereview.chromium.org/271443008/
*** Reason for rollback ***
After further issues with the QUIC handshake packets, rolling back this
change in favor of a different approach.
*** Original change description ***
Minor QUIC cleanup to combine QuicUnackedPacketMap's RemovePacket and
NeuterPacket into RemoveOrNeuterPacket.
This prevents potential bugs where a packet can be set not pending, then
neuter'd, which cuases the packet to never be removed from the unacked
packet map.
***
Merge internal change: 66573334
https://codereview.chromium.org/280753002/
Merging end_to_end_test.cc with the internal source tree.
+ Fixed comments (MB for megabytes and 256KB per sec instead of 1Mbit).
+ Made LargePostWithPacketLossAndBlockedSocket and
LargePostNoPacketLossWithDelayAndReordering to be in the same
order as the internal source tree.
Merge internal change: 66556306
https://codereview.chromium.org/283683002/
Allows QUIC connections to remain established even though the peer's
port has changed. Should make things a bit better for clients
experiencing NAT rebindign while talking QUIC to Bandaid.
QUIC connection port migration. Protected behind
FLAGS_quic_allow_port_migration
Merge internal change: 66523001
https://codereview.chromium.org/279453004/
Remove unnecessary methods from QuicUnackedPacketMap and a minor
QuicSentPacketManagerTest fix as a result.
Merge internal change: 66427453
https://codereview.chromium.org/287443003/
R=rch@chromium.org
Review URL: https://codereview.chromium.org/283693002
git-svn-id: svn://svn.chromium.org/chrome/trunk/src@270046 0039d316-1c4b-4281-b951-d872f2087c98
Diffstat (limited to 'net')
-rw-r--r-- | net/quic/congestion_control/tcp_loss_algorithm_test.cc | 2 | ||||
-rw-r--r-- | net/quic/quic_connection.cc | 87 | ||||
-rw-r--r-- | net/quic/quic_connection.h | 32 | ||||
-rw-r--r-- | net/quic/quic_flags.cc | 4 | ||||
-rw-r--r-- | net/quic/quic_flags.h | 1 | ||||
-rw-r--r-- | net/quic/quic_reliable_client_stream.cc | 3 | ||||
-rw-r--r-- | net/quic/quic_sent_packet_manager.cc | 65 | ||||
-rw-r--r-- | net/quic/quic_sent_packet_manager.h | 2 | ||||
-rw-r--r-- | net/quic/quic_sent_packet_manager_test.cc | 50 | ||||
-rw-r--r-- | net/quic/quic_session.cc | 2 | ||||
-rw-r--r-- | net/quic/quic_unacked_packet_map.cc | 63 | ||||
-rw-r--r-- | net/quic/quic_unacked_packet_map.h | 19 | ||||
-rw-r--r-- | net/quic/test_tools/quic_sent_packet_manager_peer.cc | 6 | ||||
-rw-r--r-- | net/quic/test_tools/quic_sent_packet_manager_peer.h | 3 | ||||
-rw-r--r-- | net/tools/quic/end_to_end_test.cc | 112 | ||||
-rw-r--r-- | net/tools/quic/quic_client.cc | 12 | ||||
-rw-r--r-- | net/tools/quic/quic_client.h | 4 | ||||
-rw-r--r-- | net/tools/quic/test_tools/quic_client_peer.cc | 10 | ||||
-rw-r--r-- | net/tools/quic/test_tools/quic_client_peer.h | 2 |
19 files changed, 294 insertions, 185 deletions
diff --git a/net/quic/congestion_control/tcp_loss_algorithm_test.cc b/net/quic/congestion_control/tcp_loss_algorithm_test.cc index d7d2d72..657ab2b 100644 --- a/net/quic/congestion_control/tcp_loss_algorithm_test.cc +++ b/net/quic/congestion_control/tcp_loss_algorithm_test.cc @@ -171,7 +171,7 @@ TEST_F(TcpLossAlgorithmTest, DontEarlyRetransmitNeuteredPacket) { // Early retransmit when the final packet gets acked and the first is nacked. unacked_packets_.SetNotPending(2); unacked_packets_.NackPacket(1, 1); - unacked_packets_.NeuterIfPendingOrRemovePacket(1); + unacked_packets_.NeuterPacket(1); VerifyLosses(2, NULL, 0); EXPECT_EQ(QuicTime::Zero(), loss_algorithm_.GetLossTimeout()); } diff --git a/net/quic/quic_connection.cc b/net/quic/quic_connection.cc index 6838a1e..d834f20 100644 --- a/net/quic/quic_connection.cc +++ b/net/quic/quic_connection.cc @@ -204,6 +204,7 @@ QuicConnection::QuicConnection(QuicConnectionId connection_id, random_generator_(helper->GetRandomGenerator()), connection_id_(connection_id), peer_address_(address), + migrating_peer_port_(0), last_packet_revived_(false), last_size_(0), last_decrypted_packet_level_(ENCRYPTION_NONE), @@ -234,7 +235,10 @@ QuicConnection::QuicConnection(QuicConnectionId connection_id, version_negotiation_state_(START_NEGOTIATION), is_server_(is_server), connected_(true), - address_migrating_(false), + peer_ip_changed_(false), + peer_port_changed_(false), + self_ip_changed_(false), + self_port_changed_(false), max_flow_control_receive_window_bytes_( max_flow_control_receive_window_bytes) { if (max_flow_control_receive_window_bytes_ < kDefaultFlowControlSendWindow) { @@ -1105,18 +1109,7 @@ void QuicConnection::ProcessUdpPacket(const IPEndPoint& self_address, last_packet_revived_ = false; last_size_ = packet.length(); - address_migrating_ = false; - - if (peer_address_.address().empty()) { - peer_address_ = peer_address; - } - if (self_address_.address().empty()) { - self_address_ = self_address; - } - - if (!(peer_address == peer_address_ && self_address == self_address_)) { - address_migrating_ = true; - } + CheckForAddressMigration(self_address, peer_address); stats_.bytes_received += packet.length(); ++stats_.packets_received; @@ -1141,20 +1134,45 @@ void QuicConnection::ProcessUdpPacket(const IPEndPoint& self_address, SetPingAlarm(); } +void QuicConnection::CheckForAddressMigration( + const IPEndPoint& self_address, const IPEndPoint& peer_address) { + peer_ip_changed_ = false; + peer_port_changed_ = false; + self_ip_changed_ = false; + self_port_changed_ = false; + + if (peer_address_.address().empty()) { + peer_address_ = peer_address; + } + if (self_address_.address().empty()) { + self_address_ = self_address; + } + + if (!peer_address.address().empty() && !peer_address_.address().empty()) { + peer_ip_changed_ = (peer_address.address() != peer_address_.address()); + peer_port_changed_ = (peer_address.port() != peer_address_.port()); + + // Store in case we want to migrate connection in ProcessValidatedPacket. + migrating_peer_port_ = peer_address.port(); + } + + if (!self_address.address().empty() && !self_address_.address().empty()) { + self_ip_changed_ = (self_address.address() != self_address_.address()); + self_port_changed_ = (self_address.port() != self_address_.port()); + } +} + void QuicConnection::OnCanWrite() { DCHECK(!writer_->IsWriteBlocked()); WriteQueuedPackets(); WritePendingRetransmissions(); - IsHandshake pending_handshake = visitor_->HasPendingHandshake() ? - IS_HANDSHAKE : NOT_HANDSHAKE; // Sending queued packets may have caused the socket to become write blocked, // or the congestion manager to prohibit sending. If we've sent everything // we had queued and we're still not blocked, let the visitor know it can // write more. - if (!CanWrite(NOT_RETRANSMISSION, HAS_RETRANSMITTABLE_DATA, - pending_handshake)) { + if (!CanWrite(NOT_RETRANSMISSION, HAS_RETRANSMITTABLE_DATA)) { return; } @@ -1166,11 +1184,8 @@ void QuicConnection::OnCanWrite() { // After the visitor writes, it may have caused the socket to become write // blocked or the congestion manager to prohibit sending, so check again. - pending_handshake = visitor_->HasPendingHandshake() ? - IS_HANDSHAKE : NOT_HANDSHAKE; if (visitor_->HasPendingWrites() && !resume_writes_alarm_->IsSet() && - CanWrite(NOT_RETRANSMISSION, HAS_RETRANSMITTABLE_DATA, - pending_handshake)) { + CanWrite(NOT_RETRANSMISSION, HAS_RETRANSMITTABLE_DATA)) { // We're not write blocked, but some stream didn't write out all of its // bytes. Register for 'immediate' resumption so we'll keep writing after // other connections and events have had a chance to use the thread. @@ -1185,12 +1200,23 @@ void QuicConnection::WriteIfNotBlocked() { } bool QuicConnection::ProcessValidatedPacket() { - if (address_migrating_) { + if ((!FLAGS_quic_allow_port_migration && peer_port_changed_) || + peer_ip_changed_ || self_ip_changed_ || self_port_changed_) { SendConnectionCloseWithDetails( QUIC_ERROR_MIGRATING_ADDRESS, - "Address migration is not yet a supported feature"); + "IP or port migration is not yet a supported feature"); return false; } + + // Port migration is supported, do it now if port has changed. + if (FLAGS_quic_allow_port_migration && + peer_port_changed_) { + DVLOG(1) << ENDPOINT << "Peer's port changed from " + << peer_address_.port() << " to " << migrating_peer_port_ + << ", migrating connection."; + peer_address_ = IPEndPoint(peer_address_.address(), migrating_peer_port_); + } + time_of_last_received_packet_ = clock_->Now(); DVLOG(1) << ENDPOINT << "time of last received packet: " << time_of_last_received_packet_.ToDebuggingValue(); @@ -1230,8 +1256,7 @@ void QuicConnection::WritePendingRetransmissions() { const QuicSentPacketManager::PendingRetransmission pending = sent_packet_manager_.NextPendingRetransmission(); if (GetPacketType(&pending.retransmittable_frames) == NORMAL && - !CanWrite(pending.transmission_type, HAS_RETRANSMITTABLE_DATA, - pending.retransmittable_frames.HasCryptoHandshake())) { + !CanWrite(pending.transmission_type, HAS_RETRANSMITTABLE_DATA)) { break; } @@ -1269,8 +1294,8 @@ void QuicConnection::RetransmitUnackedPackets( WriteIfNotBlocked(); } -void QuicConnection::DiscardUnencryptedPackets() { - sent_packet_manager_.DiscardUnencryptedPackets(); +void QuicConnection::NeuterUnencryptedPackets() { + sent_packet_manager_.NeuterUnencryptedPackets(); // This may have changed the retransmission timer, so re-arm it. retransmission_alarm_->Cancel(); QuicTime retransmission_time = sent_packet_manager_.GetRetransmissionTime(); @@ -1289,12 +1314,11 @@ bool QuicConnection::ShouldGeneratePacket( return true; } - return CanWrite(transmission_type, retransmittable, handshake); + return CanWrite(transmission_type, retransmittable); } bool QuicConnection::CanWrite(TransmissionType transmission_type, - HasRetransmittableData retransmittable, - IsHandshake handshake) { + HasRetransmittableData retransmittable) { if (writer_->IsWriteBlocked()) { visitor_->OnWriteBlocked(); return false; @@ -1342,8 +1366,7 @@ bool QuicConnection::WritePacket(QueuedPacket packet) { // TODO(ianswett): The congestion control should have been consulted before // serializing the packet, so this could be turned into a LOG_IF(DFATAL). if (packet.type == NORMAL && !CanWrite(packet.transmission_type, - packet.retransmittable, - packet.handshake)) { + packet.retransmittable)) { return false; } diff --git a/net/quic/quic_connection.h b/net/quic/quic_connection.h index 89affd3..f5e152f 100644 --- a/net/quic/quic_connection.h +++ b/net/quic/quic_connection.h @@ -417,9 +417,9 @@ class NET_EXPORT_PRIVATE QuicConnection // initially encrypted packets when the initial encrypter changes. void RetransmitUnackedPackets(RetransmissionType retransmission_type); - // Calls |sent_packet_manager_|'s DiscardUnencryptedPackets. Used when the + // Calls |sent_packet_manager_|'s NeuterUnencryptedPackets. Used when the // connection becomes forward secure and hasn't received acks for all packets. - void DiscardUnencryptedPackets(); + void NeuterUnencryptedPackets(); // Changes the encrypter used for level |level| to |encrypter|. The function // takes ownership of |encrypter|. @@ -458,8 +458,7 @@ class NET_EXPORT_PRIVATE QuicConnection } bool CanWrite(TransmissionType transmission_type, - HasRetransmittableData retransmittable, - IsHandshake handshake); + HasRetransmittableData retransmittable); uint32 max_flow_control_receive_window_bytes() const { return max_flow_control_receive_window_bytes_; @@ -611,6 +610,11 @@ class NET_EXPORT_PRIVATE QuicConnection // Sets the ping alarm to the appropriate value, if any. void SetPingAlarm(); + // On arrival of a new packet, checks to see if the socket addresses have + // changed since the last packet we saw on this connection. + void CheckForAddressMigration(const IPEndPoint& self_address, + const IPEndPoint& peer_address); + QuicFramer framer_; QuicConnectionHelperInterface* helper_; // Not owned. QuicPacketWriter* writer_; // Not owned. @@ -623,6 +627,8 @@ class NET_EXPORT_PRIVATE QuicConnection // client. IPEndPoint self_address_; IPEndPoint peer_address_; + // Used to store latest peer port to possibly migrate to later. + int migrating_peer_port_; bool last_packet_revived_; // True if the last packet was revived from FEC. size_t last_size_; // Size of the last received packet. @@ -734,9 +740,21 @@ class NET_EXPORT_PRIVATE QuicConnection // close. bool connected_; - // Set to true if the udp packet headers have a new self or peer address. - // This is checked later on validating a data or version negotiation packet. - bool address_migrating_; + // Set to true if the UDP packet headers have a new IP address for the peer. + // If true, do not perform connection migration. + bool peer_ip_changed_; + + // Set to true if the UDP packet headers have a new port for the peer. + // If true, and the IP has not changed, then we can migrate the connection. + bool peer_port_changed_; + + // Set to true if the UDP packet headers are addressed to a different IP. + // We do not support connection migration when the self IP changed. + bool self_ip_changed_; + + // Set to true if the UDP packet headers are addressed to a different port. + // If true, and the IP has not changed, then we can migrate the connection. + bool self_port_changed_; // If non-empty this contains the set of versions received in a // version negotiation packet. diff --git a/net/quic/quic_flags.cc b/net/quic/quic_flags.cc index c42afb6..faefb35 100644 --- a/net/quic/quic_flags.cc +++ b/net/quic/quic_flags.cc @@ -33,5 +33,9 @@ bool FLAGS_enable_quic_stream_flow_control_2 = true; bool FLAGS_enable_quic_connection_flow_control = true; bool FLAGS_quic_allow_oversized_packets_for_test = false; + // When true, the use time based loss detection instead of nack. bool FLAGS_quic_use_time_loss_detection = false; + +// If true, allow port migration of established QUIC connections. +bool FLAGS_quic_allow_port_migration = true; diff --git a/net/quic/quic_flags.h b/net/quic/quic_flags.h index 31585d3..85ccf57 100644 --- a/net/quic/quic_flags.h +++ b/net/quic/quic_flags.h @@ -13,5 +13,6 @@ NET_EXPORT_PRIVATE extern bool FLAGS_enable_quic_stream_flow_control_2; NET_EXPORT_PRIVATE extern bool FLAGS_enable_quic_connection_flow_control; NET_EXPORT_PRIVATE extern bool FLAGS_quic_allow_oversized_packets_for_test; NET_EXPORT_PRIVATE extern bool FLAGS_quic_use_time_loss_detection; +NET_EXPORT_PRIVATE extern bool FLAGS_quic_allow_port_migration; #endif // NET_QUIC_QUIC_FLAGS_H_ diff --git a/net/quic/quic_reliable_client_stream.cc b/net/quic/quic_reliable_client_stream.cc index 36296c3..6445ff2 100644 --- a/net/quic/quic_reliable_client_stream.cc +++ b/net/quic/quic_reliable_client_stream.cc @@ -94,8 +94,7 @@ void QuicReliableClientStream::OnError(int error) { bool QuicReliableClientStream::CanWrite(const CompletionCallback& callback) { bool can_write = session()->connection()->CanWrite( - NOT_RETRANSMISSION, HAS_RETRANSMITTABLE_DATA, - id() == kCryptoStreamId ? IS_HANDSHAKE : NOT_HANDSHAKE); + NOT_RETRANSMISSION, HAS_RETRANSMITTABLE_DATA); if (!can_write) { session()->MarkWriteBlocked(id(), EffectivePriority()); DCHECK(callback_.is_null()); diff --git a/net/quic/quic_sent_packet_manager.cc b/net/quic/quic_sent_packet_manager.cc index 0085f90..121a7a5 100644 --- a/net/quic/quic_sent_packet_manager.cc +++ b/net/quic/quic_sent_packet_manager.cc @@ -244,26 +244,27 @@ void QuicSentPacketManager::RetransmitUnackedPackets( // numbers with no connection to the previous ones. if (frames != NULL && (retransmission_type == ALL_PACKETS || frames->encryption_level() == ENCRYPTION_INITIAL)) { - unacked_packets_.SetNotPending(unacked_it->first); MarkForRetransmission(unacked_it->first, ALL_UNACKED_RETRANSMISSION); } ++unacked_it; } } -void QuicSentPacketManager::DiscardUnencryptedPackets() { +void QuicSentPacketManager::NeuterUnencryptedPackets() { QuicUnackedPacketMap::const_iterator unacked_it = unacked_packets_.begin(); while (unacked_it != unacked_packets_.end()) { const RetransmittableFrames* frames = unacked_it->second.retransmittable_frames; if (frames != NULL && frames->encryption_level() == ENCRYPTION_NONE) { - // Once you're forward secure, no unencrypted packets will be sent. - // Additionally, it's likely the peer will be forward secure, and no acks - // for these packets will be received, so mark the packet as handled. + // Since once you're forward secure, no unencrypted packets will be sent, + // crypto or otherwise. Unencrypted packets are neutered and abandoned, to + // ensure they are not retransmitted or considered lost from a congestion + // control perspective. pending_retransmissions_.erase(unacked_it->first); - unacked_it = MarkPacketHandled(unacked_it->first, - QuicTime::Delta::Zero()); - continue; + // TODO(ianswett): This may cause packets to linger forever in the + // UnackedPacketMap. + unacked_packets_.NeuterPacket(unacked_it->first); + unacked_packets_.SetNotPending(unacked_it->first); } ++unacked_it; } @@ -275,6 +276,9 @@ void QuicSentPacketManager::MarkForRetransmission( const TransmissionInfo& transmission_info = unacked_packets_.GetTransmissionInfo(sequence_number); LOG_IF(DFATAL, transmission_info.retransmittable_frames == NULL); + if (transmission_type != TLP_RETRANSMISSION) { + unacked_packets_.SetNotPending(sequence_number); + } // TODO(ianswett): Currently the RTO can fire while there are pending NACK // retransmissions for the same data, which is not ideal. if (ContainsKey(pending_retransmissions_, sequence_number)) { @@ -337,7 +341,11 @@ void QuicSentPacketManager::MarkPacketRevived( sequence_number, delta_largest_observed); } - unacked_packets_.NeuterIfPendingOrRemovePacket(sequence_number); + if (!transmission_info.pending) { + unacked_packets_.RemovePacket(sequence_number); + } else { + unacked_packets_.NeuterPacket(sequence_number); + } } QuicUnackedPacketMap::const_iterator QuicSentPacketManager::MarkPacketHandled( @@ -349,7 +357,7 @@ QuicUnackedPacketMap::const_iterator QuicSentPacketManager::MarkPacketHandled( } const TransmissionInfo& transmission_info = unacked_packets_.GetTransmissionInfo(sequence_number); - // If this packet is pending, remove it and inform the send algorithm. + // If this packet is pending, remove it. if (transmission_info.pending) { unacked_packets_.SetNotPending(sequence_number); } @@ -358,6 +366,13 @@ QuicUnackedPacketMap::const_iterator QuicSentPacketManager::MarkPacketHandled( SequenceNumberSet::reverse_iterator all_transmissions_it = all_transmissions.rbegin(); QuicPacketSequenceNumber newest_transmission = *all_transmissions_it; + // Remove the most recent packet, if it is pending retransmission. + pending_retransmissions_.erase(newest_transmission); + + // Two cases for MarkPacketHandled: + // 1) Handle the most recent or a crypto packet, so remove all transmissions. + // 2) Handle old transmission, keep all other pending transmissions, + // but disassociate them from one another. if (newest_transmission != sequence_number) { stats_->bytes_spuriously_retransmitted += transmission_info.bytes_sent; ++stats_->packets_spuriously_retransmitted; @@ -368,19 +383,25 @@ QuicUnackedPacketMap::const_iterator QuicSentPacketManager::MarkPacketHandled( ack_notifier_manager_.OnPacketAcked(newest_transmission, delta_largest_observed); + // TODO(ianswett): Instead of handling all crypto packets in a special way, + // only handle NULL encrypted packets in a special way. bool has_crypto_handshake = HasCryptoHandshake( unacked_packets_.GetTransmissionInfo(newest_transmission)); while (all_transmissions_it != all_transmissions.rend()) { QuicPacketSequenceNumber previous_transmission = *all_transmissions_it; - // If this packet was marked for retransmission, don't bother retransmitting - // it anymore. - pending_retransmissions_.erase(previous_transmission); + const TransmissionInfo& transmission_info = + unacked_packets_.GetTransmissionInfo(previous_transmission); + DCHECK(!ContainsKey(pending_retransmissions_, previous_transmission)); if (has_crypto_handshake) { // If it's a crypto handshake packet, discard it and all retransmissions, // since they won't be acked now that one has been processed. unacked_packets_.SetNotPending(previous_transmission); } - unacked_packets_.NeuterIfPendingOrRemovePacket(previous_transmission); + if (!transmission_info.pending) { + unacked_packets_.RemovePacket(previous_transmission); + } else { + unacked_packets_.NeuterPacket(previous_transmission); + } ++all_transmissions_it; } @@ -490,8 +511,6 @@ void QuicSentPacketManager::RetransmitCryptoPackets() { } packet_retransmitted = true; MarkForRetransmission(sequence_number, HANDSHAKE_RETRANSMISSION); - // Abandon all the crypto retransmissions now so they're not lost later. - unacked_packets_.SetNotPending(sequence_number); } DCHECK(packet_retransmitted) << "No crypto packets found to retransmit."; } @@ -529,10 +548,11 @@ void QuicSentPacketManager::RetransmitAllPackets() { bool packets_retransmitted = false; for (QuicUnackedPacketMap::const_iterator it = unacked_packets_.begin(); it != unacked_packets_.end(); ++it) { - unacked_packets_.SetNotPending(it->first); if (it->second.retransmittable_frames != NULL) { packets_retransmitted = true; MarkForRetransmission(it->first, RTO_RETRANSMISSION); + } else { + unacked_packets_.SetNotPending(it->first); } } @@ -582,16 +602,17 @@ void QuicSentPacketManager::InvokeLossDetection(QuicTime time) { // until it's known whether the FEC packet arrived. ++stats_->packets_lost; packets_lost_[sequence_number] = transmission_info; - unacked_packets_.SetNotPending(sequence_number); + DVLOG(1) << ENDPOINT << "Lost packet " << sequence_number; if (transmission_info.retransmittable_frames != NULL) { MarkForRetransmission(sequence_number, LOSS_RETRANSMISSION); } else { // Since we will not retransmit this, we need to remove it from // unacked_packets_. This is either the current transmission of - // a packet whose previous transmission has been acked, or it - // is a packet that has been TLP retransmitted. - unacked_packets_.NeuterIfPendingOrRemovePacket(sequence_number); + // a packet whose previous transmission has been acked, a packet that has + // been TLP retransmitted, or an FEC packet. + unacked_packets_.SetNotPending(sequence_number); + unacked_packets_.RemovePacket(sequence_number); } } } @@ -663,7 +684,7 @@ const QuicTime QuicSentPacketManager::GetRetransmissionTime() const { // Base the updated timer on the send time of the last packet. const QuicTime sent_time = unacked_packets_.GetLastPacketSentTime(); const QuicTime tlp_time = sent_time.Add(GetTailLossProbeDelay()); - // Ensure the tlp timer never gets set to a time in the past. + // Ensure the TLP timer never gets set to a time in the past. return QuicTime::Max(clock_->ApproximateNow(), tlp_time); } case RTO_MODE: { diff --git a/net/quic/quic_sent_packet_manager.h b/net/quic/quic_sent_packet_manager.h index 1af85c4..cca7f4c 100644 --- a/net/quic/quic_sent_packet_manager.h +++ b/net/quic/quic_sent_packet_manager.h @@ -96,7 +96,7 @@ class NET_EXPORT_PRIVATE QuicSentPacketManager { // Removes the retransmittable frames from all unencrypted packets to ensure // they don't get retransmitted. - void DiscardUnencryptedPackets(); + void NeuterUnencryptedPackets(); // Returns true if the unacked packet |sequence_number| has retransmittable // frames. This will only return false if the packet has been acked, if a diff --git a/net/quic/quic_sent_packet_manager_test.cc b/net/quic/quic_sent_packet_manager_test.cc index e44dca5..7d0bfed 100644 --- a/net/quic/quic_sent_packet_manager_test.cc +++ b/net/quic/quic_sent_packet_manager_test.cc @@ -69,22 +69,13 @@ class QuicSentPacketManagerTest : public ::testing::TestWithParam<bool> { void VerifyRetransmittablePackets(QuicPacketSequenceNumber* packets, size_t num_packets) { - SequenceNumberSet unacked = - QuicSentPacketManagerPeer::GetUnackedPackets(&manager_); - for (size_t i = 0; i < num_packets; ++i) { - EXPECT_TRUE(ContainsKey(unacked, packets[i])) << packets[i]; - } - size_t num_retransmittable = 0; - for (SequenceNumberSet::const_iterator it = unacked.begin(); - it != unacked.end(); ++it) { - if (manager_.HasRetransmittableFrames(*it)) { - ++num_retransmittable; - } - } EXPECT_EQ(num_packets, QuicSentPacketManagerPeer::GetNumRetransmittablePackets( &manager_)); - EXPECT_EQ(num_packets, num_retransmittable); + for (size_t i = 0; i < num_packets; ++i) { + EXPECT_TRUE(manager_.HasRetransmittableFrames(packets[i])) + << " packets[" << i << "]:" << packets[i]; + } } void ExpectAck(QuicPacketSequenceNumber largest_observed) { @@ -125,15 +116,18 @@ class QuicSentPacketManagerTest : public ::testing::TestWithParam<bool> { Pointwise(KeyEq(), lost_vector))); } + // Retransmits a packet as though it was a TLP retransmission, because TLP + // leaves the |old_sequence_number| pending. + // TODO(ianswett): Test with transmission types besides TLP. void RetransmitPacket(QuicPacketSequenceNumber old_sequence_number, QuicPacketSequenceNumber new_sequence_number) { QuicSentPacketManagerPeer::MarkForRetransmission( - &manager_, old_sequence_number, LOSS_RETRANSMISSION); + &manager_, old_sequence_number, TLP_RETRANSMISSION); EXPECT_TRUE(manager_.HasPendingRetransmissions()); QuicSentPacketManager::PendingRetransmission next_retransmission = manager_.NextPendingRetransmission(); EXPECT_EQ(old_sequence_number, next_retransmission.sequence_number); - EXPECT_EQ(LOSS_RETRANSMISSION, + EXPECT_EQ(TLP_RETRANSMISSION, next_retransmission.transmission_type); manager_.OnRetransmittedPacket(old_sequence_number, new_sequence_number); EXPECT_TRUE(QuicSentPacketManagerPeer::IsRetransmission( @@ -197,7 +191,6 @@ class QuicSentPacketManagerTest : public ::testing::TestWithParam<bool> { SerializedPacket packet(CreateDataPacket(sequence_number)); packet.retransmittable_frames->AddStreamFrame( new QuicStreamFrame(1, false, 0, IOVector())); - packet.retransmittable_frames->set_encryption_level(ENCRYPTION_NONE); manager_.OnSerializedPacket(packet); manager_.OnPacketSent(sequence_number, clock_.ApproximateNow(), packet.packet->length(), NOT_RETRANSMISSION, @@ -297,7 +290,7 @@ TEST_F(QuicSentPacketManagerTest, RetransmitThenAck) { TEST_F(QuicSentPacketManagerTest, RetransmitThenAckBeforeSend) { SendDataPacket(1); QuicSentPacketManagerPeer::MarkForRetransmission( - &manager_, 1, LOSS_RETRANSMISSION); + &manager_, 1, TLP_RETRANSMISSION); EXPECT_TRUE(manager_.HasPendingRetransmissions()); // Ack 1. @@ -557,9 +550,9 @@ TEST_F(QuicSentPacketManagerTest, TruncatedAck) { manager_.OnIncomingAck(received_info, clock_.Now()); // High water mark will be raised. - QuicPacketSequenceNumber unacked[] = { 2, 3, 4 }; + QuicPacketSequenceNumber unacked[] = { 2, 3, 4, 5 }; VerifyUnackedPackets(unacked, arraysize(unacked)); - QuicPacketSequenceNumber retransmittable[] = { 4 }; + QuicPacketSequenceNumber retransmittable[] = { 5 }; VerifyRetransmittablePackets(retransmittable, arraysize(retransmittable)); } @@ -988,25 +981,6 @@ TEST_F(QuicSentPacketManagerTest, EXPECT_FALSE(QuicSentPacketManagerPeer::HasPendingPackets(&manager_)); } -TEST_F(QuicSentPacketManagerTest, - CryptoHandshakeRetransmissionThenAbandonAll) { - // Send 1 crypto packet. - SendCryptoPacket(1); - EXPECT_TRUE(QuicSentPacketManagerPeer::HasUnackedCryptoPackets(&manager_)); - - // Retransmit the crypto packet as 2. - manager_.OnRetransmissionTimeout(); - RetransmitNextPacket(2); - - // Now discard all unacked unencrypted packets, which occurs when the - // connection goes forward secure. - manager_.DiscardUnencryptedPackets(); - VerifyUnackedPackets(NULL, 0); - EXPECT_FALSE(manager_.HasPendingRetransmissions()); - EXPECT_FALSE(QuicSentPacketManagerPeer::HasUnackedCryptoPackets(&manager_)); - EXPECT_FALSE(QuicSentPacketManagerPeer::HasPendingPackets(&manager_)); -} - TEST_F(QuicSentPacketManagerTest, TailLossProbeTimeoutUnsentDataPacket) { QuicSentPacketManagerPeer::SetMaxTailLossProbes(&manager_, 2); // Serialize two data packets and send the latter. diff --git a/net/quic/quic_session.cc b/net/quic/quic_session.cc index 66f150b..6d749ec 100644 --- a/net/quic/quic_session.cc +++ b/net/quic/quic_session.cc @@ -422,7 +422,7 @@ void QuicSession::OnCryptoHandshakeEvent(CryptoHandshakeEvent event) { << "Handshake confirmed without parameter negotiation."; // Discard originally encrypted packets, since they can't be decrypted by // the peer. - connection_->DiscardUnencryptedPackets(); + connection_->NeuterUnencryptedPackets(); connection_->SetOverallConnectionTimeout(QuicTime::Delta::Infinite()); max_open_streams_ = config_.max_streams_per_connection(); break; diff --git a/net/quic/quic_unacked_packet_map.cc b/net/quic/quic_unacked_packet_map.cc index d75991ed..a21d4a5 100644 --- a/net/quic/quic_unacked_packet_map.cc +++ b/net/quic/quic_unacked_packet_map.cc @@ -91,7 +91,7 @@ void QuicUnackedPacketMap::ClearPreviousRetransmissions(size_t num_to_clear) { } ++it; - NeuterIfPendingOrRemovePacket(sequence_number); + RemovePacket(sequence_number); --num_to_clear; } } @@ -119,7 +119,7 @@ void QuicUnackedPacketMap::NackPacket(QuicPacketSequenceNumber sequence_number, it->second.nack_count = max(min_nacks, it->second.nack_count); } -void QuicUnackedPacketMap::NeuterIfPendingOrRemovePacket( +void QuicUnackedPacketMap::RemovePacket( QuicPacketSequenceNumber sequence_number) { UnackedPacketMap::iterator it = unacked_packets_.find(sequence_number); if (it == unacked_packets_.end()) { @@ -127,6 +127,34 @@ void QuicUnackedPacketMap::NeuterIfPendingOrRemovePacket( return; } TransmissionInfo* transmission_info = &it->second; + DCHECK(!transmission_info->pending); + MaybeRemoveRetransmittableFrames(transmission_info); + transmission_info->all_transmissions->erase(sequence_number); + if (transmission_info->all_transmissions->empty()) { + delete transmission_info->all_transmissions; + } + unacked_packets_.erase(it); +} + +void QuicUnackedPacketMap::NeuterPacket( + QuicPacketSequenceNumber sequence_number) { + UnackedPacketMap::iterator it = unacked_packets_.find(sequence_number); + if (it == unacked_packets_.end()) { + LOG(DFATAL) << "packet is not unacked: " << sequence_number; + return; + } + TransmissionInfo* transmission_info = &it->second; + // TODO(ianswett): Ensure packets are pending before neutering them. + MaybeRemoveRetransmittableFrames(transmission_info); + if (transmission_info->all_transmissions->size() > 1) { + transmission_info->all_transmissions->erase(sequence_number); + transmission_info->all_transmissions = new SequenceNumberSet(); + transmission_info->all_transmissions->insert(sequence_number); + } +} + +void QuicUnackedPacketMap::MaybeRemoveRetransmittableFrames( + TransmissionInfo* transmission_info) { if (transmission_info->retransmittable_frames != NULL) { if (transmission_info->retransmittable_frames->HasCryptoHandshake() == IS_HANDSHAKE) { @@ -135,21 +163,6 @@ void QuicUnackedPacketMap::NeuterIfPendingOrRemovePacket( delete transmission_info->retransmittable_frames; transmission_info->retransmittable_frames = NULL; } - if (transmission_info->pending) { - // Neuter it so it can't be retransmitted. - if (transmission_info->all_transmissions->size() > 1) { - transmission_info->all_transmissions->erase(sequence_number); - transmission_info->all_transmissions = new SequenceNumberSet(); - transmission_info->all_transmissions->insert(sequence_number); - } - } else { - // Remove it. - transmission_info->all_transmissions->erase(sequence_number); - if (transmission_info->all_transmissions->empty()) { - delete transmission_info->all_transmissions; - } - unacked_packets_.erase(it); - } } // static @@ -165,13 +178,6 @@ bool QuicUnackedPacketMap::IsUnacked( return ContainsKey(unacked_packets_, sequence_number); } -bool QuicUnackedPacketMap::IsPending( - QuicPacketSequenceNumber sequence_number) const { - const TransmissionInfo* transmission_info = - FindOrNull(unacked_packets_, sequence_number); - return transmission_info != NULL && transmission_info->pending; -} - void QuicUnackedPacketMap::SetNotPending( QuicPacketSequenceNumber sequence_number) { UnackedPacketMap::iterator it = unacked_packets_.find(sequence_number); @@ -286,15 +292,6 @@ QuicUnackedPacketMap::GetLeastUnackedSentPacket() const { return unacked_packets_.begin()->first; } -SequenceNumberSet QuicUnackedPacketMap::GetUnackedPackets() const { - SequenceNumberSet unacked_packets; - for (UnackedPacketMap::const_iterator it = unacked_packets_.begin(); - it != unacked_packets_.end(); ++it) { - unacked_packets.insert(it->first); - } - return unacked_packets; -} - void QuicUnackedPacketMap::SetSent(QuicPacketSequenceNumber sequence_number, QuicTime sent_time, QuicByteCount bytes_sent, diff --git a/net/quic/quic_unacked_packet_map.h b/net/quic/quic_unacked_packet_map.h index 011760a..759bd7b 100644 --- a/net/quic/quic_unacked_packet_map.h +++ b/net/quic/quic_unacked_packet_map.h @@ -31,9 +31,6 @@ class NET_EXPORT_PRIVATE QuicUnackedPacketMap { // Returns true if the packet |sequence_number| is unacked. bool IsUnacked(QuicPacketSequenceNumber sequence_number) const; - // Returns true if the packet |sequence_number| is pending. - bool IsPending(QuicPacketSequenceNumber sequence_number) const; - // Sets the nack count to the max of the current nack count and |min_nacks|. void NackPacket(QuicPacketSequenceNumber sequence_number, size_t min_nacks); @@ -72,10 +69,6 @@ class NET_EXPORT_PRIVATE QuicUnackedPacketMap { // been acked by the peer. If there are no unacked packets, returns 0. QuicPacketSequenceNumber GetLeastUnackedSentPacket() const; - // Returns the set of sequence numbers of all unacked packets. - // Test only. - SequenceNumberSet GetUnackedPackets() const; - // Sets a packet as sent with the sent time |sent_time|. Marks the packet // as pending and tracks the |bytes_sent| if |set_pending| is true. // Packets marked as pending are expected to be marked as missing when they @@ -120,15 +113,21 @@ class NET_EXPORT_PRIVATE QuicUnackedPacketMap { // Returns true if there are any pending crypto packets. bool HasPendingCryptoPackets() const; - // Deletes the retransmittable frames associated with the packet and removes - // it from unacked packets if it's not pending. + // Removes entries from the unacked packet map, and deletes + // the retransmittable frames associated with the packet. // Does not remove any previous or subsequent transmissions of this packet. - void NeuterIfPendingOrRemovePacket(QuicPacketSequenceNumber sequence_number); + void RemovePacket(QuicPacketSequenceNumber sequence_number); + + // Neuters the specified packet. Deletes any retransmittable + // frames, and sets all_transmissions to only include itself. + void NeuterPacket(QuicPacketSequenceNumber sequence_number); // Returns true if the packet has been marked as sent by SetSent. static bool IsSentAndNotPending(const TransmissionInfo& transmission_info); private: + void MaybeRemoveRetransmittableFrames(TransmissionInfo* transmission_info); + QuicPacketSequenceNumber largest_sent_packet_; // Newly serialized retransmittable and fec packets are added to this map, diff --git a/net/quic/test_tools/quic_sent_packet_manager_peer.cc b/net/quic/test_tools/quic_sent_packet_manager_peer.cc index 479711b..7f72615 100644 --- a/net/quic/test_tools/quic_sent_packet_manager_peer.cc +++ b/net/quic/test_tools/quic_sent_packet_manager_peer.cc @@ -113,12 +113,6 @@ size_t QuicSentPacketManagerPeer::GetNumRetransmittablePackets( } // static -SequenceNumberSet QuicSentPacketManagerPeer::GetUnackedPackets( - const QuicSentPacketManager* sent_packet_manager) { - return sent_packet_manager->unacked_packets_.GetUnackedPackets(); -} - -// static QuicByteCount QuicSentPacketManagerPeer::GetBytesInFlight( const QuicSentPacketManager* sent_packet_manager) { return sent_packet_manager->unacked_packets_.bytes_in_flight(); diff --git a/net/quic/test_tools/quic_sent_packet_manager_peer.h b/net/quic/test_tools/quic_sent_packet_manager_peer.h index 105388d..fed8015 100644 --- a/net/quic/test_tools/quic_sent_packet_manager_peer.h +++ b/net/quic/test_tools/quic_sent_packet_manager_peer.h @@ -60,9 +60,6 @@ class QuicSentPacketManagerPeer { static size_t GetNumRetransmittablePackets( const QuicSentPacketManager* sent_packet_manager); - static SequenceNumberSet GetUnackedPackets( - const QuicSentPacketManager* sent_packet_manager); - static QuicByteCount GetBytesInFlight( const QuicSentPacketManager* sent_packet_manager); diff --git a/net/tools/quic/end_to_end_test.cc b/net/tools/quic/end_to_end_test.cc index ef394b9..88788cc 100644 --- a/net/tools/quic/end_to_end_test.cc +++ b/net/tools/quic/end_to_end_test.cc @@ -28,6 +28,7 @@ #include "net/quic/test_tools/quic_test_utils.h" #include "net/quic/test_tools/reliable_quic_stream_peer.h" #include "net/test/gtest_util.h" +#include "net/tools/epoll_server/epoll_server.h" #include "net/tools/quic/quic_epoll_connection_helper.h" #include "net/tools/quic/quic_in_memory_cache.h" #include "net/tools/quic/quic_packet_writer_wrapper.h" @@ -46,6 +47,7 @@ using base::StringPiece; using base::WaitableEvent; +using net::EpollServer; using net::test::GenerateBody; using net::test::QuicConnectionPeer; using net::test::QuicSessionPeer; @@ -481,7 +483,7 @@ TEST_P(EndToEndTest, DISABLED_LargePostNoPacketLoss) { client_->client()->WaitForCryptoHandshakeConfirmed(); - // 1 Mb body. + // 1 MB body. string body; GenerateBody(&body, 1024 * 1024); @@ -499,7 +501,7 @@ TEST_P(EndToEndTest, LargePostNoPacketLoss1sRTT) { client_->client()->WaitForCryptoHandshakeConfirmed(); - // 1 Mb body. + // 100 KB body. string body; GenerateBody(&body, 100 * 1024); @@ -521,7 +523,7 @@ TEST_P(EndToEndTest, LargePostWithPacketLoss) { client_->client()->WaitForCryptoHandshakeConfirmed(); SetPacketLossPercentage(30); - // 10 Kb body. + // 10 KB body. string body; GenerateBody(&body, 1024 * 10); @@ -530,42 +532,42 @@ TEST_P(EndToEndTest, LargePostWithPacketLoss) { request.AddBody(body, true); EXPECT_EQ(kFooResponseBody, client_->SendCustomSynchronousRequest(request)); + VerifyCleanConnection(true); } -TEST_P(EndToEndTest, LargePostNoPacketLossWithDelayAndReordering) { +TEST_P(EndToEndTest, LargePostWithPacketLossAndBlockedSocket) { + // Connect with lower fake packet loss than we'd like to test. Until + // b/10126687 is fixed, losing handshake packets is pretty brutal. + SetPacketLossPercentage(5); ASSERT_TRUE(Initialize()); + // Wait for the server SHLO before upping the packet loss. client_->client()->WaitForCryptoHandshakeConfirmed(); - // Both of these must be called when the writer is not actively used. - SetPacketSendDelay(QuicTime::Delta::FromMilliseconds(2)); - SetReorderPercentage(30); + SetPacketLossPercentage(10); + client_writer_->set_fake_blocked_socket_percentage(10); - // 1 Mb body. + // 10 KB body. string body; - GenerateBody(&body, 1024 * 1024); + GenerateBody(&body, 1024 * 10); HTTPMessage request(HttpConstants::HTTP_1_1, HttpConstants::POST, "/foo"); request.AddBody(body, true); EXPECT_EQ(kFooResponseBody, client_->SendCustomSynchronousRequest(request)); - VerifyCleanConnection(true); } -TEST_P(EndToEndTest, LargePostWithPacketLossAndBlockedSocket) { - // Connect with lower fake packet loss than we'd like to test. Until - // b/10126687 is fixed, losing handshake packets is pretty brutal. - SetPacketLossPercentage(5); +TEST_P(EndToEndTest, LargePostNoPacketLossWithDelayAndReordering) { ASSERT_TRUE(Initialize()); - // Wait for the server SHLO before upping the packet loss. client_->client()->WaitForCryptoHandshakeConfirmed(); - SetPacketLossPercentage(10); - client_writer_->set_fake_blocked_socket_percentage(10); + // Both of these must be called when the writer is not actively used. + SetPacketSendDelay(QuicTime::Delta::FromMilliseconds(2)); + SetReorderPercentage(30); - // 10 Kb body. + // 1 MB body. string body; - GenerateBody(&body, 1024 * 10); + GenerateBody(&body, 1024 * 1024); HTTPMessage request(HttpConstants::HTTP_1_1, HttpConstants::POST, "/foo"); @@ -655,19 +657,17 @@ TEST_P(EndToEndTest, LargePostFEC) { VerifyCleanConnection(true); } -// TODO(rtenneti): DISABLED_LargePostLargeBuffer seems to be flaky. -// http://crbug.com/370087. -TEST_P(EndToEndTest, DISABLED_LargePostLargeBuffer) { +TEST_P(EndToEndTest, LargePostSmallBandwidthLargeBuffer) { ASSERT_TRUE(Initialize()); SetPacketSendDelay(QuicTime::Delta::FromMicroseconds(1)); - // 1Mbit per second with a 128k buffer from server to client. Wireless + // 256KB per second with a 256k buffer from server to client. Wireless // clients commonly have larger buffers, but our max CWND is 200. server_writer_->set_max_bandwidth_and_buffer_size( - QuicBandwidth::FromBytesPerSecond(256 * 1024), 128 * 1024); + QuicBandwidth::FromBytesPerSecond(256 * 1024), 256 * 1024); client_->client()->WaitForCryptoHandshakeConfirmed(); - // 1 Mb body. + // 1 MB body. string body; GenerateBody(&body, 1024 * 1024); @@ -676,6 +676,8 @@ TEST_P(EndToEndTest, DISABLED_LargePostLargeBuffer) { request.AddBody(body, true); EXPECT_EQ(kFooResponseBody, client_->SendCustomSynchronousRequest(request)); + // This connection will not drop packets, because the buffer size is larger + // than the default receive window. VerifyCleanConnection(false); } @@ -790,7 +792,7 @@ TEST_P(EndToEndTest, DISABLED_LimitCongestionWindowAndRTT) { // Now use the negotiated limits with packet loss. SetPacketLossPercentage(30); - // 10 Kb body. + // 10 KB body. string body; GenerateBody(&body, 1024 * 10); @@ -953,7 +955,10 @@ class WrongAddressWriter : public QuicPacketWriterWrapper { IPEndPoint self_address_; }; -TEST_P(EndToEndTest, ConnectionMigration) { +TEST_P(EndToEndTest, ConnectionMigrationClientIPChanged) { + // Tests that the client's IP can not change during an established QUIC + // connection. If it changes, the connection is closed by the server as we do + // not yet support IP migration. ASSERT_TRUE(Initialize()); EXPECT_EQ(kFooResponseBody, client_->SendSynchronousRequest("/foo")); @@ -971,6 +976,59 @@ TEST_P(EndToEndTest, ConnectionMigration) { EXPECT_EQ(QUIC_ERROR_MIGRATING_ADDRESS, client_->connection_error()); } +TEST_P(EndToEndTest, ConnectionMigrationClientPortChanged) { + // Tests that the client's port can change during an established QUIC + // connection, and that doing so does not result in the connection being + // closed by the server. + FLAGS_quic_allow_port_migration = true; + + ASSERT_TRUE(Initialize()); + + EXPECT_EQ(kFooResponseBody, client_->SendSynchronousRequest("/foo")); + EXPECT_EQ(200u, client_->response_headers()->parsed_response_code()); + + // Store the client address which was used to send the first request. + IPEndPoint old_address = client_->client()->client_address(); + + // Stop listening on the old FD. + EpollServer* eps = client_->client()->epoll_server(); + int old_fd = client_->client()->fd(); + eps->UnregisterFD(old_fd); + close(old_fd); + + // Create a new socket, which will result in a new ephemeral port. + QuicClientPeer::CreateUDPSocket(client_->client()); + + // The packet writer needs to be updated to use the new FD. + client_->client()->CreateQuicPacketWriter(); + + // Change the internal state of the client and connection to use the new port, + // this is done because in a real NAT rebinding the client wouldn't see any + // port change, and so expects no change to incoming port. + // This is kind of ugly, but needed as we are simply swapping out the client + // FD rather than any more complex NAT rebinding simulation. + int new_port = client_->client()->client_address().port(); + QuicClientPeer::SetClientPort(client_->client(), new_port); + QuicConnectionPeer::SetSelfAddress( + client_->client()->session()->connection(), + IPEndPoint( + client_->client()->session()->connection()->self_address().address(), + new_port)); + + // Register the new FD for epoll events. + int new_fd = client_->client()->fd(); + eps->RegisterFD(new_fd, client_->client(), EPOLLIN | EPOLLOUT | EPOLLET); + + // Send a second request, using the new FD. + EXPECT_EQ(kBarResponseBody, client_->SendSynchronousRequest("/bar")); + EXPECT_EQ(200u, client_->response_headers()->parsed_response_code()); + + // Verify that the client's ephemeral port is different. + IPEndPoint new_address = client_->client()->client_address(); + EXPECT_EQ(old_address.address(), new_address.address()); + EXPECT_NE(old_address.port(), new_address.port()); +} + TEST_P(EndToEndTest, DifferentFlowControlWindows) { // Client and server can set different initial flow control receive windows. // These are sent in CHLO/SHLO. Tests that these values are exchanged properly diff --git a/net/tools/quic/quic_client.cc b/net/tools/quic/quic_client.cc index 346010f..cb2b408 100644 --- a/net/tools/quic/quic_client.cc +++ b/net/tools/quic/quic_client.cc @@ -83,6 +83,16 @@ bool QuicClient::Initialize() { epoll_server_.set_timeout_in_us(50 * 1000); crypto_config_.SetDefaults(); + if (!CreateUDPSocket()) { + return false; + } + + epoll_server_.RegisterFD(fd_, this, kEpollFlags); + initialized_ = true; + return true; +} + +bool QuicClient::CreateUDPSocket() { int address_family = server_address_.GetSockAddrFamily(); fd_ = socket(address_family, SOCK_DGRAM | SOCK_NONBLOCK, IPPROTO_UDP); if (fd_ < 0) { @@ -153,8 +163,6 @@ bool QuicClient::Initialize() { LOG(ERROR) << "Unable to get self address. Error: " << strerror(errno); } - epoll_server_.RegisterFD(fd_, this, kEpollFlags); - initialized_ = true; return true; } diff --git a/net/tools/quic/quic_client.h b/net/tools/quic/quic_client.h index 688fef0..5568b1e 100644 --- a/net/tools/quic/quic_client.h +++ b/net/tools/quic/quic_client.h @@ -181,6 +181,10 @@ class QuicClient : public EpollCallbackInterface, private: friend class net::tools::test::QuicClientPeer; + // Used during initialization: creates the UDP socket FD, sets socket options, + // and binds the socket to our address. + bool CreateUDPSocket(); + // Read a UDP packet and hand it to the framer. bool ReadAndProcessPacket(); diff --git a/net/tools/quic/test_tools/quic_client_peer.cc b/net/tools/quic/test_tools/quic_client_peer.cc index 9cd9b37..89f1c82 100644 --- a/net/tools/quic/test_tools/quic_client_peer.cc +++ b/net/tools/quic/test_tools/quic_client_peer.cc @@ -15,6 +15,16 @@ QuicCryptoClientConfig* QuicClientPeer::GetCryptoConfig(QuicClient* client) { return &client->crypto_config_; } +// static +bool QuicClientPeer::CreateUDPSocket(QuicClient* client) { + return client->CreateUDPSocket(); +} + +// static +void QuicClientPeer::SetClientPort(QuicClient* client, int port) { + client->client_address_ = IPEndPoint(client->client_address_.address(), port); +} + } // namespace test } // namespace tools } // namespace net diff --git a/net/tools/quic/test_tools/quic_client_peer.h b/net/tools/quic/test_tools/quic_client_peer.h index f4f9f7d..b26fc6d 100644 --- a/net/tools/quic/test_tools/quic_client_peer.h +++ b/net/tools/quic/test_tools/quic_client_peer.h @@ -20,6 +20,8 @@ namespace test { class QuicClientPeer { public: static QuicCryptoClientConfig* GetCryptoConfig(QuicClient* client); + static bool CreateUDPSocket(QuicClient* client); + static void SetClientPort(QuicClient* client, int port); private: DISALLOW_COPY_AND_ASSIGN(QuicClientPeer); |