summaryrefslogtreecommitdiffstats
path: root/net
diff options
context:
space:
mode:
authorrtenneti@chromium.org <rtenneti@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98>2014-05-13 07:44:22 +0000
committerrtenneti@chromium.org <rtenneti@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98>2014-05-13 07:44:22 +0000
commit6d9ca3b790e9cc01e1ae0b39730c27423e2e7d7f (patch)
treee34edb10a3166cfdf5f4e0f71e8e8e754b801ffe /net
parent4c900827947228c8642a594d682817444cd4a559 (diff)
downloadchromium_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.cc2
-rw-r--r--net/quic/quic_connection.cc87
-rw-r--r--net/quic/quic_connection.h32
-rw-r--r--net/quic/quic_flags.cc4
-rw-r--r--net/quic/quic_flags.h1
-rw-r--r--net/quic/quic_reliable_client_stream.cc3
-rw-r--r--net/quic/quic_sent_packet_manager.cc65
-rw-r--r--net/quic/quic_sent_packet_manager.h2
-rw-r--r--net/quic/quic_sent_packet_manager_test.cc50
-rw-r--r--net/quic/quic_session.cc2
-rw-r--r--net/quic/quic_unacked_packet_map.cc63
-rw-r--r--net/quic/quic_unacked_packet_map.h19
-rw-r--r--net/quic/test_tools/quic_sent_packet_manager_peer.cc6
-rw-r--r--net/quic/test_tools/quic_sent_packet_manager_peer.h3
-rw-r--r--net/tools/quic/end_to_end_test.cc112
-rw-r--r--net/tools/quic/quic_client.cc12
-rw-r--r--net/tools/quic/quic_client.h4
-rw-r--r--net/tools/quic/test_tools/quic_client_peer.cc10
-rw-r--r--net/tools/quic/test_tools/quic_client_peer.h2
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);