diff --git a/net/netns/netns_darwin.go b/net/netns/netns_darwin.go index 6a127951d..9cd8784b4 100644 --- a/net/netns/netns_darwin.go +++ b/net/netns/netns_darwin.go @@ -59,7 +59,6 @@ func controlLogf(logf logger.Logf, netMon *netmon.Monitor, network, address stri opts := probeOpts{ logf: logf, pt: hpn, - filterf: nil, race: true, cache: cache(), debugLogs: VERBOSE_LOGS, diff --git a/net/netns/netns_probe.go b/net/netns/netns_probe.go index 51060daaf..eef28d6c4 100644 --- a/net/netns/netns_probe.go +++ b/net/netns/netns_probe.go @@ -46,6 +46,8 @@ func tailscaleInterface() (*net.Interface, error) { if err != nil { return nil, err } + // If we've found an interface with the optional specified tunName, + // return it. Otherwise, return the first interface with a Tailscale IP. for _, iface := range ifs { if tunName == iface.Name { return &iface, nil @@ -70,10 +72,13 @@ func tailscaleInterface() (*net.Interface, error) { // probeResult describes an interface and whether it was able to reach // the provided address. type probeResult struct { - iface net.Interface - // TODO (barnstar): These are invariant. reachable should be true if err==nil. - reachable bool - err error + iface net.Interface + target ProbeTarget + err error +} + +func (pr probeResult) targetIsReachable() bool { + return pr.err == nil } // Tuple of the destination host, port, and network. @@ -109,10 +114,9 @@ func (hpn ProbeTarget) String() string { type probeOpts struct { logf logger.Logf pt ProbeTarget - race bool // if true, we'll pick the first interface that responds. sortf is ignored. - filterf interfaceFilter // optional pre-filter for interfaces - cache *routeCache // must be non-nil - debugLogs bool // if true, log verbose output + race bool // if true, we'll pick the first interface that responds. sortf is ignored. + cache *routeCache // must be non-nil + debugLogs bool // if true, log verbose output } func (p *probeOpts) logDebug(format string, args ...any) { @@ -136,7 +140,7 @@ func bindFnByAddrType(network, address string) bindFn { // For testing var interfacesHookFn func() ([]net.Interface, error) -type probeHookFn func(iface *net.Interface, pt ProbeTarget) error +type probeHookFn func(iface *net.Interface, pt ProbeTarget) probeResult var ( interfacesHook atomic.Value // of interfacesHookFn @@ -179,11 +183,6 @@ func probeInterfacesReachability(opts probeOpts) ([]probeResult, error) { var candidates []net.Interface for _, iface := range ifaces { - // Individual platforms can exclude potential intefaces based on platorm-specific logic. - // For example, on Darwin, we skip "utun" interfaces. - if opts.filterf != nil && !opts.filterf(iface) { - continue - } // Only consider up, non-loopback interfaces. if iface.Flags&net.FlagUp == 0 || iface.Flags&net.FlagLoopback != 0 || iface.Flags&net.FlagRunning == 0 { continue @@ -218,10 +217,8 @@ func probeInterfacesReachability(opts probeOpts) ([]probeResult, error) { } opts.logDebug("netns: probing %s for reachability to %s via %s", iface.Name, opts.pt.Host, opts.pt.Network) - probeFn := probeHook.Load().(func(*net.Interface, ProbeTarget) error) - - err := probeFn(&iface, opts.pt) - results <- probeResult{iface: iface, reachable: err == nil, err: err} + probeFn := probeHook.Load().(func(*net.Interface, ProbeTarget) probeResult) + results <- probeFn(&iface, opts.pt) }() } @@ -235,7 +232,7 @@ func probeInterfacesReachability(opts probeOpts) ([]probeResult, error) { // If we're racing, return the first reachable interface immediately. // TODO (barnstar): We should cache all reachable results so we can try alteratives if we // can't get the conn up and running later but signal early if we're racing. - if opts.race && r.reachable { + if opts.race && r.err == nil { close(done) return []probeResult{r}, nil } @@ -250,7 +247,7 @@ func probeInterfacesReachability(opts probeOpts) ([]probeResult, error) { return out, nil } -func probe(iface *net.Interface, pt ProbeTarget) error { +func probe(iface *net.Interface, pt ProbeTarget) probeResult { // For unspecified hosts, we need to listen rather than dial. if pt.Host == "0.0.0.0" || pt.Host == "::" { return probeBindListen(iface, pt) @@ -258,7 +255,7 @@ func probe(iface *net.Interface, pt ProbeTarget) error { return probeBindDial(iface, pt) } -func probeBindDial(iface *net.Interface, pt ProbeTarget) error { +func probeBindDial(iface *net.Interface, pt ProbeTarget) probeResult { // Per-probe timeout. dialCtx, dialCancel := context.WithTimeout(context.Background(), 300*time.Millisecond) defer dialCancel() @@ -271,15 +268,20 @@ func probeBindDial(iface *net.Interface, pt ProbeTarget) error { }, } + result := probeResult{ + iface: *iface, + target: pt, + } dst := net.JoinHostPort(pt.Host, pt.Port) conn, err := d.DialContext(dialCtx, pt.Network, dst) + result.err = err if err == nil { - defer conn.Close() + conn.Close() } - return err + return result } -func probeBindListen(iface *net.Interface, pt ProbeTarget) error { +func probeBindListen(iface *net.Interface, pt ProbeTarget) probeResult { lc := net.ListenConfig{ Control: func(network, address string, c syscall.RawConn) error { bindFn := bindFnByAddrType(network, address) @@ -287,21 +289,26 @@ func probeBindListen(iface *net.Interface, pt ProbeTarget) error { }, } + result := probeResult{ + iface: *iface, + target: pt, + } + dst := net.JoinHostPort(pt.Host, pt.Port) // Bind to this interface on any available port listener, err := lc.ListenPacket(context.Background(), pt.Network, dst) - if err != nil { - return err + result.err = err + if err == nil { + listener.Close() } - listener.Close() - return nil + return result } // Pre-filter for interfaces. Platform-specific code can provide a filter // to exclude certain interfaces from consideration. For example, on Darwin, // we exclude "utun" interfaces and various other types which will never provie // have general internet connectivity. -type interfaceFilter func(net.Interface) bool +type interfaceFilter func(net.Interface) probeResult func filterInPlace[T any](s []T, keep func(T) bool) []T { i := 0 @@ -362,7 +369,7 @@ func findInterfaceThatCanReach(opts probeOpts) (iface *net.Interface, err error) return nil, err } - res = filterInPlace(res, func(r probeResult) bool { return r.reachable }) + res = filterInPlace(res, func(r probeResult) bool { return r.err == nil }) if len(res) == 0 { opts.logf("netns: could not find interface on network %v to reach %s:%s on %s: %v", opts.pt.Network, opts.pt.Host, opts.pt.Port, opts.pt.Network, err) return nil, errNoAvailableInterface diff --git a/net/netns/netns_test.go b/net/netns/netns_test.go index 121f37995..13ac1a57c 100644 --- a/net/netns/netns_test.go +++ b/net/netns/netns_test.go @@ -18,7 +18,6 @@ import ( "flag" "net" "net/netip" - "sync/atomic" "testing" ) @@ -336,7 +335,7 @@ var ( ) func TestFindInterfaceThatCanReach(t *testing.T) { - origProbeHook := probeHook.Load().(func(*net.Interface, ProbeTarget) error) + origProbeHook := probeHook.Load().(func(*net.Interface, ProbeTarget) probeResult) t.Cleanup(func() { ifaceHasV4AndGlobalV6Hook = nil probeHook.Store(origProbeHook) @@ -351,18 +350,18 @@ func TestFindInterfaceThatCanReach(t *testing.T) { hookDefaultInterfaces(t) // Pre-populate cache - addr := netip.MustParseAddr("8.8.8.8") + addr := netip.MustParseAddr("1.2.3.4") cache.setCachedRoute(addr, &interfaceWlan0) // Hook should never be called when cache hits - probeHook.Store(func(iface *net.Interface, hpn ProbeTarget) error { + probeHook.Store(func(iface *net.Interface, hpn ProbeTarget) probeResult { t.Error("reachabilityHookFn should not be called when cache hits") - return nil + return probeResult{} }) opts := probeOpts{ logf: t.Logf, - pt: ProbeTarget{Host: "8.8.8.8", Port: "53", Network: "udp"}, + pt: NewProbeTarget("udp", "1.2.3.4", "42"), cache: cache, } @@ -385,13 +384,13 @@ func TestFindInterfaceThatCanReach(t *testing.T) { hookDefaultInterfaces(t) // All interfaces succeed - probeHook.Store(func(iface *net.Interface, hpn ProbeTarget) error { - return nil + probeHook.Store(func(iface *net.Interface, hpn ProbeTarget) probeResult { + return probeResult{} }) opts := probeOpts{ logf: t.Logf, - pt: ProbeTarget{Host: "1.1.1.1", Port: "53", Network: "udp"}, + pt: NewProbeTarget("udp", "1.1.1.1", "42"), cache: cache, } @@ -419,13 +418,13 @@ func TestFindInterfaceThatCanReach(t *testing.T) { hookDefaultInterfaces(t) // All interfaces fail - probeHook.Store(func(iface *net.Interface, hpn ProbeTarget) error { - return errors.New("unreachable") + probeHook.Store(func(iface *net.Interface, hpn ProbeTarget) probeResult { + return probeResult{iface: *iface, target: hpn, err: errors.New("unreachable")} }) opts := probeOpts{ logf: t.Logf, - pt: ProbeTarget{Host: "192.0.2.1", Port: "53", Network: "udp"}, + pt: NewProbeTarget("udp", "192.0.2.1", "42"), cache: cache, } @@ -451,15 +450,15 @@ func TestFindInterfaceThatCanReach(t *testing.T) { prefix2 := netip.MustParsePrefix("10.0.1.0/24") cache.setCachedRoutePrefix(prefix2, &interfaceWlan0) - probeHook.Store(func(iface *net.Interface, hpn ProbeTarget) error { + probeHook.Store(func(iface *net.Interface, hpn ProbeTarget) probeResult { t.Error("should use cache, not probe") - return nil + return probeResult{} }) // Test 10.0.1.5 -> should match more specific /24 opts1 := probeOpts{ logf: t.Logf, - pt: ProbeTarget{Host: "10.0.1.5", Port: "53", Network: "udp"}, + pt: NewProbeTarget("udp", "10.0.1.5", "42"), cache: cache, } @@ -471,7 +470,7 @@ func TestFindInterfaceThatCanReach(t *testing.T) { // Test 10.0.2.5 -> should match broader /8 opts2 := probeOpts{ logf: t.Logf, - pt: ProbeTarget{Host: "10.0.2.5", Port: "53", Network: "udp"}, + pt: NewProbeTarget("udp", "10.0.2.5", "42"), cache: cache, } @@ -492,19 +491,20 @@ func TestFindInterfaceThatCanReach(t *testing.T) { wlan0Done := make(chan struct{}) eth1Done := make(chan struct{}) - probeHook.Store(func(iface *net.Interface, hpn ProbeTarget) error { + probeHook.Store(func(iface *net.Interface, hpn ProbeTarget) probeResult { switch iface.Index { case interfaceEth0.Index: // eth0 - returns immediately - return nil + return probeResult{} case interfaceWlan0.Index: // wlan0 - waits for signal <-wlan0Done - return nil + return probeResult{} case interfaceEth1.Index: // eth1 - waits for signal <-eth1Done - return nil + return probeResult{} } - return errors.New("unknown interface") + return probeResult{err: errors.New("unknown interface")} }) + // Finish the other probes when done defer func() { close(wlan0Done) close(eth1Done) @@ -512,7 +512,7 @@ func TestFindInterfaceThatCanReach(t *testing.T) { opts := probeOpts{ logf: t.Logf, - pt: ProbeTarget{Host: "8.8.8.8", Port: "53", Network: "udp"}, + pt: NewProbeTarget("udp", "1.2.3.4", "42"), race: true, cache: cache, } @@ -530,88 +530,22 @@ func TestFindInterfaceThatCanReach(t *testing.T) { t.Logf("race mode returned interface: %s", result.Name) }) - t.Run("filterf excludes interfaces", func(t *testing.T) { - cache := NewRouteCache() - hookDefaultInterfaces(t) - - probeCount := atomic.Int32{} - probeHook.Store(func(iface *net.Interface, hpn ProbeTarget) error { - probeCount.Add(1) - return nil - }) - - opts := probeOpts{ - logf: t.Logf, - pt: ProbeTarget{Host: "8.8.8.8", Port: "53", Network: "udp"}, - cache: cache, - filterf: func(iface net.Interface) bool { - // Exclude wlan0 and eth1 - return iface.Name != "wlan0" && iface.Name != "eth1" - }, - } - - result, err := findInterfaceThatCanReach(opts) - if err != nil { - t.Fatalf("findInterfaceThatCanReach failed: %v", err) - } - - // Should only probe filtered interfaces - if probeCount.Load() > 1 { - t.Logf("probed %d interfaces after filtering", probeCount.Load()) - } - - if result != nil && (result.Name == "wlan0" || result.Name == "eth1") { - t.Errorf("filterf should have excluded %s", result.Name) - } - }) - - t.Run("handles hostname instead of IP", func(t *testing.T) { - cache := NewRouteCache() - hookDefaultInterfaces(t) - - probeHook.Store(func(iface *net.Interface, hpn ProbeTarget) error { - return nil - }) - - // Use a hostname that can't be parsed as an IP - opts := probeOpts{ - logf: t.Logf, - pt: ProbeTarget{Host: "example.com", Port: "443", Network: "tcp"}, - cache: cache, - } - - result, err := findInterfaceThatCanReach(opts) - if err != nil { - t.Fatalf("findInterfaceThatCanReach failed: %v", err) - } - - if result == nil { - t.Fatal("expected non-nil result") - } - - // Cache should not be used for hostnames - addr, parseErr := netip.ParseAddr("example.com") - if parseErr == nil && addr.IsValid() { - t.Error("example.com should not parse as valid IP") - } - }) - - t.Run("IPv6 address uses IPv6 cache table", func(t *testing.T) { + t.Run("IPv6 address caching", func(t *testing.T) { cache := NewRouteCache() hookDefaultInterfaces(t) // Pre-populate IPv6 cache - addr6 := netip.MustParseAddr("2001:4860:4860::8888") + addr6 := netip.MustParseAddr("2001:4860:4860::1234") cache.setCachedRoute(addr6, &interfaceEth1) - probeHook.Store(func(iface *net.Interface, hpn ProbeTarget) error { + probeHook.Store(func(iface *net.Interface, hpn ProbeTarget) probeResult { t.Error("should use cache for IPv6") - return nil + return probeResult{} }) opts := probeOpts{ logf: t.Logf, - pt: ProbeTarget{Host: "2001:4860:4860::8888", Port: "53", Network: "udp6"}, + pt: NewProbeTarget("udp6", "2001:4860:4860::1234", "42"), cache: cache, } @@ -625,55 +559,17 @@ func TestFindInterfaceThatCanReach(t *testing.T) { } }) - t.Run("IPv4 and IPv6 caches are independent", func(t *testing.T) { - cache := NewRouteCache() - hookDefaultInterfaces(t) - - addr4 := netip.MustParseAddr("8.8.8.8") - addr6 := netip.MustParseAddr("2001:4860:4860::8888") - - cache.setCachedRoute(addr4, &interfaceEth0) - cache.setCachedRoute(addr6, &interfaceWlan0) - - probeHook.Store(func(iface *net.Interface, hpn ProbeTarget) error { - t.Error("should use cache") - return nil - }) - - // Test IPv4 - opts4 := probeOpts{ - logf: t.Logf, - pt: ProbeTarget{Host: "8.8.8.8", Port: "53", Network: "udp"}, - cache: cache, - } - result4, _ := findInterfaceThatCanReach(opts4) - if result4 == nil || result4.Name != "eth0" { - t.Errorf("IPv4: expected eth0, got %v", result4) - } - - // Test IPv6 - opts6 := probeOpts{ - logf: t.Logf, - pt: ProbeTarget{Host: "2001:4860:4860::8888", Port: "53", Network: "udp6"}, - cache: cache, - } - result6, _ := findInterfaceThatCanReach(opts6) - if result6 == nil || result6.Name != "wlan0" { - t.Errorf("IPv6: expected wlan0, got %v", result6) - } - }) - t.Run("empty host returns error", func(t *testing.T) { cache := NewRouteCache() hookDefaultInterfaces(t) - probeHook.Store(func(iface *net.Interface, hpn ProbeTarget) error { - return nil + probeHook.Store(func(iface *net.Interface, hpn ProbeTarget) probeResult { + return probeResult{} }) opts := probeOpts{ logf: t.Logf, - pt: ProbeTarget{Host: "", Port: "53", Network: "udp"}, + pt: NewProbeTarget("udp", "", "53"), cache: cache, } @@ -693,29 +589,39 @@ func TestFindInterfaceThatCanReach(t *testing.T) { prefix := netip.MustParsePrefix("192.168.0.0/16") cache.setCachedRoutePrefix(prefix, &interfaceEth0) - probeHook.Store(func(iface *net.Interface, hpn ProbeTarget) error { + probeHook.Store(func(iface *net.Interface, hpn ProbeTarget) probeResult { t.Error("should use cached subnet") - return nil + return probeResult{} }) - // Test various IPs in the subnet - testIPs := []string{ - "192.168.0.1", - "192.168.1.1", - "192.168.255.254", + tests := []struct { + name string + ip string + want string // expected interface name + }{ + {"subnet start", "192.168.0.1", "eth0"}, + {"subnet middle", "192.168.1.1", "eth0"}, + {"subnet end", "192.168.255.254", "eth0"}, } - for _, ip := range testIPs { - opts := probeOpts{ - logf: t.Logf, - pt: ProbeTarget{Host: ip, Port: "53", Network: "udp"}, - cache: cache, - } - - result, _ := findInterfaceThatCanReach(opts) - if result == nil || result.Name != "eth0" { - t.Errorf("IP %s: expected eth0 from cached subnet, got %v", ip, result) - } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + opts := probeOpts{ + logf: t.Logf, + pt: NewProbeTarget("udp", tt.ip, "42"), + cache: cache, + } + + result, err := findInterfaceThatCanReach(opts) + if err != nil { + t.Errorf("findInterfaceThatCanReach(%s) error: %v", tt.ip, err) + } + if result == nil { + t.Errorf("IP %s: expected %s from cached subnet, got nil", tt.ip, tt.want) + } else if result.Name != tt.want { + t.Errorf("IP %s: expected %s from cached subnet, got %s", tt.ip, tt.want, result.Name) + } + }) } }) }