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.
MODIFY
quic/api/QuicTransportBase.cpp
quic/api/QuicTransportBase.cpp
@@ -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;

MODIFY
quic/api/QuicTransportFunctions.cpp
quic/api/QuicTransportFunctions.cpp
@@ -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;
}

MODIFY
quic/api/test/QuicTransportFunctionsTest.cpp
quic/api/test/QuicTransportFunctionsTest.cpp
@@ -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();

MODIFY
quic/d6d/QuicD6DStateFunctions.cpp
quic/d6d/QuicD6DStateFunctions.cpp
@@ -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

MODIFY
quic/d6d/test/QuicD6DStateFunctionsTest.cpp
quic/d6d/test/QuicD6DStateFunctionsTest.cpp
@@ -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);
}
};

MODIFY
quic/server/QuicServerTransport.cpp
quic/server/QuicServerTransport.cpp
@@ -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());
}

MODIFY
quic/state/StateData.h
quic/state/StateData.h
@@ -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;
};

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