From 3b93fd9c4430332787e6d9ed6164efb63d3a9e8b Mon Sep 17 00:00:00 2001 From: Brad Fitzpatrick Date: Fri, 15 Nov 2024 14:16:03 -0800 Subject: [PATCH] net/captivedetection: replace 10k log lines with ... less We see tons of logs of the form: 2024/11/15 19:57:29 netcheck: [v2] 76 available captive portal detection endpoints: [Endpoint{URL="http://192.73.240.161/generate_204", StatusCode=204, ExpectedContent="", SupportsTailscaleChallenge=true, Provider=DERPMapOther} Endpoint{URL="http://192.73.240.121/generate_204", StatusCode=204, ExpectedContent="", SupportsTailscaleChallenge=true, Provider=DERPMapOther} Endpoint{URL="http://192.73.240.132/generate_204", StatusCode=204, ExpectedContent="", 11:58SupportsTailscaleChallenge=true, Provider=DERPMapOther} Endpoint{URL="http://209.177.158.246/generate_204", StatusCode=204, ExpectedContent="", SupportsTailscaleChallenge=true, Provider=DERPMapOther} Endpoint{URL="http://209.177.158.15/generate_204", StatusCode=204, ExpectedContent="", SupportsTailscaleChallenge=true, Provider=DERPMapOther} Endpoint{URL="http://199.38.182.118/generate_204", StatusCode=204, ExpectedContent="", SupportsTailscaleChallenge=true, Provider=DERPMapOther} Endpoint{URL="http://192.73.243.135/generate_204", StatusCode=204, ExpectedContent="", SupportsTailscaleChallenge=true, Provider=DERPMapOther} Endpoint{URL="http://192.73.243.229/generate_204", StatusCode=204, ExpectedContent="", SupportsTailscaleChallenge=true, Provider=DERPMapOther} Endpoint{URL="http://192.73.243.141/generate_204", StatusCode=204, ExpectedContent="", SupportsTailscaleChallenge=true, Provider=DERPMapOther} Endpoint{URL="http://45.159.97.144/generate_204", StatusCode=204, ExpectedContent="", SupportsTailscaleChallenge=true, Provider=DERPMapOther} Endpoint{URL="http://45.159.97.61/generate_204", StatusCode=204, ExpectedContent="", SupportsTailscaleChallenge=true, Provider=DERPMapOther} Endpoint{URL="http://45.159.97.233/generate_204", StatusCode=204, ExpectedContent="", SupportsTailscaleChallenge=true, Provider=DERPMapOther} Endpoint{URL="http://45.159.98.196/generate_204", StatusCode=204, ExpectedContent="", SupportsTailscaleChallenge=true, Provider=DERPMapOther} Endpoint{URL="http://45.159.98.253/generate_204", StatusCode=204, ExpectedContent="", SupportsTailscaleChallenge=true, Provider=DERPMapOther} Endpoint{URL="http://45.159.98.145/generate_204", StatusCode=204, ExpectedContent="", SupportsTailscaleChallenge=true, Provider=DERPMapOther} Endpoint{URL="http://68.183.90.120/generate_204", StatusCode=204, ExpectedContent="", SupportsTailscaleChallenge=true, Provider=DERPMapOther} Endpoint{URL="http://209.177.156.94/generate_204", StatusCode=204, ExpectedContent="", SupportsTailscaleChallenge=true, Provider=DERPMapOther} Endpoint{URL="http://192.73.248.83/generate_204", StatusCode=204, ExpectedContent="", SupportsTailscaleChallenge=true, Provider=DERPMapOther} Endpoint{URL="http://209.177.156.197/generate_204", StatusCode=204, ExpectedContent="", SupportsTailscaleChallenge=true, Provider=DERPMapOther} Endpoint{URL="http://199.38.181.104/generate_204", StatusCode=204, ExpectedContent="", SupportsTailscaleChallenge=true, Provider=DERPMapOther} Endpoint{URL="http://209.177.145.120/generate_204", StatusCode=204, ExpectedContent="", SupportsTailscaleChallenge=true, Provider=DERPMapOther} Endpoint{URL="http://199.38.181.93/generate_204", StatusCode=204, ExpectedContent="", SupportsTailscaleChallenge=true, Provider=DERPMapOther} Endpoint{URL="http://199.38.181.103/generate_204", StatusCode=204, ExpectedContent="", SupportsTailscaleChallenge=true, Provider=DERPMapOther} Endpoint{URL="http://102.67.165.90/generate_204", StatusCode=204, ExpectedContent="", SupportsTailscaleChallenge=true, Provider=DERPMapOther} Endpoint{URL="http://102.67.165.185/generate_204", StatusCode=204, ExpectedContent="", SupportsTailscaleChallenge=true, Provider=DERPMapOther} Endpoint{URL="http://102.67.165.36/generate_204", StatusCode=204, ExpectedContent="", SupportsTailscaleChallenge=true, Provider=DERPMapOther} Endpoint{URL="http://176.58.90.147/generate_204", StatusCode=204, ExpectedContent="", SupportsTailscaleChallenge=true, Provider=DERPMapOther} Endpoint{URL="http://176.58.90.207/generate_204", StatusCode=204, ExpectedContent="", SupportsTailscaleChallenge=true, Provider=DERPMapOther} Endpoint{URL="http://176.58.90.104/generate_204", StatusCode=204, ExpectedContent="", SupportsTailscaleChallenge=true, Provider=DERPMapOther} Endpoint{URL="http://162.248.221.199/generate_204", StatusCode=204, ExpectedContent="", SupportsTailscaleChallenge=true, Provider=DERPMapOther} Endpoint{URL="http://162.248.221.215/generate_204", StatusCode=204, ExpectedContent="", SupportsTailscaleChallenge=true, Provider=DERPMapOther} Endpoint{URL="http://162.248.221.248/generate_204", StatusCode=204, ExpectedContent="", SupportsTailscaleChallenge=true, Provider=DERPMapOther} Endpoint{URL="http://185.34.3.232/generate_204", StatusCode=204, ExpectedContent="", SupportsTailscaleChallenge=true, Provider=DERPMapOther} Endpoint{URL="http://185.34.3.207/generate_204", StatusCode=204, ExpectedContent="", SupportsTailscaleChallenge=true, Provider=DERPMapOther} Endpoint{URL="http://185.34.3.75/generate_204", StatusCode=204, ExpectedContent="", SupportsTailscaleChallenge=true, Provider=DERPMapOther} Endpoint{URL="http://208.83.234.151/generate_204", StatusCode=204, ExpectedContent="", SupportsTailscaleChallenge=true, Provider=DERPMapOther} Endpoint{URL="http://208.83.233.233/generate_204", StatusCode=204, ExpectedContent="", SupportsTailscaleChallenge=true, Provider=DERPMapOther} Endpoint{URL="http://208.72.155.133/generate_204", StatusCode=204, ExpectedContent="", SupportsTailscaleChallenge=true, Provider=DERPMapOther} Endpoint{URL="http://185.40.234.219/generate_204", StatusCode=204, ExpectedContent="", SupportsTailscaleChallenge=true, Provider=DERPMapOther} Endpoint{URL="http://185.40.234.113/generate_204", StatusCode=204, ExpectedContent="", SupportsTailscaleChallenge=true, Provider=DERPMapOther} Endpoint{URL="http://185.40.234.77/generate_204", StatusCode=204, ExpectedContent="", SupportsTailscaleChallenge=true, Provider=DERPMapOther} Endpoint{URL="http://43.245.48.220/generate_204", StatusCode=204, ExpectedContent="", SupportsTailscaleChallenge=true, Provider=DERPMapOther} Endpoint{URL="http://43.245.48.50/generate_204", StatusCode=204, ExpectedContent="", SupportsTailscaleChallenge=true, Provider=DERPMapOther} Endpoint{URL="http://43.245.48.250/generate_204", StatusCode=204, ExpectedContent="", SupportsTailscaleChallenge=true, Provider=DERPMapOther} Endpoint{URL="http://192.73.252.65/generate_204", StatusCode=204, ExpectedContent="", SupportsTailscaleChallenge=true, Provider=DERPMapOther} Endpoint{URL="http://192.73.252.134/generate_204", StatusCode=204, ExpectedContent="", SupportsTailscaleChallenge=true, Provider=DERPMapOther} Endpoint{URL="http://208.111.34.178/generate_204", StatusCode=204, ExpectedContent="", SupportsTailscaleChallenge=true, Provider=DERPMapOther} Endpoint{URL="http://43.245.49.105/generate_204", StatusCode=204, ExpectedContent="", SupportsTailscaleChallenge=true, Provider=DERPMapOther} Endpoint{URL="http://43.245.49.83/generate_204", StatusCode=204, ExpectedContent="", SupportsTailscaleChallenge=true, Provider=DERPMapOther} Endpoint{URL="http://43.245.49.144/generate_204", StatusCode=204, ExpectedContent="", SupportsTailscaleChallenge=true, Provider=DERPMapOther} Endpoint{URL="http://176.58.92.144/generate_204", StatusCode=204, ExpectedContent="", SupportsTailscaleChallenge=true, Provider=DERPMapOther} Endpoint{URL="http://176.58.88.183/generate_204", StatusCode=204, ExpectedContent="", SupportsTailscaleChallenge=true, Provider=DERPMapOther} Endpoint{URL="http://176.58.92.254/generate_204", StatusCode=204, ExpectedContent="", SupportsTailscaleChallenge=true, Provider=DERPMapOther} Endpoint{URL="http://148.163.220.129/generate_204", StatusCode=204, ExpectedContent="", SupportsTailscaleChallenge=true, Provider=DERPMapOther} Endpoint{URL="http://148.163.220.134/generate_204", StatusCode=204, ExpectedContent="", SupportsTailscaleChallenge=true, Provider=DERPMapOther} Endpoint{URL="http://148.163.220.210/generate_204", StatusCode=204, ExpectedContent="", SupportsTailscaleChallenge=true, Provider=DERPMapOther} Endpoint{URL="http://192.73.242.187/generate_204", StatusCode=204, ExpectedContent="", SupportsTailscaleChallenge=true, Provider=DERPMapOther} Endpoint{URL="http://192.73.242.28/generate_204", StatusCode=204, ExpectedContent="", SupportsTailscaleChallenge=true, Provider=DERPMapOther} Endpoint{URL="http://192.73.242.204/generate_204", StatusCode=204, ExpectedContent="", SupportsTailscaleChallenge=true, Provider=DERPMapOther} Endpoint{URL="http://176.58.93.248/generate_204", StatusCode=204, ExpectedContent="", SupportsTailscaleChallenge=true, Provider=DERPMapOther} Endpoint{URL="http://176.58.93.147/generate_204", StatusCode=204, ExpectedContent="", SupportsTailscaleChallenge=true, Provider=DERPMapOther} Endpoint{URL="http://176.58.93.154/generate_204", StatusCode=204, ExpectedContent="", SupportsTailscaleChallenge=true, Provider=DERPMapOther} Endpoint{URL="http://192.73.244.245/generate_204", StatusCode=204, ExpectedContent="", SupportsTailscaleChallenge=true, Provider=DERPMapOther} Endpoint{URL="http://208.111.40.12/generate_204", StatusCode=204, ExpectedContent="", SupportsTailscaleChallenge=true, Provider=DERPMapOther} Endpoint{URL="http://208.111.40.216/generate_204", StatusCode=204, ExpectedContent="", SupportsTailscaleChallenge=true, Provider=DERPMapOther} Endpoint{URL="http://103.6.84.152/generate_204", StatusCode=204, ExpectedContent="", SupportsTailscaleChallenge=true, Provider=DERPMapOther} Endpoint{URL="http://205.147.105.30/generate_204", StatusCode=204, ExpectedContent="", SupportsTailscaleChallenge=true, Provider=DERPMapOther} Endpoint{URL="http://205.147.105.78/generate_204", StatusCode=204, ExpectedContent="", SupportsTailscaleChallenge=true, Provider=DERPMapOther} Endpoint{URL="http://102.67.167.245/generate_204", StatusCode=204, ExpectedContent="", SupportsTailscaleChallenge=true, Provider=DERPMapOther} Endpoint{URL="http://102.67.167.37/generate_204", StatusCode=204, ExpectedContent="", SupportsTailscaleChallenge=true, Provider=DERPMapOther} Endpoint{URL="http://102.67.167.188/generate_204", StatusCode=204, ExpectedContent="", SupportsTailscaleChallenge=true, Provider=DERPMapOther} Endpoint{URL="http://103.84.155.178/generate_204", StatusCode=204, ExpectedContent="", SupportsTailscaleChallenge=true, Provider=DERPMapOther} Endpoint{URL="http://103.84.155.188/generate_204", StatusCode=204, ExpectedContent="", SupportsTailscaleChallenge=true, Provider=DERPMapOther} Endpoint{URL="http://103.84.155.46/generate_204", StatusCode=204, ExpectedContent="", SupportsTailscaleChallenge=true, Provider=DERPMapOther} Endpoint{URL="http://controlplane.tailscale.com/generate_204", StatusCode=204, ExpectedContent="", SupportsTailscaleChallenge=false, Provider=Tailscale} Endpoint{URL="http://login.tailscale.com/generate_204", StatusCode=204, ExpectedContent="", SupportsTailscaleChallenge=false, Provider=Tailscale}] That can be much shorter. Also add a fast exit path to the concurrency on match. Doing 5 all at once is still pretty gratuitous, though. Updates #1634 Fixes #13019 Change-Id: Icdbb16572fca4477b0ee9882683a3ac6eb08e2f2 Signed-off-by: Brad Fitzpatrick --- net/captivedetection/captivedetection.go | 19 ++++++---- net/captivedetection/captivedetection_test.go | 37 +++++++++++++++---- 2 files changed, 42 insertions(+), 14 deletions(-) diff --git a/net/captivedetection/captivedetection.go b/net/captivedetection/captivedetection.go index c6e8bca3a..7d598d853 100644 --- a/net/captivedetection/captivedetection.go +++ b/net/captivedetection/captivedetection.go @@ -136,26 +136,31 @@ func interfaceNameDoesNotNeedCaptiveDetection(ifName string, goos string) bool { func (d *Detector) detectOnInterface(ctx context.Context, ifIndex int, endpoints []Endpoint) bool { defer d.httpClient.CloseIdleConnections() - d.logf("[v2] %d available captive portal detection endpoints: %v", len(endpoints), endpoints) + use := min(len(endpoints), 5) + endpoints = endpoints[:use] + d.logf("[v2] %d available captive portal detection endpoints; trying %v", len(endpoints), use) // We try to detect the captive portal more quickly by making requests to multiple endpoints concurrently. var wg sync.WaitGroup resultCh := make(chan bool, len(endpoints)) - for i, e := range endpoints { - if i >= 5 { - // Try a maximum of 5 endpoints, break out (returning false) if we run of attempts. - break - } + // Once any goroutine detects a captive portal, we shut down the others. + ctx, cancel := context.WithCancel(ctx) + defer cancel() + + for _, e := range endpoints { wg.Add(1) go func(endpoint Endpoint) { defer wg.Done() found, err := d.verifyCaptivePortalEndpoint(ctx, endpoint, ifIndex) if err != nil { - d.logf("[v1] checkCaptivePortalEndpoint failed with endpoint %v: %v", endpoint, err) + if ctx.Err() == nil { + d.logf("[v1] checkCaptivePortalEndpoint failed with endpoint %v: %v", endpoint, err) + } return } if found { + cancel() // one match is good enough resultCh <- true } }(e) diff --git a/net/captivedetection/captivedetection_test.go b/net/captivedetection/captivedetection_test.go index e74273afd..29a197d31 100644 --- a/net/captivedetection/captivedetection_test.go +++ b/net/captivedetection/captivedetection_test.go @@ -7,10 +7,12 @@ import ( "context" "runtime" "sync" + "sync/atomic" "testing" - "tailscale.com/cmd/testwrapper/flakytest" "tailscale.com/net/netmon" + "tailscale.com/syncs" + "tailscale.com/tstest/nettest" ) func TestAvailableEndpointsAlwaysAtLeastTwo(t *testing.T) { @@ -36,25 +38,46 @@ func TestDetectCaptivePortalReturnsFalse(t *testing.T) { } } -func TestAllEndpointsAreUpAndReturnExpectedResponse(t *testing.T) { - flakytest.Mark(t, "https://github.com/tailscale/tailscale/issues/13019") +func TestEndpointsAreUpAndReturnExpectedResponse(t *testing.T) { + nettest.SkipIfNoNetwork(t) + d := NewDetector(t.Logf) endpoints := availableEndpoints(nil, 0, t.Logf, runtime.GOOS) + t.Logf("testing %d endpoints", len(endpoints)) + + ctx, cancel := context.WithCancel(context.Background()) + defer cancel() + var good atomic.Bool var wg sync.WaitGroup + sem := syncs.NewSemaphore(5) for _, e := range endpoints { wg.Add(1) go func(endpoint Endpoint) { defer wg.Done() - found, err := d.verifyCaptivePortalEndpoint(context.Background(), endpoint, 0) - if err != nil { - t.Errorf("verifyCaptivePortalEndpoint failed with endpoint %v: %v", endpoint, err) + + if !sem.AcquireContext(ctx) { + return + } + defer sem.Release() + + found, err := d.verifyCaptivePortalEndpoint(ctx, endpoint, 0) + if err != nil && ctx.Err() == nil { + t.Logf("verifyCaptivePortalEndpoint failed with endpoint %v: %v", endpoint, err) } if found { - t.Errorf("verifyCaptivePortalEndpoint with endpoint %v says we're behind a captive portal, but we aren't", endpoint) + t.Logf("verifyCaptivePortalEndpoint with endpoint %v says we're behind a captive portal, but we aren't", endpoint) + return } + good.Store(true) + t.Logf("endpoint good: %v", endpoint) + cancel() }(e) } wg.Wait() + + if !good.Load() { + t.Errorf("no good endpoints found") + } }