refactor: make route exploration deterministic

Author: apple050620312

WinMTR.vcxproj

@@ -1416,6 +1416,7 @@
     <ClInclude Include="WinMTRMain.h" />
     <ClInclude Include="WinMTRNetworkData.h" />
     <ClInclude Include="WinMTRProperties.h" />
+    <ClInclude Include="WinMTRRoutePolicy.h" />
     <ClInclude Include="WinMTRSerialization.h" />
     <ClCompile Include="WinMTRUtils.ixx">
       <PrecompiledHeader Condition="'$(Configuration)|$(Platform)'=='Debug|Win32'">NotUsing</PrecompiledHeader>

WinMTRNet-Tracing.cpp

@@ -12,6 +12,7 @@ of the License.
 module;
 #pragma warning (disable : 4005)
 #include "targetver.h"
+#include "WinMTRRoutePolicy.h"
 #define WIN32_LEAN_AND_MEAN
 #define VC_EXTRALEAN
 #define NOMCX
@@ -670,6 +671,15 @@ WinMTRTraceResult WinMTRNet::DoTrace(std::stop_token stop_token, SOCKADDR_INET a
 	auto now = monotonic_now();
 	scheduler.start(this_session, current_epoch, now);
 	const auto mandatory_ttl = std::max(trace_options.start_ttl, trace_options.minimum_ttl);
+	winmtr::route::RoutePolicy route_policy({
+		.start_ttl = trace_options.start_ttl,
+		.minimum_ttl = trace_options.minimum_ttl,
+		.max_hops = trace_options.max_hops,
+		.unknown_host_limit = trace_options.unknown_host_limit,
+		.exploration_period = WinMTRUtils::PATH_EXPLORATION_PERIOD,
+		.exploration_frontier_ttls = WinMTRUtils::PATH_EXPLORATION_FRONTIER_TTLS,
+		.shrink_confirmations = WinMTRUtils::PATH_SHRINK_CONFIRMATIONS,
+	});
 	// The first path discovery is complete but staggered over one interval.  A
 	// full initial sweep avoids hiding a destination behind an early run of
 	// silent routers; subsequent cycles use the configured unknown tail.
@@ -682,13 +692,6 @@ WinMTRTraceResult WinMTRNet::DoTrace(std::stop_token stop_token, SOCKADDR_INET a
 	bool session_had_usable_reply = false;
 	bool stopping = false;
 	std::optional<winmtr::probe::MonotonicMilliseconds> drain_deadline;
-	bool exploration_cycle = true;
-	bool initial_discovery = true;
-	unsigned highest_response_ttl = 0;
-	unsigned destination_ttl = 0;
-	unsigned stable_ceiling = initial_ceiling;
-	unsigned shrink_candidate = 0;
-	unsigned shrink_confirmations = 0;
 
 	const auto notify_changed = [this] {
 		if (options != nullptr) options->notifyTraceDataChanged();
@@ -704,50 +707,8 @@ WinMTRTraceResult WinMTRNet::DoTrace(std::stop_token stop_token, SOCKADDR_INET a
 			&& minimum > reported_cycles) {
 			reported_cycles = minimum;
 			setCompletedCycles(reported_cycles, current_epoch);
-			if (exploration_cycle) {
-				const auto tail_origin = highest_response_ttl == 0
-					? trace_options.start_ttl - 1u
-					: highest_response_ttl;
-				const auto normal_ceiling = destination_ttl != 0
-					? std::max(mandatory_ttl, destination_ttl)
-					: std::min(trace_options.max_hops,
-						std::max(mandatory_ttl, tail_origin + trace_options.unknown_host_limit));
-				if (initial_discovery) {
-					stable_ceiling = normal_ceiling;
-					initial_discovery = false;
-					shrink_candidate = 0;
-					shrink_confirmations = 0;
-				}
-				else if (normal_ceiling < stable_ceiling) {
-					if (shrink_candidate == normal_ceiling) {
-						++shrink_confirmations;
-					}
-					else {
-						shrink_candidate = normal_ceiling;
-						shrink_confirmations = 1;
-					}
-					if (shrink_confirmations >= WinMTRUtils::PATH_SHRINK_CONFIRMATIONS) {
-						stable_ceiling = normal_ceiling;
-						shrink_candidate = 0;
-						shrink_confirmations = 0;
-					}
-				}
-				else {
-					stable_ceiling = normal_ceiling;
-					shrink_candidate = 0;
-					shrink_confirmations = 0;
-				}
-				scheduler.set_last_ttl(stable_ceiling, monotonic_now());
-				exploration_cycle = false;
-			}
-			if (reported_cycles % WinMTRUtils::PATH_EXPLORATION_PERIOD == 0
-				&& stable_ceiling < trace_options.max_hops) {
-				exploration_cycle = true;
-				highest_response_ttl = 0;
-				destination_ttl = 0;
-				const auto frontier_ceiling = std::min(trace_options.max_hops,
-					stable_ceiling + WinMTRUtils::PATH_EXPLORATION_FRONTIER_TTLS);
-				scheduler.set_last_ttl(frontier_ceiling, monotonic_now());
+			if (const auto ceiling = route_policy.complete_cycle(reported_cycles)) {
+				scheduler.set_last_ttl(*ceiling, monotonic_now());
 			}
 		}
 	};
@@ -760,13 +721,7 @@ WinMTRTraceResult WinMTRNet::DoTrace(std::stop_token stop_token, SOCKADDR_INET a
 			reported_cycles = 0;
 			session_reached_destination = false;
 			session_had_usable_reply = false;
-			exploration_cycle = true;
-			initial_discovery = true;
-			highest_response_ttl = 0;
-			destination_ttl = 0;
-			stable_ceiling = initial_ceiling;
-			shrink_candidate = 0;
-			shrink_confirmations = 0;
+			route_policy.reset();
 			scheduler.restart(current_epoch, now);
 			scheduler.set_last_ttl(initial_ceiling, now);
 		}
@@ -776,8 +731,8 @@ WinMTRTraceResult WinMTRNet::DoTrace(std::stop_token stop_token, SOCKADDR_INET a
 			if (found == requests.end()) continue;
 			const auto disposition = scheduler.complete(request->token,
 				request->completion_kind, request->completed_at);
-			const auto accepted_after_destination = destination_ttl != 0
-				&& request->token.ttl > std::max(mandatory_ttl, destination_ttl)
+			const auto accepted_after_destination = route_policy.destination_ttl() != 0
+				&& request->token.ttl > std::max(mandatory_ttl, route_policy.destination_ttl())
 				&& (disposition == winmtr::probe::CompletionDisposition::accepted_reply
 					|| disposition == winmtr::probe::CompletionDisposition::accepted_timeout
 					|| disposition == winmtr::probe::CompletionDisposition::accepted_local_error
@@ -802,12 +757,9 @@ WinMTRTraceResult WinMTRNet::DoTrace(std::stop_token stop_token, SOCKADDR_INET a
 					trace_options.resolve_hostnames,
 					trace_options.lookup_asn_isp);
 				session_had_usable_reply = true;
-				highest_response_ttl = std::max(highest_response_ttl, request->token.ttl);
+				route_policy.note_reply(request->token.ttl, request->destination_reply);
 				if (request->destination_reply) {
 					session_reached_destination = true;
-					destination_ttl = destination_ttl == 0
-						? request->token.ttl
-						: std::min(destination_ttl, request->token.ttl);
 					scheduler.set_last_ttl(std::max(mandatory_ttl, request->token.ttl), now);
 				}
 				notify_changed();
@@ -847,8 +799,8 @@ WinMTRTraceResult WinMTRNet::DoTrace(std::stop_token stop_token, SOCKADDR_INET a
 			drain_deadline = now + trace_options.grace_ms;
 		}
 		for (const auto& expired : scheduler.expire(now)) {
-			if (destination_ttl != 0
-				&& expired.ttl > std::max(mandatory_ttl, destination_ttl)) {
+			if (route_policy.destination_ttl() != 0
+				&& expired.ttl > std::max(mandatory_ttl, route_policy.destination_ttl())) {
 				commitPostDestinationCompletion(expired.ttl, expired.epoch);
 			}
 			else {
@@ -880,9 +832,7 @@ WinMTRTraceResult WinMTRNet::DoTrace(std::stop_token stop_token, SOCKADDR_INET a
 					commitCacheSkipped(slot.token.ttl, slot.token.epoch);
 					if (cached_destination) {
 						session_reached_destination = true;
-						destination_ttl = destination_ttl == 0
-							? slot.token.ttl
-							: std::min(destination_ttl, slot.token.ttl);
+						route_policy.note_reply(slot.token.ttl, true);
 						scheduler.set_last_ttl(std::max(mandatory_ttl, slot.token.ttl), now);
 					}
 					notify_changed();

WinMTRRoutePolicy.h

@@ -0,0 +1,140 @@
+#pragma once
+
+#include <algorithm>
+#include <cstdint>
+#include <optional>
+
+namespace winmtr::route {
+
+struct RoutePolicyConfig final {
+	unsigned start_ttl = 1;
+	unsigned minimum_ttl = 0;
+	unsigned max_hops = 30;
+	unsigned unknown_host_limit = 12;
+	unsigned exploration_period = 10;
+	unsigned exploration_frontier_ttls = 5;
+	unsigned shrink_confirmations = 2;
+};
+
+class RoutePolicy final {
+public:
+	explicit RoutePolicy(RoutePolicyConfig config) noexcept
+		: config_(normalize(config))
+	{
+		reset();
+	}
+
+	void reset() noexcept
+	{
+		exploring_ = true;
+		initial_discovery_ = true;
+		highest_response_ttl_ = 0;
+		destination_ttl_ = 0;
+		stable_ceiling_ = config_.max_hops;
+		shrink_candidate_ = 0;
+		shrink_confirmations_ = 0;
+	}
+
+	void note_reply(unsigned ttl, bool destination) noexcept
+	{
+		if (ttl < config_.start_ttl || ttl > config_.max_hops) return;
+		highest_response_ttl_ = std::max(highest_response_ttl_, ttl);
+		if (destination) {
+			destination_ttl_ = destination_ttl_ == 0
+				? ttl
+				: std::min(destination_ttl_, ttl);
+		}
+	}
+
+	// Returns a new active TTL ceiling only when the scheduler must change it.
+	[[nodiscard]] std::optional<unsigned> complete_cycle(
+		std::uint64_t completed_cycles) noexcept
+	{
+		std::optional<unsigned> next;
+		if (exploring_) {
+			const auto normal = normal_ceiling();
+			if (initial_discovery_) {
+				stable_ceiling_ = normal;
+				initial_discovery_ = false;
+				clear_shrink_candidate();
+			}
+			else if (normal < stable_ceiling_) {
+				if (shrink_candidate_ == normal) ++shrink_confirmations_;
+				else {
+					shrink_candidate_ = normal;
+					shrink_confirmations_ = 1;
+				}
+				if (shrink_confirmations_ >= config_.shrink_confirmations) {
+					stable_ceiling_ = normal;
+					clear_shrink_candidate();
+				}
+			}
+			else {
+				stable_ceiling_ = normal;
+				clear_shrink_candidate();
+			}
+			exploring_ = false;
+			next = stable_ceiling_;
+		}
+
+		if (config_.exploration_period != 0 && completed_cycles != 0
+			&& completed_cycles % config_.exploration_period == 0
+			&& stable_ceiling_ < config_.max_hops) {
+			exploring_ = true;
+			highest_response_ttl_ = 0;
+			destination_ttl_ = 0;
+			next = std::min(config_.max_hops,
+				stable_ceiling_ + config_.exploration_frontier_ttls);
+		}
+		return next;
+	}
+
+	[[nodiscard]] unsigned destination_ttl() const noexcept { return destination_ttl_; }
+	[[nodiscard]] unsigned stable_ceiling() const noexcept { return stable_ceiling_; }
+	[[nodiscard]] bool exploring() const noexcept { return exploring_; }
+
+private:
+	[[nodiscard]] static RoutePolicyConfig normalize(RoutePolicyConfig value) noexcept
+	{
+		value.start_ttl = std::max(1u, value.start_ttl);
+		value.max_hops = std::max(value.start_ttl, value.max_hops);
+		value.minimum_ttl = std::min(value.minimum_ttl, value.max_hops);
+		value.unknown_host_limit = std::max(1u, value.unknown_host_limit);
+		value.shrink_confirmations = std::max(1u, value.shrink_confirmations);
+		return value;
+	}
+
+	[[nodiscard]] unsigned mandatory_ttl() const noexcept
+	{
+		return std::max(config_.start_ttl, config_.minimum_ttl);
+	}
+
+	[[nodiscard]] unsigned normal_ceiling() const noexcept
+	{
+		if (destination_ttl_ != 0) {
+			return std::max(mandatory_ttl(), destination_ttl_);
+		}
+		const auto tail_origin = highest_response_ttl_ == 0
+			? config_.start_ttl - 1u
+			: highest_response_ttl_;
+		return std::min(config_.max_hops,
+			std::max(mandatory_ttl(), tail_origin + config_.unknown_host_limit));
+	}
+
+	void clear_shrink_candidate() noexcept
+	{
+		shrink_candidate_ = 0;
+		shrink_confirmations_ = 0;
+	}
+
+	RoutePolicyConfig config_;
+	bool exploring_ = true;
+	bool initial_discovery_ = true;
+	unsigned highest_response_ttl_ = 0;
+	unsigned destination_ttl_ = 0;
+	unsigned stable_ceiling_ = 0;
+	unsigned shrink_candidate_ = 0;
+	unsigned shrink_confirmations_ = 0;
+};
+
+} // namespace winmtr::route

tests/ProbeSchedulerTests.cpp

@@ -2,6 +2,7 @@
 #include "WinMTRAddressPolicy.h"
 #include "WinMTRJson.h"
 #include "WinMTRSerialization.h"
+#include "WinMTRRoutePolicy.h"
 
 #include <array>
 #include <cmath>
@@ -393,6 +394,53 @@ void test_deterministic_scheduler_resource_budget()
 		"resource budget scheduler starved a TTL");
 }
 
+void test_route_policy_scripted_changes()
+{
+	using winmtr::route::RoutePolicy;
+	using winmtr::route::RoutePolicyConfig;
+	RoutePolicy policy(RoutePolicyConfig{
+		.start_ttl = 1,
+		.minimum_ttl = 0,
+		.max_hops = 30,
+		.unknown_host_limit = 12,
+		.exploration_period = 10,
+		.exploration_frontier_ttls = 5,
+		.shrink_confirmations = 2,
+	});
+
+	policy.note_reply(8, true);
+	require(policy.complete_cycle(1) == 8u && policy.stable_ceiling() == 8u,
+		"initial destination did not establish the route ceiling");
+	require(policy.complete_cycle(10) == 13u && policy.exploring(),
+		"periodic frontier did not extend five TTLs");
+	policy.note_reply(12, true);
+	require(policy.complete_cycle(11) == 12u && policy.stable_ceiling() == 12u,
+		"longer route was not adopted after frontier discovery");
+
+	require(policy.complete_cycle(20) == 17u, "second frontier was not scheduled");
+	policy.note_reply(6, true);
+	require(policy.complete_cycle(21) == 12u && policy.stable_ceiling() == 12u,
+		"route shrank without the required hysteresis confirmation");
+	require(policy.complete_cycle(30) == 17u, "shrink confirmation frontier was not scheduled");
+	policy.note_reply(6, true);
+	require(policy.complete_cycle(31) == 6u && policy.stable_ceiling() == 6u,
+		"confirmed shorter route was not adopted");
+
+	policy.reset();
+	policy.note_reply(8, false);
+	require(policy.complete_cycle(1) == 20u,
+		"silent destination did not retain the configured unknown tail");
+
+	RoutePolicy minimumPolicy(RoutePolicyConfig{
+		.start_ttl = 1, .minimum_ttl = 15, .max_hops = 30,
+		.unknown_host_limit = 5, .exploration_period = 10,
+		.exploration_frontier_ttls = 5, .shrink_confirmations = 2,
+	});
+	minimumPolicy.note_reply(6, true);
+	require(minimumPolicy.complete_cycle(1) == 15u,
+		"minimum TTL was not enforced after an early destination");
+}
+
 void fuzz_json_parser_offline()
 {
 	std::uint64_t state = 0x243f6a8885a308d3ull;
@@ -428,6 +476,7 @@ int main()
 		test_global_rate_limit_is_fair();
 		test_grace_cancellation_is_not_loss();
 		test_deterministic_scheduler_resource_budget();
+		test_route_policy_scripted_changes();
 		fuzz_json_parser_offline();
 		std::cout << "All probe scheduler tests passed.\n";
 		return 0;