Validate incoming ECN feedback.
Marks will not be sent in production until paired with a congestion control that reports that it supports ECT(1) via SendAlgorithmInterface.
The bulk of the logic in this CL validates that the received ECN feedback from the peer is valid according to RFC9000, and stops sending ECN if invalid.
Also stops sending ECN if it appears the path is dropping ECN packets.
Though not clear from this CL, the actual mechanism to write ECN is for the PacketWriter to read per_packet_options_. QuicConnection checks if all the conditions to write ECN are present and clears the bits if not. GFE changes to follow.
Protected by FLAGS_quic_reloadable_flag_quic_send_ect1.
PiperOrigin-RevId: 525846393
diff --git a/quiche/quic/core/http/end_to_end_test.cc b/quiche/quic/core/http/end_to_end_test.cc
index bf8e903..6b820b1 100644
--- a/quiche/quic/core/http/end_to_end_test.cc
+++ b/quiche/quic/core/http/end_to_end_test.cc
@@ -7225,8 +7225,9 @@
server_thread_->Resume();
}
-TEST_P(EndToEndTest, ServerReportsEcn) {
+TEST_P(EndToEndTest, ServerReportsNotEct) {
// Client connects using not-ECT.
+ SetQuicReloadableFlag(quic_send_ect1, true);
ASSERT_TRUE(Initialize());
QuicConnection* client_connection = GetClientConnection();
QuicConnectionPeer::DisableEcnCodepointValidation(client_connection);
@@ -7236,44 +7237,102 @@
EXPECT_EQ(ecn->ect0, 0);
EXPECT_EQ(ecn->ect1, 0);
EXPECT_EQ(ecn->ce, 0);
- QuicPacketCount ect0 = 0, ect1 = 0;
TestPerPacketOptions options;
client_connection->set_per_packet_options(&options);
- for (QuicEcnCodepoint codepoint : {ECN_NOT_ECT, ECN_ECT0, ECN_ECT1, ECN_CE}) {
- options.ecn_codepoint = codepoint;
- client_->SendSynchronousRequest("/foo");
- if (!GetQuicRestartFlag(quic_receive_ecn) ||
- !GetQuicRestartFlag(quic_quiche_ecn_sockets) ||
- !VersionHasIetfQuicFrames(version_.transport_version) ||
- codepoint == ECN_NOT_ECT) {
- EXPECT_EQ(ecn->ect0, 0);
- EXPECT_EQ(ecn->ect1, 0);
- EXPECT_EQ(ecn->ce, 0);
- continue;
- }
- EXPECT_GT(ecn->ect0, 0);
- if (codepoint == ECN_CE) {
- EXPECT_EQ(ect0, ecn->ect0); // No more ECT(0) arriving
- EXPECT_GE(ecn->ect1, ect1); // Late-arriving ECT(1) control packets
- EXPECT_GT(ecn->ce, 0);
- continue;
- }
- EXPECT_EQ(ecn->ce, 0);
- if (codepoint == ECN_ECT1) {
- EXPECT_GE(ecn->ect0, ect0); // Late-arriving ECT(0) control packets
- ect0 = ecn->ect0;
- ect1 = ecn->ect1;
- EXPECT_GT(ect1, 0);
- continue;
- }
- // codepoint == ECN_ECT0
- ect0 = ecn->ect0;
- EXPECT_EQ(ecn->ect1, 0);
- }
+ options.ecn_codepoint = ECN_NOT_ECT;
+ client_->SendSynchronousRequest("/foo");
+ EXPECT_EQ(ecn->ect0, 0);
+ EXPECT_EQ(ecn->ect1, 0);
+ EXPECT_EQ(ecn->ce, 0);
client_->Disconnect();
}
-TEST_P(EndToEndTest, ClientReportsEcn) {
+TEST_P(EndToEndTest, ServerReportsEct0) {
+ // Client connects using not-ECT.
+ SetQuicReloadableFlag(quic_send_ect1, true);
+ ASSERT_TRUE(Initialize());
+ QuicConnection* client_connection = GetClientConnection();
+ QuicConnectionPeer::DisableEcnCodepointValidation(client_connection);
+ QuicEcnCounts* ecn = QuicSentPacketManagerPeer::GetPeerEcnCounts(
+ QuicConnectionPeer::GetSentPacketManager(client_connection),
+ APPLICATION_DATA);
+ EXPECT_EQ(ecn->ect0, 0);
+ EXPECT_EQ(ecn->ect1, 0);
+ EXPECT_EQ(ecn->ce, 0);
+ TestPerPacketOptions options;
+ client_connection->set_per_packet_options(&options);
+ options.ecn_codepoint = ECN_ECT0;
+ client_->SendSynchronousRequest("/foo");
+ if (!GetQuicRestartFlag(quic_receive_ecn) ||
+ !GetQuicRestartFlag(quic_quiche_ecn_sockets) ||
+ !VersionHasIetfQuicFrames(version_.transport_version)) {
+ EXPECT_EQ(ecn->ect0, 0);
+ } else {
+ EXPECT_GT(ecn->ect0, 0);
+ }
+ EXPECT_EQ(ecn->ect1, 0);
+ EXPECT_EQ(ecn->ce, 0);
+ client_->Disconnect();
+}
+
+TEST_P(EndToEndTest, ServerReportsEct1) {
+ // Client connects using not-ECT.
+ SetQuicReloadableFlag(quic_send_ect1, true);
+ ASSERT_TRUE(Initialize());
+ QuicConnection* client_connection = GetClientConnection();
+ QuicConnectionPeer::DisableEcnCodepointValidation(client_connection);
+ QuicEcnCounts* ecn = QuicSentPacketManagerPeer::GetPeerEcnCounts(
+ QuicConnectionPeer::GetSentPacketManager(client_connection),
+ APPLICATION_DATA);
+ EXPECT_EQ(ecn->ect0, 0);
+ EXPECT_EQ(ecn->ect1, 0);
+ EXPECT_EQ(ecn->ce, 0);
+ TestPerPacketOptions options;
+ client_connection->set_per_packet_options(&options);
+ options.ecn_codepoint = ECN_ECT1;
+ client_->SendSynchronousRequest("/foo");
+ if (!GetQuicRestartFlag(quic_receive_ecn) ||
+ !GetQuicRestartFlag(quic_quiche_ecn_sockets) ||
+ !VersionHasIetfQuicFrames(version_.transport_version)) {
+ EXPECT_EQ(ecn->ect1, 0);
+ } else {
+ EXPECT_GT(ecn->ect1, 0);
+ }
+ EXPECT_EQ(ecn->ect0, 0);
+ EXPECT_EQ(ecn->ce, 0);
+ client_->Disconnect();
+}
+
+TEST_P(EndToEndTest, ServerReportsCe) {
+ // Client connects using not-ECT.
+ SetQuicReloadableFlag(quic_send_ect1, true);
+ ASSERT_TRUE(Initialize());
+ QuicConnection* client_connection = GetClientConnection();
+ QuicConnectionPeer::DisableEcnCodepointValidation(client_connection);
+ QuicEcnCounts* ecn = QuicSentPacketManagerPeer::GetPeerEcnCounts(
+ QuicConnectionPeer::GetSentPacketManager(client_connection),
+ APPLICATION_DATA);
+ EXPECT_EQ(ecn->ect0, 0);
+ EXPECT_EQ(ecn->ect1, 0);
+ EXPECT_EQ(ecn->ce, 0);
+ TestPerPacketOptions options;
+ client_connection->set_per_packet_options(&options);
+ options.ecn_codepoint = ECN_CE;
+ client_->SendSynchronousRequest("/foo");
+ if (!GetQuicRestartFlag(quic_receive_ecn) ||
+ !GetQuicRestartFlag(quic_quiche_ecn_sockets) ||
+ !VersionHasIetfQuicFrames(version_.transport_version)) {
+ EXPECT_EQ(ecn->ce, 0);
+ } else {
+ EXPECT_GT(ecn->ce, 0);
+ }
+ EXPECT_EQ(ecn->ect0, 0);
+ EXPECT_EQ(ecn->ect1, 0);
+ client_->Disconnect();
+}
+
+TEST_P(EndToEndTest, ClientReportsEct1) {
+ SetQuicReloadableFlag(quic_send_ect1, true);
ASSERT_TRUE(Initialize());
// Wait for handshake to complete, so that we can manipulate the server
// connection without race conditions.
diff --git a/quiche/quic/core/quic_connection.cc b/quiche/quic/core/quic_connection.cc
index 01e728a..10f830d 100644
--- a/quiche/quic/core/quic_connection.cc
+++ b/quiche/quic/core/quic_connection.cc
@@ -8,6 +8,7 @@
#include <sys/types.h>
#include <algorithm>
+#include <cstddef>
#include <cstdint>
#include <iterator>
#include <limits>
@@ -64,6 +65,11 @@
// The maximum number of recorded client addresses.
const size_t kMaxReceivedClientAddressSize = 20;
+// An arbitrary limit on the number of PTOs before giving up on ECN, if no ECN-
+// marked packet is acked. Avoids abandoning ECN because of one burst loss,
+// but doesn't allow multiple RTTs of user delay in the hope of using ECN.
+const uint8_t kEcnPtoLimit = 2;
+
// Base class of all alarms owned by a QuicConnection.
class QuicConnectionAlarmDelegate : public QuicAlarm::Delegate {
public:
@@ -3122,7 +3128,7 @@
const BufferedPacket& packet = buffered_packets_.front();
WriteResult result = SendPacketToWriter(
packet.data.get(), packet.length, packet.self_address.host(),
- packet.peer_address, per_packet_options_);
+ packet.peer_address, per_packet_options_, writer_);
QUIC_DVLOG(1) << ENDPOINT << "Sending buffered packet, result: " << result;
if (IsMsgTooBig(writer_, result) && packet.length > long_term_mtu_) {
// When MSG_TOO_BIG is returned, the system typically knows what the
@@ -3489,7 +3495,7 @@
packet->release_encrypted_buffer = nullptr;
result = SendPacketToWriter(packet->encrypted_buffer, encrypted_length,
send_from_address.host(), send_to_address,
- per_packet_options_);
+ per_packet_options_, writer_);
// This is a work around for an issue with linux UDP GSO batch writers.
// When sending a GSO packet with 2 segments, if the first segment is
// larger than the path MTU, instead of EMSGSIZE, the linux kernel returns
@@ -3612,7 +3618,7 @@
const bool in_flight = sent_packet_manager_.OnPacketSent(
packet, packet_send_time, packet->transmission_type,
IsRetransmittable(*packet), /*measure_rtt=*/send_on_current_path,
- ECN_NOT_ECT);
+ last_ecn_codepoint_sent_);
QUIC_BUG_IF(quic_bug_12714_25,
perspective_ == Perspective::IS_SERVER &&
default_enable_5rto_blackhole_detection_ &&
@@ -3991,6 +3997,29 @@
}
}
+void QuicConnection::OnInFlightEcnPacketAcked() {
+ QUIC_BUG_IF(quic_bug_518619343_01, !GetQuicReloadableFlag(quic_send_ect1))
+ << "Unexpected call to OnInFlightEcnPacketAcked()";
+ // Only packets on the default path are in-flight.
+ if (!default_path_.ecn_marked_packet_acked) {
+ QUIC_DVLOG(1) << ENDPOINT << "First ECT packet acked on active path.";
+ QUIC_RELOADABLE_FLAG_COUNT_N(quic_send_ect1, 2, 2);
+ default_path_.ecn_marked_packet_acked = true;
+ }
+}
+
+void QuicConnection::OnInvalidEcnFeedback() {
+ QUIC_BUG_IF(quic_bug_518619343_02, !GetQuicReloadableFlag(quic_send_ect1))
+ << "Unexpected call to OnInvalidEcnFeedback().";
+ if (disable_ecn_codepoint_validation_) {
+ // In some tests, senders may send ECN marks in patterns that are not
+ // in accordance with the spec, and should not fail validation as a result.
+ return;
+ }
+ QUIC_DVLOG(1) << ENDPOINT << "ECN feedback is invalid, stop marking.";
+ ClearEcnCodepoint();
+}
+
std::unique_ptr<QuicSelfIssuedConnectionIdManager>
QuicConnection::MakeSelfIssuedConnectionIdManager() {
QUICHE_DCHECK((perspective_ == Perspective::IS_CLIENT &&
@@ -4144,36 +4173,60 @@
address) != known_server_addresses_.cend();
}
-void QuicConnection::ClearEcnCodepoint() {
+QuicEcnCodepoint QuicConnection::GetEcnCodepointToSend(
+ const QuicSocketAddress& destination_address) const {
+ const QuicEcnCodepoint original_codepoint = GetNextEcnCodepoint();
+ if (disable_ecn_codepoint_validation_) {
+ return original_codepoint;
+ }
+ // Don't send ECN marks on alternate paths. Sending ECN marks might
+ // cause the connectivity check to fail on some networks.
+ if (destination_address != peer_address()) {
+ return ECN_NOT_ECT;
+ }
+ // If the path might drop ECN marked packets, send retransmission without
+ // them.
+ if (in_probe_time_out_ && !default_path_.ecn_marked_packet_acked) {
+ return ECN_NOT_ECT;
+ }
+ switch (original_codepoint) {
+ case ECN_NOT_ECT:
+ break;
+ case ECN_ECT0:
+ if (!sent_packet_manager_.GetSendAlgorithm()->SupportsECT0()) {
+ return ECN_NOT_ECT;
+ }
+ break;
+ case ECN_ECT1:
+ if (!sent_packet_manager_.GetSendAlgorithm()->SupportsECT1()) {
+ return ECN_NOT_ECT;
+ }
+ break;
+ case ECN_CE:
+ return ECN_NOT_ECT;
+ }
+ return original_codepoint;
+}
+
+void QuicConnection::ClearEcnCodepoint() { MaybeSetEcnCodepoint(ECN_NOT_ECT); }
+
+void QuicConnection::MaybeSetEcnCodepoint(QuicEcnCodepoint ecn_codepoint) {
if (per_packet_options_ != nullptr) {
- per_packet_options_->ecn_codepoint = ECN_NOT_ECT;
+ per_packet_options_->ecn_codepoint = ecn_codepoint;
}
}
WriteResult QuicConnection::SendPacketToWriter(
const char* buffer, size_t buf_len, const QuicIpAddress& self_address,
- const QuicSocketAddress& peer_address, PerPacketOptions* options) {
- if (!disable_ecn_codepoint_validation_) {
- switch (GetNextEcnCodepoint()) {
- case ECN_NOT_ECT:
- break;
- case ECN_ECT0:
- if (!sent_packet_manager_.GetSendAlgorithm()->SupportsECT0()) {
- ClearEcnCodepoint();
- }
- break;
- case ECN_ECT1:
- if (!sent_packet_manager_.GetSendAlgorithm()->SupportsECT1()) {
- ClearEcnCodepoint();
- }
- break;
- case ECN_CE:
- ClearEcnCodepoint();
- break;
- }
- }
- return writer_->WritePacket(buffer, buf_len, self_address, peer_address,
- options);
+ const QuicSocketAddress& destination_address, PerPacketOptions* options,
+ QuicPacketWriter* writer) {
+ QuicEcnCodepoint original_codepoint = GetNextEcnCodepoint();
+ last_ecn_codepoint_sent_ = GetEcnCodepointToSend(destination_address);
+ MaybeSetEcnCodepoint(last_ecn_codepoint_sent_);
+ WriteResult result = writer->WritePacket(buffer, buf_len, self_address,
+ destination_address, options);
+ MaybeSetEcnCodepoint(original_codepoint);
+ return result;
}
void QuicConnection::OnRetransmissionTimeout() {
@@ -4280,6 +4333,24 @@
if (!HasQueuedData() && !retransmission_alarm_->IsSet()) {
SetRetransmissionAlarm();
}
+ if (GetNextEcnCodepoint() == ECN_NOT_ECT ||
+ default_path_.ecn_marked_packet_acked) {
+ return;
+ }
+ ++default_path_.ecn_pto_count;
+ if (default_path_.ecn_pto_count == kEcnPtoLimit) {
+ // Give up on ECN. There are two scenarios:
+ // 1. All packets are suffering PTO. In this case, the connection
+ // abandons ECN after 1 failed ECT(1) flight and one failed Not-ECT
+ // flight.
+ // 2. Only ECN packets are suffering PTO. In that case, alternating
+ // flights will have ECT(1). On the second ECT(1) failure, the
+ // connection will abandon.
+ // This behavior is in the range of acceptable choices in S13.4.2 of RFC
+ // 9000.
+ QUIC_DVLOG(1) << ENDPOINT << "ECN packets PTO 3 times.";
+ OnInvalidEcnFeedback();
+ }
}
void QuicConnection::SetEncrypter(EncryptionLevel level,
@@ -5092,9 +5163,9 @@
<< default_path_.server_connection_id << std::endl
<< quiche::QuicheTextUtils::HexDump(absl::string_view(
packet->encrypted_buffer, packet->encrypted_length));
- WriteResult result = writer->WritePacket(
+ WriteResult result = SendPacketToWriter(
packet->encrypted_buffer, packet->encrypted_length, self_address.host(),
- peer_address, per_packet_options_);
+ peer_address, per_packet_options_, writer);
// If using a batch writer and the probing packet is buffered, flush it.
if (writer->IsBatchMode() && result.status == WRITE_STATUS_OK &&
@@ -5113,7 +5184,7 @@
// Send in currrent path. Call OnPacketSent regardless of the write result.
sent_packet_manager_.OnPacketSent(
packet.get(), packet_send_time, packet->transmission_type,
- NO_RETRANSMITTABLE_DATA, measure_rtt, ECN_NOT_ECT);
+ NO_RETRANSMITTABLE_DATA, measure_rtt, last_ecn_codepoint_sent_);
if (debug_visitor_ != nullptr) {
if (sent_packet_manager_.unacked_packets().empty()) {
@@ -5996,7 +6067,7 @@
} else {
WriteResult result = SendPacketToWriter(
buffer, length, coalesced_packet_.self_address().host(),
- coalesced_packet_.peer_address(), per_packet_options_);
+ coalesced_packet_.peer_address(), per_packet_options_, writer_);
if (IsWriteError(result.status)) {
OnWriteError(result.error_code);
return false;
@@ -7117,6 +7188,8 @@
send_algorithm = nullptr;
rtt_stats = absl::nullopt;
stateless_reset_token.reset();
+ ecn_marked_packet_acked = false;
+ ecn_pto_count = 0;
}
QuicConnection::PathState::PathState(PathState&& other) {
diff --git a/quiche/quic/core/quic_connection.h b/quiche/quic/core/quic_connection.h
index 55714ad..eadad65 100644
--- a/quiche/quic/core/quic_connection.h
+++ b/quiche/quic/core/quic_connection.h
@@ -724,6 +724,8 @@
// QuicSentPacketManager::NetworkChangeVisitor
void OnCongestionChange() override;
void OnPathMtuIncreased(QuicPacketLength packet_size) override;
+ void OnInFlightEcnPacketAcked() override;
+ void OnInvalidEcnFeedback() override;
// QuicNetworkBlackholeDetector::Delegate
void OnPathDegradingDetected() override;
@@ -1465,6 +1467,12 @@
// validating migrated peer address. Nullptr otherwise.
std::unique_ptr<SendAlgorithmInterface> send_algorithm;
absl::optional<RttStats> rtt_stats;
+ // If true, an ECN packet was acked on this path, so the path probably isn't
+ // dropping ECN-marked packets.
+ bool ecn_marked_packet_acked = false;
+ // How many total PTOs have fired since the connection started sending ECN
+ // on this path, but before an ECN-marked packet has been acked.
+ uint8_t ecn_pto_count = 0;
};
using QueuedPacketList = std::list<SerializedPacket>;
@@ -1960,21 +1968,37 @@
// Returns true if |address| is known server address.
bool IsKnownServerAddress(const QuicSocketAddress& address) const;
- // Retrieves the ECN codepoint to be sent on the next packet.
+ // Retrieves the ECN codepoint stored in per_packet_options_, unless the flag
+ // is not set.
QuicEcnCodepoint GetNextEcnCodepoint() const {
- return (per_packet_options_ != nullptr) ? per_packet_options_->ecn_codepoint
- : ECN_NOT_ECT;
+ return (per_packet_options_ != nullptr &&
+ GetQuicReloadableFlag(quic_send_ect1))
+ ? per_packet_options_->ecn_codepoint
+ : ECN_NOT_ECT;
}
+ // Retrieves the ECN codepoint to be sent on the next packet.
+ QuicEcnCodepoint GetEcnCodepointToSend(
+ const QuicSocketAddress& destination_address) const;
+
// Sets the ECN codepoint to Not-ECT.
void ClearEcnCodepoint();
- // Writes the packet to the writer and clears the ECN codepoint in |options|
- // if it is invalid.
+ // Set the ECN codepoint, but only if set_per_packet_options has been called.
+ void MaybeSetEcnCodepoint(QuicEcnCodepoint ecn_codepoint);
+
+ // Writes the packet to |writer| with the ECN mark specified in |options|. If
+ // by spec the connection should not send an ECN mark, or the packet is
+ // not on the default path, or it's PTO probe before an ECN packet has been
+ // successfully acked on the path, or QUIC reloadable flag quic_send_ect1 is
+ // false, then it sends Not-ECT instead. Will also set last_ecn_sent_
+ // appropriately. At the end, restores the original setting unless the flag
+ // is false.
WriteResult SendPacketToWriter(const char* buffer, size_t buf_len,
const QuicIpAddress& self_address,
- const QuicSocketAddress& peer_address,
- PerPacketOptions* options);
+ const QuicSocketAddress& destination_address,
+ PerPacketOptions* options,
+ QuicPacketWriter* writer);
QuicConnectionContext context_;
@@ -2376,10 +2400,14 @@
// Endpoints should never mark packets with Congestion Experienced (CE), as
// this is only done by routers. Endpoints cannot send ECT(0) or ECT(1) if
// their congestion control cannot respond to these signals in accordance with
- // the spec, or if the QUIC implementation doesn't validate ECN feedback. When
- // true, the connection will not verify that the requested codepoint adheres
- // to these policies. This is only accessible through QuicConnectionPeer.
+ // the spec, or ECN feedback doesn't conform to the spec. When true, the
+ // connection will not verify that the requested codepoint adheres to these
+ // policies. This is only accessible through QuicConnectionPeer.
bool disable_ecn_codepoint_validation_ = false;
+
+ // The ECN codepoint of the last packet to be sent to the writer, which
+ // might be different from the next codepoint in per_packet_options_.
+ QuicEcnCodepoint last_ecn_codepoint_sent_ = ECN_NOT_ECT;
};
} // namespace quic
diff --git a/quiche/quic/core/quic_connection_test.cc b/quiche/quic/core/quic_connection_test.cc
index 7853992..a274129 100644
--- a/quiche/quic/core/quic_connection_test.cc
+++ b/quiche/quic/core/quic_connection_test.cc
@@ -17462,6 +17462,7 @@
}
TEST_P(QuicConnectionTest, EcnCodepointsRejected) {
+ SetQuicReloadableFlag(quic_send_ect1, true);
TestPerPacketOptions per_packet_options;
connection_.set_per_packet_options(&per_packet_options);
for (QuicEcnCodepoint ecn : {ECN_NOT_ECT, ECN_ECT0, ECN_ECT1, ECN_CE}) {
@@ -17473,12 +17474,13 @@
}
EXPECT_CALL(connection_, OnSerializedPacket(_));
SendPing();
- EXPECT_EQ(per_packet_options.ecn_codepoint, ECN_NOT_ECT);
+ EXPECT_EQ(per_packet_options.ecn_codepoint, ecn);
EXPECT_EQ(writer_->last_ecn_sent(), ECN_NOT_ECT);
}
}
TEST_P(QuicConnectionTest, EcnCodepointsAccepted) {
+ SetQuicReloadableFlag(quic_send_ect1, true);
TestPerPacketOptions per_packet_options;
connection_.set_per_packet_options(&per_packet_options);
for (QuicEcnCodepoint ecn : {ECN_NOT_ECT, ECN_ECT0, ECN_ECT1, ECN_CE}) {
@@ -17494,12 +17496,26 @@
if (ecn == ECN_CE) {
expected_codepoint = ECN_NOT_ECT;
}
- EXPECT_EQ(per_packet_options.ecn_codepoint, expected_codepoint);
+ EXPECT_EQ(per_packet_options.ecn_codepoint, ecn);
EXPECT_EQ(writer_->last_ecn_sent(), expected_codepoint);
}
}
+TEST_P(QuicConnectionTest, EcnCodepointsRejectedIfFlagIsFalse) {
+ SetQuicReloadableFlag(quic_send_ect1, false);
+ TestPerPacketOptions per_packet_options;
+ connection_.set_per_packet_options(&per_packet_options);
+ for (QuicEcnCodepoint ecn : {ECN_NOT_ECT, ECN_ECT0, ECN_ECT1, ECN_CE}) {
+ per_packet_options.ecn_codepoint = ecn;
+ EXPECT_CALL(connection_, OnSerializedPacket(_));
+ SendPing();
+ EXPECT_EQ(per_packet_options.ecn_codepoint, ECN_NOT_ECT);
+ EXPECT_EQ(writer_->last_ecn_sent(), ECN_NOT_ECT);
+ }
+}
+
TEST_P(QuicConnectionTest, EcnValidationDisabled) {
+ SetQuicReloadableFlag(quic_send_ect1, true);
TestPerPacketOptions per_packet_options;
connection_.set_per_packet_options(&per_packet_options);
QuicConnectionPeer::DisableEcnCodepointValidation(&connection_);
@@ -17512,6 +17528,66 @@
}
}
+TEST_P(QuicConnectionTest, RtoDisablesEcnMarking) {
+ SetQuicReloadableFlag(quic_send_ect1, true);
+ EXPECT_CALL(*send_algorithm_, SupportsECT1()).WillRepeatedly(Return(true));
+ TestPerPacketOptions per_packet_options;
+ per_packet_options.ecn_codepoint = ECN_ECT1;
+ connection_.set_per_packet_options(&per_packet_options);
+ QuicPacketCreatorPeer::SetPacketNumber(
+ QuicConnectionPeer::GetPacketCreator(&connection_), 1);
+ SendPing();
+ connection_.OnRetransmissionTimeout();
+ EXPECT_EQ(writer_->last_ecn_sent(), ECN_NOT_ECT);
+ EXPECT_EQ(per_packet_options.ecn_codepoint, ECN_ECT1);
+ // On 2nd RTO, QUIC abandons ECN.
+ connection_.OnRetransmissionTimeout();
+ EXPECT_EQ(writer_->last_ecn_sent(), ECN_NOT_ECT);
+ EXPECT_EQ(per_packet_options.ecn_codepoint, ECN_NOT_ECT);
+}
+
+TEST_P(QuicConnectionTest, RtoDoesntDisableEcnMarkingIfEcnAcked) {
+ EXPECT_CALL(*send_algorithm_, SupportsECT1()).WillRepeatedly(Return(true));
+ TestPerPacketOptions per_packet_options;
+ per_packet_options.ecn_codepoint = ECN_ECT1;
+ connection_.set_per_packet_options(&per_packet_options);
+ QuicPacketCreatorPeer::SetPacketNumber(
+ QuicConnectionPeer::GetPacketCreator(&connection_), 1);
+ if (!GetQuicReloadableFlag(quic_send_ect1)) {
+ EXPECT_QUIC_BUG(connection_.OnInFlightEcnPacketAcked(),
+ "Unexpected call to OnInFlightEcnPacketAcked()");
+ return;
+ } else {
+ connection_.OnInFlightEcnPacketAcked();
+ }
+ SendPing();
+ // Because an ECN packet was acked, PTOs have no effect on ECN settings.
+ connection_.OnRetransmissionTimeout();
+ QuicEcnCodepoint expected_codepoint =
+ GetQuicReloadableFlag(quic_send_ect1) ? ECN_ECT1 : ECN_NOT_ECT;
+ EXPECT_EQ(writer_->last_ecn_sent(), expected_codepoint);
+ EXPECT_EQ(per_packet_options.ecn_codepoint, expected_codepoint);
+ connection_.OnRetransmissionTimeout();
+ EXPECT_EQ(writer_->last_ecn_sent(), expected_codepoint);
+ EXPECT_EQ(per_packet_options.ecn_codepoint, expected_codepoint);
+}
+
+TEST_P(QuicConnectionTest, InvalidFeedbackCancelsEcn) {
+ EXPECT_CALL(*send_algorithm_, SupportsECT1()).WillRepeatedly(Return(true));
+ TestPerPacketOptions per_packet_options;
+ per_packet_options.ecn_codepoint = ECN_ECT1;
+ connection_.set_per_packet_options(&per_packet_options);
+ EXPECT_EQ(per_packet_options.ecn_codepoint, ECN_ECT1);
+ if (!GetQuicReloadableFlag(quic_send_ect1)) {
+ EXPECT_QUIC_BUG(connection_.OnInvalidEcnFeedback(),
+ "Unexpected call to OnInvalidEcnFeedback().");
+ return;
+ } else {
+ connection_.OnInvalidEcnFeedback();
+ }
+ EXPECT_EQ(per_packet_options.ecn_codepoint, ECN_NOT_ECT);
+}
+
} // namespace
} // namespace test
} // namespace quic
diff --git a/quiche/quic/core/quic_flags_list.h b/quiche/quic/core/quic_flags_list.h
index cebbbff..612a6de 100644
--- a/quiche/quic/core/quic_flags_list.h
+++ b/quiche/quic/core/quic_flags_list.h
@@ -93,6 +93,8 @@
QUIC_FLAG(quic_restart_flag_quic_quiche_ecn_sockets, true)
// When true, report received ECN markings to the peer.
QUIC_FLAG(quic_restart_flag_quic_receive_ecn, true)
+// When true, sends QUIC packets marked ECT(1).
+QUIC_FLAG(quic_reloadable_flag_quic_send_ect1, false)
// When true, support draft-ietf-quic-v2-08
QUIC_FLAG(quic_reloadable_flag_quic_enable_version_2_draft_08, false)
// When true, the BB2U copt causes BBR2 to wait two rounds with out draining the queue before exiting PROBE_UP and BB2S has the same effect in STARTUP.
diff --git a/quiche/quic/core/quic_sent_packet_manager.cc b/quiche/quic/core/quic_sent_packet_manager.cc
index 1a098c9..6dc502e 100644
--- a/quiche/quic/core/quic_sent_packet_manager.cc
+++ b/quiche/quic/core/quic_sent_packet_manager.cc
@@ -315,12 +315,15 @@
void QuicSentPacketManager::PostProcessNewlyAckedPackets(
QuicPacketNumber ack_packet_number, EncryptionLevel ack_decrypted_level,
const QuicAckFrame& ack_frame, QuicTime ack_receive_time, bool rtt_updated,
- QuicByteCount prior_bytes_in_flight) {
+ QuicByteCount prior_bytes_in_flight,
+ absl::optional<QuicEcnCounts> ecn_counts) {
unacked_packets_.NotifyAggregatedStreamFrameAcked(
last_ack_frame_.ack_delay_time);
InvokeLossDetection(ack_receive_time);
- MaybeInvokeCongestionEvent(rtt_updated, prior_bytes_in_flight,
- ack_receive_time);
+ MaybeInvokeCongestionEvent(
+ rtt_updated, prior_bytes_in_flight, ack_receive_time, ecn_counts,
+ peer_ack_ecn_counts_[QuicUtils::GetPacketNumberSpace(
+ ack_decrypted_level)]);
unacked_packets_.RemoveObsoletePackets();
sustained_bandwidth_recorder_.RecordEstimate(
@@ -354,18 +357,37 @@
}
void QuicSentPacketManager::MaybeInvokeCongestionEvent(
- bool rtt_updated, QuicByteCount prior_in_flight, QuicTime event_time) {
+ bool rtt_updated, QuicByteCount prior_in_flight, QuicTime event_time,
+ absl::optional<QuicEcnCounts> ecn_counts,
+ const QuicEcnCounts& previous_counts) {
if (!rtt_updated && packets_acked_.empty() && packets_lost_.empty()) {
return;
}
const bool overshooting_detected =
stats_->overshooting_detected_with_network_parameters_adjusted;
+ // A connection should send at most one flavor of ECT, so only one variable
+ // is necessary.
+ QuicPacketCount newly_acked_ect = 0, newly_acked_ce = 0;
+ if (ecn_counts.has_value()) {
+ QUICHE_DCHECK(GetQuicReloadableFlag(quic_send_ect1));
+ newly_acked_ect = ecn_counts->ect1 - previous_counts.ect1;
+ if (newly_acked_ect == 0) {
+ newly_acked_ect = ecn_counts->ect0 - previous_counts.ect0;
+ } else {
+ QUIC_BUG_IF(quic_bug_518619343_04,
+ ecn_counts->ect0 - previous_counts.ect0)
+ << "Sent ECT(0) and ECT(1) newly acked in the same ACK.";
+ }
+ newly_acked_ce = ecn_counts->ce - previous_counts.ce;
+ }
if (using_pacing_) {
pacing_sender_.OnCongestionEvent(rtt_updated, prior_in_flight, event_time,
- packets_acked_, packets_lost_, 0, 0);
+ packets_acked_, packets_lost_,
+ newly_acked_ect, newly_acked_ce);
} else {
send_algorithm_->OnCongestionEvent(rtt_updated, prior_in_flight, event_time,
- packets_acked_, packets_lost_, 0, 0);
+ packets_acked_, packets_lost_,
+ newly_acked_ect, newly_acked_ce);
}
if (debug_delegate_ != nullptr && !overshooting_detected &&
stats_->overshooting_detected_with_network_parameters_adjusted) {
@@ -622,6 +644,28 @@
return frame;
}
+void QuicSentPacketManager::RecordEcnMarkingSent(QuicEcnCodepoint ecn_codepoint,
+ EncryptionLevel level) {
+ PacketNumberSpace space = QuicUtils::GetPacketNumberSpace(level);
+ switch (ecn_codepoint) {
+ case ECN_NOT_ECT:
+ break;
+ case ECN_ECT0:
+ ++ect0_packets_sent_[space];
+ break;
+ case ECN_ECT1:
+ ++ect1_packets_sent_[space];
+ break;
+ case ECN_CE:
+ // Test only: endpoints MUST NOT send CE. As CE reports will have to
+ // correspond to either an ECT(0) or an ECT(1) packet to be valid, just
+ // increment both to avoid validation failure.
+ ++ect0_packets_sent_[space];
+ ++ect1_packets_sent_[space];
+ break;
+ }
+}
+
bool QuicSentPacketManager::OnPacketSent(
SerializedPacket* mutable_packet, QuicTime sent_time,
TransmissionType transmission_type,
@@ -672,6 +716,7 @@
}
}
}
+ RecordEcnMarkingSent(ecn_codepoint, packet.encryption_level);
unacked_packets_.AddSentPacket(mutable_packet, transmission_type, sent_time,
in_flight, measure_rtt, ecn_codepoint);
// Reset the retransmission timer anytime a pending packet is sent.
@@ -699,7 +744,9 @@
QuicByteCount prior_in_flight = unacked_packets_.bytes_in_flight();
const QuicTime now = clock_->Now();
InvokeLossDetection(now);
- MaybeInvokeCongestionEvent(false, prior_in_flight, now);
+ MaybeInvokeCongestionEvent(false, prior_in_flight, now,
+ absl::optional<QuicEcnCounts>(),
+ peer_ack_ecn_counts_[APPLICATION_DATA]);
return LOSS_MODE;
}
case PTO_MODE:
@@ -1226,11 +1273,63 @@
}
}
+bool QuicSentPacketManager::IsEcnFeedbackValid(
+ PacketNumberSpace space, const absl::optional<QuicEcnCounts>& ecn_counts,
+ QuicPacketCount newly_acked_ect0, QuicPacketCount newly_acked_ect1) {
+ if (!ecn_counts.has_value()) {
+ if (newly_acked_ect0 > 0 || newly_acked_ect1 > 0) {
+ QUIC_DVLOG(1) << ENDPOINT
+ << "ECN packets acknowledged, no counts reported.";
+ return false;
+ }
+ return true;
+ }
+ if (ecn_counts->ect0 < peer_ack_ecn_counts_[space].ect0 ||
+ ecn_counts->ect1 < peer_ack_ecn_counts_[space].ect1 ||
+ ecn_counts->ce < peer_ack_ecn_counts_[space].ce) {
+ QUIC_DVLOG(1) << ENDPOINT << "Reported ECN count declined.";
+ return false;
+ }
+ if (ecn_counts->ect0 > ect0_packets_sent_[space] ||
+ ecn_counts->ect1 > ect1_packets_sent_[space] ||
+ (ecn_counts->ect0 + ecn_counts->ect1 + ecn_counts->ce >
+ ect0_packets_sent_[space] + ect1_packets_sent_[space])) {
+ QUIC_DVLOG(1) << ENDPOINT << "Reported ECT + CE exceeds packets sent:"
+ << " reported " << ecn_counts->ToString() << " , ECT(0) sent "
+ << ect0_packets_sent_[space] << " , ECT(1) sent "
+ << ect1_packets_sent_[space];
+ return false;
+ }
+ if ((newly_acked_ect0 >
+ (ecn_counts->ect0 + ecn_counts->ce - peer_ack_ecn_counts_[space].ect0 +
+ peer_ack_ecn_counts_[space].ce)) ||
+ (newly_acked_ect1 >
+ (ecn_counts->ect1 + ecn_counts->ce - peer_ack_ecn_counts_[space].ect1 +
+ peer_ack_ecn_counts_[space].ce))) {
+ QUIC_DVLOG(1) << ENDPOINT
+ << "Peer acked packet but did not report the ECN mark: "
+ << " New ECN counts: " << ecn_counts->ToString()
+ << " Old ECN counts: "
+ << peer_ack_ecn_counts_[space].ToString()
+ << " Newly acked ECT(0) : " << newly_acked_ect0
+ << " Newly acked ECT(1) : " << newly_acked_ect1;
+ return false;
+ }
+ return true;
+}
+
AckResult QuicSentPacketManager::OnAckFrameEnd(
QuicTime ack_receive_time, QuicPacketNumber ack_packet_number,
EncryptionLevel ack_decrypted_level,
const absl::optional<QuicEcnCounts>& ecn_counts) {
QuicByteCount prior_bytes_in_flight = unacked_packets_.bytes_in_flight();
+ QuicPacketCount newly_acked_ect0 = 0;
+ QuicPacketCount newly_acked_ect1 = 0;
+ PacketNumberSpace acked_packet_number_space =
+ QuicUtils::GetPacketNumberSpace(ack_decrypted_level);
+ QuicPacketNumber old_largest_acked =
+ unacked_packets_.GetLargestAckedOfPacketNumberSpace(
+ acked_packet_number_space);
// Reverse packets_acked_ so that it is in ascending order.
std::reverse(packets_acked_.begin(), packets_acked_.end());
for (AckedPacket& acked_packet : packets_acked_) {
@@ -1291,22 +1390,58 @@
// Unackable packets are skipped earlier.
largest_newly_acked_ = acked_packet.packet_number;
}
+ switch (info->ecn_codepoint) {
+ case ECN_NOT_ECT:
+ break;
+ case ECN_CE:
+ // ECN_CE should only happen in tests. Feedback validation doesn't track
+ // newly acked CEs, and if newly_acked_ect0 and newly_acked_ect1 are
+ // lower than expected that won't fail validation. So when it's CE don't
+ // increment anything.
+ break;
+ case ECN_ECT0:
+ ++newly_acked_ect0;
+ if (info->in_flight) {
+ network_change_visitor_->OnInFlightEcnPacketAcked();
+ }
+ break;
+ case ECN_ECT1:
+ ++newly_acked_ect1;
+ if (info->in_flight) {
+ network_change_visitor_->OnInFlightEcnPacketAcked();
+ }
+ break;
+ }
unacked_packets_.MaybeUpdateLargestAckedOfPacketNumberSpace(
packet_number_space, acked_packet.packet_number);
MarkPacketHandled(acked_packet.packet_number, info, ack_receive_time,
last_ack_frame_.ack_delay_time,
acked_packet.receive_timestamp);
}
- PacketNumberSpace packet_number_space =
- QuicUtils::GetPacketNumberSpace(ack_decrypted_level);
+ // Validate ECN feedback.
+ absl::optional<QuicEcnCounts> valid_ecn_counts;
+ if (GetQuicReloadableFlag(quic_send_ect1)) {
+ QUIC_RELOADABLE_FLAG_COUNT_N(quic_send_ect1, 1, 2);
+ if (IsEcnFeedbackValid(acked_packet_number_space, ecn_counts,
+ newly_acked_ect0, newly_acked_ect1)) {
+ valid_ecn_counts = ecn_counts;
+ } else if (!old_largest_acked.IsInitialized() ||
+ old_largest_acked <
+ unacked_packets_.GetLargestAckedOfPacketNumberSpace(
+ acked_packet_number_space)) {
+ // RFC 9000 S13.4.2.1: "An endpoint MUST NOT fail ECN validation as a
+ // result of processing an ACK frame that does not increase the largest
+ // acknowledged packet number."
+ network_change_visitor_->OnInvalidEcnFeedback();
+ }
+ }
const bool acked_new_packet = !packets_acked_.empty();
PostProcessNewlyAckedPackets(ack_packet_number, ack_decrypted_level,
last_ack_frame_, ack_receive_time, rtt_updated_,
- prior_bytes_in_flight);
- if (ecn_counts.has_value()) {
- peer_ack_ecn_counts_[packet_number_space] = ecn_counts.value();
+ prior_bytes_in_flight, valid_ecn_counts);
+ if (valid_ecn_counts.has_value()) {
+ peer_ack_ecn_counts_[acked_packet_number_space] = valid_ecn_counts.value();
}
-
return acked_new_packet ? PACKETS_NEWLY_ACKED : NO_PACKETS_NEWLY_ACKED;
}
diff --git a/quiche/quic/core/quic_sent_packet_manager.h b/quiche/quic/core/quic_sent_packet_manager.h
index e6c8105..a0e5c6a 100644
--- a/quiche/quic/core/quic_sent_packet_manager.h
+++ b/quiche/quic/core/quic_sent_packet_manager.h
@@ -105,6 +105,14 @@
// Called when the Path MTU may have increased.
virtual void OnPathMtuIncreased(QuicPacketLength packet_size) = 0;
+
+ // Called when a in-flight packet sent on the current default path with ECN
+ // markings is acked.
+ virtual void OnInFlightEcnPacketAcked() = 0;
+
+ // Called when an ACK frame with ECN counts has invalid values, or an ACK
+ // acknowledges packets with ECN marks and there are no ECN counts.
+ virtual void OnInvalidEcnFeedback() = 0;
};
// The retransmission timer is a single timer which switches modes depending
@@ -500,7 +508,9 @@
// triggered.
void MaybeInvokeCongestionEvent(bool rtt_updated,
QuicByteCount prior_in_flight,
- QuicTime event_time);
+ QuicTime event_time,
+ absl::optional<QuicEcnCounts> ecn_counts,
+ const QuicEcnCounts& previous_counts);
// Removes the retransmittability and in flight properties from the packet at
// |info| due to receipt by the peer.
@@ -521,7 +531,8 @@
EncryptionLevel ack_decrypted_level,
const QuicAckFrame& ack_frame,
QuicTime ack_receive_time, bool rtt_updated,
- QuicByteCount prior_bytes_in_flight);
+ QuicByteCount prior_bytes_in_flight,
+ absl::optional<QuicEcnCounts> ecn_counts);
// Notify observers that packet with QuicTransmissionInfo |info| is a spurious
// retransmission. It is caller's responsibility to guarantee the packet with
@@ -556,6 +567,20 @@
void OnAckFrequencyFrameAcked(
const QuicAckFrequencyFrame& ack_frequency_frame);
+ // Checks if newly reported ECN counts are valid given what has been reported
+ // in the past. |space| is the packet number space the counts apply to.
+ // |ecn_counts| is what the peer reported. |newly_acked_ect0| and
+ // |newly_acked_ect1| count the number of previously unacked packets with
+ // those markings that appeared in an ack block for the first time.
+ bool IsEcnFeedbackValid(PacketNumberSpace space,
+ const absl::optional<QuicEcnCounts>& ecn_counts,
+ QuicPacketCount newly_acked_ect0,
+ QuicPacketCount newly_acked_ect1);
+
+ // Update counters for the number of ECN-marked packets sent.
+ void RecordEcnMarkingSent(QuicEcnCodepoint ecn_codepoint,
+ EncryptionLevel level);
+
// Newly serialized retransmittable packets are added to this map, which
// contains owning pointers to any contained frames. If a packet is
// retransmitted, this map will contain entries for both the old and the new
@@ -671,6 +696,11 @@
// Whether to ignore the ack_delay in received ACKs.
bool ignore_ack_delay_;
+ // The total number of packets sent with ECT(0) or ECT(1) in each packet
+ // number space over the life of the connection.
+ QuicPacketCount ect0_packets_sent_[NUM_PACKET_NUMBER_SPACES] = {0, 0, 0};
+ QuicPacketCount ect1_packets_sent_[NUM_PACKET_NUMBER_SPACES] = {0, 0, 0};
+
// Most recent ECN codepoint counts received in an ACK frame sent by the peer.
QuicEcnCounts peer_ack_ecn_counts_[NUM_PACKET_NUMBER_SPACES];
};
diff --git a/quiche/quic/core/quic_sent_packet_manager_test.cc b/quiche/quic/core/quic_sent_packet_manager_test.cc
index 86a1d40..d222b6b 100644
--- a/quiche/quic/core/quic_sent_packet_manager_test.cc
+++ b/quiche/quic/core/quic_sent_packet_manager_test.cc
@@ -259,18 +259,23 @@
}
void SendDataPacket(uint64_t packet_number) {
- SendDataPacket(packet_number, ENCRYPTION_INITIAL);
+ SendDataPacket(packet_number, ENCRYPTION_INITIAL, ECN_NOT_ECT);
}
void SendDataPacket(uint64_t packet_number,
EncryptionLevel encryption_level) {
+ SendDataPacket(packet_number, encryption_level, ECN_NOT_ECT);
+ }
+
+ void SendDataPacket(uint64_t packet_number, EncryptionLevel encryption_level,
+ QuicEcnCodepoint ecn_codepoint) {
EXPECT_CALL(*send_algorithm_,
OnPacketSent(_, BytesInFlight(),
QuicPacketNumber(packet_number), _, _));
SerializedPacket packet(CreateDataPacket(packet_number));
packet.encryption_level = encryption_level;
manager_.OnPacketSent(&packet, clock_.Now(), NOT_RETRANSMISSION,
- HAS_RETRANSMITTABLE_DATA, true, ECN_NOT_ECT);
+ HAS_RETRANSMITTABLE_DATA, true, ecn_codepoint);
}
void SendPingPacket(uint64_t packet_number,
@@ -3190,14 +3195,28 @@
}
TEST_F(QuicSentPacketManagerTest, EcnCountsAreStored) {
+ if (!GetQuicReloadableFlag(quic_send_ect1)) {
+ return;
+ }
absl::optional<QuicEcnCounts> ecn_counts1, ecn_counts2, ecn_counts3;
- ecn_counts1 = {1, 2, 3};
+ ecn_counts1 = {1, 0, 3};
+ ecn_counts2 = {0, 3, 1};
+ ecn_counts3 = {0, 2, 0};
+ SendDataPacket(1, ENCRYPTION_INITIAL, ECN_ECT0);
+ SendDataPacket(2, ENCRYPTION_INITIAL, ECN_ECT0);
+ SendDataPacket(3, ENCRYPTION_INITIAL, ECN_ECT0);
+ SendDataPacket(4, ENCRYPTION_INITIAL, ECN_ECT0);
+ SendDataPacket(5, ENCRYPTION_HANDSHAKE, ECN_ECT1);
+ SendDataPacket(6, ENCRYPTION_HANDSHAKE, ECN_ECT1);
+ SendDataPacket(7, ENCRYPTION_HANDSHAKE, ECN_ECT1);
+ SendDataPacket(8, ENCRYPTION_HANDSHAKE, ECN_ECT1);
+ SendDataPacket(9, ENCRYPTION_FORWARD_SECURE, ECN_ECT1);
+ SendDataPacket(10, ENCRYPTION_FORWARD_SECURE, ECN_ECT1);
manager_.OnAckFrameEnd(clock_.Now(), QuicPacketNumber(1), ENCRYPTION_INITIAL,
ecn_counts1);
- ecn_counts2 = {0, 3, 1};
manager_.OnAckFrameEnd(clock_.Now(), QuicPacketNumber(2),
ENCRYPTION_HANDSHAKE, ecn_counts2);
- ecn_counts3 = {0, 2, 0};
+
manager_.OnAckFrameEnd(clock_.Now(), QuicPacketNumber(3),
ENCRYPTION_FORWARD_SECURE, ecn_counts3);
EXPECT_EQ(
@@ -3211,6 +3230,226 @@
ecn_counts3);
}
+TEST_F(QuicSentPacketManagerTest, EcnCountsReceived) {
+ if (!GetQuicReloadableFlag(quic_send_ect1)) {
+ return;
+ }
+ // Basic ECN reporting test. The reported counts are equal to the total sent,
+ // but more than the total acked. This is legal per the spec.
+ for (uint64_t i = 1; i <= 3; ++i) {
+ SendDataPacket(i, ENCRYPTION_FORWARD_SECURE, ECN_ECT1);
+ }
+ // Ack the last two packets, but report 3 counts (ack of 1 was lost).
+ EXPECT_CALL(*network_change_visitor_, OnInFlightEcnPacketAcked()).Times(2);
+ manager_.OnAckFrameStart(QuicPacketNumber(3), QuicTime::Delta::Infinite(),
+ clock_.Now());
+ manager_.OnAckRange(QuicPacketNumber(2), QuicPacketNumber(4));
+ EXPECT_CALL(*send_algorithm_,
+ OnCongestionEvent(_, _, _, Pointwise(PacketNumberEq(), {2, 3}),
+ IsEmpty(), 2, 1))
+ .Times(1);
+ EXPECT_CALL(*network_change_visitor_, OnCongestionChange()).Times(1);
+ absl::optional<QuicEcnCounts> ecn_counts = QuicEcnCounts();
+ ecn_counts->ect1 = QuicPacketCount(2);
+ ecn_counts->ce = QuicPacketCount(1);
+ EXPECT_EQ(PACKETS_NEWLY_ACKED,
+ manager_.OnAckFrameEnd(clock_.Now(), QuicPacketNumber(1),
+ ENCRYPTION_FORWARD_SECURE, ecn_counts));
+}
+
+TEST_F(QuicSentPacketManagerTest, PeerDecrementsEcnCounts) {
+ if (!GetQuicReloadableFlag(quic_send_ect1)) {
+ return;
+ }
+ for (uint64_t i = 1; i <= 5; ++i) {
+ SendDataPacket(i, ENCRYPTION_FORWARD_SECURE, ECN_ECT1);
+ }
+ // Ack all three packets).
+ EXPECT_CALL(*network_change_visitor_, OnInFlightEcnPacketAcked()).Times(3);
+ manager_.OnAckFrameStart(QuicPacketNumber(3), QuicTime::Delta::Infinite(),
+ clock_.Now());
+ manager_.OnAckRange(QuicPacketNumber(1), QuicPacketNumber(4));
+ EXPECT_CALL(*send_algorithm_,
+ OnCongestionEvent(_, _, _, Pointwise(PacketNumberEq(), {1, 2, 3}),
+ IsEmpty(), 2, 1))
+ .Times(1);
+ EXPECT_CALL(*network_change_visitor_, OnCongestionChange()).Times(1);
+ absl::optional<QuicEcnCounts> ecn_counts = QuicEcnCounts();
+ ecn_counts->ect1 = QuicPacketCount(2);
+ ecn_counts->ce = QuicPacketCount(1);
+ EXPECT_EQ(PACKETS_NEWLY_ACKED,
+ manager_.OnAckFrameEnd(clock_.Now(), QuicPacketNumber(1),
+ ENCRYPTION_FORWARD_SECURE, ecn_counts));
+ // New ack, counts decline
+ EXPECT_CALL(*network_change_visitor_, OnInFlightEcnPacketAcked()).Times(1);
+ manager_.OnAckFrameStart(QuicPacketNumber(4), QuicTime::Delta::Infinite(),
+ clock_.Now());
+ manager_.OnAckRange(QuicPacketNumber(4), QuicPacketNumber(5));
+ EXPECT_CALL(*network_change_visitor_, OnInvalidEcnFeedback());
+ EXPECT_CALL(*send_algorithm_,
+ OnCongestionEvent(_, _, _, Pointwise(PacketNumberEq(), {4}),
+ IsEmpty(), 0, 0))
+ .Times(1);
+ EXPECT_CALL(*network_change_visitor_, OnCongestionChange()).Times(1);
+ ecn_counts = QuicEcnCounts();
+ ecn_counts->ect1 = QuicPacketCount(3);
+ ecn_counts->ce = QuicPacketCount(0); // Reduced CE count
+ EXPECT_EQ(PACKETS_NEWLY_ACKED,
+ manager_.OnAckFrameEnd(clock_.Now(), QuicPacketNumber(2),
+ ENCRYPTION_FORWARD_SECURE, ecn_counts));
+}
+
+TEST_F(QuicSentPacketManagerTest, TooManyEcnCountsReported) {
+ if (!GetQuicReloadableFlag(quic_send_ect1)) {
+ return;
+ }
+ for (uint64_t i = 1; i <= 3; ++i) {
+ SendDataPacket(i, ENCRYPTION_FORWARD_SECURE, ECN_ECT1);
+ }
+ // Ack the last two packets, but report 3 counts (ack of 1 was lost).
+ EXPECT_CALL(*network_change_visitor_, OnInFlightEcnPacketAcked()).Times(2);
+ manager_.OnAckFrameStart(QuicPacketNumber(3), QuicTime::Delta::Infinite(),
+ clock_.Now());
+ manager_.OnAckRange(QuicPacketNumber(2), QuicPacketNumber(4));
+ absl::optional<QuicEcnCounts> ecn_counts = QuicEcnCounts();
+ // Report 4 counts, but only 3 packets were sent.
+ ecn_counts->ect1 = QuicPacketCount(3);
+ ecn_counts->ce = QuicPacketCount(1);
+ EXPECT_CALL(*network_change_visitor_, OnInvalidEcnFeedback());
+ EXPECT_CALL(*send_algorithm_,
+ OnCongestionEvent(_, _, _, Pointwise(PacketNumberEq(), {2, 3}),
+ IsEmpty(), 0, 0))
+ .Times(1);
+ EXPECT_CALL(*network_change_visitor_, OnCongestionChange()).Times(1);
+
+ EXPECT_EQ(PACKETS_NEWLY_ACKED,
+ manager_.OnAckFrameEnd(clock_.Now(), QuicPacketNumber(1),
+ ENCRYPTION_FORWARD_SECURE, ecn_counts));
+}
+
+TEST_F(QuicSentPacketManagerTest, PeerReportsWrongCodepoint) {
+ if (!GetQuicReloadableFlag(quic_send_ect1)) {
+ return;
+ }
+ for (uint64_t i = 1; i <= 3; ++i) {
+ SendDataPacket(i, ENCRYPTION_FORWARD_SECURE, ECN_ECT1);
+ }
+ // Ack the last two packets, but report 3 counts (ack of 1 was lost).
+ EXPECT_CALL(*network_change_visitor_, OnInFlightEcnPacketAcked()).Times(2);
+ manager_.OnAckFrameStart(QuicPacketNumber(3), QuicTime::Delta::Infinite(),
+ clock_.Now());
+ manager_.OnAckRange(QuicPacketNumber(2), QuicPacketNumber(4));
+ absl::optional<QuicEcnCounts> ecn_counts = QuicEcnCounts();
+ // Report the wrong codepoint.
+ ecn_counts->ect0 = QuicPacketCount(2);
+ ecn_counts->ce = QuicPacketCount(1);
+ EXPECT_CALL(*network_change_visitor_, OnInvalidEcnFeedback());
+ EXPECT_CALL(*send_algorithm_,
+ OnCongestionEvent(_, _, _, Pointwise(PacketNumberEq(), {2, 3}),
+ IsEmpty(), 0, 0))
+ .Times(1);
+ EXPECT_CALL(*network_change_visitor_, OnCongestionChange()).Times(1);
+
+ EXPECT_EQ(PACKETS_NEWLY_ACKED,
+ manager_.OnAckFrameEnd(clock_.Now(), QuicPacketNumber(1),
+ ENCRYPTION_FORWARD_SECURE, ecn_counts));
+}
+
+TEST_F(QuicSentPacketManagerTest, TooFewEcnCountsReported) {
+ if (!GetQuicReloadableFlag(quic_send_ect1)) {
+ return;
+ }
+ for (uint64_t i = 1; i <= 3; ++i) {
+ SendDataPacket(i, ENCRYPTION_FORWARD_SECURE, ECN_ECT1);
+ }
+ // Ack the last two packets, but report 3 counts (ack of 1 was lost).
+ EXPECT_CALL(*network_change_visitor_, OnInFlightEcnPacketAcked()).Times(2);
+ manager_.OnAckFrameStart(QuicPacketNumber(3), QuicTime::Delta::Infinite(),
+ clock_.Now());
+ manager_.OnAckRange(QuicPacketNumber(2), QuicPacketNumber(4));
+ EXPECT_CALL(*network_change_visitor_, OnInvalidEcnFeedback());
+ EXPECT_CALL(*send_algorithm_,
+ OnCongestionEvent(_, _, _, Pointwise(PacketNumberEq(), {2, 3}),
+ IsEmpty(), 0, 0))
+ .Times(1);
+ EXPECT_CALL(*network_change_visitor_, OnCongestionChange()).Times(1);
+ absl::optional<QuicEcnCounts> ecn_counts = QuicEcnCounts();
+ // 2 ECN packets were newly acked, but only one count was reported.
+ ecn_counts->ect1 = QuicPacketCount(1);
+ ecn_counts->ce = QuicPacketCount(0);
+ EXPECT_EQ(PACKETS_NEWLY_ACKED,
+ manager_.OnAckFrameEnd(clock_.Now(), QuicPacketNumber(1),
+ ENCRYPTION_FORWARD_SECURE, ecn_counts));
+}
+
+TEST_F(QuicSentPacketManagerTest,
+ EcnCountsNotValidatedIfLargestAckedUnchanged) {
+ if (!GetQuicReloadableFlag(quic_send_ect1)) {
+ return;
+ }
+ for (uint64_t i = 1; i <= 3; ++i) {
+ SendDataPacket(i, ENCRYPTION_FORWARD_SECURE, ECN_ECT1);
+ }
+ // Ack two packets.
+ EXPECT_CALL(*network_change_visitor_, OnInFlightEcnPacketAcked()).Times(2);
+ manager_.OnAckFrameStart(QuicPacketNumber(3), QuicTime::Delta::Infinite(),
+ clock_.Now());
+ manager_.OnAckRange(QuicPacketNumber(2), QuicPacketNumber(4));
+ EXPECT_CALL(*send_algorithm_,
+ OnCongestionEvent(_, _, _, Pointwise(PacketNumberEq(), {2, 3}),
+ IsEmpty(), 2, 1))
+ .Times(1);
+ EXPECT_CALL(*network_change_visitor_, OnCongestionChange()).Times(1);
+ absl::optional<QuicEcnCounts> ecn_counts = QuicEcnCounts();
+ ecn_counts->ect1 = QuicPacketCount(2);
+ ecn_counts->ce = QuicPacketCount(1);
+ EXPECT_EQ(PACKETS_NEWLY_ACKED,
+ manager_.OnAckFrameEnd(clock_.Now(), QuicPacketNumber(1),
+ ENCRYPTION_FORWARD_SECURE, ecn_counts));
+ // Ack the first packet, which will not update largest_acked.
+ EXPECT_CALL(*network_change_visitor_, OnInFlightEcnPacketAcked()).Times(1);
+ manager_.OnAckFrameStart(QuicPacketNumber(3), QuicTime::Delta::Infinite(),
+ clock_.Now());
+ manager_.OnAckRange(QuicPacketNumber(1), QuicPacketNumber(4));
+ EXPECT_CALL(*send_algorithm_,
+ OnCongestionEvent(_, _, _, Pointwise(PacketNumberEq(), {1}),
+ IsEmpty(), 0, 0))
+ .Times(1);
+ EXPECT_CALL(*network_change_visitor_, OnCongestionChange()).Times(1);
+ ecn_counts = QuicEcnCounts();
+ // Counts decline, but there's no validation because largest_acked didn't
+ // change.
+ ecn_counts->ect1 = QuicPacketCount(2);
+ ecn_counts->ce = QuicPacketCount(0); // Reduced CE count
+ EXPECT_EQ(PACKETS_NEWLY_ACKED,
+ manager_.OnAckFrameEnd(clock_.Now(), QuicPacketNumber(2),
+ ENCRYPTION_FORWARD_SECURE, ecn_counts));
+}
+
+TEST_F(QuicSentPacketManagerTest, EcnAckedButNoMarksReported) {
+ if (!GetQuicReloadableFlag(quic_send_ect1)) {
+ return;
+ }
+ for (uint64_t i = 1; i <= 3; ++i) {
+ SendDataPacket(i, ENCRYPTION_FORWARD_SECURE, ECN_ECT1);
+ }
+ // Ack the last two packets, but report 3 counts (ack of 1 was lost).
+ EXPECT_CALL(*network_change_visitor_, OnInFlightEcnPacketAcked()).Times(2);
+ manager_.OnAckFrameStart(QuicPacketNumber(3), QuicTime::Delta::Infinite(),
+ clock_.Now());
+ manager_.OnAckRange(QuicPacketNumber(2), QuicPacketNumber(4));
+ EXPECT_CALL(*network_change_visitor_, OnInvalidEcnFeedback());
+ EXPECT_CALL(*send_algorithm_,
+ OnCongestionEvent(_, _, _, Pointwise(PacketNumberEq(), {2, 3}),
+ IsEmpty(), 0, 0))
+ .Times(1);
+ EXPECT_CALL(*network_change_visitor_, OnCongestionChange()).Times(1);
+ absl::optional<QuicEcnCounts> ecn_counts = absl::nullopt;
+ EXPECT_EQ(PACKETS_NEWLY_ACKED,
+ manager_.OnAckFrameEnd(clock_.Now(), QuicPacketNumber(1),
+ ENCRYPTION_FORWARD_SECURE, ecn_counts));
+}
+
} // namespace
} // namespace test
} // namespace quic
diff --git a/quiche/quic/test_tools/quic_connection_peer.cc b/quiche/quic/test_tools/quic_connection_peer.cc
index 8bddb19..e065495 100644
--- a/quiche/quic/test_tools/quic_connection_peer.cc
+++ b/quiche/quic/test_tools/quic_connection_peer.cc
@@ -621,6 +621,10 @@
// static
void QuicConnectionPeer::DisableEcnCodepointValidation(
QuicConnection* connection) {
+ // disable_ecn_codepoint_validation_ doesn't work correctly if the flag
+ // isn't set; all tests that don't set the flag should hit this bug.
+ QUIC_BUG_IF(quic_bug_518619343_03, !GetQuicReloadableFlag(quic_send_ect1))
+ << "Test disables ECN validation without setting quic_send_ect1";
connection->disable_ecn_codepoint_validation_ = true;
}
diff --git a/quiche/quic/test_tools/quic_test_utils.h b/quiche/quic/test_tools/quic_test_utils.h
index 2835ac1..a60de26 100644
--- a/quiche/quic/test_tools/quic_test_utils.h
+++ b/quiche/quic/test_tools/quic_test_utils.h
@@ -1308,6 +1308,8 @@
MOCK_METHOD(void, OnCongestionChange, (), (override));
MOCK_METHOD(void, OnPathMtuIncreased, (QuicPacketLength), (override));
+ MOCK_METHOD(void, OnInFlightEcnPacketAcked, (), (override));
+ MOCK_METHOD(void, OnInvalidEcnFeedback, (), (override));
};
class MockQuicConnectionDebugVisitor : public QuicConnectionDebugVisitor {
