diff --git a/quic/api/QuicTransportFunctions.cpp b/quic/api/QuicTransportFunctions.cpp index f65cebc87..0d292b0ee 100644 --- a/quic/api/QuicTransportFunctions.cpp +++ b/quic/api/QuicTransportFunctions.cpp @@ -203,122 +203,141 @@ void updateConnection( conn.qLogger->addPacket(packet, encodedSize); } for (const auto& frame : packet.frames) { - folly::variant_match( - frame, - [&](const WriteStreamFrame& writeStreamFrame) { - retransmittable = true; - auto stream = CHECK_NOTNULL( - conn.streamManager->getStream(writeStreamFrame.streamId)); - auto newStreamDataWritten = handleStreamWritten( - conn, - *stream, - writeStreamFrame.offset, - writeStreamFrame.len, - writeStreamFrame.fin, - packetNum, - packetNumberSpace); - if (newStreamDataWritten) { - updateFlowControlOnWriteToSocket(*stream, writeStreamFrame.len); - maybeWriteBlockAfterSocketWrite(*stream); - conn.streamManager->updateWritableStreams(*stream); - } - conn.streamManager->updateLossStreams(*stream); - }, - [&](const WriteCryptoFrame& writeCryptoFrame) { - retransmittable = true; - auto protectionType = packet.header.getProtectionType(); - // NewSessionTicket is sent in crypto frame encrypted with 1-rtt key, - // however, it is not part of handshake - isHandshake = - (protectionType == ProtectionType::Initial || - protectionType == ProtectionType::Handshake); - auto encryptionLevel = - protectionTypeToEncryptionLevel(protectionType); - handleStreamWritten( - conn, - *getCryptoStream(*conn.cryptoState, encryptionLevel), - writeCryptoFrame.offset, - writeCryptoFrame.len, - false, - packetNum, - packetNumberSpace); - }, - [&](const WriteAckFrame& writeAckFrame) { - DCHECK(!ackFrameCounter++) - << "Send more than one WriteAckFrame " << conn; - auto largestAckedPacketWritten = writeAckFrame.ackBlocks.back().end; - VLOG(10) << nodeToString(conn.nodeType) - << " sent packet with largestAcked=" - << largestAckedPacketWritten << " packetNum=" << packetNum - << " " << conn; - updateAckSendStateOnSentPacketWithAcks( - conn, - getAckState(conn, packetNumberSpace), - largestAckedPacketWritten); - }, - [&](const RstStreamFrame& rstStreamFrame) { - retransmittable = true; - VLOG(10) << nodeToString(conn.nodeType) - << " sent reset streams in packetNum=" << packetNum << " " - << conn; - auto resetIter = - conn.pendingEvents.resets.find(rstStreamFrame.streamId); - // TODO: this can happen because we clone RST_STREAM frames. Should we - // start to treat RST_STREAM in the same way we treat window update? - if (resetIter != conn.pendingEvents.resets.end()) { - conn.pendingEvents.resets.erase(resetIter); - } else { - DCHECK(packetEvent.hasValue()) - << " reset missing from pendingEvents for non-clone packet"; - } - }, - [&](const MaxDataFrame& maxDataFrame) { - CHECK(!connWindowUpdateSent++) - << "Send more than one connection window update " << conn; - VLOG(10) << nodeToString(conn.nodeType) - << " sent conn window update packetNum=" << packetNum << " " - << conn; - retransmittable = true; - VLOG(10) << nodeToString(conn.nodeType) - << " sent conn window update in packetNum=" << packetNum - << " " << conn; - onConnWindowUpdateSent( - conn, packetNum, maxDataFrame.maximumData, sentTime); - }, - [&](const MaxStreamDataFrame& maxStreamDataFrame) { - auto stream = CHECK_NOTNULL( - conn.streamManager->getStream(maxStreamDataFrame.streamId)); - retransmittable = true; - VLOG(10) << nodeToString(conn.nodeType) - << " sent packet with window update packetNum=" << packetNum - << " stream=" << maxStreamDataFrame.streamId << " " << conn; - onStreamWindowUpdateSent( - *stream, packetNum, maxStreamDataFrame.maximumData, sentTime); - }, - [&](const StreamDataBlockedFrame& streamBlockedFrame) { - VLOG(10) << nodeToString(conn.nodeType) - << " sent blocked stream frame packetNum=" << packetNum - << " " << conn; - retransmittable = true; - conn.streamManager->removeBlocked(streamBlockedFrame.streamId); - }, - [&](const PaddingFrame&) { - // do not mark padding as retransmittable. There are several reasons - // for this: - // 1. We might need to pad ACK packets to make it so that we can - // sample them correctly for header encryption. ACK packets may not - // count towards congestion window, so the padding frames in those - // ack packets should not count towards the window either - // 2. Of course we do not want to retransmit the ACK frames. - }, - [&](const QuicSimpleFrame& simpleFrame) { - retransmittable = true; - // We don't want this triggered for cloned frames. - if (!packetEvent.hasValue()) { - updateSimpleFrameOnPacketSent(conn, simpleFrame); - } - }, - [&](const auto&) { retransmittable = true; }); + switch (frame.type()) { + case QuicWriteFrame::Type::WriteStreamFrame_E: { + const WriteStreamFrame& writeStreamFrame = *frame.asWriteStreamFrame(); + retransmittable = true; + auto stream = CHECK_NOTNULL( + conn.streamManager->getStream(writeStreamFrame.streamId)); + auto newStreamDataWritten = handleStreamWritten( + conn, + *stream, + writeStreamFrame.offset, + writeStreamFrame.len, + writeStreamFrame.fin, + packetNum, + packetNumberSpace); + if (newStreamDataWritten) { + updateFlowControlOnWriteToSocket(*stream, writeStreamFrame.len); + maybeWriteBlockAfterSocketWrite(*stream); + conn.streamManager->updateWritableStreams(*stream); + } + conn.streamManager->updateLossStreams(*stream); + break; + } + case QuicWriteFrame::Type::WriteCryptoFrame_E: { + const WriteCryptoFrame& writeCryptoFrame = *frame.asWriteCryptoFrame(); + retransmittable = true; + auto protectionType = packet.header.getProtectionType(); + // NewSessionTicket is sent in crypto frame encrypted with 1-rtt key, + // however, it is not part of handshake + isHandshake = + (protectionType == ProtectionType::Initial || + protectionType == ProtectionType::Handshake); + auto encryptionLevel = protectionTypeToEncryptionLevel(protectionType); + handleStreamWritten( + conn, + *getCryptoStream(*conn.cryptoState, encryptionLevel), + writeCryptoFrame.offset, + writeCryptoFrame.len, + false, + packetNum, + packetNumberSpace); + break; + } + case QuicWriteFrame::Type::WriteAckFrame_E: { + const WriteAckFrame& writeAckFrame = *frame.asWriteAckFrame(); + DCHECK(!ackFrameCounter++) + << "Send more than one WriteAckFrame " << conn; + auto largestAckedPacketWritten = writeAckFrame.ackBlocks.back().end; + VLOG(10) << nodeToString(conn.nodeType) + << " sent packet with largestAcked=" + << largestAckedPacketWritten << " packetNum=" << packetNum + << " " << conn; + updateAckSendStateOnSentPacketWithAcks( + conn, + getAckState(conn, packetNumberSpace), + largestAckedPacketWritten); + break; + } + case QuicWriteFrame::Type::RstStreamFrame_E: { + const RstStreamFrame& rstStreamFrame = *frame.asRstStreamFrame(); + retransmittable = true; + VLOG(10) << nodeToString(conn.nodeType) + << " sent reset streams in packetNum=" << packetNum << " " + << conn; + auto resetIter = + conn.pendingEvents.resets.find(rstStreamFrame.streamId); + // TODO: this can happen because we clone RST_STREAM frames. Should we + // start to treat RST_STREAM in the same way we treat window update? + if (resetIter != conn.pendingEvents.resets.end()) { + conn.pendingEvents.resets.erase(resetIter); + } else { + DCHECK(packetEvent.hasValue()) + << " reset missing from pendingEvents for non-clone packet"; + } + break; + } + case QuicWriteFrame::Type::MaxDataFrame_E: { + const MaxDataFrame& maxDataFrame = *frame.asMaxDataFrame(); + CHECK(!connWindowUpdateSent++) + << "Send more than one connection window update " << conn; + VLOG(10) << nodeToString(conn.nodeType) + << " sent conn window update packetNum=" << packetNum << " " + << conn; + retransmittable = true; + VLOG(10) << nodeToString(conn.nodeType) + << " sent conn window update in packetNum=" << packetNum << " " + << conn; + onConnWindowUpdateSent( + conn, packetNum, maxDataFrame.maximumData, sentTime); + break; + } + case QuicWriteFrame::Type::MaxStreamDataFrame_E: { + const MaxStreamDataFrame& maxStreamDataFrame = + *frame.asMaxStreamDataFrame(); + auto stream = CHECK_NOTNULL( + conn.streamManager->getStream(maxStreamDataFrame.streamId)); + retransmittable = true; + VLOG(10) << nodeToString(conn.nodeType) + << " sent packet with window update packetNum=" << packetNum + << " stream=" << maxStreamDataFrame.streamId << " " << conn; + onStreamWindowUpdateSent( + *stream, packetNum, maxStreamDataFrame.maximumData, sentTime); + break; + } + case QuicWriteFrame::Type::StreamDataBlockedFrame_E: { + const StreamDataBlockedFrame& streamBlockedFrame = + *frame.asStreamDataBlockedFrame(); + VLOG(10) << nodeToString(conn.nodeType) + << " sent blocked stream frame packetNum=" << packetNum << " " + << conn; + retransmittable = true; + conn.streamManager->removeBlocked(streamBlockedFrame.streamId); + break; + } + case QuicWriteFrame::Type::QuicSimpleFrame_E: { + const QuicSimpleFrame& simpleFrame = *frame.asQuicSimpleFrame(); + retransmittable = true; + // We don't want this triggered for cloned frames. + if (!packetEvent.hasValue()) { + updateSimpleFrameOnPacketSent(conn, simpleFrame); + } + break; + } + case QuicWriteFrame::Type::PaddingFrame_E: { + // do not mark padding as retransmittable. There are several reasons + // for this: + // 1. We might need to pad ACK packets to make it so that we can + // sample them correctly for header encryption. ACK packets may not + // count towards congestion window, so the padding frames in those + // ack packets should not count towards the window either + // 2. Of course we do not want to retransmit the ACK frames. + break; + } + default: + retransmittable = true; + } } // TODO: Now pureAck is equivalent to non retransmittable packet. This might diff --git a/quic/api/test/QuicPacketSchedulerTest.cpp b/quic/api/test/QuicPacketSchedulerTest.cpp index a04e02691..bb331d669 100644 --- a/quic/api/test/QuicPacketSchedulerTest.cpp +++ b/quic/api/test/QuicPacketSchedulerTest.cpp @@ -366,7 +366,7 @@ TEST_F(QuicPacketSchedulerTest, WriteOnlyOutstandingPacketsTest) { // Write those framses with a regular builder writeFrame(connCloseFrame, regularBuilder); - writeFrame(maxStreamFrame, regularBuilder); + writeFrame(QuicSimpleFrame(maxStreamFrame), regularBuilder); writeFrame(pingFrame, regularBuilder); writeAckFrame(ackMeta, regularBuilder); @@ -386,15 +386,15 @@ TEST_F(QuicPacketSchedulerTest, WriteOnlyOutstandingPacketsTest) { // Test that the only frame that's written is maxdataframe EXPECT_GE(writtenPacket.packet.frames.size(), 1); auto& writtenFrame = writtenPacket.packet.frames.at(0); - auto maxDataFrame = boost::get(&writtenFrame); + auto maxDataFrame = writtenFrame.asMaxDataFrame(); CHECK(maxDataFrame); for (auto& frame : writtenPacket.packet.frames) { bool present = false; /* the next four frames should not be written */ - present |= boost::get(&frame) ? true : false; - present |= boost::get(&frame) ? true : false; - present |= boost::get(&frame) ? true : false; - present |= boost::get(&frame) ? true : false; + present |= frame.asConnectionCloseFrame() ? true : false; + present |= frame.asQuicSimpleFrame() ? true : false; + present |= frame.asPingFrame() ? true : false; + present |= frame.asWriteAckFrame() ? true : false; ASSERT_FALSE(present); } } @@ -535,12 +535,10 @@ TEST_F(QuicPacketSchedulerTest, CloneSchedulerUseNormalSchedulerFirst) { conn.ackStates.appDataAckState.nextPacketNum, shortHeader.getPacketSequenceNum()); EXPECT_EQ(1, result.second->packet.frames.size()); - folly::variant_match( - result.second->packet.frames.front(), - [&](const MaxDataFrame& frame) { EXPECT_EQ(2832, frame.maximumData); }, - [&](const auto&) { - ASSERT_FALSE(true); // should not happen - }); + MaxDataFrame* maxDataFrame = + result.second->packet.frames.front().asMaxDataFrame(); + ASSERT_NE(maxDataFrame, nullptr); + EXPECT_EQ(2832, maxDataFrame->maximumData); EXPECT_TRUE(folly::IOBufEqualTo{}( *folly::IOBuf::copyBuffer("if you are the dealer"), *result.second->header)); @@ -580,18 +578,25 @@ TEST_F(QuicPacketSchedulerTest, CloneWillGenerateNewWindowUpdate) { EXPECT_EQ(expectedPacketEvent, *packetResult.first); int32_t verifyConnWindowUpdate = 1, verifyStreamWindowUpdate = 1; for (const auto& frame : packetResult.second->packet.frames) { - folly::variant_match( - frame, - [&](const MaxStreamDataFrame& maxStreamDataFrame) { - EXPECT_EQ(stream->id, maxStreamDataFrame.streamId); - verifyStreamWindowUpdate--; - }, - [&](const MaxDataFrame&) { verifyConnWindowUpdate--; }, - [&](const PaddingFrame&) {}, - [&](const auto&) { - // should never happen - EXPECT_TRUE(false); - }); + switch (frame.type()) { + case QuicWriteFrame::Type::MaxStreamDataFrame_E: { + const MaxStreamDataFrame& maxStreamDataFrame = + *frame.asMaxStreamDataFrame(); + EXPECT_EQ(stream->id, maxStreamDataFrame.streamId); + verifyStreamWindowUpdate--; + break; + } + case QuicWriteFrame::Type::MaxDataFrame_E: { + verifyConnWindowUpdate--; + break; + } + case QuicWriteFrame::Type::PaddingFrame_E: { + break; + } + default: + // should never happen + EXPECT_TRUE(false); + } } EXPECT_EQ(0, verifyStreamWindowUpdate); EXPECT_EQ(0, verifyConnWindowUpdate); @@ -600,15 +605,21 @@ TEST_F(QuicPacketSchedulerTest, CloneWillGenerateNewWindowUpdate) { EXPECT_GE(packetResult.second->packet.frames.size(), 2); uint32_t streamWindowUpdateCounter = 0; uint32_t connWindowUpdateCounter = 0; - for (auto& streamFlowControl : - all_frames(packetResult.second->packet.frames)) { + for (auto& frame : packetResult.second->packet.frames) { + auto streamFlowControl = frame.asMaxStreamDataFrame(); + if (!streamFlowControl) { + continue; + } streamWindowUpdateCounter++; - EXPECT_EQ(1700, streamFlowControl.maximumData); + EXPECT_EQ(1700, streamFlowControl->maximumData); } - for (auto& connFlowControl : - all_frames(packetResult.second->packet.frames)) { + for (auto& frame : packetResult.second->packet.frames) { + auto connFlowControl = frame.asMaxDataFrame(); + if (!connFlowControl) { + continue; + } connWindowUpdateCounter++; - EXPECT_EQ(3300, connFlowControl.maximumData); + EXPECT_EQ(3300, connFlowControl->maximumData); } EXPECT_EQ(1, connWindowUpdateCounter); EXPECT_EQ(1, streamWindowUpdateCounter); diff --git a/quic/api/test/QuicTransportFunctionsTest.cpp b/quic/api/test/QuicTransportFunctionsTest.cpp index 28597bc96..dff3af0a4 100644 --- a/quic/api/test/QuicTransportFunctionsTest.cpp +++ b/quic/api/test/QuicTransportFunctionsTest.cpp @@ -1223,14 +1223,13 @@ TEST_F(QuicTransportFunctionsTest, NothingWritten) { 0); } -template -const FrameType& getFirstFrameInOutstandingPackets( - const std::deque& outstandingPackets) { +const QuicWriteFrame& getFirstFrameInOutstandingPackets( + const std::deque& outstandingPackets, + QuicWriteFrame::Type frameType) { for (const auto& packet : outstandingPackets) { for (const auto& frame : packet.packet.frames) { - auto decodedFrame = boost::get(&frame); - if (decodedFrame) { - return *decodedFrame; + if (frame.type() == frameType) { + return frame; } } } @@ -1272,8 +1271,10 @@ TEST_F(QuicTransportFunctionsTest, WriteBlockedFrameWhenBlocked) { EXPECT_LT(sentBytes, 200); EXPECT_GT(conn->ackStates.appDataAckState.nextPacketNum, originalNextSeq); - auto blocked = getFirstFrameInOutstandingPackets( - conn->outstandingPackets); + auto blocked = *getFirstFrameInOutstandingPackets( + conn->outstandingPackets, + QuicWriteFrame::Type::StreamDataBlockedFrame_E) + .asStreamDataBlockedFrame(); EXPECT_EQ(blocked.streamId, stream1->id); // Since everything is blocked, we shouldn't write a blocked again, so we @@ -1758,7 +1759,7 @@ TEST_F(QuicTransportFunctionsTest, UpdateConnectionCloneCounter) { auto connWindowUpdate = MaxDataFrame(conn->flowControlState.advertisedMaxOffset); conn->pendingEvents.connWindowUpdate = true; - packet.packet.frames.push_back(connWindowUpdate); + packet.packet.frames.emplace_back(connWindowUpdate); PacketEvent packetEvent = 100; conn->outstandingPacketEvents.insert(packetEvent); updateConnection(*conn, packetEvent, packet.packet, TimePoint(), 123); @@ -1788,7 +1789,7 @@ TEST_F(QuicTransportFunctionsTest, ClonedBlocked) { auto packet = buildEmptyPacket(*conn, PacketNumberSpace::AppData); auto stream = conn->streamManager->createNextBidirectionalStream().value(); StreamDataBlockedFrame blockedFrame(stream->id, 1000); - packet.packet.frames.push_back(blockedFrame); + packet.packet.frames.emplace_back(blockedFrame); conn->outstandingPacketEvents.insert(packetEvent); // This shall not crash updateConnection( @@ -1805,8 +1806,8 @@ TEST_F(QuicTransportFunctionsTest, TwoConnWindowUpdateWillCrash) { auto packet = buildEmptyPacket(*conn, PacketNumberSpace::Handshake); MaxDataFrame connWindowUpdate( 1000 + conn->flowControlState.advertisedMaxOffset); - packet.packet.frames.push_back(connWindowUpdate); - packet.packet.frames.push_back(connWindowUpdate); + packet.packet.frames.emplace_back(connWindowUpdate); + packet.packet.frames.emplace_back(connWindowUpdate); conn->pendingEvents.connWindowUpdate = true; EXPECT_DEATH( updateConnection( @@ -1859,7 +1860,7 @@ TEST_F(QuicTransportFunctionsTest, ClonedRst) { auto packet = buildEmptyPacket(*conn, PacketNumberSpace::AppData); RstStreamFrame rstStreamFrame( stream->id, GenericApplicationErrorCode::UNKNOWN, 0); - packet.packet.frames.push_back(rstStreamFrame); + packet.packet.frames.emplace_back(std::move(rstStreamFrame)); conn->outstandingPacketEvents.insert(packetEvent); // This shall not crash updateConnection( diff --git a/quic/api/test/QuicTransportTest.cpp b/quic/api/test/QuicTransportTest.cpp index 1eb8bdd2e..2c79e5de7 100644 --- a/quic/api/test/QuicTransportTest.cpp +++ b/quic/api/test/QuicTransportTest.cpp @@ -235,8 +235,7 @@ size_t bufLength( void dropPackets(QuicServerConnectionState& conn) { for (const auto& packet : conn.outstandingPackets) { for (const auto& frame : packet.packet.frames) { - const WriteStreamFrame* streamFrame = - boost::get(&frame); + const WriteStreamFrame* streamFrame = frame.asWriteStreamFrame(); if (!streamFrame) { continue; } @@ -285,15 +284,18 @@ void verifyCorrectness( bool finSet = false; std::vector offsets; for (const auto& packet : conn.outstandingPackets) { - for (const auto& streamFrame : - all_frames(packet.packet.frames)) { - if (streamFrame.streamId != id) { + for (const auto& frame : packet.packet.frames) { + auto streamFrame = frame.asWriteStreamFrame(); + if (!streamFrame) { continue; } - offsets.push_back(streamFrame.offset); - endOffset = std::max(endOffset, streamFrame.offset + streamFrame.len); - totalLen += streamFrame.len; - finSet |= streamFrame.fin; + if (streamFrame->streamId != id) { + continue; + } + offsets.push_back(streamFrame->offset); + endOffset = std::max(endOffset, streamFrame->offset + streamFrame->len); + totalLen += streamFrame->len; + finSet |= streamFrame->fin; } } auto stream = conn.streamManager->findStream(id); @@ -592,8 +594,12 @@ TEST_F(QuicTransportTest, WriteFlowControl) { auto& packet = getFirstOutstandingPacket(conn, PacketNumberSpace::AppData)->packet; bool blockedFound = false; - for (auto& blocked : all_frames(packet.frames)) { - EXPECT_EQ(blocked.streamId, streamId); + for (auto& frame : packet.frames) { + auto blocked = frame.asStreamDataBlockedFrame(); + if (!blocked) { + continue; + } + EXPECT_EQ(blocked->streamId, streamId); blockedFound = true; } EXPECT_TRUE(blockedFound); @@ -782,10 +788,14 @@ TEST_F(QuicTransportTest, WriteImmediateAcks) { EXPECT_GE(packet.frames.size(), 1); bool ackFound = false; - for (auto& ackFrame : all_frames(packet.frames)) { - EXPECT_EQ(ackFrame.ackBlocks.size(), 1); - EXPECT_EQ(start, ackFrame.ackBlocks.front().start); - EXPECT_EQ(end, ackFrame.ackBlocks.front().end); + for (auto& frame : packet.frames) { + auto ackFrame = frame.asWriteAckFrame(); + if (!ackFrame) { + continue; + } + EXPECT_EQ(ackFrame->ackBlocks.size(), 1); + EXPECT_EQ(start, ackFrame->ackBlocks.front().start); + EXPECT_EQ(end, ackFrame->ackBlocks.front().end); ackFound = true; } EXPECT_TRUE(ackFound); @@ -835,10 +845,14 @@ TEST_F(QuicTransportTest, WritePendingAckIfHavingData) { EXPECT_GE(packet.frames.size(), 2); bool ackFound = false; - for (auto& ackFrame : all_frames(packet.frames)) { - EXPECT_EQ(ackFrame.ackBlocks.size(), 1); - EXPECT_EQ(ackFrame.ackBlocks.front().start, start); - EXPECT_EQ(ackFrame.ackBlocks.front().end, end); + for (auto& frame : packet.frames) { + auto ackFrame = frame.asWriteAckFrame(); + if (!ackFrame) { + continue; + } + EXPECT_EQ(ackFrame->ackBlocks.size(), 1); + EXPECT_EQ(ackFrame->ackBlocks.front().start, start); + EXPECT_EQ(ackFrame->ackBlocks.front().end, end); ackFound = true; } EXPECT_TRUE(ackFound); @@ -864,10 +878,14 @@ TEST_F(QuicTransportTest, RstStream) { ->packet; EXPECT_GE(packet.frames.size(), 1); bool rstFound = false; - for (auto& frame : all_frames(packet.frames)) { - EXPECT_EQ(streamId, frame.streamId); - EXPECT_EQ(0, frame.offset); - EXPECT_EQ(GenericApplicationErrorCode::UNKNOWN, frame.errorCode); + for (auto& frame : packet.frames) { + auto rstFrame = frame.asRstStreamFrame(); + if (!rstFrame) { + continue; + } + EXPECT_EQ(streamId, rstFrame->streamId); + EXPECT_EQ(0, rstFrame->offset); + EXPECT_EQ(GenericApplicationErrorCode::UNKNOWN, rstFrame->errorCode); rstFound = true; } EXPECT_TRUE(rstFound); @@ -896,15 +914,20 @@ TEST_F(QuicTransportTest, StopSending) { ->packet; EXPECT_EQ(14, packet.frames.size()); bool foundStopSending = false; - for (auto& simpleFrame : all_frames(packet.frames)) { + for (auto& frame : packet.frames) { + const QuicSimpleFrame* simpleFrame = frame.asQuicSimpleFrame(); + if (!simpleFrame) { + continue; + } folly::variant_match( - simpleFrame, + *simpleFrame, [&](const StopSendingFrame& frame) { EXPECT_EQ(streamId, frame.streamId); EXPECT_EQ(GenericApplicationErrorCode::UNKNOWN, frame.errorCode); foundStopSending = true; }, - [&](auto&) {}); + [&](auto&) { + }); } EXPECT_TRUE(foundStopSending); } @@ -932,9 +955,13 @@ TEST_F(QuicTransportTest, SendPathChallenge) { transport_->getConnectionState(), PacketNumberSpace::AppData) ->packet; bool foundPathChallenge = false; - for (auto& simpleFrame : all_frames(packet.frames)) { + for (auto& frame : packet.frames) { + const QuicSimpleFrame* simpleFrame = frame.asQuicSimpleFrame(); + if (!simpleFrame) { + continue; + } folly::variant_match( - simpleFrame, + *simpleFrame, [&](const PathChallengeFrame& frame) { EXPECT_EQ(frame, pathChallenge); foundPathChallenge = true; @@ -1133,9 +1160,13 @@ TEST_F(QuicTransportTest, SendPathResponse) { auto packet = getLastOutstandingPacket(conn, PacketNumberSpace::AppData)->packet; bool foundPathResponse = false; - for (auto& simpleFrame : all_frames(packet.frames)) { + for (auto& frame : packet.frames) { + const QuicSimpleFrame* simpleFrame = frame.asQuicSimpleFrame(); + if (!simpleFrame) { + continue; + } folly::variant_match( - simpleFrame, + *simpleFrame, [&](const PathResponseFrame& frame) { EXPECT_EQ(frame, pathResponse); foundPathResponse = true; @@ -1244,9 +1275,13 @@ TEST_F(QuicTransportTest, SendNewConnectionIdFrame) { transport_->getConnectionState(), PacketNumberSpace::AppData) ->packet; bool foundNewConnectionId = false; - for (auto& simpleFrame : all_frames(packet.frames)) { + for (auto& frame : packet.frames) { + const QuicSimpleFrame* simpleFrame = frame.asQuicSimpleFrame(); + if (!simpleFrame) { + continue; + } folly::variant_match( - simpleFrame, + *simpleFrame, [&](const NewConnectionIdFrame& frame) { EXPECT_EQ(frame, newConnId); foundNewConnectionId = true; @@ -1379,9 +1414,13 @@ TEST_F(QuicTransportTest, SendRetireConnectionIdFrame) { transport_->getConnectionState(), PacketNumberSpace::AppData) ->packet; bool foundRetireConnectionId = false; - for (auto& simpleFrame : all_frames(packet.frames)) { + for (auto& frame : packet.frames) { + const QuicSimpleFrame* simpleFrame = frame.asQuicSimpleFrame(); + if (!simpleFrame) { + continue; + } folly::variant_match( - simpleFrame, + *simpleFrame, [&](const RetireConnectionIdFrame& frame) { EXPECT_EQ(frame, retireConnId); foundRetireConnectionId = true; @@ -1500,10 +1539,14 @@ TEST_F(QuicTransportTest, RstWrittenStream) { EXPECT_GE(packet.frames.size(), 1); bool foundReset = false; - for (auto& frame : all_frames(packet.frames)) { - EXPECT_EQ(streamId, frame.streamId); - EXPECT_EQ(currentWriteOffset, frame.offset); - EXPECT_EQ(GenericApplicationErrorCode::UNKNOWN, frame.errorCode); + for (auto& frame : packet.frames) { + auto rstStream = frame.asRstStreamFrame(); + if (!rstStream) { + continue; + } + EXPECT_EQ(streamId, rstStream->streamId); + EXPECT_EQ(currentWriteOffset, rstStream->offset); + EXPECT_EQ(GenericApplicationErrorCode::UNKNOWN, rstStream->errorCode); foundReset = true; } EXPECT_TRUE(foundReset); @@ -1530,9 +1573,13 @@ TEST_F(QuicTransportTest, RstStreamUDPWriteFailNonFatal) { EXPECT_GE(packet.frames.size(), 1); bool foundReset = false; - for (auto& frame : all_frames(packet.frames)) { - EXPECT_EQ(streamId, frame.streamId); - EXPECT_EQ(GenericApplicationErrorCode::UNKNOWN, frame.errorCode); + for (auto& frame : packet.frames) { + auto rstStream = frame.asRstStreamFrame(); + if (!rstStream) { + continue; + } + EXPECT_EQ(streamId, rstStream->streamId); + EXPECT_EQ(GenericApplicationErrorCode::UNKNOWN, rstStream->errorCode); foundReset = true; } EXPECT_TRUE(foundReset); @@ -1596,10 +1643,14 @@ TEST_F(QuicTransportTest, WriteAfterSendRst) { EXPECT_GE(packet.frames.size(), 1); bool foundReset = false; - for (auto& frame : all_frames(packet.frames)) { - EXPECT_EQ(streamId, frame.streamId); - EXPECT_EQ(currentWriteOffset, frame.offset); - EXPECT_EQ(GenericApplicationErrorCode::UNKNOWN, frame.errorCode); + for (auto& frame : packet.frames) { + auto rstFrame = frame.asRstStreamFrame(); + if (!rstFrame) { + continue; + } + EXPECT_EQ(streamId, rstFrame->streamId); + EXPECT_EQ(currentWriteOffset, rstFrame->offset); + EXPECT_EQ(GenericApplicationErrorCode::UNKNOWN, rstFrame->errorCode); foundReset = true; } EXPECT_TRUE(foundReset); @@ -1671,8 +1722,12 @@ TEST_F(QuicTransportTest, WriteWindowUpdate) { getLastOutstandingPacket(conn, PacketNumberSpace::AppData)->packet; EXPECT_GE(packet.frames.size(), 1); bool connWindowFound = false; - for (auto& connWindowUpdate : all_frames(packet.frames)) { - EXPECT_EQ(100, connWindowUpdate.maximumData); + for (auto& frame : packet.frames) { + auto connWindowUpdate = frame.asMaxDataFrame(); + if (!connWindowUpdate) { + continue; + } + EXPECT_EQ(100, connWindowUpdate->maximumData); connWindowFound = true; } @@ -1702,7 +1757,7 @@ TEST_F(QuicTransportTest, WriteWindowUpdate) { auto packet1 = getLastOutstandingPacket(conn, PacketNumberSpace::AppData)->packet; const MaxStreamDataFrame* streamWindowUpdate = - boost::get(&packet1.frames.front()); + packet1.frames.front().asMaxStreamDataFrame(); EXPECT_TRUE(streamWindowUpdate); } @@ -2278,7 +2333,7 @@ TEST_F(QuicTransportTest, WriteStreamFromMiddleOfMap) { auto& packet = *getFirstOutstandingPacket(conn, PacketNumberSpace::AppData); EXPECT_EQ(1, packet.packet.frames.size()); auto& frame = packet.packet.frames.front(); - const WriteStreamFrame* streamFrame = boost::get(&frame); + const WriteStreamFrame* streamFrame = frame.asWriteStreamFrame(); EXPECT_TRUE(streamFrame); EXPECT_EQ(streamFrame->streamId, s1); conn.outstandingPackets.clear(); @@ -2301,7 +2356,7 @@ TEST_F(QuicTransportTest, WriteStreamFromMiddleOfMap) { auto& packet2 = *getFirstOutstandingPacket(conn, PacketNumberSpace::AppData); EXPECT_EQ(1, packet2.packet.frames.size()); auto& frame2 = packet2.packet.frames.front(); - const WriteStreamFrame* streamFrame2 = boost::get(&frame2); + const WriteStreamFrame* streamFrame2 = frame2.asWriteStreamFrame(); EXPECT_TRUE(streamFrame2); EXPECT_EQ(streamFrame2->streamId, s2); conn.outstandingPackets.clear(); @@ -2324,10 +2379,10 @@ TEST_F(QuicTransportTest, WriteStreamFromMiddleOfMap) { EXPECT_EQ(2, packet3.packet.frames.size()); auto& frame3 = packet3.packet.frames.front(); auto& frame4 = packet3.packet.frames.back(); - const WriteStreamFrame* streamFrame3 = boost::get(&frame3); + const WriteStreamFrame* streamFrame3 = frame3.asWriteStreamFrame(); EXPECT_TRUE(streamFrame3); EXPECT_EQ(streamFrame3->streamId, s2); - const WriteStreamFrame* streamFrame4 = boost::get(&frame4); + const WriteStreamFrame* streamFrame4 = frame4.asWriteStreamFrame(); EXPECT_TRUE(streamFrame4); EXPECT_EQ(streamFrame4->streamId, s1); transport_->close(folly::none); diff --git a/quic/client/QuicClientTransport.cpp b/quic/client/QuicClientTransport.cpp index e335b0d9c..41c91fe91 100644 --- a/quic/client/QuicClientTransport.cpp +++ b/quic/client/QuicClientTransport.cpp @@ -295,51 +295,61 @@ void QuicClientTransport::processPacketData( // TODO: replace this with a better solution later. cancelHandshakeCryptoStreamRetransmissions(*conn_->cryptoState); } - folly::variant_match( - packetFrame, - [&](const WriteAckFrame& frame) { - DCHECK(!frame.ackBlocks.empty()); - VLOG(4) << "Client received ack for largestAcked=" - << frame.ackBlocks.back().end << " " << *this; - commonAckVisitorForAckFrame(ackState, frame); - }, - [&](const RstStreamFrame& frame) { - VLOG(4) << "Client received ack for reset frame stream=" - << frame.streamId << " " << *this; + switch (packetFrame.type()) { + case QuicWriteFrame::Type::WriteAckFrame_E: { + const WriteAckFrame& frame = *packetFrame.asWriteAckFrame(); + DCHECK(!frame.ackBlocks.empty()); + VLOG(4) << "Client received ack for largestAcked=" + << frame.ackBlocks.back().end << " " << *this; + commonAckVisitorForAckFrame(ackState, frame); + break; + } + case QuicWriteFrame::Type::RstStreamFrame_E: { + const RstStreamFrame& frame = *packetFrame.asRstStreamFrame(); + VLOG(4) << "Client received ack for reset frame stream=" + << frame.streamId << " " << *this; - auto stream = - conn_->streamManager->getStream(frame.streamId); - if (stream) { - invokeStreamSendStateMachine( - *conn_, *stream, StreamEvents::RstAck(frame)); - } - }, - [&](const WriteStreamFrame& frame) { - auto ackedStream = - conn_->streamManager->getStream(frame.streamId); - VLOG(4) << "Client got ack for stream=" << frame.streamId - << " offset=" << frame.offset - << " fin=" << frame.fin << " data=" << frame.len - << " closed=" << (ackedStream == nullptr) << " " - << *this; - if (ackedStream) { - invokeStreamSendStateMachine( - *conn_, - *ackedStream, - StreamEvents::AckStreamFrame(frame)); - } - }, - [&](const WriteCryptoFrame& frame) { - auto cryptoStream = getCryptoStream( - *conn_->cryptoState, - protectionTypeToEncryptionLevel( - outstandingProtectionType)); - processCryptoStreamAck( - *cryptoStream, frame.offset, frame.len); - }, - [&](const auto& /* frame */) { - // Ignore other frames. - }); + auto stream = conn_->streamManager->getStream(frame.streamId); + if (stream) { + invokeStreamSendStateMachine( + *conn_, *stream, StreamEvents::RstAck(frame)); + } + break; + } + case QuicWriteFrame::Type::WriteStreamFrame_E: { + const WriteStreamFrame& frame = + *packetFrame.asWriteStreamFrame(); + + auto ackedStream = + conn_->streamManager->getStream(frame.streamId); + VLOG(4) << "Client got ack for stream=" << frame.streamId + << " offset=" << frame.offset << " fin=" << frame.fin + << " data=" << frame.len + << " closed=" << (ackedStream == nullptr) << " " + << *this; + if (ackedStream) { + invokeStreamSendStateMachine( + *conn_, + *ackedStream, + StreamEvents::AckStreamFrame(frame)); + } + break; + } + case QuicWriteFrame::Type::WriteCryptoFrame_E: { + const WriteCryptoFrame& frame = + *packetFrame.asWriteCryptoFrame(); + auto cryptoStream = getCryptoStream( + *conn_->cryptoState, + protectionTypeToEncryptionLevel( + outstandingProtectionType)); + processCryptoStreamAck( + *cryptoStream, frame.offset, frame.len); + break; + } + default: + // ignore other frames. + break; + } }, markPacketLoss, receiveTimePoint); diff --git a/quic/client/QuicClientTransport.h b/quic/client/QuicClientTransport.h index 064d4c9d2..a75b7bee1 100644 --- a/quic/client/QuicClientTransport.h +++ b/quic/client/QuicClientTransport.h @@ -167,6 +167,11 @@ class QuicClientTransport void happyEyeballsConnAttemptDelayTimeoutExpired() noexcept; + void handleAckFrame( + const OutstandingPacket& outstandingPacket, + const QuicWriteFrame& packetFrame, + const ReadAckFrame&); + // From ClientHandshake::HandshakeCallback void onNewCachedPsk( fizz::client::NewCachedPsk& newCachedPsk) noexcept override; diff --git a/quic/client/test/QuicClientTransportTest.cpp b/quic/client/test/QuicClientTransportTest.cpp index 1aaa94f3a..6100b00e3 100644 --- a/quic/client/test/QuicClientTransportTest.cpp +++ b/quic/client/test/QuicClientTransportTest.cpp @@ -3934,12 +3934,11 @@ RegularQuicWritePacket* findPacketWithStream( StreamId streamId) { auto op = findOutstandingPacket(conn, [=](OutstandingPacket& packet) { for (auto& frame : packet.packet.frames) { - bool tryPacket = folly::variant_match( - frame, - [streamId](WriteStreamFrame& streamFrame) { - return streamFrame.streamId == streamId; - }, - [](auto&) { return false; }); + bool tryPacket = false; + WriteStreamFrame* streamFrame = frame.asWriteStreamFrame(); + if (streamFrame) { + tryPacket = streamFrame->streamId == streamId; + } if (tryPacket) { return true; } diff --git a/quic/codec/QuicPacketRebuilder.cpp b/quic/codec/QuicPacketRebuilder.cpp index 104e8cf11..987adaeab 100644 --- a/quic/codec/QuicPacketRebuilder.cpp +++ b/quic/codec/QuicPacketRebuilder.cpp @@ -53,108 +53,128 @@ folly::Optional PacketRebuilder::rebuildFromPacket( iter != packet.packet.frames.cend(); iter++) { const QuicWriteFrame& frame = *iter; - writeSuccess = folly::variant_match( - frame, - [&](const WriteAckFrame& ackFrame) { - auto& packetHeader = builder_.getPacketHeader(); - uint64_t ackDelayExponent = - (packetHeader.getHeaderForm() == HeaderForm::Long) - ? kDefaultAckDelayExponent - : conn_.transportSettings.ackDelayExponent; - AckFrameMetaData meta( - ackFrame.ackBlocks, ackFrame.ackDelay, ackDelayExponent); - auto ackWriteResult = writeAckFrame(meta, builder_); - return ackWriteResult.hasValue(); - }, - [&](const WriteStreamFrame& streamFrame) { - auto stream = conn_.streamManager->getStream(streamFrame.streamId); - if (stream && retransmittable(*stream)) { - auto streamData = cloneRetransmissionBuffer(streamFrame, stream); - auto bufferLen = - streamData ? streamData->computeChainDataLength() : 0; - auto dataLen = writeStreamFrameHeader( - builder_, - streamFrame.streamId, - streamFrame.offset, - bufferLen, - bufferLen, - streamFrame.fin); - bool ret = dataLen.hasValue() && *dataLen == streamFrame.len; - if (ret) { - writeStreamFrameData(builder_, std::move(streamData), *dataLen); - notPureAck = true; - return true; - } - return false; + switch (frame.type()) { + case QuicWriteFrame::Type::WriteAckFrame_E: { + const WriteAckFrame& ackFrame = *frame.asWriteAckFrame(); + auto& packetHeader = builder_.getPacketHeader(); + uint64_t ackDelayExponent = + (packetHeader.getHeaderForm() == HeaderForm::Long) + ? kDefaultAckDelayExponent + : conn_.transportSettings.ackDelayExponent; + AckFrameMetaData meta( + ackFrame.ackBlocks, ackFrame.ackDelay, ackDelayExponent); + auto ackWriteResult = writeAckFrame(meta, builder_); + writeSuccess = ackWriteResult.hasValue(); + break; + } + case QuicWriteFrame::Type::WriteStreamFrame_E: { + const WriteStreamFrame& streamFrame = *frame.asWriteStreamFrame(); + auto stream = conn_.streamManager->getStream(streamFrame.streamId); + if (stream && retransmittable(*stream)) { + auto streamData = cloneRetransmissionBuffer(streamFrame, stream); + auto bufferLen = + streamData ? streamData->computeChainDataLength() : 0; + auto dataLen = writeStreamFrameHeader( + builder_, + streamFrame.streamId, + streamFrame.offset, + bufferLen, + bufferLen, + streamFrame.fin); + bool ret = dataLen.hasValue() && *dataLen == streamFrame.len; + if (ret) { + writeStreamFrameData(builder_, std::move(streamData), *dataLen); + notPureAck = true; + writeSuccess = true; + break; } - // If a stream is already Closed, we should not clone and resend this - // stream data. But should we abort the cloning of this packet and - // move on to the next packet? I'm gonna err on the aggressive side - // for now and call it success. - return true; - }, - [&](const WriteCryptoFrame& cryptoFrame) { - // initialStream and handshakeStream can only be in handshake packet, - // so they are not clonable - CHECK(!packet.isHandshake); - // key update not supported - DCHECK( - packet.packet.header.getProtectionType() == - ProtectionType::KeyPhaseZero); - auto& stream = conn_.cryptoState->oneRttStream; - auto buf = cloneCryptoRetransmissionBuffer(cryptoFrame, stream); + writeSuccess = false; + break; + } + // If a stream is already Closed, we should not clone and resend this + // stream data. But should we abort the cloning of this packet and + // move on to the next packet? I'm gonna err on the aggressive side + // for now and call it success. + writeSuccess = true; + break; + } + case QuicWriteFrame::Type::WriteCryptoFrame_E: { + const WriteCryptoFrame& cryptoFrame = *frame.asWriteCryptoFrame(); + // initialStream and handshakeStream can only be in handshake packet, + // so they are not clonable + CHECK(!packet.isHandshake); + // key update not supported + DCHECK( + packet.packet.header.getProtectionType() == + ProtectionType::KeyPhaseZero); + auto& stream = conn_.cryptoState->oneRttStream; + auto buf = cloneCryptoRetransmissionBuffer(cryptoFrame, stream); - // No crypto data found to be cloned, just skip - if (!buf) { - return true; - } - auto cryptoWriteResult = - writeCryptoFrame(cryptoFrame.offset, std::move(buf), builder_); - bool ret = cryptoWriteResult.hasValue() && - cryptoWriteResult->offset == cryptoFrame.offset && - cryptoWriteResult->len == cryptoFrame.len; - notPureAck |= ret; - return ret; - }, - [&](const MaxDataFrame&) { - shouldWriteWindowUpdate = true; - auto ret = 0 != writeFrame(generateMaxDataFrame(conn_), builder_); - windowUpdateWritten |= ret; - notPureAck |= ret; - return true; - }, - [&](const MaxStreamDataFrame& maxStreamDataFrame) { - auto stream = - conn_.streamManager->getStream(maxStreamDataFrame.streamId); - if (!stream || !stream->shouldSendFlowControl()) { - return true; - } - shouldWriteWindowUpdate = true; - auto ret = - 0 != writeFrame(generateMaxStreamDataFrame(*stream), builder_); - windowUpdateWritten |= ret; - notPureAck |= ret; - return true; - }, - [&](const PaddingFrame& paddingFrame) { - return writeFrame(paddingFrame, builder_) != 0; - }, - [&](const QuicSimpleFrame& simpleFrame) { - auto updatedSimpleFrame = - updateSimpleFrameOnPacketClone(conn_, simpleFrame); - if (!updatedSimpleFrame) { - return true; - } - bool ret = - writeSimpleFrame(std::move(*updatedSimpleFrame), builder_) != 0; - notPureAck |= ret; - return ret; - }, - [&](const auto& otherFrame) { - bool ret = writeFrame(otherFrame, builder_) != 0; - notPureAck |= ret; - return ret; - }); + // No crypto data found to be cloned, just skip + if (!buf) { + writeSuccess = true; + break; + } + auto cryptoWriteResult = + writeCryptoFrame(cryptoFrame.offset, std::move(buf), builder_); + bool ret = cryptoWriteResult.hasValue() && + cryptoWriteResult->offset == cryptoFrame.offset && + cryptoWriteResult->len == cryptoFrame.len; + notPureAck |= ret; + writeSuccess = ret; + break; + } + case QuicWriteFrame::Type::MaxDataFrame_E: { + shouldWriteWindowUpdate = true; + auto ret = 0 != writeFrame(generateMaxDataFrame(conn_), builder_); + windowUpdateWritten |= ret; + notPureAck |= ret; + writeSuccess = true; + break; + } + case QuicWriteFrame::Type::MaxStreamDataFrame_E: { + const MaxStreamDataFrame& maxStreamDataFrame = + *frame.asMaxStreamDataFrame(); + auto stream = + conn_.streamManager->getStream(maxStreamDataFrame.streamId); + if (!stream || !stream->shouldSendFlowControl()) { + writeSuccess = true; + break; + } + shouldWriteWindowUpdate = true; + auto ret = + 0 != writeFrame(generateMaxStreamDataFrame(*stream), builder_); + windowUpdateWritten |= ret; + notPureAck |= ret; + writeSuccess = true; + break; + } + case QuicWriteFrame::Type::PaddingFrame_E: { + const PaddingFrame& paddingFrame = *frame.asPaddingFrame(); + writeSuccess = writeFrame(paddingFrame, builder_) != 0; + break; + } + case QuicWriteFrame::Type::QuicSimpleFrame_E: { + const QuicSimpleFrame& simpleFrame = *frame.asQuicSimpleFrame(); + auto updatedSimpleFrame = + updateSimpleFrameOnPacketClone(conn_, simpleFrame); + if (!updatedSimpleFrame) { + writeSuccess = true; + break; + } + bool ret = + writeSimpleFrame(std::move(*updatedSimpleFrame), builder_) != 0; + notPureAck |= ret; + writeSuccess = ret; + break; + } + default: { + bool ret = writeFrame(QuicWriteFrame(frame), builder_) != 0; + notPureAck |= ret; + writeSuccess = ret; + break; + } + } if (!writeSuccess) { return folly::none; } diff --git a/quic/codec/QuicWriteCodec.cpp b/quic/codec/QuicWriteCodec.cpp index 91cc78ebe..c0219b35e 100644 --- a/quic/codec/QuicWriteCodec.cpp +++ b/quic/codec/QuicWriteCodec.cpp @@ -339,7 +339,7 @@ size_t writeSimpleFrame( } else { builder.write(errorCode); } - builder.appendFrame(std::move(stopSendingFrame)); + builder.appendFrame(QuicSimpleFrame(std::move(stopSendingFrame))); return stopSendingFrameSize; } // no space left in packet @@ -358,7 +358,7 @@ size_t writeSimpleFrame( builder.write(streamId); builder.write(maximumData); builder.write(minimumStreamOffset); - builder.appendFrame(std::move(minStreamDataFrame)); + builder.appendFrame(QuicSimpleFrame(std::move(minStreamDataFrame))); return minStreamDataFrameSize; } // no space left in packet @@ -376,7 +376,8 @@ size_t writeSimpleFrame( builder.write(frameType); builder.write(streamId); builder.write(minimumStreamOffset); - builder.appendFrame(std::move(expiredStreamDataFrame)); + builder.appendFrame( + QuicSimpleFrame(std::move(expiredStreamDataFrame))); return expiredStreamDataFrameSize; } // no space left in packet @@ -389,7 +390,7 @@ size_t writeSimpleFrame( if (packetSpaceCheck(spaceLeft, pathChallengeFrameSize)) { builder.write(frameType); builder.writeBE(pathChallengeFrame.pathData); - builder.appendFrame(std::move(pathChallengeFrame)); + builder.appendFrame(QuicSimpleFrame(std::move(pathChallengeFrame))); return pathChallengeFrameSize; } // no space left in packet @@ -402,7 +403,7 @@ size_t writeSimpleFrame( if (packetSpaceCheck(spaceLeft, pathResponseFrameSize)) { builder.write(frameType); builder.writeBE(pathResponseFrame.pathData); - builder.appendFrame(std::move(pathResponseFrame)); + builder.appendFrame(QuicSimpleFrame(std::move(pathResponseFrame))); return pathResponseFrameSize; } // no space left in packet @@ -429,7 +430,7 @@ size_t writeSimpleFrame( builder.push( newConnectionIdFrame.token.data(), newConnectionIdFrame.token.size()); - builder.appendFrame(std::move(newConnectionIdFrame)); + builder.appendFrame(QuicSimpleFrame(std::move(newConnectionIdFrame))); return newConnectionIdFrameSize; } // no space left in packet @@ -446,7 +447,7 @@ size_t writeSimpleFrame( if (packetSpaceCheck(spaceLeft, maxStreamsFrameSize)) { builder.write(intFrameType); builder.write(streamCount); - builder.appendFrame(maxStreamsFrame); + builder.appendFrame(QuicSimpleFrame(maxStreamsFrame)); return maxStreamsFrameSize; } return size_t(0); @@ -460,7 +461,7 @@ size_t writeSimpleFrame( if (packetSpaceCheck(spaceLeft, retireConnectionIdFrameSize)) { builder.write(frameType); builder.write(sequence); - builder.appendFrame(retireConnectionIdFrame); + builder.appendFrame(QuicSimpleFrame(retireConnectionIdFrame)); return retireConnectionIdFrameSize; } // no space left in packet @@ -473,206 +474,218 @@ size_t writeFrame(QuicWriteFrame&& frame, PacketBuilderInterface& builder) { uint64_t spaceLeft = builder.remainingSpaceInPkt(); - return folly::variant_match( - frame, - [&](PaddingFrame& paddingFrame) { - QuicInteger intFrameType(static_cast(FrameType::PADDING)); - if (packetSpaceCheck(spaceLeft, intFrameType.getSize())) { - builder.write(intFrameType); - builder.appendFrame(std::move(paddingFrame)); - return intFrameType.getSize(); + switch (frame.type()) { + case QuicWriteFrame::Type::PaddingFrame_E: { + PaddingFrame& paddingFrame = *frame.asPaddingFrame(); + QuicInteger intFrameType(static_cast(FrameType::PADDING)); + if (packetSpaceCheck(spaceLeft, intFrameType.getSize())) { + builder.write(intFrameType); + builder.appendFrame(std::move(paddingFrame)); + return intFrameType.getSize(); + } + return size_t(0); + } + case QuicWriteFrame::Type::PingFrame_E: { + PingFrame& pingFrame = *frame.asPingFrame(); + QuicInteger intFrameType(static_cast(FrameType::PING)); + if (packetSpaceCheck(spaceLeft, intFrameType.getSize())) { + builder.write(intFrameType); + builder.appendFrame(std::move(pingFrame)); + return intFrameType.getSize(); + } + // no space left in packet + return size_t(0); + } + case QuicWriteFrame::Type::RstStreamFrame_E: { + RstStreamFrame& rstStreamFrame = *frame.asRstStreamFrame(); + QuicInteger intFrameType(static_cast(FrameType::RST_STREAM)); + QuicInteger streamId(rstStreamFrame.streamId); + QuicInteger offset(rstStreamFrame.offset); + QuicInteger errorCode(static_cast(rstStreamFrame.errorCode)); + auto version = builder.getVersion(); + size_t errorSize = version == QuicVersion::MVFST_OLD + ? sizeof(ApplicationErrorCode) + : errorCode.getSize(); + auto rstStreamFrameSize = intFrameType.getSize() + errorSize + + streamId.getSize() + offset.getSize(); + if (packetSpaceCheck(spaceLeft, rstStreamFrameSize)) { + builder.write(intFrameType); + builder.write(streamId); + if (version == QuicVersion::MVFST_OLD) { + builder.writeBE( + static_cast(rstStreamFrame.errorCode)); + } else { + builder.write(errorCode); } - return size_t(0); - }, - [&](PingFrame& pingFrame) { - QuicInteger intFrameType(static_cast(FrameType::PING)); - if (packetSpaceCheck(spaceLeft, intFrameType.getSize())) { - builder.write(intFrameType); - builder.appendFrame(std::move(pingFrame)); - return intFrameType.getSize(); + builder.write(offset); + builder.appendFrame(std::move(rstStreamFrame)); + return rstStreamFrameSize; + } + // no space left in packet + return size_t(0); + } + case QuicWriteFrame::Type::MaxDataFrame_E: { + MaxDataFrame& maxDataFrame = *frame.asMaxDataFrame(); + QuicInteger intFrameType(static_cast(FrameType::MAX_DATA)); + QuicInteger maximumData(maxDataFrame.maximumData); + auto frameSize = intFrameType.getSize() + maximumData.getSize(); + if (packetSpaceCheck(spaceLeft, frameSize)) { + builder.write(intFrameType); + builder.write(maximumData); + builder.appendFrame(std::move(maxDataFrame)); + return frameSize; + } + // no space left in packet + return size_t(0); + } + case QuicWriteFrame::Type::MaxStreamDataFrame_E: { + MaxStreamDataFrame& maxStreamDataFrame = *frame.asMaxStreamDataFrame(); + QuicInteger intFrameType( + static_cast(FrameType::MAX_STREAM_DATA)); + QuicInteger streamId(maxStreamDataFrame.streamId); + QuicInteger maximumData(maxStreamDataFrame.maximumData); + auto maxStreamDataFrameSize = + intFrameType.getSize() + streamId.getSize() + maximumData.getSize(); + if (packetSpaceCheck(spaceLeft, maxStreamDataFrameSize)) { + builder.write(intFrameType); + builder.write(streamId); + builder.write(maximumData); + builder.appendFrame(std::move(maxStreamDataFrame)); + return maxStreamDataFrameSize; + } + // no space left in packet + return size_t(0); + } + case QuicWriteFrame::Type::DataBlockedFrame_E: { + DataBlockedFrame& blockedFrame = *frame.asDataBlockedFrame(); + QuicInteger intFrameType(static_cast(FrameType::DATA_BLOCKED)); + QuicInteger dataLimit(blockedFrame.dataLimit); + auto blockedFrameSize = intFrameType.getSize() + dataLimit.getSize(); + if (packetSpaceCheck(spaceLeft, blockedFrameSize)) { + builder.write(intFrameType); + builder.write(dataLimit); + builder.appendFrame(std::move(blockedFrame)); + return blockedFrameSize; + } + // no space left in packet + return size_t(0); + } + case QuicWriteFrame::Type::StreamDataBlockedFrame_E: { + StreamDataBlockedFrame& streamBlockedFrame = + *frame.asStreamDataBlockedFrame(); + QuicInteger intFrameType( + static_cast(FrameType::STREAM_DATA_BLOCKED)); + QuicInteger streamId(streamBlockedFrame.streamId); + QuicInteger dataLimit(streamBlockedFrame.dataLimit); + auto blockedFrameSize = + intFrameType.getSize() + streamId.getSize() + dataLimit.getSize(); + if (packetSpaceCheck(spaceLeft, blockedFrameSize)) { + builder.write(intFrameType); + builder.write(streamId); + builder.write(dataLimit); + builder.appendFrame(std::move(streamBlockedFrame)); + return blockedFrameSize; + } + // no space left in packet + return size_t(0); + } + case QuicWriteFrame::Type::StreamsBlockedFrame_E: { + StreamsBlockedFrame& streamsBlockedFrame = *frame.asStreamsBlockedFrame(); + auto frameType = streamsBlockedFrame.isForBidirectionalStream() + ? FrameType::STREAMS_BLOCKED_BIDI + : FrameType::STREAMS_BLOCKED_UNI; + QuicInteger intFrameType(static_cast(frameType)); + QuicInteger streamId(streamsBlockedFrame.streamLimit); + auto streamBlockedFrameSize = intFrameType.getSize() + streamId.getSize(); + if (packetSpaceCheck(spaceLeft, streamBlockedFrameSize)) { + builder.write(intFrameType); + builder.write(streamId); + builder.appendFrame(std::move(streamsBlockedFrame)); + return streamBlockedFrameSize; + } + // no space left in packet + return size_t(0); + } + case QuicWriteFrame::Type::ConnectionCloseFrame_E: { + ConnectionCloseFrame& connectionCloseFrame = + *frame.asConnectionCloseFrame(); + QuicInteger intFrameType( + static_cast(FrameType::CONNECTION_CLOSE)); + QuicInteger reasonLength(connectionCloseFrame.reasonPhrase.size()); + QuicInteger closingFrameType( + static_cast(connectionCloseFrame.closingFrameType)); + QuicInteger errorCode( + static_cast(connectionCloseFrame.errorCode)); + auto version = builder.getVersion(); + size_t errorSize = version == QuicVersion::MVFST_OLD + ? sizeof(TransportErrorCode) + : errorCode.getSize(); + auto connCloseFrameSize = intFrameType.getSize() + errorSize + + closingFrameType.getSize() + reasonLength.getSize() + + connectionCloseFrame.reasonPhrase.size(); + if (packetSpaceCheck(spaceLeft, connCloseFrameSize)) { + builder.write(intFrameType); + if (version == QuicVersion::MVFST_OLD) { + builder.writeBE( + static_cast::type>( + connectionCloseFrame.errorCode)); + } else { + builder.write(errorCode); } - // no space left in packet - return size_t(0); - }, - [&](RstStreamFrame& rstStreamFrame) { - QuicInteger intFrameType(static_cast(FrameType::RST_STREAM)); - QuicInteger streamId(rstStreamFrame.streamId); - QuicInteger offset(rstStreamFrame.offset); - QuicInteger errorCode(static_cast(rstStreamFrame.errorCode)); - auto version = builder.getVersion(); - size_t errorSize = version == QuicVersion::MVFST_OLD - ? sizeof(ApplicationErrorCode) - : errorCode.getSize(); - auto rstStreamFrameSize = intFrameType.getSize() + errorSize + - streamId.getSize() + offset.getSize(); - if (packetSpaceCheck(spaceLeft, rstStreamFrameSize)) { - builder.write(intFrameType); - builder.write(streamId); - if (version == QuicVersion::MVFST_OLD) { - builder.writeBE( - static_cast(rstStreamFrame.errorCode)); - } else { - builder.write(errorCode); - } - builder.write(offset); - builder.appendFrame(std::move(rstStreamFrame)); - return rstStreamFrameSize; + builder.write(closingFrameType); + builder.write(reasonLength); + builder.push( + (const uint8_t*)connectionCloseFrame.reasonPhrase.data(), + connectionCloseFrame.reasonPhrase.size()); + builder.appendFrame(std::move(connectionCloseFrame)); + return connCloseFrameSize; + } + // no space left in packet + return size_t(0); + } + case QuicWriteFrame::Type::ApplicationCloseFrame_E: { + ApplicationCloseFrame& applicationCloseFrame = + *frame.asApplicationCloseFrame(); + QuicInteger intFrameType( + static_cast(FrameType::APPLICATION_CLOSE)); + QuicInteger reasonLength(applicationCloseFrame.reasonPhrase.size()); + QuicInteger errorCode( + static_cast(applicationCloseFrame.errorCode)); + auto version = builder.getVersion(); + size_t errorSize = version == QuicVersion::MVFST_OLD + ? sizeof(ApplicationErrorCode) + : errorCode.getSize(); + auto applicationCloseFrameSize = intFrameType.getSize() + errorSize + + reasonLength.getSize() + applicationCloseFrame.reasonPhrase.size(); + if (packetSpaceCheck(spaceLeft, applicationCloseFrameSize)) { + builder.write(intFrameType); + if (version == QuicVersion::MVFST_OLD) { + builder.writeBE(static_cast( + applicationCloseFrame.errorCode)); + } else { + builder.write(errorCode); } - // no space left in packet - return size_t(0); - }, - [&](MaxDataFrame& maxDataFrame) { - QuicInteger intFrameType(static_cast(FrameType::MAX_DATA)); - QuicInteger maximumData(maxDataFrame.maximumData); - auto frameSize = intFrameType.getSize() + maximumData.getSize(); - if (packetSpaceCheck(spaceLeft, frameSize)) { - builder.write(intFrameType); - builder.write(maximumData); - builder.appendFrame(std::move(maxDataFrame)); - return frameSize; - } - // no space left in packet - return size_t(0); - }, - [&](MaxStreamDataFrame& maxStreamDataFrame) { - QuicInteger intFrameType( - static_cast(FrameType::MAX_STREAM_DATA)); - QuicInteger streamId(maxStreamDataFrame.streamId); - QuicInteger maximumData(maxStreamDataFrame.maximumData); - auto maxStreamDataFrameSize = - intFrameType.getSize() + streamId.getSize() + maximumData.getSize(); - if (packetSpaceCheck(spaceLeft, maxStreamDataFrameSize)) { - builder.write(intFrameType); - builder.write(streamId); - builder.write(maximumData); - builder.appendFrame(std::move(maxStreamDataFrame)); - return maxStreamDataFrameSize; - } - // no space left in packet - return size_t(0); - }, - [&](DataBlockedFrame& blockedFrame) { - QuicInteger intFrameType(static_cast(FrameType::DATA_BLOCKED)); - QuicInteger dataLimit(blockedFrame.dataLimit); - auto blockedFrameSize = intFrameType.getSize() + dataLimit.getSize(); - if (packetSpaceCheck(spaceLeft, blockedFrameSize)) { - builder.write(intFrameType); - builder.write(dataLimit); - builder.appendFrame(std::move(blockedFrame)); - return blockedFrameSize; - } - // no space left in packet - return size_t(0); - }, - [&](StreamDataBlockedFrame& streamBlockedFrame) { - QuicInteger intFrameType( - static_cast(FrameType::STREAM_DATA_BLOCKED)); - QuicInteger streamId(streamBlockedFrame.streamId); - QuicInteger dataLimit(streamBlockedFrame.dataLimit); - auto blockedFrameSize = - intFrameType.getSize() + streamId.getSize() + dataLimit.getSize(); - if (packetSpaceCheck(spaceLeft, blockedFrameSize)) { - builder.write(intFrameType); - builder.write(streamId); - builder.write(dataLimit); - builder.appendFrame(std::move(streamBlockedFrame)); - return blockedFrameSize; - } - // no space left in packet - return size_t(0); - }, - [&](StreamsBlockedFrame& streamsBlockedFrame) { - auto frameType = streamsBlockedFrame.isForBidirectionalStream() - ? FrameType::STREAMS_BLOCKED_BIDI - : FrameType::STREAMS_BLOCKED_UNI; - QuicInteger intFrameType(static_cast(frameType)); - QuicInteger streamId(streamsBlockedFrame.streamLimit); - auto streamBlockedFrameSize = - intFrameType.getSize() + streamId.getSize(); - if (packetSpaceCheck(spaceLeft, streamBlockedFrameSize)) { - builder.write(intFrameType); - builder.write(streamId); - builder.appendFrame(std::move(streamsBlockedFrame)); - return streamBlockedFrameSize; - } - // no space left in packet - return size_t(0); - }, - [&](ConnectionCloseFrame& connectionCloseFrame) { - QuicInteger intFrameType( - static_cast(FrameType::CONNECTION_CLOSE)); - QuicInteger reasonLength(connectionCloseFrame.reasonPhrase.size()); - QuicInteger closingFrameType( - static_cast(connectionCloseFrame.closingFrameType)); - QuicInteger errorCode( - static_cast(connectionCloseFrame.errorCode)); - auto version = builder.getVersion(); - size_t errorSize = version == QuicVersion::MVFST_OLD - ? sizeof(TransportErrorCode) - : errorCode.getSize(); - auto connCloseFrameSize = intFrameType.getSize() + errorSize + - closingFrameType.getSize() + reasonLength.getSize() + - connectionCloseFrame.reasonPhrase.size(); - if (packetSpaceCheck(spaceLeft, connCloseFrameSize)) { - builder.write(intFrameType); - if (version == QuicVersion::MVFST_OLD) { - builder.writeBE( - static_cast::type>( - connectionCloseFrame.errorCode)); - } else { - builder.write(errorCode); - } - builder.write(closingFrameType); - builder.write(reasonLength); - builder.push( - (const uint8_t*)connectionCloseFrame.reasonPhrase.data(), - connectionCloseFrame.reasonPhrase.size()); - builder.appendFrame(std::move(connectionCloseFrame)); - return connCloseFrameSize; - } - // no space left in packet - return size_t(0); - }, - [&](ApplicationCloseFrame& applicationCloseFrame) { - QuicInteger intFrameType( - static_cast(FrameType::APPLICATION_CLOSE)); - QuicInteger reasonLength(applicationCloseFrame.reasonPhrase.size()); - QuicInteger errorCode( - static_cast(applicationCloseFrame.errorCode)); - auto version = builder.getVersion(); - size_t errorSize = version == QuicVersion::MVFST_OLD - ? sizeof(ApplicationErrorCode) - : errorCode.getSize(); - auto applicationCloseFrameSize = intFrameType.getSize() + errorSize + - reasonLength.getSize() + applicationCloseFrame.reasonPhrase.size(); - if (packetSpaceCheck(spaceLeft, applicationCloseFrameSize)) { - builder.write(intFrameType); - if (version == QuicVersion::MVFST_OLD) { - builder.writeBE(static_cast( - applicationCloseFrame.errorCode)); - } else { - builder.write(errorCode); - } - builder.write(reasonLength); - builder.push( - (const uint8_t*)applicationCloseFrame.reasonPhrase.data(), - applicationCloseFrame.reasonPhrase.size()); - builder.appendFrame(std::move(applicationCloseFrame)); - return applicationCloseFrameSize; - } - // no space left in packet - return size_t(0); - }, - [&](QuicSimpleFrame& simpleFrame) { - return writeSimpleFrame(std::move(simpleFrame), builder); - }, - [&](auto&) -> size_t { - // TODO add support for: RETIRE_CONNECTION_ID and NEW_TOKEN frames - auto errorStr = folly::to( - "Unknown / unsupported frame type received at ", __func__); - VLOG(2) << errorStr; - throw QuicTransportException( - errorStr, TransportErrorCode::FRAME_ENCODING_ERROR); - }); + builder.write(reasonLength); + builder.push( + (const uint8_t*)applicationCloseFrame.reasonPhrase.data(), + applicationCloseFrame.reasonPhrase.size()); + builder.appendFrame(std::move(applicationCloseFrame)); + return applicationCloseFrameSize; + } + // no space left in packet + return size_t(0); + } + case QuicWriteFrame::Type::QuicSimpleFrame_E: { + return writeSimpleFrame(std::move(*frame.asQuicSimpleFrame()), builder); + } + default: { + // TODO add support for: RETIRE_CONNECTION_ID and NEW_TOKEN frames + auto errorStr = folly::to( + "Unknown / unsupported frame type received at ", __func__); + VLOG(2) << errorStr; + throw QuicTransportException( + errorStr, TransportErrorCode::FRAME_ENCODING_ERROR); + } + } } } // namespace quic diff --git a/quic/codec/Types.h b/quic/codec/Types.h index 2bb4d78dc..21ad07306 100644 --- a/quic/codec/Types.h +++ b/quic/codec/Types.h @@ -592,22 +592,25 @@ using QuicSimpleFrame = boost::variant< DECLARE_VARIANT_TYPE(QuicFrame, QUIC_FRAME) +#define QUIC_WRITE_FRAME(F, ...) \ + F(PaddingFrame, __VA_ARGS__) \ + F(RstStreamFrame, __VA_ARGS__) \ + F(ConnectionCloseFrame, __VA_ARGS__) \ + F(ApplicationCloseFrame, __VA_ARGS__) \ + F(MaxDataFrame, __VA_ARGS__) \ + F(MaxStreamDataFrame, __VA_ARGS__) \ + F(PingFrame, __VA_ARGS__) \ + F(DataBlockedFrame, __VA_ARGS__) \ + F(StreamDataBlockedFrame, __VA_ARGS__) \ + F(StreamsBlockedFrame, __VA_ARGS__) \ + F(WriteAckFrame, __VA_ARGS__) \ + F(WriteStreamFrame, __VA_ARGS__) \ + F(WriteCryptoFrame, __VA_ARGS__) \ + F(QuicSimpleFrame, __VA_ARGS__) \ + F(NoopFrame, __VA_ARGS__) + // Types of frames which are written. -using QuicWriteFrame = boost::variant< - PaddingFrame, - RstStreamFrame, - ConnectionCloseFrame, - ApplicationCloseFrame, - MaxDataFrame, - MaxStreamDataFrame, - PingFrame, - DataBlockedFrame, - StreamDataBlockedFrame, - StreamsBlockedFrame, - WriteAckFrame, - WriteStreamFrame, - WriteCryptoFrame, - QuicSimpleFrame>; +DECLARE_VARIANT_TYPE(QuicWriteFrame, QUIC_WRITE_FRAME) enum class HeaderForm : bool { Long = 1, diff --git a/quic/codec/test/QuicPacketRebuilderTest.cpp b/quic/codec/test/QuicPacketRebuilderTest.cpp index 0da47504d..5b4b2c807 100644 --- a/quic/codec/test/QuicPacketRebuilderTest.cpp +++ b/quic/codec/test/QuicPacketRebuilderTest.cpp @@ -80,7 +80,7 @@ TEST_F(QuicPacketRebuilderTest, RebuildPacket) { // Write them with a regular builder writeFrame(connCloseFrame, regularBuilder1); - writeFrame(maxStreamsFrame, regularBuilder1); + writeFrame(QuicSimpleFrame(maxStreamsFrame), regularBuilder1); writeFrame(pingFrame, regularBuilder1); writeAckFrame(ackMeta, regularBuilder1); writeStreamFrameHeader( @@ -123,44 +123,61 @@ TEST_F(QuicPacketRebuilderTest, RebuildPacket) { stream->currentReadOffset + stream->flowControlState.windowSize, stream->flowControlState.advertisedMaxOffset); for (const auto& frame : packet2.packet.frames) { - folly::variant_match( - frame, - [](const ConnectionCloseFrame& closeFrame) { - EXPECT_EQ( - TransportErrorCode::FRAME_ENCODING_ERROR, closeFrame.errorCode); - EXPECT_EQ("The sun is in the sky.", closeFrame.reasonPhrase); - EXPECT_EQ(FrameType::ACK, closeFrame.closingFrameType); - }, - [](const QuicSimpleFrame& simpleFrame) { - auto maxStreamFrame = boost::get(simpleFrame); - EXPECT_EQ(4321, maxStreamFrame.maxStreams); - }, - [](const PingFrame& ping) { EXPECT_EQ(PingFrame(), ping); }, - [](const WriteAckFrame& ack) { - EXPECT_EQ(Interval(10, 100), ack.ackBlocks.front()); - EXPECT_EQ(Interval(200, 1000), ack.ackBlocks.back()); - }, - [&buf, &streamId](const WriteStreamFrame& streamFrame) { - EXPECT_EQ(streamId, streamFrame.streamId); - EXPECT_EQ(0, streamFrame.offset); - EXPECT_EQ(buf->computeChainDataLength(), streamFrame.len); - EXPECT_EQ(true, streamFrame.fin); - }, - [&cryptoOffset, &cryptoBuf](const WriteCryptoFrame& frame) { - EXPECT_EQ(frame.offset, cryptoOffset); - EXPECT_EQ(frame.len, cryptoBuf->computeChainDataLength()); - }, - [&expectedConnFlowControlValue](const MaxDataFrame& maxData) { - EXPECT_EQ(expectedConnFlowControlValue, maxData.maximumData); - }, - [&streamId, &expectedStreamFlowControlValue]( - const MaxStreamDataFrame& maxStreamData) { - EXPECT_EQ(streamId, maxStreamData.streamId); - EXPECT_EQ(expectedStreamFlowControlValue, maxStreamData.maximumData); - }, - [](const auto&) { - EXPECT_TRUE(false); /* should never happen*/ - }); + switch (frame.type()) { + case QuicWriteFrame::Type::ConnectionCloseFrame_E: { + const ConnectionCloseFrame& closeFrame = + *frame.asConnectionCloseFrame(); + EXPECT_EQ( + TransportErrorCode::FRAME_ENCODING_ERROR, closeFrame.errorCode); + EXPECT_EQ("The sun is in the sky.", closeFrame.reasonPhrase); + EXPECT_EQ(FrameType::ACK, closeFrame.closingFrameType); + break; + } + case QuicWriteFrame::Type::QuicSimpleFrame_E: { + const QuicSimpleFrame& simpleFrame = *frame.asQuicSimpleFrame(); + auto maxStreamFrame = boost::get(simpleFrame); + EXPECT_EQ(4321, maxStreamFrame.maxStreams); + break; + } + case QuicWriteFrame::Type::PingFrame_E: { + const PingFrame& ping = *frame.asPingFrame(); + EXPECT_EQ(PingFrame(), ping); + break; + } + case QuicWriteFrame::Type::WriteAckFrame_E: { + const WriteAckFrame& ack = *frame.asWriteAckFrame(); + EXPECT_EQ(Interval(10, 100), ack.ackBlocks.front()); + EXPECT_EQ(Interval(200, 1000), ack.ackBlocks.back()); + break; + } + case QuicWriteFrame::Type::WriteStreamFrame_E: { + const WriteStreamFrame& streamFrame = *frame.asWriteStreamFrame(); + EXPECT_EQ(streamId, streamFrame.streamId); + EXPECT_EQ(0, streamFrame.offset); + EXPECT_EQ(buf->computeChainDataLength(), streamFrame.len); + EXPECT_EQ(true, streamFrame.fin); + break; + } + case QuicWriteFrame::Type::WriteCryptoFrame_E: { + const WriteCryptoFrame& cryptoFrame = *frame.asWriteCryptoFrame(); + EXPECT_EQ(cryptoFrame.offset, cryptoOffset); + EXPECT_EQ(cryptoFrame.len, cryptoBuf->computeChainDataLength()); + break; + } + case QuicWriteFrame::Type::MaxDataFrame_E: { + const MaxDataFrame& maxData = *frame.asMaxDataFrame(); + EXPECT_EQ(expectedConnFlowControlValue, maxData.maximumData); + break; + } + case QuicWriteFrame::Type::MaxStreamDataFrame_E: { + const MaxStreamDataFrame& maxStreamData = *frame.asMaxStreamDataFrame(); + EXPECT_EQ(streamId, maxStreamData.streamId); + EXPECT_EQ(expectedStreamFlowControlValue, maxStreamData.maximumData); + break; + } + default: + EXPECT_TRUE(false); /* should never happen*/ + } } EXPECT_TRUE(folly::IOBufEqualTo()(*packet1.header, *packet2.header)); // TODO: I don't have a good way to verify body without decode them @@ -286,17 +303,14 @@ TEST_F(QuicPacketRebuilderTest, RebuildDataStreamAndEmptyCryptoStream) { // rebuilder writes frames to regularBuilder2 EXPECT_EQ(packet1.packet.frames.size(), packet2.packet.frames.size() + 1); for (const auto& frame : packet2.packet.frames) { - folly::variant_match( - frame, - [&buf, &streamId](const WriteStreamFrame& streamFrame) { - EXPECT_EQ(streamId, streamFrame.streamId); - EXPECT_EQ(0, streamFrame.offset); - EXPECT_EQ(buf->computeChainDataLength(), streamFrame.len); - EXPECT_EQ(true, streamFrame.fin); - }, - [](const auto&) { - EXPECT_TRUE(false); /* should never happen*/ - }); + const WriteStreamFrame* streamFrame = frame.asWriteStreamFrame(); + if (!streamFrame) { + EXPECT_TRUE(false); /* should never happen*/ + } + EXPECT_EQ(streamId, streamFrame->streamId); + EXPECT_EQ(0, streamFrame->offset); + EXPECT_EQ(buf->computeChainDataLength(), streamFrame->len); + EXPECT_EQ(true, streamFrame->fin); } EXPECT_TRUE(folly::IOBufEqualTo()(*packet1.header, *packet2.header)); } diff --git a/quic/codec/test/QuicWriteCodecTest.cpp b/quic/codec/test/QuicWriteCodecTest.cpp index b92cdbc53..ed4f9ac2a 100644 --- a/quic/codec/test/QuicWriteCodecTest.cpp +++ b/quic/codec/test/QuicWriteCodecTest.cpp @@ -127,7 +127,7 @@ TEST_F(QuicWriteCodecTest, WriteStreamFrameToEmptyPacket) { auto regularPacket = std::move(builtOut.first); EXPECT_EQ(regularPacket.frames.size(), 1); - auto resultFrame = boost::get(regularPacket.frames.back()); + auto& resultFrame = *regularPacket.frames.back().asWriteStreamFrame(); EXPECT_EQ(resultFrame.streamId, streamId); EXPECT_EQ(resultFrame.offset, offset); EXPECT_EQ(resultFrame.len, 10); @@ -174,7 +174,7 @@ TEST_F(QuicWriteCodecTest, WriteStreamFrameToPartialPacket) { auto builtOut = std::move(pktBuilder).buildPacket(); auto regularPacket = std::move(builtOut.first); EXPECT_EQ(regularPacket.frames.size(), 1); - auto resultFrame = boost::get(regularPacket.frames.back()); + auto& resultFrame = *regularPacket.frames.back().asWriteStreamFrame(); EXPECT_EQ(resultFrame.streamId, streamId); EXPECT_EQ(resultFrame.offset, offset); EXPECT_EQ(resultFrame.len, 20); @@ -241,14 +241,14 @@ TEST_F(QuicWriteCodecTest, WriteTwoStreamFrames) { auto builtOut = std::move(pktBuilder).buildPacket(); auto regularPacket = std::move(builtOut.first); EXPECT_EQ(regularPacket.frames.size(), 2); - auto resultFrame = boost::get(regularPacket.frames.front()); + auto& resultFrame = *regularPacket.frames.front().asWriteStreamFrame(); EXPECT_EQ(resultFrame.streamId, streamId1); EXPECT_EQ(resultFrame.offset, offset1); EXPECT_EQ(resultFrame.len, 30); outputBuf->trimStart(8); EXPECT_TRUE(folly::IOBufEqualTo()(inputBuf, outputBuf)); - auto resultFrame2 = boost::get(regularPacket.frames.back()); + auto& resultFrame2 = *regularPacket.frames.back().asWriteStreamFrame(); EXPECT_EQ(resultFrame2.streamId, streamId2); EXPECT_EQ(resultFrame2.offset, offset2); EXPECT_EQ(resultFrame2.len, remainingSpace - 7); @@ -307,7 +307,7 @@ TEST_F(QuicWriteCodecTest, WriteStreamFramePartialData) { auto builtOut = std::move(pktBuilder).buildPacket(); auto regularPacket = builtOut.first; EXPECT_EQ(regularPacket.frames.size(), 1); - auto resultFrame = boost::get(regularPacket.frames.back()); + auto& resultFrame = *regularPacket.frames.back().asWriteStreamFrame(); EXPECT_EQ(resultFrame.streamId, streamId); EXPECT_EQ(resultFrame.offset, offset); EXPECT_EQ(resultFrame.len, 33); @@ -385,7 +385,7 @@ TEST_F(QuicWriteCodecTest, WriteStreamSpaceForOneByte) { auto regularPacket = builtOut.first; EXPECT_EQ(regularPacket.frames.size(), 1); - auto resultFrame = boost::get(regularPacket.frames.back()); + auto& resultFrame = *regularPacket.frames.back().asWriteStreamFrame(); EXPECT_EQ(resultFrame.streamId, streamId); EXPECT_EQ(resultFrame.offset, offset); EXPECT_EQ(resultFrame.len, 1); @@ -430,7 +430,7 @@ TEST_F(QuicWriteCodecTest, WriteFinToEmptyPacket) { auto regularPacket = builtOut.first; EXPECT_EQ(regularPacket.frames.size(), 1); - auto resultFrame = boost::get(regularPacket.frames.back()); + auto& resultFrame = *regularPacket.frames.back().asWriteStreamFrame(); EXPECT_EQ(resultFrame.streamId, streamId); EXPECT_EQ(resultFrame.offset, offset); EXPECT_EQ(inputBuf->computeChainDataLength(), resultFrame.len); @@ -592,8 +592,7 @@ TEST_F(QuicWriteCodecTest, AckFrameVeryLargeAckRange) { auto builtOut = std::move(pktBuilder).buildPacket(); auto regularPacket = builtOut.first; EXPECT_EQ(regularPacket.frames.size(), 1); - WriteAckFrame ackFrame = - boost::get(regularPacket.frames.back()); + WriteAckFrame& ackFrame = *regularPacket.frames.back().asWriteAckFrame(); EXPECT_EQ(ackFrame.ackBlocks.size(), 1); EXPECT_EQ(ackFrame.ackBlocks.front().start, 1); @@ -640,8 +639,7 @@ TEST_F(QuicWriteCodecTest, WriteSimpleAckFrame) { EXPECT_EQ(kDefaultUDPSendPacketLen - 11, pktBuilder.remainingSpaceInPkt()); auto builtOut = std::move(pktBuilder).buildPacket(); auto regularPacket = builtOut.first; - WriteAckFrame ackFrame = - boost::get(regularPacket.frames.back()); + WriteAckFrame& ackFrame = *regularPacket.frames.back().asWriteAckFrame(); EXPECT_EQ(ackFrame.ackBlocks.size(), 2); auto iter = ackFrame.ackBlocks.cbegin(); EXPECT_EQ(iter->start, 101); @@ -675,8 +673,7 @@ TEST_F(QuicWriteCodecTest, WriteAckFrameWillSaveAckDelay) { writeAckFrame(meta, pktBuilder); auto builtOut = std::move(pktBuilder).buildPacket(); auto regularPacket = builtOut.first; - WriteAckFrame ackFrame = - boost::get(regularPacket.frames.back()); + WriteAckFrame& ackFrame = *regularPacket.frames.back().asWriteAckFrame(); EXPECT_EQ(ackDelay, ackFrame.ackDelay); } @@ -714,8 +711,7 @@ TEST_F(QuicWriteCodecTest, VerifyNumAckBlocksSizeAccounted) { auto builtOut = std::move(pktBuilder).buildPacket(); auto regularPacket = builtOut.first; EXPECT_EQ(regularPacket.frames.size(), 1); - WriteAckFrame ackFrame = - boost::get(regularPacket.frames.back()); + WriteAckFrame& ackFrame = *regularPacket.frames.back().asWriteAckFrame(); EXPECT_EQ(ackFrame.ackBlocks.size(), 64); EXPECT_EQ(ackFrame.ackBlocks.front().start, 746); @@ -787,8 +783,7 @@ TEST_F(QuicWriteCodecTest, OnlyAckLargestPacket) { auto builtOut = std::move(pktBuilder).buildPacket(); auto regularPacket = builtOut.first; EXPECT_EQ(regularPacket.frames.size(), 1); - WriteAckFrame ackFrame = - boost::get(regularPacket.frames.back()); + WriteAckFrame& ackFrame = *regularPacket.frames.back().asWriteAckFrame(); EXPECT_EQ(ackFrame.ackBlocks.size(), 1); EXPECT_EQ(ackFrame.ackBlocks.front().start, 1000); EXPECT_EQ(ackFrame.ackBlocks.front().end, 1000); @@ -838,8 +833,7 @@ TEST_F(QuicWriteCodecTest, WriteSomeAckBlocks) { auto builtOut = std::move(pktBuilder).buildPacket(); auto regularPacket = builtOut.first; EXPECT_EQ(regularPacket.frames.size(), 1); - WriteAckFrame ackFrame = - boost::get(regularPacket.frames.back()); + WriteAckFrame& ackFrame = *regularPacket.frames.back().asWriteAckFrame(); EXPECT_EQ(ackFrame.ackBlocks.size(), 14); // Verify the on wire bytes via decoder: @@ -886,8 +880,7 @@ TEST_F(QuicWriteCodecTest, OnlyHasSpaceForFirstAckBlock) { EXPECT_EQ(ackFrameWriteResult.bytesWritten, 7); EXPECT_EQ(pktBuilder.remainingSpaceInPkt(), 3); auto builtOut = std::move(pktBuilder).buildPacket(); - WriteAckFrame ackFrame = - boost::get(builtOut.first.frames.back()); + WriteAckFrame& ackFrame = *builtOut.first.frames.back().asWriteAckFrame(); EXPECT_EQ(ackFrame.ackBlocks.size(), 1); EXPECT_EQ(ackFrame.ackBlocks.front().start, 1000); EXPECT_EQ(ackFrame.ackBlocks.front().end, 1000); @@ -917,7 +910,7 @@ TEST_F(QuicWriteCodecTest, WriteMaxStreamData) { auto regularPacket = builtOut.first; EXPECT_EQ(bytesWritten, 3); auto& resultMaxStreamDataFrame = - boost::get(regularPacket.frames[0]); + *regularPacket.frames[0].asMaxStreamDataFrame(); EXPECT_EQ(id, resultMaxStreamDataFrame.streamId); EXPECT_EQ(offset, resultMaxStreamDataFrame.maximumData); @@ -949,7 +942,7 @@ TEST_F(QuicWriteCodecTest, WriteMaxData) { auto builtOut = std::move(pktBuilder).buildPacket(); auto regularPacket = builtOut.first; EXPECT_EQ(bytesWritten, 3); - auto& resultMaxDataFrame = boost::get(regularPacket.frames[0]); + auto& resultMaxDataFrame = *regularPacket.frames[0].asMaxDataFrame(); EXPECT_EQ(1000, resultMaxDataFrame.maximumData); auto wireBuf = std::move(builtOut.second); @@ -975,7 +968,8 @@ TEST_F(QuicWriteCodecTest, WriteMaxStreamId) { uint64_t maxStream = i; bool isBidirectional = true; MaxStreamsFrame maxStreamsFrame(maxStream, isBidirectional); - auto bytesWritten = writeFrame(maxStreamsFrame, pktBuilder); + auto bytesWritten = + writeFrame(QuicSimpleFrame(maxStreamsFrame), pktBuilder); auto builtOut = std::move(pktBuilder).buildPacket(); auto regularPacket = builtOut.first; @@ -983,7 +977,7 @@ TEST_F(QuicWriteCodecTest, WriteMaxStreamId) { // 1 byte for the type and up to 2 bytes for the stream count. EXPECT_EQ(1 + streamCountSize, bytesWritten); auto& resultMaxStreamIdFrame = boost::get( - boost::get(regularPacket.frames[0])); + *regularPacket.frames[0].asQuicSimpleFrame()); EXPECT_EQ(i, resultMaxStreamIdFrame.maxStreams); auto wireBuf = std::move(builtOut.second); @@ -1003,7 +997,8 @@ TEST_F(QuicWriteCodecTest, WriteUniMaxStreamId) { uint64_t maxStream = i; bool isBidirectional = false; MaxStreamsFrame maxStreamsFrame(maxStream, isBidirectional); - auto bytesWritten = writeFrame(maxStreamsFrame, pktBuilder); + auto bytesWritten = + writeFrame(QuicSimpleFrame(maxStreamsFrame), pktBuilder); auto builtOut = std::move(pktBuilder).buildPacket(); auto regularPacket = builtOut.first; @@ -1011,7 +1006,7 @@ TEST_F(QuicWriteCodecTest, WriteUniMaxStreamId) { // 1 byte for the type and up to 2 bytes for the stream count. EXPECT_EQ(1 + streamCountSize, bytesWritten); auto& resultMaxStreamIdFrame = boost::get( - boost::get(regularPacket.frames[0])); + *regularPacket.frames[0].asQuicSimpleFrame()); EXPECT_EQ(i, resultMaxStreamIdFrame.maxStreams); auto wireBuf = std::move(builtOut.second); @@ -1030,7 +1025,7 @@ TEST_F(QuicWriteCodecTest, NoSpaceForMaxStreamId) { setupCommonExpects(pktBuilder); StreamId maxStream = 0x1234; MaxStreamsFrame maxStreamIdFrame(maxStream, true); - EXPECT_EQ(0, writeFrame(maxStreamIdFrame, pktBuilder)); + EXPECT_EQ(0, writeFrame(QuicSimpleFrame(maxStreamIdFrame), pktBuilder)); } TEST_F(QuicWriteCodecTest, WriteConnClose) { @@ -1039,14 +1034,15 @@ TEST_F(QuicWriteCodecTest, WriteConnClose) { std::string reasonPhrase("You are fired"); ConnectionCloseFrame connectionCloseFrame( TransportErrorCode::PROTOCOL_VIOLATION, reasonPhrase); - auto connCloseBytesWritten = writeFrame(connectionCloseFrame, pktBuilder); + auto connCloseBytesWritten = + writeFrame(std::move(connectionCloseFrame), pktBuilder); auto builtOut = std::move(pktBuilder).buildPacket(); auto regularPacket = builtOut.first; // 6 == ErrorCode(2) + FrameType(1) + reasonPhrase-len(2) EXPECT_EQ(4 + reasonPhrase.size(), connCloseBytesWritten); auto& resultConnCloseFrame = - boost::get(regularPacket.frames[0]); + *regularPacket.frames[0].asConnectionCloseFrame(); EXPECT_EQ( TransportErrorCode::PROTOCOL_VIOLATION, resultConnCloseFrame.errorCode); EXPECT_EQ("You are fired", resultConnCloseFrame.reasonPhrase); @@ -1074,7 +1070,7 @@ TEST_F(QuicWriteCodecTest, DecodeConnCloseLarge) { auto builtOut = std::move(pktBuilder).buildPacket(); auto regularPacket = builtOut.first; auto& resultConnCloseFrame = - boost::get(regularPacket.frames[0]); + *regularPacket.frames[0].asConnectionCloseFrame(); EXPECT_EQ( TransportErrorCode::PROTOCOL_VIOLATION, resultConnCloseFrame.errorCode); EXPECT_EQ(resultConnCloseFrame.reasonPhrase, reasonPhrase); @@ -1091,7 +1087,7 @@ TEST_F(QuicWriteCodecTest, NoSpaceConnClose) { std::string reasonPhrase("You are all fired"); ConnectionCloseFrame connCloseFrame( TransportErrorCode::PROTOCOL_VIOLATION, reasonPhrase); - EXPECT_EQ(0, writeFrame(connCloseFrame, pktBuilder)); + EXPECT_EQ(0, writeFrame(std::move(connCloseFrame), pktBuilder)); } TEST_F(QuicWriteCodecTest, DecodeAppCloseLarge) { @@ -1101,12 +1097,12 @@ TEST_F(QuicWriteCodecTest, DecodeAppCloseLarge) { reasonPhrase.resize(kMaxReasonPhraseLength + 10); ApplicationCloseFrame applicationCloseFrame( GenericApplicationErrorCode::UNKNOWN, reasonPhrase); - writeFrame(applicationCloseFrame, pktBuilder); + writeFrame(std::move(applicationCloseFrame), pktBuilder); auto builtOut = std::move(pktBuilder).buildPacket(); auto regularPacket = builtOut.first; auto& resultAppCloseFrame = - boost::get(regularPacket.frames[0]); + *regularPacket.frames[0].asApplicationCloseFrame(); EXPECT_EQ( GenericApplicationErrorCode::UNKNOWN, resultAppCloseFrame.errorCode); EXPECT_EQ(resultAppCloseFrame.reasonPhrase, reasonPhrase); @@ -1119,13 +1115,12 @@ TEST_F(QuicWriteCodecTest, DecodeAppCloseLarge) { TEST_F(QuicWriteCodecTest, WritePing) { MockQuicPacketBuilder pktBuilder; setupCommonExpects(pktBuilder); - PingFrame pingFrame; - auto pingBytesWritten = writeFrame(pingFrame, pktBuilder); + auto pingBytesWritten = writeFrame(PingFrame(), pktBuilder); auto builtOut = std::move(pktBuilder).buildPacket(); auto regularPacket = builtOut.first; EXPECT_EQ(1, pingBytesWritten); - EXPECT_NO_THROW(boost::get(regularPacket.frames[0])); + EXPECT_NE(regularPacket.frames[0].asPingFrame(), nullptr); auto wireBuf = std::move(builtOut.second); folly::io::Cursor cursor(wireBuf.get()); @@ -1140,20 +1135,18 @@ TEST_F(QuicWriteCodecTest, NoSpaceForPing) { MockQuicPacketBuilder pktBuilder; pktBuilder.remaining_ = 0; setupCommonExpects(pktBuilder); - PingFrame pingFrame; - EXPECT_EQ(0, writeFrame(pingFrame, pktBuilder)); + EXPECT_EQ(0, writeFrame(PingFrame(), pktBuilder)); } TEST_F(QuicWriteCodecTest, WritePadding) { MockQuicPacketBuilder pktBuilder; setupCommonExpects(pktBuilder); - PaddingFrame paddingFrame; - auto paddingBytesWritten = writeFrame(paddingFrame, pktBuilder); + auto paddingBytesWritten = writeFrame(PaddingFrame(), pktBuilder); auto builtOut = std::move(pktBuilder).buildPacket(); auto regularPacket = builtOut.first; EXPECT_EQ(1, paddingBytesWritten); - EXPECT_NO_THROW(boost::get(regularPacket.frames[0])); + EXPECT_NE(regularPacket.frames[0].asPaddingFrame(), nullptr); auto wireBuf = std::move(builtOut.second); folly::io::Cursor cursor(wireBuf.get()); @@ -1183,7 +1176,7 @@ TEST_F(QuicWriteCodecTest, WriteStreamBlocked) { auto builtOut = std::move(pktBuilder).buildPacket(); auto regularPacket = builtOut.first; EXPECT_EQ(blockedBytesWritten, 7); - EXPECT_NO_THROW(boost::get(regularPacket.frames[0])); + EXPECT_NE(regularPacket.frames[0].asStreamDataBlockedFrame(), nullptr); auto wireBuf = std::move(builtOut.second); folly::io::Cursor cursor(wireBuf.get()); @@ -1217,8 +1210,7 @@ TEST_F(QuicWriteCodecTest, WriteRstStream) { auto builtOut = std::move(pktBuilder).buildPacket(); auto regularPacket = builtOut.first; EXPECT_EQ(13, rstStreamBytesWritten); - auto& resultRstStreamFrame = - boost::get(regularPacket.frames[0]); + auto& resultRstStreamFrame = *regularPacket.frames[0].asRstStreamFrame(); EXPECT_EQ(errorCode, resultRstStreamFrame.errorCode); EXPECT_EQ(id, resultRstStreamFrame.streamId); EXPECT_EQ(offset, resultRstStreamFrame.offset); @@ -1255,7 +1247,7 @@ TEST_F(QuicWriteCodecTest, WriteBlockedFrame) { auto builtOut = std::move(pktBuilder).buildPacket(); auto regularPacket = builtOut.first; EXPECT_EQ(bytesWritten, 5); - EXPECT_NO_THROW(boost::get(regularPacket.frames[0])); + EXPECT_NE(regularPacket.frames[0].asDataBlockedFrame(), nullptr); auto wireBuf = std::move(builtOut.second); folly::io::Cursor cursor(wireBuf.get()); @@ -1279,13 +1271,13 @@ TEST_F(QuicWriteCodecTest, WriteStreamIdNeeded) { setupCommonExpects(pktBuilder); StreamId blockedStreamId = 0x211; MaxStreamsFrame streamIdNeeded(blockedStreamId, true); - auto bytesWritten = writeFrame(streamIdNeeded, pktBuilder); + auto bytesWritten = writeFrame(QuicSimpleFrame(streamIdNeeded), pktBuilder); auto builtOut = std::move(pktBuilder).buildPacket(); auto regularPacket = builtOut.first; EXPECT_EQ(bytesWritten, 3); EXPECT_NO_THROW(boost::get( - boost::get(regularPacket.frames[0]))); + *regularPacket.frames[0].asQuicSimpleFrame())); auto wireBuf = std::move(builtOut.second); folly::io::Cursor cursor(wireBuf.get()); @@ -1302,7 +1294,7 @@ TEST_F(QuicWriteCodecTest, NoSpaceForStreamIdNeeded) { setupCommonExpects(pktBuilder); StreamId blockedStreamId = 0x211; MaxStreamsFrame streamIdNeeded(blockedStreamId, true); - EXPECT_EQ(0, writeFrame(streamIdNeeded, pktBuilder)); + EXPECT_EQ(0, writeFrame(QuicSimpleFrame(streamIdNeeded), pktBuilder)); } TEST_F(QuicWriteCodecTest, WriteNewConnId) { @@ -1311,13 +1303,13 @@ TEST_F(QuicWriteCodecTest, WriteNewConnId) { StatelessResetToken token; memset(token.data(), 'a', token.size()); NewConnectionIdFrame newConnId(1, 0, getTestConnectionId(), token); - auto bytesWritten = writeFrame(newConnId, pktBuilder); + auto bytesWritten = writeFrame(QuicSimpleFrame(newConnId), pktBuilder); auto builtOut = std::move(pktBuilder).buildPacket(); auto regularPacket = builtOut.first; EXPECT_EQ(bytesWritten, 28); auto& resultNewConnIdFrame = boost::get( - boost::get(regularPacket.frames[0])); + *regularPacket.frames[0].asQuicSimpleFrame()); EXPECT_EQ(resultNewConnIdFrame.sequenceNumber, 1); EXPECT_EQ(resultNewConnIdFrame.retirePriorTo, 0); EXPECT_EQ(resultNewConnIdFrame.connectionId, getTestConnectionId()); @@ -1338,13 +1330,13 @@ TEST_F(QuicWriteCodecTest, WriteRetireConnId) { MockQuicPacketBuilder pktBuilder; setupCommonExpects(pktBuilder); RetireConnectionIdFrame retireConnId(3); - auto bytesWritten = writeFrame(retireConnId, pktBuilder); + auto bytesWritten = writeFrame(QuicSimpleFrame(retireConnId), pktBuilder); auto builtOut = std::move(pktBuilder).buildPacket(); auto regularPacket = builtOut.first; EXPECT_EQ(bytesWritten, 2); auto resultRetireConnIdFrame = boost::get( - boost::get(regularPacket.frames[0])); + *regularPacket.frames[0].asQuicSimpleFrame()); EXPECT_EQ(resultRetireConnIdFrame.sequenceNumber, 3); auto wireBuf = std::move(builtOut.second); @@ -1385,7 +1377,7 @@ TEST_F(QuicWriteCodecTest, NoSpaceForNewConnId) { setupCommonExpects(pktBuilder); NewConnectionIdFrame newConnId( 1, 0, getTestConnectionId(), StatelessResetToken()); - EXPECT_EQ(0, writeFrame(newConnId, pktBuilder)); + EXPECT_EQ(0, writeFrame(QuicSimpleFrame(newConnId), pktBuilder)); } TEST_F(QuicWriteCodecTest, WriteExpiredStreamDataFrame) { @@ -1394,13 +1386,14 @@ TEST_F(QuicWriteCodecTest, WriteExpiredStreamDataFrame) { StreamId id = 10; uint64_t offset = 0x08; ExpiredStreamDataFrame expiredStreamDataFrame(id, offset); - auto bytesWritten = writeFrame(expiredStreamDataFrame, pktBuilder); + auto bytesWritten = + writeFrame(QuicSimpleFrame(expiredStreamDataFrame), pktBuilder); auto builtOut = std::move(pktBuilder).buildPacket(); auto regularPacket = builtOut.first; EXPECT_EQ(bytesWritten, 4); auto result = boost::get( - boost::get(regularPacket.frames[0])); + *regularPacket.frames[0].asQuicSimpleFrame()); EXPECT_EQ(id, result.streamId); EXPECT_EQ(offset, result.minimumStreamOffset); @@ -1424,13 +1417,14 @@ TEST_F(QuicWriteCodecTest, WriteMinStreamDataFrame) { uint64_t maximumData = 0x64; uint64_t offset = 0x08; MinStreamDataFrame minStreamDataFrame(id, maximumData, offset); - auto bytesWritten = writeFrame(minStreamDataFrame, pktBuilder); + auto bytesWritten = + writeFrame(QuicSimpleFrame(minStreamDataFrame), pktBuilder); auto builtOut = std::move(pktBuilder).buildPacket(); auto regularPacket = builtOut.first; EXPECT_EQ(bytesWritten, 6); auto result = boost::get( - boost::get(regularPacket.frames[0])); + *regularPacket.frames[0].asQuicSimpleFrame()); EXPECT_EQ(id, result.streamId); EXPECT_EQ(maximumData, result.maximumData); EXPECT_EQ(offset, result.minimumStreamOffset); @@ -1461,7 +1455,7 @@ TEST_F(QuicWriteCodecTest, WritePathChallenge) { auto regularPacket = builtOut.first; auto result = boost::get( - boost::get(regularPacket.frames[0])); + *regularPacket.frames[0].asQuicSimpleFrame()); EXPECT_EQ(result.pathData, pathData); auto wireBuf = std::move(builtOut.second); @@ -1486,7 +1480,7 @@ TEST_F(QuicWriteCodecTest, WritePathResponse) { auto regularPacket = builtOut.first; auto result = boost::get( - boost::get(regularPacket.frames[0])); + *regularPacket.frames[0].asQuicSimpleFrame()); EXPECT_EQ(result.pathData, pathData); auto wireBuf = std::move(builtOut.second); diff --git a/quic/common/Variant.h b/quic/common/Variant.h index 556230695..ffc077525 100644 --- a/quic/common/Variant.h +++ b/quic/common/Variant.h @@ -30,9 +30,14 @@ namespace quic { return nullptr; \ } -#define UNION_CTORS(X, NAME) \ - NAME(X&& x) : type_(Type::X##_E) { \ - new (&X##_) X(std::move(x)); \ +#define UNION_CTORS(X, NAME) \ + NAME(X&& x) : type_(Type::X##_E) { \ + new (&X##_) X(std::move(x)); \ + } + +#define UNION_COPY_CTORS(X, NAME) \ + NAME(const X& x) : type_(Type::X##_E) { \ + new (&X##_) X(x); \ } #define UNION_MOVE_CASES(X, other) \ @@ -56,6 +61,8 @@ namespace quic { \ X(UNION_CTORS, NAME) \ \ + X(UNION_COPY_CTORS, NAME) \ + \ NAME(NAME&& other) { \ switch (other.type_) { X(UNION_MOVE_CASES, other) } \ type_ = other.type_; \ diff --git a/quic/common/test/TestUtils.cpp b/quic/common/test/TestUtils.cpp index 45c840eac..e62168a0d 100644 --- a/quic/common/test/TestUtils.cpp +++ b/quic/common/test/TestUtils.cpp @@ -70,8 +70,12 @@ PacketNum rstStreamAndSendPacket( conn.transportSettings.writeConnectionDataPacketsLimit); for (const auto& packet : conn.outstandingPackets) { - for (const auto& frame : all_frames(packet.packet.frames)) { - if (frame.streamId == stream.id) { + for (const auto& frame : packet.packet.frames) { + auto rstFrame = frame.asRstStreamFrame(); + if (!rstFrame) { + continue; + } + if (rstFrame->streamId == stream.id) { return packet.packet.header.getPacketSequenceNum(); } } diff --git a/quic/common/test/TestUtils.h b/quic/common/test/TestUtils.h index 3e305e500..005cbd423 100644 --- a/quic/common/test/TestUtils.h +++ b/quic/common/test/TestUtils.h @@ -276,14 +276,13 @@ auto findFrameInPacketFunc() { return [&](auto& p) { return std::find_if( p.packet.frames.begin(), p.packet.frames.end(), [&](auto& f) { + QuicSimpleFrame* simpleFrame = f.asQuicSimpleFrame(); + if (!simpleFrame) { + return false; + } return folly::variant_match( - f, - [&](QuicSimpleFrame& s) { - return folly::variant_match( - s, - [&](FrameType&) { return true; }, - [&](auto&) { return false; }); - }, + *simpleFrame, + [&](FrameType&) { return true; }, [&](auto&) { return false; }); }) != p.packet.frames.end(); }; diff --git a/quic/logging/QLogger.cpp b/quic/logging/QLogger.cpp index d5bb0f799..8479d44bb 100644 --- a/quic/logging/QLogger.cpp +++ b/quic/logging/QLogger.cpp @@ -195,62 +195,88 @@ std::unique_ptr QLogger::createPacketEvent( uint64_t numPaddingFrames = 0; // looping through the packet to store logs created from frames in the packet for (const auto& quicFrame : writePacket.frames) { - folly::variant_match( - quicFrame, - [&](const PaddingFrame& /* unused */) { ++numPaddingFrames; }, - [&](const RstStreamFrame& frame) { - event->frames.push_back(std::make_unique( - frame.streamId, frame.errorCode, frame.offset)); - }, - [&](const ConnectionCloseFrame& frame) { - event->frames.push_back(std::make_unique( - frame.errorCode, frame.reasonPhrase, frame.closingFrameType)); - }, - [&](const ApplicationCloseFrame& frame) { - event->frames.push_back(std::make_unique( - frame.errorCode, frame.reasonPhrase)); - }, - [&](const MaxDataFrame& frame) { - event->frames.push_back( - std::make_unique(frame.maximumData)); - }, - [&](const MaxStreamDataFrame& frame) { - event->frames.push_back(std::make_unique( - frame.streamId, frame.maximumData)); - }, - [&](const StreamsBlockedFrame& frame) { - event->frames.push_back(std::make_unique( - frame.streamLimit, frame.isForBidirectional)); - }, - [&](const PingFrame& /* unused */) { - event->frames.push_back(std::make_unique()); - }, - [&](const DataBlockedFrame& frame) { - event->frames.push_back( - std::make_unique(frame.dataLimit)); - }, - [&](const StreamDataBlockedFrame& frame) { - event->frames.push_back(std::make_unique( - frame.streamId, frame.dataLimit)); - }, - [&](const WriteAckFrame& frame) { - event->frames.push_back(std::make_unique( - frame.ackBlocks, frame.ackDelay)); - }, - [&](const WriteStreamFrame& frame) { - event->frames.push_back(std::make_unique( - frame.streamId, frame.offset, frame.len, frame.fin)); - }, - [&](const WriteCryptoFrame& frame) { - event->frames.push_back( - std::make_unique(frame.offset, frame.len)); - }, - [&](const QuicSimpleFrame& simpleFrame) { - addQuicSimpleFrameToEvent(event.get(), simpleFrame); - }, - [&](const auto& /* unused */) { - // Ignore other frames. - }); + switch (quicFrame.type()) { + case QuicWriteFrame::Type::PaddingFrame_E: + ++numPaddingFrames; + break; + case QuicWriteFrame::Type::RstStreamFrame_E: { + const RstStreamFrame& frame = *quicFrame.asRstStreamFrame(); + event->frames.push_back(std::make_unique( + frame.streamId, frame.errorCode, frame.offset)); + break; + } + case QuicWriteFrame::Type::ConnectionCloseFrame_E: { + const ConnectionCloseFrame& frame = *quicFrame.asConnectionCloseFrame(); + event->frames.push_back(std::make_unique( + frame.errorCode, frame.reasonPhrase, frame.closingFrameType)); + break; + } + case QuicWriteFrame::Type::ApplicationCloseFrame_E: { + const ApplicationCloseFrame& frame = + *quicFrame.asApplicationCloseFrame(); + event->frames.push_back(std::make_unique( + frame.errorCode, frame.reasonPhrase)); + break; + } + case QuicWriteFrame::Type::MaxDataFrame_E: { + const MaxDataFrame& frame = *quicFrame.asMaxDataFrame(); + event->frames.push_back( + std::make_unique(frame.maximumData)); + break; + } + case QuicWriteFrame::Type::MaxStreamDataFrame_E: { + const MaxStreamDataFrame& frame = *quicFrame.asMaxStreamDataFrame(); + event->frames.push_back(std::make_unique( + frame.streamId, frame.maximumData)); + break; + } + case QuicWriteFrame::Type::StreamsBlockedFrame_E: { + const StreamsBlockedFrame& frame = *quicFrame.asStreamsBlockedFrame(); + event->frames.push_back(std::make_unique( + frame.streamLimit, frame.isForBidirectional)); + break; + } + case QuicWriteFrame::Type::PingFrame_E: + event->frames.push_back(std::make_unique()); + break; + case QuicWriteFrame::Type::DataBlockedFrame_E: { + const DataBlockedFrame& frame = *quicFrame.asDataBlockedFrame(); + event->frames.push_back( + std::make_unique(frame.dataLimit)); + break; + } + case QuicWriteFrame::Type::StreamDataBlockedFrame_E: { + const StreamDataBlockedFrame& frame = *quicFrame.asStreamDataBlockedFrame(); + event->frames.push_back(std::make_unique( + frame.streamId, frame.dataLimit)); + break; + } + case QuicWriteFrame::Type::WriteAckFrame_E: { + const WriteAckFrame& frame = *quicFrame.asWriteAckFrame(); + event->frames.push_back(std::make_unique( + frame.ackBlocks, frame.ackDelay)); + break; + } + case QuicWriteFrame::Type::WriteStreamFrame_E: { + const WriteStreamFrame& frame = *quicFrame.asWriteStreamFrame(); + event->frames.push_back(std::make_unique( + frame.streamId, frame.offset, frame.len, frame.fin)); + break; + } + case QuicWriteFrame::Type::WriteCryptoFrame_E: { + const WriteCryptoFrame& frame = *quicFrame.asWriteCryptoFrame(); + event->frames.push_back( + std::make_unique(frame.offset, frame.len)); + break; + } + case QuicWriteFrame::Type::QuicSimpleFrame_E: { + const QuicSimpleFrame& simpleFrame = *quicFrame.asQuicSimpleFrame(); + addQuicSimpleFrameToEvent(event.get(), simpleFrame); + break; + } + default: + break; + } } if (numPaddingFrames > 0) { event->frames.push_back( diff --git a/quic/loss/QuicLossFunctions.cpp b/quic/loss/QuicLossFunctions.cpp index d8068e214..9a5f56336 100644 --- a/quic/loss/QuicLossFunctions.cpp +++ b/quic/loss/QuicLossFunctions.cpp @@ -57,138 +57,149 @@ void markPacketLoss( bool processed, PacketNum currentPacketNum) { for (auto& packetFrame : packet.frames) { - folly::variant_match( - packetFrame, - [&](MaxStreamDataFrame& frame) { - // For all other frames, we process it if it's not from a clone - // packet, or if the clone and its siblings have never been processed. - // But for both MaxData and MaxStreamData, clone and its siblings may - // have different values. So we process it if it matches the - // latestMaxDataPacket or latestMaxStreamDataPacket. If an older - // packet also has such frames, it's ok to skip process of such loss - // since newer value is already sent in later packets. - auto stream = conn.streamManager->getStream(frame.streamId); - if (!stream) { - return; - } - // TODO: check for the stream is in Open or HalfClosedLocal state, the - // peer doesn't need a flow control update in these cases. - if (stream->latestMaxStreamDataPacket == currentPacketNum) { - onStreamWindowUpdateLost(*stream); - } - }, - [&](MaxDataFrame&) { - if (conn.latestMaxDataPacket == currentPacketNum) { - onConnWindowUpdateLost(conn); - } - }, - // For other frame types, we only process them if the packet is not a - // processed clone. - [&](WriteStreamFrame& frame) { - if (processed) { - return; - } - auto stream = conn.streamManager->getStream(frame.streamId); - if (!stream) { - return; - } - auto bufferItr = std::lower_bound( - stream->retransmissionBuffer.begin(), - stream->retransmissionBuffer.end(), - frame.offset, - [](const auto& buffer, const auto& offset) { - return buffer.offset < offset; - }); - if (bufferItr == stream->retransmissionBuffer.end()) { - // It's possible that the stream was reset or data on the stream was - // skipped while we discovered that its packet was lost so we might - // not have the offset. - return; - } - // The original rxmt offset might have been bumped up after it was - // shrunk due to egress partially reliable skip. - if (!ackFrameMatchesRetransmitBuffer(*stream, frame, *bufferItr)) { - return; - } - stream->lossBuffer.insert( - std::upper_bound( - stream->lossBuffer.begin(), - stream->lossBuffer.end(), - bufferItr->offset, - [](const auto& offset, const auto& buffer) { - return offset < buffer.offset; - }), - std::move(*bufferItr)); - stream->retransmissionBuffer.erase(bufferItr); - conn.streamManager->updateLossStreams(*stream); - }, - [&](WriteCryptoFrame& frame) { - if (processed) { - return; - } - auto protectionType = packet.header.getProtectionType(); - auto encryptionLevel = - protectionTypeToEncryptionLevel(protectionType); - auto cryptoStream = - getCryptoStream(*conn.cryptoState, encryptionLevel); + switch (packetFrame.type()) { + case QuicWriteFrame::Type::MaxStreamDataFrame_E: { + MaxStreamDataFrame& frame = *packetFrame.asMaxStreamDataFrame(); + // For all other frames, we process it if it's not from a clone + // packet, or if the clone and its siblings have never been processed. + // But for both MaxData and MaxStreamData, clone and its siblings may + // have different values. So we process it if it matches the + // latestMaxDataPacket or latestMaxStreamDataPacket. If an older + // packet also has such frames, it's ok to skip process of such loss + // since newer value is already sent in later packets. + auto stream = conn.streamManager->getStream(frame.streamId); + if (!stream) { + break; + } + // TODO: check for the stream is in Open or HalfClosedLocal state, the + // peer doesn't need a flow control update in these cases. + if (stream->latestMaxStreamDataPacket == currentPacketNum) { + onStreamWindowUpdateLost(*stream); + } + break; + } + case QuicWriteFrame::Type::MaxDataFrame_E: { + if (conn.latestMaxDataPacket == currentPacketNum) { + onConnWindowUpdateLost(conn); + } + break; + } + // For other frame types, we only process them if the packet is not a + // processed clone. + case QuicWriteFrame::Type::WriteStreamFrame_E: { + WriteStreamFrame frame = *packetFrame.asWriteStreamFrame(); + if (processed) { + break; + } + auto stream = conn.streamManager->getStream(frame.streamId); + if (!stream) { + break; + } + auto bufferItr = std::lower_bound( + stream->retransmissionBuffer.begin(), + stream->retransmissionBuffer.end(), + frame.offset, + [](const auto& buffer, const auto& offset) { + return buffer.offset < offset; + }); + if (bufferItr == stream->retransmissionBuffer.end()) { + // It's possible that the stream was reset or data on the stream was + // skipped while we discovered that its packet was lost so we might + // not have the offset. + break; + } + // The original rxmt offset might have been bumped up after it was + // shrunk due to egress partially reliable skip. + if (!ackFrameMatchesRetransmitBuffer(*stream, frame, *bufferItr)) { + break; + } + stream->lossBuffer.insert( + std::upper_bound( + stream->lossBuffer.begin(), + stream->lossBuffer.end(), + bufferItr->offset, + [](const auto& offset, const auto& buffer) { + return offset < buffer.offset; + }), + std::move(*bufferItr)); + stream->retransmissionBuffer.erase(bufferItr); + conn.streamManager->updateLossStreams(*stream); + break; + } + case QuicWriteFrame::Type::WriteCryptoFrame_E: { + WriteCryptoFrame& frame = *packetFrame.asWriteCryptoFrame(); + if (processed) { + break; + } + auto protectionType = packet.header.getProtectionType(); + auto encryptionLevel = protectionTypeToEncryptionLevel(protectionType); + auto cryptoStream = getCryptoStream(*conn.cryptoState, encryptionLevel); - auto bufferItr = std::lower_bound( - cryptoStream->retransmissionBuffer.begin(), - cryptoStream->retransmissionBuffer.end(), - frame.offset, - [](const auto& buffer, const auto& offset) { - return buffer.offset < offset; - }); - if (bufferItr == cryptoStream->retransmissionBuffer.end()) { - // It's possible that the stream was reset while we discovered that - // it's packet was lost so we might not have the offset. - return; - } - DCHECK_EQ(bufferItr->offset, frame.offset); - cryptoStream->lossBuffer.insert( - std::upper_bound( - cryptoStream->lossBuffer.begin(), - cryptoStream->lossBuffer.end(), - bufferItr->offset, - [](const auto& offset, const auto& buffer) { - return offset < buffer.offset; - }), - std::move(*bufferItr)); - cryptoStream->retransmissionBuffer.erase(bufferItr); - }, - [&](RstStreamFrame& frame) { - if (processed) { - return; - } - auto stream = conn.streamManager->getStream(frame.streamId); - if (!stream) { - // If the stream is dead, ignore the retransmissions of the rst - // stream. - return; - } - // Add the lost RstStreamFrame back to pendingEvents: - conn.pendingEvents.resets.insert({frame.streamId, frame}); - }, - [&](StreamDataBlockedFrame& frame) { - if (processed) { - return; - } - auto stream = conn.streamManager->getStream(frame.streamId); - // TODO: check for retransmittable - if (!stream) { - return; - } - onBlockedLost(*stream); - }, - [&](QuicSimpleFrame& frame) { - if (processed) { - return; - } - updateSimpleFrameOnPacketLoss(conn, frame); - }, - [&](auto&) { - // ignore the rest of the frames. - }); + auto bufferItr = std::lower_bound( + cryptoStream->retransmissionBuffer.begin(), + cryptoStream->retransmissionBuffer.end(), + frame.offset, + [](const auto& buffer, const auto& offset) { + return buffer.offset < offset; + }); + if (bufferItr == cryptoStream->retransmissionBuffer.end()) { + // It's possible that the stream was reset while we discovered that + // it's packet was lost so we might not have the offset. + break; + } + DCHECK_EQ(bufferItr->offset, frame.offset); + cryptoStream->lossBuffer.insert( + std::upper_bound( + cryptoStream->lossBuffer.begin(), + cryptoStream->lossBuffer.end(), + bufferItr->offset, + [](const auto& offset, const auto& buffer) { + return offset < buffer.offset; + }), + std::move(*bufferItr)); + cryptoStream->retransmissionBuffer.erase(bufferItr); + break; + } + case QuicWriteFrame::Type::RstStreamFrame_E: { + RstStreamFrame& frame = *packetFrame.asRstStreamFrame(); + if (processed) { + break; + } + auto stream = conn.streamManager->getStream(frame.streamId); + if (!stream) { + // If the stream is dead, ignore the retransmissions of the rst + // stream. + break; + } + // Add the lost RstStreamFrame back to pendingEvents: + conn.pendingEvents.resets.insert({frame.streamId, frame}); + break; + } + case QuicWriteFrame::Type::StreamDataBlockedFrame_E: { + StreamDataBlockedFrame& frame = *packetFrame.asStreamDataBlockedFrame(); + if (processed) { + break; + } + auto stream = conn.streamManager->getStream(frame.streamId); + // TODO: check for retransmittable + if (!stream) { + break; + } + onBlockedLost(*stream); + break; + } + case QuicWriteFrame::Type::QuicSimpleFrame_E: { + QuicSimpleFrame& frame = *packetFrame.asQuicSimpleFrame(); + if (processed) { + break; + } + updateSimpleFrameOnPacketLoss(conn, frame); + break; + } + default: + // ignore the rest of the frames. + break; + } } } } // namespace quic diff --git a/quic/loss/test/QuicLossFunctionsTest.cpp b/quic/loss/test/QuicLossFunctionsTest.cpp index cb63ce001..0c7e77f8f 100644 --- a/quic/loss/test/QuicLossFunctionsTest.cpp +++ b/quic/loss/test/QuicLossFunctionsTest.cpp @@ -696,10 +696,14 @@ TEST_F(QuicLossFunctionsTest, TestMarkRstLoss) { auto& packet2 = getLastOutstandingPacket(*conn, PacketNumberSpace::AppData)->packet; bool rstFound = false; - for (auto& frame : all_frames(packet2.frames)) { - EXPECT_EQ(stream->id, frame.streamId); - EXPECT_EQ(GenericApplicationErrorCode::UNKNOWN, frame.errorCode); - EXPECT_EQ(currentOffset, frame.offset); + for (auto& frame : packet2.frames) { + auto resetFrame = frame.asRstStreamFrame(); + if (!resetFrame) { + continue; + } + EXPECT_EQ(stream->id, resetFrame->streamId); + EXPECT_EQ(GenericApplicationErrorCode::UNKNOWN, resetFrame->errorCode); + EXPECT_EQ(currentOffset, resetFrame->offset); rstFound = true; } EXPECT_TRUE(rstFound); @@ -1235,12 +1239,19 @@ TEST_F(QuicLossFunctionsTest, TestMarkPacketLossProcessedPacket) { for (const auto& frame : getLastOutstandingPacket(*conn, PacketNumberSpace::AppData) ->packet.frames) { - folly::variant_match( - frame, - [&](const WriteStreamFrame&) { streamDataCounter++; }, - [&](const MaxStreamDataFrame&) { streamWindowUpdateCounter++; }, - [&](const MaxDataFrame&) { connWindowUpdateCounter++; }, - [](const auto&) { ASSERT_TRUE(false); }); + switch (frame.type()) { + case QuicWriteFrame::Type::WriteStreamFrame_E: + streamDataCounter++; + break; + case QuicWriteFrame::Type::MaxStreamDataFrame_E: + streamWindowUpdateCounter++; + break; + case QuicWriteFrame::Type::MaxDataFrame_E: + connWindowUpdateCounter++; + break; + default: + CHECK(false) << "unexpected frame=" << (int)frame.type(); + } } EXPECT_EQ(1, streamDataCounter); EXPECT_EQ(1, streamWindowUpdateCounter); diff --git a/quic/server/state/ServerStateMachine.cpp b/quic/server/state/ServerStateMachine.cpp index 5802fa09a..700ea88f8 100644 --- a/quic/server/state/ServerStateMachine.cpp +++ b/quic/server/state/ServerStateMachine.cpp @@ -756,47 +756,56 @@ void onServerReadDataFromOpen( [&](const OutstandingPacket&, const QuicWriteFrame& packetFrame, const ReadAckFrame&) { - folly::variant_match( - packetFrame, - [&](const WriteStreamFrame& frame) { - VLOG(4) - << "Server received ack for stream=" << frame.streamId - << " offset=" << frame.offset << " fin=" << frame.fin - << " len=" << frame.len << " " << conn; - auto ackedStream = - conn.streamManager->getStream(frame.streamId); - if (ackedStream) { - invokeStreamSendStateMachine( - conn, - *ackedStream, - StreamEvents::AckStreamFrame(frame)); - } - }, - [&](const WriteCryptoFrame& frame) { - auto cryptoStream = - getCryptoStream(*conn.cryptoState, encryptionLevel); - processCryptoStreamAck( - *cryptoStream, frame.offset, frame.len); - }, - [&](const RstStreamFrame& frame) { - VLOG(4) << "Server received ack for reset stream=" - << frame.streamId << " " << conn; - auto stream = - conn.streamManager->getStream(frame.streamId); - if (stream) { - invokeStreamSendStateMachine( - conn, *stream, StreamEvents::RstAck(frame)); - } - }, - [&](const WriteAckFrame& frame) { - DCHECK(!frame.ackBlocks.empty()); - VLOG(4) << "Server received ack for largestAcked=" - << frame.ackBlocks.back().end << " " << conn; - commonAckVisitorForAckFrame(ackState, frame); - }, - [&](const auto& /*frame*/) { - // Ignore other frames. - }); + switch (packetFrame.type()) { + case QuicWriteFrame::Type::WriteStreamFrame_E: { + const WriteStreamFrame& frame = + *packetFrame.asWriteStreamFrame(); + VLOG(4) + << "Server received ack for stream=" << frame.streamId + << " offset=" << frame.offset << " fin=" << frame.fin + << " len=" << frame.len << " " << conn; + auto ackedStream = + conn.streamManager->getStream(frame.streamId); + if (ackedStream) { + invokeStreamSendStateMachine( + conn, + *ackedStream, + StreamEvents::AckStreamFrame(frame)); + } + break; + } + case QuicWriteFrame::Type::WriteCryptoFrame_E: { + const WriteCryptoFrame& frame = + *packetFrame.asWriteCryptoFrame(); + auto cryptoStream = + getCryptoStream(*conn.cryptoState, encryptionLevel); + processCryptoStreamAck( + *cryptoStream, frame.offset, frame.len); + break; + } + case QuicWriteFrame::Type::RstStreamFrame_E: { + const RstStreamFrame& frame = *packetFrame.asRstStreamFrame(); + VLOG(4) << "Server received ack for reset stream=" + << frame.streamId << " " << conn; + auto stream = conn.streamManager->getStream(frame.streamId); + if (stream) { + invokeStreamSendStateMachine( + conn, *stream, StreamEvents::RstAck(frame)); + } + break; + } + case QuicWriteFrame::Type::WriteAckFrame_E: { + const WriteAckFrame& frame = *packetFrame.asWriteAckFrame(); + DCHECK(!frame.ackBlocks.empty()); + VLOG(4) << "Server received ack for largestAcked=" + << frame.ackBlocks.back().end << " " << conn; + commonAckVisitorForAckFrame(ackState, frame); + break; + } + default: { + break; + } + } }, markPacketLoss, readData.networkData.receiveTimePoint); diff --git a/quic/server/test/QuicServerTransportTest.cpp b/quic/server/test/QuicServerTransportTest.cpp index 5706bf18e..9e9261a1e 100644 --- a/quic/server/test/QuicServerTransportTest.cpp +++ b/quic/server/test/QuicServerTransportTest.cpp @@ -1116,7 +1116,7 @@ TEST_F(QuicServerTransportTest, TestOpenAckStreamFrame) { PacketNum currentPacket = packet.packet.header.getPacketSequenceNum(); ASSERT_FALSE(packet.packet.frames.empty()); for (auto& quicFrame : packet.packet.frames) { - auto frame = boost::get(&quicFrame); + auto frame = quicFrame.asWriteStreamFrame(); if (!frame) { continue; } @@ -1374,7 +1374,7 @@ TEST_F(QuicServerTransportTest, RecvStopSendingFrame) { StopSendingFrame stopSendingFrame( streamId, GenericApplicationErrorCode::UNKNOWN); ASSERT_TRUE(builder.canBuildPacket()); - writeFrame(std::move(stopSendingFrame), builder); + writeFrame(QuicSimpleFrame(stopSendingFrame), builder); auto packet = std::move(builder).buildPacket(); EXPECT_CALL( connCallback, @@ -1416,7 +1416,7 @@ TEST_F(QuicServerTransportTest, RecvStopSendingFrameAfterCloseStream) { StopSendingFrame stopSendingFrame( streamId, GenericApplicationErrorCode::UNKNOWN); ASSERT_TRUE(builder.canBuildPacket()); - writeFrame(std::move(stopSendingFrame), builder); + writeFrame(QuicSimpleFrame(stopSendingFrame), builder); auto packet = std::move(builder).buildPacket(); server->resetStream(streamId, GenericApplicationErrorCode::UNKNOWN); EXPECT_CALL(connCallback, onStopSending(_, _)).Times(0); @@ -1499,7 +1499,7 @@ TEST_F(QuicServerTransportTest, RecvStopSendingFrameAfterHalfCloseRemote) { builder, 0x00, stream->currentReadOffset, 0, 10, true); ASSERT_TRUE(dataLen.hasValue()); ASSERT_EQ(*dataLen, 0); - writeFrame(std::move(stopSendingFrame), builder); + writeFrame(QuicSimpleFrame(stopSendingFrame), builder); auto packet = std::move(builder).buildPacket(); EXPECT_CALL( connCallback, @@ -1523,7 +1523,7 @@ TEST_F(QuicServerTransportTest, RecvStopSendingBeforeStream) { StopSendingFrame stopSendingFrame( streamId, GenericApplicationErrorCode::UNKNOWN); ASSERT_TRUE(builder.canBuildPacket()); - writeFrame(std::move(stopSendingFrame), builder); + writeFrame(QuicSimpleFrame(stopSendingFrame), builder); auto packet = std::move(builder).buildPacket(); EXPECT_CALL(connCallback, onNewBidirectionalStream(streamId)); EXPECT_CALL( @@ -1578,8 +1578,8 @@ TEST_F(QuicServerTransportTest, RecvStopSendingFrameAfterReset) { StopSendingFrame stopSendingFrame2( streamId2, GenericApplicationErrorCode::UNKNOWN); ASSERT_TRUE(builder.canBuildPacket()); - writeFrame(std::move(stopSendingFrame1), builder); - writeFrame(std::move(stopSendingFrame2), builder); + writeFrame(QuicSimpleFrame(stopSendingFrame1), builder); + writeFrame(QuicSimpleFrame(stopSendingFrame2), builder); auto packet = std::move(builder).buildPacket(); EXPECT_CALL( connCallback, onStopSending(_, GenericApplicationErrorCode::UNKNOWN)) @@ -1605,7 +1605,7 @@ TEST_F(QuicServerTransportTest, StopSendingLoss) { StopSendingFrame stopSendingFrame( streamId, GenericApplicationErrorCode::UNKNOWN); ASSERT_TRUE(builder.canBuildPacket()); - writeFrame(stopSendingFrame, builder); + writeFrame(QuicSimpleFrame(stopSendingFrame), builder); auto packet = std::move(builder).buildPacket(); markPacketLoss( server->getNonConstConn(), @@ -1635,7 +1635,7 @@ TEST_F(QuicServerTransportTest, StopSendingLossAfterStreamClosed) { StopSendingFrame stopSendingFrame( streamId, GenericApplicationErrorCode::UNKNOWN); ASSERT_TRUE(builder.canBuildPacket()); - writeFrame(stopSendingFrame, builder); + writeFrame(QuicSimpleFrame(stopSendingFrame), builder); auto packet = std::move(builder).buildPacket(); // clear out all the streams, this is not a great way to simulate closed diff --git a/quic/state/stream/test/StreamStateMachineTest.cpp b/quic/state/stream/test/StreamStateMachineTest.cpp index 94728fb90..9801b70f3 100644 --- a/quic/state/stream/test/StreamStateMachineTest.cpp +++ b/quic/state/stream/test/StreamStateMachineTest.cpp @@ -205,9 +205,10 @@ TEST_F(QuicOpenStateTest, AckStream) { EXPECT_EQ(stream->retransmissionBuffer.size(), 1); EXPECT_EQ(1, conn->outstandingPackets.size()); - auto& streamFrame = boost::get( - getFirstOutstandingPacket(*conn, PacketNumberSpace::AppData) - ->packet.frames.front()); + auto& streamFrame = + *getFirstOutstandingPacket(*conn, PacketNumberSpace::AppData) + ->packet.frames.front() + .asWriteStreamFrame(); StreamEvents::AckStreamFrame ack(streamFrame); invokeHandler(stream->send, ack, *stream); @@ -256,8 +257,9 @@ TEST_F(QuicOpenStateTest, RetxBufferSortedAfterAck) { EXPECT_EQ(3, stream->retransmissionBuffer.size()); EXPECT_EQ(3, conn->outstandingPackets.size()); auto packet = conn->outstandingPackets[folly::Random::rand32() % 3]; - auto streamFrame = boost::get( - conn->outstandingPackets[std::rand() % 3].packet.frames.front()); + auto streamFrame = *conn->outstandingPackets[std::rand() % 3] + .packet.frames.front() + .asWriteStreamFrame(); StreamEvents::AckStreamFrame ack(streamFrame); invokeHandler(stream->send, ack, *stream); EXPECT_EQ(2, stream->retransmissionBuffer.size()); @@ -289,9 +291,10 @@ TEST_F(QuicOpenStateTest, AckStreamAfterSkip) { EXPECT_EQ(stream->retransmissionBuffer.size(), 1); EXPECT_EQ(1, conn->outstandingPackets.size()); - auto& streamFrame = boost::get( - getFirstOutstandingPacket(*conn, PacketNumberSpace::AppData) - ->packet.frames.front()); + auto& streamFrame = + *getFirstOutstandingPacket(*conn, PacketNumberSpace::AppData) + ->packet.frames.front() + .asWriteStreamFrame(); PacketNum packetNum(1); MinStreamDataFrame minDataFrame(stream->id, 1000, 100); @@ -333,9 +336,10 @@ TEST_F(QuicOpenStateTest, AckStreamAfterSkipHalfBuf) { EXPECT_EQ(stream->retransmissionBuffer.size(), 1); EXPECT_EQ(1, conn->outstandingPackets.size()); - auto& streamFrame = boost::get( - getFirstOutstandingPacket(*conn, PacketNumberSpace::AppData) - ->packet.frames.front()); + auto& streamFrame = + *getFirstOutstandingPacket(*conn, PacketNumberSpace::AppData) + ->packet.frames.front() + .asWriteStreamFrame(); PacketNum packetNum(1); // Skip ~0.5 buffers. @@ -387,11 +391,11 @@ TEST_F(QuicOpenStateTest, AckStreamAfterSkipOneAndAHalfBuf) { auto streamFrameIt = getFirstOutstandingPacket(*conn, PacketNumberSpace::AppData); auto& streamFrame1 = - boost::get(streamFrameIt->packet.frames.front()); - auto& streamFrame2 = boost::get( - getNextOutstandingPacket( - *conn, PacketNumberSpace::AppData, ++streamFrameIt) - ->packet.frames.front()); + *streamFrameIt->packet.frames.front().asWriteStreamFrame(); + auto& streamFrame2 = *getNextOutstandingPacket( + *conn, PacketNumberSpace::AppData, ++streamFrameIt) + ->packet.frames.front() + .asWriteStreamFrame(); PacketNum packetNum(1); // Skip ~1.5 buffers. @@ -502,9 +506,10 @@ TEST_F(QuicHalfClosedRemoteStateTest, AckStream) { EXPECT_EQ(stream->retransmissionBuffer.size(), 1); EXPECT_EQ(1, conn->outstandingPackets.size()); - auto& streamFrame = boost::get( - getFirstOutstandingPacket(*conn, PacketNumberSpace::AppData) - ->packet.frames.front()); + auto& streamFrame = + *getFirstOutstandingPacket(*conn, PacketNumberSpace::AppData) + ->packet.frames.front() + .asWriteStreamFrame(); StreamEvents::AckStreamFrame ack(streamFrame); invokeHandler(stream->send, ack, *stream); @@ -541,9 +546,10 @@ TEST_F(QuicHalfClosedRemoteStateTest, AckStreamAfterSkip) { EXPECT_EQ(stream->retransmissionBuffer.size(), 1); EXPECT_EQ(1, conn->outstandingPackets.size()); - auto& streamFrame = boost::get( - getFirstOutstandingPacket(*conn, PacketNumberSpace::AppData) - ->packet.frames.front()); + auto& streamFrame = + *getFirstOutstandingPacket(*conn, PacketNumberSpace::AppData) + ->packet.frames.front() + .asWriteStreamFrame(); PacketNum packetNum(1); MinStreamDataFrame minDataFrame(stream->id, 1000, 100); diff --git a/quic/state/test/AckHandlersTest.cpp b/quic/state/test/AckHandlersTest.cpp index dc794f784..4f3e07e2d 100644 --- a/quic/state/test/AckHandlersTest.cpp +++ b/quic/state/test/AckHandlersTest.cpp @@ -81,7 +81,7 @@ TEST_P(AckHandlersTest, TestAckMultipleSequentialBlocks) { GetParam(), ackFrame, [&](const auto&, const auto& packetFrame, const ReadAckFrame&) { - auto& stream = boost::get(packetFrame); + auto& stream = *packetFrame.asWriteStreamFrame(); streams.emplace_back(stream); }, testLossHandler(lostPackets), @@ -152,7 +152,7 @@ TEST_P(AckHandlersTest, TestAckBlocksWithGaps) { GetParam(), ackFrame, [&](const auto&, const auto& packetFrame, const ReadAckFrame&) { - auto& stream = boost::get(packetFrame); + auto& stream = *packetFrame.asWriteStreamFrame(); streams.emplace_back(stream); }, testLossHandler(lostPackets), @@ -259,7 +259,7 @@ TEST_P(AckHandlersTest, TestNonSequentialPacketNumbers) { GetParam(), ackFrame, [&](const auto&, const auto& packetFrame, const ReadAckFrame&) { - auto& stream = boost::get(packetFrame); + auto& stream = *packetFrame.asWriteStreamFrame(); streams.emplace_back(stream); }, testLossHandler(lostPackets), @@ -337,15 +337,10 @@ TEST_P(AckHandlersTest, AckVisitorForAckTest) { auto ackedPacketNum = outstandingPacket.packet.header.getPacketSequenceNum(); EXPECT_EQ(ackedPacketNum, firstReceivedAck.largestAcked); - folly::variant_match( - packetFrame, - [&](const WriteAckFrame& frame) { - commonAckVisitorForAckFrame( - conn.ackStates.appDataAckState, frame); - }, - [&](const auto& /* frame */) { - // Ignore other frames. - }); + const WriteAckFrame* frame = packetFrame.asWriteAckFrame(); + if (frame) { + commonAckVisitorForAckFrame(conn.ackStates.appDataAckState, *frame); + } }, [](auto& /* conn */, auto& /* packet */, @@ -368,15 +363,10 @@ TEST_P(AckHandlersTest, AckVisitorForAckTest) { GetParam(), secondReceivedAck, [&](const auto&, const auto& packetFrame, const ReadAckFrame&) { - folly::variant_match( - packetFrame, - [&](const WriteAckFrame& frame) { - commonAckVisitorForAckFrame( - conn.ackStates.appDataAckState, frame); - }, - [&](const auto& /* frame */) { - // Ignore other frames. - }); + const WriteAckFrame* frame = packetFrame.asWriteAckFrame(); + if (frame) { + commonAckVisitorForAckFrame(conn.ackStates.appDataAckState, *frame); + } }, [](auto& /* conn */, auto& /* packet */,