diff --git a/quic/api/QuicTransportBase.cpp b/quic/api/QuicTransportBase.cpp index de12d939d..2d81873fc 100644 --- a/quic/api/QuicTransportBase.cpp +++ b/quic/api/QuicTransportBase.cpp @@ -1924,6 +1924,10 @@ void QuicTransportBase::onNetworkData( // Received data could contain valid path response, in which case // path validation timeout should be canceled schedulePathValidationTimeout(); + + // If ECN is enabled, make sure that the packet marking is happening as + // expected + validateECNState(); } else { // In the closed state, we would want to write a close if possible // however the write looper will not be set. @@ -3334,11 +3338,14 @@ void QuicTransportBase::updateSocketTosSettings() { if (conn_->transportSettings.enableEcnOnEgress) { if (conn_->transportSettings.useL4sEcn) { conn_->socketTos.fields.ecn = kEcnECT1; + conn_->ecnState = ECNState::AttemptingL4S; } else { conn_->socketTos.fields.ecn = kEcnECT0; + conn_->ecnState = ECNState::AttemptingECN; } } else { conn_->socketTos.fields.ecn = 0; + conn_->ecnState = ECNState::NotAttempted; } if (socket_ && socket_->isBound() && @@ -3947,6 +3954,78 @@ void QuicTransportBase::updatePacketProcessorsPrewriteRequests() { conn_->socketCmsgsState.targetWriteCount = conn_->writeCount; } +void QuicTransportBase::validateECNState() { + if (conn_->ecnState == ECNState::NotAttempted || + conn_->ecnState == ECNState::FailedValidation) { + // Verification not needed + return; + } + const auto& minExpectedMarkedPacketsCount = + conn_->ackStates.appDataAckState.minimumExpectedEcnMarksEchoed; + if (minExpectedMarkedPacketsCount < 10) { + // We wait for 10 ack-eliciting app data packets to be marked before trying + // to validate ECN. + return; + } + const auto& maxExpectedMarkedPacketsCount = conn_->lossState.totalPacketsSent; + + auto markedPacketCount = conn_->ackStates.appDataAckState.ecnCECountEchoed; + + if (conn_->ecnState == ECNState::AttemptingECN || + conn_->ecnState == ECNState::ValidatedECN) { + // Check the number of marks seen (ECT0 + CE). ECT1 should be zero. + markedPacketCount += conn_->ackStates.appDataAckState.ecnECT0CountEchoed; + + if (markedPacketCount >= minExpectedMarkedPacketsCount && + markedPacketCount <= maxExpectedMarkedPacketsCount && + conn_->ackStates.appDataAckState.ecnECT1CountEchoed == 0) { + if (conn_->ecnState != ECNState::ValidatedECN) { + conn_->ecnState = ECNState::ValidatedECN; + VLOG(4) << fmt::format( + "ECN validation successful. Marked {} of {} expected", + markedPacketCount, + minExpectedMarkedPacketsCount); + } + } else { + conn_->ecnState = ECNState::FailedValidation; + VLOG(4) << fmt::format( + "ECN validation failed. Marked {} of {} expected", + markedPacketCount, + minExpectedMarkedPacketsCount); + } + } else if ( + conn_->ecnState == ECNState::AttemptingL4S || + conn_->ecnState == ECNState::ValidatedL4S) { + // Check the number of marks seen (ECT1 + CE). ECT0 should be zero. + markedPacketCount += conn_->ackStates.appDataAckState.ecnECT1CountEchoed; + + if (markedPacketCount >= minExpectedMarkedPacketsCount && + markedPacketCount <= maxExpectedMarkedPacketsCount && + conn_->ackStates.appDataAckState.ecnECT0CountEchoed == 0) { + if (conn_->ecnState != ECNState::ValidatedL4S) { + conn_->ecnState = ECNState::ValidatedL4S; + VLOG(4) << fmt::format( + "L4S validation successful. Marked {} of {} expected", + markedPacketCount, + minExpectedMarkedPacketsCount); + } + } else { + conn_->ecnState = ECNState::FailedValidation; + VLOG(4) << fmt::format( + "L4S validation failed. Marked {} of {} expected", + markedPacketCount, + minExpectedMarkedPacketsCount); + } + } + + if (conn_->ecnState == ECNState::FailedValidation) { + conn_->socketTos.fields.ecn = 0; + CHECK(socket_ && socket_->isBound()); + socket_->setTosOrTrafficClass(conn_->socketTos.value); + VLOG(4) << "ECN validation failed. Disabling ECN"; + } +} + folly::Optional QuicTransportBase::getAdditionalCmsgsForAsyncUDPSocket() { if (conn_->socketCmsgsState.additionalCmsgs) { diff --git a/quic/api/QuicTransportBase.h b/quic/api/QuicTransportBase.h index cda75bee4..4fe47c793 100644 --- a/quic/api/QuicTransportBase.h +++ b/quic/api/QuicTransportBase.h @@ -1003,6 +1003,16 @@ class QuicTransportBase : public QuicSocket, QuicStreamPrioritiesObserver { bool isTimeoutScheduled(QuicTimerCallback* callback) const; + /** + * Helper function to validate that the number of ECN packet marks match the + * expected value, depending on the ECN state of the connection. + * + * If ECN is enabled, this function validates it's working correctly. If ECN + * is not enabled or has already failed validation, this function does + * nothing. + */ + void validateECNState(); + private: /** * Helper functions to handle new streams. diff --git a/quic/api/test/QuicTransportTest.cpp b/quic/api/test/QuicTransportTest.cpp index 533dd6d8c..a8fc1e97a 100644 --- a/quic/api/test/QuicTransportTest.cpp +++ b/quic/api/test/QuicTransportTest.cpp @@ -5165,6 +5165,7 @@ TEST_F(QuicTransportTest, UpdateSocketTosSettingsNoECN) { auto& conn = transport_->getConnectionState(); EXPECT_EQ(conn.socketTos.fields.ecn, 0); + EXPECT_EQ(conn.ecnState, ECNState::NotAttempted); } TEST_F(QuicTransportTest, UpdateSocketTosSettingsClassicECN) { @@ -5182,6 +5183,7 @@ TEST_F(QuicTransportTest, UpdateSocketTosSettingsClassicECN) { auto& conn = transport_->getConnectionState(); EXPECT_EQ(conn.socketTos.fields.ecn, kEcnECT0); + EXPECT_EQ(conn.ecnState, ECNState::AttemptingECN); } TEST_F(QuicTransportTest, UpdateSocketTosSettingsL4SECN) { @@ -5199,6 +5201,7 @@ TEST_F(QuicTransportTest, UpdateSocketTosSettingsL4SECN) { auto& conn = transport_->getConnectionState(); EXPECT_EQ(conn.socketTos.fields.ecn, kEcnECT1); + EXPECT_EQ(conn.ecnState, ECNState::AttemptingL4S); // Setting the same transport settings should not trigger any calls to the // socket @@ -5209,10 +5212,12 @@ TEST_F(QuicTransportTest, UpdateSocketTosSettingsL4SECN) { EXPECT_CALL(*socket_, setTosOrTrafficClass(0)).Times(1); transportSettings.enableEcnOnEgress = false; transport_->setTransportSettings(transportSettings); + EXPECT_EQ(conn.ecnState, ECNState::NotAttempted); } TEST_F(QuicTransportTest, UpdateSocketTosSettingsDoNotSetTosOnUnboundSocket) { - // Pretend the socket is bound so the transport can attempt to set ToS. + // Pretend the socket is not bound so the transport does not attempt to set + // ToS. EXPECT_CALL(*socket_, isBound).WillRepeatedly(Return(false)); EXPECT_CALL(*socket_, setRecvTos).Times(0); @@ -5229,5 +5234,373 @@ TEST_F(QuicTransportTest, UpdateSocketTosSettingsDoNotSetTosOnUnboundSocket) { EXPECT_EQ(conn.socketTos.fields.ecn, kEcnECT0); } +TEST_F(QuicTransportTest, ValidateECNValidationNotNeeded) { + // All the tests here should not change anything about the socket state. + EXPECT_CALL(*socket_, setRecvTos).Times(0); + EXPECT_CALL(*socket_, setTosOrTrafficClass(0)).Times(0); + + auto& conn = transport_->getConnectionState(); + + // This is the default. + ASSERT_EQ(conn.ecnState, ECNState::NotAttempted); + transport_->validateECN(); + EXPECT_EQ(conn.ecnState, ECNState::NotAttempted); + + // If validation already failed, nothing should happen either. + conn.ecnState = ECNState::FailedValidation; + transport_->validateECN(); + EXPECT_EQ(conn.ecnState, ECNState::FailedValidation); +} + +TEST_F(QuicTransportTest, ValidateECNSuccess) { + auto& conn = transport_->getConnectionState(); + + { + conn.ecnState = ECNState::AttemptingECN; + // Not enough packets to validate ECN + conn.lossState.totalPacketsSent = 20; + conn.ackStates.appDataAckState.minimumExpectedEcnMarksEchoed = 9; + conn.ackStates.appDataAckState.ecnECT0CountEchoed = 9; + conn.ackStates.appDataAckState.ecnECT1CountEchoed = 0; + conn.ackStates.appDataAckState.ecnCECountEchoed = 0; + transport_->validateECN(); + EXPECT_EQ(conn.ecnState, ECNState::AttemptingECN); + } + // ==== From attempting to validated =========== + { + conn.ecnState = ECNState::AttemptingECN; + // Marked exactly equal expected. + // Expected 10. 9 ECT0 + 1 CE + conn.lossState.totalPacketsSent = 20; + conn.ackStates.appDataAckState.minimumExpectedEcnMarksEchoed = 10; + conn.ackStates.appDataAckState.ecnECT0CountEchoed = 9; + conn.ackStates.appDataAckState.ecnECT1CountEchoed = 0; + conn.ackStates.appDataAckState.ecnCECountEchoed = 1; + transport_->validateECN(); + EXPECT_EQ(conn.ecnState, ECNState::ValidatedECN); + } + + { + conn.ecnState = ECNState::AttemptingECN; + // Marked more than expected and less than total sent + // Expected 10. 15 ECT0 + 4 CE + conn.lossState.totalPacketsSent = 20; + conn.ackStates.appDataAckState.minimumExpectedEcnMarksEchoed = 10; + conn.ackStates.appDataAckState.ecnECT0CountEchoed = 15; + conn.ackStates.appDataAckState.ecnECT1CountEchoed = 0; + conn.ackStates.appDataAckState.ecnCECountEchoed = 4; + transport_->validateECN(); + EXPECT_EQ(conn.ecnState, ECNState::ValidatedECN); + } + + // ==== Already validated =========== + + { + conn.ecnState = ECNState::ValidatedECN; + // Marked exactly equal expected. + // Expected 10. 9 ECT0 + 1 CE + conn.lossState.totalPacketsSent = 20; + conn.ackStates.appDataAckState.minimumExpectedEcnMarksEchoed = 10; + conn.ackStates.appDataAckState.ecnECT0CountEchoed = 9; + conn.ackStates.appDataAckState.ecnECT1CountEchoed = 0; + conn.ackStates.appDataAckState.ecnCECountEchoed = 1; + transport_->validateECN(); + EXPECT_EQ(conn.ecnState, ECNState::ValidatedECN); + } + + { + conn.ecnState = ECNState::ValidatedECN; + // Marked more than expected and less than total sent + // Expected 10. 15 ECT0 + 4 CE + conn.lossState.totalPacketsSent = 20; + conn.ackStates.appDataAckState.minimumExpectedEcnMarksEchoed = 10; + conn.ackStates.appDataAckState.ecnECT0CountEchoed = 15; + conn.ackStates.appDataAckState.ecnECT1CountEchoed = 0; + conn.ackStates.appDataAckState.ecnCECountEchoed = 4; + transport_->validateECN(); + EXPECT_EQ(conn.ecnState, ECNState::ValidatedECN); + } +} + +TEST_F(QuicTransportTest, ValidateECNFailure) { + auto& conn = transport_->getConnectionState(); + + // Pretend the socket is bound so the transport can attempt to set ToS. + EXPECT_CALL(*socket_, isBound).WillRepeatedly(Return(true)); + + // ==== From attempting to failed =========== + { + conn.ecnState = ECNState::AttemptingECN; + conn.socketTos.fields.ecn = kEcnECT0; + // Marked less than expected. + // Expected 10. 9 ECT0 + 0 CE + conn.lossState.totalPacketsSent = 20; + conn.ackStates.appDataAckState.minimumExpectedEcnMarksEchoed = 10; + conn.ackStates.appDataAckState.ecnECT0CountEchoed = 9; + conn.ackStates.appDataAckState.ecnECT1CountEchoed = 0; + conn.ackStates.appDataAckState.ecnCECountEchoed = 0; + EXPECT_CALL(*socket_, setTosOrTrafficClass(0)).Times(1); + transport_->validateECN(); + EXPECT_EQ(conn.ecnState, ECNState::FailedValidation); + EXPECT_EQ(conn.socketTos.fields.ecn, 0); + } + + { + conn.ecnState = ECNState::AttemptingECN; + conn.socketTos.fields.ecn = kEcnECT0; + // Marked more than total. + // Expected 10. 19 ECT0 + 2 CE + conn.lossState.totalPacketsSent = 20; + conn.ackStates.appDataAckState.minimumExpectedEcnMarksEchoed = 10; + conn.ackStates.appDataAckState.ecnECT0CountEchoed = 19; + conn.ackStates.appDataAckState.ecnECT1CountEchoed = 0; + conn.ackStates.appDataAckState.ecnCECountEchoed = 2; + EXPECT_CALL(*socket_, setTosOrTrafficClass(0)).Times(1); + transport_->validateECN(); + EXPECT_EQ(conn.ecnState, ECNState::FailedValidation); + EXPECT_EQ(conn.socketTos.fields.ecn, 0); + } + + { + conn.ecnState = ECNState::AttemptingECN; + conn.socketTos.fields.ecn = kEcnECT0; + // Wrong ECT marking received + // Expected 10. 1 ECT1 + conn.lossState.totalPacketsSent = 20; + conn.ackStates.appDataAckState.minimumExpectedEcnMarksEchoed = 10; + conn.ackStates.appDataAckState.ecnECT0CountEchoed = 0; + conn.ackStates.appDataAckState.ecnECT1CountEchoed = 1; + conn.ackStates.appDataAckState.ecnCECountEchoed = 0; + EXPECT_CALL(*socket_, setTosOrTrafficClass(0)).Times(1); + transport_->validateECN(); + EXPECT_EQ(conn.ecnState, ECNState::FailedValidation); + EXPECT_EQ(conn.socketTos.fields.ecn, 0); + } + + // ==== From valdiated to failed =========== + { + conn.ecnState = ECNState::ValidatedECN; + conn.socketTos.fields.ecn = kEcnECT0; + // Marked less than expected. + // Expected 10. 9 ECT0 + 0 CE + conn.lossState.totalPacketsSent = 20; + conn.ackStates.appDataAckState.minimumExpectedEcnMarksEchoed = 10; + conn.ackStates.appDataAckState.ecnECT0CountEchoed = 9; + conn.ackStates.appDataAckState.ecnECT1CountEchoed = 0; + conn.ackStates.appDataAckState.ecnCECountEchoed = 0; + EXPECT_CALL(*socket_, setTosOrTrafficClass(0)).Times(1); + transport_->validateECN(); + EXPECT_EQ(conn.ecnState, ECNState::FailedValidation); + EXPECT_EQ(conn.socketTos.fields.ecn, 0); + } + + { + conn.ecnState = ECNState::ValidatedECN; + conn.socketTos.fields.ecn = kEcnECT0; + // Marked more than total. + // Expected 10. 19 ECT0 + 2 CE + conn.lossState.totalPacketsSent = 20; + conn.ackStates.appDataAckState.minimumExpectedEcnMarksEchoed = 10; + conn.ackStates.appDataAckState.ecnECT0CountEchoed = 19; + conn.ackStates.appDataAckState.ecnECT1CountEchoed = 0; + conn.ackStates.appDataAckState.ecnCECountEchoed = 2; + EXPECT_CALL(*socket_, setTosOrTrafficClass(0)).Times(1); + transport_->validateECN(); + EXPECT_EQ(conn.ecnState, ECNState::FailedValidation); + EXPECT_EQ(conn.socketTos.fields.ecn, 0); + } + + { + conn.ecnState = ECNState::ValidatedECN; + conn.socketTos.fields.ecn = kEcnECT0; + // Wrong ECT marking received + // Expected 10. 1 ECT1 + conn.lossState.totalPacketsSent = 20; + conn.ackStates.appDataAckState.minimumExpectedEcnMarksEchoed = 10; + conn.ackStates.appDataAckState.ecnECT0CountEchoed = 0; + conn.ackStates.appDataAckState.ecnECT1CountEchoed = 1; + conn.ackStates.appDataAckState.ecnCECountEchoed = 0; + EXPECT_CALL(*socket_, setTosOrTrafficClass(0)).Times(1); + transport_->validateECN(); + EXPECT_EQ(conn.ecnState, ECNState::FailedValidation); + EXPECT_EQ(conn.socketTos.fields.ecn, 0); + } +} + +TEST_F(QuicTransportTest, ValidateL4SSuccess) { + auto& conn = transport_->getConnectionState(); + + { + conn.ecnState = ECNState::AttemptingL4S; + // Not enough packets to validate L4S + conn.lossState.totalPacketsSent = 20; + conn.ackStates.appDataAckState.minimumExpectedEcnMarksEchoed = 9; + conn.ackStates.appDataAckState.ecnECT0CountEchoed = 9; + conn.ackStates.appDataAckState.ecnECT1CountEchoed = 0; + conn.ackStates.appDataAckState.ecnCECountEchoed = 0; + transport_->validateECN(); + EXPECT_EQ(conn.ecnState, ECNState::AttemptingL4S); + } + // ==== From attempting to validated =========== + { + conn.ecnState = ECNState::AttemptingL4S; + // Marked exactly equal expected. + // Expected 10. 9 ECT1 + 1 CE + conn.lossState.totalPacketsSent = 20; + conn.ackStates.appDataAckState.minimumExpectedEcnMarksEchoed = 10; + conn.ackStates.appDataAckState.ecnECT0CountEchoed = 0; + conn.ackStates.appDataAckState.ecnECT1CountEchoed = 9; + conn.ackStates.appDataAckState.ecnCECountEchoed = 1; + transport_->validateECN(); + EXPECT_EQ(conn.ecnState, ECNState::ValidatedL4S); + } + + { + conn.ecnState = ECNState::AttemptingL4S; + // Marked more than expected and less than total sent + // Expected 10. 15 ECT1 + 4 CE + conn.lossState.totalPacketsSent = 20; + conn.ackStates.appDataAckState.minimumExpectedEcnMarksEchoed = 10; + conn.ackStates.appDataAckState.ecnECT0CountEchoed = 0; + conn.ackStates.appDataAckState.ecnECT1CountEchoed = 15; + conn.ackStates.appDataAckState.ecnCECountEchoed = 4; + transport_->validateECN(); + EXPECT_EQ(conn.ecnState, ECNState::ValidatedL4S); + } + + // ==== Already validated =========== + + { + conn.ecnState = ECNState::ValidatedL4S; + // Marked exactly equal expected. + // Expected 10. 9 ECT1 + 1 CE + conn.lossState.totalPacketsSent = 20; + conn.ackStates.appDataAckState.minimumExpectedEcnMarksEchoed = 10; + conn.ackStates.appDataAckState.ecnECT0CountEchoed = 0; + conn.ackStates.appDataAckState.ecnECT1CountEchoed = 9; + conn.ackStates.appDataAckState.ecnCECountEchoed = 1; + transport_->validateECN(); + EXPECT_EQ(conn.ecnState, ECNState::ValidatedL4S); + } + + { + conn.ecnState = ECNState::ValidatedL4S; + // Marked more than expected and less than total sent + // Expected 10. 15 ECT1 + 4 CE + conn.lossState.totalPacketsSent = 20; + conn.ackStates.appDataAckState.minimumExpectedEcnMarksEchoed = 10; + conn.ackStates.appDataAckState.ecnECT0CountEchoed = 0; + conn.ackStates.appDataAckState.ecnECT1CountEchoed = 15; + conn.ackStates.appDataAckState.ecnCECountEchoed = 4; + transport_->validateECN(); + EXPECT_EQ(conn.ecnState, ECNState::ValidatedL4S); + } +} + +TEST_F(QuicTransportTest, ValidateL4SFailure) { + auto& conn = transport_->getConnectionState(); + + // Pretend the socket is bound so the transport can attempt to set ToS. + EXPECT_CALL(*socket_, isBound).WillRepeatedly(Return(true)); + + // ==== From attempting to failed =========== + { + conn.ecnState = ECNState::AttemptingL4S; + conn.socketTos.fields.ecn = kEcnECT1; + // Marked less than expected. + // Expected 10. 9 ECT1 + 0 CE + conn.lossState.totalPacketsSent = 20; + conn.ackStates.appDataAckState.minimumExpectedEcnMarksEchoed = 10; + conn.ackStates.appDataAckState.ecnECT0CountEchoed = 0; + conn.ackStates.appDataAckState.ecnECT1CountEchoed = 9; + conn.ackStates.appDataAckState.ecnCECountEchoed = 0; + EXPECT_CALL(*socket_, setTosOrTrafficClass(0)).Times(1); + transport_->validateECN(); + EXPECT_EQ(conn.ecnState, ECNState::FailedValidation); + EXPECT_EQ(conn.socketTos.fields.ecn, 0); + } + + { + conn.ecnState = ECNState::AttemptingL4S; + conn.socketTos.fields.ecn = kEcnECT1; + // Marked more than total. + // Expected 10. 19 ECT1 + 2 CE + conn.lossState.totalPacketsSent = 20; + conn.ackStates.appDataAckState.minimumExpectedEcnMarksEchoed = 10; + conn.ackStates.appDataAckState.ecnECT0CountEchoed = 0; + conn.ackStates.appDataAckState.ecnECT1CountEchoed = 19; + conn.ackStates.appDataAckState.ecnCECountEchoed = 2; + EXPECT_CALL(*socket_, setTosOrTrafficClass(0)).Times(1); + transport_->validateECN(); + EXPECT_EQ(conn.ecnState, ECNState::FailedValidation); + EXPECT_EQ(conn.socketTos.fields.ecn, 0); + } + + { + conn.ecnState = ECNState::AttemptingL4S; + conn.socketTos.fields.ecn = kEcnECT1; + // Wrong ECT marking received + // Expected 10. 1 ECT0 + conn.lossState.totalPacketsSent = 20; + conn.ackStates.appDataAckState.minimumExpectedEcnMarksEchoed = 10; + conn.ackStates.appDataAckState.ecnECT0CountEchoed = 1; + conn.ackStates.appDataAckState.ecnECT1CountEchoed = 0; + conn.ackStates.appDataAckState.ecnCECountEchoed = 0; + EXPECT_CALL(*socket_, setTosOrTrafficClass(0)).Times(1); + transport_->validateECN(); + EXPECT_EQ(conn.ecnState, ECNState::FailedValidation); + EXPECT_EQ(conn.socketTos.fields.ecn, 0); + } + + // ==== From valdiated to failed =========== + { + conn.ecnState = ECNState::ValidatedL4S; + conn.socketTos.fields.ecn = kEcnECT1; + // Marked less than expected. + // Expected 10. 9 ECT1 + 0 CE + conn.lossState.totalPacketsSent = 20; + conn.ackStates.appDataAckState.minimumExpectedEcnMarksEchoed = 10; + conn.ackStates.appDataAckState.ecnECT0CountEchoed = 9; + conn.ackStates.appDataAckState.ecnECT1CountEchoed = 0; + conn.ackStates.appDataAckState.ecnCECountEchoed = 0; + EXPECT_CALL(*socket_, setTosOrTrafficClass(0)).Times(1); + transport_->validateECN(); + EXPECT_EQ(conn.ecnState, ECNState::FailedValidation); + EXPECT_EQ(conn.socketTos.fields.ecn, 0); + } + + { + conn.ecnState = ECNState::ValidatedL4S; + conn.socketTos.fields.ecn = kEcnECT1; + // Marked more than total. + // Expected 10. 19 ECT1 + 2 CE + conn.lossState.totalPacketsSent = 20; + conn.ackStates.appDataAckState.minimumExpectedEcnMarksEchoed = 10; + conn.ackStates.appDataAckState.ecnECT0CountEchoed = 0; + conn.ackStates.appDataAckState.ecnECT1CountEchoed = 19; + conn.ackStates.appDataAckState.ecnCECountEchoed = 2; + EXPECT_CALL(*socket_, setTosOrTrafficClass(0)).Times(1); + transport_->validateECN(); + EXPECT_EQ(conn.ecnState, ECNState::FailedValidation); + EXPECT_EQ(conn.socketTos.fields.ecn, 0); + } + + { + conn.ecnState = ECNState::ValidatedL4S; + conn.socketTos.fields.ecn = kEcnECT0; + // Wrong ECT marking received + // Expected 10. 1 ECT0 + conn.lossState.totalPacketsSent = 20; + conn.ackStates.appDataAckState.minimumExpectedEcnMarksEchoed = 10; + conn.ackStates.appDataAckState.ecnECT0CountEchoed = 1; + conn.ackStates.appDataAckState.ecnECT1CountEchoed = 0; + conn.ackStates.appDataAckState.ecnCECountEchoed = 0; + EXPECT_CALL(*socket_, setTosOrTrafficClass(0)).Times(1); + transport_->validateECN(); + EXPECT_EQ(conn.ecnState, ECNState::FailedValidation); + EXPECT_EQ(conn.socketTos.fields.ecn, 0); + } +} + } // namespace test } // namespace quic diff --git a/quic/api/test/TestQuicTransport.h b/quic/api/test/TestQuicTransport.h index bb8f09813..20d482887 100644 --- a/quic/api/test/TestQuicTransport.h +++ b/quic/api/test/TestQuicTransport.h @@ -186,6 +186,10 @@ class TestQuicTransport return folly::none; } + void validateECN() { + QuicTransportBase::validateECNState(); + } + std::unique_ptr aead; std::unique_ptr headerCipher; bool closed{false}; diff --git a/quic/state/AckHandlers.cpp b/quic/state/AckHandlers.cpp index 2dec30ba0..015c2aeb1 100644 --- a/quic/state/AckHandlers.cpp +++ b/quic/state/AckHandlers.cpp @@ -122,6 +122,8 @@ AckEvent processAckFrame( const TimePoint& ackReceiveTime) { const auto nowTime = Clock::now(); + updateEcnCountEchoed(conn, pnSpace, frame); + // TODO: send error if we get an ack for a packet we've not sent t18721184 auto ack = AckEvent::Builder() .setAckTime(ackReceiveTime) @@ -395,6 +397,9 @@ AckEvent processAckFrame( // run the ACKed packet visitor ackedPacketVisitor(*outstandingPacket); + // Update ecn counts + incrementEcnCountForAckedPacket(conn, pnSpace); + const auto processAllFrames = packetWithHandlerContextItr->processAllFrames; AckEvent::AckPacket::DetailsPerStream detailsPerStream; for (auto& packetFrame : outstandingPacket->packet.frames) { @@ -751,4 +756,30 @@ void commonAckVisitorForAckFrame( } } } + +void incrementEcnCountForAckedPacket( + QuicConnectionStateBase& conn, + PacketNumberSpace pnSpace) { + if (conn.ecnState == ECNState::NotAttempted || + conn.ecnState == ECNState::FailedValidation) { + // Nothing to update. + return; + } + auto& ackState = getAckState(conn, pnSpace); + ackState.minimumExpectedEcnMarksEchoed++; +} + +void updateEcnCountEchoed( + QuicConnectionStateBase& conn, + PacketNumberSpace pnSpace, + const ReadAckFrame& readAckFrame) { + // Track the reflected ECN markings in the current pn space + auto& ackState = getAckState(conn, pnSpace); + ackState.ecnECT0CountEchoed = + std::max(ackState.ecnECT0CountEchoed, readAckFrame.ecnECT0Count); + ackState.ecnECT1CountEchoed = + std::max(ackState.ecnECT1CountEchoed, readAckFrame.ecnECT1Count); + ackState.ecnCECountEchoed = + std::max(ackState.ecnCECountEchoed, readAckFrame.ecnCECount); +} } // namespace quic diff --git a/quic/state/AckHandlers.h b/quic/state/AckHandlers.h index 638f6c27b..8b37988cd 100644 --- a/quic/state/AckHandlers.h +++ b/quic/state/AckHandlers.h @@ -76,4 +76,32 @@ void parseAckReceiveTimestamps( const quic::ReadAckFrame& frame, folly::F14FastMap& packetReceiveTimeStamps, folly::Optional firstPacketNum); + +/** + * Update the outgoing ECN marking count for an outstanding packet that has been + * acked. If a packet is acked and the connection is using ECN/L4S, this + * function updates the ackState to expect more ECN marks to be echoed by the + * peer. + * + * Note: that we don't track the value of the actual mark sent in the packet. + * (1) This is fine because we do not allow the ECN mark to change during the + * lifetime of a connection. It can only be turned off if ECN marking validation + * fails. + * (2) This avoids adding more fields to the outstanding packet metadata. + * + * Note: Since only ack-eliciting packets are tracked as outstanding packets, + * the ECN count tracked by this function is only a minimum. Non-ack eliciting + * packets that are acked will not hit this function. + */ +void incrementEcnCountForAckedPacket( + QuicConnectionStateBase& conn, + PacketNumberSpace pnSpace); + +/** + * Update the ECN counts echoed by the pear in its ACK frame + */ +void updateEcnCountEchoed( + QuicConnectionStateBase& conn, + PacketNumberSpace pnSpace, + const ReadAckFrame& readAckFrame); } // namespace quic diff --git a/quic/state/AckStates.h b/quic/state/AckStates.h index 2853387b3..a6c84fac6 100644 --- a/quic/state/AckStates.h +++ b/quic/state/AckStates.h @@ -45,6 +45,16 @@ struct AckState : WriteAckFrameState { bool needsToSendAckImmediately{false}; // Count of outstanding packets received with retransmittable data. uint8_t numRxPacketsRecvd{0}; + // Out of the outstanding packets acked by the peer, how many were sent when + // the connection is using ECN marking. This is used to verify that the peer + // is correctly echoing the ECN marking in its ACKs. Note that this is a + // minimum count because it only tracks ack-eliciting packets that we sent + // (non-ack eliciting packets are not tracked as outstanding packets) + uint32_t minimumExpectedEcnMarksEchoed{0}; + // The counts of ECN counts echoed by the peer. + uint32_t ecnECT0CountEchoed{0}; + uint32_t ecnECT1CountEchoed{0}; + uint32_t ecnCECountEchoed{0}; }; struct AckStates { diff --git a/quic/state/test/AckHandlersTest.cpp b/quic/state/test/AckHandlersTest.cpp index bcc544ede..b352fdb8e 100644 --- a/quic/state/test/AckHandlersTest.cpp +++ b/quic/state/test/AckHandlersTest.cpp @@ -276,6 +276,9 @@ TEST_P(AckHandlersTest, TestAckWithECN) { conn.outstandings.packets.back().nonDsrPacketSequenceNumber = getAckState(conn, GetParam().pnSpace).nonDsrPacketSequenceNumber++; + // To enable accounting of sent marked packets + conn.ecnState = ECNState::ValidatedL4S; + ReadAckFrame ackFrame; auto ackReceiveTime = packetSendTime + 1ms; ackFrame.largestAcked = 5; @@ -296,6 +299,14 @@ TEST_P(AckHandlersTest, TestAckWithECN) { EXPECT_EQ(ackEvent.ecnECT0Count, 100); EXPECT_EQ(ackEvent.ecnECT1Count, 200); EXPECT_EQ(ackEvent.ecnCECount, 300); + + if (GetParam().pnSpace == PacketNumberSpace::AppData) { + auto& ackState = getAckState(conn, GetParam().pnSpace); + EXPECT_EQ(ackState.minimumExpectedEcnMarksEchoed, 1); + EXPECT_EQ(ackState.ecnECT0CountEchoed, 100); + EXPECT_EQ(ackState.ecnECT1CountEchoed, 200); + EXPECT_EQ(ackState.ecnCECountEchoed, 300); + } } TEST_P(AckHandlersTest, TestSpuriousLossFullRemoval) {