Record sent ECN marks correctly for coalesced and buffered packets. Repro of flaky test failure now passes. Protected by quic_reloadable_flag_quic_send_ect1. PiperOrigin-RevId: 529557120
diff --git a/quiche/quic/core/quic_coalesced_packet.cc b/quiche/quic/core/quic_coalesced_packet.cc index 802fac4..67091d6 100644 --- a/quiche/quic/core/quic_coalesced_packet.cc +++ b/quiche/quic/core/quic_coalesced_packet.cc
@@ -11,7 +11,7 @@ namespace quic { QuicCoalescedPacket::QuicCoalescedPacket() - : length_(0), max_packet_length_(0) {} + : length_(0), max_packet_length_(0), ecn_codepoint_(ECN_NOT_ECT) {} QuicCoalescedPacket::~QuicCoalescedPacket() { Clear(); } @@ -19,7 +19,8 @@ const SerializedPacket& packet, const QuicSocketAddress& self_address, const QuicSocketAddress& peer_address, quiche::QuicheBufferAllocator* allocator, - QuicPacketLength current_max_packet_length) { + QuicPacketLength current_max_packet_length, + QuicEcnCodepoint ecn_codepoint) { if (packet.encrypted_length == 0) { QUIC_BUG(quic_bug_10611_1) << "Trying to coalesce an empty packet"; return true; @@ -52,6 +53,10 @@ // Do not coalesce packets of the same encryption level. return false; } + if (ecn_codepoint != ecn_codepoint_) { + // Do not coalesce packets with different ECN codepoints. + return false; + } } if (length_ + packet.encrypted_length > max_packet_length_) { @@ -66,6 +71,7 @@ if (length_ > 0) { QUIC_CODE_COUNT(QUIC_SUCCESSFULLY_COALESCED_MULTIPLE_PACKETS); } + ecn_codepoint_ = ecn_codepoint; length_ += packet.encrypted_length; transmission_types_[packet.encryption_level] = packet.transmission_type; if (packet.encryption_level == ENCRYPTION_INITIAL) {
diff --git a/quiche/quic/core/quic_coalesced_packet.h b/quiche/quic/core/quic_coalesced_packet.h index 21a6e1e..b362937 100644 --- a/quiche/quic/core/quic_coalesced_packet.h +++ b/quiche/quic/core/quic_coalesced_packet.h
@@ -26,7 +26,8 @@ const QuicSocketAddress& self_address, const QuicSocketAddress& peer_address, quiche::QuicheBufferAllocator* allocator, - QuicPacketLength current_max_packet_length); + QuicPacketLength current_max_packet_length, + QuicEcnCodepoint ecn_codepoint); // Clears this coalesced packet. void Clear(); @@ -66,6 +67,8 @@ std::vector<size_t> packet_lengths() const; + QuicEcnCodepoint ecn_codepoint() const { return ecn_codepoint_; } + private: friend class test::QuicCoalescedPacketPeer; @@ -89,6 +92,9 @@ // frames are copied to allow it be re-serialized when this coalesced packet // gets sent. std::unique_ptr<SerializedPacket> initial_packet_; + + // A coalesced packet shares an ECN codepoint. + QuicEcnCodepoint ecn_codepoint_; }; } // namespace quic
diff --git a/quiche/quic/core/quic_coalesced_packet_test.cc b/quiche/quic/core/quic_coalesced_packet_test.cc index eb69372..00ba7d8 100644 --- a/quiche/quic/core/quic_coalesced_packet_test.cc +++ b/quiche/quic/core/quic_coalesced_packet_test.cc
@@ -31,7 +31,7 @@ packet1.retransmittable_frames.push_back( QuicFrame(QuicStreamFrame(1, true, 0, 100))); ASSERT_TRUE(coalesced.MaybeCoalescePacket(packet1, self_address, peer_address, - &allocator, 1500)); + &allocator, 1500, ECN_NOT_ECT)); EXPECT_EQ(PTO_RETRANSMISSION, coalesced.TransmissionTypeOfPacket(ENCRYPTION_INITIAL)); EXPECT_EQ(1500u, coalesced.max_packet_length()); @@ -40,12 +40,14 @@ EXPECT_EQ( "total_length: 1500 padding_size: 1000 packets: {ENCRYPTION_INITIAL}", coalesced.ToString(1500)); + EXPECT_EQ(coalesced.ecn_codepoint(), ECN_NOT_ECT); // Cannot coalesce packet of the same encryption level. SerializedPacket packet2(QuicPacketNumber(2), PACKET_4BYTE_PACKET_NUMBER, buffer, 500, false, false); - EXPECT_FALSE(coalesced.MaybeCoalescePacket(packet2, self_address, - peer_address, &allocator, 1500)); + EXPECT_FALSE(coalesced.MaybeCoalescePacket( + packet2, self_address, peer_address, &allocator, 1500, ECN_NOT_ECT)); + EXPECT_EQ(coalesced.ecn_codepoint(), ECN_NOT_ECT); SerializedPacket packet3(QuicPacketNumber(3), PACKET_4BYTE_PACKET_NUMBER, buffer, 500, false, false); @@ -53,7 +55,7 @@ packet3.encryption_level = ENCRYPTION_ZERO_RTT; packet3.transmission_type = LOSS_RETRANSMISSION; ASSERT_TRUE(coalesced.MaybeCoalescePacket(packet3, self_address, peer_address, - &allocator, 1500)); + &allocator, 1500, ECN_NOT_ECT)); EXPECT_EQ(1500u, coalesced.max_packet_length()); EXPECT_EQ(1000u, coalesced.length()); EXPECT_EQ(2u, coalesced.NumberOfPackets()); @@ -63,6 +65,7 @@ "total_length: 1500 padding_size: 500 packets: {ENCRYPTION_INITIAL, " "ENCRYPTION_ZERO_RTT}", coalesced.ToString(1500)); + EXPECT_EQ(coalesced.ecn_codepoint(), ECN_NOT_ECT); SerializedPacket packet4(QuicPacketNumber(4), PACKET_4BYTE_PACKET_NUMBER, buffer, 500, false, false); @@ -70,28 +73,31 @@ // Cannot coalesce packet of changed self/peer address. EXPECT_FALSE(coalesced.MaybeCoalescePacket( packet4, QuicSocketAddress(QuicIpAddress::Loopback4(), 3), peer_address, - &allocator, 1500)); + &allocator, 1500, ECN_NOT_ECT)); // Packet does not fit. SerializedPacket packet5(QuicPacketNumber(5), PACKET_4BYTE_PACKET_NUMBER, buffer, 501, false, false); packet5.encryption_level = ENCRYPTION_FORWARD_SECURE; - EXPECT_FALSE(coalesced.MaybeCoalescePacket(packet5, self_address, - peer_address, &allocator, 1500)); + EXPECT_FALSE(coalesced.MaybeCoalescePacket( + packet5, self_address, peer_address, &allocator, 1500, ECN_NOT_ECT)); EXPECT_EQ(1500u, coalesced.max_packet_length()); EXPECT_EQ(1000u, coalesced.length()); EXPECT_EQ(2u, coalesced.NumberOfPackets()); + EXPECT_EQ(coalesced.ecn_codepoint(), ECN_NOT_ECT); // Max packet number length changed. SerializedPacket packet6(QuicPacketNumber(6), PACKET_4BYTE_PACKET_NUMBER, buffer, 100, false, false); packet6.encryption_level = ENCRYPTION_FORWARD_SECURE; - EXPECT_QUIC_BUG(coalesced.MaybeCoalescePacket(packet6, self_address, - peer_address, &allocator, 1000), - "Max packet length changes in the middle of the write path"); + EXPECT_QUIC_BUG( + coalesced.MaybeCoalescePacket(packet6, self_address, peer_address, + &allocator, 1000, ECN_NOT_ECT), + "Max packet length changes in the middle of the write path"); EXPECT_EQ(1500u, coalesced.max_packet_length()); EXPECT_EQ(1000u, coalesced.length()); EXPECT_EQ(2u, coalesced.NumberOfPackets()); + EXPECT_EQ(coalesced.ecn_codepoint(), ECN_NOT_ECT); } TEST(QuicCoalescedPacketTest, CopyEncryptedBuffers) { @@ -111,10 +117,11 @@ packet2.encryption_level = ENCRYPTION_FORWARD_SECURE; ASSERT_TRUE(coalesced.MaybeCoalescePacket(packet1, self_address, peer_address, - &allocator, 1500)); + &allocator, 1500, ECN_NOT_ECT)); ASSERT_TRUE(coalesced.MaybeCoalescePacket(packet2, self_address, peer_address, - &allocator, 1500)); + &allocator, 1500, ECN_NOT_ECT)); EXPECT_EQ(1000u, coalesced.length()); + EXPECT_EQ(coalesced.ecn_codepoint(), ECN_NOT_ECT); char copy_buffer[1000]; size_t length_copied = 0; @@ -152,7 +159,7 @@ packet1.retransmittable_frames.push_back( QuicFrame(QuicStreamFrame(1, true, 0, 100))); ASSERT_TRUE(coalesced.MaybeCoalescePacket(packet1, self_address, peer_address, - &allocator, 1500)); + &allocator, 1500, ECN_NOT_ECT)); EXPECT_EQ(PTO_RETRANSMISSION, coalesced.TransmissionTypeOfPacket(ENCRYPTION_INITIAL)); EXPECT_EQ(1500u, coalesced.max_packet_length()); @@ -160,16 +167,17 @@ EXPECT_EQ( "total_length: 1500 padding_size: 1000 packets: {ENCRYPTION_INITIAL}", coalesced.ToString(1500)); - // Neuter initial packet. + EXPECT_EQ(coalesced.ecn_codepoint(), ECN_NOT_ECT); coalesced.NeuterInitialPacket(); EXPECT_EQ(0u, coalesced.max_packet_length()); EXPECT_EQ(0u, coalesced.length()); EXPECT_EQ("total_length: 0 padding_size: 0 packets: {}", coalesced.ToString(0)); + EXPECT_EQ(coalesced.ecn_codepoint(), ECN_NOT_ECT); // Coalesce initial packet again. ASSERT_TRUE(coalesced.MaybeCoalescePacket(packet1, self_address, peer_address, - &allocator, 1500)); + &allocator, 1500, ECN_NOT_ECT)); SerializedPacket packet2(QuicPacketNumber(3), PACKET_4BYTE_PACKET_NUMBER, buffer, 500, false, false); @@ -177,7 +185,7 @@ packet2.encryption_level = ENCRYPTION_ZERO_RTT; packet2.transmission_type = LOSS_RETRANSMISSION; ASSERT_TRUE(coalesced.MaybeCoalescePacket(packet2, self_address, peer_address, - &allocator, 1500)); + &allocator, 1500, ECN_NOT_ECT)); EXPECT_EQ(1500u, coalesced.max_packet_length()); EXPECT_EQ(1000u, coalesced.length()); EXPECT_EQ(LOSS_RETRANSMISSION, @@ -186,6 +194,7 @@ "total_length: 1500 padding_size: 500 packets: {ENCRYPTION_INITIAL, " "ENCRYPTION_ZERO_RTT}", coalesced.ToString(1500)); + EXPECT_EQ(coalesced.ecn_codepoint(), ECN_NOT_ECT); // Neuter initial packet. coalesced.NeuterInitialPacket(); @@ -194,18 +203,52 @@ EXPECT_EQ( "total_length: 1500 padding_size: 1000 packets: {ENCRYPTION_ZERO_RTT}", coalesced.ToString(1500)); + EXPECT_EQ(coalesced.ecn_codepoint(), ECN_NOT_ECT); SerializedPacket packet3(QuicPacketNumber(5), PACKET_4BYTE_PACKET_NUMBER, buffer, 501, false, false); packet3.encryption_level = ENCRYPTION_FORWARD_SECURE; EXPECT_TRUE(coalesced.MaybeCoalescePacket(packet3, self_address, peer_address, - &allocator, 1500)); + &allocator, 1500, ECN_NOT_ECT)); EXPECT_EQ(1500u, coalesced.max_packet_length()); EXPECT_EQ(1001u, coalesced.length()); + EXPECT_EQ(coalesced.ecn_codepoint(), ECN_NOT_ECT); // Neuter initial packet. coalesced.NeuterInitialPacket(); EXPECT_EQ(1500u, coalesced.max_packet_length()); EXPECT_EQ(1001u, coalesced.length()); + EXPECT_EQ(coalesced.ecn_codepoint(), ECN_NOT_ECT); +} + +TEST(QuicCoalescedPacketTest, DoNotCoalesceDifferentEcn) { + QuicCoalescedPacket coalesced; + EXPECT_EQ("total_length: 0 padding_size: 0 packets: {}", + coalesced.ToString(0)); + quiche::SimpleBufferAllocator allocator; + EXPECT_EQ(0u, coalesced.length()); + EXPECT_EQ(0u, coalesced.NumberOfPackets()); + char buffer[1000]; + QuicSocketAddress self_address(QuicIpAddress::Loopback4(), 1); + QuicSocketAddress peer_address(QuicIpAddress::Loopback4(), 2); + SerializedPacket packet1(QuicPacketNumber(1), PACKET_4BYTE_PACKET_NUMBER, + buffer, 500, false, false); + packet1.transmission_type = PTO_RETRANSMISSION; + QuicAckFrame ack_frame(InitAckFrame(1)); + packet1.nonretransmittable_frames.push_back(QuicFrame(&ack_frame)); + packet1.retransmittable_frames.push_back( + QuicFrame(QuicStreamFrame(1, true, 0, 100))); + ASSERT_TRUE(coalesced.MaybeCoalescePacket(packet1, self_address, peer_address, + &allocator, 1500, ECN_ECT1)); + EXPECT_EQ(coalesced.ecn_codepoint(), ECN_ECT1); + + SerializedPacket packet2(QuicPacketNumber(2), PACKET_4BYTE_PACKET_NUMBER, + buffer, 500, false, false); + packet2.nonretransmittable_frames.push_back(QuicFrame(QuicPaddingFrame(100))); + packet2.encryption_level = ENCRYPTION_ZERO_RTT; + packet2.transmission_type = LOSS_RETRANSMISSION; + EXPECT_FALSE(coalesced.MaybeCoalescePacket( + packet2, self_address, peer_address, &allocator, 1500, ECN_NOT_ECT)); + EXPECT_EQ(coalesced.ecn_codepoint(), ECN_ECT1); } } // namespace
diff --git a/quiche/quic/core/quic_connection.cc b/quiche/quic/core/quic_connection.cc index 9fd036f..4ff1e85 100644 --- a/quiche/quic/core/quic_connection.cc +++ b/quiche/quic/core/quic_connection.cc
@@ -3119,7 +3119,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_, writer_); + packet.peer_address, writer_, packet.ecn_codepoint); 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 @@ -3438,7 +3438,8 @@ if (!coalesced_packet_.MaybeCoalescePacket( *packet, send_from_address, send_to_address, helper_->GetStreamSendBufferAllocator(), - packet_creator_.max_packet_length())) { + packet_creator_.max_packet_length(), + GetEcnCodepointToSend(send_to_address))) { // Failed to coalesce packet, flush current coalesced packet. if (!FlushCoalescedPacket()) { QUIC_BUG_IF(quic_connection_connected_after_flush_coalesced_failure, @@ -3451,7 +3452,8 @@ if (!coalesced_packet_.MaybeCoalescePacket( *packet, send_from_address, send_to_address, helper_->GetStreamSendBufferAllocator(), - packet_creator_.max_packet_length())) { + packet_creator_.max_packet_length(), + GetEcnCodepointToSend(send_to_address))) { // Failed to coalesce packet even it is the only packet, raise a write // error. QUIC_DLOG(ERROR) << ENDPOINT << "Failed to coalesce packet"; @@ -3466,12 +3468,14 @@ packet_creator_.SetSoftMaxPacketLength( coalesced_packet_.max_packet_length() - coalesced_packet_.length()); } + last_ecn_codepoint_sent_ = coalesced_packet_.ecn_codepoint(); break; case BUFFER: QUIC_DVLOG(1) << ENDPOINT << "Adding packet: " << packet->packet_number << " to buffered packets"; + last_ecn_codepoint_sent_ = GetEcnCodepointToSend(send_to_address); buffered_packets_.emplace_back(*packet, send_from_address, - send_to_address); + send_to_address, last_ecn_codepoint_sent_); break; case SEND_TO_WRITER: // Stop using coalescer from now on. @@ -3484,9 +3488,9 @@ // // writer_->WritePacket transfers buffer ownership back to the writer. packet->release_encrypted_buffer = nullptr; - result = SendPacketToWriter(packet->encrypted_buffer, encrypted_length, - send_from_address.host(), send_to_address, - per_packet_options_, writer_); + result = SendPacketToWriter( + packet->encrypted_buffer, encrypted_length, send_from_address.host(), + send_to_address, writer_, GetEcnCodepointToSend(send_to_address)); // 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 @@ -3521,7 +3525,7 @@ QUIC_DVLOG(1) << ENDPOINT << "Adding packet: " << packet->packet_number << " to buffered packets"; buffered_packets_.emplace_back(*packet, send_from_address, - send_to_address); + send_to_address, last_ecn_codepoint_sent_); } } @@ -3946,7 +3950,8 @@ first_serialized_one_rtt_packet_ == nullptr && serialized_packet.encryption_level == ENCRYPTION_FORWARD_SECURE) { first_serialized_one_rtt_packet_ = std::make_unique<BufferedPacket>( - serialized_packet, self_address(), peer_address()); + serialized_packet, self_address(), peer_address(), + GetEcnCodepointToSend(peer_address())); } SendOrQueuePacket(std::move(serialized_packet)); } @@ -4212,13 +4217,13 @@ WriteResult QuicConnection::SendPacketToWriter( const char* buffer, size_t buf_len, const QuicIpAddress& self_address, - const QuicSocketAddress& destination_address, PerPacketOptions* options, - QuicPacketWriter* writer) { + const QuicSocketAddress& destination_address, QuicPacketWriter* writer, + const QuicEcnCodepoint ecn_codepoint) { 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); + last_ecn_codepoint_sent_ = ecn_codepoint; + MaybeSetEcnCodepoint(ecn_codepoint); + WriteResult result = writer->WritePacket( + buffer, buf_len, self_address, destination_address, per_packet_options_); MaybeSetEcnCodepoint(original_codepoint); return result; } @@ -4970,17 +4975,18 @@ QuicConnection::BufferedPacket::BufferedPacket( const SerializedPacket& packet, const QuicSocketAddress& self_address, - const QuicSocketAddress& peer_address) + const QuicSocketAddress& peer_address, const QuicEcnCodepoint ecn_codepoint) : BufferedPacket(packet.encrypted_buffer, packet.encrypted_length, - self_address, peer_address) {} + self_address, peer_address, ecn_codepoint) {} QuicConnection::BufferedPacket::BufferedPacket( const char* encrypted_buffer, QuicPacketLength encrypted_length, const QuicSocketAddress& self_address, - const QuicSocketAddress& peer_address) + const QuicSocketAddress& peer_address, const QuicEcnCodepoint ecn_codepoint) : length(encrypted_length), self_address(self_address), - peer_address(peer_address) { + peer_address(peer_address), + ecn_codepoint(ecn_codepoint) { data = std::make_unique<char[]>(encrypted_length); memcpy(data.get(), encrypted_buffer, encrypted_length); } @@ -5159,7 +5165,7 @@ packet->encrypted_buffer, packet->encrypted_length)); WriteResult result = SendPacketToWriter( packet->encrypted_buffer, packet->encrypted_length, self_address.host(), - peer_address, per_packet_options_, writer); + peer_address, writer, GetEcnCodepointToSend(peer_address)); // If using a batch writer and the probing packet is buffered, flush it. if (writer->IsBatchMode() && result.status == WRITE_STATUS_OK && @@ -6057,11 +6063,13 @@ << "Buffering coalesced packet of len: " << length; buffered_packets_.emplace_back( buffer, static_cast<QuicPacketLength>(length), - coalesced_packet_.self_address(), coalesced_packet_.peer_address()); + coalesced_packet_.self_address(), coalesced_packet_.peer_address(), + coalesced_packet_.ecn_codepoint()); } else { WriteResult result = SendPacketToWriter( buffer, length, coalesced_packet_.self_address().host(), - coalesced_packet_.peer_address(), per_packet_options_, writer_); + coalesced_packet_.peer_address(), writer_, + coalesced_packet_.ecn_codepoint()); if (IsWriteError(result.status)) { OnWriteError(result.error_code); return false; @@ -6073,7 +6081,8 @@ << "Buffering coalesced packet of len: " << length; buffered_packets_.emplace_back( buffer, static_cast<QuicPacketLength>(length), - coalesced_packet_.self_address(), coalesced_packet_.peer_address()); + coalesced_packet_.self_address(), coalesced_packet_.peer_address(), + coalesced_packet_.ecn_codepoint()); } } } @@ -6425,7 +6434,7 @@ buffered_packets_.emplace_back( first_serialized_one_rtt_packet_->data.get(), first_serialized_one_rtt_packet_->length, self_address(), - peer_address()); + peer_address(), first_serialized_one_rtt_packet_->ecn_codepoint); packet_buffered = true; } break;
diff --git a/quiche/quic/core/quic_connection.h b/quiche/quic/core/quic_connection.h index 09c1eed..5645e96 100644 --- a/quiche/quic/core/quic_connection.h +++ b/quiche/quic/core/quic_connection.h
@@ -1480,11 +1480,13 @@ struct QUIC_EXPORT_PRIVATE BufferedPacket { BufferedPacket(const SerializedPacket& packet, const QuicSocketAddress& self_address, - const QuicSocketAddress& peer_address); + const QuicSocketAddress& peer_address, + QuicEcnCodepoint ecn_codepoint); BufferedPacket(const char* encrypted_buffer, QuicPacketLength encrypted_length, const QuicSocketAddress& self_address, - const QuicSocketAddress& peer_address); + const QuicSocketAddress& peer_address, + QuicEcnCodepoint ecn_codepoint); // Please note, this buffered packet contains random bytes (and is not // *actually* a QUIC packet). BufferedPacket(QuicRandom& random, QuicPacketLength encrypted_length, @@ -1500,6 +1502,7 @@ // Self and peer addresses when the packet is serialized. const QuicSocketAddress self_address; const QuicSocketAddress peer_address; + QuicEcnCodepoint ecn_codepoint = ECN_NOT_ECT; }; // ReceivedPacketInfo comprises the received packet information. @@ -1983,18 +1986,13 @@ // 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. + // Writes the packet to |writer| with the ECN mark specified in + // |ecn_codepoint|. Will also set last_ecn_sent_ appropriately. WriteResult SendPacketToWriter(const char* buffer, size_t buf_len, const QuicIpAddress& self_address, const QuicSocketAddress& destination_address, - PerPacketOptions* options, - QuicPacketWriter* writer); + QuicPacketWriter* writer, + const QuicEcnCodepoint ecn_codepoint); QuicConnectionContext context_;
diff --git a/quiche/quic/core/quic_connection_test.cc b/quiche/quic/core/quic_connection_test.cc index 64a0358..6a0e9d0 100644 --- a/quiche/quic/core/quic_connection_test.cc +++ b/quiche/quic/core/quic_connection_test.cc
@@ -17571,6 +17571,66 @@ EXPECT_EQ(per_packet_options.ecn_codepoint, ECN_NOT_ECT); } +TEST_P(QuicConnectionTest, StateMatchesSentEcn) { + 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); + SendPing(); + QuicSentPacketManager* sent_packet_manager = + QuicConnectionPeer::GetSentPacketManager(&connection_); + EXPECT_EQ(writer_->last_ecn_sent(), ECN_ECT1); + EXPECT_EQ( + QuicSentPacketManagerPeer::GetEct1Sent(sent_packet_manager, INITIAL_DATA), + 1); +} + +TEST_P(QuicConnectionTest, CoalescedPacketSplitsEcn) { + if (!connection_.version().CanSendCoalescedPackets()) { + return; + } + 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); + // All these steps are necessary to send an INITIAL ping and save it to be + // coalesced, instead of just calling SendPing() and sending it immediately. + char buffer[1000]; + creator_->set_encryption_level(ENCRYPTION_INITIAL); + QuicFrames frames; + QuicPingFrame ping; + frames.emplace_back(QuicFrame(ping)); + SerializedPacket packet1 = QuicPacketCreatorPeer::SerializeAllFrames( + creator_, frames, buffer, sizeof(buffer)); + connection_.SendOrQueuePacket(std::move(packet1)); + creator_->set_encryption_level(ENCRYPTION_FORWARD_SECURE); + EXPECT_CALL(*send_algorithm_, SupportsECT0()).WillRepeatedly(Return(true)); + // If not for the line below, these packets would coalesce. + per_packet_options.ecn_codepoint = ECN_ECT0; + EXPECT_EQ(writer_->packets_write_attempts(), 0); + SendPing(); + EXPECT_EQ(writer_->packets_write_attempts(), 2); + EXPECT_EQ(writer_->last_ecn_sent(), ECN_ECT0); +} + +TEST_P(QuicConnectionTest, BufferedPacketRetainsOldEcn) { + 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); + writer_->SetWriteBlocked(); + EXPECT_CALL(visitor_, OnWriteBlocked()).Times(2); + SendPing(); + EXPECT_CALL(*send_algorithm_, SupportsECT0()).WillRepeatedly(Return(true)); + per_packet_options.ecn_codepoint = ECN_ECT0; + writer_->SetWritable(); + connection_.OnCanWrite(); + EXPECT_EQ(writer_->last_ecn_sent(), ECN_ECT1); +} + } // namespace } // namespace test } // namespace quic
diff --git a/quiche/quic/core/quic_packet_creator_test.cc b/quiche/quic/core/quic_packet_creator_test.cc index ec8bcd7..b2fec7b 100644 --- a/quiche/quic/core/quic_packet_creator_test.cc +++ b/quiche/quic/core/quic_packet_creator_test.cc
@@ -2276,9 +2276,9 @@ SerializedPacket serialized = SerializeAllFrames(frames_); EXPECT_EQ(level, serialized.encryption_level); frames_.clear(); - ASSERT_TRUE(coalesced.MaybeCoalescePacket(serialized, self_address, - peer_address, &allocator, - creator_.max_packet_length())); + ASSERT_TRUE(coalesced.MaybeCoalescePacket( + serialized, self_address, peer_address, &allocator, + creator_.max_packet_length(), ECN_NOT_ECT)); } char buffer[kMaxOutgoingPacketSize]; size_t coalesced_length = creator_.SerializeCoalescedPacket(
diff --git a/quiche/quic/test_tools/quic_sent_packet_manager_peer.cc b/quiche/quic/test_tools/quic_sent_packet_manager_peer.cc index c895415..d6256fe 100644 --- a/quiche/quic/test_tools/quic_sent_packet_manager_peer.cc +++ b/quiche/quic/test_tools/quic_sent_packet_manager_peer.cc
@@ -182,5 +182,17 @@ return &(sent_packet_manager->peer_ack_ecn_counts_[space]); } +// static +QuicPacketCount QuicSentPacketManagerPeer::GetEct0Sent( + QuicSentPacketManager* sent_packet_manager, PacketNumberSpace space) { + return sent_packet_manager->ect0_packets_sent_[space]; +} + +// static +QuicPacketCount QuicSentPacketManagerPeer::GetEct1Sent( + QuicSentPacketManager* sent_packet_manager, PacketNumberSpace space) { + return sent_packet_manager->ect1_packets_sent_[space]; +} + } // namespace test } // namespace quic
diff --git a/quiche/quic/test_tools/quic_sent_packet_manager_peer.h b/quiche/quic/test_tools/quic_sent_packet_manager_peer.h index e960619..c0ea673 100644 --- a/quiche/quic/test_tools/quic_sent_packet_manager_peer.h +++ b/quiche/quic/test_tools/quic_sent_packet_manager_peer.h
@@ -88,6 +88,12 @@ static QuicEcnCounts* GetPeerEcnCounts( QuicSentPacketManager* sent_packet_manager, PacketNumberSpace space); + + static QuicPacketCount GetEct0Sent(QuicSentPacketManager* sent_packet_manager, + PacketNumberSpace space); + + static QuicPacketCount GetEct1Sent(QuicSentPacketManager* sent_packet_manager, + PacketNumberSpace space); }; } // namespace test