Loading DnsTlsDispatcher.cpp +6 −6 Original line number Diff line number Diff line Loading @@ -348,15 +348,15 @@ bool DnsTlsDispatcher::Transport::checkRevalidationNecessary(DnsTlsTransport::Re if (code == DnsTlsTransport::Response::network_error) { continuousfailureCount++; LOG(DEBUG) << "continuousfailureCount incremented: network_error, count = " LOG(WARNING) << "continuousfailureCount incremented: network_error, count = " << continuousfailureCount; } else if (latencyTooHigh) { continuousfailureCount++; LOG(DEBUG) << "continuousfailureCount incremented: latency too High, count = " LOG(WARNING) << "continuousfailureCount incremented: latency too High, count = " << continuousfailureCount; } else if (continuousfailureCount > 0) { } else { continuousfailureCount = 0; LOG(DEBUG) << "continuousfailureCount reset"; LOG(WARNING) << "continuousfailureCount reset"; } // triggerThreshold must be greater than 0 because the value of revalidationEnabled is true. Loading PrivateDnsConfiguration.cpp +21 −43 Original line number Diff line number Diff line Loading @@ -61,11 +61,6 @@ bool parseServer(const char* server, sockaddr_storage* parsed) { return true; } // Returns true if the IPrivateDnsServer was created when the mode was opportunistic. bool isForOpportunisticMode(const PrivateDnsConfiguration::ServerIdentity& identity) { return identity.provider.empty(); } int PrivateDnsConfiguration::set(int32_t netId, uint32_t mark, const std::vector<std::string>& servers, const std::string& name, const std::string& caCert) { Loading Loading @@ -207,11 +202,20 @@ void PrivateDnsConfiguration::startValidation(const ServerIdentity& identity, un const bool avoidBadPrivateDns = Experiments::getInstance()->getFlag("avoid_bad_private_dns", 0); std::optional<int64_t> latencyThreshold; if (avoidBadPrivateDns) { const int maxLatency = Experiments::getInstance()->getFlag( "max_private_dns_latency_threshold_ms", kMaxPrivateDnsLatencyThresholdMs); const int minLatency = Experiments::getInstance()->getFlag( "min_private_dns_latency_threshold_ms", kMinPrivateDnsLatencyThresholdMs); std::optional<int64_t> latencyThreshold; const auto do53Latency = resolv_stats_get_average_response_time(netId, PROTO_UDP); const int target = do53Latency.has_value() ? (3 * do53Latency.value().count() / 1000) : 0; // The time is limited to the range [minLatency, maxLatency]. latencyThreshold = std::clamp(target, minLatency, maxLatency); } const bool isOpportunisticMode = server.name.empty(); // cat /proc/sys/net/ipv4/tcp_syn_retries yields "6". // Loading @@ -228,20 +232,7 @@ void PrivateDnsConfiguration::startValidation(const ServerIdentity& identity, un // (6 SYNs per ip, 4 ips per validation pass, 24 passes per day). auto backoff = mBackoffBuilder.build(); for (int attempt = 1; /**/; ++attempt) { // Because the time between two probes is at least one minute, there might already be // some traffic sent to Do53 servers during the time. Update latencyThreshold every // time before the probe. if (avoidBadPrivateDns && isForOpportunisticMode(identity)) { const auto do53Latency = resolv_stats_get_average_response_time(netId, PROTO_UDP); const int target = do53Latency.has_value() ? (3 * do53Latency.value().count() / 1000) : minLatency; // The threshold is limited to the range [minLatency, maxLatency]. latencyThreshold = std::clamp(target, minLatency, maxLatency); } while (true) { // ::validate() is a blocking call that performs network operations. // It can take milliseconds to minutes, up to the SYN retry limit. LOG(WARNING) << "Validating DnsTlsServer " << server.toIpString() << " with mark 0x" Loading @@ -250,28 +241,20 @@ void PrivateDnsConfiguration::startValidation(const ServerIdentity& identity, un Stopwatch stopwatch; const bool gotAnswer = DnsTlsTransport::validate(server, server.validationMark()); const int32_t timeTaken = saturate_cast<int32_t>(stopwatch.timeTakenUs() / 1000); LOG(WARNING) << fmt::format( "validateDnsTlsServer returned {} for {}, took {}ms, attempt {}", gotAnswer, server.toIpString(), timeTaken, attempt); // Prevent from endlessly sending traffic on the network in opportunistic mode. bool maxAttemptsReached = false; if (avoidBadPrivateDns && attempt >= kOpportunisticModeMaxAttempts && isForOpportunisticMode(identity)) { maxAttemptsReached = true; LOG(WARNING) << "Max attempts reached: " << kOpportunisticModeMaxAttempts; } LOG(WARNING) << fmt::format("validateDnsTlsServer returned {} for {}, took {}ms", gotAnswer, server.toIpString(), timeTaken); const int64_t targetTime = latencyThreshold.value_or(INT64_MAX); const bool latencyTooHigh = timeTaken > targetTime; if (latencyTooHigh) { bool latencyTooHigh = false; if (isOpportunisticMode && timeTaken > targetTime) { latencyTooHigh = true; LOG(WARNING) << "validateDnsTlsServer took too long: threshold is " << targetTime << "ms"; } // TODO: combine these boolean variables into a bitwise variable. const bool needs_reeval = this->recordPrivateDnsValidation( identity, netId, gotAnswer, isRevalidation, latencyTooHigh, maxAttemptsReached); identity, netId, gotAnswer, isRevalidation, latencyTooHigh); if (!needs_reeval) { break; Loading Loading @@ -322,8 +305,7 @@ void PrivateDnsConfiguration::sendPrivateDnsValidationEvent(const ServerIdentity bool PrivateDnsConfiguration::recordPrivateDnsValidation(const ServerIdentity& identity, unsigned netId, bool gotAnswer, bool isRevalidation, bool latencyTooHigh, bool maxAttemptsReached) { bool isRevalidation, bool latencyTooHigh) { constexpr bool NEEDS_REEVALUATION = true; constexpr bool DONT_REEVALUATE = false; Loading Loading @@ -354,10 +336,6 @@ bool PrivateDnsConfiguration::recordPrivateDnsValidation(const ServerIdentity& i reevaluationStatus = DONT_REEVALUATE; } if (maxAttemptsReached) { reevaluationStatus = DONT_REEVALUATE; } bool success = gotAnswer; auto& tracker = netPair->second; auto serverPair = tracker.find(identity); Loading PrivateDnsConfiguration.h +2 −5 Original line number Diff line number Diff line Loading @@ -98,9 +98,6 @@ class PrivateDnsConfiguration { static constexpr int kMaxPrivateDnsLatencyThresholdMs = 2000; static constexpr int kMinPrivateDnsLatencyThresholdMs = 500; // (Opportunistic mode only) The maximum attempts to send a probe to a private DNS server. static constexpr int kOpportunisticModeMaxAttempts = 3; PrivateDnsConfiguration() = default; // Launchs a thread to run the validation for |server| on the network |netId|. Loading @@ -109,8 +106,8 @@ class PrivateDnsConfiguration { REQUIRES(mPrivateDnsLock); bool recordPrivateDnsValidation(const ServerIdentity& identity, unsigned netId, bool success, bool isRevalidation, bool latencyTooHigh, bool maxAttemptsReached) EXCLUDES(mPrivateDnsLock); bool isRevalidation, bool latencyTooHigh) EXCLUDES(mPrivateDnsLock); void sendPrivateDnsValidationEvent(const ServerIdentity& identity, unsigned netId, bool success) REQUIRES(mPrivateDnsLock); Loading PrivateDnsConfigurationTest.cpp +0 −52 Original line number Diff line number Diff line Loading @@ -156,8 +156,6 @@ class PrivateDnsConfigurationTest : public ::testing::Test { static constexpr char kBackend[] = "127.0.2.1"; static constexpr char kServer1[] = "127.0.2.2"; static constexpr char kServer2[] = "127.0.2.3"; static constexpr int kOpportunisticModeMaxAttempts = PrivateDnsConfiguration::kOpportunisticModeMaxAttempts; MockObserver mObserver; PrivateDnsConfiguration mPdc; Loading Loading @@ -286,56 +284,6 @@ TEST_F(PrivateDnsConfigurationTest, ValidationProbingTime) { backend.setResponseDelayMs(0); } // Tests that Private DNS validation won't be endless if the server works and it's slow. TEST_F(PrivateDnsConfigurationTest, ValidationMaxProbes) { ScopedSystemProperty sp1(kMinPrivateDnsLatencyThresholdMsFlag, "100"); ScopedSystemProperty sp2(kMaxPrivateDnsLatencyThresholdMsFlag, "200"); const int serverLatencyMs = 300; // TODO: Complete STRICT test after the dependency of DnsTlsFrontend is removed. static const struct TestConfig { std::string dnsMode; bool avoidBadPrivateDns; Validation expectedValidationResult; int expectedProbes; } testConfigs[] = { // clang-format off {"OPPORTUNISTIC", false, Validation::success, 1}, {"OPPORTUNISTIC", true, Validation::fail, kOpportunisticModeMaxAttempts}, // clang-format on }; for (const auto& config : testConfigs) { SCOPED_TRACE( fmt::format("testConfig: [{}, {}]", config.dnsMode, config.avoidBadPrivateDns)); ScopedSystemProperty sp3(kAvoidBadPrivateDnsFlag, (config.avoidBadPrivateDns ? "1" : "0")); forceExperimentsInstanceUpdate(); backend.setResponseDelayMs(serverLatencyMs); testing::InSequence seq; EXPECT_CALL(mObserver, onValidationStateUpdate(kServer1, Validation::in_process, kNetId)) .Times(config.expectedProbes); EXPECT_CALL(mObserver, onValidationStateUpdate(kServer1, config.expectedValidationResult, kNetId)); EXPECT_EQ(mPdc.set(kNetId, kMark, {kServer1}, {}, {}), 0); expectPrivateDnsStatus(PrivateDnsMode::OPPORTUNISTIC); // probing time + backoff delay const int expectedValidationTimeMs = config.expectedProbes * serverLatencyMs + (config.expectedProbes - 1) * 1000; ASSERT_TRUE(PollForCondition([&]() { return mObserver.runningThreads == 0; }, std::chrono::milliseconds(expectedValidationTimeMs + 1000))); // Reset the state for the next round. mPdc.clear(kNetId); testing::Mock::VerifyAndClearExpectations(&mObserver); } backend.setResponseDelayMs(0); } TEST_F(PrivateDnsConfigurationTest, ValidationBlock) { backend.setDeferredResp(true); Loading Loading
DnsTlsDispatcher.cpp +6 −6 Original line number Diff line number Diff line Loading @@ -348,15 +348,15 @@ bool DnsTlsDispatcher::Transport::checkRevalidationNecessary(DnsTlsTransport::Re if (code == DnsTlsTransport::Response::network_error) { continuousfailureCount++; LOG(DEBUG) << "continuousfailureCount incremented: network_error, count = " LOG(WARNING) << "continuousfailureCount incremented: network_error, count = " << continuousfailureCount; } else if (latencyTooHigh) { continuousfailureCount++; LOG(DEBUG) << "continuousfailureCount incremented: latency too High, count = " LOG(WARNING) << "continuousfailureCount incremented: latency too High, count = " << continuousfailureCount; } else if (continuousfailureCount > 0) { } else { continuousfailureCount = 0; LOG(DEBUG) << "continuousfailureCount reset"; LOG(WARNING) << "continuousfailureCount reset"; } // triggerThreshold must be greater than 0 because the value of revalidationEnabled is true. Loading
PrivateDnsConfiguration.cpp +21 −43 Original line number Diff line number Diff line Loading @@ -61,11 +61,6 @@ bool parseServer(const char* server, sockaddr_storage* parsed) { return true; } // Returns true if the IPrivateDnsServer was created when the mode was opportunistic. bool isForOpportunisticMode(const PrivateDnsConfiguration::ServerIdentity& identity) { return identity.provider.empty(); } int PrivateDnsConfiguration::set(int32_t netId, uint32_t mark, const std::vector<std::string>& servers, const std::string& name, const std::string& caCert) { Loading Loading @@ -207,11 +202,20 @@ void PrivateDnsConfiguration::startValidation(const ServerIdentity& identity, un const bool avoidBadPrivateDns = Experiments::getInstance()->getFlag("avoid_bad_private_dns", 0); std::optional<int64_t> latencyThreshold; if (avoidBadPrivateDns) { const int maxLatency = Experiments::getInstance()->getFlag( "max_private_dns_latency_threshold_ms", kMaxPrivateDnsLatencyThresholdMs); const int minLatency = Experiments::getInstance()->getFlag( "min_private_dns_latency_threshold_ms", kMinPrivateDnsLatencyThresholdMs); std::optional<int64_t> latencyThreshold; const auto do53Latency = resolv_stats_get_average_response_time(netId, PROTO_UDP); const int target = do53Latency.has_value() ? (3 * do53Latency.value().count() / 1000) : 0; // The time is limited to the range [minLatency, maxLatency]. latencyThreshold = std::clamp(target, minLatency, maxLatency); } const bool isOpportunisticMode = server.name.empty(); // cat /proc/sys/net/ipv4/tcp_syn_retries yields "6". // Loading @@ -228,20 +232,7 @@ void PrivateDnsConfiguration::startValidation(const ServerIdentity& identity, un // (6 SYNs per ip, 4 ips per validation pass, 24 passes per day). auto backoff = mBackoffBuilder.build(); for (int attempt = 1; /**/; ++attempt) { // Because the time between two probes is at least one minute, there might already be // some traffic sent to Do53 servers during the time. Update latencyThreshold every // time before the probe. if (avoidBadPrivateDns && isForOpportunisticMode(identity)) { const auto do53Latency = resolv_stats_get_average_response_time(netId, PROTO_UDP); const int target = do53Latency.has_value() ? (3 * do53Latency.value().count() / 1000) : minLatency; // The threshold is limited to the range [minLatency, maxLatency]. latencyThreshold = std::clamp(target, minLatency, maxLatency); } while (true) { // ::validate() is a blocking call that performs network operations. // It can take milliseconds to minutes, up to the SYN retry limit. LOG(WARNING) << "Validating DnsTlsServer " << server.toIpString() << " with mark 0x" Loading @@ -250,28 +241,20 @@ void PrivateDnsConfiguration::startValidation(const ServerIdentity& identity, un Stopwatch stopwatch; const bool gotAnswer = DnsTlsTransport::validate(server, server.validationMark()); const int32_t timeTaken = saturate_cast<int32_t>(stopwatch.timeTakenUs() / 1000); LOG(WARNING) << fmt::format( "validateDnsTlsServer returned {} for {}, took {}ms, attempt {}", gotAnswer, server.toIpString(), timeTaken, attempt); // Prevent from endlessly sending traffic on the network in opportunistic mode. bool maxAttemptsReached = false; if (avoidBadPrivateDns && attempt >= kOpportunisticModeMaxAttempts && isForOpportunisticMode(identity)) { maxAttemptsReached = true; LOG(WARNING) << "Max attempts reached: " << kOpportunisticModeMaxAttempts; } LOG(WARNING) << fmt::format("validateDnsTlsServer returned {} for {}, took {}ms", gotAnswer, server.toIpString(), timeTaken); const int64_t targetTime = latencyThreshold.value_or(INT64_MAX); const bool latencyTooHigh = timeTaken > targetTime; if (latencyTooHigh) { bool latencyTooHigh = false; if (isOpportunisticMode && timeTaken > targetTime) { latencyTooHigh = true; LOG(WARNING) << "validateDnsTlsServer took too long: threshold is " << targetTime << "ms"; } // TODO: combine these boolean variables into a bitwise variable. const bool needs_reeval = this->recordPrivateDnsValidation( identity, netId, gotAnswer, isRevalidation, latencyTooHigh, maxAttemptsReached); identity, netId, gotAnswer, isRevalidation, latencyTooHigh); if (!needs_reeval) { break; Loading Loading @@ -322,8 +305,7 @@ void PrivateDnsConfiguration::sendPrivateDnsValidationEvent(const ServerIdentity bool PrivateDnsConfiguration::recordPrivateDnsValidation(const ServerIdentity& identity, unsigned netId, bool gotAnswer, bool isRevalidation, bool latencyTooHigh, bool maxAttemptsReached) { bool isRevalidation, bool latencyTooHigh) { constexpr bool NEEDS_REEVALUATION = true; constexpr bool DONT_REEVALUATE = false; Loading Loading @@ -354,10 +336,6 @@ bool PrivateDnsConfiguration::recordPrivateDnsValidation(const ServerIdentity& i reevaluationStatus = DONT_REEVALUATE; } if (maxAttemptsReached) { reevaluationStatus = DONT_REEVALUATE; } bool success = gotAnswer; auto& tracker = netPair->second; auto serverPair = tracker.find(identity); Loading
PrivateDnsConfiguration.h +2 −5 Original line number Diff line number Diff line Loading @@ -98,9 +98,6 @@ class PrivateDnsConfiguration { static constexpr int kMaxPrivateDnsLatencyThresholdMs = 2000; static constexpr int kMinPrivateDnsLatencyThresholdMs = 500; // (Opportunistic mode only) The maximum attempts to send a probe to a private DNS server. static constexpr int kOpportunisticModeMaxAttempts = 3; PrivateDnsConfiguration() = default; // Launchs a thread to run the validation for |server| on the network |netId|. Loading @@ -109,8 +106,8 @@ class PrivateDnsConfiguration { REQUIRES(mPrivateDnsLock); bool recordPrivateDnsValidation(const ServerIdentity& identity, unsigned netId, bool success, bool isRevalidation, bool latencyTooHigh, bool maxAttemptsReached) EXCLUDES(mPrivateDnsLock); bool isRevalidation, bool latencyTooHigh) EXCLUDES(mPrivateDnsLock); void sendPrivateDnsValidationEvent(const ServerIdentity& identity, unsigned netId, bool success) REQUIRES(mPrivateDnsLock); Loading
PrivateDnsConfigurationTest.cpp +0 −52 Original line number Diff line number Diff line Loading @@ -156,8 +156,6 @@ class PrivateDnsConfigurationTest : public ::testing::Test { static constexpr char kBackend[] = "127.0.2.1"; static constexpr char kServer1[] = "127.0.2.2"; static constexpr char kServer2[] = "127.0.2.3"; static constexpr int kOpportunisticModeMaxAttempts = PrivateDnsConfiguration::kOpportunisticModeMaxAttempts; MockObserver mObserver; PrivateDnsConfiguration mPdc; Loading Loading @@ -286,56 +284,6 @@ TEST_F(PrivateDnsConfigurationTest, ValidationProbingTime) { backend.setResponseDelayMs(0); } // Tests that Private DNS validation won't be endless if the server works and it's slow. TEST_F(PrivateDnsConfigurationTest, ValidationMaxProbes) { ScopedSystemProperty sp1(kMinPrivateDnsLatencyThresholdMsFlag, "100"); ScopedSystemProperty sp2(kMaxPrivateDnsLatencyThresholdMsFlag, "200"); const int serverLatencyMs = 300; // TODO: Complete STRICT test after the dependency of DnsTlsFrontend is removed. static const struct TestConfig { std::string dnsMode; bool avoidBadPrivateDns; Validation expectedValidationResult; int expectedProbes; } testConfigs[] = { // clang-format off {"OPPORTUNISTIC", false, Validation::success, 1}, {"OPPORTUNISTIC", true, Validation::fail, kOpportunisticModeMaxAttempts}, // clang-format on }; for (const auto& config : testConfigs) { SCOPED_TRACE( fmt::format("testConfig: [{}, {}]", config.dnsMode, config.avoidBadPrivateDns)); ScopedSystemProperty sp3(kAvoidBadPrivateDnsFlag, (config.avoidBadPrivateDns ? "1" : "0")); forceExperimentsInstanceUpdate(); backend.setResponseDelayMs(serverLatencyMs); testing::InSequence seq; EXPECT_CALL(mObserver, onValidationStateUpdate(kServer1, Validation::in_process, kNetId)) .Times(config.expectedProbes); EXPECT_CALL(mObserver, onValidationStateUpdate(kServer1, config.expectedValidationResult, kNetId)); EXPECT_EQ(mPdc.set(kNetId, kMark, {kServer1}, {}, {}), 0); expectPrivateDnsStatus(PrivateDnsMode::OPPORTUNISTIC); // probing time + backoff delay const int expectedValidationTimeMs = config.expectedProbes * serverLatencyMs + (config.expectedProbes - 1) * 1000; ASSERT_TRUE(PollForCondition([&]() { return mObserver.runningThreads == 0; }, std::chrono::milliseconds(expectedValidationTimeMs + 1000))); // Reset the state for the next round. mPdc.clear(kNetId); testing::Mock::VerifyAndClearExpectations(&mObserver); } backend.setResponseDelayMs(0); } TEST_F(PrivateDnsConfigurationTest, ValidationBlock) { backend.setDeferredResp(true); Loading