Browse Source

ipn/ipnstate: address TODO about garbage during peer sorting

Updates #cleanup

Change-Id: I34938bca70a95571cc62ce1f76eaab5db8c2c3ef
Signed-off-by: Brad Fitzpatrick <[email protected]>
Brad Fitzpatrick 2 years ago
parent
commit
d506a55c8a
2 changed files with 24 additions and 13 deletions
  1. 18 13
      ipn/ipnstate/ipnstate.go
  2. 6 0
      types/key/node.go

+ 18 - 13
ipn/ipnstate/ipnstate.go

@@ -12,6 +12,7 @@ import (
 	"io"
 	"log"
 	"net/netip"
+	"slices"
 	"sort"
 	"strings"
 	"sync"
@@ -680,25 +681,29 @@ func (pr *PingResult) ToPingResponse(pingType tailcfg.PingType) *tailcfg.PingRes
 	}
 }
 
-// SortPeers sorts peers by either the DNS name, hostname, Tailscale IP,
-// or current public key.
+// SortPeers sorts peers by either their DNS name, hostname, Tailscale IP,
+// or ultimately their current public key.
 func SortPeers(peers []*PeerStatus) {
-	sort.Slice(peers, func(i, j int) bool { return sortKey(peers[i]) < sortKey(peers[j]) })
+	slices.SortStableFunc(peers, (*PeerStatus).compare)
 }
 
-func sortKey(ps *PeerStatus) string {
-	if ps.DNSName != "" {
-		return ps.DNSName
+func (a *PeerStatus) compare(b *PeerStatus) int {
+	if a.DNSName != "" || b.DNSName != "" {
+		if v := strings.Compare(a.DNSName, b.DNSName); v != 0 {
+			return v
+		}
 	}
-	if ps.HostName != "" {
-		return ps.HostName
+	if a.HostName != "" || b.HostName != "" {
+		if v := strings.Compare(a.HostName, b.HostName); v != 0 {
+			return v
+		}
 	}
-	// TODO(bradfitz): add PeerStatus.Less and avoid these allocs in a Less func.
-	if len(ps.TailscaleIPs) > 0 {
-		return ps.TailscaleIPs[0].String()
+	if len(a.TailscaleIPs) > 0 && len(b.TailscaleIPs) > 0 {
+		if v := a.TailscaleIPs[0].Compare(b.TailscaleIPs[0]); v != 0 {
+			return v
+		}
 	}
-	raw := ps.PublicKey.Raw32()
-	return string(raw[:])
+	return a.PublicKey.Compare(b.PublicKey)
 }
 
 // DebugDERPRegionReport is the result of a "tailscale debug derp" command,

+ 6 - 0
types/key/node.go

@@ -168,6 +168,12 @@ func (p NodePublic) Shard() uint8 {
 	return s ^ uint8(p.k[2]+p.k[12])
 }
 
+// Compare returns -1, 0, or 1, depending on whether p orders before p2,
+// using bytes.Compare on the bytes of the public key.
+func (p NodePublic) Compare(p2 NodePublic) int {
+	return bytes.Compare(p.k[:], p2.k[:])
+}
+
 // ParseNodePublicUntyped parses an untyped 64-character hex value
 // as a NodePublic.
 //