overlay/tun_darwin.go

@@ -48,7 +48,12 @@ type tun struct {
 	routeSweepEvery time.Duration
 	closed          bool
 
-	// lastRouteEvent is when the routing socket last reported something that may have freed a prefix, in unix nanos
+	// routeMovers is who last changed each unsafe_route prefix since the last reconcile, for its logs. It has its own
+	// lock so the route watcher never waits on a reconcile
+	routeMoversMu sync.Mutex
+	routeMovers   map[netip.Prefix]routeMover
+
+	// lastRouteEvent is when the routing socket last reported something that may have moved a prefix, in unix nanos
 	lastRouteEvent atomic.Int64
 }
 

overlay/tun_darwin_route_test.go

@@ -10,6 +10,7 @@ import (
 	"net/netip"
 	"os"
 	"os/exec"
+	"path/filepath"
 	"strings"
 	"testing"
 	"time"
@@ -117,7 +118,7 @@ const (
 	interfaceChanged = "7000050e000000004288000023000000060000060e000000dc05000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000073e5ce6a0000000000000000000001000000000000000000"
 )
 
-func TestRoutesMayBeFreed(t *testing.T) {
+func TestRoutesMayHaveMoved(t *testing.T) {
 	cidr := netip.MustParsePrefix("192.168.1.0/24")
 	via := routing.Gateways{routing.NewGateway(netip.MustParseAddr("10.250.0.2"), 1)}
 	routes := []Route{{Cidr: cidr, Via: via, Install: true}}
@@ -144,27 +145,73 @@ func TestRoutesMayBeFreed(t *testing.T) {
 		return b
 	}
 	lan, mask := [4]byte{192, 168, 1, 0}, [4]byte{255, 255, 255, 0}
+	routesMayHaveMoved := func(b []byte, routes []Route) bool {
+		moved, _, _ := routesMayHaveMoved(b, routes)
+		return moved
+	}
 
-	assert.True(t, routesMayBeFreed(request(unix.RTM_DELETE, unix.RTF_UP, lan, mask), routes))
-	assert.False(t, routesMayBeFreed(request(unix.RTM_ADD, unix.RTF_UP, lan, mask), routes))
-	assert.False(t, routesMayBeFreed(request(unix.RTM_DELETE, unix.RTF_UP|unix.RTF_IFSCOPE, lan, mask), routes), "a scoped route never held the prefix")
-	assert.False(t, routesMayBeFreed(request(unix.RTM_DELETE, unix.RTF_UP, lan, [4]byte{255, 255, 0, 0}), routes), "only an exact prefix counts")
-	assert.False(t, routesMayBeFreed(request(unix.RTM_DELETE, unix.RTF_UP, lan, mask), []Route{{Cidr: cidr, Install: true}}), "a route with no via is never installed")
-	assert.False(t, routesMayBeFreed(request(unix.RTM_DELETE, unix.RTF_UP, lan, mask), []Route{{Cidr: cidr, Via: via}}), "install: false")
+	assert.True(t, routesMayHaveMoved(request(unix.RTM_DELETE, unix.RTF_UP, lan, mask), routes))
+	assert.True(t, routesMayHaveMoved(request(unix.RTM_ADD, unix.RTF_UP, lan, mask), routes))
+	assert.True(t, routesMayHaveMoved(request(unix.RTM_CHANGE, unix.RTF_UP, lan, mask), routes))
+	assert.False(t, routesMayHaveMoved(request(unix.RTM_GET, unix.RTF_UP, lan, mask), routes))
+	assert.False(t, routesMayHaveMoved(request(unix.RTM_DELETE, unix.RTF_UP|unix.RTF_IFSCOPE, lan, mask), routes), "a scoped route never held the prefix")
+	assert.False(t, routesMayHaveMoved(request(unix.RTM_ADD, unix.RTF_UP|unix.RTF_IFSCOPE, lan, mask), routes), "configd adds a scoped copy for each interface")
+	assert.False(t, routesMayHaveMoved(request(unix.RTM_DELETE, unix.RTF_UP, lan, [4]byte{255, 255, 0, 0}), routes), "only an exact prefix counts")
+	assert.False(t, routesMayHaveMoved(request(unix.RTM_DELETE, unix.RTF_UP, lan, mask), []Route{{Cidr: cidr, Install: true}}), "a route with no via is never installed")
+	assert.False(t, routesMayHaveMoved(request(unix.RTM_DELETE, unix.RTF_UP, lan, mask), []Route{{Cidr: cidr, Via: via}}), "install: false")
 
 	failed := request(unix.RTM_DELETE, unix.RTF_UP, lan, mask)
 	binary.NativeEndian.PutUint32(failed[24:28], uint32(unix.ESRCH))
-	assert.False(t, routesMayBeFreed(failed, routes), "a failed request is echoed too")
+	assert.False(t, routesMayHaveMoved(failed, routes), "a failed request is echoed too")
 
 	_, err := netroute.ParseRIB(unix.NET_RT_DUMP, raw(kernelSubnetDelete))
 	require.Error(t, err, "if x/net/route learns to parse these, match their prefix instead")
-	assert.True(t, routesMayBeFreed(raw(kernelSubnetDelete), routes), "the kernel's own deletes could be any prefix")
+	assert.True(t, routesMayHaveMoved(raw(kernelSubnetDelete), routes), "the kernel's own deletes could be any prefix")
 	neighbor := raw(kernelSubnetDelete)
 	binary.NativeEndian.PutUint32(neighbor[8:12], unix.RTF_UP|unix.RTF_HOST|unix.RTF_LLINFO)
-	assert.False(t, routesMayBeFreed(neighbor, routes), "a neighbor expiring")
+	assert.False(t, routesMayHaveMoved(neighbor, routes), "a neighbor expiring")
+
+	assert.True(t, routesMayHaveMoved(raw(addressRemoved), routes))
+	assert.True(t, routesMayHaveMoved(raw(interfaceChanged), routes), "detaching an interface deletes its routes without a message for each")
+}
+
+// A request names its prefix and who made it, the kernel's own and interface changes name neither
+func TestRoutesMayHaveMovedSaysWho(t *testing.T) {
+	cidr := netip.MustParsePrefix("192.168.1.0/24")
+	routes := []Route{{Cidr: cidr, Via: routing.Gateways{routing.NewGateway(netip.MustParseAddr("10.250.0.2"), 1)}, Install: true}}
+	m := &netroute.RouteMessage{
+		Version: unix.RTM_VERSION,
+		Type:    unix.RTM_CHANGE,
+		Flags:   unix.RTF_UP,
+		ID:      1234,
+		Addrs: []netroute.Addr{
+			unix.RTAX_DST:     &netroute.Inet4Addr{IP: [4]byte{192, 168, 1, 0}},
+			unix.RTAX_GATEWAY: &netroute.LinkAddr{Index: 4},
+			unix.RTAX_NETMASK: &netroute.Inet4Addr{IP: [4]byte{255, 255, 255, 0}},
+		},
+	}
+	b, err := m.Marshal()
+	require.NoError(t, err)
+	moved, p, pid := routesMayHaveMoved(b, routes)
+	assert.True(t, moved)
+	assert.Equal(t, cidr, p)
+	assert.Equal(t, int32(1234), pid)
 
-	assert.True(t, routesMayBeFreed(raw(addressRemoved), routes))
-	assert.True(t, routesMayBeFreed(raw(interfaceChanged), routes), "detaching an interface deletes its routes without a message for each")
+	for name, s := range map[string]string{"kernel": kernelSubnetDelete, "ifinfo": interfaceChanged} {
+		b, err := hex.DecodeString(s)
+		require.NoError(t, err)
+		moved, p, pid := routesMayHaveMoved(b, routes)
+		assert.True(t, moved, name)
+		assert.False(t, p.IsValid(), name)
+		assert.Zero(t, pid, name)
+	}
+}
+
+func TestProcessName(t *testing.T) {
+	assert.Equal(t, "launchd", processName(1))
+	self := filepath.Base(os.Args[0])
+	assert.Equal(t, self[:min(len(self), 16)], processName(int32(os.Getpid())))
+	assert.Empty(t, processName(-1))
 }
 
 // newRouteTestTun is an activated tun for the root tests below with an unsafe_route for each of cidrs. sweep replaces
@@ -223,8 +270,15 @@ func routeArgs(action, cidr string, extra ...string) []string {
 }
 
 func routeCmd(t *testing.T, action, cidr string, extra ...string) {
-	out, err := exec.Command("route", routeArgs(action, cidr, extra...)...).CombinedOutput()
+	requireRouteOK(t, exec.Command("route", routeArgs(action, cidr, extra...)...))
+}
+
+// requireRouteOK runs cmd, which runs route(8). route exits 0 when the kernel rejects its request, only what it prints
+// says so
+func requireRouteOK(t *testing.T, cmd *exec.Cmd) {
+	out, err := cmd.CombinedOutput()
 	require.NoError(t, err, string(out))
+	require.NotContains(t, string(out), "writing to routing socket")
 }
 
 // cleanupRoute deletes cidr's route when the test ends, whoever holds it
@@ -276,8 +330,7 @@ func TestTunYieldsToARouteThatReplacesIt(t *testing.T) {
 	require.Equal(t, ours, routeVia(t, cidr))
 
 	cleanupRoute(t, cidr)
-	out, err := exec.Command("sh", "-c", "route -q -n delete -net "+cidr+" && route -q -n add -net "+cidr+" -interface lo0").CombinedOutput()
-	require.NoError(t, err, string(out))
+	requireRouteOK(t, exec.Command("sh", "-c", "route -q -n delete -net "+cidr+" && route -q -n add -net "+cidr+" -interface lo0"))
 	lo0, err := net.InterfaceByName("lo0")
 	require.NoError(t, err)
 	time.Sleep(2 * routeRetryDelay)
@@ -312,8 +365,8 @@ func TestTunYieldsAfterLeavingARouteDeletedByHand(t *testing.T) {
 		sweep   time.Duration
 		misfile func(t *testing.T, tn *tun, cidr string)
 	}{
-		// The table went quiet with nothing in its place
-		"settled": {"10.251.6.0/24", 2 * routeRetryDelay, func(t *testing.T, tn *tun, cidr string) {
+		// The table went quiet with nothing in its place. The minute sweep leaves noticing the lan's route to the watcher
+		"settled": {"10.251.6.0/24", 0, func(t *testing.T, tn *tun, cidr string) {
 			require.Eventually(t, func() bool { return routeStateOf(tn, cidr).kind == routeRemoved }, 5*time.Second, 50*time.Millisecond)
 		}},
 		// The sweep that caps a table that never settles
@@ -342,6 +395,31 @@ func TestTunYieldsAfterLeavingARouteDeletedByHand(t *testing.T) {
 	}
 }
 
+// Changing our route to another interface hands it the prefix, ours goes back once theirs is removed rather than being
+// taken for a hand delete
+func TestTunYieldsToARouteChangedAway(t *testing.T) {
+	for _, cidr := range []string{"10.251.8.0/24", "2001:db8:258::/64"} {
+		t.Run(cidr, func(t *testing.T) {
+			tn, _, _ := newRouteTestTun(t, 0, cidr)
+			ours := tn.linkAddr.Load().Index
+			require.Equal(t, ours, routeVia(t, cidr))
+
+			cleanupRoute(t, cidr)
+			routeCmd(t, "change", cidr, "-interface", "lo0")
+			lo0, err := net.InterfaceByName("lo0")
+			require.NoError(t, err)
+			require.Equal(t, lo0.Index, routeVia(t, cidr))
+			require.Eventually(t, func() bool {
+				s := routeStateOf(tn, cidr)
+				return s.kind == routeYielded && s.holder == lo0.Index
+			}, 5*time.Second, 50*time.Millisecond)
+
+			routeCmd(t, "delete", cidr)
+			assert.Eventually(t, func() bool { return routeVia(t, cidr) == ours }, 5*time.Second, 50*time.Millisecond)
+		})
+	}
+}
+
 // Detaching an interface deletes its routes without a routing socket message for each, like unplugging the dock whose
 // ethernet configd routed the lan through
 func TestTunAddsRouteWhenItsInterfaceDetaches(t *testing.T) {
@@ -357,9 +435,10 @@ func TestTunAddsRouteWhenItsInterfaceDetaches(t *testing.T) {
 	t.Cleanup(func() { _ = exec.Command("ifconfig", feth, "destroy").Run() })
 	require.NoError(t, exec.Command("ifconfig", feth, "inet", "198.51.100.5/24", "up").Run())
 
-	// Static, like the lan routes configd installs
+	// Static and cloning with no gateway, like the lan routes configd installs. Named by the feth's address, macOS 27
+	// refuses one named by the feth itself
 	cidr := "10.251.2.0/24"
-	routeCmd(t, "add", cidr, "-interface", feth)
+	routeCmd(t, "add", cidr, "-interface", "198.51.100.5")
 	cleanupRoute(t, cidr)
 	ifc, err := net.InterfaceByName(feth)
 	require.NoError(t, err)

overlay/tun_routes_darwin.go

@@ -59,6 +59,34 @@ type routeState struct {
 	holder int
 }
 
+// routeMover is the process that last added, changed or deleted a route for an unsafe_route's prefix, for logs. Its name
+// is looked up as the message arrives, route(8) has exited by the time the table settles
+type routeMover struct {
+	pid  int32
+	name string
+}
+
+// logArgs appends m to a log line's args, when we know who it was
+func (m routeMover) logArgs(args ...any) []any {
+	if m.pid == 0 {
+		return args
+	}
+	args = append(args, "pid", m.pid)
+	if m.name != "" {
+		args = append(args, "process", m.name)
+	}
+	return args
+}
+
+// processName is the name of process pid, truncated to 16 bytes, or empty when it has already exited
+func processName(pid int32) string {
+	kp, err := unix.SysctlKinfoProc("kern.proc.pid", int(pid))
+	if err != nil || kp.Proc.P_pid != pid {
+		return ""
+	}
+	return unix.ByteSliceToString(kp.Proc.P_comm[:])
+}
+
 // installRoutes returns the routes we put in the system routing table, one per prefix
 func installRoutes(routes []Route) []Route {
 	seen := make(map[netip.Prefix]struct{}, len(routes))
@@ -206,6 +234,11 @@ func (t *tun) reconcileRoutes() {
 		return
 	}
 
+	t.routeMoversMu.Lock()
+	movers := t.routeMovers
+	t.routeMovers = nil
+	t.routeMoversMu.Unlock()
+
 	msgs, err := fetchRoutes()
 	if err != nil {
 		t.l.Error("Failed to read the system routing table", "error", err)
@@ -216,6 +249,7 @@ func (t *tun) reconcileRoutes() {
 	for _, r := range installRoutes(*t.Routes.Load()) {
 		p := r.Cidr.Masked()
 		prev := t.routeStates[p]
+		by := movers[p]
 
 		holder := routeInterfaceIn(msgs, p)
 		switch {
@@ -225,12 +259,12 @@ func (t *tun) reconcileRoutes() {
 		case holder != 0:
 			// Another interface taking a prefix we left alone puts ours back in line behind theirs
 			if prev.kind != routeYielded || prev.holder != holder {
-				t.l.Info("unsafe_route is routed through another interface, it will be added back when that route is removed", "route", r.Cidr, "interface", interfaceName(holder))
+				t.l.Info("unsafe_route is routed through another interface, it will be added back when that route is removed", by.logArgs("route", r.Cidr, "interface", interfaceName(holder))...)
 			}
 			states[p] = routeState{kind: routeYielded, holder: holder}
 
 		case prev.kind == routeInstalled:
-			t.l.Warn("unsafe_route was removed outside nebula, leaving it until a reload or another interface takes its destination", "route", r.Cidr)
+			t.l.Warn("unsafe_route was removed outside nebula, leaving it until a reload or another interface takes its destination", by.logArgs("route", r.Cidr)...)
 			states[p] = routeState{kind: routeRemoved}
 
 		case prev.kind == routeRemoved:
@@ -241,7 +275,7 @@ func (t *tun) reconcileRoutes() {
 			switch {
 			case err == nil:
 				if prev.kind == routeYielded {
-					t.l.Info("unsafe_route's destination is free again, added it", "route", r)
+					t.l.Info("unsafe_route's destination is free again, added it", by.logArgs("route", r)...)
 				} else {
 					t.l.Info("Added route", "route", r)
 				}
@@ -261,7 +295,7 @@ func (t *tun) reconcileRoutes() {
 }
 
 // startRouteWatch starts reconciling the unsafe_routes, every routeSweepEvery and, when watch isn't nil, once the
-// routing table settles after anything that may have freed a prefix
+// routing table settles after anything that may have moved a prefix
 func (t *tun) startRouteWatch(watch *os.File) {
 	t.routesMu.Lock()
 	defer t.routesMu.Unlock()
@@ -308,7 +342,7 @@ func openRouteSocket() (*os.File, error) {
 }
 
 // watchRoutes reconciles the unsafe_routes once the routing table has been quiet for routeRetryDelay after a message
-// that may have freed a prefix. A table that keeps changing is reconciled every routeSweepEvery regardless.
+// that may have moved a prefix. A table that keeps changing is reconciled every routeSweepEvery regardless.
 func (t *tun) watchRoutes(f *os.File) {
 	var reconciled atomic.Int64
 	settle := time.AfterFunc(time.Hour, func() {
@@ -319,6 +353,7 @@ func (t *tun) watchRoutes(f *os.File) {
 	defer settle.Stop()
 
 	var burst time.Time
+	self := int32(os.Getpid())
 	b := make([]byte, os.Getpagesize())
 	for {
 		n, err := f.Read(b)
@@ -329,9 +364,19 @@ func (t *tun) watchRoutes(f *os.File) {
 			return
 		}
 
-		if !routesMayBeFreed(b[:n], *t.Routes.Load()) {
+		moved, p, pid := routesMayHaveMoved(b[:n], *t.Routes.Load())
+		if !moved {
 			continue
 		}
+		// Our own are already in routeStates
+		if p.IsValid() && pid != self {
+			t.routeMoversMu.Lock()
+			if t.routeMovers == nil {
+				t.routeMovers = make(map[netip.Prefix]routeMover)
+			}
+			t.routeMovers[p] = routeMover{pid: pid, name: processName(pid)}
+			t.routeMoversMu.Unlock()
+		}
 
 		now := time.Now()
 		t.lastRouteEvent.Store(now.UnixNano())
@@ -342,56 +387,59 @@ func (t *tun) watchRoutes(f *os.File) {
 	}
 }
 
-// routesMayBeFreed reports whether b, one routing socket message, may have freed a prefix one of routes installs. That's
-// a route for it removed, or an interface that lost an address or changed state, since detaching an interface deletes
-// its routes without a message of their own. The header is read directly, x/net/route skips the RTM_IFINFO the socket
-// sends, which has no interface address, and misreads the unpadded sockaddrs in messages the kernel makes itself.
-func routesMayBeFreed(b []byte, routes []Route) bool {
+// routesMayHaveMoved reports whether b, one routing socket message, may have freed a prefix one of routes installs or
+// handed it to another interface. That's a route for it added, changed or removed, or an interface that lost an address
+// or changed state, since detaching an interface deletes its routes without a message of their own. The header is read
+// directly, x/net/route skips the RTM_IFINFO the socket sends, which has no interface address, and misreads the
+// unpadded sockaddrs in messages the kernel makes itself. A request naming one of the prefixes also returns that prefix
+// and the pid of the process that made it.
+func routesMayHaveMoved(b []byte, routes []Route) (moved bool, p netip.Prefix, pid int32) {
 	if len(b) < 4 {
-		return false
+		return false, p, 0
 	}
 	switch int(b[3]) {
 	case unix.RTM_IFINFO, unix.RTM_DELADDR:
-		return true
-	case unix.RTM_DELETE:
+		return true, p, 0
+	case unix.RTM_ADD, unix.RTM_CHANGE, unix.RTM_DELETE:
 	default:
-		return false
+		return false, p, 0
 	}
 
 	if len(b) < unix.SizeofRtMsghdr {
-		return false
+		return false, p, 0
 	}
 	hdr := (*unix.RtMsghdr)(unsafe.Pointer(&b[0]))
 	// A failed request is reported too, and a scoped route never held the prefix
 	if hdr.Errno != 0 || hdr.Flags&unix.RTF_IFSCOPE != 0 {
-		return false
+		return false, p, 0
 	}
 
 	if hdr.Pid == 0 {
 		// The kernel's own, which x/net/route can't parse. Neighbors coming and going are host routes, anything else
 		// could be holding a prefix
-		return hdr.Flags&(unix.RTF_HOST|unix.RTF_LLINFO) == 0
+		return hdr.Flags&(unix.RTF_HOST|unix.RTF_LLINFO) == 0, p, 0
 	}
 
 	// Replies to a request carry the requester's padded sockaddrs
 	msgs, err := netroute.ParseRIB(unix.NET_RT_DUMP, b)
 	if err != nil || len(msgs) != 1 {
-		return false
+		return false, p, 0
 	}
 	m, ok := msgs[0].(*netroute.RouteMessage)
 	if !ok || len(m.Addrs) <= unix.RTAX_NETMASK {
-		return false
+		return false, p, 0
 	}
-	p, ok := routePrefix(m.Addrs[unix.RTAX_DST], m.Addrs[unix.RTAX_NETMASK])
+	p, ok = routePrefix(m.Addrs[unix.RTAX_DST], m.Addrs[unix.RTAX_NETMASK])
 	if !ok {
-		return false
+		return false, netip.Prefix{}, 0
 	}
+	p = p.Masked()
 	for _, r := range installRoutes(routes) {
 		if r.Cidr.Masked() == p {
-			return true
+			return true, p, hdr.Pid
 		}
 	}
-	return false
+	return false, netip.Prefix{}, 0
 }
 
 // interfaceName is the name of the interface with index i, for logs