From bcf7b63d7e8cd104e61c984dcc7047cc23ca697e Mon Sep 17 00:00:00 2001 From: Andrew Dunham Date: Fri, 21 Apr 2023 11:35:16 -0400 Subject: [PATCH] wgengine/magicsock: add hysteresis to endpoint selection Avoid selecting an endpoint as "better" than the current endpoint if the total latency improvement is less than 1%. This adds some hysteresis to avoid flapping between endpoints for a minimal improvement in latency. Signed-off-by: Andrew Dunham Change-Id: If8312e1768ea65c4b4d4e13d8de284b3825d7a73 --- wgengine/magicsock/magicsock.go | 22 ++++++++++++++++++++++ wgengine/magicsock/magicsock_test.go | 6 +++++- 2 files changed, 27 insertions(+), 1 deletion(-) diff --git a/wgengine/magicsock/magicsock.go b/wgengine/magicsock/magicsock.go index 123b2e70b..95af25891 100644 --- a/wgengine/magicsock/magicsock.go +++ b/wgengine/magicsock/magicsock.go @@ -4921,6 +4921,28 @@ func betterAddr(a, b addrLatency) bool { return false } } + + // If we get here, then both addresses are the same IP type (i.e. both + // IPv4 or both IPv6). All decisions below are made solely on latency. + // + // Determine how much the latencies differ; we ensure the larger + // latency is the denominator, so this fraction will always be <= 1.0. + var latencyFraction float64 + if a.latency >= b.latency { + latencyFraction = float64(b.latency) / float64(a.latency) + } else { + latencyFraction = float64(a.latency) / float64(b.latency) + } + + // Don't change anything if the latency improvement is less than 1%; we + // want a bit of "stickiness" (a.k.a. hysteresis) to avoid flapping if + // there's two roughly-equivalent endpoints. + if latencyFraction >= 0.99 { + return false + } + + // The total difference is >1%, so a is better than b if it's + // lower-latency. return a.latency < b.latency } diff --git a/wgengine/magicsock/magicsock_test.go b/wgengine/magicsock/magicsock_test.go index c050cfc20..9115a3eea 100644 --- a/wgengine/magicsock/magicsock_test.go +++ b/wgengine/magicsock/magicsock_test.go @@ -1633,9 +1633,13 @@ func TestBetterAddr(t *testing.T) { {a: zero, b: zero, want: false}, {a: al("10.0.0.2:123", 5*ms), b: zero, want: true}, {a: zero, b: al("10.0.0.2:123", 5*ms), want: false}, - {a: al("10.0.0.2:123", 5*ms), b: al("1.2.3.4:555", 6*ms), want: true}, + {a: al("10.0.0.2:123", 5*ms), b: al("1.2.3.4:555", 10*ms), want: true}, {a: al("10.0.0.2:123", 5*ms), b: al("10.0.0.2:123", 10*ms), want: false}, // same IPPort + // Don't prefer b to a if it's not substantially better. + {a: al("10.0.0.2:123", 100*ms), b: al("1.2.3.4:555", 101*ms), want: false}, + {a: al("10.0.0.2:123", 100*ms), b: al("1.2.3.4:555", 103*ms), want: true}, + // Prefer IPv6 if roughly equivalent: { a: al("[2001::5]:123", 100*ms),