diff --git a/quic/api/Observer.h b/quic/api/Observer.h index 4eac4c252..0ba331ced 100644 --- a/quic/api/Observer.h +++ b/quic/api/Observer.h @@ -34,30 +34,36 @@ class Observer { * Specifies events observer wants to receive. */ struct Config { + virtual ~Config() = default; + // following flags enable support for various callbacks. // observer and socket lifecycle callbacks are always enabled. bool evbEvents{false}; - bool appDataSentEvents{false}; - bool appLimitedEvents{false}; + bool packetsWrittenEvents{false}; + bool appRateLimitedEvents{false}; bool lossEvents{false}; bool spuriousLossEvents{false}; bool pmtuEvents{false}; bool rttSamples{false}; bool knobFrameEvents{false}; + virtual void enableAllEvents() { + evbEvents = true; + packetsWrittenEvents = true; + appRateLimitedEvents = true; + rttSamples = true; + lossEvents = true; + spuriousLossEvents = true; + pmtuEvents = true; + knobFrameEvents = true; + } + /** * Returns a config where all events are enabled. */ static Config getConfigAllEventsEnabled() { Config config = {}; - config.evbEvents = true; - config.appDataSentEvents = true; - config.appLimitedEvents = true; - config.rttSamples = true; - config.lossEvents = true; - config.spuriousLossEvents = true; - config.pmtuEvents = true; - config.knobFrameEvents = true; + config.enableAllEvents(); return config; } }; diff --git a/quic/api/QuicTransportBase.cpp b/quic/api/QuicTransportBase.cpp index 4f08269a2..745ff2c82 100644 --- a/quic/api/QuicTransportBase.cpp +++ b/quic/api/QuicTransportBase.cpp @@ -2726,13 +2726,7 @@ void QuicTransportBase::writeSocketData() { auto packetsBefore = conn_->outstandings.numOutstanding(); // Signal Observers the POTENTIAL start of a Write Block. if (conn_->waitingForAppData && conn_->congestionController) { - Observer::AppLimitedEvent startWritingFromAppLimitedEvent( - conn_->outstandings.packets, conn_->writeCount); - for (const auto& cb : *observers_) { - if (cb->getConfig().appDataSentEvents) { - cb->startWritingFromAppLimited(this, startWritingFromAppLimitedEvent); - } - } + notifyStartWritingFromAppRateLimited(); conn_->waitingForAppData = false; } writeData(); @@ -2749,13 +2743,7 @@ void QuicTransportBase::writeSocketData() { // These may/may not be app data packets, it it up to the Observers // to deal with these packets. if (packetWritten && conn_->congestionController) { - Observer::AppLimitedEvent packetsWrittenEvent( - conn_->outstandings.packets, conn_->writeCount); - for (const auto& cb : *observers_) { - if (cb->getConfig().appDataSentEvents) { - cb->packetsWritten(this, packetsWrittenEvent); - } - } + notifyPacketsWritten(); } if (conn_->loopDetectorCallback && packetWritten) { conn_->writeDebugState.currentEmptyLoopCount = 0; @@ -2792,13 +2780,7 @@ void QuicTransportBase::writeSocketData() { conn_->congestionController->setAppLimited(); // notify via connection call and any observer callbacks connCallback_->onAppRateLimited(); - Observer::AppLimitedEvent appLimitedEvent( - conn_->outstandings.packets, conn_->writeCount); - for (const auto& cb : *observers_) { - if (cb->getConfig().appLimitedEvents) { - cb->appRateLimited(this, appLimitedEvent); - } - } + notifyAppRateLimited(); conn_->waitingForAppData = true; } } @@ -3237,4 +3219,34 @@ QuicSocket::WriteResult QuicTransportBase::setDSRPacketizationRequestSender( return folly::unit; } +void QuicTransportBase::notifyStartWritingFromAppRateLimited() { + Observer::AppLimitedEvent startWritingFromAppLimitedEvent( + conn_->outstandings.packets, conn_->writeCount); + for (const auto& cb : *observers_) { + if (cb->getConfig().appRateLimitedEvents) { + cb->startWritingFromAppLimited(this, startWritingFromAppLimitedEvent); + } + } +} + +void QuicTransportBase::notifyPacketsWritten() { + Observer::AppLimitedEvent packetsWrittenEvent( + conn_->outstandings.packets, conn_->writeCount); + for (const auto& cb : *observers_) { + if (cb->getConfig().packetsWrittenEvents) { + cb->packetsWritten(this, packetsWrittenEvent); + } + } +} + +void QuicTransportBase::notifyAppRateLimited() { + Observer::AppLimitedEvent appRateLimitedEvent( + conn_->outstandings.packets, conn_->writeCount); + for (const auto& cb : *observers_) { + if (cb->getConfig().appRateLimitedEvents) { + cb->appRateLimited(this, appRateLimitedEvent); + } + } +} + } // namespace quic diff --git a/quic/api/QuicTransportBase.h b/quic/api/QuicTransportBase.h index 1eb08adfb..f8451b6f0 100644 --- a/quic/api/QuicTransportBase.h +++ b/quic/api/QuicTransportBase.h @@ -718,6 +718,12 @@ class QuicTransportBase : public QuicSocket { void validateCongestionAndPacing(CongestionControlType& type); + // Helpers to notify all registered observers about specific events during + // socket write (if enabled in the observer's config). + void notifyStartWritingFromAppRateLimited(); + void notifyPacketsWritten(); + void notifyAppRateLimited(); + /** * Callback when we receive a transport knob */ diff --git a/quic/api/test/QuicTransportTest.cpp b/quic/api/test/QuicTransportTest.cpp index 1db75d041..e6170416b 100644 --- a/quic/api/test/QuicTransportTest.cpp +++ b/quic/api/test/QuicTransportTest.cpp @@ -357,8 +357,8 @@ TEST_F(QuicTransportTest, NotAppLimitedWithNoWritableBytesWithObservers) { })); Observer::Config config = {}; - config.appLimitedEvents = true; - config.appDataSentEvents = true; + config.packetsWrittenEvents = true; + config.appRateLimitedEvents = true; auto cb = std::make_unique>(config); EXPECT_CALL(*cb, observerAttach(transport_.get())); @@ -390,8 +390,8 @@ TEST_F(QuicTransportTest, NotAppLimitedWithLargeBufferWithObservers) { .WillRepeatedly(Return(5000)); Observer::Config config = {}; - config.appLimitedEvents = true; - config.appDataSentEvents = true; + config.packetsWrittenEvents = true; + config.appRateLimitedEvents = true; auto cb = std::make_unique>(config); EXPECT_CALL(*cb, observerAttach(transport_.get())); @@ -414,8 +414,8 @@ TEST_F(QuicTransportTest, NotAppLimitedWithLargeBufferWithObservers) { TEST_F(QuicTransportTest, AppLimitedWithObservers) { Observer::Config config = {}; - config.appLimitedEvents = true; - config.appDataSentEvents = true; + config.packetsWrittenEvents = true; + config.appRateLimitedEvents = true; auto cb1 = std::make_unique>(config); auto cb2 = std::make_unique>(config); EXPECT_CALL(*cb1, observerAttach(transport_.get())); diff --git a/quic/api/test/TestQuicTransport.h b/quic/api/test/TestQuicTransport.h index 37913ec06..e39da4c06 100644 --- a/quic/api/test/TestQuicTransport.h +++ b/quic/api/test/TestQuicTransport.h @@ -132,6 +132,18 @@ class TestQuicTransport setIdleTimer(); } + void invokeNotifyStartWritingFromAppRateLimited() { + notifyStartWritingFromAppRateLimited(); + } + + void invokeNotifyPacketsWritten() { + notifyPacketsWritten(); + } + + void invokeNotifyAppRateLimited() { + notifyAppRateLimited(); + } + std::unique_ptr aead; std::unique_ptr headerCipher; bool closed{false}; diff --git a/quic/tools/tperf/tperf.cpp b/quic/tools/tperf/tperf.cpp index a32ccee9d..a41a80395 100644 --- a/quic/tools/tperf/tperf.cpp +++ b/quic/tools/tperf/tperf.cpp @@ -194,7 +194,7 @@ class TPerfAcceptObserver : public AcceptObserver { // Create an observer config, only enabling events we are interested in // receiving. Observer::Config config = {}; - config.appLimitedEvents = true; + config.appRateLimitedEvents = true; config.pmtuEvents = true; config.rttSamples = true; config.lossEvents = true;