Commit 6d9ca3b7 authored by rtenneti@chromium.org's avatar rtenneti@chromium.org

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
parent 4c900827
...@@ -171,7 +171,7 @@ TEST_F(TcpLossAlgorithmTest, DontEarlyRetransmitNeuteredPacket) { ...@@ -171,7 +171,7 @@ TEST_F(TcpLossAlgorithmTest, DontEarlyRetransmitNeuteredPacket) {
// Early retransmit when the final packet gets acked and the first is nacked. // Early retransmit when the final packet gets acked and the first is nacked.
unacked_packets_.SetNotPending(2); unacked_packets_.SetNotPending(2);
unacked_packets_.NackPacket(1, 1); unacked_packets_.NackPacket(1, 1);
unacked_packets_.NeuterIfPendingOrRemovePacket(1); unacked_packets_.NeuterPacket(1);
VerifyLosses(2, NULL, 0); VerifyLosses(2, NULL, 0);
EXPECT_EQ(QuicTime::Zero(), loss_algorithm_.GetLossTimeout()); EXPECT_EQ(QuicTime::Zero(), loss_algorithm_.GetLossTimeout());
} }
......
...@@ -204,6 +204,7 @@ QuicConnection::QuicConnection(QuicConnectionId connection_id, ...@@ -204,6 +204,7 @@ QuicConnection::QuicConnection(QuicConnectionId connection_id,
random_generator_(helper->GetRandomGenerator()), random_generator_(helper->GetRandomGenerator()),
connection_id_(connection_id), connection_id_(connection_id),
peer_address_(address), peer_address_(address),
migrating_peer_port_(0),
last_packet_revived_(false), last_packet_revived_(false),
last_size_(0), last_size_(0),
last_decrypted_packet_level_(ENCRYPTION_NONE), last_decrypted_packet_level_(ENCRYPTION_NONE),
...@@ -234,7 +235,10 @@ QuicConnection::QuicConnection(QuicConnectionId connection_id, ...@@ -234,7 +235,10 @@ QuicConnection::QuicConnection(QuicConnectionId connection_id,
version_negotiation_state_(START_NEGOTIATION), version_negotiation_state_(START_NEGOTIATION),
is_server_(is_server), is_server_(is_server),
connected_(true), 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_(
max_flow_control_receive_window_bytes) { max_flow_control_receive_window_bytes) {
if (max_flow_control_receive_window_bytes_ < kDefaultFlowControlSendWindow) { if (max_flow_control_receive_window_bytes_ < kDefaultFlowControlSendWindow) {
...@@ -1105,18 +1109,7 @@ void QuicConnection::ProcessUdpPacket(const IPEndPoint& self_address, ...@@ -1105,18 +1109,7 @@ void QuicConnection::ProcessUdpPacket(const IPEndPoint& self_address,
last_packet_revived_ = false; last_packet_revived_ = false;
last_size_ = packet.length(); last_size_ = packet.length();
address_migrating_ = false; CheckForAddressMigration(self_address, peer_address);
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;
}
stats_.bytes_received += packet.length(); stats_.bytes_received += packet.length();
++stats_.packets_received; ++stats_.packets_received;
...@@ -1141,20 +1134,45 @@ void QuicConnection::ProcessUdpPacket(const IPEndPoint& self_address, ...@@ -1141,20 +1134,45 @@ void QuicConnection::ProcessUdpPacket(const IPEndPoint& self_address,
SetPingAlarm(); 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() { void QuicConnection::OnCanWrite() {
DCHECK(!writer_->IsWriteBlocked()); DCHECK(!writer_->IsWriteBlocked());
WriteQueuedPackets(); WriteQueuedPackets();
WritePendingRetransmissions(); WritePendingRetransmissions();
IsHandshake pending_handshake = visitor_->HasPendingHandshake() ?
IS_HANDSHAKE : NOT_HANDSHAKE;
// Sending queued packets may have caused the socket to become write blocked, // Sending queued packets may have caused the socket to become write blocked,
// or the congestion manager to prohibit sending. If we've sent everything // 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 // we had queued and we're still not blocked, let the visitor know it can
// write more. // write more.
if (!CanWrite(NOT_RETRANSMISSION, HAS_RETRANSMITTABLE_DATA, if (!CanWrite(NOT_RETRANSMISSION, HAS_RETRANSMITTABLE_DATA)) {
pending_handshake)) {
return; return;
} }
...@@ -1166,11 +1184,8 @@ void QuicConnection::OnCanWrite() { ...@@ -1166,11 +1184,8 @@ void QuicConnection::OnCanWrite() {
// After the visitor writes, it may have caused the socket to become write // After the visitor writes, it may have caused the socket to become write
// blocked or the congestion manager to prohibit sending, so check again. // 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() && if (visitor_->HasPendingWrites() && !resume_writes_alarm_->IsSet() &&
CanWrite(NOT_RETRANSMISSION, HAS_RETRANSMITTABLE_DATA, CanWrite(NOT_RETRANSMISSION, HAS_RETRANSMITTABLE_DATA)) {
pending_handshake)) {
// We're not write blocked, but some stream didn't write out all of its // 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 // bytes. Register for 'immediate' resumption so we'll keep writing after
// other connections and events have had a chance to use the thread. // other connections and events have had a chance to use the thread.
...@@ -1185,12 +1200,23 @@ void QuicConnection::WriteIfNotBlocked() { ...@@ -1185,12 +1200,23 @@ void QuicConnection::WriteIfNotBlocked() {
} }
bool QuicConnection::ProcessValidatedPacket() { 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( SendConnectionCloseWithDetails(
QUIC_ERROR_MIGRATING_ADDRESS, QUIC_ERROR_MIGRATING_ADDRESS,
"Address migration is not yet a supported feature"); "IP or port migration is not yet a supported feature");
return false; 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(); time_of_last_received_packet_ = clock_->Now();
DVLOG(1) << ENDPOINT << "time of last received packet: " DVLOG(1) << ENDPOINT << "time of last received packet: "
<< time_of_last_received_packet_.ToDebuggingValue(); << time_of_last_received_packet_.ToDebuggingValue();
...@@ -1230,8 +1256,7 @@ void QuicConnection::WritePendingRetransmissions() { ...@@ -1230,8 +1256,7 @@ void QuicConnection::WritePendingRetransmissions() {
const QuicSentPacketManager::PendingRetransmission pending = const QuicSentPacketManager::PendingRetransmission pending =
sent_packet_manager_.NextPendingRetransmission(); sent_packet_manager_.NextPendingRetransmission();
if (GetPacketType(&pending.retransmittable_frames) == NORMAL && if (GetPacketType(&pending.retransmittable_frames) == NORMAL &&
!CanWrite(pending.transmission_type, HAS_RETRANSMITTABLE_DATA, !CanWrite(pending.transmission_type, HAS_RETRANSMITTABLE_DATA)) {
pending.retransmittable_frames.HasCryptoHandshake())) {
break; break;
} }
...@@ -1269,8 +1294,8 @@ void QuicConnection::RetransmitUnackedPackets( ...@@ -1269,8 +1294,8 @@ void QuicConnection::RetransmitUnackedPackets(
WriteIfNotBlocked(); WriteIfNotBlocked();
} }
void QuicConnection::DiscardUnencryptedPackets() { void QuicConnection::NeuterUnencryptedPackets() {
sent_packet_manager_.DiscardUnencryptedPackets(); sent_packet_manager_.NeuterUnencryptedPackets();
// This may have changed the retransmission timer, so re-arm it. // This may have changed the retransmission timer, so re-arm it.
retransmission_alarm_->Cancel(); retransmission_alarm_->Cancel();
QuicTime retransmission_time = sent_packet_manager_.GetRetransmissionTime(); QuicTime retransmission_time = sent_packet_manager_.GetRetransmissionTime();
...@@ -1289,12 +1314,11 @@ bool QuicConnection::ShouldGeneratePacket( ...@@ -1289,12 +1314,11 @@ bool QuicConnection::ShouldGeneratePacket(
return true; return true;
} }
return CanWrite(transmission_type, retransmittable, handshake); return CanWrite(transmission_type, retransmittable);
} }
bool QuicConnection::CanWrite(TransmissionType transmission_type, bool QuicConnection::CanWrite(TransmissionType transmission_type,
HasRetransmittableData retransmittable, HasRetransmittableData retransmittable) {
IsHandshake handshake) {
if (writer_->IsWriteBlocked()) { if (writer_->IsWriteBlocked()) {
visitor_->OnWriteBlocked(); visitor_->OnWriteBlocked();
return false; return false;
...@@ -1342,8 +1366,7 @@ bool QuicConnection::WritePacket(QueuedPacket packet) { ...@@ -1342,8 +1366,7 @@ bool QuicConnection::WritePacket(QueuedPacket packet) {
// TODO(ianswett): The congestion control should have been consulted before // TODO(ianswett): The congestion control should have been consulted before
// serializing the packet, so this could be turned into a LOG_IF(DFATAL). // serializing the packet, so this could be turned into a LOG_IF(DFATAL).
if (packet.type == NORMAL && !CanWrite(packet.transmission_type, if (packet.type == NORMAL && !CanWrite(packet.transmission_type,
packet.retransmittable, packet.retransmittable)) {
packet.handshake)) {
return false; return false;
} }
......
...@@ -417,9 +417,9 @@ class NET_EXPORT_PRIVATE QuicConnection ...@@ -417,9 +417,9 @@ class NET_EXPORT_PRIVATE QuicConnection
// initially encrypted packets when the initial encrypter changes. // initially encrypted packets when the initial encrypter changes.
void RetransmitUnackedPackets(RetransmissionType retransmission_type); 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. // 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 // Changes the encrypter used for level |level| to |encrypter|. The function
// takes ownership of |encrypter|. // takes ownership of |encrypter|.
...@@ -458,8 +458,7 @@ class NET_EXPORT_PRIVATE QuicConnection ...@@ -458,8 +458,7 @@ class NET_EXPORT_PRIVATE QuicConnection
} }
bool CanWrite(TransmissionType transmission_type, bool CanWrite(TransmissionType transmission_type,
HasRetransmittableData retransmittable, HasRetransmittableData retransmittable);
IsHandshake handshake);
uint32 max_flow_control_receive_window_bytes() const { uint32 max_flow_control_receive_window_bytes() const {
return max_flow_control_receive_window_bytes_; return max_flow_control_receive_window_bytes_;
...@@ -611,6 +610,11 @@ class NET_EXPORT_PRIVATE QuicConnection ...@@ -611,6 +610,11 @@ class NET_EXPORT_PRIVATE QuicConnection
// Sets the ping alarm to the appropriate value, if any. // Sets the ping alarm to the appropriate value, if any.
void SetPingAlarm(); 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_; QuicFramer framer_;
QuicConnectionHelperInterface* helper_; // Not owned. QuicConnectionHelperInterface* helper_; // Not owned.
QuicPacketWriter* writer_; // Not owned. QuicPacketWriter* writer_; // Not owned.
...@@ -623,6 +627,8 @@ class NET_EXPORT_PRIVATE QuicConnection ...@@ -623,6 +627,8 @@ class NET_EXPORT_PRIVATE QuicConnection
// client. // client.
IPEndPoint self_address_; IPEndPoint self_address_;
IPEndPoint peer_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. bool last_packet_revived_; // True if the last packet was revived from FEC.
size_t last_size_; // Size of the last received packet. size_t last_size_; // Size of the last received packet.
...@@ -734,9 +740,21 @@ class NET_EXPORT_PRIVATE QuicConnection ...@@ -734,9 +740,21 @@ class NET_EXPORT_PRIVATE QuicConnection
// close. // close.
bool connected_; bool connected_;
// Set to true if the udp packet headers have a new self or peer address. // Set to true if the UDP packet headers have a new IP address for the peer.
// This is checked later on validating a data or version negotiation packet. // If true, do not perform connection migration.
bool address_migrating_; 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 // If non-empty this contains the set of versions received in a
// version negotiation packet. // version negotiation packet.
......
...@@ -33,5 +33,9 @@ bool FLAGS_enable_quic_stream_flow_control_2 = true; ...@@ -33,5 +33,9 @@ bool FLAGS_enable_quic_stream_flow_control_2 = true;
bool FLAGS_enable_quic_connection_flow_control = true; bool FLAGS_enable_quic_connection_flow_control = true;
bool FLAGS_quic_allow_oversized_packets_for_test = false; bool FLAGS_quic_allow_oversized_packets_for_test = false;
// When true, the use time based loss detection instead of nack. // When true, the use time based loss detection instead of nack.
bool FLAGS_quic_use_time_loss_detection = false; bool FLAGS_quic_use_time_loss_detection = false;
// If true, allow port migration of established QUIC connections.
bool FLAGS_quic_allow_port_migration = true;
...@@ -13,5 +13,6 @@ NET_EXPORT_PRIVATE extern bool FLAGS_enable_quic_stream_flow_control_2; ...@@ -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_enable_quic_connection_flow_control;
NET_EXPORT_PRIVATE extern bool FLAGS_quic_allow_oversized_packets_for_test; 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_use_time_loss_detection;
NET_EXPORT_PRIVATE extern bool FLAGS_quic_allow_port_migration;
#endif // NET_QUIC_QUIC_FLAGS_H_ #endif // NET_QUIC_QUIC_FLAGS_H_
...@@ -94,8 +94,7 @@ void QuicReliableClientStream::OnError(int error) { ...@@ -94,8 +94,7 @@ void QuicReliableClientStream::OnError(int error) {
bool QuicReliableClientStream::CanWrite(const CompletionCallback& callback) { bool QuicReliableClientStream::CanWrite(const CompletionCallback& callback) {
bool can_write = session()->connection()->CanWrite( bool can_write = session()->connection()->CanWrite(
NOT_RETRANSMISSION, HAS_RETRANSMITTABLE_DATA, NOT_RETRANSMISSION, HAS_RETRANSMITTABLE_DATA);
id() == kCryptoStreamId ? IS_HANDSHAKE : NOT_HANDSHAKE);
if (!can_write) { if (!can_write) {
session()->MarkWriteBlocked(id(), EffectivePriority()); session()->MarkWriteBlocked(id(), EffectivePriority());
DCHECK(callback_.is_null()); DCHECK(callback_.is_null());
......
...@@ -244,26 +244,27 @@ void QuicSentPacketManager::RetransmitUnackedPackets( ...@@ -244,26 +244,27 @@ void QuicSentPacketManager::RetransmitUnackedPackets(
// numbers with no connection to the previous ones. // numbers with no connection to the previous ones.
if (frames != NULL && (retransmission_type == ALL_PACKETS || if (frames != NULL && (retransmission_type == ALL_PACKETS ||
frames->encryption_level() == ENCRYPTION_INITIAL)) { frames->encryption_level() == ENCRYPTION_INITIAL)) {
unacked_packets_.SetNotPending(unacked_it->first);
MarkForRetransmission(unacked_it->first, ALL_UNACKED_RETRANSMISSION); MarkForRetransmission(unacked_it->first, ALL_UNACKED_RETRANSMISSION);
} }
++unacked_it; ++unacked_it;
} }
} }
void QuicSentPacketManager::DiscardUnencryptedPackets() { void QuicSentPacketManager::NeuterUnencryptedPackets() {
QuicUnackedPacketMap::const_iterator unacked_it = unacked_packets_.begin(); QuicUnackedPacketMap::const_iterator unacked_it = unacked_packets_.begin();
while (unacked_it != unacked_packets_.end()) { while (unacked_it != unacked_packets_.end()) {
const RetransmittableFrames* frames = const RetransmittableFrames* frames =
unacked_it->second.retransmittable_frames; unacked_it->second.retransmittable_frames;
if (frames != NULL && frames->encryption_level() == ENCRYPTION_NONE) { if (frames != NULL && frames->encryption_level() == ENCRYPTION_NONE) {
// Once you're forward secure, no unencrypted packets will be sent. // Since once you're forward secure, no unencrypted packets will be sent,
// Additionally, it's likely the peer will be forward secure, and no acks // crypto or otherwise. Unencrypted packets are neutered and abandoned, to
// for these packets will be received, so mark the packet as handled. // ensure they are not retransmitted or considered lost from a congestion
// control perspective.
pending_retransmissions_.erase(unacked_it->first); pending_retransmissions_.erase(unacked_it->first);
unacked_it = MarkPacketHandled(unacked_it->first, // TODO(ianswett): This may cause packets to linger forever in the
QuicTime::Delta::Zero()); // UnackedPacketMap.
continue; unacked_packets_.NeuterPacket(unacked_it->first);
unacked_packets_.SetNotPending(unacked_it->first);
} }
++unacked_it; ++unacked_it;
} }
...@@ -275,6 +276,9 @@ void QuicSentPacketManager::MarkForRetransmission( ...@@ -275,6 +276,9 @@ void QuicSentPacketManager::MarkForRetransmission(
const TransmissionInfo& transmission_info = const TransmissionInfo& transmission_info =
unacked_packets_.GetTransmissionInfo(sequence_number); unacked_packets_.GetTransmissionInfo(sequence_number);
LOG_IF(DFATAL, transmission_info.retransmittable_frames == NULL); 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 // TODO(ianswett): Currently the RTO can fire while there are pending NACK
// retransmissions for the same data, which is not ideal. // retransmissions for the same data, which is not ideal.
if (ContainsKey(pending_retransmissions_, sequence_number)) { if (ContainsKey(pending_retransmissions_, sequence_number)) {
...@@ -337,7 +341,11 @@ void QuicSentPacketManager::MarkPacketRevived( ...@@ -337,7 +341,11 @@ void QuicSentPacketManager::MarkPacketRevived(
sequence_number, delta_largest_observed); 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( QuicUnackedPacketMap::const_iterator QuicSentPacketManager::MarkPacketHandled(
...@@ -349,7 +357,7 @@ QuicUnackedPacketMap::const_iterator QuicSentPacketManager::MarkPacketHandled( ...@@ -349,7 +357,7 @@ QuicUnackedPacketMap::const_iterator QuicSentPacketManager::MarkPacketHandled(
} }
const TransmissionInfo& transmission_info = const TransmissionInfo& transmission_info =
unacked_packets_.GetTransmissionInfo(sequence_number); 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) { if (transmission_info.pending) {
unacked_packets_.SetNotPending(sequence_number); unacked_packets_.SetNotPending(sequence_number);
} }
...@@ -358,6 +366,13 @@ QuicUnackedPacketMap::const_iterator QuicSentPacketManager::MarkPacketHandled( ...@@ -358,6 +366,13 @@ QuicUnackedPacketMap::const_iterator QuicSentPacketManager::MarkPacketHandled(
SequenceNumberSet::reverse_iterator all_transmissions_it = SequenceNumberSet::reverse_iterator all_transmissions_it =
all_transmissions.rbegin(); all_transmissions.rbegin();
QuicPacketSequenceNumber newest_transmission = *all_transmissions_it; 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) { if (newest_transmission != sequence_number) {
stats_->bytes_spuriously_retransmitted += transmission_info.bytes_sent; stats_->bytes_spuriously_retransmitted += transmission_info.bytes_sent;
++stats_->packets_spuriously_retransmitted; ++stats_->packets_spuriously_retransmitted;
...@@ -368,19 +383,25 @@ QuicUnackedPacketMap::const_iterator QuicSentPacketManager::MarkPacketHandled( ...@@ -368,19 +383,25 @@ QuicUnackedPacketMap::const_iterator QuicSentPacketManager::MarkPacketHandled(
ack_notifier_manager_.OnPacketAcked(newest_transmission, ack_notifier_manager_.OnPacketAcked(newest_transmission,
delta_largest_observed); 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( bool has_crypto_handshake = HasCryptoHandshake(
unacked_packets_.GetTransmissionInfo(newest_transmission)); unacked_packets_.GetTransmissionInfo(newest_transmission));
while (all_transmissions_it != all_transmissions.rend()) { while (all_transmissions_it != all_transmissions.rend()) {
QuicPacketSequenceNumber previous_transmission = *all_transmissions_it; QuicPacketSequenceNumber previous_transmission = *all_transmissions_it;
// If this packet was marked for retransmission, don't bother retransmitting const TransmissionInfo& transmission_info =
// it anymore. unacked_packets_.GetTransmissionInfo(previous_transmission);
pending_retransmissions_.erase(previous_transmission); DCHECK(!ContainsKey(pending_retransmissions_, previous_transmission));
if (has_crypto_handshake) { if (has_crypto_handshake) {
// If it's a crypto handshake packet, discard it and all retransmissions, // If it's a crypto handshake packet, discard it and all retransmissions,
// since they won't be acked now that one has been processed. // since they won't be acked now that one has been processed.
unacked_packets_.SetNotPending(previous_transmission); 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; ++all_transmissions_it;
} }
...@@ -490,8 +511,6 @@ void QuicSentPacketManager::RetransmitCryptoPackets() { ...@@ -490,8 +511,6 @@ void QuicSentPacketManager::RetransmitCryptoPackets() {
} }
packet_retransmitted = true; packet_retransmitted = true;
MarkForRetransmission(sequence_number, HANDSHAKE_RETRANSMISSION); 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."; DCHECK(packet_retransmitted) << "No crypto packets found to retransmit.";
} }
...@@ -529,10 +548,11 @@ void QuicSentPacketManager::RetransmitAllPackets() { ...@@ -529,10 +548,11 @@ void QuicSentPacketManager::RetransmitAllPackets() {
bool packets_retransmitted = false; bool packets_retransmitted = false;
for (QuicUnackedPacketMap::const_iterator it = unacked_packets_.begin(); for (QuicUnackedPacketMap::const_iterator it = unacked_packets_.begin();
it != unacked_packets_.end(); ++it) { it != unacked_packets_.end(); ++it) {
unacked_packets_.SetNotPending(it->first);
if (it->second.retransmittable_frames != NULL) { if (it->second.retransmittable_frames != NULL) {
packets_retransmitted = true; packets_retransmitted = true;
MarkForRetransmission(it->first, RTO_RETRANSMISSION); MarkForRetransmission(it->first, RTO_RETRANSMISSION);
} else {
unacked_packets_.SetNotPending(it->first);
} }
} }
...@@ -582,16 +602,17 @@ void QuicSentPacketManager::InvokeLossDetection(QuicTime time) { ...@@ -582,16 +602,17 @@ void QuicSentPacketManager::InvokeLossDetection(QuicTime time) {
// until it's known whether the FEC packet arrived. // until it's known whether the FEC packet arrived.
++stats_->packets_lost; ++stats_->packets_lost;
packets_lost_[sequence_number] = transmission_info; packets_lost_[sequence_number] = transmission_info;
unacked_packets_.SetNotPending(sequence_number); DVLOG(1) << ENDPOINT << "Lost packet " << sequence_number;
if (transmission_info.retransmittable_frames != NULL) { if (transmission_info.retransmittable_frames != NULL) {
MarkForRetransmission(sequence_number, LOSS_RETRANSMISSION); MarkForRetransmission(sequence_number, LOSS_RETRANSMISSION);
} else { } else {
// Since we will not retransmit this, we need to remove it from // Since we will not retransmit this, we need to remove it from
// unacked_packets_. This is either the current transmission of // unacked_packets_. This is either the current transmission of
// a packet whose previous transmission has been acked, or it // a packet whose previous transmission has been acked, a packet that has
// is a packet that has been TLP retransmitted. // been TLP retransmitted, or an FEC packet.
unacked_packets_.NeuterIfPendingOrRemovePacket(sequence_number); unacked_packets_.SetNotPending(sequence_number);
unacked_packets_.RemovePacket(sequence_number);
} }
} }
} }
...@@ -663,7 +684,7 @@ const QuicTime QuicSentPacketManager::GetRetransmissionTime() const { ...@@ -663,7 +684,7 @@ const QuicTime QuicSentPacketManager::GetRetransmissionTime() const {
// Base the updated timer on the send time of the last packet. // Base the updated timer on the send time of the last packet.
const QuicTime sent_time = unacked_packets_.GetLastPacketSentTime(); const QuicTime sent_time = unacked_packets_.GetLastPacketSentTime();
const QuicTime tlp_time = sent_time.Add(GetTailLossProbeDelay()); 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); return QuicTime::Max(clock_->ApproximateNow(), tlp_time);
} }
case RTO_MODE: { case RTO_MODE: {
......
...@@ -96,7 +96,7 @@ class NET_EXPORT_PRIVATE QuicSentPacketManager { ...@@ -96,7 +96,7 @@ class NET_EXPORT_PRIVATE QuicSentPacketManager {
// Removes the retransmittable frames from all unencrypted packets to ensure // Removes the retransmittable frames from all unencrypted packets to ensure
// they don't get retransmitted. // they don't get retransmitted.
void DiscardUnencryptedPackets(); void NeuterUnencryptedPackets();
// Returns true if the unacked packet |sequence_number| has retransmittable // Returns true if the unacked packet |sequence_number| has retransmittable
// frames. This will only return false if the packet has been acked, if a // frames. This will only return false if the packet has been acked, if a
......
...@@ -69,22 +69,13 @@ class QuicSentPacketManagerTest : public ::testing::TestWithParam<bool> { ...@@ -69,22 +69,13 @@ class QuicSentPacketManagerTest : public ::testing::TestWithParam<bool> {
void VerifyRetransmittablePackets(QuicPacketSequenceNumber* packets, void VerifyRetransmittablePackets(QuicPacketSequenceNumber* packets,
size_t num_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, EXPECT_EQ(num_packets,
QuicSentPacketManagerPeer::GetNumRetransmittablePackets( QuicSentPacketManagerPeer::GetNumRetransmittablePackets(
&manager_)); &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) { void ExpectAck(QuicPacketSequenceNumber largest_observed) {
...@@ -125,15 +116,18 @@ class QuicSentPacketManagerTest : public ::testing::TestWithParam<bool> { ...@@ -125,15 +116,18 @@ class QuicSentPacketManagerTest : public ::testing::TestWithParam<bool> {
Pointwise(KeyEq(), lost_vector))); 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, void RetransmitPacket(QuicPacketSequenceNumber old_sequence_number,
QuicPacketSequenceNumber new_sequence_number) { QuicPacketSequenceNumber new_sequence_number) {
QuicSentPacketManagerPeer::MarkForRetransmission( QuicSentPacketManagerPeer::MarkForRetransmission(
&manager_, old_sequence_number, LOSS_RETRANSMISSION); &manager_, old_sequence_number, TLP_RETRANSMISSION);
EXPECT_TRUE(manager_.HasPendingRetransmissions()); EXPECT_TRUE(manager_.HasPendingRetransmissions());
QuicSentPacketManager::PendingRetransmission next_retransmission = QuicSentPacketManager::PendingRetransmission next_retransmission =
manager_.NextPendingRetransmission(); manager_.NextPendingRetransmission();
EXPECT_EQ(old_sequence_number, next_retransmission.sequence_number); EXPECT_EQ(old_sequence_number, next_retransmission.sequence_number);
EXPECT_EQ(LOSS_RETRANSMISSION, EXPECT_EQ(TLP_RETRANSMISSION,
next_retransmission.transmission_type); next_retransmission.transmission_type);
manager_.OnRetransmittedPacket(old_sequence_number, new_sequence_number); manager_.OnRetransmittedPacket(old_sequence_number, new_sequence_number);
EXPECT_TRUE(QuicSentPacketManagerPeer::IsRetransmission( EXPECT_TRUE(QuicSentPacketManagerPeer::IsRetransmission(
...@@ -197,7 +191,6 @@ class QuicSentPacketManagerTest : public ::testing::TestWithParam<bool> { ...@@ -197,7 +191,6 @@ class QuicSentPacketManagerTest : public ::testing::TestWithParam<bool> {
SerializedPacket packet(CreateDataPacket(sequence_number)); SerializedPacket packet(CreateDataPacket(sequence_number));
packet.retransmittable_frames->AddStreamFrame( packet.retransmittable_frames->AddStreamFrame(
new QuicStreamFrame(1, false, 0, IOVector())); new QuicStreamFrame(1, false, 0, IOVector()));
packet.retransmittable_frames->set_encryption_level(ENCRYPTION_NONE);
manager_.OnSerializedPacket(packet); manager_.OnSerializedPacket(packet);
manager_.OnPacketSent(sequence_number, clock_.ApproximateNow(), manager_.OnPacketSent(sequence_number, clock_.ApproximateNow(),
packet.packet->length(), NOT_RETRANSMISSION, packet.packet->length(), NOT_RETRANSMISSION,
...@@ -297,7 +290,7 @@ TEST_F(QuicSentPacketManagerTest, RetransmitThenAck) { ...@@ -297,7 +290,7 @@ TEST_F(QuicSentPacketManagerTest, RetransmitThenAck) {
TEST_F(QuicSentPacketManagerTest, RetransmitThenAckBeforeSend) { TEST_F(QuicSentPacketManagerTest, RetransmitThenAckBeforeSend) {
SendDataPacket(1); SendDataPacket(1);
QuicSentPacketManagerPeer::MarkForRetransmission( QuicSentPacketManagerPeer::MarkForRetransmission(
&manager_, 1, LOSS_RETRANSMISSION); &manager_, 1, TLP_RETRANSMISSION);
EXPECT_TRUE(manager_.HasPendingRetransmissions()); EXPECT_TRUE(manager_.HasPendingRetransmissions());
// Ack 1. // Ack 1.
...@@ -557,9 +550,9 @@ TEST_F(QuicSentPacketManagerTest, TruncatedAck) { ...@@ -557,9 +550,9 @@ TEST_F(QuicSentPacketManagerTest, TruncatedAck) {
manager_.OnIncomingAck(received_info, clock_.Now()); manager_.OnIncomingAck(received_info, clock_.Now());
// High water mark will be raised. // High water mark will be raised.
QuicPacketSequenceNumber unacked[] = { 2, 3, 4 }; QuicPacketSequenceNumber unacked[] = { 2, 3, 4, 5 };
VerifyUnackedPackets(unacked, arraysize(unacked)); VerifyUnackedPackets(unacked, arraysize(unacked));
QuicPacketSequenceNumber retransmittable[] = { 4 }; QuicPacketSequenceNumber retransmittable[] = { 5 };
VerifyRetransmittablePackets(retransmittable, arraysize(retransmittable)); VerifyRetransmittablePackets(retransmittable, arraysize(retransmittable));
} }
...@@ -988,25 +981,6 @@ TEST_F(QuicSentPacketManagerTest, ...@@ -988,25 +981,6 @@ TEST_F(QuicSentPacketManagerTest,
EXPECT_FALSE(QuicSentPacketManagerPeer::HasPendingPackets(&manager_)); 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) { TEST_F(QuicSentPacketManagerTest, TailLossProbeTimeoutUnsentDataPacket) {
QuicSentPacketManagerPeer::SetMaxTailLossProbes(&manager_, 2); QuicSentPacketManagerPeer::SetMaxTailLossProbes(&manager_, 2);
// Serialize two data packets and send the latter. // Serialize two data packets and send the latter.
......
...@@ -422,7 +422,7 @@ void QuicSession::OnCryptoHandshakeEvent(CryptoHandshakeEvent event) { ...@@ -422,7 +422,7 @@ void QuicSession::OnCryptoHandshakeEvent(CryptoHandshakeEvent event) {
<< "Handshake confirmed without parameter negotiation."; << "Handshake confirmed without parameter negotiation.";
// Discard originally encrypted packets, since they can't be decrypted by // Discard originally encrypted packets, since they can't be decrypted by
// the peer. // the peer.
connection_->DiscardUnencryptedPackets(); connection_->NeuterUnencryptedPackets();
connection_->SetOverallConnectionTimeout(QuicTime::Delta::Infinite()); connection_->SetOverallConnectionTimeout(QuicTime::Delta::Infinite());
max_open_streams_ = config_.max_streams_per_connection(); max_open_streams_ = config_.max_streams_per_connection();
break; break;
......
...@@ -91,7 +91,7 @@ void QuicUnackedPacketMap::ClearPreviousRetransmissions(size_t num_to_clear) { ...@@ -91,7 +91,7 @@ void QuicUnackedPacketMap::ClearPreviousRetransmissions(size_t num_to_clear) {
} }
++it; ++it;
NeuterIfPendingOrRemovePacket(sequence_number); RemovePacket(sequence_number);
--num_to_clear; --num_to_clear;
} }
} }
...@@ -119,7 +119,7 @@ void QuicUnackedPacketMap::NackPacket(QuicPacketSequenceNumber sequence_number, ...@@ -119,7 +119,7 @@ void QuicUnackedPacketMap::NackPacket(QuicPacketSequenceNumber sequence_number,
it->second.nack_count = max(min_nacks, it->second.nack_count); it->second.nack_count = max(min_nacks, it->second.nack_count);
} }
void QuicUnackedPacketMap::NeuterIfPendingOrRemovePacket( void QuicUnackedPacketMap::RemovePacket(
QuicPacketSequenceNumber sequence_number) { QuicPacketSequenceNumber sequence_number) {
UnackedPacketMap::iterator it = unacked_packets_.find(sequence_number); UnackedPacketMap::iterator it = unacked_packets_.find(sequence_number);
if (it == unacked_packets_.end()) { if (it == unacked_packets_.end()) {
...@@ -127,28 +127,41 @@ void QuicUnackedPacketMap::NeuterIfPendingOrRemovePacket( ...@@ -127,28 +127,41 @@ void QuicUnackedPacketMap::NeuterIfPendingOrRemovePacket(
return; return;
} }
TransmissionInfo* transmission_info = &it->second; TransmissionInfo* transmission_info = &it->second;
if (transmission_info->retransmittable_frames != NULL) { DCHECK(!transmission_info->pending);
if (transmission_info->retransmittable_frames->HasCryptoHandshake() MaybeRemoveRetransmittableFrames(transmission_info);
== IS_HANDSHAKE) { transmission_info->all_transmissions->erase(sequence_number);
--pending_crypto_packet_count_; if (transmission_info->all_transmissions->empty()) {
delete transmission_info->all_transmissions;
} }
delete transmission_info->retransmittable_frames; unacked_packets_.erase(it);
transmission_info->retransmittable_frames = NULL; }
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;
} }
if (transmission_info->pending) { TransmissionInfo* transmission_info = &it->second;
// Neuter it so it can't be retransmitted. // TODO(ianswett): Ensure packets are pending before neutering them.
MaybeRemoveRetransmittableFrames(transmission_info);
if (transmission_info->all_transmissions->size() > 1) { if (transmission_info->all_transmissions->size() > 1) {
transmission_info->all_transmissions->erase(sequence_number); transmission_info->all_transmissions->erase(sequence_number);
transmission_info->all_transmissions = new SequenceNumberSet(); transmission_info->all_transmissions = new SequenceNumberSet();
transmission_info->all_transmissions->insert(sequence_number); transmission_info->all_transmissions->insert(sequence_number);
} }
} else { }
// Remove it.
transmission_info->all_transmissions->erase(sequence_number); void QuicUnackedPacketMap::MaybeRemoveRetransmittableFrames(
if (transmission_info->all_transmissions->empty()) { TransmissionInfo* transmission_info) {
delete transmission_info->all_transmissions; if (transmission_info->retransmittable_frames != NULL) {
if (transmission_info->retransmittable_frames->HasCryptoHandshake()
== IS_HANDSHAKE) {
--pending_crypto_packet_count_;
} }
unacked_packets_.erase(it); delete transmission_info->retransmittable_frames;
transmission_info->retransmittable_frames = NULL;
} }
} }
...@@ -165,13 +178,6 @@ bool QuicUnackedPacketMap::IsUnacked( ...@@ -165,13 +178,6 @@ bool QuicUnackedPacketMap::IsUnacked(
return ContainsKey(unacked_packets_, sequence_number); 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( void QuicUnackedPacketMap::SetNotPending(
QuicPacketSequenceNumber sequence_number) { QuicPacketSequenceNumber sequence_number) {
UnackedPacketMap::iterator it = unacked_packets_.find(sequence_number); UnackedPacketMap::iterator it = unacked_packets_.find(sequence_number);
...@@ -286,15 +292,6 @@ QuicUnackedPacketMap::GetLeastUnackedSentPacket() const { ...@@ -286,15 +292,6 @@ QuicUnackedPacketMap::GetLeastUnackedSentPacket() const {
return unacked_packets_.begin()->first; 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, void QuicUnackedPacketMap::SetSent(QuicPacketSequenceNumber sequence_number,
QuicTime sent_time, QuicTime sent_time,
QuicByteCount bytes_sent, QuicByteCount bytes_sent,
......
...@@ -31,9 +31,6 @@ class NET_EXPORT_PRIVATE QuicUnackedPacketMap { ...@@ -31,9 +31,6 @@ class NET_EXPORT_PRIVATE QuicUnackedPacketMap {
// Returns true if the packet |sequence_number| is unacked. // Returns true if the packet |sequence_number| is unacked.
bool IsUnacked(QuicPacketSequenceNumber sequence_number) const; 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|. // Sets the nack count to the max of the current nack count and |min_nacks|.
void NackPacket(QuicPacketSequenceNumber sequence_number, void NackPacket(QuicPacketSequenceNumber sequence_number,
size_t min_nacks); size_t min_nacks);
...@@ -72,10 +69,6 @@ class NET_EXPORT_PRIVATE QuicUnackedPacketMap { ...@@ -72,10 +69,6 @@ class NET_EXPORT_PRIVATE QuicUnackedPacketMap {
// been acked by the peer. If there are no unacked packets, returns 0. // been acked by the peer. If there are no unacked packets, returns 0.
QuicPacketSequenceNumber GetLeastUnackedSentPacket() const; 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 // 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. // 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 // Packets marked as pending are expected to be marked as missing when they
...@@ -120,15 +113,21 @@ class NET_EXPORT_PRIVATE QuicUnackedPacketMap { ...@@ -120,15 +113,21 @@ class NET_EXPORT_PRIVATE QuicUnackedPacketMap {
// Returns true if there are any pending crypto packets. // Returns true if there are any pending crypto packets.
bool HasPendingCryptoPackets() const; bool HasPendingCryptoPackets() const;
// Deletes the retransmittable frames associated with the packet and removes // Removes entries from the unacked packet map, and deletes
// it from unacked packets if it's not pending. // the retransmittable frames associated with the packet.
// Does not remove any previous or subsequent transmissions of this 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. // Returns true if the packet has been marked as sent by SetSent.
static bool IsSentAndNotPending(const TransmissionInfo& transmission_info); static bool IsSentAndNotPending(const TransmissionInfo& transmission_info);
private: private:
void MaybeRemoveRetransmittableFrames(TransmissionInfo* transmission_info);
QuicPacketSequenceNumber largest_sent_packet_; QuicPacketSequenceNumber largest_sent_packet_;
// Newly serialized retransmittable and fec packets are added to this map, // Newly serialized retransmittable and fec packets are added to this map,
......
...@@ -112,12 +112,6 @@ size_t QuicSentPacketManagerPeer::GetNumRetransmittablePackets( ...@@ -112,12 +112,6 @@ size_t QuicSentPacketManagerPeer::GetNumRetransmittablePackets(
return sent_packet_manager->unacked_packets_.GetNumRetransmittablePackets(); return sent_packet_manager->unacked_packets_.GetNumRetransmittablePackets();
} }
// static
SequenceNumberSet QuicSentPacketManagerPeer::GetUnackedPackets(
const QuicSentPacketManager* sent_packet_manager) {
return sent_packet_manager->unacked_packets_.GetUnackedPackets();
}
// static // static
QuicByteCount QuicSentPacketManagerPeer::GetBytesInFlight( QuicByteCount QuicSentPacketManagerPeer::GetBytesInFlight(
const QuicSentPacketManager* sent_packet_manager) { const QuicSentPacketManager* sent_packet_manager) {
......
...@@ -60,9 +60,6 @@ class QuicSentPacketManagerPeer { ...@@ -60,9 +60,6 @@ class QuicSentPacketManagerPeer {
static size_t GetNumRetransmittablePackets( static size_t GetNumRetransmittablePackets(
const QuicSentPacketManager* sent_packet_manager); const QuicSentPacketManager* sent_packet_manager);
static SequenceNumberSet GetUnackedPackets(
const QuicSentPacketManager* sent_packet_manager);
static QuicByteCount GetBytesInFlight( static QuicByteCount GetBytesInFlight(
const QuicSentPacketManager* sent_packet_manager); const QuicSentPacketManager* sent_packet_manager);
......
...@@ -28,6 +28,7 @@ ...@@ -28,6 +28,7 @@
#include "net/quic/test_tools/quic_test_utils.h" #include "net/quic/test_tools/quic_test_utils.h"
#include "net/quic/test_tools/reliable_quic_stream_peer.h" #include "net/quic/test_tools/reliable_quic_stream_peer.h"
#include "net/test/gtest_util.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_epoll_connection_helper.h"
#include "net/tools/quic/quic_in_memory_cache.h" #include "net/tools/quic/quic_in_memory_cache.h"
#include "net/tools/quic/quic_packet_writer_wrapper.h" #include "net/tools/quic/quic_packet_writer_wrapper.h"
...@@ -46,6 +47,7 @@ ...@@ -46,6 +47,7 @@
using base::StringPiece; using base::StringPiece;
using base::WaitableEvent; using base::WaitableEvent;
using net::EpollServer;
using net::test::GenerateBody; using net::test::GenerateBody;
using net::test::QuicConnectionPeer; using net::test::QuicConnectionPeer;
using net::test::QuicSessionPeer; using net::test::QuicSessionPeer;
...@@ -481,7 +483,7 @@ TEST_P(EndToEndTest, DISABLED_LargePostNoPacketLoss) { ...@@ -481,7 +483,7 @@ TEST_P(EndToEndTest, DISABLED_LargePostNoPacketLoss) {
client_->client()->WaitForCryptoHandshakeConfirmed(); client_->client()->WaitForCryptoHandshakeConfirmed();
// 1 Mb body. // 1 MB body.
string body; string body;
GenerateBody(&body, 1024 * 1024); GenerateBody(&body, 1024 * 1024);
...@@ -499,7 +501,7 @@ TEST_P(EndToEndTest, LargePostNoPacketLoss1sRTT) { ...@@ -499,7 +501,7 @@ TEST_P(EndToEndTest, LargePostNoPacketLoss1sRTT) {
client_->client()->WaitForCryptoHandshakeConfirmed(); client_->client()->WaitForCryptoHandshakeConfirmed();
// 1 Mb body. // 100 KB body.
string body; string body;
GenerateBody(&body, 100 * 1024); GenerateBody(&body, 100 * 1024);
...@@ -521,7 +523,7 @@ TEST_P(EndToEndTest, LargePostWithPacketLoss) { ...@@ -521,7 +523,7 @@ TEST_P(EndToEndTest, LargePostWithPacketLoss) {
client_->client()->WaitForCryptoHandshakeConfirmed(); client_->client()->WaitForCryptoHandshakeConfirmed();
SetPacketLossPercentage(30); SetPacketLossPercentage(30);
// 10 Kb body. // 10 KB body.
string body; string body;
GenerateBody(&body, 1024 * 10); GenerateBody(&body, 1024 * 10);
...@@ -530,42 +532,42 @@ TEST_P(EndToEndTest, LargePostWithPacketLoss) { ...@@ -530,42 +532,42 @@ TEST_P(EndToEndTest, LargePostWithPacketLoss) {
request.AddBody(body, true); request.AddBody(body, true);
EXPECT_EQ(kFooResponseBody, client_->SendCustomSynchronousRequest(request)); 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()); ASSERT_TRUE(Initialize());
// Wait for the server SHLO before upping the packet loss.
client_->client()->WaitForCryptoHandshakeConfirmed(); client_->client()->WaitForCryptoHandshakeConfirmed();
// Both of these must be called when the writer is not actively used. SetPacketLossPercentage(10);
SetPacketSendDelay(QuicTime::Delta::FromMilliseconds(2)); client_writer_->set_fake_blocked_socket_percentage(10);
SetReorderPercentage(30);
// 1 Mb body. // 10 KB body.
string body; string body;
GenerateBody(&body, 1024 * 1024); GenerateBody(&body, 1024 * 10);
HTTPMessage request(HttpConstants::HTTP_1_1, HTTPMessage request(HttpConstants::HTTP_1_1,
HttpConstants::POST, "/foo"); HttpConstants::POST, "/foo");
request.AddBody(body, true); request.AddBody(body, true);
EXPECT_EQ(kFooResponseBody, client_->SendCustomSynchronousRequest(request)); EXPECT_EQ(kFooResponseBody, client_->SendCustomSynchronousRequest(request));
VerifyCleanConnection(true);
} }
TEST_P(EndToEndTest, LargePostWithPacketLossAndBlockedSocket) { TEST_P(EndToEndTest, LargePostNoPacketLossWithDelayAndReordering) {
// 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()); ASSERT_TRUE(Initialize());
// Wait for the server SHLO before upping the packet loss.
client_->client()->WaitForCryptoHandshakeConfirmed(); client_->client()->WaitForCryptoHandshakeConfirmed();
SetPacketLossPercentage(10); // Both of these must be called when the writer is not actively used.
client_writer_->set_fake_blocked_socket_percentage(10); SetPacketSendDelay(QuicTime::Delta::FromMilliseconds(2));
SetReorderPercentage(30);
// 10 Kb body. // 1 MB body.
string body; string body;
GenerateBody(&body, 1024 * 10); GenerateBody(&body, 1024 * 1024);
HTTPMessage request(HttpConstants::HTTP_1_1, HTTPMessage request(HttpConstants::HTTP_1_1,
HttpConstants::POST, "/foo"); HttpConstants::POST, "/foo");
...@@ -655,19 +657,17 @@ TEST_P(EndToEndTest, LargePostFEC) { ...@@ -655,19 +657,17 @@ TEST_P(EndToEndTest, LargePostFEC) {
VerifyCleanConnection(true); VerifyCleanConnection(true);
} }
// TODO(rtenneti): DISABLED_LargePostLargeBuffer seems to be flaky. TEST_P(EndToEndTest, LargePostSmallBandwidthLargeBuffer) {
// http://crbug.com/370087.
TEST_P(EndToEndTest, DISABLED_LargePostLargeBuffer) {
ASSERT_TRUE(Initialize()); ASSERT_TRUE(Initialize());
SetPacketSendDelay(QuicTime::Delta::FromMicroseconds(1)); 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. // clients commonly have larger buffers, but our max CWND is 200.
server_writer_->set_max_bandwidth_and_buffer_size( server_writer_->set_max_bandwidth_and_buffer_size(
QuicBandwidth::FromBytesPerSecond(256 * 1024), 128 * 1024); QuicBandwidth::FromBytesPerSecond(256 * 1024), 256 * 1024);
client_->client()->WaitForCryptoHandshakeConfirmed(); client_->client()->WaitForCryptoHandshakeConfirmed();
// 1 Mb body. // 1 MB body.
string body; string body;
GenerateBody(&body, 1024 * 1024); GenerateBody(&body, 1024 * 1024);
...@@ -676,6 +676,8 @@ TEST_P(EndToEndTest, DISABLED_LargePostLargeBuffer) { ...@@ -676,6 +676,8 @@ TEST_P(EndToEndTest, DISABLED_LargePostLargeBuffer) {
request.AddBody(body, true); request.AddBody(body, true);
EXPECT_EQ(kFooResponseBody, client_->SendCustomSynchronousRequest(request)); 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); VerifyCleanConnection(false);
} }
...@@ -790,7 +792,7 @@ TEST_P(EndToEndTest, DISABLED_LimitCongestionWindowAndRTT) { ...@@ -790,7 +792,7 @@ TEST_P(EndToEndTest, DISABLED_LimitCongestionWindowAndRTT) {
// Now use the negotiated limits with packet loss. // Now use the negotiated limits with packet loss.
SetPacketLossPercentage(30); SetPacketLossPercentage(30);
// 10 Kb body. // 10 KB body.
string body; string body;
GenerateBody(&body, 1024 * 10); GenerateBody(&body, 1024 * 10);
...@@ -953,7 +955,10 @@ class WrongAddressWriter : public QuicPacketWriterWrapper { ...@@ -953,7 +955,10 @@ class WrongAddressWriter : public QuicPacketWriterWrapper {
IPEndPoint self_address_; 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()); ASSERT_TRUE(Initialize());
EXPECT_EQ(kFooResponseBody, client_->SendSynchronousRequest("/foo")); EXPECT_EQ(kFooResponseBody, client_->SendSynchronousRequest("/foo"));
...@@ -971,6 +976,59 @@ TEST_P(EndToEndTest, ConnectionMigration) { ...@@ -971,6 +976,59 @@ TEST_P(EndToEndTest, ConnectionMigration) {
EXPECT_EQ(QUIC_ERROR_MIGRATING_ADDRESS, client_->connection_error()); 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) { TEST_P(EndToEndTest, DifferentFlowControlWindows) {
// Client and server can set different initial flow control receive windows. // Client and server can set different initial flow control receive windows.
// These are sent in CHLO/SHLO. Tests that these values are exchanged properly // These are sent in CHLO/SHLO. Tests that these values are exchanged properly
......
...@@ -83,6 +83,16 @@ bool QuicClient::Initialize() { ...@@ -83,6 +83,16 @@ bool QuicClient::Initialize() {
epoll_server_.set_timeout_in_us(50 * 1000); epoll_server_.set_timeout_in_us(50 * 1000);
crypto_config_.SetDefaults(); 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(); int address_family = server_address_.GetSockAddrFamily();
fd_ = socket(address_family, SOCK_DGRAM | SOCK_NONBLOCK, IPPROTO_UDP); fd_ = socket(address_family, SOCK_DGRAM | SOCK_NONBLOCK, IPPROTO_UDP);
if (fd_ < 0) { if (fd_ < 0) {
...@@ -153,8 +163,6 @@ bool QuicClient::Initialize() { ...@@ -153,8 +163,6 @@ bool QuicClient::Initialize() {
LOG(ERROR) << "Unable to get self address. Error: " << strerror(errno); LOG(ERROR) << "Unable to get self address. Error: " << strerror(errno);
} }
epoll_server_.RegisterFD(fd_, this, kEpollFlags);
initialized_ = true;
return true; return true;
} }
......
...@@ -181,6 +181,10 @@ class QuicClient : public EpollCallbackInterface, ...@@ -181,6 +181,10 @@ class QuicClient : public EpollCallbackInterface,
private: private:
friend class net::tools::test::QuicClientPeer; 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. // Read a UDP packet and hand it to the framer.
bool ReadAndProcessPacket(); bool ReadAndProcessPacket();
......
...@@ -15,6 +15,16 @@ QuicCryptoClientConfig* QuicClientPeer::GetCryptoConfig(QuicClient* client) { ...@@ -15,6 +15,16 @@ QuicCryptoClientConfig* QuicClientPeer::GetCryptoConfig(QuicClient* client) {
return &client->crypto_config_; 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 test
} // namespace tools } // namespace tools
} // namespace net } // namespace net
...@@ -20,6 +20,8 @@ namespace test { ...@@ -20,6 +20,8 @@ namespace test {
class QuicClientPeer { class QuicClientPeer {
public: public:
static QuicCryptoClientConfig* GetCryptoConfig(QuicClient* client); static QuicCryptoClientConfig* GetCryptoConfig(QuicClient* client);
static bool CreateUDPSocket(QuicClient* client);
static void SetClientPort(QuicClient* client, int port);
private: private:
DISALLOW_COPY_AND_ASSIGN(QuicClientPeer); DISALLOW_COPY_AND_ASSIGN(QuicClientPeer);
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment