all: avoid repeated default interface lookups
On some platforms (notably macOS and iOS) we look up the default interface to bind outgoing connections to. This is both duplicated work and results in logspam when the default interface is not available (i.e. when a phone has no connectivity, we log an error and thus cause more things that we will try to upload and fail). Fixed by passing around a netmon.Monitor to more places, so that we can use its cached interface state. Fixes #7850 Updates #7621 Signed-off-by: Mihai Parparita <mihai@tailscale.com>
This commit is contained in:
committed by
Mihai Parparita
parent
7f17e04a5a
commit
7330aa593e
@@ -249,7 +249,7 @@ func (d *TestIGD) handlePCPQuery(pkt []byte, src netip.AddrPort) {
|
||||
|
||||
func newTestClient(t *testing.T, igd *TestIGD) *Client {
|
||||
var c *Client
|
||||
c = NewClient(t.Logf, nil, func() {
|
||||
c = NewClient(t.Logf, nil, nil, func() {
|
||||
t.Logf("port map changed")
|
||||
t.Logf("have mapping: %v", c.HaveMapping())
|
||||
})
|
||||
|
||||
@@ -21,6 +21,7 @@ import (
|
||||
"tailscale.com/net/interfaces"
|
||||
"tailscale.com/net/netaddr"
|
||||
"tailscale.com/net/neterror"
|
||||
"tailscale.com/net/netmon"
|
||||
"tailscale.com/net/netns"
|
||||
"tailscale.com/net/sockstats"
|
||||
"tailscale.com/types/logger"
|
||||
@@ -59,6 +60,7 @@ const trustServiceStillAvailableDuration = 10 * time.Minute
|
||||
// Client is a port mapping client.
|
||||
type Client struct {
|
||||
logf logger.Logf
|
||||
netMon *netmon.Monitor // optional; nil means interfaces will be looked up on-demand
|
||||
ipAndGateway func() (gw, ip netip.Addr, ok bool)
|
||||
onChange func() // or nil
|
||||
debug DebugKnobs
|
||||
@@ -153,15 +155,19 @@ func (m *pmpMapping) Release(ctx context.Context) {
|
||||
|
||||
// NewClient returns a new portmapping client.
|
||||
//
|
||||
// The netMon parameter is optional; if non-nil it's used to do faster interface
|
||||
// lookups.
|
||||
//
|
||||
// The debug argument allows configuring the behaviour of the portmapper for
|
||||
// debugging; if nil, a sensible set of defaults will be used.
|
||||
//
|
||||
// The optional onChange argument specifies a func to run in a new
|
||||
// goroutine whenever the port mapping status has changed. If nil,
|
||||
// it doesn't make a callback.
|
||||
func NewClient(logf logger.Logf, debug *DebugKnobs, onChange func()) *Client {
|
||||
func NewClient(logf logger.Logf, netMon *netmon.Monitor, debug *DebugKnobs, onChange func()) *Client {
|
||||
ret := &Client{
|
||||
logf: logf,
|
||||
netMon: netMon,
|
||||
ipAndGateway: interfaces.LikelyHomeRouterIP,
|
||||
onChange: onChange,
|
||||
}
|
||||
@@ -271,7 +277,7 @@ func (c *Client) listenPacket(ctx context.Context, network, addr string) (nettyp
|
||||
}
|
||||
return pc.(*net.UDPConn), nil
|
||||
}
|
||||
pc, err := netns.Listener(c.logf).ListenPacket(ctx, network, addr)
|
||||
pc, err := netns.Listener(c.logf, c.netMon).ListenPacket(ctx, network, addr)
|
||||
if err != nil {
|
||||
return nil, err
|
||||
}
|
||||
|
||||
@@ -16,7 +16,7 @@ func TestCreateOrGetMapping(t *testing.T) {
|
||||
if v, _ := strconv.ParseBool(os.Getenv("HIT_NETWORK")); !v {
|
||||
t.Skip("skipping test without HIT_NETWORK=1")
|
||||
}
|
||||
c := NewClient(t.Logf, nil, nil)
|
||||
c := NewClient(t.Logf, nil, nil, nil)
|
||||
defer c.Close()
|
||||
c.SetLocalPort(1234)
|
||||
for i := 0; i < 2; i++ {
|
||||
@@ -32,7 +32,7 @@ func TestClientProbe(t *testing.T) {
|
||||
if v, _ := strconv.ParseBool(os.Getenv("HIT_NETWORK")); !v {
|
||||
t.Skip("skipping test without HIT_NETWORK=1")
|
||||
}
|
||||
c := NewClient(t.Logf, nil, nil)
|
||||
c := NewClient(t.Logf, nil, nil, nil)
|
||||
defer c.Close()
|
||||
for i := 0; i < 3; i++ {
|
||||
if i > 0 {
|
||||
@@ -47,7 +47,7 @@ func TestClientProbeThenMap(t *testing.T) {
|
||||
if v, _ := strconv.ParseBool(os.Getenv("HIT_NETWORK")); !v {
|
||||
t.Skip("skipping test without HIT_NETWORK=1")
|
||||
}
|
||||
c := NewClient(t.Logf, nil, nil)
|
||||
c := NewClient(t.Logf, nil, nil, nil)
|
||||
defer c.Close()
|
||||
c.SetLocalPort(1234)
|
||||
res, err := c.Probe(context.Background())
|
||||
|
||||
@@ -237,7 +237,7 @@ func (c *Client) upnpHTTPClientLocked() *http.Client {
|
||||
if c.uPnPHTTPClient == nil {
|
||||
c.uPnPHTTPClient = &http.Client{
|
||||
Transport: &http.Transport{
|
||||
DialContext: netns.NewDialer(c.logf).DialContext,
|
||||
DialContext: netns.NewDialer(c.logf, c.netMon).DialContext,
|
||||
IdleConnTimeout: 2 * time.Second, // LAN is cheap
|
||||
},
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user