diff --git a/quic/api/QuicPacketScheduler.cpp b/quic/api/QuicPacketScheduler.cpp index 697b231c3..5394cc7c1 100644 --- a/quic/api/QuicPacketScheduler.cpp +++ b/quic/api/QuicPacketScheduler.cpp @@ -920,14 +920,19 @@ SchedulingResult CloningScheduler::scheduleFramesForPacket( } // The packet is already a clone if (outstandingPacket.maybeClonedPacketIdentifier) { - // If packet has CRYPTO frame, don't clone it again even if not processed - // yet, move on to give next packet a chance to be cloned const auto& frames = outstandingPacket.packet.frames; - if (conn_.transportSettings.cloneAllPacketsWithCryptoFrame && - std::find_if(frames.begin(), frames.end(), [](const auto& frame) { - return frame.type() == QuicWriteFrame::Type::WriteCryptoFrame; - }) != frames.end()) { - continue; + if (conn_.transportSettings.cloneAllPacketsWithCryptoFrame) { + // Has CRYPTO frame + if (std::find_if(frames.begin(), frames.end(), [](const auto& frame) { + return frame.type() == QuicWriteFrame::Type::WriteCryptoFrame; + }) != frames.end()) { + auto mostRecentOutstandingPacketIdentifier = + conn_.outstandings.packets.back().maybeClonedPacketIdentifier; + if (mostRecentOutstandingPacketIdentifier == + outstandingPacket.maybeClonedPacketIdentifier) { + continue; + } + } } // Otherwise, clone until it is processed if (conn_.outstandings.clonedPacketIdentifiers.count( diff --git a/quic/api/test/QuicPacketSchedulerTest.cpp b/quic/api/test/QuicPacketSchedulerTest.cpp index b7934b1b0..14a6f7d91 100644 --- a/quic/api/test/QuicPacketSchedulerTest.cpp +++ b/quic/api/test/QuicPacketSchedulerTest.cpp @@ -642,29 +642,60 @@ TEST_P( FrameScheduler noopScheduler("frame", conn); CloningScheduler cloningScheduler(noopScheduler, conn, "cryptoClone", 0); - // First packet has a crypto frame - PacketNum firstPacketNum = addOutstandingPacket(conn); - conn.outstandings.packets.back().packet.frames.push_back( - WriteCryptoFrame(0, 1)); - ClonedPacketIdentifier clonedPacketIdentifier( - PacketNumberSpace::AppData, firstPacketNum); - conn.outstandings.packets.back().maybeClonedPacketIdentifier = - clonedPacketIdentifier; - // It is not processed yet - conn.outstandings.clonedPacketIdentifiers.insert(clonedPacketIdentifier); - // There needs to have retransmittable frame for the rebuilder to work - conn.outstandings.packets.back().packet.frames.push_back( - MaxDataFrame(conn.flowControlState.advertisedMaxOffset)); + PacketNum firstPacketNum = addInitialOutstandingPacket(conn); + { + conn.outstandings.packets.back().packet.frames.push_back( + WriteCryptoFrame(0, 1)); + ClonedPacketIdentifier clonedPacketIdentifier( + PacketNumberSpace::Initial, firstPacketNum); + conn.outstandings.packets.back().maybeClonedPacketIdentifier = + clonedPacketIdentifier; + // It is not processed yet + conn.outstandings.clonedPacketIdentifiers.insert(clonedPacketIdentifier); + // There needs to have retransmittable frame for the rebuilder to work + conn.outstandings.packets.back().packet.frames.push_back( + MaxDataFrame(conn.flowControlState.advertisedMaxOffset)); + } - PacketNum secondPacketNum = addOutstandingPacket(conn); - // There needs to have retransmittable frame for the rebuilder to work - conn.outstandings.packets.back().packet.frames.push_back( - MaxDataFrame(conn.flowControlState.advertisedMaxOffset)); + PacketNum secondPacketNum = addInitialOutstandingPacket(conn); + { + conn.outstandings.packets.back().packet.frames.push_back( + WriteCryptoFrame(0, 1)); + ClonedPacketIdentifier clonedPacketIdentifier( + PacketNumberSpace::Initial, secondPacketNum); + conn.outstandings.packets.back().maybeClonedPacketIdentifier = + clonedPacketIdentifier; + // It is not processed yet + conn.outstandings.clonedPacketIdentifiers.insert(clonedPacketIdentifier); + // There needs to have retransmittable frame for the rebuilder to work + conn.outstandings.packets.back().packet.frames.push_back( + MaxDataFrame(conn.flowControlState.advertisedMaxOffset)); + } - ShortHeader header( - ProtectionType::KeyPhaseOne, + // Add a third outstanding packet, which is a clone of the first packet + { + addInitialOutstandingPacket(conn); + conn.outstandings.packets.back().packet.frames.push_back( + WriteCryptoFrame(0, 1)); + ClonedPacketIdentifier clonedPacketIdentifier( + PacketNumberSpace::Initial, firstPacketNum); + conn.outstandings.packets.back().maybeClonedPacketIdentifier = + clonedPacketIdentifier; + // It is not processed yet + conn.outstandings.clonedPacketIdentifiers.insert(clonedPacketIdentifier); + // There needs to have retransmittable frame for the rebuilder to work + conn.outstandings.packets.back().packet.frames.push_back( + MaxDataFrame(conn.flowControlState.advertisedMaxOffset)); + } + + std::vector zeroConnIdData(quic::kDefaultConnectionIdSize, 0); + ConnectionId srcConnId(zeroConnIdData); + LongHeader header( + LongHeader::Types::Initial, + srcConnId, conn.clientConnectionId.value_or(getTestConnectionId()), - getNextPacketNum(conn, PacketNumberSpace::AppData)); + getNextPacketNum(conn, PacketNumberSpace::Initial), + QuicVersion::MVFST); RegularQuicPacketBuilder builder( conn.udpSendPacketLen, std::move(header), @@ -692,22 +723,26 @@ TEST_F(QuicPacketSchedulerTest, DoNotSkipUnclonedCryptoPacket) { CloningScheduler cloningScheduler(noopScheduler, conn, "cryptoClone", 0); // First packet has a crypto frame - PacketNum firstPacketNum = addOutstandingPacket(conn); + PacketNum firstPacketNum = addInitialOutstandingPacket(conn); conn.outstandings.packets.back().packet.frames.push_back( WriteCryptoFrame(0, 1)); // There needs to have retransmittable frame for the rebuilder to work conn.outstandings.packets.back().packet.frames.push_back( MaxDataFrame(conn.flowControlState.advertisedMaxOffset)); - addOutstandingPacket(conn); + addInitialOutstandingPacket(conn); // There needs to have retransmittable frame for the rebuilder to work conn.outstandings.packets.back().packet.frames.push_back( MaxDataFrame(conn.flowControlState.advertisedMaxOffset)); - ShortHeader header( - ProtectionType::KeyPhaseOne, + std::vector zeroConnIdData(quic::kDefaultConnectionIdSize, 0); + ConnectionId srcConnId(zeroConnIdData); + LongHeader header( + LongHeader::Types::Initial, + srcConnId, conn.clientConnectionId.value_or(getTestConnectionId()), - getNextPacketNum(conn, PacketNumberSpace::AppData)); + getNextPacketNum(conn, PacketNumberSpace::Initial), + QuicVersion::MVFST); RegularQuicPacketBuilder builder( conn.udpSendPacketLen, std::move(header),