logpolicy: rename target env var to TS_LOG_TARGET

Signed-Off-By: Christine Dodrill <xe@tailscale.com>

cmd/tailscale/cli/status.go

@@ -65,7 +65,17 @@ func runStatus(ctx context.Context, args []string) error {
 			log.Fatal(*n.ErrMessage)
 		}
 		if n.Status != nil {
-			ch <- n.Status
+			select {
+			case ch <- n.Status:
+			default:
+				// A status update from somebody else's request.
+				// Ignoring this matters mostly for "tailscale status -web"
+				// mode, otherwise the channel send would block forever
+				// and pump would stop reading from tailscaled, which
+				// previously caused tailscaled to block (while holding
+				// a mutex), backing up unrelated clients.
+				// See https://github.com/tailscale/tailscale/issues/1234
+			}
 		}
 	})
 	go pump(ctx, bc, c)

ipn/message_test.go

@@ -16,9 +16,7 @@ import (
 
 func TestReadWrite(t *testing.T) {
 	tstest.PanicOnLog()
-
-	rc := tstest.NewResourceCheck()
-	defer rc.Assert(t)
+	tstest.ResourceCheck(t)
 
 	buf := bytes.Buffer{}
 	err := WriteMsg(&buf, []byte("Test string1"))
@@ -64,9 +62,7 @@ func TestReadWrite(t *testing.T) {
 
 func TestClientServer(t *testing.T) {
 	tstest.PanicOnLog()
-
-	rc := tstest.NewResourceCheck()
-	defer rc.Assert(t)
+	tstest.ResourceCheck(t)
 
 	b := &FakeBackend{}
 	var bs *BackendServer

logpolicy/logpolicy.go

@@ -17,6 +17,7 @@ import (
 	"log"
 	"net"
 	"net/http"
+	"net/url"
 	"os"
 	"os/exec"
 	"path/filepath"
@@ -387,6 +388,13 @@ func New(collection string) *Policy {
 		HTTPC: &http.Client{Transport: newLogtailTransport(logtail.DefaultHost)},
 	}
 
+	if val, ok := os.LookupEnv("TS_LOG_TARGET"); ok {
+		log.Println("You have enabled a non-default log target. Doing without being told to by Tailscale staff or your network administrator will make getting support difficult.")
+		c.BaseURL = val
+		u, _ := url.Parse(val)
+		c.HTTPC = &http.Client{Transport: newLogtailTransport(u.Host)}
+	}
+
 	filchBuf, filchErr := filch.New(filepath.Join(dir, cmdName), filch.Options{})
 	if filchBuf != nil {
 		c.Buffer = filchBuf

net/interfaces/interfaces_darwin_cgo.go

@@ -15,7 +15,7 @@ package interfaces
 
 // privateGatewayIPFromRoute returns the private gateway ip address from rtm, if it exists.
 // Otherwise, it returns 0.
-int privateGatewayIPFromRoute(struct rt_msghdr2 *rtm)
+uint32_t privateGatewayIPFromRoute(struct rt_msghdr2 *rtm)
 {
     // sockaddrs are after the message header
     struct sockaddr* dst_sa = (struct sockaddr *)(rtm + 1);
@@ -38,7 +38,7 @@ int privateGatewayIPFromRoute(struct rt_msghdr2 *rtm)
         return 0; // gateway not IPv4
 
     struct sockaddr_in* gateway_si= (struct sockaddr_in *)gateway_sa;
-    int ip;
+    uint32_t ip;
     ip = gateway_si->sin_addr.s_addr;
 
     unsigned char a, b;
@@ -62,7 +62,7 @@ int privateGatewayIPFromRoute(struct rt_msghdr2 *rtm)
 // If no private gateway IP address was found, it returns 0.
 // On an error, it returns an error code in (0, 255].
 // Any private gateway IP address is > 255.
-int privateGatewayIP()
+uint32_t privateGatewayIP()
 {
     size_t needed;
     int mib[6];
@@ -90,7 +90,7 @@ int privateGatewayIP()
 	struct rt_msghdr2 *rtm;
     for (next = buf; next < lim; next += rtm->rtm_msglen) {
         rtm = (struct rt_msghdr2 *)next;
-		int ip;
+        uint32_t ip;
         ip = privateGatewayIPFromRoute(rtm);
         if (ip) {
             free(buf);

portlist/portlist_test.go

@@ -12,8 +12,7 @@ import (
 )
 
 func TestGetList(t *testing.T) {
-	rc := tstest.NewResourceCheck()
-	defer rc.Assert(t)
+	tstest.ResourceCheck(t)
 
 	pl, err := GetList(nil)
 	if err != nil {
@@ -26,8 +25,7 @@ func TestGetList(t *testing.T) {
 }
 
 func TestIgnoreLocallyBoundPorts(t *testing.T) {
-	rc := tstest.NewResourceCheck()
-	defer rc.Assert(t)
+	tstest.ResourceCheck(t)
 
 	ln, err := net.Listen("tcp", "127.0.0.1:0")
 	if err != nil {

tailcfg/tailcfg.go

@@ -634,6 +634,8 @@ type MapRequest struct {
 	// Current DebugFlags values are:
 	//     * "warn-ip-forwarding-off": client is trying to be a subnet
 	//       router but their IP forwarding is broken.
+	//     * "v6-overlay": IPv6 development flag to have control send
+	//       v6 node addrs
 	//     * "minimize-netmap": have control minimize the netmap, removing
 	//       peers that are unreachable per ACLS.
 	DebugFlags []string `json:",omitempty"`

tstest/resource.go

@@ -14,64 +14,30 @@ import (
 	"github.com/google/go-cmp/cmp"
 )
 
-type ResourceCheck struct {
-	startNumRoutines int
-	startDump        string
-}
-
-func NewResourceCheck() *ResourceCheck {
-	// NOTE(apenwarr): I'd rather not pre-generate a goroutine dump here.
-	//  However, it turns out to be tricky to debug when eg. the initial
-	//  goroutine count > the ending goroutine count, because of course
-	//  the missing ones are not in the final dump. Also, we have to
-	//  render the profile as a string right away, because the
-	//  pprof.Profile object doesn't stay stable over time. Every time
-	//  you render the string, you might get a different answer.
-	r := &ResourceCheck{}
-	r.startNumRoutines, r.startDump = goroutineDump()
-	return r
-}
-
-func goroutineDump() (int, string) {
-	p := pprof.Lookup("goroutine")
-	b := &bytes.Buffer{}
-	p.WriteTo(b, 1)
-	return p.Count(), b.String()
-}
-
-func (r *ResourceCheck) Assert(t testing.TB) {
-	if t.Failed() {
-		// Something else went wrong.
-		// Assume that that is responsible for the leak
-		// and don't pile on a bunch of extra of output.
-		return
-	}
-	t.Helper()
-	want := r.startNumRoutines
-
-	// Some goroutines might be still exiting, so give them a chance
-	got := runtime.NumGoroutine()
-	if want != got {
-		_, dump := goroutineDump()
+func ResourceCheck(tb testing.TB) {
+	startN, startStacks := goroutines()
+	tb.Cleanup(func() {
+		if tb.Failed() {
+			// Something else went wrong.
+			return
+		}
+		tb.Helper()
+		// Goroutines might be still exiting.
 		for i := 0; i < 100; i++ {
-			got = runtime.NumGoroutine()
-			if want == got {
-				break
+			if runtime.NumGoroutine() <= startN {
+				return
 			}
 			time.Sleep(1 * time.Millisecond)
 		}
-
-		// If the count is *still* wrong, that's a failure.
-		if want != got {
-			t.Logf("goroutine diff:\n%v\n", cmp.Diff(r.startDump, dump))
-			t.Logf("goroutine count: expected %d, got %d\n", want, got)
-			// Don't fail if there are *fewer* goroutines than
-			// expected. That just might be some leftover ones
-			// from the previous test, which are pretty hard to
-			// eliminate.
-			if want < got {
-				t.Fatalf("ResourceCheck: goroutine count: expected %d, got %d\n", want, got)
-			}
-		}
-	}
+		endN, endStacks := goroutines()
+		tb.Logf("goroutine diff:\n%v\n", cmp.Diff(startStacks, endStacks))
+		tb.Fatalf("goroutine count: expected %d, got %d\n", startN, endN)
+	})
+}
+
+func goroutines() (int, []byte) {
+	p := pprof.Lookup("goroutine")
+	b := new(bytes.Buffer)
+	p.WriteTo(b, 1)
+	return p.Count(), b.Bytes()
 }

wgengine/magicsock/magicsock_test.go

@@ -331,8 +331,7 @@ func meshStacks(logf logger.Logf, ms []*magicStack) (cleanup func()) {
 
 func TestNewConn(t *testing.T) {
 	tstest.PanicOnLog()
-	rc := tstest.NewResourceCheck()
-	defer rc.Assert(t)
+	tstest.ResourceCheck(t)
 
 	epCh := make(chan string, 16)
 	epFunc := func(endpoints []string) {
@@ -398,8 +397,7 @@ func pickPort(t testing.TB) uint16 {
 
 func TestDerpIPConstant(t *testing.T) {
 	tstest.PanicOnLog()
-	rc := tstest.NewResourceCheck()
-	defer rc.Assert(t)
+	tstest.ResourceCheck(t)
 
 	if DerpMagicIP != derpMagicIP.String() {
 		t.Errorf("str %q != IP %v", DerpMagicIP, derpMagicIP)
@@ -411,8 +409,7 @@ func TestDerpIPConstant(t *testing.T) {
 
 func TestPickDERPFallback(t *testing.T) {
 	tstest.PanicOnLog()
-	rc := tstest.NewResourceCheck()
-	defer rc.Assert(t)
+	tstest.ResourceCheck(t)
 
 	c := newConn()
 	c.derpMap = derpmap.Prod()
@@ -519,8 +516,7 @@ func parseCIDR(t *testing.T, addr string) netaddr.IPPrefix {
 // -count=10000 to be sure.
 func TestDeviceStartStop(t *testing.T) {
 	tstest.PanicOnLog()
-	rc := tstest.NewResourceCheck()
-	defer rc.Assert(t)
+	tstest.ResourceCheck(t)
 
 	conn, err := NewConn(Options{
 		EndpointsFunc:           func(eps []string) {},
@@ -838,8 +834,7 @@ func newPinger(t *testing.T, logf logger.Logf, src, dst *magicStack) (cleanup fu
 // get exercised.
 func testActiveDiscovery(t *testing.T, d *devices) {
 	tstest.PanicOnLog()
-	rc := tstest.NewResourceCheck()
-	defer rc.Assert(t)
+	tstest.ResourceCheck(t)
 
 	tlogf, setT := makeNestable(t)
 	setT(t)
@@ -900,8 +895,7 @@ func testActiveDiscovery(t *testing.T, d *devices) {
 
 func testTwoDevicePing(t *testing.T, d *devices) {
 	tstest.PanicOnLog()
-	rc := tstest.NewResourceCheck()
-	defer rc.Assert(t)
+	tstest.ResourceCheck(t)
 
 	// This gets reassigned inside every test, so that the connections
 	// all log using the "current" t.Logf function. Sigh.
@@ -1145,8 +1139,7 @@ func testTwoDevicePing(t *testing.T, d *devices) {
 // TestAddrSet tests addrSet appendDests and updateDst.
 func TestAddrSet(t *testing.T) {
 	tstest.PanicOnLog()
-	rc := tstest.NewResourceCheck()
-	defer rc.Assert(t)
+	tstest.ResourceCheck(t)
 
 	mustIPPortPtr := func(s string) *netaddr.IPPort {
 		t.Helper()

wgengine/router/router_linux.go

@@ -116,7 +116,7 @@ func newUserspaceRouter(logf logger.Logf, _ *device.Device, tunDev tun.Device) (
 
 	v6err := checkIPv6()
 	if v6err != nil {
-		logf("disabling IPv6 due to system IPv6 config: %v", v6err)
+		logf("disabling tunneled IPv6 due to system IPv6 config: %v", v6err)
 	}
 	supportsV6 := v6err == nil
 	supportsV6NAT := supportsV6 && supportsV6NAT()
@@ -1045,18 +1045,22 @@ func checkIPv6() error {
 		return errors.New("disable_ipv6 is set")
 	}
 
-	// Older kernels don't support IPv6 policy routing.
+	// Older kernels don't support IPv6 policy routing. Some kernels
+	// support policy routing but don't have this knob, so absence of
+	// the knob is not fatal.
 	bs, err = ioutil.ReadFile("/proc/sys/net/ipv6/conf/all/disable_policy")
-	if err != nil {
-		// Absent knob means policy routing is unsupported.
-		return err
+	if err == nil {
+		disabled, err = strconv.ParseBool(strings.TrimSpace(string(bs)))
+		if err != nil {
+			return errors.New("disable_policy has invalid bool")
+		}
+		if disabled {
+			return errors.New("disable_policy is set")
+		}
 	}
-	disabled, err = strconv.ParseBool(strings.TrimSpace(string(bs)))
-	if err != nil {
-		return errors.New("disable_policy has invalid bool")
-	}
-	if disabled {
-		return errors.New("disable_policy is set")
+
+	if err := checkIPRuleSupportsV6(); err != nil {
+		return fmt.Errorf("kernel doesn't support IPv6 policy routing: %w", err)
 	}
 
 	// Some distros ship ip6tables separately from iptables.
@@ -1064,10 +1068,6 @@ func checkIPv6() error {
 		return err
 	}
 
-	if err := checkIPRuleSupportsV6(); err != nil {
-		return err
-	}
-
 	return nil
 }
 
@@ -1088,13 +1088,17 @@ func supportsV6NAT() bool {
 }
 
 func checkIPRuleSupportsV6() error {
-	// First add a rule for "ip rule del" to delete.
-	// We ignore the "add" operation's error because it can also
-	// fail if the rule already exists.
-	exec.Command("ip", "-6", "rule", "add",
-		"pref", "123", "fwmark", tailscaleBypassMark, "table", fmt.Sprint(tailscaleRouteTable)).Run()
-	out, err := exec.Command("ip", "-6", "rule", "del",
-		"pref", "123", "fwmark", tailscaleBypassMark, "table", fmt.Sprint(tailscaleRouteTable)).CombinedOutput()
+	add := []string{"-6", "rule", "add", "pref", "1234", "fwmark", tailscaleBypassMark, "table", tailscaleRouteTable}
+	del := []string{"-6", "rule", "del", "pref", "1234", "fwmark", tailscaleBypassMark, "table", tailscaleRouteTable}
+
+	// First delete the rule unconditionally, and don't check for
+	// errors. This is just cleaning up anything that might be already
+	// there.
+	exec.Command("ip", del...).Run()
+
+	// Try adding the rule. This will fail on systems that support
+	// IPv6, but not IPv6 policy routing.
+	out, err := exec.Command("ip", add...).CombinedOutput()
 	if err != nil {
 		out = bytes.TrimSpace(out)
 		var detail interface{} = out
@@ -1103,5 +1107,8 @@ func checkIPRuleSupportsV6() error {
 		}
 		return fmt.Errorf("ip -6 rule failed: %s", detail)
 	}
+
+	// Delete again.
+	exec.Command("ip", del...).Run()
 	return nil
 }

wgengine/tsdns/tsdns_test.go

@@ -275,8 +275,7 @@ func TestResolveReverse(t *testing.T) {
 }
 
 func TestDelegate(t *testing.T) {
-	rc := tstest.NewResourceCheck()
-	defer rc.Assert(t)
+	tstest.ResourceCheck(t)
 
 	dnsHandleFunc("test.site.", resolveToIP(testipv4, testipv6, "dns.test.site."))
 	dnsHandleFunc("nxdomain.site.", resolveToNXDOMAIN)

wgengine/wglog/wglog.go

@@ -36,6 +36,10 @@ func NewLogger(logf logger.Logf) *Logger {
 			// Drop those; there are a lot of them, and they're just noise.
 			return
 		}
+		if strings.Contains(msg, "Failed to send data packet") {
+			// Drop. See https://github.com/tailscale/tailscale/issues/1239.
+			return
+		}
 		r := ret.replacer.Load()
 		if r == nil {
 			// No replacements specified; log as originally planned.