diff --git a/quic/QuicConstants.h b/quic/QuicConstants.h index 81bd4b01b..6f280736d 100644 --- a/quic/QuicConstants.h +++ b/quic/QuicConstants.h @@ -438,8 +438,6 @@ constexpr uint16_t kDefaultRxPacketsBeforeAckBeforeInit = 10; constexpr uint16_t kDefaultRxPacketsBeforeAckAfterInit = 10; /* Ack timer */ -// TODO: These numbers are shamlessly taken from Chromium code. We have no idea -// how good/bad this is. // Ack timeout = SRTT * kAckTimerFactor constexpr double kAckTimerFactor = 0.25; // max ack timeout: 25ms diff --git a/quic/api/IoBufQuicBatch.cpp b/quic/api/IoBufQuicBatch.cpp index 430da3f36..165ba75ed 100644 --- a/quic/api/IoBufQuicBatch.cpp +++ b/quic/api/IoBufQuicBatch.cpp @@ -97,9 +97,6 @@ bool IOBufQuicBatch::flushInternal() { folly::Optional secondSocketErrno; if (happyEyeballsState_.shouldWriteToSecondSocket) { - // TODO: if the errno is EMSGSIZE, and we move on with the second socket, - // we actually miss the chance to fix our UDP packet size with the first - // socket. auto consumed = batchWriter_->write( *happyEyeballsState_.secondSocket, happyEyeballsState_.secondPeerAddress); @@ -160,9 +157,8 @@ bool IOBufQuicBatch::flushInternal() { } if (!written) { - // This can happen normally, so ignore for now. Now we treat EAGAIN same + // This can happen normally, so ignore. Now we treat most errors same // as a loss to avoid looping. - // TODO: Remove once we use write event from libevent. return false; // done } diff --git a/quic/api/QuicPacketScheduler.cpp b/quic/api/QuicPacketScheduler.cpp index 8bdfc1715..fec882def 100644 --- a/quic/api/QuicPacketScheduler.cpp +++ b/quic/api/QuicPacketScheduler.cpp @@ -242,7 +242,7 @@ SchedulingResult FrameScheduler::scheduleFramesForPacket( if (rstScheduler_ && rstScheduler_->hasPendingRsts()) { rstWritten = rstScheduler_->writeRsts(wrapper); } - // TODO: Long time ago we decided RST has higher priority than Acks. Why tho? + // Long time ago we decided RST has higher priority than Acks. if (ackScheduler_ && ackScheduler_->hasPendingAcks()) { if (cryptoDataWritten || rstWritten) { // If packet has non ack data, it is subject to congestion control. We @@ -570,7 +570,6 @@ bool RstStreamScheduler::hasPendingRsts() const { bool RstStreamScheduler::writeRsts(PacketBuilderInterface& builder) { bool rstWritten = false; for (const auto& resetStream : conn_.pendingEvents.resets) { - // TODO: here, maybe coordinate scheduling of RST_STREAMS and streams. auto bytesWritten = writeFrame(resetStream.second, builder); if (!bytesWritten) { break; diff --git a/quic/api/QuicPacketScheduler.h b/quic/api/QuicPacketScheduler.h index 14d5ea898..4c9ca144a 100644 --- a/quic/api/QuicPacketScheduler.h +++ b/quic/api/QuicPacketScheduler.h @@ -323,9 +323,6 @@ class CloningScheduler : public QuicPacketScheduler { public: // Normally a scheduler takes in a const conn, and update conn later. But for // this one I want to update conn right inside this class itself. - // TODO: Passing cipherOverhead into the CloningScheduler to recalculate the - // correct writableBytes isn't ideal. But unblock me or others from quickly - // testing it on load test. :( CloningScheduler( FrameScheduler& scheduler, QuicConnectionStateBase& conn, diff --git a/quic/api/QuicTransportBase.cpp b/quic/api/QuicTransportBase.cpp index c849f0878..9a29abe53 100644 --- a/quic/api/QuicTransportBase.cpp +++ b/quic/api/QuicTransportBase.cpp @@ -206,7 +206,6 @@ void QuicTransportBase::closeNow( // the drain timeout may have been scheduled by a previous close, in which // case, our close would not take effect. This cancels the drain timeout in // this case and expires the timeout. - // TODO: fix this in a better way. if (drainTimeout_.isScheduled()) { drainTimeout_.cancelTimeout(); drainTimeoutExpired(); @@ -365,7 +364,6 @@ void QuicTransportBase::closeImpl( peekLooper_->stop(); writeLooper_->stop(); - // TODO: invoke connection close callbacks. cancelAllAppCallbacks(cancelCode); // Clear out all the pending events, we don't need them any more. @@ -1487,8 +1485,6 @@ void QuicTransportBase::processCallbacksAfterNetworkData() { handleKnobCallbacks(); - // TODO: we're currently assuming that canceling write callbacks will not - // cause reset of random streams. Maybe get rid of that assumption later. for (auto pendingResetIt = conn_->pendingEvents.resets.begin(); pendingResetIt != conn_->pendingEvents.resets.end(); pendingResetIt++) { @@ -1589,8 +1585,7 @@ void QuicTransportBase::processCallbacksAfterNetworkData() { } // If the connection flow control is unblocked, we might be unblocked - // on the streams now. TODO: maybe do this only when we know connection - // flow control changed. + // on the streams now. auto writeCallbackIt = pendingWriteCallbacks_.begin(); while (writeCallbackIt != pendingWriteCallbacks_.end()) { @@ -2293,7 +2288,6 @@ void QuicTransportBase::lossTimeoutExpired() noexcept { FOLLY_MAYBE_UNUSED auto self = sharedGuard(); try { onLossDetectionAlarm(*conn_, markPacketLoss); - // TODO: remove this trace when Pacing is ready to land if (conn_->qLogger) { conn_->qLogger->addTransportStateUpdate(kLossTimeoutExpired); } diff --git a/quic/api/QuicTransportFunctions.cpp b/quic/api/QuicTransportFunctions.cpp index 9fcd934f4..7bd1a00f9 100644 --- a/quic/api/QuicTransportFunctions.cpp +++ b/quic/api/QuicTransportFunctions.cpp @@ -103,9 +103,6 @@ WriteQuicDataResult writeQuicDataToSocketImpl( uint64_t packetLimit, bool exceptCryptoStream) { auto builder = ShortHeaderBuilder(); - // TODO: In FrameScheduler, Retx is prioritized over new data. We should - // add a flag to the Scheduler to control the priority between them and see - // which way is better. WriteQuicDataResult result; auto& packetsWritten = result.packetsWritten; auto& probesWritten = result.probesWritten; @@ -1203,7 +1200,6 @@ void writeCloseCommon( << " sent close packetNum=" << packetNum << " in space=" << pnSpace << " " << connection; // Increment the sequence number. - // TODO: Do not increase pn if write fails increaseNextPacketNum(connection, pnSpace); // best effort writing to the socket, ignore any errors. auto ret = sock.write(connection.peerAddress, packetBuf); @@ -1357,7 +1353,6 @@ uint64_t writeConnectionDataToSocket( writableBytes -= cipherOverhead; } - // TODO: Select a different DataPathFunc based on TransportSettings const auto& dataPlainFunc = connection.transportSettings.dataPathType == DataPathType::ChainedMemory ? iobufChainBasedBuildScheduleEncrypt diff --git a/quic/api/QuicTransportFunctions.h b/quic/api/QuicTransportFunctions.h index 50b24a538..7fbdb1fd9 100644 --- a/quic/api/QuicTransportFunctions.h +++ b/quic/api/QuicTransportFunctions.h @@ -148,7 +148,6 @@ uint64_t writeZeroRttDataToSocket( /** * Whether we should and can write data. * - * TODO: We should probably split "should" and "can" into two APIs. */ WriteDataReason shouldWriteData(const QuicConnectionStateBase& conn); bool hasAckDataToWrite(const QuicConnectionStateBase& conn); diff --git a/quic/client/QuicClientTransport.cpp b/quic/client/QuicClientTransport.cpp index 348cf1ac3..6486014a8 100644 --- a/quic/client/QuicClientTransport.cpp +++ b/quic/client/QuicClientTransport.cpp @@ -83,9 +83,6 @@ QuicClientTransport::QuicClientTransport( conn_->readCodec->setCodecParameters(CodecParameters( conn_->peerAckDelayExponent, conn_->originalVersion.value())); - // TODO: generate this once we can generate the packet sequence number - // correctly. - // conn_->nextSequenceNum = folly::Random::secureRandom(); VLOG(10) << "client created " << *conn_; } @@ -331,9 +328,6 @@ void QuicClientTransport::processPacketData( // We got a packet that was not the version negotiation packet, that means // that the version is now bound to the new packet. - // TODO: move this into the state machine. - // TODO: get this from the crypto layer instead. This would be a security vuln - // if we don't. if (!conn_->version) { conn_->version = conn_->originalVersion; } @@ -752,8 +746,6 @@ void QuicClientTransport::onReadData( NetworkDataSingle&& networkData) { if (closeState_ == CloseState::CLOSED) { // If we are closed, then we shoudn't process new network data. - // TODO: we might want to process network data if we decide that we should - // exit draining state early QUIC_STATS( statsCallback_, onPacketDropped, PacketDropReason::CLIENT_STATE_CLOSED); if (conn_->qLogger) { @@ -791,9 +783,6 @@ void QuicClientTransport::onReadData( } void QuicClientTransport::writeData() { - // TODO: replace with write in state machine. - // TODO: change to draining when we move the client to have a draining state - // as well. QuicVersion version = conn_->version.value_or(*conn_->originalVersion); const ConnectionId& srcConnId = *conn_->clientConnectionId; const ConnectionId* destConnId = @@ -950,11 +939,7 @@ void QuicClientTransport::writeData() { void QuicClientTransport::startCryptoHandshake() { auto self = this->shared_from_this(); - // Set idle timer whenever crypto starts so that we can restart the idle timer - // after a version negotiation as well. setIdleTimer(); - // TODO: no need to close the transport if there is an error in the - // handshake. // We need to update the flow control settings every time we start a crypto // handshake. This is so that we can reset the flow control settings when // we go through version negotiation as well. diff --git a/quic/client/state/ClientStateMachine.cpp b/quic/client/state/ClientStateMachine.cpp index da248f312..9bb882cfd 100644 --- a/quic/client/state/ClientStateMachine.cpp +++ b/quic/client/state/ClientStateMachine.cpp @@ -150,7 +150,6 @@ void processServerInitialParams( maxStreamDataBidiRemote.value_or(0); conn.flowControlState.peerAdvertisedInitialMaxStreamOffsetUni = maxStreamDataUni.value_or(0); - // TODO Make idleTimeout disableable via transport parameter. conn.streamManager->setMaxLocalBidirectionalStreams( maxStreamsBidi.value_or(0)); conn.peerAdvertisedInitialMaxStreamsBidi = maxStreamsBidi.value_or(0); @@ -166,7 +165,6 @@ void processServerInitialParams( } conn.peerAckDelayExponent = ackDelayExponent.value_or(kDefaultAckDelayExponent); - // TODO: udpSendPacketLen should also be limited by PMTU if (conn.transportSettings.canIgnorePathMTU) { if (*packetSize > kDefaultMaxUDPPayload) { *packetSize = kDefaultUDPSendPacketLen; diff --git a/quic/client/state/ClientStateMachine.h b/quic/client/state/ClientStateMachine.h index 27819fd99..1b0739552 100644 --- a/quic/client/state/ClientStateMachine.h +++ b/quic/client/state/ClientStateMachine.h @@ -77,8 +77,6 @@ struct QuicClientConnectionState : public QuicConnectionStateBase { handshakeFactory(std::move(handshakeFactoryIn)) { cryptoState = std::make_unique(); congestionController = std::make_unique(*this); - // TODO: this is wrong, it should be the handshake finish time. But i need - // a relatively sane time now to make the timestamps all sane. connectionTime = Clock::now(); originalVersion = QuicVersion::MVFST; DCHECK(handshakeFactory); diff --git a/quic/codec/Decode.cpp b/quic/codec/Decode.cpp index ec5eaa474..71ce7b1b6 100644 --- a/quic/codec/Decode.cpp +++ b/quic/codec/Decode.cpp @@ -569,7 +569,6 @@ NewConnectionIdFrame decodeNewConnectionIdFrame(folly::io::Cursor& cursor) { RetireConnectionIdFrame decodeRetireConnectionIdFrame( folly::io::Cursor& cursor) { - // TODO we parse this frame, but return NoopFrame. Add proper support for it! auto sequenceNum = decodeQuicInteger(cursor); if (!sequenceNum) { throw QuicTransportException( @@ -746,7 +745,6 @@ QuicFrame parseFrame( const CodecParameters& params) { folly::io::Cursor cursor(queue.front()); auto frameTypeInt = decodeQuicInteger(cursor); - // TODO add an new api to determine whether the frametype is encoded minimally if (!frameTypeInt) { throw QuicTransportException( "Invalid frame-type field", TransportErrorCode::FRAME_ENCODING_ERROR); diff --git a/quic/codec/QuicPacketBuilder.h b/quic/codec/QuicPacketBuilder.h index af0293f6b..c522a9f36 100644 --- a/quic/codec/QuicPacketBuilder.h +++ b/quic/codec/QuicPacketBuilder.h @@ -36,7 +36,6 @@ constexpr auto kLongHeaderHeaderSize = sizeof(uint8_t) /* Type bytes */ + kReservedPacketLenSize /* minimal size of length */ + kReservedPacketNumSize /* packet number */; -// TODO: i'm sure this isn't the optimal value: // Appender growth byte size for in PacketBuilder: constexpr size_t kAppenderGrowthSize = 100; diff --git a/quic/codec/QuicReadCodec.cpp b/quic/codec/QuicReadCodec.cpp index 51844c536..6dde47c2e 100644 --- a/quic/codec/QuicReadCodec.cpp +++ b/quic/codec/QuicReadCodec.cpp @@ -168,7 +168,6 @@ CodecResult QuicReadCodec::parseLongHeaderPacket( CipherUnavailable(std::move(currentPacketData), 0, protectionType)); } - // TODO: decrypt the long header. PacketNum expectedNextPacketNum = 0; folly::Optional largestReceivedPacketNum; switch (longHeaderTypeToProtectionType(type)) { diff --git a/quic/codec/QuicWriteCodec.cpp b/quic/codec/QuicWriteCodec.cpp index 79690146a..421836580 100644 --- a/quic/codec/QuicWriteCodec.cpp +++ b/quic/codec/QuicWriteCodec.cpp @@ -262,8 +262,6 @@ folly::Optional writeAckFrame( WriteAckFrame ackFrame; uint64_t spaceLeft = builder.remainingSpaceInPkt(); uint64_t beginningSpace = spaceLeft; - // Reserve enough space a full packet of ACKs with 2 byte varints. - // TODO is this a good heuristic? ackFrame.ackBlocks.reserve(spaceLeft / 4); // We could technically split the range if the size of the representation of diff --git a/quic/codec/Types.h b/quic/codec/Types.h index 222c90739..42a6cce8a 100644 --- a/quic/codec/Types.h +++ b/quic/codec/Types.h @@ -226,8 +226,6 @@ struct ReadCryptoFrame { : offset(offsetIn), data(folly::IOBuf::create(0)) {} // Stuff stored in a variant type needs to be copyable. - // TODO: can we make this copyable only by the variant, but not - // by anyone else. ReadCryptoFrame(const ReadCryptoFrame& other) { offset = other.offset; if (other.data) { @@ -278,8 +276,6 @@ struct ReadNewTokenFrame { ReadNewTokenFrame(Buf tokenIn) : token(std::move(tokenIn)) {} // Stuff stored in a variant type needs to be copyable. - // TODO: can we make this copyable only by the variant, but not - // by anyone else. ReadNewTokenFrame(const ReadNewTokenFrame& other) { if (other.token) { token = other.token->clone(); @@ -367,8 +363,6 @@ struct ReadStreamFrame { fin(finIn) {} // Stuff stored in a variant type needs to be copyable. - // TODO: can we make this copyable only by the variant, but not - // by anyone else. ReadStreamFrame(const ReadStreamFrame& other) { streamId = other.streamId; offset = other.offset; @@ -724,7 +718,6 @@ struct LongHeaderInvariant { LongHeaderInvariant(QuicVersion ver, ConnectionId scid, ConnectionId dcid); }; -// TODO: split this into read and write types. struct LongHeader { public: virtual ~LongHeader() = default; diff --git a/quic/congestion_control/Bbr.cpp b/quic/congestion_control/Bbr.cpp index fcee23e52..97a955d7b 100644 --- a/quic/congestion_control/Bbr.cpp +++ b/quic/congestion_control/Bbr.cpp @@ -37,7 +37,6 @@ BbrCongestionController::BbrCongestionController(QuicConnectionStateBase& conn) conn.udpSendPacketLen * conn.transportSettings.maxCwndInMss), pacingWindow_( conn.udpSendPacketLen * conn.transportSettings.initCwndInMss), - // TODO: experiment with longer window len for ack aggregation filter maxAckHeightFilter_(kBandwidthWindowLength, 0, 0) {} CongestionControlType BbrCongestionController::type() const noexcept { @@ -81,8 +80,6 @@ void BbrCongestionController::onPacketLoss( // We need to make sure CONSERVATIVE can last for a round trip, so update // endOfRoundTrip_ to the latest sent packet. endOfRoundTrip_ = Clock::now(); - - // TODO: maybe set appLimited in recovery based on config } recoveryWindow_ = recoveryWindow_ > @@ -193,7 +190,6 @@ void BbrCongestionController::onPacketAcked( } bool newRoundTrip = updateRoundTripCounter(ack.largestAckedPacketSentTime); - // TODO: I actually don't know why the last one is so special bool lastAckedPacketAppLimited = ack.ackedPackets.empty() ? false : ack.largestAckedPacketAppLimited; if (bandwidthSampler_) { @@ -269,7 +265,6 @@ void BbrCongestionController::updatePacing() noexcept { } else { pacingWindow_ = std::max(pacingWindow_, targetPacingWindow); } - // TODO: slower pacing if we are in STARTUP and loss has happened if (state_ == BbrState::Startup) { conn_.pacer->setRttFactor( conn_.transportSettings.startupRttFactor.first, diff --git a/quic/congestion_control/QuicCubic.h b/quic/congestion_control/QuicCubic.h index 6d876e354..31dc789ae 100644 --- a/quic/congestion_control/QuicCubic.h +++ b/quic/congestion_control/QuicCubic.h @@ -55,7 +55,6 @@ class Cubic : public CongestionController { * spreadacrossRtt: if the pacing bursts should be spread across RTT or all * close to the beginning of an RTT round */ - // TODO: We haven't experimented with setting ackTrain and tcpFriendly explicit Cubic( QuicConnectionStateBase& conn, uint64_t initCwndBytes = 0, diff --git a/quic/loss/QuicLossFunctions.cpp b/quic/loss/QuicLossFunctions.cpp index 7ae41b8bf..9a1da26bb 100644 --- a/quic/loss/QuicLossFunctions.cpp +++ b/quic/loss/QuicLossFunctions.cpp @@ -181,7 +181,6 @@ void markPacketLoss( break; } auto stream = conn.streamManager->getStream(frame.streamId); - // TODO: check for retransmittable if (!stream) { break; } diff --git a/quic/server/QuicServerTransport.cpp b/quic/server/QuicServerTransport.cpp index 011084520..730b9eb56 100644 --- a/quic/server/QuicServerTransport.cpp +++ b/quic/server/QuicServerTransport.cpp @@ -54,9 +54,6 @@ QuicServerTransport::QuicServerTransport( conn_.reset(tempConn.release()); conn_->observers = observers_; - // TODO: generate this when we can encode the packet sequence number - // correctly. - // conn_->nextSequenceNum = folly::Random::secureRandom(); setConnectionCallback(&cb); registerAllTransportKnobParamHandlers(); } @@ -73,8 +70,6 @@ QuicServerTransport::~QuicServerTransport() { false); } -// TODO: refactor this API so that the factory does not have to create an -// owning reference. QuicServerTransport::Ptr QuicServerTransport::make( folly::EventBase* evb, std::unique_ptr sock, diff --git a/quic/server/QuicServerWorker.cpp b/quic/server/QuicServerWorker.cpp index dca080fa2..3ba7d3b3d 100644 --- a/quic/server/QuicServerWorker.cpp +++ b/quic/server/QuicServerWorker.cpp @@ -1202,7 +1202,6 @@ void QuicServerWorker::onConnectionUnbound( } } - // TODO: verify we are removing the right transport sourceAddressMap_.erase(source); } diff --git a/quic/server/state/ServerStateMachine.cpp b/quic/server/state/ServerStateMachine.cpp index 01faeafef..26ed1ff14 100644 --- a/quic/server/state/ServerStateMachine.cpp +++ b/quic/server/state/ServerStateMachine.cpp @@ -813,7 +813,6 @@ void onServerReadDataFromOpen( auto packetNum = regularOptional->header.getPacketSequenceNum(); auto packetNumberSpace = regularOptional->header.getPacketNumberSpace(); - // TODO: enforce constraints on other protection levels. auto& regularPacket = *regularOptional; bool isProtectedPacket = protectionLevel == ProtectionType::ZeroRtt || @@ -909,8 +908,6 @@ void onServerReadDataFromOpen( bool isNonProbingPacket = false; bool handshakeConfirmedThisLoop = false; - // TODO: possibly drop the packet here, but rolling back state of - // what we've already processed is difficult. for (auto& quicFrame : regularPacket.frames) { switch (quicFrame.type()) { case QuicFrame::Type::ReadAckFrame: { @@ -1404,7 +1401,6 @@ QuicServerConnectionState::createAndAddNewSelfConnId() { transportSettings.statelessResetTokenSecret.value(), serverAddr.getFullyQualified()); - // TODO Possibly change this mechanism later // The default connectionId algo has 36 bits of randomness. auto encodedCid = connIdAlgo->encodeConnectionId(*serverConnIdParams); size_t encodedTimes = 0; diff --git a/quic/server/state/ServerStateMachine.h b/quic/server/state/ServerStateMachine.h index 6274589c1..623f53f53 100644 --- a/quic/server/state/ServerStateMachine.h +++ b/quic/server/state/ServerStateMachine.h @@ -148,8 +148,6 @@ struct QuicServerConnectionState : public QuicConnectionStateBase { // Create the crypto stream. cryptoState = std::make_unique(); congestionController = std::make_unique(*this); - // TODO: this is wrong, it should be the handshake finish time. But i need - // a relatively sane time now to make the timestamps all sane. connectionTime = Clock::now(); supportedVersions = std::vector{ {QuicVersion::MVFST, diff --git a/quic/state/AckHandlers.cpp b/quic/state/AckHandlers.cpp index fca17fb7f..8412df854 100644 --- a/quic/state/AckHandlers.cpp +++ b/quic/state/AckHandlers.cpp @@ -81,8 +81,6 @@ void processAckFrame( break; } - // TODO: only process ACKs from packets which are sent from a greater than - // or equal to crypto protection level. auto eraseEnd = rPacketIt; while (rPacketIt != conn.outstandings.packets.rend()) { auto currentPacketNum = rPacketIt->packet.header.getPacketSequenceNum(); diff --git a/quic/state/AckStates.h b/quic/state/AckStates.h index 2113c8323..e62d0ec89 100644 --- a/quic/state/AckStates.h +++ b/quic/state/AckStates.h @@ -39,8 +39,6 @@ struct AckState { // Flag indicating that if we need to send ack immediately. This will be set // to true if we got packets with retransmittable data and haven't sent the // ack for the first time. - // TODO: upgrade this to be bundled like non-retransmittable packets once we - // figured out how to do timeout. bool needsToSendAckImmediately{false}; // Count of oustanding packets received with retransmittable data. uint8_t numRxPacketsRecvd{0}; diff --git a/quic/state/QuicStateFunctions.cpp b/quic/state/QuicStateFunctions.cpp index 3c5794565..65de8454b 100644 --- a/quic/state/QuicStateFunctions.cpp +++ b/quic/state/QuicStateFunctions.cpp @@ -126,7 +126,6 @@ void updateAckSendStateOnRecvPacket( << static_cast(ackState.numNonRxPacketsRecvd) << " numRxPacketsRecvd=" << static_cast(ackState.numRxPacketsRecvd); - // TODO: experiment with outOfOrder and ack timer for NonRxPacket too conn.pendingEvents.scheduleAckTimeout = false; ackState.needsToSendAckImmediately = true; } diff --git a/quic/state/SimpleFrameFunctions.cpp b/quic/state/SimpleFrameFunctions.cpp index eb82802d5..5e42dd3cb 100644 --- a/quic/state/SimpleFrameFunctions.cpp +++ b/quic/state/SimpleFrameFunctions.cpp @@ -243,7 +243,6 @@ bool updateSimpleFrameOnPacketReceived( return true; } case QuicSimpleFrame::Type::KnobFrame: { - // TODO it's const so we have to copy it const KnobFrame& knobFrame = *frame.asKnobFrame(); conn.pendingEvents.knobs.emplace_back( knobFrame.knobSpace, knobFrame.id, knobFrame.blob->clone()); diff --git a/quic/state/StateData.h b/quic/state/StateData.h index 6d9bd32a0..fc708a86e 100644 --- a/quic/state/StateData.h +++ b/quic/state/StateData.h @@ -115,7 +115,6 @@ struct OutstandingsInfo { // All PacketEvents of this connection. If a OutstandingPacket doesn't have an // associatedEvent or if it's not in this set, there is no need to process its // frames upon ack or loss. - // TODO: Enforce only AppTraffic packets to be clonable folly::F14FastSet packetEvents; // Number of outstanding packets not including cloned @@ -341,11 +340,6 @@ struct CongestionController { /** * Take bytes out of flight without mutating other states of the controller - * TODO(yangchi): I'm not sure how long I'd like to keep this API. This is a - * temporary workaround the fact that there are packets we will need to take - * out of outstandings.packets but not considered loss for congestion - * control perspective. In long term, we shouldn't take them out of - * outstandings.packets, then we don't have to do this. */ virtual void onRemoveBytesFromInflight(uint64_t) = 0; virtual void onPacketSent(const OutstandingPacket& packet) = 0; @@ -848,8 +842,6 @@ struct QuicConnectionStateBase : public folly::DelayedDestruction { // Flag indicating whether the socket is currently waiting for the app to // write data. All new sockets start off in this state where they wait for the // application to pump data to the socket. - // TODO: Merge this flag with the existing appLimited flag that exists on each - // Congestion Controller. bool waitingForAppData{true}; // Monotonically increasing counter that is incremented each time there is a