wgengine/magicsock: add MTU to addrLatency and rename to addrQuality

Add a field to record the wire MTU of the path to this address to the
addrLatency struct and rename it addrQuality.

Updates #311

Signed-off-by: Val <valerie@tailscale.com>
This commit is contained in:
Val 2023-09-20 14:44:10 +02:00 committed by valscale
parent 425cf9aa9d
commit 67926ede39
2 changed files with 31 additions and 14 deletions

View File

@ -65,7 +65,7 @@ type endpoint struct {
lastFullPing mono.Time // last time we pinged all disco or wireguard only endpoints lastFullPing mono.Time // last time we pinged all disco or wireguard only endpoints
derpAddr netip.AddrPort // fallback/bootstrap path, if non-zero (non-zero for well-behaved clients) derpAddr netip.AddrPort // fallback/bootstrap path, if non-zero (non-zero for well-behaved clients)
bestAddr addrLatency // best non-DERP path; zero if none bestAddr addrQuality // best non-DERP path; zero if none
bestAddrAt mono.Time // time best address re-confirmed bestAddrAt mono.Time // time best address re-confirmed
trustBestAddrUntil mono.Time // time when bestAddr expires trustBestAddrUntil mono.Time // time when bestAddr expires
sentPing map[stun.TxID]sentPing sentPing map[stun.TxID]sentPing
@ -207,7 +207,7 @@ func (de *endpoint) deleteEndpointLocked(why string, ep netip.AddrPort) {
What: "deleteEndpointLocked-bestAddr-" + why, What: "deleteEndpointLocked-bestAddr-" + why,
From: de.bestAddr, From: de.bestAddr,
}) })
de.bestAddr = addrLatency{} de.bestAddr = addrQuality{}
} }
} }
@ -906,7 +906,7 @@ func (de *endpoint) addCandidateEndpoint(ep netip.AddrPort, forRxPingTxID stun.T
// //
// de.mu must be held. // de.mu must be held.
func (de *endpoint) clearBestAddrLocked() { func (de *endpoint) clearBestAddrLocked() {
de.bestAddr = addrLatency{} de.bestAddr = addrQuality{}
de.bestAddrAt = 0 de.bestAddrAt = 0
de.trustBestAddrUntil = 0 de.trustBestAddrUntil = 0
} }
@ -996,7 +996,7 @@ func (de *endpoint) handlePongConnLocked(m *disco.Pong, di *discoInfo, src netip
// Promote this pong response to our current best address if it's lower latency. // Promote this pong response to our current best address if it's lower latency.
// TODO(bradfitz): decide how latency vs. preference order affects decision // TODO(bradfitz): decide how latency vs. preference order affects decision
if !isDerp { if !isDerp {
thisPong := addrLatency{sp.to, latency} thisPong := addrQuality{sp.to, latency, 0}
if betterAddr(thisPong, de.bestAddr) { if betterAddr(thisPong, de.bestAddr) {
de.c.logf("magicsock: disco: node %v %v now using %v", de.publicKey.ShortString(), de.discoShort(), sp.to) de.c.logf("magicsock: disco: node %v %v now using %v", de.publicKey.ShortString(), de.discoShort(), sp.to)
de.debugUpdates.Add(EndpointChange{ de.debugUpdates.Add(EndpointChange{
@ -1022,19 +1022,28 @@ func (de *endpoint) handlePongConnLocked(m *disco.Pong, di *discoInfo, src netip
return return
} }
// addrLatency is an IPPort with an associated latency. // addrQuality is an IPPort with an associated latency and path mtu.
type addrLatency struct { type addrQuality struct {
netip.AddrPort netip.AddrPort
latency time.Duration latency time.Duration
wireMTU tstun.WireMTU
} }
func (a addrLatency) String() string { func (a addrQuality) String() string {
return a.AddrPort.String() + "@" + a.latency.String() return fmt.Sprintf("%v@%v+%v", a.AddrPort, a.latency, a.wireMTU)
} }
// betterAddr reports whether a is a better addr to use than b. // betterAddr reports whether a is a better addr to use than b.
func betterAddr(a, b addrLatency) bool { func betterAddr(a, b addrQuality) bool {
if a.AddrPort == b.AddrPort { if a.AddrPort == b.AddrPort {
if a.wireMTU > b.wireMTU {
// TODO(val): Think harder about the case of lower
// latency and smaller or unknown MTU, and higher
// latency but larger MTU. Probably in most cases the
// largest MTU will also be the lowest latency but we
// can't depend on that.
return true
}
return false return false
} }
if !b.IsValid() { if !b.IsValid() {

View File

@ -1648,10 +1648,13 @@ func TestEndpointSetsEqual(t *testing.T) {
func TestBetterAddr(t *testing.T) { func TestBetterAddr(t *testing.T) {
const ms = time.Millisecond const ms = time.Millisecond
al := func(ipps string, d time.Duration) addrLatency { al := func(ipps string, d time.Duration) addrQuality {
return addrLatency{netip.MustParseAddrPort(ipps), d} return addrQuality{AddrPort: netip.MustParseAddrPort(ipps), latency: d}
} }
zero := addrLatency{} almtu := func(ipps string, d time.Duration, mtu tstun.WireMTU) addrQuality {
return addrQuality{AddrPort: netip.MustParseAddrPort(ipps), latency: d, wireMTU: mtu}
}
zero := addrQuality{}
const ( const (
publicV4 = "1.2.3.4:555" publicV4 = "1.2.3.4:555"
@ -1662,7 +1665,7 @@ func TestBetterAddr(t *testing.T) {
) )
tests := []struct { tests := []struct {
a, b addrLatency a, b addrQuality
want bool // whether a is better than b want bool // whether a is better than b
}{ }{
{a: zero, b: zero, want: false}, {a: zero, b: zero, want: false},
@ -1724,7 +1727,12 @@ func TestBetterAddr(t *testing.T) {
b: al(publicV6, 100*ms), b: al(publicV6, 100*ms),
want: true, want: true,
}, },
// If addresses are equal, prefer larger MTU
{
a: almtu(publicV4, 30*ms, 1500),
b: almtu(publicV4, 30*ms, 0),
want: true,
},
// Private IPs are preferred over public IPs even if the public // Private IPs are preferred over public IPs even if the public
// IP is IPv6. // IP is IPv6.
{ {