state: compare peer-visible Hostinfo through views

Field by field: no allocation, no reflection.

Updates #3531

hscontrol/state/maprequest.go

@@ -10,6 +10,7 @@ import (
 	"github.com/rs/zerolog"
 	"github.com/rs/zerolog/log"
 	"tailscale.com/tailcfg"
+	"tailscale.com/types/views"
 )
 
 // mapRequestDelta carries the classified facts extracted from one MapRequest
@@ -154,30 +155,56 @@ func hostinfoEqual(oldHI, newHI *tailcfg.Hostinfo) bool {
 	return oldCopy.Equal(&newCopy)
 }
 
-// peerHostinfo keeps the Hostinfo fields another node's client reads from
-// a peer: what tailscale status shows, PeerAPI services, SSH known hosts,
-// exit-node location, app-connector eligibility, and the routes this server
-// feeds into policy. Everything else is stored but never fanned out, and a
-// peer's NetInfo is never read at all.
-func peerHostinfo(hi *tailcfg.Hostinfo) *tailcfg.Hostinfo {
-	if hi == nil {
-		return nil
+// peerHostinfoEqual reports whether a and b agree on the Hostinfo fields
+// another node's client reads from a peer: what tailscale status shows, PeerAPI
+// services, SSH known hosts, exit-node location, app-connector eligibility, and
+// the routes this server feeds into policy. Everything else is stored but never
+// fanned out, and a peer's NetInfo is never read at all.
+func peerHostinfoEqual(a, b tailcfg.HostinfoView) bool {
+	if !a.Valid() || !b.Valid() {
+		return a.Valid() == b.Valid()
 	}
 
-	return &tailcfg.Hostinfo{
-		Hostname:     hi.Hostname,
-		OS:           hi.OS,
-		Services:     hi.Services,
-		SSH_HostKeys: hi.SSH_HostKeys,
-		Location:     hi.Location,
-		AppConnector: hi.AppConnector,
-		RoutableIPs:  hi.RoutableIPs,
-	}
+	return a.Hostname() == b.Hostname() &&
+		a.OS() == b.OS() &&
+		servicesEqual(a.Services(), b.Services()) &&
+		views.SliceEqual(a.SSH_HostKeys(), b.SSH_HostKeys()) &&
+		locationEqual(a.Location(), b.Location()) &&
+		a.AppConnector() == b.AppConnector() &&
+		views.SliceEqual(a.RoutableIPs(), b.RoutableIPs())
 }
 
-// peerHostinfoEqual reports whether the fields peers read are unchanged.
-func peerHostinfoEqual(oldHI, newHI *tailcfg.Hostinfo) bool {
-	return peerHostinfo(oldHI).Equal(peerHostinfo(newHI))
+// servicesEqual compares field by field, as [tailcfg.Service] is
+// incomparable.
+func servicesEqual(a, b views.Slice[tailcfg.Service]) bool {
+	if a.Len() != b.Len() {
+		return false
+	}
+
+	for i := range a.Len() {
+		x, y := a.At(i), b.At(i)
+		if x.Proto != y.Proto || x.Port != y.Port || x.Description != y.Description {
+			return false
+		}
+	}
+
+	return true
+}
+
+// locationEqual compares field by field, as [tailcfg.LocationView] has no
+// Equal.
+func locationEqual(a, b tailcfg.LocationView) bool {
+	if !a.Valid() || !b.Valid() {
+		return a.Valid() == b.Valid()
+	}
+
+	return a.Country() == b.Country() &&
+		a.CountryCode() == b.CountryCode() &&
+		a.City() == b.City() &&
+		a.CityCode() == b.CityCode() &&
+		a.Latitude() == b.Latitude() &&
+		a.Longitude() == b.Longitude() &&
+		a.Priority() == b.Priority()
 }
 
 // netInfoEqualIgnoringDERP compares two NetInfo values via

hscontrol/state/maprequest_test.go

@@ -3,6 +3,7 @@ package state
 import (
 	"fmt"
 	"net/netip"
+	"reflect"
 	"strings"
 	"sync"
 	"sync/atomic"
@@ -860,12 +861,60 @@ func TestPeerHostinfoEqual(t *testing.T) {
 		t.Run(tt.name, func(t *testing.T) {
 			changed := base()
 			tt.mutate(changed)
-			require.Equal(t, tt.want, peerHostinfoEqual(base(), changed))
+			require.Equal(t, tt.want, peerHostinfoEqual(base().View(), changed.View()))
 		})
 	}
 
-	require.True(t, peerHostinfoEqual(nil, nil))
-	require.False(t, peerHostinfoEqual(nil, base()))
+	require.True(t, peerHostinfoEqual(tailcfg.HostinfoView{}, tailcfg.HostinfoView{}))
+	require.False(t, peerHostinfoEqual(tailcfg.HostinfoView{}, base().View()))
+}
+
+// TestPeerHostinfoEqualSeesEveryField fails when tailscale adds a field to
+// [tailcfg.Service] or [tailcfg.Location] that servicesEqual or locationEqual
+// does not compare.
+func TestPeerHostinfoEqualSeesEveryField(t *testing.T) {
+	set := func(t *testing.T, f reflect.Value) {
+		t.Helper()
+
+		switch {
+		case f.Kind() == reflect.String:
+			f.SetString("x")
+		case f.CanInt():
+			f.SetInt(1)
+		case f.CanUint():
+			f.SetUint(1)
+		case f.CanFloat():
+			f.SetFloat(1)
+		default:
+			t.Fatalf("field kind %s not handled; compare it", f.Kind())
+		}
+	}
+
+	for _, field := range reflect.VisibleFields(reflect.TypeFor[tailcfg.Service]()) {
+		if !field.IsExported() {
+			continue
+		}
+
+		t.Run("Service."+field.Name, func(t *testing.T) {
+			var svc tailcfg.Service
+			set(t, reflect.ValueOf(&svc).Elem().FieldByIndex(field.Index))
+
+			a := &tailcfg.Hostinfo{Services: []tailcfg.Service{{}}}
+			b := &tailcfg.Hostinfo{Services: []tailcfg.Service{svc}}
+			assert.False(t, peerHostinfoEqual(a.View(), b.View()))
+		})
+	}
+
+	for _, field := range reflect.VisibleFields(reflect.TypeFor[tailcfg.Location]()) {
+		t.Run("Location."+field.Name, func(t *testing.T) {
+			var loc tailcfg.Location
+			set(t, reflect.ValueOf(&loc).Elem().FieldByIndex(field.Index))
+
+			a := &tailcfg.Hostinfo{Location: &tailcfg.Location{}}
+			b := &tailcfg.Hostinfo{Location: &loc}
+			assert.False(t, peerHostinfoEqual(a.View(), b.View()))
+		})
+	}
 }
 
 func TestNetInfoEqualIgnoringDERP(t *testing.T) {

hscontrol/state/state.go

@@ -3402,7 +3402,7 @@ func (s *State) UpdateNodeFromMapRequest(id types.NodeID, req tailcfg.MapRequest
 		delta.hostinfoChanged = newHostinfo != nil &&
 			!hostinfoEqual(currentNode.Hostinfo, newHostinfo)
 		delta.peerHostinfoChanged = newHostinfo != nil &&
-			!peerHostinfoEqual(currentNode.Hostinfo, newHostinfo)
+			!peerHostinfoEqual(currentNode.Hostinfo.View(), newHostinfo.View())
 		delta.dnsMetadataChanged = newHostinfo != nil &&
 			(currentNode.Hostinfo == nil ||
 				currentNode.Hostinfo.Hostname != newHostinfo.Hostname ||