From 0e3fb91a39613888f910f62a61d9d01925471bec Mon Sep 17 00:00:00 2001 From: Mihai Parparita Date: Fri, 3 Feb 2023 16:47:51 -0800 Subject: [PATCH] net/dns/resolver: remove maxDoHInFlight It was originally added to control memory use on iOS (#2490), but then was relaxed conditionally when running on iOS 15 (#3098). Now that we require iOS 15, there's no need for the limit at all, so simplify back to the original state. Signed-off-by: Mihai Parparita --- net/dns/resolver/doh_test.go | 4 +-- net/dns/resolver/forwarder.go | 43 ------------------------------ net/dns/resolver/forwarder_test.go | 28 ------------------- 3 files changed, 1 insertion(+), 74 deletions(-) diff --git a/net/dns/resolver/doh_test.go b/net/dns/resolver/doh_test.go index 3b651e7f1..a9c284761 100644 --- a/net/dns/resolver/doh_test.go +++ b/net/dns/resolver/doh_test.go @@ -45,9 +45,7 @@ func TestDoH(t *testing.T) { t.Fatal("no known DoH") } - f := &forwarder{ - dohSem: make(chan struct{}, 10), - } + f := &forwarder{} for _, urlBase := range prefixes { t.Run(urlBase, func(t *testing.T) { diff --git a/net/dns/resolver/forwarder.go b/net/dns/resolver/forwarder.go index fd7437198..8bd9b719d 100644 --- a/net/dns/resolver/forwarder.go +++ b/net/dns/resolver/forwarder.go @@ -15,16 +15,13 @@ import ( "net/http" "net/netip" "net/url" - "runtime" "sort" - "strconv" "strings" "sync" "time" dns "golang.org/x/net/dns/dnsmessage" "tailscale.com/envknob" - "tailscale.com/hostinfo" "tailscale.com/net/dns/publicdns" "tailscale.com/net/dnscache" "tailscale.com/net/neterror" @@ -181,7 +178,6 @@ type forwarder struct { linkMon *monitor.Mon linkSel ForwardLinkSelector // TODO(bradfitz): remove this when tsdial.Dialer absorbs it dialer *tsdial.Dialer - dohSem chan struct{} ctx context.Context // good until Close ctxCancel context.CancelFunc // closes ctx @@ -209,36 +205,12 @@ func init() { rand.Seed(time.Now().UnixNano()) } -func maxDoHInFlight(goos string) int { - if goos != "ios" { - return 1000 // effectively unlimited - } - // iOS < 15 limits the memory to 15MB for NetworkExtensions. - // iOS >= 15 gives us 50MB. - // See: https://tailscale.com/blog/go-linker/ - ver := hostinfo.GetOSVersion() - if ver == "" { - // Unknown iOS version, be cautious. - return 10 - } - major, _, ok := strings.Cut(ver, ".") - if !ok { - // Unknown iOS version, be cautious. - return 10 - } - if m, err := strconv.Atoi(major); err != nil || m < 15 { - return 10 - } - return 1000 -} - func newForwarder(logf logger.Logf, linkMon *monitor.Mon, linkSel ForwardLinkSelector, dialer *tsdial.Dialer) *forwarder { f := &forwarder{ logf: logger.WithPrefix(logf, "forward: "), linkMon: linkMon, linkSel: linkSel, dialer: dialer, - dohSem: make(chan struct{}, maxDoHInFlight(runtime.GOOS)), } f.ctx, f.ctxCancel = context.WithCancel(context.Background()) return f @@ -433,22 +405,7 @@ func (f *forwarder) getKnownDoHClientForProvider(urlBase string) (c *http.Client const dohType = "application/dns-message" -func (f *forwarder) releaseDoHSem() { <-f.dohSem } - func (f *forwarder) sendDoH(ctx context.Context, urlBase string, c *http.Client, packet []byte) ([]byte, error) { - // Bound the number of HTTP requests in flight. This primarily - // matters for iOS where we're very memory constrained and - // HTTP requests are heavier on iOS where we don't include - // HTTP/2 for binary size reasons (as binaries on iOS linked - // with Go code cost memory proportional to the binary size, - // for reasons not fully understood). - select { - case f.dohSem <- struct{}{}: - case <-ctx.Done(): - return nil, ctx.Err() - } - defer f.releaseDoHSem() - metricDNSFwdDoH.Add(1) req, err := http.NewRequestWithContext(ctx, "POST", urlBase, bytes.NewReader(packet)) if err != nil { diff --git a/net/dns/resolver/forwarder_test.go b/net/dns/resolver/forwarder_test.go index e11d6afad..dad165fe7 100644 --- a/net/dns/resolver/forwarder_test.go +++ b/net/dns/resolver/forwarder_test.go @@ -12,7 +12,6 @@ import ( "time" dns "golang.org/x/net/dns/dnsmessage" - "tailscale.com/hostinfo" "tailscale.com/types/dnstype" ) @@ -143,33 +142,6 @@ func TestGetRCode(t *testing.T) { } } -func TestMaxDoHInFlight(t *testing.T) { - tests := []struct { - goos string - ver string - want int - }{ - {"ios", "", 10}, - {"ios", "1532", 10}, - {"ios", "9.3.2", 10}, - {"ios", "14.3.2", 10}, - {"ios", "15.3.2", 1000}, - {"ios", "20.3.2", 1000}, - {"android", "", 1000}, - {"darwin", "", 1000}, - {"linux", "", 1000}, - } - for _, tc := range tests { - t.Run(fmt.Sprintf("%s-%s", tc.goos, tc.ver), func(t *testing.T) { - hostinfo.SetOSVersion(tc.ver) - got := maxDoHInFlight(tc.goos) - if got != tc.want { - t.Errorf("got %d; want %d", got, tc.want) - } - }) - } -} - var testDNS = flag.Bool("test-dns", false, "run tests that require a working DNS server") func TestGetKnownDoHClientForProvider(t *testing.T) {