Group the HostInfo fields the packet paths touch

Makefile

@@ -222,14 +222,11 @@ test-cov-html:
 	go test -coverprofile=coverage.out
 	go tool cover -html=coverage.out
 
-# The package builds only compile. The final line links an android binary so a linker-only failure,
-# such as the //go:linkname reference anet makes, cannot pass CI.
 build-test-mobile:
 	GOARCH=amd64 GOOS=ios go build $(shell go list ./... | grep -v '/cmd/\|/examples/')
 	GOARCH=arm64 GOOS=ios go build $(shell go list ./... | grep -v '/cmd/\|/examples/')
 	GOARCH=amd64 GOOS=android go build $(shell go list ./... | grep -v '/cmd/\|/examples/')
 	GOARCH=arm64 GOOS=android go build $(shell go list ./... | grep -v '/cmd/\|/examples/')
-	GOARCH=arm64 GOOS=android go build -ldflags=-checklinkname=0 -o /dev/null ${NEBULA_CMD_PATH}
 
 bench:
 	go test -bench=.

connection_manager.go

@@ -105,11 +105,12 @@ func (cm *connectionManager) getInactivityTimeout() time.Duration {
 }
 
 func (cm *connectionManager) In(h *HostInfo) {
-	h.in.Store(true)
+	h.markIn()
 }
 
-func (cm *connectionManager) Out(h *HostInfo) {
-	h.out.Store(true)
+// Out records outbound traffic and reports whether the local network changed since this tunnel last sent.
+func (cm *connectionManager) Out(h *HostInfo) bool {
+	return h.markOut(cm.intf.rebindEpoch.Load())
 }
 
 func (cm *connectionManager) RelayUsed(localIndex uint32) {
@@ -128,8 +129,7 @@ func (cm *connectionManager) RelayUsed(localIndex uint32) {
 // getAndResetTrafficCheck returns if there was any inbound or outbound traffic within the last tick and
 // resets the state for this local index
 func (cm *connectionManager) getAndResetTrafficCheck(h *HostInfo, now time.Time) (bool, bool) {
-	in := h.in.Swap(false)
-	out := h.out.Swap(false)
+	in, out := h.takeTraffic()
 	if in || out {
 		h.lastUsed = now
 	}
@@ -340,7 +340,7 @@ func (cm *connectionManager) makeTrafficDecision(localIndex uint32, now time.Tim
 				"tunnelCheck", m{"state": "alive", "method": "passive"},
 			)
 		}
-		hostinfo.pendingDeletion.Store(false)
+		hostinfo.setPendingDeletion(false)
 
 		if mainHostInfo {
 			decision = tryRehandshake
@@ -363,7 +363,7 @@ func (cm *connectionManager) makeTrafficDecision(localIndex uint32, now time.Tim
 		return decision, hostinfo, primary
 	}
 
-	if hostinfo.pendingDeletion.Load() {
+	if hostinfo.isPendingDeletion() {
 		// We have already sent a test packet and nothing was returned, this hostinfo is dead
 		hostinfo.logger(cm.l).Info("Tunnel status",
 			"tunnelCheck", m{"state": "dead", "method": "active"},
@@ -414,7 +414,7 @@ func (cm *connectionManager) makeTrafficDecision(localIndex uint32, now time.Tim
 		}
 	}
 
-	hostinfo.pendingDeletion.Store(true)
+	hostinfo.setPendingDeletion(true)
 	cm.trafficTimer.Add(hostinfo.localIndexId, cm.pendingDeletionInterval)
 	return decision, hostinfo, nil
 }

connection_manager_test.go

@@ -86,25 +86,25 @@ func Test_NewConnectionManagerTest(t *testing.T) {
 	// We saw traffic out to vpnIp
 	nc.Out(hostinfo)
 	nc.In(hostinfo)
-	assert.False(t, hostinfo.pendingDeletion.Load())
+	assert.False(t, hostinfo.isPendingDeletion())
 	assert.Contains(t, nc.hostMap.Hosts, hostinfo.vpnAddrs[0])
 	assert.Contains(t, nc.hostMap.Indexes, hostinfo.localIndexId)
-	assert.True(t, hostinfo.out.Load())
-	assert.True(t, hostinfo.in.Load())
+	assert.True(t, hostinfo.sentSinceCheck())
+	assert.True(t, (hostinfo.state.Load()&stateIn != 0))
 
 	// Do a traffic check tick, should not be pending deletion but should not have any in/out packets recorded
 	nc.doTrafficCheck(hostinfo.localIndexId, p, nb, out, time.Now())
-	assert.False(t, hostinfo.pendingDeletion.Load())
-	assert.False(t, hostinfo.out.Load())
-	assert.False(t, hostinfo.in.Load())
+	assert.False(t, hostinfo.isPendingDeletion())
+	assert.False(t, hostinfo.sentSinceCheck())
+	assert.False(t, (hostinfo.state.Load()&stateIn != 0))
 
 	// Do another traffic check tick, this host should be pending deletion now
 	nc.Out(hostinfo)
-	assert.True(t, hostinfo.out.Load())
+	assert.True(t, hostinfo.sentSinceCheck())
 	nc.doTrafficCheck(hostinfo.localIndexId, p, nb, out, time.Now())
-	assert.True(t, hostinfo.pendingDeletion.Load())
-	assert.False(t, hostinfo.out.Load())
-	assert.False(t, hostinfo.in.Load())
+	assert.True(t, hostinfo.isPendingDeletion())
+	assert.False(t, hostinfo.sentSinceCheck())
+	assert.False(t, (hostinfo.state.Load()&stateIn != 0))
 	assert.Contains(t, nc.hostMap.Indexes, hostinfo.localIndexId)
 	assert.Contains(t, nc.hostMap.Hosts, hostinfo.vpnAddrs[0])
 
@@ -168,33 +168,33 @@ func Test_NewConnectionManagerTest2(t *testing.T) {
 	// We saw traffic out to vpnIp
 	nc.Out(hostinfo)
 	nc.In(hostinfo)
-	assert.True(t, hostinfo.in.Load())
-	assert.True(t, hostinfo.out.Load())
-	assert.False(t, hostinfo.pendingDeletion.Load())
+	assert.True(t, (hostinfo.state.Load()&stateIn != 0))
+	assert.True(t, hostinfo.sentSinceCheck())
+	assert.False(t, hostinfo.isPendingDeletion())
 	assert.Contains(t, nc.hostMap.Hosts, hostinfo.vpnAddrs[0])
 	assert.Contains(t, nc.hostMap.Indexes, hostinfo.localIndexId)
 
 	// Do a traffic check tick, should not be pending deletion but should not have any in/out packets recorded
 	nc.doTrafficCheck(hostinfo.localIndexId, p, nb, out, time.Now())
-	assert.False(t, hostinfo.pendingDeletion.Load())
-	assert.False(t, hostinfo.out.Load())
-	assert.False(t, hostinfo.in.Load())
+	assert.False(t, hostinfo.isPendingDeletion())
+	assert.False(t, hostinfo.sentSinceCheck())
+	assert.False(t, (hostinfo.state.Load()&stateIn != 0))
 
 	// Do another traffic check tick, this host should be pending deletion now
 	nc.Out(hostinfo)
 	nc.doTrafficCheck(hostinfo.localIndexId, p, nb, out, time.Now())
-	assert.True(t, hostinfo.pendingDeletion.Load())
-	assert.False(t, hostinfo.out.Load())
-	assert.False(t, hostinfo.in.Load())
+	assert.True(t, hostinfo.isPendingDeletion())
+	assert.False(t, hostinfo.sentSinceCheck())
+	assert.False(t, (hostinfo.state.Load()&stateIn != 0))
 	assert.Contains(t, nc.hostMap.Indexes, hostinfo.localIndexId)
 	assert.Contains(t, nc.hostMap.Hosts, hostinfo.vpnAddrs[0])
 
 	// We saw traffic, should no longer be pending deletion
 	nc.In(hostinfo)
 	nc.doTrafficCheck(hostinfo.localIndexId, p, nb, out, time.Now())
-	assert.False(t, hostinfo.pendingDeletion.Load())
-	assert.False(t, hostinfo.out.Load())
-	assert.False(t, hostinfo.in.Load())
+	assert.False(t, hostinfo.isPendingDeletion())
+	assert.False(t, hostinfo.sentSinceCheck())
+	assert.False(t, (hostinfo.state.Load()&stateIn != 0))
 	assert.Contains(t, nc.hostMap.Indexes, hostinfo.localIndexId)
 	assert.Contains(t, nc.hostMap.Hosts, hostinfo.vpnAddrs[0])
 }
@@ -253,31 +253,31 @@ func Test_NewConnectionManager_DisconnectInactive(t *testing.T) {
 	// Do a traffic check tick, in and out should be cleared but should not be pending deletion
 	nc.Out(hostinfo)
 	nc.In(hostinfo)
-	assert.True(t, hostinfo.out.Load())
-	assert.True(t, hostinfo.in.Load())
+	assert.True(t, hostinfo.sentSinceCheck())
+	assert.True(t, (hostinfo.state.Load()&stateIn != 0))
 
 	now := time.Now()
 	decision, _, _ := nc.makeTrafficDecision(hostinfo.localIndexId, now)
 	assert.Equal(t, tryRehandshake, decision)
 	assert.Equal(t, now, hostinfo.lastUsed)
-	assert.False(t, hostinfo.pendingDeletion.Load())
-	assert.False(t, hostinfo.out.Load())
-	assert.False(t, hostinfo.in.Load())
+	assert.False(t, hostinfo.isPendingDeletion())
+	assert.False(t, hostinfo.sentSinceCheck())
+	assert.False(t, (hostinfo.state.Load()&stateIn != 0))
 
 	decision, _, _ = nc.makeTrafficDecision(hostinfo.localIndexId, now.Add(time.Second*5))
 	assert.Equal(t, doNothing, decision)
 	assert.Equal(t, now, hostinfo.lastUsed)
-	assert.False(t, hostinfo.pendingDeletion.Load())
-	assert.False(t, hostinfo.out.Load())
-	assert.False(t, hostinfo.in.Load())
+	assert.False(t, hostinfo.isPendingDeletion())
+	assert.False(t, hostinfo.sentSinceCheck())
+	assert.False(t, (hostinfo.state.Load()&stateIn != 0))
 
 	// Do another traffic check tick, should still not be pending deletion
 	decision, _, _ = nc.makeTrafficDecision(hostinfo.localIndexId, now.Add(time.Second*10))
 	assert.Equal(t, doNothing, decision)
 	assert.Equal(t, now, hostinfo.lastUsed)
-	assert.False(t, hostinfo.pendingDeletion.Load())
-	assert.False(t, hostinfo.out.Load())
-	assert.False(t, hostinfo.in.Load())
+	assert.False(t, hostinfo.isPendingDeletion())
+	assert.False(t, hostinfo.sentSinceCheck())
+	assert.False(t, (hostinfo.state.Load()&stateIn != 0))
 	assert.Contains(t, nc.hostMap.Indexes, hostinfo.localIndexId)
 	assert.Contains(t, nc.hostMap.Hosts, hostinfo.vpnAddrs[0])
 
@@ -285,9 +285,9 @@ func Test_NewConnectionManager_DisconnectInactive(t *testing.T) {
 	decision, _, _ = nc.makeTrafficDecision(hostinfo.localIndexId, now.Add(time.Minute*10))
 	assert.Equal(t, closeTunnel, decision)
 	assert.Equal(t, now, hostinfo.lastUsed)
-	assert.False(t, hostinfo.pendingDeletion.Load())
-	assert.False(t, hostinfo.out.Load())
-	assert.False(t, hostinfo.in.Load())
+	assert.False(t, hostinfo.isPendingDeletion())
+	assert.False(t, hostinfo.sentSinceCheck())
+	assert.False(t, (hostinfo.state.Load()&stateIn != 0))
 	assert.Contains(t, nc.hostMap.Indexes, hostinfo.localIndexId)
 	assert.Contains(t, nc.hostMap.Hosts, hostinfo.vpnAddrs[0])
 }

control.go

@@ -212,7 +212,7 @@ func (c *Control) RebindUDPServer() {
 	c.f.lightHouse.SendUpdate()
 
 	// Let the main interface know that we rebound so that underlying tunnels know to trigger punches from their remotes
-	c.f.rebindCount++
+	c.f.rebindEpoch.Add(1)
 }
 
 // ListHostmapHosts returns details about the actual or pending (handshaking) hostmap by vpn ip

go.mod

@@ -22,7 +22,6 @@ require (
 	github.com/stefanberger/go-pkcs11uri v0.0.0-20230803200340-78284954bff6
 	github.com/stretchr/testify v1.11.1
 	github.com/vishvananda/netlink v1.3.1
-	github.com/wlynxg/anet v0.0.5
 	go.uber.org/goleak v1.3.0
 	go.yaml.in/yaml/v3 v3.0.4
 	golang.org/x/crypto v0.54.0

go.sum

@@ -149,8 +149,6 @@ github.com/vishvananda/netlink v1.3.1 h1:3AEMt62VKqz90r0tmNhog0r/PpWKmrEShJU0wJW
 github.com/vishvananda/netlink v1.3.1/go.mod h1:ARtKouGSTGchR8aMwmkzC0qiNPrrWO5JS/XMVl45+b4=
 github.com/vishvananda/netns v0.0.5 h1:DfiHV+j8bA32MFM7bfEunvT8IAqQ/NzSJHtcmW5zdEY=
 github.com/vishvananda/netns v0.0.5/go.mod h1:SpkAiCQRtJ6TvvxPnOSyH3BMl6unz3xZlaprSwhNNJM=
-github.com/wlynxg/anet v0.0.5 h1:J3VJGi1gvo0JwZ/P1/Yc/8p63SoW98B5dHkYDmpgvvU=
-github.com/wlynxg/anet v0.0.5/go.mod h1:eay5PRQr7fIVAMbTbchTnO9gG65Hg/uYGdc7mguHxoA=
 github.com/yuin/goldmark v1.1.27/go.mod h1:3hX8gzYuyVAZsxl0MRgGTJEmQBFcNTphYh9decYSb74=
 github.com/yuin/goldmark v1.2.1/go.mod h1:3hX8gzYuyVAZsxl0MRgGTJEmQBFcNTphYh9decYSb74=
 go.uber.org/goleak v1.3.0 h1:2K3zAYmnTNqV73imy9J1T3WC+gmCePx2hEGkimedGto=

hostmap.go

@@ -238,18 +238,30 @@ const (
 )
 
 type HostInfo struct {
+	// The first cache line is everything the packet paths touch. Grouping them here means a send or receive
+	// pulls in one line instead of two, which is what the layout looked like when state lived at the end.
+
 	remote          atomic.Pointer[netip.AddrPort]
-	remotes         *RemoteList
-	promoteCounter  atomic.Uint32
 	ConnectionState *ConnectionState
+
+	// state holds everything the hot paths need to touch per packet, in one word: whether we have seen traffic
+	// each way since the connection manager last looked, whether it has given up on us, and the
+	// Interface.rebindEpoch this tunnel last sent under. Keeping the epoch here means it survives the traffic
+	// bits being cleared, so a tunnel that has not sent since a rebind still notices when it does.
+	state atomic.Uint32
+
+	promoteCounter atomic.Uint32
 	remoteIndexId  uint32
 	localIndexId   uint32
+	remotes        *RemoteList
 
 	// vpnAddrs is a list of vpn addresses assigned to this host that are within our own vpn networks
 	// The host may have other vpn addresses that are outside our
 	// vpn networks but were removed because they are not usable
 	vpnAddrs []netip.Addr
 
+	// Everything below is off the packet path: handshakes, relays, roaming and the connection manager.
+
 	// networks is a combination of specific vpn addresses (not prefixes!) and full unsafe networks assigned to this host.
 	networks   *bart.Table[NetworkType]
 	relayState RelayState
@@ -262,11 +274,6 @@ type HostInfo struct {
 	// This is used to limit lighthouse re-queries in chatty clients
 	nextLHQuery atomic.Int64
 
-	// lastRebindCount is the other side of Interface.rebindCount, if these values don't match then we need to ask LH
-	// for a punch from the remote end of this tunnel. The goal being to prime their conntrack for our traffic just like
-	// with a handshake
-	lastRebindCount int8
-
 	// lastHandshakeTime records the time the remote side told us about at the stage when the handshake was completed locally
 	// Stage 1 packet will contain it if I am a responder, stage 2 packet if I am an initiator
 	// This is used to avoid an attack where a handshake packet is replayed after some time
@@ -275,9 +282,6 @@ type HostInfo struct {
 	lastRoam       time.Time
 	lastRoamRemote netip.AddrPort
 
-	//TODO: in, out, and others might benefit from being an atomic.Int32. We could collapse connectionManager pendingDeletion, relayUsed, and in/out into this 1 thing
-	in, out, pendingDeletion atomic.Bool
-
 	// lastUsed tracks the last time ConnectionManager checked the tunnel and it was in use.
 	// This value will be behind against actual tunnel utilization in the hot path.
 	// This should only be used by the ConnectionManagers ticker routine.
@@ -658,7 +662,7 @@ func (hm *HostMap) unlockedAddHostInfo(hostinfo *HostInfo, f *Interface) {
 	hm.Indexes[hostinfo.localIndexId] = hostinfo
 	hm.RemoteIndexes[hostinfo.remoteIndexId] = hostinfo
 
-	hostinfo.out.Store(true)
+	hostinfo.markOut(f.rebindEpoch.Load())
 	if f.connectionManager != nil { // f.connectionManager is only nil in some unit tests
 		f.connectionManager.trafficTimer.Add(hostinfo.localIndexId, f.connectionManager.checkInterval)
 	}
@@ -759,6 +763,64 @@ func (i *HostInfo) TryPromoteBest(preferredRanges []netip.Prefix, ifce *Interfac
 	}
 }
 
+// Bits within HostInfo.state. Everything above stateEpochShift is the rebind epoch.
+const (
+	stateIn uint32 = 1 << iota
+	stateOut
+	statePendingDeletion
+
+	stateFlags      = stateIn | stateOut | statePendingDeletion
+	stateEpochShift = 3
+)
+
+// markIn records inbound traffic. Reading first keeps the cache line shared on the common path, where the bit
+// is already set.
+func (i *HostInfo) markIn() {
+	if i.state.Load()&stateIn == 0 {
+		i.state.Or(stateIn)
+	}
+}
+
+// markOut records that we sent on this tunnel under the given rebind epoch. It reports whether the epoch moved
+// since our last send, which means the local network changed and we want the far side to punch at us again.
+// The common path is a single load that matches and returns.
+func (i *HostInfo) markOut(epoch uint32) bool {
+	e := epoch << stateEpochShift
+	for {
+		old := i.state.Load()
+		if old&stateOut != 0 && old&^stateFlags == e {
+			return false
+		}
+
+		if i.state.CompareAndSwap(old, old&stateFlags|stateOut|e) {
+			return old&^stateFlags != e
+		}
+	}
+}
+
+// sentSinceCheck reports whether anything has been sent since the connection manager last looked.
+func (i *HostInfo) sentSinceCheck() bool {
+	return i.state.Load()&stateOut != 0
+}
+
+// takeTraffic clears both traffic bits, leaving the epoch alone, and reports what they were.
+func (i *HostInfo) takeTraffic() (in bool, out bool) {
+	old := i.state.And(^(stateIn | stateOut))
+	return old&stateIn != 0, old&stateOut != 0
+}
+
+func (i *HostInfo) setPendingDeletion(v bool) {
+	if v {
+		i.state.Or(statePendingDeletion)
+	} else {
+		i.state.And(^statePendingDeletion)
+	}
+}
+
+func (i *HostInfo) isPendingDeletion() bool {
+	return i.state.Load()&statePendingDeletion != 0
+}
+
 func (i *HostInfo) GetCert() *cert.CachedCertificate {
 	if i.ConnectionState != nil {
 		return i.ConnectionState.peerCert
@@ -868,28 +930,10 @@ func (i *HostInfo) logger(l *slog.Logger) *slog.Logger {
 
 // Utility functions
 
-func localAddrs(l *slog.Logger, allowList *LocalAllowList) ([]netip.Addr, error) {
-	return collectLocalAddrs(l, allowList, localInterfaces, localInterfaceAddrs)
-}
-
-// collectLocalAddrs takes its enumerators as arguments so tests can drive the filtering and the
-// failure branches without depending on the addresses of whatever host they run on. It reports
-// failures to the caller rather than logging them, because it runs on every lighthouse update and
-// only the caller can tell a new failure from a repeat of the same one.
-func collectLocalAddrs(
-	l *slog.Logger,
-	allowList *LocalAllowList,
-	interfaces func() ([]net.Interface, error),
-	interfaceAddrs func(*net.Interface) ([]net.Addr, error),
-) ([]netip.Addr, error) {
+func localAddrs(l *slog.Logger, allowList *LocalAllowList) []netip.Addr {
 	//FIXME: This function is pretty garbage
 	var finalAddrs []netip.Addr
-	var errs []error
-	ifaces, err := interfaces()
-	if err != nil {
-		return nil, fmt.Errorf("failed to enumerate local interfaces: %w", err)
-	}
-
+	ifaces, _ := net.Interfaces()
 	for _, i := range ifaces {
 		allow := allowList.AllowName(i.Name)
 		if l.Enabled(context.Background(), logging.LevelTrace) {
@@ -902,12 +946,7 @@ func collectLocalAddrs(
 		if !allow {
 			continue
 		}
-		addrs, err := interfaceAddrs(&i)
-		if err != nil {
-			errs = append(errs, fmt.Errorf("failed to get addresses for %s: %w", i.Name, err))
-			continue
-		}
-
+		addrs, _ := i.Addrs()
 		for _, rawAddr := range addrs {
 			var addr netip.Addr
 			switch v := rawAddr.(type) {
@@ -942,5 +981,5 @@ func collectLocalAddrs(
 			}
 		}
 	}
-	return finalAddrs, errors.Join(errs...)
+	return finalAddrs
 }

hostmap_test.go

@@ -1,8 +1,6 @@
 package nebula
 
 import (
-	"errors"
-	"net"
 	"net/netip"
 	"slices"
 	"testing"
@@ -403,83 +401,3 @@ func TestHostMap_RelayState(t *testing.T) {
 	assert.Equal(t, []netip.Addr{}, h1.relayState.relays)
 
 }
-
-func TestCollectLocalAddrs(t *testing.T) {
-	ifaces := []net.Interface{
-		{Index: 1, Name: "lo"},
-		{Index: 2, Name: "eth0"},
-		{Index: 3, Name: "docker0"},
-	}
-	addrs := map[string][]net.Addr{
-		"lo": {
-			&net.IPNet{IP: net.ParseIP("127.0.0.1"), Mask: net.CIDRMask(8, 32)},
-			&net.IPNet{IP: net.ParseIP("::1"), Mask: net.CIDRMask(128, 128)},
-		},
-		"eth0": {
-			&net.IPNet{IP: net.ParseIP("10.0.0.5"), Mask: net.CIDRMask(24, 32)},
-			&net.IPNet{IP: net.ParseIP("fe80::1"), Mask: net.CIDRMask(64, 128)},
-			&net.IPAddr{IP: net.ParseIP("fd00::5")},
-		},
-		"docker0": {
-			&net.IPNet{IP: net.ParseIP("172.17.0.1"), Mask: net.CIDRMask(16, 32)},
-		},
-	}
-
-	enumerate := func() ([]net.Interface, error) { return ifaces, nil }
-	addrsFor := func(i *net.Interface) ([]net.Addr, error) { return addrs[i.Name], nil }
-
-	// Loopback and link local are dropped, everything else on every interface is kept.
-	out, err := collectLocalAddrs(test.NewLogger(), nil, enumerate, addrsFor)
-	require.NoError(t, err)
-	assert.Equal(t, []netip.Addr{
-		netip.MustParseAddr("10.0.0.5"),
-		netip.MustParseAddr("fd00::5"),
-		netip.MustParseAddr("172.17.0.1"),
-	}, out)
-
-	// An interface the allow list rejects by name is never asked for its addresses.
-	c := config.NewC(test.NewLogger())
-	c.Settings["allowlist"] = map[string]any{
-		"interfaces": map[string]any{`docker.*`: false},
-	}
-	al, err := NewLocalAllowListFromConfig(c, "allowlist")
-	require.NoError(t, err)
-
-	asked := make(map[string]struct{})
-	countingAddrsFor := func(i *net.Interface) ([]net.Addr, error) {
-		asked[i.Name] = struct{}{}
-		return addrs[i.Name], nil
-	}
-	out, err = collectLocalAddrs(test.NewLogger(), al, enumerate, countingAddrsFor)
-	require.NoError(t, err)
-	assert.Equal(t, []netip.Addr{
-		netip.MustParseAddr("10.0.0.5"),
-		netip.MustParseAddr("fd00::5"),
-	}, out)
-	assert.NotContains(t, asked, "docker0")
-
-	// A failure to enumerate interfaces at all is reported rather than silently advertising nothing.
-	out, err = collectLocalAddrs(
-		test.NewLogger(),
-		nil,
-		func() ([]net.Interface, error) { return nil, errors.New("netlinkrib: permission denied") },
-		addrsFor,
-	)
-	assert.Nil(t, out)
-	require.EqualError(t, err, "failed to enumerate local interfaces: netlinkrib: permission denied")
-
-	// One interface failing is reported and skipped, the rest are still collected.
-	out, err = collectLocalAddrs(
-		test.NewLogger(),
-		nil,
-		enumerate,
-		func(i *net.Interface) ([]net.Addr, error) {
-			if i.Name == "eth0" {
-				return nil, errors.New("nope")
-			}
-			return addrs[i.Name], nil
-		},
-	)
-	assert.Equal(t, []netip.Addr{netip.MustParseAddr("172.17.0.1")}, out)
-	require.EqualError(t, err, "failed to get addresses for eth0: nope")
-}

inside.go

@@ -365,17 +365,12 @@ func (f *Interface) sendNoMetrics(t header.MessageType, st header.MessageSubType
 
 	//l.WithField("trace", string(debug.Stack())).Error("out Header ", &Header{Version, t, st, 0, hostinfo.remoteIndexId, c}, p)
 	out = header.Encode(out, header.Version, t, st, hostinfo.remoteIndexId, c)
-	f.connectionManager.Out(hostinfo)
-
-	// Query our LH if we haven't since the last time we've been rebound, this will cause the remote to punch against
-	// all our addrs and enable a faster roaming.
-	if t != header.CloseTunnel && hostinfo.lastRebindCount != f.rebindCount {
-		//NOTE: there is an update hole if a tunnel isn't used and exactly 256 rebinds occur before the tunnel is
-		// finally used again. This tunnel would eventually be torn down and recreated if this action didn't help.
+	// We rebound since this tunnel last sent, so the local network moved. Ask the lighthouse to have the far side
+	// punch at where we are now, which primes their conntrack the same way a handshake would.
+	if f.connectionManager.Out(hostinfo) && t != header.CloseTunnel {
 		f.lightHouse.QueryServer(hostinfo.vpnAddrs[0])
-		hostinfo.lastRebindCount = f.rebindCount
 		if f.l.Enabled(context.Background(), slog.LevelDebug) {
-			f.l.Debug("Lighthouse update triggered for punch due to rebind counter",
+			f.l.Debug("Lighthouse update triggered for punch due to rebind epoch",
 				"vpnAddrs", hostinfo.vpnAddrs,
 			)
 		}

interface.go

@@ -82,8 +82,10 @@ type Interface struct {
 	sendRecvErrorConfig   recvErrorConfig
 	acceptRecvErrorConfig recvErrorConfig
 
-	// rebindCount is used to decide if an active tunnel should trigger a punch notification through a lighthouse
-	rebindCount int8
+	// rebindEpoch bumps every time the udp listener is rebound, which means the local network moved. Tunnels
+	// compare it against their own copy to decide they need a punch from the far side. Read on every send, only
+	// written on a rebind, so the cache line stays shared across the routines.
+	rebindEpoch atomic.Uint32
 	version     string
 
 	conntrackCacheTimeout time.Duration

lighthouse.go

@@ -40,10 +40,6 @@ type LightHouse struct {
 	// addresses rather than whatever this machine's NICs happen to be. Set it before Start.
 	localAddrsFn func(*LocalAllowList) []netip.Addr
 
-	// lastLocalAddrsErr is the previous localAddrsFn failure. Enumeration runs on every update, so an
-	// unchanged failure is demoted to Debug rather than warning every lighthouse.interval forever.
-	lastLocalAddrsErr atomic.Pointer[string]
-
 	// Local cache of answers from light houses
 	// map of vpn addr to answers
 	addrMap map[netip.Addr]*RemoteList
@@ -116,9 +112,7 @@ func NewLightHouseFromConfig(ctx context.Context, l *slog.Logger, c *config.C, c
 		l:                  l,
 	}
 	h.localAddrsFn = func(al *LocalAllowList) []netip.Addr {
-		addrs, err := localAddrs(h.l, al)
-		h.logLocalAddrsErr(err)
-		return addrs
+		return localAddrs(h.l, al)
 	}
 
 	lighthouses := make([]netip.Addr, 0)
@@ -919,26 +913,6 @@ func (lh *LightHouse) TriggerUpdate() {
 	}
 }
 
-// logLocalAddrsErr reports a localAddrs failure at Warn the first time it is seen and at Debug while
-// it persists unchanged, so a permanent failure does not warn on every update forever.
-func (lh *LightHouse) logLocalAddrsErr(err error) {
-	if err == nil {
-		lh.lastLocalAddrsErr.Store(nil)
-		return
-	}
-
-	msg := err.Error()
-	prev := lh.lastLocalAddrsErr.Swap(&msg)
-	if prev != nil && *prev == msg {
-		if lh.l.Enabled(context.Background(), slog.LevelDebug) {
-			lh.l.Debug("Failed to collect local addresses to advertise", "error", err)
-		}
-		return
-	}
-
-	lh.l.Warn("Failed to collect local addresses to advertise", "error", err)
-}
-
 func (lh *LightHouse) SendUpdate() {
 	var v4 []*V4AddrPort
 	var v6 []*V6AddrPort

lighthouse_test.go

@@ -1,9 +1,7 @@
 package nebula
 
 import (
-	"bytes"
 	"encoding/binary"
-	"errors"
 	"fmt"
 	"net/netip"
 	"testing"
@@ -740,32 +738,3 @@ func TestLighthouse_DeletesWork(t *testing.T) {
 	out = lh.Query(testHost)
 	assert.Nil(t, out)
 }
-
-func TestLightHouse_logLocalAddrsErr(t *testing.T) {
-	out := &bytes.Buffer{}
-	lh := &LightHouse{l: test.NewLoggerWithOutput(out)}
-
-	// The first sighting of a failure warns.
-	lh.logLocalAddrsErr(errors.New("permission denied"))
-	assert.Contains(t, out.String(), "level=WARN")
-	assert.Contains(t, out.String(), "permission denied")
-
-	// Repeating unchanged does not warn again, which is what keeps a permanent failure from warning
-	// on every lighthouse.interval for the life of the process.
-	out.Reset()
-	lh.logLocalAddrsErr(errors.New("permission denied"))
-	assert.NotContains(t, out.String(), "level=WARN")
-
-	// A different failure is a new event and warns.
-	out.Reset()
-	lh.logLocalAddrsErr(errors.New("something else"))
-	assert.Contains(t, out.String(), "level=WARN")
-	assert.Contains(t, out.String(), "something else")
-
-	// Recovering resets, so the same failure returning later warns again.
-	out.Reset()
-	lh.logLocalAddrsErr(nil)
-	assert.Empty(t, out.String())
-	lh.logLocalAddrsErr(errors.New("something else"))
-	assert.Contains(t, out.String(), "level=WARN")
-}

localaddrs.go

@@ -1,13 +0,0 @@
-//go:build !android
-
-package nebula
-
-import "net"
-
-func localInterfaces() ([]net.Interface, error) {
-	return net.Interfaces()
-}
-
-func localInterfaceAddrs(i *net.Interface) ([]net.Addr, error) {
-	return i.Addrs()
-}

localaddrs_android.go

@@ -1,32 +0,0 @@
-//go:build android
-
-package nebula
-
-import (
-	"net"
-
-	"github.com/wlynxg/anet"
-)
-
-// anet relies on //go:linkname and so needs -ldflags=-checklinkname=0 on Go 1.23+. Nebula ships no
-// Android binaries of its own, so that burden falls on consumers linking Android artifacts.
-
-func init() {
-	// anet only takes its bind-free path when it believes it is on API 30+, and detecting the running
-	// device's level requires cgo. Pin it so a CGO_ENABLED=0 build cannot quietly fall back to the
-	// denied path. The bind-free path is correct on older releases too, just unnecessary there.
-	anet.SetAndroidVersion(11)
-}
-
-// The app sandbox denies bind() on netlink_route_socket, so the stdlib's RTM_GETLINK enumeration
-// fails with EACCES and we advertise no underlay addresses at all. anet reads RTM_GETADDR from an
-// unbound socket instead, so this must not be collapsed back into net.Interfaces.
-func localInterfaces() ([]net.Interface, error) {
-	return anet.Interfaces()
-}
-
-// net.Interface.Addrs goes back through the denied netlink path, so addresses have to come from anet
-// as well. anet cannot report HardwareAddr, which localAddrs does not read.
-func localInterfaceAddrs(i *net.Interface) ([]net.Addr, error) {
-	return anet.InterfaceAddrsByInterface(i)
-}