diff:
@@ -64,3 +64,5 @@ install(
EXPORT mvfst-exports
DESTINATION lib
)
+
+add_subdirectory(test)

diff:
+# Copyright (c) Facebook, Inc. and its affiliates.
+#
+# This source code is licensed under the MIT license found in the
+# LICENSE file in the root directory of this source tree.
+
+if(NOT BUILD_TESTS)
+ return()
+endif()
+
+quic_add_test(TARGET ClientStateMachineTest
+ SOURCES
+ ClientStateMachineTest.cpp
+ DEPENDS
+ Folly::folly
+ mvfst_client
+ mvfst_test_utils
+)

diff:
+// Copyright 2004-present Facebook. All Rights Reserved.
+
+#include <quic/client/state/ClientStateMachine.h>
+#include <quic/client/handshake/CachedServerTransportParameters.h>
+#include <quic/client/handshake/ClientHandshake.h>
+#include <quic/handshake/CryptoFactory.h>
+#include <quic/handshake/TransportParameters.h>
+#include "quic/client/test/Mocks.h"
+
+using namespace ::testing;
+using namespace std::literals::chrono_literals;
+
+namespace quic::test {
+
+namespace {
+// Use non-default values to test for nops
+constexpr auto idleTimeout = kDefaultIdleTimeout + 1s;
+constexpr auto maxRecvPacketSize = 1420;
+constexpr auto initialMaxData = kDefaultConnectionWindowSize + 2;
+constexpr auto initialMaxStreamDataBidiLocal = kDefaultStreamWindowSize + 3;
+constexpr auto initialMaxStreamDataBidiRemote = kDefaultStreamWindowSize + 4;
+constexpr auto initialMaxStreamDataUni = kDefaultStreamWindowSize + 5;
+constexpr auto initialMaxStreamsBidi = kDefaultMaxStreamsBidirectional + 6;
+constexpr auto initialMaxStreamsUni = kDefaultMaxStreamsUnidirectional + 7;
+const CachedServerTransportParameters kParams{
+ std::chrono::milliseconds(idleTimeout).count(),
+ maxRecvPacketSize,
+ initialMaxData,
+ initialMaxStreamDataBidiLocal,
+ initialMaxStreamDataBidiRemote,
+ initialMaxStreamDataUni,
+ initialMaxStreamsBidi,
+ initialMaxStreamsUni};
+} // namespace
+
+class ClientStateMachineTest : public Test {
+ public:
+ void SetUp() override {
+ mockFactory_ = std::make_shared<MockClientHandshakeFactory>();
+ EXPECT_CALL(*mockFactory_, makeClientHandshake(_))
+ .WillOnce(Invoke(
+ [&](QuicClientConnectionState* conn)
+ -> std::unique_ptr<quic::ClientHandshake> {
+ auto handshake = std::make_unique<MockClientHandshake>(conn);
+ mockHandshake_ = handshake.get();
+ return handshake;
+ }));
+ client_ = std::make_unique<QuicClientConnectionState>(mockFactory_);
+ }
+
+ std::shared_ptr<MockClientHandshakeFactory> mockFactory_;
+ MockClientHandshake* mockHandshake_;
+ std::unique_ptr<QuicClientConnectionState> client_;
+};
+
+TEST_F(ClientStateMachineTest, TestUpdateTransportParamsNotIgnorePathMTU) {
+ updateTransportParamsFromCachedEarlyParams(*client_, kParams);
+ EXPECT_EQ(client_->udpSendPacketLen, kDefaultUDPSendPacketLen);
+}
+
+TEST_F(ClientStateMachineTest, TestUpdateTransportParamsFromCachedEarlyParams) {
+ client_->transportSettings.canIgnorePathMTU = true;
+
+ updateTransportParamsFromCachedEarlyParams(*client_, kParams);
+ EXPECT_EQ(client_->peerIdleTimeout, idleTimeout);
+ EXPECT_EQ(client_->udpSendPacketLen, maxRecvPacketSize);
+ EXPECT_EQ(client_->flowControlState.peerAdvertisedMaxOffset, initialMaxData);
+ EXPECT_EQ(
+ client_->flowControlState.peerAdvertisedInitialMaxStreamOffsetBidiLocal,
+ initialMaxStreamDataBidiLocal);
+ EXPECT_EQ(
+ client_->flowControlState.peerAdvertisedInitialMaxStreamOffsetBidiRemote,
+ initialMaxStreamDataBidiRemote);
+ EXPECT_EQ(
+ client_->flowControlState.peerAdvertisedInitialMaxStreamOffsetUni,
+ initialMaxStreamDataUni);
+
+ for (unsigned long i = 0; i < initialMaxStreamsBidi; i++) {
+ EXPECT_TRUE(
+ client_->streamManager->createNextBidirectionalStream().hasValue());
+ }
+ EXPECT_TRUE(
+ client_->streamManager->createNextBidirectionalStream().hasError());
+ for (unsigned long i = 0; i < initialMaxStreamsUni; i++) {
+ EXPECT_TRUE(
+ client_->streamManager->createNextUnidirectionalStream().hasValue());
+ }
+ EXPECT_TRUE(
+ client_->streamManager->createNextUnidirectionalStream().hasError());
+}
+
+} // namespace quic::test

diff:
+/*
+ * Copyright (c) Facebook, Inc. and its affiliates.
+ *
+ * This source code is licensed under the MIT license found in the
+ * LICENSE file in the root directory of this source tree.
+ *
+ */
+
+#pragma once
+
+#include <folly/portability/GMock.h>
+#include <quic/client/handshake/CachedServerTransportParameters.h>
+#include <quic/client/handshake/ClientHandshake.h>
+#include <quic/handshake/CryptoFactory.h>
+#include <quic/handshake/TransportParameters.h>
+
+namespace quic {
+namespace test {
+
+class MockClientHandshakeFactory : public ClientHandshakeFactory {
+ public:
+ MOCK_METHOD1(
+ makeClientHandshake,
+ std::unique_ptr<ClientHandshake>(QuicClientConnectionState*));
+};
+
+class MockClientHandshake : public ClientHandshake {
+ public:
+ MockClientHandshake(QuicClientConnectionState* conn)
+ : ClientHandshake(conn) {}
+ ~MockClientHandshake() override {
+ destroy();
+ }
+ // Legacy workaround for move-only types
+ void doHandshake(
+ std::unique_ptr<folly::IOBuf> data,
+ EncryptionLevel encryptionLevel) override {
+ doHandshakeImpl(data.get(), encryptionLevel);
+ }
+ MOCK_METHOD2(doHandshakeImpl, void(folly::IOBuf*, EncryptionLevel));
+ MOCK_METHOD0(getRetryPacketCipher, std::unique_ptr<Aead>());
+ MOCK_METHOD1(removePsk, void(const folly::Optional<std::string>&));
+ MOCK_CONST_METHOD0(getCryptoFactory, const CryptoFactory&());
+ MOCK_CONST_METHOD0(isTLSResumed, bool());
+ MOCK_METHOD0(getZeroRttRejected, folly::Optional<bool>());
+ MOCK_METHOD0(
+ getServerTransportParams,
+ folly::Optional<ServerTransportParameters>());
+ MOCK_METHOD0(destroy, void());
+
+ MOCK_METHOD1(
+ connectImpl,
+ folly::Optional<CachedServerTransportParameters>(
+ folly::Optional<std::string>));
+ MOCK_METHOD0(getReadRecordLayerEncryptionLevel, EncryptionLevel());
+ MOCK_METHOD1(processSocketData, void(folly::IOBufQueue& queue));
+ MOCK_METHOD0(matchEarlyParameters, bool());
+ MOCK_METHOD2(
+ buildCiphers,
+ std::pair<std::unique_ptr<Aead>, std::unique_ptr<PacketNumberCipher>>(
+ ClientHandshake::CipherKind kind,
+ folly::ByteRange secret));
+ MOCK_CONST_METHOD0(
+ getApplicationProtocol,
+ const folly::Optional<std::string>&());
+};
+
+} // namespace test
+} // namespace quic

original_message:
Add basic client state machine unit tests

Summary: This adds the Mocks for ClientHandshake and ClientHandshakeFactory. I added a simple test for `updateTransportParamsFromCachedEarlyParams`.

Reviewed By: mjoras

Differential Revision: D23308946

fbshipit-source-id: 6c22412efc6708ad13cd64ade3e9bccc7c80305d


change_type: MODIFY

diff:
@@ -1271,8 +1271,8 @@ void QuicTransportBase::cancelByteEventCallbacksForStream(
// invoke those with offset below provided offset.
while (!streamByteEvents.empty()) {
// decomposition not supported for xplat
- const auto cbOffset = streamByteEvents.front().first;
- const auto callback = streamByteEvents.front().second;
+ const auto cbOffset = streamByteEvents.front().offset;
+ const auto callback = streamByteEvents.front().callback;
if (!offset.has_value() || cbOffset < *offset) {
streamByteEvents.pop_front();
ByteEventCancellation cancellation = {};
@@ -1316,8 +1316,8 @@ void QuicTransportBase::cancelByteEventCallbacks(const ByteEvent::Type type) {
const auto streamId = byteEventMapIt.first;
const auto callbackMap = byteEventMapIt.second;
for (const auto& callbackMapIt : callbackMap) {
- const auto offset = callbackMapIt.first;
- const auto callback = callbackMapIt.second;
+ const auto offset = callbackMapIt.offset;
+ const auto callback = callbackMapIt.callback;
ByteEventCancellation cancellation = {};
cancellation.id = streamId;
cancellation.offset = offset;
@@ -1548,8 +1548,8 @@ void QuicTransportBase::processCallbacksAfterWriteData() {
// lambda to help get the next callback to call for this stream
auto getNextTxCallbackForStreamAndCleanup =
- [this, &largestOffsetTxed](const auto& streamId)
- -> folly::Optional<std::pair<uint64_t, ByteEventCallback*>> {
+ [this, &largestOffsetTxed](
+ const auto& streamId) -> folly::Optional<ByteEventDetail> {
auto txCallbacksForStreamIt = txCallbacks_.find(streamId);
if (txCallbacksForStreamIt == txCallbacks_.end() ||
txCallbacksForStreamIt->second.empty()) {
@@ -1557,7 +1557,7 @@ void QuicTransportBase::processCallbacksAfterWriteData() {
}
auto& txCallbacksForStream = txCallbacksForStreamIt->second;
- if (txCallbacksForStream.front().first > *largestOffsetTxed) {
+ if (txCallbacksForStream.front().offset > *largestOffsetTxed) {
return folly::none;
}
@@ -1570,16 +1570,15 @@ void QuicTransportBase::processCallbacksAfterWriteData() {
return result;
};
- folly::Optional<std::pair<uint64_t, ByteEventCallback*>>
- nextOffsetAndCallback;
+ folly::Optional<ByteEventDetail> nextOffsetAndCallback;
while (
(nextOffsetAndCallback =
getNextTxCallbackForStreamAndCleanup(streamId))) {
ByteEvent byteEvent = {};
byteEvent.id = streamId;
- byteEvent.offset = nextOffsetAndCallback->first;
+ byteEvent.offset = nextOffsetAndCallback->offset;
byteEvent.type = ByteEvent::Type::TX;
- nextOffsetAndCallback->second->onByteEvent(byteEvent);
+ nextOffsetAndCallback->callback->onByteEvent(byteEvent);
// connection may be closed by callback
if (closeState_ != CloseState::OPEN) {
@@ -1641,15 +1640,15 @@ void QuicTransportBase::processCallbacksAfterNetworkData() {
deliveryCallbacksForAckedStream->second.empty()) {
break;
}
- if (deliveryCallbacksForAckedStream->second.front().first >
+ if (deliveryCallbacksForAckedStream->second.front().offset >
*maxOffsetToDeliver) {
break;
}
auto deliveryCallbackAndOffset =
deliveryCallbacksForAckedStream->second.front();
deliveryCallbacksForAckedStream->second.pop_front();
- auto currentDeliveryCallbackOffset = deliveryCallbackAndOffset.first;
- auto deliveryCallback = deliveryCallbackAndOffset.second;
+ auto currentDeliveryCallbackOffset = deliveryCallbackAndOffset.offset;
+ auto deliveryCallback = deliveryCallbackAndOffset.callback;
ByteEvent byteEvent = {};
byteEvent.id = streamId;
@@ -2129,9 +2128,7 @@ QuicTransportBase::registerByteEventCallback(
byteEventMapIt->second.begin(),
byteEventMapIt->second.end(),
offset,
- [&](uint64_t o, const std::pair<uint64_t, ByteEventCallback*>& p) {
- return o < p.first;
- });
+ [&](uint64_t o, const ByteEventDetail& p) { return o < p.offset; });
byteEventMapIt->second.emplace(pos, offset, cb);
}
auto stream = conn_->streamManager->getStream(id);
@@ -2162,9 +2159,7 @@ QuicTransportBase::registerByteEventCallback(
streamByteEventCbIt->second.begin(),
streamByteEventCbIt->second.end(),
offset,
- [&](const std::pair<uint64_t, ByteEventCallback*>& p, uint64_t o) {
- return p.first < o;
- });
+ [&](const ByteEventDetail& p, uint64_t o) { return p.offset < o; });
streamByteEventCbIt->second.erase(pos);
ByteEvent byteEvent = {};

diff:
@@ -666,8 +666,14 @@ class QuicTransportBase : public QuicSocket {
PingCallback* callback,
std::chrono::milliseconds pingTimeout);
- using ByteEventMap = folly::
- F14FastMap<StreamId, std::deque<std::pair<uint64_t, ByteEventCallback*>>>;
+ struct ByteEventDetail {
+ ByteEventDetail(uint64_t offsetIn, ByteEventCallback* callbackIn)
+ : offset(offsetIn), callback(callbackIn) {}
+ uint64_t offset;
+ ByteEventCallback* callback;
+ };
+
+ using ByteEventMap = folly::F14FastMap<StreamId, std::deque<ByteEventDetail>>;
ByteEventMap& getByteEventMap(const ByteEvent::Type type);
FOLLY_NODISCARD const ByteEventMap& getByteEventMapConst(
const ByteEvent::Type type) const;

diff:
@@ -356,10 +356,10 @@ class TestQuicTransport
for (auto& cbs : deliveryCb->second) {
ByteEvent event = {};
event.id = id;
- event.offset = cbs.first;
+ event.offset = cbs.offset;
event.type = ByteEvent::Type::ACK;
event.srtt = stream->conn.lossState.srtt;
- cbs.second->onByteEvent(event);
+ cbs.callback->onByteEvent(event);
if (closeState_ != CloseState::OPEN) {
break;
}
@@ -372,9 +372,9 @@ class TestQuicTransport
for (auto& cbs : txCallbacksForStream->second) {
ByteEvent event = {};
event.id = id;
- event.offset = cbs.first;
+ event.offset = cbs.offset;
event.type = ByteEvent::Type::TX;
- cbs.second->onByteEvent(event);
+ cbs.callback->onByteEvent(event);
if (closeState_ != CloseState::OPEN) {
break;
}

license: MIT License

repo: facebookincubator/mvfst

original_message:
Refactor pair of offset and ByteEventCallback to a struct

Summary: ^ This might increase readability. e.g. "cbs->offset" is more readable than "cbs->first".

Reviewed By: bschlinker, yangchi

Differential Revision: D23361655

fbshipit-source-id: 811eb4439c2c717059ec5f0d15255ec5ede9a2b8


diff:
@@ -286,6 +286,94 @@ Buf StatelessResetPacketBuilder::buildPacket() && {
return std::move(data_);
}
+RegularSizeEnforcedPacketBuilder::RegularSizeEnforcedPacketBuilder(
+ Packet packet,
+ uint64_t enforcedSize,
+ uint32_t cipherOverhead)
+ : packet_(std::move(packet.packet)),
+ header_(std::move(packet.header)),
+ body_(std::move(packet.body)),
+ bodyAppender_(body_.get(), kAppenderGrowthSize),
+ enforcedSize_(enforcedSize),
+ cipherOverhead_(cipherOverhead) {}
+
+bool RegularSizeEnforcedPacketBuilder::canBuildPacket() const noexcept {
+ // We only force size of packets with short header, because d6d probes always
+ // have short headers and there's no other situations for this type of builder
+ const ShortHeader* shortHeader = packet_.header.asShort();
+ // We also don't want to send packets longer than kDefaultMaxUDPPayload
+ return shortHeader && enforcedSize_ <= kDefaultMaxUDPPayload &&
+ (body_->computeChainDataLength() + header_->computeChainDataLength() +
+ cipherOverhead_ <
+ enforcedSize_);
+}
+
+PacketBuilderInterface::Packet
+RegularSizeEnforcedPacketBuilder::buildPacket() && {
+ // Store counters on the stack to overhead from function calls
+ size_t extraDataWritten = 0;
+ size_t bodyLength = body_->computeChainDataLength();
+ size_t headerLength = header_->computeChainDataLength();
+ while (extraDataWritten + bodyLength + headerLength + cipherOverhead_ <
+ enforcedSize_) {
+ QuicInteger paddingType(static_cast<uint8_t>(FrameType::PADDING));
+ paddingType.encode([&](auto val) { bodyAppender_.writeBE(val); });
+ extraDataWritten++;
+ }
+ return Packet(std::move(packet_), std::move(header_), std::move(body_));
+}
+
+InplaceSizeEnforcedPacketBuilder::InplaceSizeEnforcedPacketBuilder(
+ BufAccessor& bufAccessor,
+ Packet packet,
+ uint64_t enforcedSize,
+ uint32_t cipherOverhead)
+ : bufAccessor_(bufAccessor),
+ iobuf_(bufAccessor_.obtain()),
+ packet_(std::move(packet.packet)),
+ header_(std::move(packet.header)),
+ body_(std::move(packet.body)),
+ enforcedSize_(enforcedSize),
+ cipherOverhead_(cipherOverhead) {}
+
+bool InplaceSizeEnforcedPacketBuilder::canBuildPacket() const noexcept {
+ const ShortHeader* shortHeader = packet_.header.asShort();
+ size_t encryptedPacketSize =
+ header_->length() + body_->length() + cipherOverhead_;
+ size_t delta = enforcedSize_ - encryptedPacketSize;
+ return shortHeader && enforcedSize_ <= kDefaultMaxUDPPayload &&
+ encryptedPacketSize < enforcedSize_ && iobuf_->tailroom() >= delta;
+}
+
+PacketBuilderInterface::Packet
+InplaceSizeEnforcedPacketBuilder::buildPacket() && {
+ // Create bodyWriter
+ size_t encryptedPacketSize =
+ header_->length() + body_->length() + cipherOverhead_;
+ size_t paddingSize = enforcedSize_ - encryptedPacketSize;
+ BufWriter bodyWriter(*iobuf_, paddingSize);
+
+ // Store counters on the stack to overhead from function calls
+ size_t extraDataWritten = 0;
+ size_t bodyLength = body_->computeChainDataLength();
+ size_t headerLength = header_->computeChainDataLength();
+ while (extraDataWritten + bodyLength + headerLength + cipherOverhead_ <
+ enforcedSize_) {
+ QuicInteger paddingType(static_cast<uint8_t>(FrameType::PADDING));
+ paddingType.encode([&](auto val) { bodyWriter.writeBE(val); });
+ extraDataWritten++;
+ }
+
+ PacketBuilderInterface::Packet builtPacket(
+ std::move(packet_),
+ std::move(header_),
+ folly::IOBuf::wrapBuffer(body_->data(), iobuf_->tail() - body_->data()));
+
+ // Release internal iobuf
+ bufAccessor_.release(std::move(iobuf_));
+ return builtPacket;
+}
+
VersionNegotiationPacketBuilder::VersionNegotiationPacketBuilder(
ConnectionId sourceConnectionId,
ConnectionId destinationConnectionId,

diff:
@@ -255,6 +255,88 @@ class RegularQuicPacketBuilder final : public PacketBuilderInterface {
folly::Optional<PacketNumEncodingResult> packetNumberEncoding_;
};
+/**
+ * A less involving interface for packet builder, this enables polymorphism
+ * for wrapper-like packet builders (e.g. RegularSizeEnforcedPacketBuilder).
+ */
+class WrapperPacketBuilderInterface {
+ public:
+ using Packet = PacketBuilderInterface::Packet;
+
+ virtual ~WrapperPacketBuilderInterface() = default;
+
+ FOLLY_NODISCARD virtual bool canBuildPacket() const noexcept = 0;
+
+ virtual Packet buildPacket() && = 0;
+};
+
+/**
+ * This builder will enforce the packet size by appending padding frames for
+ * chained memory. This means appending IOBuf at the end of the chain. The
+ * caller should ensure canBuildPacket() returns true before constructing the
+ * builder.
+ */
+class RegularSizeEnforcedPacketBuilder : public WrapperPacketBuilderInterface {
+ public:
+ using Packet = PacketBuilderInterface::Packet;
+
+ explicit RegularSizeEnforcedPacketBuilder(
+ Packet packet,
+ uint64_t enforcedSize,
+ uint32_t cipherOverhead);
+
+ /**
+ * Returns true when packet has short header, and that enforced size >
+ * current packet size + cipher overhead, otherwise false
+ */
+ FOLLY_NODISCARD bool canBuildPacket() const noexcept override;
+
+ Packet buildPacket() && override;
+
+ private:
+ RegularQuicWritePacket packet_;
+ Buf header_;
+ Buf body_;
+ BufAppender bodyAppender_;
+ uint64_t enforcedSize_;
+ uint32_t cipherOverhead_;
+};
+
+/**
+ * This builder will enforce the packet size by appending padding frames for
+ * continuous memory. This means pushing padding frame directly to the current
+ * tail offset. The caller should ensure canBuildPacket() returns true before
+ * constructing the builder.
+ */
+class InplaceSizeEnforcedPacketBuilder : public WrapperPacketBuilderInterface {
+ public:
+ using Packet = PacketBuilderInterface::Packet;
+
+ explicit InplaceSizeEnforcedPacketBuilder(
+ BufAccessor& bufAccessor,
+ Packet packet,
+ uint64_t enforcedSize,
+ uint32_t cipherOverhead);
+
+ /**
+ * Returns true when packet has short header, and that enforced size> current
+ * packet size + cipher oveahead and that iobuf has enough tailroom,
+ * otherwise false
+ */
+ FOLLY_NODISCARD bool canBuildPacket() const noexcept override;
+
+ Packet buildPacket() && override;
+
+ private:
+ BufAccessor& bufAccessor_;
+ Buf iobuf_;
+ RegularQuicWritePacket packet_;
+ Buf header_;
+ Buf body_;
+ uint64_t enforcedSize_;
+ uint32_t cipherOverhead_;
+};
+
class VersionNegotiationPacketBuilder {
public:
explicit VersionNegotiationPacketBuilder(

diff:
@@ -109,7 +109,7 @@ class QuicPacketBuilderTest : public TestWithParam<TestFlavor> {
folly::assume_unreachable();
}
- private:
+ protected:
std::unique_ptr<BufAccessor> simpleBufAccessor_;
};
@@ -277,6 +277,67 @@ TEST_P(QuicPacketBuilderTest, ShortHeaderRegularPacket) {
EXPECT_EQ(pktNum, decodedHeader.getPacketSequenceNum());
}
+TEST_P(QuicPacketBuilderTest, EnforcePacketSizeWithCipherOverhead) {
+ auto connId = getTestConnectionId();
+ PacketNum pktNum = 222;
+
+ PacketNum largestAckedPacketNum = 0;
+ size_t cipherOverhead = 2;
+ uint64_t enforcedSize = 1400;
+ auto aead = std::make_unique<NiceMock<MockAead>>();
+ auto aead_ = aead.get();
+ EXPECT_CALL(*aead_, _inplaceEncrypt(_, _, _))
+ .WillRepeatedly(Invoke([&](auto& buf, auto, auto) {
+ auto overhead = folly::IOBuf::create(1000);
+ overhead->append(cipherOverhead);
+ auto clone = buf->clone();
+ clone->prependChain(std::move(overhead));
+ return std::move(clone);
+ }));
+
+ auto builder = testBuilderProvider(
+ GetParam(),
+ kDefaultUDPSendPacketLen,
+ ShortHeader(ProtectionType::KeyPhaseZero, connId, pktNum),
+ largestAckedPacketNum,
+ 2000);
+ builder->accountForCipherOverhead(cipherOverhead);
+ builder->encodePacketHeader();
+
+ // write out at least one frame
+ writeFrame(PaddingFrame(), *builder);
+ EXPECT_TRUE(builder->canBuildPacket());
+ auto builtOut = std::move(*builder).buildPacket();
+
+ auto param = GetParam();
+ if (param == TestFlavor::Regular) {
+ EXPECT_EQ(builtOut.body->isManagedOne(), true);
+ RegularSizeEnforcedPacketBuilder sizeEnforcedBuilder(
+ std::move(builtOut), enforcedSize, cipherOverhead);
+ EXPECT_TRUE(sizeEnforcedBuilder.canBuildPacket());
+ auto out = std::move(sizeEnforcedBuilder).buildPacket();
+ EXPECT_EQ(
+ out.header->computeChainDataLength() +
+ out.body->computeChainDataLength(),
+ enforcedSize - cipherOverhead);
+ auto buf = packetToBuf(out, aead_);
+ EXPECT_EQ(buf->computeChainDataLength(), enforcedSize);
+
+ } else {
+ EXPECT_EQ(builtOut.body->isManagedOne(), false);
+ InplaceSizeEnforcedPacketBuilder sizeEnforcedBuilder(
+ *simpleBufAccessor_, std::move(builtOut), enforcedSize, cipherOverhead);
+ EXPECT_TRUE(sizeEnforcedBuilder.canBuildPacket());
+ auto out = std::move(sizeEnforcedBuilder).buildPacket();
+ EXPECT_EQ(
+ out.header->computeChainDataLength() +
+ out.body->computeChainDataLength(),
+ enforcedSize - cipherOverhead);
+ auto buf = packetToBuf(out, aead_);
+ EXPECT_EQ(buf->computeChainDataLength(), enforcedSize);
+ }
+}
+
TEST_P(QuicPacketBuilderTest, ShortHeaderWithNoFrames) {
auto connId = getTestConnectionId();
PacketNum pktNum = 222;

original_message:
Add back SizeEnforcedPacketBuilder

Summary: ^

Reviewed By: yangchi

Differential Revision: D23432975

fbshipit-source-id: 65892a1bb5146dd12b5f3f89ec689f9d4326d035


diff:
@@ -213,18 +213,25 @@ void processClientInitialParams(
if (conn.transportSettings.d6dConfig.enabled) {
// Sanity check
- if (d6dBasePMTU && *d6dBasePMTU >= kMinMaxUDPPayload &&
+ if (d6dBasePMTU) {
+ if (*d6dBasePMTU >= kMinMaxUDPPayload &&
*d6dBasePMTU <= kDefaultMaxUDPPayload) {
- // The reason to take the max is because we don't want d6d to send probes
- // with a smaller packet size than udpSendPacketLen, which would be
- // useless and cause meaningless delay on finding the upper bound.
+ // The reason to take the max is because we don't want d6d to send
+ // probes with a smaller packet size than udpSendPacketLen, which would
+ // be useless and cause meaningless delay on finding the upper bound.
conn.d6d.basePMTU = std::max(*d6dBasePMTU, conn.udpSendPacketLen);
VLOG(10) << "conn.d6d.basePMTU=" << conn.d6d.basePMTU;
} else {
LOG(ERROR) << "client d6dBasePMTU fails sanity check: " << *d6dBasePMTU;
+ // We treat base pmtu transport param as client's swich to activate d6d,
+ // so not receiving that means there's no need to configure the rest d6d
+ // params
+ return;
+ }
}
- if (d6dRaiseTimeout && *d6dRaiseTimeout >= kMinD6DRaiseTimeout.count()) {
+ if (d6dRaiseTimeout) {
+ if (*d6dRaiseTimeout >= kMinD6DRaiseTimeout.count()) {
conn.d6d.raiseTimeout = std::chrono::seconds(*d6dRaiseTimeout);
VLOG(10) << "conn.d6d.raiseTimeout=" << conn.d6d.raiseTimeout.count();
} else {
@@ -233,6 +240,7 @@ void processClientInitialParams(
}
}
}
+}
void updateHandshakeState(QuicServerConnectionState& conn) {
// Zero RTT read cipher is available after chlo is processed with the

original_message:
log error only when d6d transport parameters are present

Summary:
Currently it logs error even if transport parameters are not present, but in
that we should assume the client doesn't want d6d, so we should not log error at
all.

Reviewed By: yangchi

Differential Revision: D23681935

fbshipit-source-id: 6420ff3053cd6c7d2d0ddad854f481e4f422c992


diff:
@@ -304,6 +304,11 @@ bool QuicServerTransport::hasWriteCipher() const {
return conn_->oneRttWriteCipher != nullptr;
}
+bool QuicServerTransport::hasReadCipher() const {
+ return conn_->readCodec != nullptr &&
+ conn_->readCodec->getOneRttReadCipher() != nullptr;
+}
+
std::shared_ptr<QuicTransportBase> QuicServerTransport::sharedGuard() {
return shared_from_this();
}

diff:
@@ -138,6 +138,7 @@ class QuicServerTransport
void maybeNotifyConnectionIdBound();
void maybeWriteNewSessionTicket();
void maybeIssueConnectionIds();
+ bool hasReadCipher() const;
private:
RoutingCallback* routingCb_{nullptr};

original_message:
Add hasReadCipher

Summary:
This is one of the pre-condition of starting d6d probing. Starting d6d once
hasWriterCipher() is too early becaue an overlarge PMTU could potentially
cause the server to send oversized handshake packet and disrupt handshake.

Reviewed By: yangchi

Differential Revision: D23698784

fbshipit-source-id: f18824a8ef516421832d8cc769f4880a0841e492


diff:
@@ -765,9 +765,24 @@ struct QuicConnectionStateBase : public folly::DelayedDestruction {
uint32_t packetSize;
};
- struct D6DState {
- // The <packet sequence number, udp payload size> of the last sent d6d probe
+ // States of d6d state machine, see
+ // https://tools.ietf.org/id/draft-ietf-tsvwg-datagram-plpmtud-21.html#name-state-machine
+ enum class D6DMachineState : uint8_t {
+ // Connection is not established yet
+ DISABLED,
+ // Probe using base pmtu
+ BASE,
+ // Incrementally probe using larger pmtu
+ SEARCHING,
+ // Sleep for raise timeout before going to SEARCHING
+ SEARCH_COMPLETE,
+ // Effective pmtu is less than base pmtu, continue probing with smaller
// packet
+ ERROR
+ };
+
+ struct D6DState {
+ // The lastest d6d probe packet transmitted
folly::Optional<D6DProbePacket> lastProbe;
// The number of outstanding probe packets
@@ -776,7 +791,7 @@ struct QuicConnectionStateBase : public folly::DelayedDestruction {
// The base PMTU to start probing with
uint16_t basePMTU{kDefaultD6DBasePMTU};
- // Current probe size
+ // Current probe size, dynamically adjusted by the probing algorithm
uint32_t currentProbeSize{kDefaultD6DBasePMTU};
// The raise timeout
@@ -785,6 +800,9 @@ struct QuicConnectionStateBase : public folly::DelayedDestruction {
// The probe timeout
std::chrono::seconds probeTimeout{kDefaultD6DProbeTimeout};
+ // D6D Machine State
+ D6DMachineState state{D6DMachineState::DISABLED};
+
// Probe size raiser
std::unique_ptr<ProbeSizeRaiser> raiser;
};

original_message:
Add D6D Machine State

Summary:
This adds an enum for d6d state machine's state. We've used the name D6DState,
so I chose D6DMachineState as the enum name.

Reviewed By: mjoras

Differential Revision: D23715384

fbshipit-source-id: 291b2c268c126282e74caba6f3f4a5a28f69edd4


message:
Suppress error for oversized packets if it's d6d probe
Summary: This was missed (https://github.com/facebookincubator/mvfst/commit/9cdb922288d1eee19b9b31800de3c1fcd295cf48).

diff:
@@ -267,7 +267,10 @@ DataPathResult continuousMemoryBuildScheduleEncrypt(
packetBuf->prepend(prevSize);
connection.bufAccessor->release(std::move(packetBuf));
#if !FOLLY_MOBILE
- if (encodedSize > connection.udpSendPacketLen) {
+ bool isD6DProbe = pnSpace == PacketNumberSpace::AppData &&
+ connection.d6d.lastProbe.hasValue() &&
+ connection.d6d.lastProbe->packetNum == packetNum;
+ if (!isD6DProbe && encodedSize > connection.udpSendPacketLen) {
LOG_EVERY_N(ERROR, 5000)
<< "Quic sending pkt larger than limit, encodedSize=" << encodedSize;
}

original_message:
Suppress error for oversized packets if it's d6d probe

Summary: This was missed D22551400 (https://github.com/facebookincubator/mvfst/commit/9cdb922288d1eee19b9b31800de3c1fcd295cf48).

Reviewed By: mjoras

Differential Revision: D23733719

fbshipit-source-id: d1226ccad932f0d3201ce23281ab08eda005a43b


diff:
@@ -34,7 +34,8 @@ template <
typename TimeT,
typename TimeDeltaT,
typename = std::enable_if_t<
- std::is_arithmetic_v<TimeT> && std::is_arithmetic_v<TimeDeltaT>>>
+ std::is_arithmetic<TimeT>::value &&
+ std::is_arithmetic<TimeDeltaT>::value>>
class WindowedCounter {
public:
/**

original_message:
Use is_arithmetic<T>::value for C++11 compatibility

Summary:
Some xplat build platforms only support C++11, where is_arithmetic_v is not a
thing yet.

Reviewed By: avasylev

Differential Revision: D23736168

fbshipit-source-id: eef246adbefe110e1c71caa58bf0b0ccfb2da7b5


message:
Fix erroneous condition caused by merging
Summary: This condition check is removed by (https://github.com/facebookincubator/mvfst/commit/c07c2158d96901a5e2c18d9d3428db44e427a035), but then added by (https://github.com/facebookincubator/mvfst/commit/a8d5c156a1236e026fd730721c6bea2cbd9d25fd).

diff:
@@ -219,7 +219,7 @@ folly::Optional<CongestionController::LossEvent> detectLossPackets(
break;
}
auto currentPacketNumberSpace = pkt.packet.header.getPacketNumberSpace();
- if (currentPacketNumberSpace != pnSpace || pkt.metadata.isD6DProbe) {
+ if (currentPacketNumberSpace != pnSpace) {
iter++;
continue;
}

original_message:
Fix erroneous condition caused by merging

Summary: This condition check is removed by D23734194 (https://github.com/facebookincubator/mvfst/commit/c07c2158d96901a5e2c18d9d3428db44e427a035), but then added by D23711641 (https://github.com/facebookincubator/mvfst/commit/a8d5c156a1236e026fd730721c6bea2cbd9d25fd).

Reviewed By: yangchi

Differential Revision: D23910768

fbshipit-source-id: 1a8db641a43a76bc82d5be657964b11f350727f6


diff:
@@ -2446,12 +2446,14 @@ void QuicTransportBase::idleTimeoutExpired(bool drain) noexcept {
void QuicTransportBase::d6dProbeTimeoutExpired() noexcept {
VLOG(4) << __func__ << " " << *this;
FOLLY_MAYBE_UNUSED auto self = sharedGuard();
+ conn_->pendingEvents.d6d.scheduleProbeTimeout = false;
onD6DProbeTimeoutExpired(*conn_);
}
void QuicTransportBase::d6dRaiseTimeoutExpired() noexcept {
VLOG(4) << __func__ << " " << *this;
FOLLY_MAYBE_UNUSED auto self = sharedGuard();
+ conn_->pendingEvents.d6d.scheduleRaiseTimeout = false;
onD6DRaiseTimeoutExpired(*conn_);
}
@@ -2530,7 +2532,7 @@ void QuicTransportBase::schedulePathValidationTimeout() {
}
void QuicTransportBase::scheduleD6DProbeTimeout() {
- if (conn_->pendingEvents.scheduleD6DProbeTimeout) {
+ if (conn_->pendingEvents.d6d.scheduleProbeTimeout) {
if (!d6dProbeTimeout_.isScheduled()) {
VLOG(10) << __func__ << "timeout=" << conn_->d6d.probeTimeout.count()
<< "ms " << *this;
@@ -2546,7 +2548,7 @@ void QuicTransportBase::scheduleD6DProbeTimeout() {
}
void QuicTransportBase::scheduleD6DRaiseTimeout() {
- if (conn_->pendingEvents.scheduleD6DRaiseTimeout) {
+ if (conn_->pendingEvents.d6d.scheduleRaiseTimeout) {
if (!d6dRaiseTimeout_.isScheduled()) {
VLOG(10) << __func__ << "timeout=" << conn_->d6d.raiseTimeout.count()
<< "s " << *this;

diff:
@@ -1343,7 +1343,7 @@ uint64_t writeD6DProbeToSocket(
const Aead& aead,
const PacketNumberCipher& headerCipher,
QuicVersion version) {
- if (!connection.pendingEvents.sendD6DProbePacket) {
+ if (!connection.pendingEvents.d6d.sendProbePacket) {
return 0;
}
auto builder = ShortHeaderBuilder();
@@ -1373,7 +1373,7 @@ uint64_t writeD6DProbeToSocket(
<< " writing d6d probes using scheduler=D6DScheduler"
<< connection;
if (written > 0) {
- connection.pendingEvents.sendD6DProbePacket = false;
+ connection.pendingEvents.d6d.sendProbePacket = false;
}
return written;
}

diff:
@@ -2526,7 +2526,7 @@ TEST_F(QuicTransportFunctionsTest, WriteD6DProbesWithInplaceBuilder) {
auto conn = createConn();
conn->transportSettings.dataPathType = DataPathType::ContinuousMemory;
conn->d6d.currentProbeSize = 1450;
- conn->pendingEvents.sendD6DProbePacket = true;
+ conn->pendingEvents.d6d.sendProbePacket = true;
auto simpleBufAccessor =
std::make_unique<SimpleBufAccessor>(kDefaultMaxUDPPayload * 16);
auto outputBuf = simpleBufAccessor->obtain();

diff:
@@ -40,7 +40,7 @@ void onD6DRaiseTimeoutExpired(QuicConnectionStateBase& conn) {
auto& d6d = conn.d6d;
if (d6d.state == D6DMachineState::SEARCH_COMPLETE) {
d6d.state = D6DMachineState::SEARCHING;
- conn.pendingEvents.sendD6DProbePacket = true;
+ conn.pendingEvents.d6d.sendProbePacket = true;
} else {
LOG(ERROR) << "d6d: raise timeout expired in state: "
<< toString(d6d.state);
@@ -62,7 +62,7 @@ void onD6DLastProbeAcked(QuicConnectionStateBase& conn) {
CHECK(maybeNextProbeSize.hasValue());
d6d.currentProbeSize = *maybeNextProbeSize;
d6d.state = D6DMachineState::SEARCHING;
- conn.pendingEvents.sendD6DProbePacket = true;
+ conn.pendingEvents.d6d.sendProbePacket = true;
break;
case D6DMachineState::SEARCHING:
CHECK_GT(lastProbeSize, conn.udpSendPacketLen);
@@ -74,8 +74,8 @@ void onD6DLastProbeAcked(QuicConnectionStateBase& conn) {
// raiser's internal upper bound, in both cases the search is
// completed
d6d.state = D6DMachineState::SEARCH_COMPLETE;
- conn.pendingEvents.scheduleD6DRaiseTimeout = true;
- conn.pendingEvents.scheduleD6DProbeTimeout = false;
+ conn.pendingEvents.d6d.scheduleRaiseTimeout = true;
+ conn.pendingEvents.d6d.scheduleProbeTimeout = false;
}
break;
case D6DMachineState::ERROR:
@@ -104,24 +104,24 @@ void onD6DLastProbeLost(QuicConnectionStateBase& conn) {
// connection prior to this state.
}
// In both BASE and ERROR state, we need to keep sending probes
- conn.pendingEvents.sendD6DProbePacket = true;
+ conn.pendingEvents.d6d.sendProbePacket = true;
break;
case D6DMachineState::SEARCHING:
if (d6d.outstandingProbes >= kDefaultD6DMaxOutstandingProbes) {
// We've lost enough consecutive probes, which should indicate
// that the upper bound is reached
d6d.state = D6DMachineState::SEARCH_COMPLETE;
- conn.pendingEvents.scheduleD6DRaiseTimeout = true;
+ conn.pendingEvents.d6d.scheduleRaiseTimeout = true;
return;
}
// Otherwise, the loss could be due to congestion, so we keep
// sending probe
// TODO: pace d6d probing when there's congestion
- conn.pendingEvents.sendD6DProbePacket = true;
+ conn.pendingEvents.d6d.sendProbePacket = true;
break;
case D6DMachineState::ERROR:
// Keep probing with min probe size
- conn.pendingEvents.sendD6DProbePacket = true;
+ conn.pendingEvents.d6d.sendProbePacket = true;
break;
default:
LOG(ERROR) << "d6d: probe timeout expired in state: "
@@ -161,7 +161,7 @@ void detectPMTUBlackhole(
conn.udpSendPacketLen = d6d.basePMTU;
// Cancel existing raise timeout if any
- conn.pendingEvents.scheduleD6DRaiseTimeout = false;
+ conn.pendingEvents.d6d.scheduleRaiseTimeout = false;
}
}
} // namespace quic

diff:
@@ -50,11 +50,11 @@ class QuicD6DStateFunctionsTest : public Test {
conn.d6d.state = fixture.stateBegin;
conn.d6d.outstandingProbes = fixture.outstandingProbes;
conn.d6d.currentProbeSize = fixture.currentProbeSizeBegin;
- conn.pendingEvents.sendD6DProbePacket = fixture.sendProbeBegin;
+ conn.pendingEvents.d6d.sendProbePacket = fixture.sendProbeBegin;
onD6DLastProbeLost(conn);
EXPECT_EQ(conn.d6d.state, fixture.stateEnd);
EXPECT_EQ(conn.d6d.currentProbeSize, fixture.currentProbeSizeEnd);
- EXPECT_EQ(conn.pendingEvents.sendD6DProbePacket, fixture.sendProbeEnd);
+ EXPECT_EQ(conn.pendingEvents.d6d.sendProbePacket, fixture.sendProbeEnd);
}
};

diff:
@@ -280,7 +280,7 @@ void QuicServerTransport::writeData() {
packetLimit);
// D6D probes should be paced
- if (packetLimit && conn_->pendingEvents.sendD6DProbePacket) {
+ if (packetLimit && conn_->pendingEvents.d6d.sendProbePacket) {
writeD6DProbeToSocket(
*socket_,
*conn_,
@@ -540,7 +540,7 @@ void QuicServerTransport::maybeStartD6DProbing() {
if (self->closeState_ == CloseState::CLOSED) {
return;
}
- self->conn_->pendingEvents.sendD6DProbePacket = true;
+ self->conn_->pendingEvents.d6d.sendProbePacket = true;
},
kDefaultD6DKickStartDelay.count());
}

diff:
@@ -606,6 +606,20 @@ struct QuicConnectionStateBase : public folly::DelayedDestruction {
// Whether or not we received a new packet before a write.
bool receivedNewPacketBeforeWrite{false};
+ // D6D related events
+ struct PendingD6DEvents {
+ // If we should schedule/cancel d6d raise timeout, if it's not
+ // already scheduled/canceled
+ bool scheduleRaiseTimeout{false};
+
+ // If we should schedule/cancel d6d probe timeout, if it's not
+ // already scheduled/canceled
+ bool scheduleProbeTimeout{false};
+
+ // To send a d6d probe packet
+ bool sendProbePacket{false};
+ };
+
struct PendingEvents {
Resets resets;
@@ -620,14 +634,6 @@ struct QuicConnectionStateBase : public folly::DelayedDestruction {
// If we should schedule a new Ack timeout, if it's not already scheduled
bool scheduleAckTimeout{false};
- // If we should schedule/cancel d6d raise timeout, if it's not
- // already scheduled/canceled
- bool scheduleD6DRaiseTimeout{false};
-
- // If we should schedule/cancel d6d probe timeout, if it's not
- // already scheduled/canceled
- bool scheduleD6DProbeTimeout{false};
-
// Whether a connection level window update is due to send
bool connWindowUpdate{false};
@@ -645,12 +651,12 @@ struct QuicConnectionStateBase : public folly::DelayedDestruction {
// To send a ping frame
bool sendPing{false};
- // To send a d6d probe packet
- bool sendD6DProbePacket{false};
-
// Do we need to send data blocked frame when connection is blocked.
bool sendDataBlocked{false};
+ // D6D related events
+ PendingD6DEvents d6d;
+
std::vector<KnobFrame> knobs;
};

original_message:
Refactor d6d pending events into a single struct

Summary: As a drive-by: fixed a bug where I didn't cancel the pending event when timeouts expire.

Reviewed By: mjoras

Differential Revision: D23910767

fbshipit-source-id: 233e590c17a6c6b7a5f4a251bd8f78f6dfae3c0b


diff:
@@ -1621,8 +1621,13 @@ void QuicTransportBase::processCallbacksAfterWriteData() {
void QuicTransportBase::handleKnobCallbacks() {
for (auto& knobFrame : conn_->pendingEvents.knobs) {
+ if (knobFrame.knobSpace != kDefaultQuicTransportKnobSpace) {
connCallback_->onKnob(
knobFrame.knobSpace, knobFrame.id, std::move(knobFrame.blob));
+ } else {
+ // KnobId is ignored
+ onTransportKnobs(std::move(knobFrame.blob));
+ }
}
conn_->pendingEvents.knobs.clear();
}
@@ -3071,4 +3076,12 @@ const QuicTransportBase::ByteEventMap& QuicTransportBase::getByteEventMapConst(
folly::assume_unreachable();
}
+void QuicTransportBase::onTransportKnobs(Buf knobBlob) {
+ // Not yet implemented,
+ VLOG(4) << "Received transport knobs: "
+ << std::string(
+ reinterpret_cast<const char*>(knobBlob->data()),
+ knobBlob->length());
+}
+
} // namespace quic

diff:
@@ -726,6 +726,11 @@ class QuicTransportBase : public QuicSocket {
void scheduleD6DProbeTimeout();
void scheduleD6DTxTimeout();
+ /**
+ * Callback when we receive a transport knob
+ */
+ void onTransportKnobs(Buf knobBlob);
+
struct ByteEventDetail {
ByteEventDetail(uint64_t offsetIn, ByteEventCallback* callbackIn)
: offset(offsetIn), callback(callbackIn) {}

diff:
@@ -685,6 +685,8 @@ void QuicClientTransport::onReadData(
socket_->setErrMessageCallback(nullptr);
connCallback_->onReplaySafe();
}
+
+ maybeSendTransportKnobs();
}
void QuicClientTransport::writeData() {
@@ -1686,4 +1688,21 @@ void QuicClientTransport::trackDatagramReceived(size_t len) {
QUIC_STATS(statsCallback_, onRead, len);
}
+void QuicClientTransport::maybeSendTransportKnobs() {
+ if (!transportKnobsSent_ && hasWriteCipher()) {
+ for (const auto& knob : conn_->transportSettings.knobs) {
+ auto res =
+ setKnob(knob.space, knob.id, folly::IOBuf::copyBuffer(knob.blob));
+ if (res.hasError()) {
+ if (res.error() != LocalErrorCode::KNOB_FRAME_UNSUPPORTED) {
+ LOG(ERROR) << "Unexpected error while sending knob frames";
+ }
+ // No point in keep trying if transport does not support knob frame
+ break;
+ }
+ }
+ transportKnobsSent_ = true;
+ }
+}
+
} // namespace quic

diff:
@@ -213,6 +213,12 @@ class QuicClientTransport
void adjustGROBuffers();
void trackDatagramReceived(size_t len);
+ /**
+ * Send quic transport knobs defined by transportSettings.knobs to peer. This
+ * calls setKnobs() internally.
+ */
+ void maybeSendTransportKnobs();
+
bool replaySafeNotified_{false};
// Set it QuicClientTransport is in a self owning mode. This will be cleaned
// up when the caller invokes a terminal call to the transport.
@@ -227,5 +233,7 @@ class QuicClientTransport
// supports GRO. otherwise kDefaultNumGROBuffers
uint32_t numGROBuffers_{kDefaultNumGROBuffers};
RecvmmsgStorage recvmmsgStorage_;
+ // We will only send transport knobs once, this flag keeps track of it
+ bool transportKnobsSent_{false};
};
} // namespace quic

original_message:
Send knob frames for transport knobs

Summary: Adds routines to send transport knobs.

Reviewed By: mjoras

Differential Revision: D23825714

fbshipit-source-id: a61b5650eacf0767054e18b59c731072e364c9b2


diff:
@@ -727,7 +727,7 @@ SchedulingResult D6DProbeScheduler::scheduleFramesForPacket(
<< " seq: " << conn_.d6d.lastProbe->packetNum
<< " packet size: " << conn_.d6d.lastProbe->packetSize;
- conn_.d6d.lastProbe = QuicConnectionStateBase::D6DProbePacket(
+ conn_.d6d.lastProbe = D6DProbePacket(
resultPacket.packet.header.getPacketSequenceNum(), probeSize_);
probeSent_ = true;

diff:
@@ -364,7 +364,7 @@ TEST_F(QuicTransportFunctionsTest, TestUpdateConnectionD6DNotConsumeSendPing) {
auto packet = buildEmptyPacket(*conn, PacketNumberSpace::AppData);
packet.packet.frames.push_back(PingFrame());
auto packetNum = packet.packet.header.getPacketSequenceNum();
- conn->d6d.lastProbe = QuicConnectionStateBase::D6DProbePacket(packetNum, 50);
+ conn->d6d.lastProbe = D6DProbePacket(packetNum, 50);
updateConnection(*conn, folly::none, packet.packet, Clock::now(), 50);
EXPECT_EQ(1, conn->outstandings.packets.size());
EXPECT_TRUE(conn->outstandings.packets.front().metadata.isD6DProbe);

diff:
# This source code is licensed under the MIT license found in the
# LICENSE file in the root directory of this source tree.
+# Probe Raiser
add_library(
mvfst_d6d_probe_raiser INTERFACE
)
@@ -13,6 +14,7 @@ target_include_directories(
$<INSTALL_INTERFACE:include/>
)
+# State Functions
add_library(
mvfst_d6d_state_functions STATIC
QuicD6DStateFunctions.cpp
@@ -41,6 +43,35 @@ target_link_libraries(
mvfst_state_machine
)
+# Types
+add_library(
+ mvfst_d6d_types STATIC
+ Types.cpp
+)
+
+target_include_directories(
+ mvfst_d6d_types PUBLIC
+ $<BUILD_INTERFACE:${QUIC_FBCODE_ROOT}>
+ $<INSTALL_INTERFACE:include/>
+)
+
+target_compile_options(
+ mvfst_d6d_types
+ PRIVATE
+ ${_QUIC_COMMON_COMPILE_OPTIONS}
+)
+
+add_dependencies(
+ mvfst_d6d_types
+ mvfst_codec_types
+)
+
+target_link_libraries(
+ mvfst_d6d_types PUBLIC
+ Folly::folly
+ mvfst_codec_types
+)
+
file(
GLOB_RECURSE QUIC_API_HEADERS_TOINSTALL
RELATIVE ${CMAKE_CURRENT_SOURCE_DIR}
@@ -64,4 +95,10 @@ install(
DESTINATION lib
)
+install(
+ TARGETS mvfst_d6d_types
+ EXPORT mvfst-exports
+ DESTINATION lib
+)
+
add_subdirectory(test)

diff:
namespace quic {
-namespace {
-using D6DMachineState = QuicConnectionStateBase::D6DMachineState;
-
-std::string toString(
- const ::quic::QuicConnectionStateBase::D6DMachineState state) {
- switch (state) {
- case D6DMachineState::DISABLED:
- return "DISABLED";
- case D6DMachineState::BASE:
- return "BASE";
- case D6DMachineState::SEARCHING:
- return "SEARCHING";
- case D6DMachineState::SEARCH_COMPLETE:
- return "SEARCH_COMPLETE";
- case D6DMachineState::ERROR:
- return "ERROR";
- }
- folly::assume_unreachable();
-}
-
-} // namespace
-
inline void scheduleProbeAfterDelay(
QuicConnectionStateBase& conn,
std::chrono::milliseconds delay) {

change_type: ADD

diff:
+/*
+ * Copyright (c) Facebook, Inc. and its affiliates.
+ *
+ * This source code is licensed under the MIT license found in the
+ * LICENSE file in the root directory of this source tree.
+ *
+ */
+
+#include <quic/d6d/Types.h>
+#include <folly/lang/Assume.h>
+
+namespace quic {
+std::string toString(const D6DMachineState state) {
+ switch (state) {
+ case D6DMachineState::DISABLED:
+ return "DISABLED";
+ case D6DMachineState::BASE:
+ return "BASE";
+ case D6DMachineState::SEARCHING:
+ return "SEARCHING";
+ case D6DMachineState::SEARCH_COMPLETE:
+ return "SEARCH_COMPLETE";
+ case D6DMachineState::ERROR:
+ return "ERROR";
+ }
+ folly::assume_unreachable();
+}
+
+std::string toString(const ProbeSizeRaiserType type) {
+ switch (type) {
+ case ProbeSizeRaiserType::ConstantStep:
+ return "ConstantStep";
+ case ProbeSizeRaiserType::BinarySearch:
+ return "BinarySearch";
+ }
+ folly::assume_unreachable();
+}
+
+} // namespace quic

diff:
+/*
+ * Copyright (c) Facebook, Inc. and its affiliates.
+ *
+ * This source code is licensed under the MIT license found in the
+ * LICENSE file in the root directory of this source tree.
+ *
+ */
+
+#pragma once
+
+#include <quic/codec/Types.h>
+#include <string>
+
+namespace quic {
+struct D6DProbePacket {
+ D6DProbePacket() = delete;
+
+ explicit D6DProbePacket(PacketNum packetNumIn, uint32_t packetSizeIn)
+ : packetNum(packetNumIn), packetSize(packetSizeIn) {}
+
+ // Packet num
+ PacketNum packetNum;
+
+ // Udp packet payload size
+ uint32_t packetSize;
+};
+
+// States of d6d state machine, see
+// https://tools.ietf.org/id/draft-ietf-tsvwg-datagram-plpmtud-21.html#name-state-machine
+enum class D6DMachineState : uint8_t {
+ // Connection is not established yet
+ DISABLED = 0,
+ // Probe using base pmtu
+ BASE = 1,
+ // Incrementally probe using larger pmtu
+ SEARCHING = 2,
+ // Sleep for raise timeout before going to SEARCHING
+ SEARCH_COMPLETE = 3,
+ // Effective pmtu is less than base pmtu, continue probing with smaller
+ // packet
+ ERROR = 4
+};
+
+/**
+ * Two simple probe size raiser. Only server makes use of this value.
+ * ConstantSize: raise pmtu at constant step size
+ * BinarySearch: raise pmtu using binary search
+ */
+enum class ProbeSizeRaiserType : uint8_t { ConstantStep = 0, BinarySearch = 1 };
+
+std::string toString(const D6DMachineState state);
+std::string toString(const ProbeSizeRaiserType type);
+
+} // namespace quic

diff:
@@ -19,10 +19,6 @@ using namespace testing;
namespace quic {
namespace test {
-namespace {
-using D6DMachineState = QuicConnectionStateBase::D6DMachineState;
-} // namespace
-
struct D6DProbeLostTestFixture {
D6DMachineState stateBegin;
D6DMachineState stateEnd;
@@ -149,7 +145,7 @@ TEST_F(QuicD6DStateFunctionsTest, D6DProbeAckedInBase) {
false,
true,
d6d.currentProbeSize);
- d6d.lastProbe = QuicConnectionStateBase::D6DProbePacket(
+ d6d.lastProbe = D6DProbePacket(
pkt.packet.header.getPacketSequenceNum(), pkt.metadata.encodedSize);
d6d.raiser = std::make_unique<MockProbeSizeRaiser>();
auto mockRaiser = dynamic_cast<MockProbeSizeRaiser*>(d6d.raiser.get());
@@ -177,7 +173,7 @@ TEST_F(QuicD6DStateFunctionsTest, D6DProbeAckedInSearchingOne) {
false,
true,
d6d.currentProbeSize);
- d6d.lastProbe = QuicConnectionStateBase::D6DProbePacket(
+ d6d.lastProbe = D6DProbePacket(
pkt.packet.header.getPacketSequenceNum(), pkt.metadata.encodedSize);
d6d.raiser = std::make_unique<MockProbeSizeRaiser>();
auto mockRaiser = dynamic_cast<MockProbeSizeRaiser*>(d6d.raiser.get());
@@ -205,7 +201,7 @@ TEST_F(QuicD6DStateFunctionsTest, D6DProbeAckedInSearchingMax) {
false,
true,
d6d.currentProbeSize);
- d6d.lastProbe = QuicConnectionStateBase::D6DProbePacket(
+ d6d.lastProbe = D6DProbePacket(
pkt.packet.header.getPacketSequenceNum(), pkt.metadata.encodedSize);
d6d.raiser = std::make_unique<MockProbeSizeRaiser>();
auto mockRaiser = dynamic_cast<MockProbeSizeRaiser*>(d6d.raiser.get());
@@ -232,7 +228,7 @@ TEST_F(QuicD6DStateFunctionsTest, D6DProbeAckedInError) {
false,
true,
d6d.currentProbeSize);
- d6d.lastProbe = QuicConnectionStateBase::D6DProbePacket(
+ d6d.lastProbe = D6DProbePacket(
pkt.packet.header.getPacketSequenceNum(), pkt.metadata.encodedSize);
d6d.raiser = std::make_unique<MockProbeSizeRaiser>();
auto mockRaiser = dynamic_cast<MockProbeSizeRaiser*>(d6d.raiser.get());

diff:
@@ -33,10 +33,6 @@ using namespace folly;
namespace quic {
namespace test {
-namespace {
-using D6DMachineState = QuicConnectionStateBase::D6DMachineState;
-} // namespace
-
class MockLossTimeout {
public:
MOCK_METHOD0(cancelLossTimeout, void());
@@ -265,8 +261,8 @@ PacketNum QuicLossFunctionsTest::sendPacket(
conn.pendingEvents.setLossDetectionAlarm = true;
if (isD6DProbe) {
++conn.d6d.outstandingProbes;
- conn.d6d.lastProbe = QuicConnectionStateBase::D6DProbePacket(
- encodedSize, conn.lossState.largestSent.value());
+ conn.d6d.lastProbe =
+ D6DProbePacket(encodedSize, conn.lossState.largestSent.value());
}
return conn.lossState.largestSent.value();
}
@@ -377,8 +373,8 @@ struct PMTUBlackholeDetectionTestFixture {
std::chrono::microseconds detectLossTimeDelta;
std::chrono::microseconds largestLostTimeDelta;
std::chrono::microseconds smallestLostTimeDelta;
- QuicConnectionStateBase::D6DMachineState beginState;
- QuicConnectionStateBase::D6DMachineState endState;
+ D6DMachineState beginState;
+ D6DMachineState endState;
struct PacketSpec {
std::chrono::microseconds timeDelta;
@@ -394,7 +390,7 @@ void QuicLossFunctionsTest::runDetectPMTUBlackholeTest(
// 0 rtt means all packets are automatically lost upon sending
conn->lossState.srtt = 0s;
conn->lossState.lrtt = 0s;
- conn->d6d.state = QuicConnectionStateBase::D6DMachineState::BASE;
+ conn->d6d.state = D6DMachineState::BASE;
conn->d6d.currentProbeSize = fixture.beginPMTU;
conn->udpSendPacketLen = fixture.beginPMTU;
conn->d6d.thresholdCounter =

diff:
@@ -513,16 +513,16 @@ void QuicServerTransport::maybeNotifyTransportReady() {
void QuicServerTransport::maybeStartD6DProbing() {
if (!d6dProbingStarted_ && hasReadCipher() &&
- conn_->d6d.state == QuicConnectionStateBase::D6DMachineState::BASE) {
+ conn_->d6d.state == D6DMachineState::BASE) {
QUIC_TRACE(fst_trace, *conn_, "start d6d probing");
d6dProbingStarted_ = true;
auto& d6d = conn_->d6d;
switch (conn_->transportSettings.d6dConfig.raiserType) {
- case D6DConfig::ProbeSizeRaiserType::ConstantStep:
+ case ProbeSizeRaiserType::ConstantStep:
d6d.raiser = std::make_unique<ConstantStepProbeSizeRaiser>(
conn_->transportSettings.d6dConfig.probeRaiserConstantStepSize);
break;
- case D6DConfig::ProbeSizeRaiserType::BinarySearch:
+ case ProbeSizeRaiserType::BinarySearch:
d6d.raiser = std::make_unique<BinarySearchProbeSizeRaiser>(
kMinMaxUDPPayload, d6d.maxPMTU);
}

diff:
@@ -234,9 +234,8 @@ void processClientInitialParams(
VLOG(10) << "conn.d6d.basePMTU=" << conn.d6d.basePMTU;
// Start from base
- conn.d6d.state = QuicConnectionStateBase::D6DMachineState::BASE;
- conn.d6d.meta.lastNonSearchState =
- QuicConnectionStateBase::D6DMachineState::DISABLED;
+ conn.d6d.state = D6DMachineState::BASE;
+ conn.d6d.meta.lastNonSearchState = D6DMachineState::DISABLED;
conn.d6d.meta.timeLastNonSearchState = Clock::now();
} else {
LOG(ERROR) << "client d6dBasePMTU fails sanity check: " << *d6dBasePMTU;

diff:
@@ -32,6 +32,7 @@ add_dependencies(
mvfst_codec
mvfst_codec_types
mvfst_handshake
+ mvfst_d6d_types
mvfst_d6d_probe_raiser
)
@@ -44,6 +45,7 @@ target_link_libraries(
mvfst_codec
mvfst_codec_types
mvfst_handshake
+ mvfst_d6d_types
mvfst_d6d_probe_raiser
)

diff:
@@ -772,35 +772,6 @@ struct QuicConnectionStateBase : public folly::DelayedDestruction {
HappyEyeballsState happyEyeballsState;
- struct D6DProbePacket {
- D6DProbePacket() = delete;
-
- explicit D6DProbePacket(PacketNum packetNumIn, uint32_t packetSizeIn)
- : packetNum(packetNumIn), packetSize(packetSizeIn) {}
-
- // Packet num
- PacketNum packetNum;
-
- // Udp packet payload size
- uint32_t packetSize;
- };
-
- // States of d6d state machine, see
- // https://tools.ietf.org/id/draft-ietf-tsvwg-datagram-plpmtud-21.html#name-state-machine
- enum class D6DMachineState : uint8_t {
- // Connection is not established yet
- DISABLED = 0,
- // Probe using base pmtu
- BASE = 1,
- // Incrementally probe using larger pmtu
- SEARCHING = 2,
- // Sleep for raise timeout before going to SEARCHING
- SEARCH_COMPLETE = 3,
- // Effective pmtu is less than base pmtu, continue probing with smaller
- // packet
- ERROR = 4
- };
-
// Meta state of d6d, mostly useful for analytics. D6D can operate without it.
struct D6DMetaState {
// Cumulative count of acked packets

diff:
#include <quic/QuicConstants.h>
#include <quic/codec/QuicConnectionId.h>
+#include <quic/d6d/Types.h>
#include <chrono>
namespace quic {
@@ -93,16 +94,6 @@ struct D6DConfig {
std::chrono::seconds blackholeDetectionWindow{
kDefaultD6DBlackholeDetectionWindow};
- /**
- * Two simple probe size raiser. Only server makes use of this value.
- * ConstantSize: raise pmtu at constant step size
- * BinarySearch: raise pmtu using binary search
- */
- enum class ProbeSizeRaiserType : uint8_t {
- ConstantStep = 0,
- BinarySearch = 1
- };
-
/**
* Default raiser is constant step , since overshot caused by binary
* search slows down convergence. Might change in the future when we

original_message:
Make commonly-used d6d types individual target

Summary: This reduces dependencies for both testing and instrumentation.

Reviewed By: mjoras

Differential Revision: D23997313

fbshipit-source-id: 5eb3a790c7bb2569dc1e941e3911ad4aac4e9258


diff:
#include <quic/QuicException.h>
#include <quic/common/SmallVec.h>
+#include <quic/d6d/Types.h>
#include <quic/state/OutstandingPacket.h>
namespace quic {
@@ -72,6 +73,62 @@ class InstrumentationObserver {
lastAckedPacketInfo;
};
+ struct PMTUBlackholeEvent {
+ explicit PMTUBlackholeEvent(
+ TimePoint blackholeTimeIn,
+ std::chrono::microseconds timeSinceLastNonSearchStateIn,
+ D6DMachineState lastNonSearchStateIn,
+ D6DMachineState currentStateIn,
+ uint64_t udpSendPacketLenIn,
+ uint64_t lastProbeSizeIn,
+ uint64_t blackholeDetectionWindowIn,
+ uint64_t blackholeDetectionThresholdIn,
+ const quic::OutstandingPacket& pkt)
+ : blackholeTime(blackholeTimeIn),
+ timeSinceLastNonSearchState(timeSinceLastNonSearchStateIn),
+ lastNonSearchState(lastNonSearchStateIn),
+ currentState(currentStateIn),
+ udpSendPacketLen(udpSendPacketLenIn),
+ lastProbeSize(lastProbeSizeIn),
+ blackholeDetectionWindow(blackholeDetectionWindowIn),
+ blackholeDetectionThreshold(blackholeDetectionThresholdIn),
+ triggeringPacketMetadata(pkt.metadata) {}
+ TimePoint blackholeTime;
+ // How long since last "stable" state
+ std::chrono::microseconds timeSinceLastNonSearchState;
+ D6DMachineState lastNonSearchState;
+ D6DMachineState currentState;
+ uint64_t udpSendPacketLen;
+ uint64_t lastProbeSize;
+ uint64_t blackholeDetectionWindow;
+ uint64_t blackholeDetectionThreshold;
+ // The metadata of the packet that triggerred blackhole signal
+ const quic::OutstandingPacketMetadata triggeringPacketMetadata;
+ };
+
+ struct PMTUUpperBoundEvent {
+ explicit PMTUUpperBoundEvent(
+ TimePoint upperBoundTimeIn,
+ std::chrono::microseconds timeSinceLastNonSearchStateIn,
+ D6DMachineState lastNonSearchStateIn,
+ uint64_t upperBoundPMTUIn,
+ uint64_t cumulativeProbesSentIn,
+ ProbeSizeRaiserType probeSizeRaiserTypeIn)
+ : upperBoundTime(upperBoundTimeIn),
+ timeSinceLastNonSearchState(timeSinceLastNonSearchStateIn),
+ lastNonSearchState(lastNonSearchStateIn),
+ upperBoundPMTU(upperBoundPMTUIn),
+ cumulativeProbesSent(cumulativeProbesSentIn),
+ probeSizeRaiserType(probeSizeRaiserTypeIn) {}
+ TimePoint upperBoundTime;
+ // How long it took to reach upperbound
+ std::chrono::microseconds timeSinceLastNonSearchState;
+ D6DMachineState lastNonSearchState;
+ uint64_t upperBoundPMTU;
+ uint64_t cumulativeProbesSent;
+ ProbeSizeRaiserType probeSizeRaiserType;
+ };
+
virtual ~InstrumentationObserver() = default;
/**
@@ -110,6 +167,24 @@ class InstrumentationObserver {
virtual void rttSampleGenerated(
QuicSocket*, /* socket */
const PacketRTT& /* RTT sample */) {}
+
+ /**
+ * pmtuBlackholeDetected() is invoked when a PMTU blackhole is detected.
+ *
+ * @param pmtuBlackholeEvent const reference to the PMTU blackhole event
+ */
+ virtual void pmtuBlackholeDetected(
+ QuicSocket*, /* socket */
+ const PMTUBlackholeEvent& /* pmtuBlackholeEvent */) {}
+
+ /**
+ * pmtuUpperBoundDetected() is invoked when a PMTU upperbound is detected.
+ *
+ * @param pmtuUpperBoundEvent const reference to the PMTU upperbound event
+ */
+ virtual void pmtuUpperBoundDetected(
+ QuicSocket*, /* socket */
+ const PMTUUpperBoundEvent& /* pmtuUpperBoundEvent */) {}
};
// Container for instrumentation observers.

diff:
@@ -338,6 +338,18 @@ class MockInstrumentationObserver : public InstrumentationObserver {
,
rttSampleGenerated,
void(QuicSocket*, const PacketRTT&));
+ GMOCK_METHOD2_(
+ ,
+ noexcept,
+ ,
+ pmtuBlackholeDetected,
+ void(QuicSocket*, const PMTUBlackholeEvent&));
+ GMOCK_METHOD2_(
+ ,
+ noexcept,
+ ,
+ pmtuUpperBoundDetected,
+ void(QuicSocket*, const PMTUUpperBoundEvent&));
static auto getLossPacketMatcher(bool reorderLoss, bool timeoutLoss) {
return AllOf(

diff:
@@ -16,6 +16,67 @@ inline void scheduleProbeAfterDelay(
conn.pendingEvents.d6d.sendProbeDelay = delay;
}
+/**
+ * A helper function that reports PMTU Upper Bound event.
+ * Returns the event timestamp
+ */
+static TimePoint reportUpperBound(QuicConnectionStateBase& conn) {
+ auto& d6d = conn.d6d;
+ const auto lastProbeSize = d6d.lastProbe->packetSize;
+ const auto now = Clock::now();
+ if (conn.instrumentationObservers_.size() > 0) {
+ InstrumentationObserver::PMTUUpperBoundEvent upperBoundEvent(
+ now,
+ std::chrono::duration_cast<std::chrono::microseconds>(
+ now - d6d.meta.timeLastNonSearchState),
+ d6d.meta.lastNonSearchState,
+ lastProbeSize,
+ d6d.meta.totalTxedProbes,
+ conn.transportSettings.d6dConfig.raiserType);
+ // enqueue a function for every observer to invoke callback
+ for (const auto& observer : conn.instrumentationObservers_) {
+ conn.pendingCallbacks.emplace_back(
+ [observer, upperBoundEvent](QuicSocket* qSocket) {
+ observer->pmtuUpperBoundDetected(qSocket, upperBoundEvent);
+ });
+ }
+ }
+ return now;
+}
+
+/**
+ * Helper function that reports PMTU blackhole event.
+ * Returns the event timestamp
+ */
+static TimePoint reportBlackhole(
+ QuicConnectionStateBase& conn,
+ const OutstandingPacket& packet) {
+ auto& d6d = conn.d6d;
+ const auto now = Clock::now();
+ if (conn.instrumentationObservers_.size() > 0) {
+ InstrumentationObserver::PMTUBlackholeEvent blackholeEvent(
+ now,
+ std::chrono::duration_cast<std::chrono::microseconds>(
+ now - d6d.meta.timeLastNonSearchState),
+ d6d.meta.lastNonSearchState,
+ d6d.state,
+ conn.udpSendPacketLen,
+ d6d.lastProbe->packetSize,
+ d6d.thresholdCounter->getWindow(),
+ d6d.thresholdCounter->getThreshold(),
+ packet);
+
+ // If there are observers, enqueue a function to invoke callback
+ for (const auto& observer : conn.instrumentationObservers_) {
+ conn.pendingCallbacks.emplace_back(
+ [observer, blackholeEvent](QuicSocket* qSocket) {
+ observer->pmtuBlackholeDetected(qSocket, blackholeEvent);
+ });
+ }
+ }
+ return now;
+}
+
void onD6DProbeTimeoutExpired(QuicConnectionStateBase& conn) {
onD6DLastProbeLost(conn);
}
@@ -46,6 +107,8 @@ void onD6DLastProbeAcked(QuicConnectionStateBase& conn) {
CHECK(maybeNextProbeSize.hasValue());
d6d.currentProbeSize = *maybeNextProbeSize;
d6d.state = D6DMachineState::SEARCHING;
+ d6d.meta.lastNonSearchState = D6DMachineState::BASE;
+ d6d.meta.timeLastNonSearchState = Clock::now();
scheduleProbeAfterDelay(conn, kDefaultD6DProbeDelayWhenAcked);
break;
case D6DMachineState::SEARCHING:
@@ -67,6 +130,7 @@ void onD6DLastProbeAcked(QuicConnectionStateBase& conn) {
d6d.state = D6DMachineState::SEARCH_COMPLETE;
conn.pendingEvents.d6d.scheduleRaiseTimeout = true;
conn.pendingEvents.d6d.scheduleProbeTimeout = false;
+ reportUpperBound(conn);
}
break;
case D6DMachineState::ERROR:
@@ -74,6 +138,8 @@ void onD6DLastProbeAcked(QuicConnectionStateBase& conn) {
// We should try sending base pmtu-sized packet now.
d6d.currentProbeSize = d6d.basePMTU;
d6d.state = D6DMachineState::BASE;
+ d6d.meta.lastNonSearchState = D6DMachineState::ERROR;
+ d6d.meta.timeLastNonSearchState = Clock::now();
scheduleProbeAfterDelay(conn, kDefaultD6DProbeDelayWhenAcked);
break;
default:
@@ -90,6 +156,8 @@ void onD6DLastProbeLost(QuicConnectionStateBase& conn) {
// remedy is to probe much smaller
d6d.state = D6DMachineState::ERROR;
d6d.currentProbeSize = kMinMaxUDPPayload;
+ d6d.meta.lastNonSearchState = D6DMachineState::BASE;
+ d6d.meta.timeLastNonSearchState = Clock::now();
// TODO: reduce udpSendPacketLen below base once we can trust
// this signal. Currently assuming quic is going to terminate
@@ -104,6 +172,8 @@ void onD6DLastProbeLost(QuicConnectionStateBase& conn) {
// that the upper bound is reached
d6d.state = D6DMachineState::SEARCH_COMPLETE;
conn.pendingEvents.d6d.scheduleRaiseTimeout = true;
+ conn.pendingEvents.d6d.scheduleProbeTimeout = false;
+ reportUpperBound(conn);
return;
}
// Otherwise, the loss could be due to congestion, so we keep
@@ -149,10 +219,15 @@ void detectPMTUBlackhole(
.count())) {
LOG(ERROR) << "PMTU blackhole detected on packet loss, reducing PMTU from "
<< conn.udpSendPacketLen << " to base " << d6d.basePMTU;
+
+ auto eventTime = reportBlackhole(conn, packet);
+ if (d6d.state != D6DMachineState::SEARCHING) {
+ d6d.meta.lastNonSearchState = d6d.state;
+ d6d.meta.timeLastNonSearchState = eventTime;
+ }
d6d.state = D6DMachineState::BASE;
d6d.currentProbeSize = d6d.basePMTU;
conn.udpSendPacketLen = d6d.basePMTU;
-
// Cancel existing raise timeout if any
conn.pendingEvents.d6d.scheduleRaiseTimeout = false;
}

diff:
#include <gmock/gmock.h>
#include <gtest/gtest.h>
+#include <quic/api/test/Mocks.h>
#include <quic/common/test/TestUtils.h>
#include <quic/d6d/QuicD6DStateFunctions.h>
#include <quic/d6d/test/Mocks.h>
#include <quic/state/StateData.h>
using namespace testing;
+using namespace std::literals::chrono_literals;
namespace quic {
namespace test {
+// timeLastNonSearchState can only increase
+enum class TimeLastNonSearchStateEnd : uint8_t { EQ, GE };
+
struct D6DProbeLostTestFixture {
D6DMachineState stateBegin;
D6DMachineState stateEnd;
@@ -29,6 +34,11 @@ struct D6DProbeLostTestFixture {
uint64_t outstandingProbes;
uint32_t currentProbeSizeBegin;
uint32_t currentProbeSizeEnd;
+ D6DProbePacket lastProbe;
+ D6DMachineState lastNonSearchStateBegin;
+ D6DMachineState lastNonSearchStateEnd;
+ TimePoint timeLastNonSearchStateBegin;
+ TimeLastNonSearchStateEnd timeLastNonSearchStateEndE;
};
RegularQuicWritePacket makeTestShortPacket() {
@@ -46,7 +56,10 @@ class QuicD6DStateFunctionsTest : public Test {
conn.d6d.state = fixture.stateBegin;
conn.d6d.outstandingProbes = fixture.outstandingProbes;
conn.d6d.currentProbeSize = fixture.currentProbeSizeBegin;
+ conn.d6d.lastProbe = fixture.lastProbe;
conn.pendingEvents.d6d.sendProbePacket = fixture.sendProbeBegin;
+ conn.d6d.meta.lastNonSearchState = fixture.lastNonSearchStateBegin;
+ conn.d6d.meta.timeLastNonSearchState = fixture.timeLastNonSearchStateBegin;
onD6DLastProbeLost(conn);
EXPECT_EQ(conn.d6d.state, fixture.stateEnd);
EXPECT_EQ(conn.d6d.currentProbeSize, fixture.currentProbeSizeEnd);
@@ -57,11 +70,24 @@ class QuicD6DStateFunctionsTest : public Test {
} else {
ASSERT_FALSE(conn.pendingEvents.d6d.sendProbeDelay.hasValue());
}
+ EXPECT_EQ(conn.d6d.meta.lastNonSearchState, fixture.lastNonSearchStateEnd);
+ switch (fixture.timeLastNonSearchStateEndE) {
+ case TimeLastNonSearchStateEnd::EQ:
+ EXPECT_EQ(
+ conn.d6d.meta.timeLastNonSearchState,
+ fixture.timeLastNonSearchStateBegin);
+ break;
+ default:
+ EXPECT_GE(
+ conn.d6d.meta.timeLastNonSearchState,
+ fixture.timeLastNonSearchStateBegin);
+ }
}
};
TEST_F(QuicD6DStateFunctionsTest, D6DProbeTimeoutExpiredOneInBase) {
QuicConnectionStateBase conn(QuicNodeType::Server);
+ auto now = Clock::now();
// One probe lost in BASE state
D6DProbeLostTestFixture oneProbeLostInBase = {
D6DMachineState::BASE, // stateBegin
@@ -70,13 +96,19 @@ TEST_F(QuicD6DStateFunctionsTest, D6DProbeTimeoutExpiredOneInBase) {
kDefaultD6DProbeDelayWhenLost, // sendProbeEnd
1, // outstandingProbes
conn.d6d.basePMTU, // currentProbeSizeBegin
- conn.d6d.basePMTU // currentProbeSizeEnd
+ conn.d6d.basePMTU, // currentProbeSizeEnd
+ D6DProbePacket(0, conn.d6d.basePMTU + 10),
+ D6DMachineState::DISABLED, // lastNonSearchStateBegin
+ D6DMachineState::DISABLED, // lastNonSearchStateEnd
+ now, // timeLastNonSearchStateBegin
+ TimeLastNonSearchStateEnd::EQ // timeLastNonSearchStateEndE
};
runD6DProbeLostTest(conn, oneProbeLostInBase);
}
TEST_F(QuicD6DStateFunctionsTest, D6DProbeTimeoutExpiredMaxInBase) {
QuicConnectionStateBase conn(QuicNodeType::Server);
+ auto now = Clock::now();
// max number of probes lost in BASE state
D6DProbeLostTestFixture maxNumProbesLostInBase = {
D6DMachineState::BASE,
@@ -85,12 +117,18 @@ TEST_F(QuicD6DStateFunctionsTest, D6DProbeTimeoutExpiredMaxInBase) {
kDefaultD6DProbeDelayWhenLost,
kDefaultD6DMaxOutstandingProbes,
conn.d6d.basePMTU,
- kMinMaxUDPPayload};
+ kMinMaxUDPPayload,
+ D6DProbePacket(0, conn.d6d.basePMTU + 10),
+ D6DMachineState::DISABLED,
+ D6DMachineState::BASE,
+ now,
+ TimeLastNonSearchStateEnd::GE};
runD6DProbeLostTest(conn, maxNumProbesLostInBase);
}
TEST_F(QuicD6DStateFunctionsTest, D6DProbeTimeoutExpiredOneInSearching) {
QuicConnectionStateBase conn(QuicNodeType::Server);
+ auto now = Clock::now();
// One probe lots in SEARCHING state
D6DProbeLostTestFixture oneProbeLostInSearching = {
D6DMachineState::SEARCHING,
@@ -99,12 +137,18 @@ TEST_F(QuicD6DStateFunctionsTest, D6DProbeTimeoutExpiredOneInSearching) {
kDefaultD6DProbeDelayWhenLost,
1,
static_cast<uint32_t>(conn.d6d.basePMTU + 10),
- static_cast<uint32_t>(conn.d6d.basePMTU + 10)};
+ static_cast<uint32_t>(conn.d6d.basePMTU + 10),
+ D6DProbePacket(0, conn.d6d.basePMTU + 10),
+ D6DMachineState::BASE,
+ D6DMachineState::BASE,
+ now,
+ TimeLastNonSearchStateEnd::EQ};
runD6DProbeLostTest(conn, oneProbeLostInSearching);
}
TEST_F(QuicD6DStateFunctionsTest, D6DProbeTimeoutExpiredMaxInSearching) {
QuicConnectionStateBase conn(QuicNodeType::Server);
+ auto now = Clock::now();
// Max number of probes lost in SEARCHING state
D6DProbeLostTestFixture maxProbesLostInSearching = {
D6DMachineState::SEARCHING,
@@ -113,12 +157,18 @@ TEST_F(QuicD6DStateFunctionsTest, D6DProbeTimeoutExpiredMaxInSearching) {
folly::none,
kDefaultD6DMaxOutstandingProbes,
static_cast<uint32_t>(conn.d6d.basePMTU + 10),
- static_cast<uint32_t>(conn.d6d.basePMTU + 10)};
+ static_cast<uint32_t>(conn.d6d.basePMTU + 10),
+ D6DProbePacket(0, conn.d6d.basePMTU + 10),
+ D6DMachineState::BASE,
+ D6DMachineState::BASE,
+ now,
+ TimeLastNonSearchStateEnd::EQ};
runD6DProbeLostTest(conn, maxProbesLostInSearching);
}
TEST_F(QuicD6DStateFunctionsTest, D6DProbeTimeoutExpiredOneInError) {
QuicConnectionStateBase conn(QuicNodeType::Server);
+ auto now = Clock::now();
// Probe lost in ERROR state
D6DProbeLostTestFixture probeLostInError = {
D6DMachineState::ERROR,
@@ -127,7 +177,12 @@ TEST_F(QuicD6DStateFunctionsTest, D6DProbeTimeoutExpiredOneInError) {
kDefaultD6DProbeDelayWhenLost,
kDefaultD6DMaxOutstandingProbes + 1,
kMinMaxUDPPayload,
- kMinMaxUDPPayload};
+ kMinMaxUDPPayload,
+ D6DProbePacket(0, conn.d6d.basePMTU + 10),
+ D6DMachineState::BASE,
+ D6DMachineState::BASE,
+ now,
+ TimeLastNonSearchStateEnd::EQ};
runD6DProbeLostTest(conn, probeLostInError);
}
@@ -135,15 +190,22 @@ TEST_F(QuicD6DStateFunctionsTest, D6DProbeAckedInBase) {
QuicConnectionStateBase conn(QuicNodeType::Server);
const uint16_t expectPMTU = 1400;
auto& d6d = conn.d6d;
+ auto now = Clock::now();
+ auto mockInstrumentationObserver =
+ std::make_unique<StrictMock<MockInstrumentationObserver>>();
+ conn.instrumentationObservers_.push_back(mockInstrumentationObserver.get());
d6d.state = D6DMachineState::BASE;
d6d.outstandingProbes = 1;
d6d.currentProbeSize = d6d.basePMTU;
+ d6d.meta.lastNonSearchState = D6DMachineState::DISABLED;
+ d6d.meta.timeLastNonSearchState = now;
auto pkt = OutstandingPacket(
makeTestShortPacket(),
Clock::now(),
d6d.currentProbeSize,
false,
true,
+ d6d.currentProbeSize,
d6d.currentProbeSize);
d6d.lastProbe = D6DProbePacket(
pkt.packet.header.getPacketSequenceNum(), pkt.metadata.encodedSize);
@@ -152,26 +214,40 @@ TEST_F(QuicD6DStateFunctionsTest, D6DProbeAckedInBase) {
EXPECT_CALL(*mockRaiser, raiseProbeSize(d6d.currentProbeSize))
.Times(1)
.WillOnce(Return(expectPMTU));
+ EXPECT_CALL(*mockInstrumentationObserver, pmtuUpperBoundDetected(_, _))
+ .Times(0);
onD6DLastProbeAcked(conn);
+ for (auto& callback : conn.pendingCallbacks) {
+ callback(nullptr);
+ }
EXPECT_EQ(d6d.state, D6DMachineState::SEARCHING);
EXPECT_EQ(d6d.currentProbeSize, expectPMTU);
EXPECT_EQ(conn.udpSendPacketLen, d6d.basePMTU);
+ EXPECT_EQ(d6d.meta.lastNonSearchState, D6DMachineState::BASE);
+ EXPECT_GE(d6d.meta.timeLastNonSearchState, now);
}
TEST_F(QuicD6DStateFunctionsTest, D6DProbeAckedInSearchingOne) {
QuicConnectionStateBase conn(QuicNodeType::Server);
const uint16_t expectPMTU = 1400;
auto& d6d = conn.d6d;
+ auto now = Clock::now();
+ auto mockInstrumentationObserver =
+ std::make_unique<StrictMock<MockInstrumentationObserver>>();
+ conn.instrumentationObservers_.push_back(mockInstrumentationObserver.get());
d6d.state = D6DMachineState::SEARCHING;
d6d.outstandingProbes = 1;
conn.udpSendPacketLen = 1250;
d6d.currentProbeSize = 1300;
+ d6d.meta.lastNonSearchState = D6DMachineState::BASE;
+ d6d.meta.timeLastNonSearchState = now;
auto pkt = OutstandingPacket(
makeTestShortPacket(),
Clock::now(),
d6d.currentProbeSize,
false,
true,
+ d6d.currentProbeSize,
d6d.currentProbeSize);
d6d.lastProbe = D6DProbePacket(
pkt.packet.header.getPacketSequenceNum(), pkt.metadata.encodedSize);
@@ -180,26 +256,41 @@ TEST_F(QuicD6DStateFunctionsTest, D6DProbeAckedInSearchingOne) {
EXPECT_CALL(*mockRaiser, raiseProbeSize(d6d.currentProbeSize))
.Times(1)
.WillOnce(Return(expectPMTU));
+ EXPECT_CALL(*mockInstrumentationObserver, pmtuUpperBoundDetected(_, _))
+ .Times(0);
onD6DLastProbeAcked(conn);
+ for (auto& callback : conn.pendingCallbacks) {
+ callback(nullptr);
+ }
EXPECT_EQ(d6d.state, D6DMachineState::SEARCHING);
EXPECT_EQ(d6d.currentProbeSize, expectPMTU);
EXPECT_EQ(conn.udpSendPacketLen, 1300);
+ EXPECT_EQ(d6d.meta.lastNonSearchState, D6DMachineState::BASE);
+ EXPECT_EQ(d6d.meta.timeLastNonSearchState, now);
}
TEST_F(QuicD6DStateFunctionsTest, D6DProbeAckedInSearchingMax) {
QuicConnectionStateBase conn(QuicNodeType::Server);
const uint16_t oversize = 1500;
auto& d6d = conn.d6d;
+ auto now = Clock::now();
+ auto mockInstrumentationObserver =
+ std::make_unique<StrictMock<MockInstrumentationObserver>>();
+ conn.instrumentationObservers_.push_back(mockInstrumentationObserver.get());
d6d.state = D6DMachineState::SEARCHING;
d6d.outstandingProbes = 3;
conn.udpSendPacketLen = 1400;
d6d.currentProbeSize = 1450;
+ d6d.meta.lastNonSearchState = D6DMachineState::BASE;
+ d6d.meta.timeLastNonSearchState = now;
+ d6d.meta.totalTxedProbes = 10;
auto pkt = OutstandingPacket(
makeTestShortPacket(),
Clock::now(),
d6d.currentProbeSize,
false,
true,
+ d6d.currentProbeSize,
d6d.currentProbeSize);
d6d.lastProbe = D6DProbePacket(
pkt.packet.header.getPacketSequenceNum(), pkt.metadata.encodedSize);
@@ -208,25 +299,50 @@ TEST_F(QuicD6DStateFunctionsTest, D6DProbeAckedInSearchingMax) {
EXPECT_CALL(*mockRaiser, raiseProbeSize(d6d.currentProbeSize))
.Times(1)
.WillOnce(Return(oversize));
+ EXPECT_CALL(*mockInstrumentationObserver, pmtuUpperBoundDetected(_, _))
+ .Times(1)
+ .WillOnce(Invoke(
+ [&](QuicSocket* /* qSocket */,
+ const InstrumentationObserver::PMTUUpperBoundEvent& event) {
+ EXPECT_LT(now, event.upperBoundTime);
+ EXPECT_LT(0us, event.timeSinceLastNonSearchState);
+ EXPECT_EQ(D6DMachineState::BASE, event.lastNonSearchState);
+ EXPECT_EQ(1450, event.upperBoundPMTU);
+ EXPECT_EQ(10, event.cumulativeProbesSent);
+ EXPECT_EQ(
+ ProbeSizeRaiserType::ConstantStep, event.probeSizeRaiserType);
+ }));
onD6DLastProbeAcked(conn);
+ for (auto& callback : conn.pendingCallbacks) {
+ callback(nullptr);
+ }
EXPECT_EQ(d6d.state, D6DMachineState::SEARCH_COMPLETE);
EXPECT_EQ(d6d.currentProbeSize, 1450);
EXPECT_EQ(conn.udpSendPacketLen, 1450);
+ EXPECT_EQ(d6d.meta.lastNonSearchState, D6DMachineState::BASE);
+ EXPECT_EQ(d6d.meta.timeLastNonSearchState, now);
}
TEST_F(QuicD6DStateFunctionsTest, D6DProbeAckedInError) {
QuicConnectionStateBase conn(QuicNodeType::Server);
auto& d6d = conn.d6d;
+ auto now = Clock::now();
+ auto mockInstrumentationObserver =
+ std::make_unique<StrictMock<MockInstrumentationObserver>>();
+ conn.instrumentationObservers_.push_back(mockInstrumentationObserver.get());
d6d.state = D6DMachineState::ERROR;
d6d.outstandingProbes = 3;
conn.udpSendPacketLen = d6d.basePMTU;
d6d.currentProbeSize = d6d.basePMTU - 20;
+ d6d.meta.lastNonSearchState = D6DMachineState::BASE;
+ d6d.meta.timeLastNonSearchState = now;
auto pkt = OutstandingPacket(
makeTestShortPacket(),
Clock::now(),
d6d.currentProbeSize,
false,
true,
+ d6d.currentProbeSize,
d6d.currentProbeSize);
d6d.lastProbe = D6DProbePacket(
pkt.packet.header.getPacketSequenceNum(), pkt.metadata.encodedSize);
@@ -235,10 +351,144 @@ TEST_F(QuicD6DStateFunctionsTest, D6DProbeAckedInError) {
EXPECT_CALL(*mockRaiser, raiseProbeSize(d6d.currentProbeSize))
.Times(1)
.WillOnce(Return(1300)); // Won't be used
+ EXPECT_CALL(*mockInstrumentationObserver, pmtuUpperBoundDetected(_, _))
+ .Times(0);
onD6DLastProbeAcked(conn);
+ for (auto& callback : conn.pendingCallbacks) {
+ callback(nullptr);
+ }
+ EXPECT_EQ(d6d.state, D6DMachineState::BASE);
+ EXPECT_EQ(d6d.currentProbeSize, d6d.basePMTU);
+ EXPECT_EQ(conn.udpSendPacketLen, d6d.basePMTU);
+ EXPECT_EQ(d6d.meta.lastNonSearchState, D6DMachineState::ERROR);
+ EXPECT_GE(d6d.meta.timeLastNonSearchState, now);
+}
+
+TEST_F(QuicD6DStateFunctionsTest, BlackholeInSearching) {
+ QuicConnectionStateBase conn(QuicNodeType::Server);
+ auto& d6d = conn.d6d;
+ auto now = Clock::now();
+ auto mockInstrumentationObserver =
+ std::make_unique<StrictMock<MockInstrumentationObserver>>();
+ conn.instrumentationObservers_.push_back(mockInstrumentationObserver.get());
+ d6d.state = D6DMachineState::SEARCHING;
+ d6d.outstandingProbes = 2;
+ conn.udpSendPacketLen = d6d.basePMTU + 20;
+ d6d.currentProbeSize = d6d.basePMTU + 30;
+ d6d.meta.lastNonSearchState = D6DMachineState::BASE;
+ d6d.meta.timeLastNonSearchState = now;
+ auto pkt = OutstandingPacket(
+ makeTestShortPacket(),
+ now + 10s,
+ d6d.currentProbeSize,
+ false,
+ true,
+ d6d.currentProbeSize,
+ d6d.currentProbeSize);
+ d6d.lastProbe = D6DProbePacket(
+ pkt.packet.header.getPacketSequenceNum(), pkt.metadata.encodedSize);
+
+ auto lostPacket = OutstandingPacket(
+ makeTestShortPacket(),
+ now + 8s,
+ conn.udpSendPacketLen,
+ false,
+ conn.udpSendPacketLen + d6d.currentProbeSize,
+ conn.udpSendPacketLen + d6d.currentProbeSize);
+
+ d6d.thresholdCounter = std::make_unique<WindowedCounter<uint64_t, uint64_t>>(
+ std::chrono::microseconds(kDefaultD6DBlackholeDetectionWindow).count(),
+ 1); // Threshold of 1 will cause window to be set to 0
+
+ EXPECT_CALL(*mockInstrumentationObserver, pmtuBlackholeDetected(_, _))
+ .Times(1)
+ .WillOnce(
+ Invoke([&](QuicSocket* /* qSocket */,
+ const InstrumentationObserver::PMTUBlackholeEvent& event) {
+ EXPECT_LE(d6d.meta.timeLastNonSearchState, event.blackholeTime);
+ EXPECT_EQ(D6DMachineState::BASE, event.lastNonSearchState);
+ EXPECT_EQ(D6DMachineState::SEARCHING, event.currentState);
+ EXPECT_EQ(d6d.basePMTU + 20, event.udpSendPacketLen);
+ EXPECT_EQ(d6d.basePMTU + 30, event.lastProbeSize);
+ EXPECT_EQ(0, event.blackholeDetectionWindow);
+ EXPECT_EQ(1, event.blackholeDetectionThreshold);
+ EXPECT_EQ(
+ d6d.basePMTU + 20, event.triggeringPacketMetadata.encodedSize);
+ }));
+
+ detectPMTUBlackhole(conn, lostPacket);
+ for (auto& callback : conn.pendingCallbacks) {
+ callback(nullptr);
+ }
+
+ EXPECT_EQ(d6d.state, D6DMachineState::BASE);
+ EXPECT_EQ(d6d.currentProbeSize, d6d.basePMTU);
+ EXPECT_EQ(conn.udpSendPacketLen, d6d.basePMTU);
+ EXPECT_EQ(d6d.meta.lastNonSearchState, D6DMachineState::BASE);
+ EXPECT_GE(d6d.meta.timeLastNonSearchState, now);
+}
+
+TEST_F(QuicD6DStateFunctionsTest, BlackholeInSearchComplete) {
+ QuicConnectionStateBase conn(QuicNodeType::Server);
+ auto& d6d = conn.d6d;
+ auto now = Clock::now();
+ auto mockInstrumentationObserver =
+ std::make_unique<StrictMock<MockInstrumentationObserver>>();
+ conn.instrumentationObservers_.push_back(mockInstrumentationObserver.get());
+ d6d.state = D6DMachineState::SEARCH_COMPLETE;
+ conn.udpSendPacketLen = d6d.basePMTU + 20;
+ d6d.currentProbeSize = d6d.basePMTU + 20;
+ d6d.meta.lastNonSearchState = D6DMachineState::BASE;
+ d6d.meta.timeLastNonSearchState = now;
+ auto pkt = OutstandingPacket(
+ makeTestShortPacket(),
+ now + 10s,
+ d6d.currentProbeSize,
+ false,
+ true,
+ d6d.currentProbeSize,
+ d6d.currentProbeSize);
+ d6d.lastProbe = D6DProbePacket(
+ pkt.packet.header.getPacketSequenceNum(), pkt.metadata.encodedSize);
+
+ auto lostPacket = OutstandingPacket(
+ makeTestShortPacket(),
+ now + 12s,
+ conn.udpSendPacketLen,
+ false,
+ conn.udpSendPacketLen + d6d.currentProbeSize,
+ conn.udpSendPacketLen + d6d.currentProbeSize);
+
+ d6d.thresholdCounter = std::make_unique<WindowedCounter<uint64_t, uint64_t>>(
+ std::chrono::microseconds(kDefaultD6DBlackholeDetectionWindow).count(),
+ 1); // Threshold of 1 will cause window to be set to 0
+
+ EXPECT_CALL(*mockInstrumentationObserver, pmtuBlackholeDetected(_, _))
+ .Times(1)
+ .WillOnce(
+ Invoke([&](QuicSocket* /* qSocket */,
+ const InstrumentationObserver::PMTUBlackholeEvent& event) {
+ EXPECT_EQ(d6d.meta.timeLastNonSearchState, event.blackholeTime);
+ EXPECT_EQ(D6DMachineState::BASE, event.lastNonSearchState);
+ EXPECT_EQ(D6DMachineState::SEARCH_COMPLETE, event.currentState);
+ EXPECT_EQ(d6d.basePMTU + 20, event.udpSendPacketLen);
+ EXPECT_EQ(d6d.basePMTU + 20, event.lastProbeSize);
+ EXPECT_EQ(0, event.blackholeDetectionWindow);
+ EXPECT_EQ(1, event.blackholeDetectionThreshold);
+ EXPECT_EQ(
+ d6d.basePMTU + 20, event.triggeringPacketMetadata.encodedSize);
+ }));
+
+ detectPMTUBlackhole(conn, lostPacket);
+ for (auto& callback : conn.pendingCallbacks) {
+ callback(nullptr);
+ }
+
EXPECT_EQ(d6d.state, D6DMachineState::BASE);
EXPECT_EQ(d6d.currentProbeSize, d6d.basePMTU);
EXPECT_EQ(conn.udpSendPacketLen, d6d.basePMTU);
+ EXPECT_EQ(d6d.meta.lastNonSearchState, D6DMachineState::SEARCH_COMPLETE);
+ EXPECT_GE(d6d.meta.timeLastNonSearchState, now);
}
} // namespace test

original_message:
Add instrumentation callback for pmtu blackhole and upperbound

Summary:
This adds two events that we care mostly about d6d's perf: detecting blackhole
and detecting upperbound.

Reviewed By: bschlinker

Differential Revision: D23972141

fbshipit-source-id: bb6db6d1548ecddc9b255149e04a4c4844e9ec54


diff:
@@ -24,6 +24,9 @@ static TimePoint reportUpperBound(QuicConnectionStateBase& conn) {
auto& d6d = conn.d6d;
const auto lastProbeSize = d6d.lastProbe->packetSize;
const auto now = Clock::now();
+
+ QUIC_STATS(
+ conn.statsCallback, onConnectionPMTUUpperBoundDetected, lastProbeSize);
if (conn.instrumentationObservers_.size() > 0) {
InstrumentationObserver::PMTUUpperBoundEvent upperBoundEvent(
now,
@@ -51,6 +54,10 @@ static TimePoint reportUpperBound(QuicConnectionStateBase& conn) {
static TimePoint reportBlackhole(
QuicConnectionStateBase& conn,
const OutstandingPacket& packet) {
+ QUIC_STATS(
+ conn.statsCallback,
+ onConnectionPMTUBlackholeDetected,
+ packet.metadata.encodedSize);
auto& d6d = conn.d6d;
const auto now = Clock::now();
if (conn.instrumentationObservers_.size() > 0) {
@@ -117,6 +124,7 @@ void onD6DLastProbeAcked(QuicConnectionStateBase& conn) {
LOG(ERROR) << "D6D lastProbeSize <= udpSendPacketLen";
return;
}
+ QUIC_STATS(conn.statsCallback, onConnectionPMTURaised);
conn.udpSendPacketLen = lastProbeSize;
if (maybeNextProbeSize.hasValue() &&
*maybeNextProbeSize > conn.udpSendPacketLen &&

diff:
@@ -149,6 +149,26 @@ class LogQuicStats : public quic::QuicTransportStatsCallback {
<< "onUDPSocketWriteError errorType=" << toString(errorType);
}
+ void onConnectionD6DStarted() override {
+ VLOG(2) << prefix_ << "onConnectionD6DStarted";
+ }
+
+ void onConnectionPMTURaised() override {
+ VLOG(2) << prefix_ << "onConnectionPMTURaised";
+ }
+
+ void onConnectionPMTUBlackholeDetected(
+ uint64_t pmtuAtBlackholeTime) override {
+ VLOG(2) << prefix_
+ << "onConnectionPMTUBlackholeDetected pmtuAtBlackholeTime="
+ << pmtuAtBlackholeTime;
+ }
+
+ void onConnectionPMTUUpperBoundDetected(uint64_t pmtuUpperBound) override {
+ VLOG(2) << prefix_ << "onConnectionPMTUUpperBoundDetected pmtuUpperBound="
+ << pmtuUpperBound;
+ }
+
private:
std::string prefix_;
};

diff:
@@ -536,6 +536,7 @@ void QuicServerTransport::maybeStartD6DProbing() {
// connections, for which probing is relatively expensive and less
// valuable
conn_->pendingEvents.d6d.sendProbeDelay = kDefaultD6DKickStartDelay;
+ QUIC_STATS(conn_->statsCallback, onConnectionD6DStarted);
}
}

diff:
@@ -136,6 +136,15 @@ class QuicTransportStatsCallback {
virtual void onUDPSocketWriteError(SocketErrorType errorType) = 0;
+ virtual void onConnectionD6DStarted() = 0;
+
+ virtual void onConnectionPMTURaised() = 0;
+
+ virtual void onConnectionPMTUBlackholeDetected(
+ uint64_t pmtuAtBlackholeTime) = 0;
+
+ virtual void onConnectionPMTUUpperBoundDetected(uint64_t pmtuUpperBound) = 0;
+
static const char* toString(ConnectionCloseReason reason) {
switch (reason) {
case ConnectionCloseReason::NONE:

diff:
@@ -47,6 +47,10 @@ class MockQuicStats : public QuicTransportStatsCallback {
MOCK_METHOD1(onRead, void(size_t));
MOCK_METHOD1(onWrite, void(size_t));
MOCK_METHOD1(onUDPSocketWriteError, void(SocketErrorType));
+ MOCK_METHOD0(onConnectionD6DStarted, void());
+ MOCK_METHOD0(onConnectionPMTURaised, void());
+ MOCK_METHOD1(onConnectionPMTUBlackholeDetected, void(uint64_t));
+ MOCK_METHOD1(onConnectionPMTUUpperBoundDetected, void(uint64_t));
};
class MockQuicStatsFactory : public QuicTransportStatsCallbackFactory {

original_message:
Add quic stats for d6d events

Summary: This increases visibility over d6d lifecycle.

Reviewed By: avasylev

Differential Revision: D24026410

fbshipit-source-id: ee382a5fb60e48d4ee441ad760251673a10c85da


diff:
#include <quic/common/test/TestUtils.h>
#include <quic/congestion_control/ServerCongestionControllerFactory.h>
#include <quic/fizz/client/handshake/FizzClientQuicHandshakeContext.h>
+#include <quic/server/AcceptObserver.h>
#include <quic/server/QuicServer.h>
#include <quic/server/QuicServerTransport.h>
#include <quic/server/QuicSharedUDPSocketFactory.h>
@@ -73,10 +74,95 @@ DEFINE_uint32(
num_server_worker,
1,
"Max number of mvfst server worker threads");
+DEFINE_bool(log_rtt_sample, false, "Log rtt sample events");
+DEFINE_bool(log_loss, false, "Log packet loss events");
+DEFINE_bool(log_app_rate_limited, false, "Log app rate limited events");
+DEFINE_bool(log_pmtu_upperbound, false, "Log pmtu upper bound events");
+DEFINE_bool(log_pmtu_blackhole, false, "Log pmtu blackbole events");
namespace quic {
namespace tperf {
+namespace {
+
+class TPerfInstrumentationObserver : public InstrumentationObserver {
+ public:
+ void observerDetach(QuicSocket* /* socket */) noexcept override {
+ // do nothing
+ }
+
+ void appRateLimited(QuicSocket* /* socket */) override {
+ if (FLAGS_log_app_rate_limited) {
+ LOG(INFO) << "appRateLimited detected";
+ }
+ }
+
+ void packetLossDetected(
+ QuicSocket*, /* socket */
+ const struct ObserverLossEvent& /* lossEvent */) override {
+ if (FLAGS_log_loss) {
+ LOG(INFO) << "packetLoss detected";
+ }
+ }
+
+ void rttSampleGenerated(
+ QuicSocket*, /* socket */
+ const PacketRTT& /* RTT sample */) override {
+ if (FLAGS_log_rtt_sample) {
+ LOG(INFO) << "rttSample generated";
+ }
+ }
+
+ void pmtuBlackholeDetected(
+ QuicSocket*, /* socket */
+ const PMTUBlackholeEvent& /* Blackhole event */) override {
+ if (FLAGS_log_pmtu_blackhole) {
+ LOG(INFO) << "pmtuBlackhole detected";
+ }
+ }
+
+ void pmtuUpperBoundDetected(
+ QuicSocket*, /* socket */
+ const PMTUUpperBoundEvent& event) override {
+ if (FLAGS_log_pmtu_upperbound) {
+ LOG(INFO) << "pmtuUpperBound detected after "
+ << event.cumulativeProbesSent << " d6d probes\n"
+ << "pmtu upperbound is " << event.upperBoundPMTU;
+ }
+ }
+};
+
+/**
+ * A helper accpetor observer that installs instrumentation observers to
+ * transport upon accpet
+ */
+class TPerfAcceptObserver : public AcceptObserver {
+ public:
+ TPerfAcceptObserver()
+ : tperfInstObserver_(std::make_unique<TPerfInstrumentationObserver>()) {}
+
+ void accept(QuicTransportBase* transport) noexcept override {
+ transport->addInstrumentationObserver(tperfInstObserver_.get());
+ }
+
+ void acceptorDestroy(QuicServerWorker* /* worker */) noexcept override {
+ LOG(INFO) << "quic server worker destroyed";
+ }
+
+ void observerAttach(QuicServerWorker* /* worker */) noexcept override {
+ LOG(INFO) << "TPerfAcceptObserver attached";
+ }
+
+ void observerDetach(QuicServerWorker* /* worker */) noexcept override {
+ LOG(INFO) << "TPerfAcceptObserver detached";
+ }
+
+ private:
+ std::unique_ptr<TPerfInstrumentationObserver> tperfInstObserver_;
+};
+
+} // namespace
+
class ServerStreamHandler : public quic::QuicSocket::ConnectionCallback,
public quic::QuicSocket::ReadCallback,
public quic::QuicSocket::WriteCallback {
@@ -292,7 +378,10 @@ class TPerfServer {
uint64_t maxBytesPerStream,
uint32_t maxReceivePacketSize,
bool useInplaceWrite)
- : host_(host), port_(port), server_(QuicServer::createQuicServer()) {
+ : host_(host),
+ port_(port),
+ acceptObserver_(std::make_unique<TPerfAcceptObserver>()),
+ server_(QuicServer::createQuicServer()) {
eventBase_.setName("tperf_server");
server_->setQuicServerTransportFactory(
std::make_unique<TPerfServerTransportFactory>(
@@ -331,6 +420,10 @@ class TPerfServer {
folly::SocketAddress addr1(host_.c_str(), port_);
addr1.setFromHostPort(host_, port_);
server_->start(addr1, FLAGS_num_server_worker);
+ auto workerEvbs = server_->getWorkerEvbs();
+ for (auto evb : workerEvbs) {
+ server_->addAcceptObserver(evb, acceptObserver_.get());
+ }
LOG(INFO) << "tperf server started at: " << addr1.describe();
eventBase_.loopForever();
}
@@ -339,6 +432,7 @@ class TPerfServer {
std::string host_;
uint16_t port_;
folly::EventBase eventBase_;
+ std::unique_ptr<TPerfAcceptObserver> acceptObserver_;
std::shared_ptr<quic::QuicServer> server_;
};

original_message:
Add accept and instrumentation observer to tperf server

Summary: This helps us get more information when running a tperf test

Reviewed By: bschlinker

Differential Revision: D24114523

fbshipit-source-id: 48ae376b6ceb1c08f20027b87b6c9d976c0466ec


message:
Add d6d configs
Summary: ^

diff:
@@ -79,12 +79,52 @@ DEFINE_bool(log_loss, false, "Log packet loss events");
DEFINE_bool(log_app_rate_limited, false, "Log app rate limited events");
DEFINE_bool(log_pmtu_upperbound, false, "Log pmtu upper bound events");
DEFINE_bool(log_pmtu_blackhole, false, "Log pmtu blackbole events");
+DEFINE_bool(d6d_enabled, false, "Enable d6d");
+DEFINE_uint32(
+ d6d_probe_raiser_constant_step_size,
+ 10,
+ "Server only. The constant step size used to increase PMTU, only meaningful to ConstantStep probe size raiser");
+DEFINE_uint32(
+ d6d_probe_raiser_type,
+ 0,
+ "Server only. The type of probe size raiser. 0: ConstantStep, 1: BinarySearch");
+DEFINE_uint32(
+ d6d_blackhole_detection_window_secs,
+ 5,
+ "Server only. PMTU blackhole detection window in secs");
+DEFINE_uint32(
+ d6d_blackhole_detection_threshold,
+ 5,
+ "Server only. PMTU blackhole detection threshold, in # of packets");
+DEFINE_uint32(
+ d6d_base_pmtu,
+ 1252,
+ "Client only. The base PMTU advertised to server");
+DEFINE_uint32(
+ d6d_raise_timeout_secs,
+ 600,
+ "Client only. The raise timeout advertised to server");
+DEFINE_uint32(
+ d6d_probe_timeout_secs,
+ 600,
+ "Client only. The probe timeout advertised to server");
namespace quic {
namespace tperf {
namespace {
+ProbeSizeRaiserType parseRaiserType(uint32_t type) {
+ auto maybeRaiserType = static_cast<ProbeSizeRaiserType>(type);
+ switch (maybeRaiserType) {
+ case ProbeSizeRaiserType::ConstantStep:
+ case ProbeSizeRaiserType::BinarySearch:
+ return maybeRaiserType;
+ default:
+ throw std::runtime_error("Invalid raiser type, must be 0 or 1.");
+ }
+}
+
class TPerfInstrumentationObserver : public InstrumentationObserver {
public:
void observerDetach(QuicSocket* /* socket */) noexcept override {
@@ -407,8 +447,17 @@ class TPerfServer {
settings.maxBatchSize = 16;
}
settings.maxRecvPacketSize = maxReceivePacketSize;
- settings.canIgnorePathMTU = true;
+ settings.canIgnorePathMTU = !FLAGS_d6d_enabled;
settings.copaDeltaParam = FLAGS_latency_factor;
+ settings.d6dConfig.enabled = FLAGS_d6d_enabled;
+ settings.d6dConfig.probeRaiserConstantStepSize =
+ FLAGS_d6d_probe_raiser_constant_step_size;
+ settings.d6dConfig.raiserType =
+ parseRaiserType(FLAGS_d6d_probe_raiser_type);
+ settings.d6dConfig.blackholeDetectionWindow =
+ std::chrono::seconds(FLAGS_d6d_blackhole_detection_window_secs);
+ settings.d6dConfig.blackholeDetectionThreshold =
+ FLAGS_d6d_blackhole_detection_threshold;
server_->setCongestionControllerFactory(
std::make_shared<ServerCongestionControllerFactory>());
server_->setTransportSettings(settings);
@@ -601,7 +650,13 @@ class TPerfClient : public quic::QuicSocket::ConnectionCallback,
settings.maxBatchSize = 16;
}
settings.maxRecvPacketSize = maxReceivePacketSize_;
- settings.canIgnorePathMTU = true;
+ settings.canIgnorePathMTU = !FLAGS_d6d_enabled;
+ settings.d6dConfig.enabled = FLAGS_d6d_enabled;
+ settings.d6dConfig.advertisedBasePMTU = FLAGS_d6d_base_pmtu;
+ settings.d6dConfig.advertisedRaiseTimeout =
+ std::chrono::seconds(FLAGS_d6d_raise_timeout_secs);
+ settings.d6dConfig.advertisedProbeTimeout =
+ std::chrono::seconds(FLAGS_d6d_probe_timeout_secs);
quicClient_->setTransportSettings(settings);
LOG(INFO) << "TPerfClient connecting to " << addr.describe();

original_message:
Add d6d configs

Summary: ^

Reviewed By: mjoras, lnicco

Differential Revision: D24114530

fbshipit-source-id: 1f70732b9bc0656d67cbe0ac1aca1e9affcf2438


diff:
@@ -257,11 +257,11 @@ using TransportPartialReliabilitySetting = bool;
constexpr uint16_t kPartialReliabilityParameterId = 0xFF00; // subject to change
-constexpr uint16_t kD6DBasePMTUParameterId = 0x0377;
+constexpr uint16_t kD6DBasePMTUParameterId = 0xFF77;
-constexpr uint16_t kD6DRaiseTimeoutParameterId = 0xF695;
+constexpr uint16_t kD6DRaiseTimeoutParameterId = 0xFF95;
-constexpr uint16_t kD6DProbeTimeoutParameterId = 0x9012;
+constexpr uint16_t kD6DProbeTimeoutParameterId = 0xFF12;
constexpr uint32_t kDrainFactor = 3;

diff:
@@ -872,6 +872,9 @@ void QuicClientTransport::startCryptoHandshake() {
// Add partial reliability parameter to customTransportParameters_.
setPartialReliabilityTransportParameter();
+ setD6DBasePMTUTransportParameter();
+ setD6DRaiseTimeoutTransportParameter();
+ setD6DProbeTimeoutTransportParameter();
auto paramsExtension = std::make_shared<ClientTransportParametersExtension>(
conn_->originalVersion.value(),

diff:
@@ -244,6 +244,12 @@ class QuicClientTransportIntegrationTest : public TestWithParam<TestingParams> {
std::shared_ptr<QuicServer> createServer(ProcessId processId) {
auto server = QuicServer::createQuicServer();
auto transportSettings = server->getTransportSettings();
+ auto statsFactory = std::make_unique<NiceMock<MockQuicStatsFactory>>();
+ ON_CALL(*statsFactory, make()).WillByDefault(Invoke([&]() {
+ auto newStatsCallback = std::make_unique<NiceMock<MockQuicStats>>();
+ statsCallbacks_.push_back(newStatsCallback.get());
+ return newStatsCallback;
+ }));
transportSettings.zeroRttSourceTokenMatchingPolicy =
ZeroRttSourceTokenMatchingPolicy::LIMIT_IF_NO_EXACT_MATCH;
server->setTransportSettings(transportSettings);
@@ -251,6 +257,7 @@ class QuicClientTransportIntegrationTest : public TestWithParam<TestingParams> {
std::make_unique<EchoServerTransportFactory>());
server->setQuicUDPSocketFactory(
std::make_unique<QuicSharedUDPSocketFactory>());
+ server->setTransportStatsCallbackFactory(std::move(statsFactory));
server->setFizzContext(serverCtx);
server->setSupportedVersion({getVersion(), MVFST1});
folly::SocketAddress addr("::1", 0);
@@ -368,6 +375,7 @@ class QuicClientTransportIntegrationTest : public TestWithParam<TestingParams> {
std::shared_ptr<QuicServer> server_;
bool connected_{false};
std::shared_ptr<MockQuicStats> quicStats_;
+ std::vector<MockQuicStats*> statsCallbacks_;
};
class StreamData {
@@ -1020,6 +1028,29 @@ TEST_P(QuicClientTransportIntegrationTest, PartialReliabilityEnabledTest) {
EXPECT_TRUE(client->isPartiallyReliableTransport());
}
+TEST_P(QuicClientTransportIntegrationTest, D6DEnabledTest) {
+ expectTransportCallbacks();
+ TransportSettings settings;
+ settings.d6dConfig.enabled = true;
+ client->setTransportSettings(settings);
+
+ TransportSettings serverSettings;
+ serverSettings.d6dConfig.enabled = true;
+ serverSettings.statelessResetTokenSecret = getRandSecret();
+ server_->setTransportSettings(serverSettings);
+
+ // we only use 1 worker in test
+ EXPECT_EQ(1, statsCallbacks_.size());
+ EXPECT_CALL(*statsCallbacks_[0], onConnectionD6DStarted()).Times(1);
+
+ client->start(&clientConnCallback);
+ EXPECT_CALL(clientConnCallback, onTransportReady()).WillOnce(Invoke([&] {
+ CHECK(client->getConn().oneRttWriteCipher);
+ eventbase_.terminateLoopSoon();
+ }));
+ eventbase_.loopForever();
+}
+
TEST_P(
QuicClientTransportIntegrationTest,
PartialReliabilityEnableDisableRunTimeTest) {

original_message:
Correctify d6d transport parameter id and xmit them during handshake

Summary: We need to increase those parameter Ids above 0xFF00, and xmit them.

Reviewed By: mjoras

Differential Revision: D24115117

fbshipit-source-id: cf8b4970ba8640a83bde22c1f9fdfa1d3e53e3f9


diff:
@@ -128,7 +128,7 @@ void onD6DLastProbeAcked(QuicConnectionStateBase& conn) {
conn.udpSendPacketLen = lastProbeSize;
if (maybeNextProbeSize.hasValue() &&
*maybeNextProbeSize > conn.udpSendPacketLen &&
- *maybeNextProbeSize < d6d.maxPMTU) {
+ *maybeNextProbeSize <= d6d.maxPMTU) {
d6d.currentProbeSize = *maybeNextProbeSize;
scheduleProbeAfterDelay(conn, kDefaultD6DProbeDelayWhenAcked);
} else {

diff:
@@ -491,5 +491,43 @@ TEST_F(QuicD6DStateFunctionsTest, BlackholeInSearchComplete) {
EXPECT_GE(d6d.meta.timeLastNonSearchState, now);
}
+TEST_F(QuicD6DStateFunctionsTest, ReachMaxPMTU) {
+ QuicConnectionStateBase conn(QuicNodeType::Server);
+ auto& d6d = conn.d6d;
+ auto now = Clock::now();
+ auto mockInstrumentationObserver =
+ std::make_unique<StrictMock<MockInstrumentationObserver>>();
+ conn.instrumentationObservers_.push_back(mockInstrumentationObserver.get());
+ d6d.state = D6DMachineState::SEARCHING;
+ d6d.maxPMTU = 1452;
+ d6d.outstandingProbes = 1;
+ conn.udpSendPacketLen = 1400;
+ d6d.currentProbeSize = 1442;
+ d6d.meta.lastNonSearchState = D6DMachineState::BASE;
+ d6d.meta.timeLastNonSearchState = now;
+ d6d.meta.totalTxedProbes = 10;
+ auto pkt = OutstandingPacket(
+ makeTestShortPacket(),
+ Clock::now(),
+ d6d.currentProbeSize,
+ false,
+ true,
+ d6d.currentProbeSize,
+ d6d.currentProbeSize);
+ d6d.lastProbe = D6DProbePacket(
+ pkt.packet.header.getPacketSequenceNum(), pkt.metadata.encodedSize);
+ d6d.raiser = std::make_unique<MockProbeSizeRaiser>();
+ auto mockRaiser = dynamic_cast<MockProbeSizeRaiser*>(d6d.raiser.get());
+ EXPECT_CALL(*mockRaiser, raiseProbeSize(d6d.currentProbeSize))
+ .Times(1)
+ .WillOnce(Return(1452));
+ onD6DLastProbeAcked(conn);
+ EXPECT_EQ(d6d.state, D6DMachineState::SEARCHING);
+ EXPECT_EQ(d6d.currentProbeSize, 1452);
+ EXPECT_EQ(conn.udpSendPacketLen, 1442);
+ EXPECT_EQ(d6d.meta.lastNonSearchState, D6DMachineState::BASE);
+ EXPECT_EQ(d6d.meta.timeLastNonSearchState, now);
+}
+
} // namespace test
} // namespace quic

original_message:
Allow maybeNextProbeSize to be equal to maxPMTU

Summary: ^

Reviewed By: mjoras

Differential Revision: D24115139

fbshipit-source-id: 59d416829b64c0d57364ae74a6c321646a7b1da8


diff:
@@ -25,8 +25,7 @@ static TimePoint reportUpperBound(QuicConnectionStateBase& conn) {
const auto lastProbeSize = d6d.lastProbe->packetSize;
const auto now = Clock::now();
- QUIC_STATS(
- conn.statsCallback, onConnectionPMTUUpperBoundDetected, lastProbeSize);
+ QUIC_STATS(conn.statsCallback, onConnectionPMTUUpperBoundDetected);
if (conn.instrumentationObservers_.size() > 0) {
InstrumentationObserver::PMTUUpperBoundEvent upperBoundEvent(
now,
@@ -54,10 +53,7 @@ static TimePoint reportUpperBound(QuicConnectionStateBase& conn) {
static TimePoint reportBlackhole(
QuicConnectionStateBase& conn,
const OutstandingPacket& packet) {
- QUIC_STATS(
- conn.statsCallback,
- onConnectionPMTUBlackholeDetected,
- packet.metadata.encodedSize);
+ QUIC_STATS(conn.statsCallback, onConnectionPMTUBlackholeDetected);
auto& d6d = conn.d6d;
const auto now = Clock::now();
if (conn.instrumentationObservers_.size() > 0) {

diff:
@@ -157,16 +157,12 @@ class LogQuicStats : public quic::QuicTransportStatsCallback {
VLOG(2) << prefix_ << "onConnectionPMTURaised";
}
- void onConnectionPMTUBlackholeDetected(
- uint64_t pmtuAtBlackholeTime) override {
- VLOG(2) << prefix_
- << "onConnectionPMTUBlackholeDetected pmtuAtBlackholeTime="
- << pmtuAtBlackholeTime;
+ void onConnectionPMTUBlackholeDetected() override {
+ VLOG(2) << prefix_ << "onConnectionPMTUBlackholeDetected";
}
- void onConnectionPMTUUpperBoundDetected(uint64_t pmtuUpperBound) override {
- VLOG(2) << prefix_ << "onConnectionPMTUUpperBoundDetected pmtuUpperBound="
- << pmtuUpperBound;
+ void onConnectionPMTUUpperBoundDetected() override {
+ VLOG(2) << prefix_ << "onConnectionPMTUUpperBoundDetected";
}
private:

diff:
@@ -140,10 +140,9 @@ class QuicTransportStatsCallback {
virtual void onConnectionPMTURaised() = 0;
- virtual void onConnectionPMTUBlackholeDetected(
- uint64_t pmtuAtBlackholeTime) = 0;
+ virtual void onConnectionPMTUBlackholeDetected() = 0;
- virtual void onConnectionPMTUUpperBoundDetected(uint64_t pmtuUpperBound) = 0;
+ virtual void onConnectionPMTUUpperBoundDetected() = 0;
static const char* toString(ConnectionCloseReason reason) {
switch (reason) {

diff:
@@ -49,8 +49,8 @@ class MockQuicStats : public QuicTransportStatsCallback {
MOCK_METHOD1(onUDPSocketWriteError, void(SocketErrorType));
MOCK_METHOD0(onConnectionD6DStarted, void());
MOCK_METHOD0(onConnectionPMTURaised, void());
- MOCK_METHOD1(onConnectionPMTUBlackholeDetected, void(uint64_t));
- MOCK_METHOD1(onConnectionPMTUUpperBoundDetected, void(uint64_t));
+ MOCK_METHOD0(onConnectionPMTUBlackholeDetected, void());
+ MOCK_METHOD0(onConnectionPMTUUpperBoundDetected, void());
};
class MockQuicStatsFactory : public QuicTransportStatsCallbackFactory {

original_message:
Report pmtu blackholes and upperbounds by occurences

Summary: We should report blackholes and upperbounds by occurrence.

Reviewed By: avasylev

Differential Revision: D24400499

fbshipit-source-id: 2085317943ce378599e734567ff19ae9e7500ed9