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 <mihai@tailscale.com>
This commit is contained in:
Mihai Parparita 2023-02-03 16:47:51 -08:00 committed by Mihai Parparita
parent b6908181ff
commit 0e3fb91a39
3 changed files with 1 additions and 74 deletions

View File

@ -45,9 +45,7 @@ func TestDoH(t *testing.T) {
t.Fatal("no known DoH") t.Fatal("no known DoH")
} }
f := &forwarder{ f := &forwarder{}
dohSem: make(chan struct{}, 10),
}
for _, urlBase := range prefixes { for _, urlBase := range prefixes {
t.Run(urlBase, func(t *testing.T) { t.Run(urlBase, func(t *testing.T) {

View File

@ -15,16 +15,13 @@
"net/http" "net/http"
"net/netip" "net/netip"
"net/url" "net/url"
"runtime"
"sort" "sort"
"strconv"
"strings" "strings"
"sync" "sync"
"time" "time"
dns "golang.org/x/net/dns/dnsmessage" dns "golang.org/x/net/dns/dnsmessage"
"tailscale.com/envknob" "tailscale.com/envknob"
"tailscale.com/hostinfo"
"tailscale.com/net/dns/publicdns" "tailscale.com/net/dns/publicdns"
"tailscale.com/net/dnscache" "tailscale.com/net/dnscache"
"tailscale.com/net/neterror" "tailscale.com/net/neterror"
@ -181,7 +178,6 @@ type forwarder struct {
linkMon *monitor.Mon linkMon *monitor.Mon
linkSel ForwardLinkSelector // TODO(bradfitz): remove this when tsdial.Dialer absorbs it linkSel ForwardLinkSelector // TODO(bradfitz): remove this when tsdial.Dialer absorbs it
dialer *tsdial.Dialer dialer *tsdial.Dialer
dohSem chan struct{}
ctx context.Context // good until Close ctx context.Context // good until Close
ctxCancel context.CancelFunc // closes ctx ctxCancel context.CancelFunc // closes ctx
@ -209,36 +205,12 @@ func init() {
rand.Seed(time.Now().UnixNano()) 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 { func newForwarder(logf logger.Logf, linkMon *monitor.Mon, linkSel ForwardLinkSelector, dialer *tsdial.Dialer) *forwarder {
f := &forwarder{ f := &forwarder{
logf: logger.WithPrefix(logf, "forward: "), logf: logger.WithPrefix(logf, "forward: "),
linkMon: linkMon, linkMon: linkMon,
linkSel: linkSel, linkSel: linkSel,
dialer: dialer, dialer: dialer,
dohSem: make(chan struct{}, maxDoHInFlight(runtime.GOOS)),
} }
f.ctx, f.ctxCancel = context.WithCancel(context.Background()) f.ctx, f.ctxCancel = context.WithCancel(context.Background())
return f return f
@ -433,22 +405,7 @@ func (f *forwarder) getKnownDoHClientForProvider(urlBase string) (c *http.Client
const dohType = "application/dns-message" 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) { 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) metricDNSFwdDoH.Add(1)
req, err := http.NewRequestWithContext(ctx, "POST", urlBase, bytes.NewReader(packet)) req, err := http.NewRequestWithContext(ctx, "POST", urlBase, bytes.NewReader(packet))
if err != nil { if err != nil {

View File

@ -12,7 +12,6 @@
"time" "time"
dns "golang.org/x/net/dns/dnsmessage" dns "golang.org/x/net/dns/dnsmessage"
"tailscale.com/hostinfo"
"tailscale.com/types/dnstype" "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") var testDNS = flag.Bool("test-dns", false, "run tests that require a working DNS server")
func TestGetKnownDoHClientForProvider(t *testing.T) { func TestGetKnownDoHClientForProvider(t *testing.T) {