mirrors
/
tailscale
mirror of https://github.com/tailscale/tailscale.git
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

net/netcheck, wgengine/magicsock: make netmon.Monitor required 

This has been a TODO for ages. Time to do it.

The goal is to move more network state accessors to netmon.Monitor
where they can be cheaper/cached.

Updates tailscale/corp#10910
Updates tailscale/corp#18960
Updates #7967
Updates #3299

Change-Id: I60fc6508cd2d8d079260bda371fc08b6318bcaf1
Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>
This commit is contained in:
Brad Fitzpatrick 2024-04-26 18:28:01 -07:00
parent 4dece0c359
commit 8d3fa880be
5 changed files with 78 additions and 67 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

1
cmd/tailscale/cli/netcheck.go
View File

@ -53,6 +53,7 @@ func runNetcheck(ctx context.Context, args []string) error {
return err
}
c := &netcheck.Client{
NetMon: netMon,
PortMapper: portmapper.NewClient(logf, netMon, nil, nil, nil),
UseDNSCache: false, // always resolve, don't cache
}

28
net/netcheck/netcheck.go
View File

@ -159,6 +159,11 @@ func cloneDurationMap(m map[int]time.Duration) map[int]time.Duration {
// active probes, and must receive STUN packet replies via ReceiveSTUNPacket.
// Client can be used in a standalone fashion via the Standalone method.
type Client struct {
// NetMon is the netmon.Monitor to use to get the current
// (cached) network interface.
// It must be non-nil.
NetMon *netmon.Monitor
// Verbose enables verbose logging.
Verbose bool
@ -166,13 +171,6 @@ type Client struct {
// If nil, log.Printf is used.
Logf logger.Logf
// NetMon optionally provides a netmon.Monitor to use to get the current
// (cached) network interface.
// If nil, the interface will be looked up dynamically.
// TODO(bradfitz): make NetMon required. As of 2023-08-01, it basically always is
// present anyway.
NetMon *netmon.Monitor
// TimeNow, if non-nil, is used instead of time.Now.
TimeNow func() time.Time
@ -781,6 +779,9 @@ func (c *Client) GetReport(ctx context.Context, dm *tailcfg.DERPMap, opts *GetRe
if dm == nil {
return nil, errors.New("netcheck: GetReport: DERP map is nil")
}
if c.NetMon == nil {
return nil, errors.New("netcheck: GetReport: Client.NetMon is nil")
}
c.mu.Lock()
if c.curState != nil {
@ -844,18 +845,7 @@ func (c *Client) GetReport(ctx context.Context, dm *tailcfg.DERPMap, opts *GetRe
return c.finishAndStoreReport(rs, dm), nil
}
var ifState *interfaces.State
if c.NetMon == nil {
directState, err := interfaces.GetState()
if err != nil {
c.logf("[v1] interfaces: %v", err)
return nil, err
} else {
ifState = directState
}
} else {
ifState = c.NetMon.InterfaceState()
}
ifState := c.NetMon.InterfaceState()
// See if IPv6 works at all, or if it's been hard disabled at the
// OS level.

38
net/netcheck/netcheck_test.go
View File

@ -19,10 +19,12 @@ import (
"time"
"tailscale.com/net/interfaces"
"tailscale.com/net/netmon"
"tailscale.com/net/stun"
"tailscale.com/net/stun/stuntest"
"tailscale.com/tailcfg"
"tailscale.com/tstest"
"tailscale.com/types/logger"
)
func TestHairpinSTUN(t *testing.T) {
@ -154,13 +156,25 @@ func TestHairpinWait(t *testing.T) {
})
}
func newTestClient(t testing.TB) *Client {
netMon, err := netmon.New(logger.WithPrefix(t.Logf, "... netmon: "))
if err != nil {
t.Fatalf("netmon.New: %v", err)
}
t.Cleanup(func() { netMon.Close() })
c := &Client{
NetMon: netMon,
Logf: t.Logf,
}
return c
}
func TestBasic(t *testing.T) {
stunAddr, cleanup := stuntest.Serve(t)
defer cleanup()
c := &Client{
Logf: t.Logf,
}
c := newTestClient(t)
ctx, cancel := context.WithTimeout(context.Background(), 1*time.Second)
defer cancel()
@ -202,9 +216,8 @@ func TestWorksWhenUDPBlocked(t *testing.T) {
dm := stuntest.DERPMapOf(stunAddr)
dm.Regions[1].Nodes[0].STUNOnly = true
c := &Client{
Logf: t.Logf,
}
c := newTestClient(t)
ctx, cancel := context.WithTimeout(context.Background(), 250*time.Millisecond)
defer cancel()
@ -895,14 +908,11 @@ func TestNoCaptivePortalWhenUDP(t *testing.T) {
stunAddr, cleanup := stuntest.Serve(t)
defer cleanup()
c := &Client{
Logf: t.Logf,
testEnoughRegions: 1,
// Set the delay long enough that we have time to cancel it
// when our STUN probe succeeds.
testCaptivePortalDelay: 10 * time.Second,
}
c := newTestClient(t)
c.testEnoughRegions = 1
// Set the delay long enough that we have time to cancel it
// when our STUN probe succeeds.
c.testCaptivePortalDelay = 10 * time.Second
ctx, cancel := context.WithTimeout(context.Background(), 1*time.Second)
defer cancel()

12
wgengine/magicsock/magicsock.go
View File

@ -90,7 +90,7 @@ type Conn struct {
idleFunc func() time.Duration // nil means unknown
testOnlyPacketListener nettype.PacketListener
noteRecvActivity func(key.NodePublic) // or nil, see Options.NoteRecvActivity
netMon *netmon.Monitor // or nil
netMon *netmon.Monitor // must be non-nil
health *health.Tracker // or nil
controlKnobs *controlknobs.Knobs // or nil
@ -370,7 +370,7 @@ type Options struct {
NoteRecvActivity func(key.NodePublic)
// NetMon is the network monitor to use.
// If nil, the portmapper won't be used.
// It must be non-nil.
NetMon *netmon.Monitor
// HealthTracker optionally specifies the health tracker to
@ -451,6 +451,10 @@ func newConn() *Conn {
// As the set of possible endpoints for a Conn changes, the
// callback opts.EndpointsFunc is called.
func NewConn(opts Options) (*Conn, error) {
if opts.NetMon == nil {
return nil, errors.New("magicsock.Options.NetMon must be non-nil")
}
c := newConn()
c.port.Store(uint32(opts.Port))
c.controlKnobs = opts.ControlKnobs
@ -464,9 +468,7 @@ func NewConn(opts Options) (*Conn, error) {
DisableAll: func() bool { return opts.DisablePortMapper || c.onlyTCP443.Load() },
}
c.portMapper = portmapper.NewClient(logger.WithPrefix(c.logf, "portmapper: "), opts.NetMon, portMapOpts, opts.ControlKnobs, c.onPortMapChanged)
if opts.NetMon != nil {
c.portMapper.SetGatewayLookupFunc(opts.NetMon.GatewayAndSelfIP)
}
c.portMapper.SetGatewayLookupFunc(opts.NetMon.GatewayAndSelfIP)
c.netMon = opts.NetMon
c.health = opts.HealthTracker
c.onPortUpdate = opts.OnPortUpdate

66
wgengine/magicsock/magicsock_test.go
View File

@ -47,6 +47,7 @@ import (
"tailscale.com/net/connstats"
"tailscale.com/net/netaddr"
"tailscale.com/net/netcheck"
"tailscale.com/net/netmon"
"tailscale.com/net/packet"
"tailscale.com/net/ping"
"tailscale.com/net/stun/stuntest"
@ -155,6 +156,7 @@ type magicStack struct {
tsTun *tstun.Wrapper // wrapped tun that implements filtering and wgengine hooks
dev *device.Device // the wireguard-go Device that connects the previous things
wgLogger *wglog.Logger // wireguard-go log wrapper
netMon *netmon.Monitor // always non-nil
}
// newMagicStack builds and initializes an idle magicsock and
@ -168,8 +170,14 @@ func newMagicStack(t testing.TB, logf logger.Logf, l nettype.PacketListener, der
func newMagicStackWithKey(t testing.TB, logf logger.Logf, l nettype.PacketListener, derpMap *tailcfg.DERPMap, privateKey key.NodePrivate) *magicStack {
t.Helper()
netMon, err := netmon.New(logf)
if err != nil {
t.Fatalf("netmon.New: %v", err)
}
epCh := make(chan []tailcfg.Endpoint, 100) // arbitrary
conn, err := NewConn(Options{
NetMon: netMon,
Logf: logf,
DisablePortMapper: true,
TestOnlyPacketListener: l,
@ -211,6 +219,7 @@ func newMagicStackWithKey(t testing.TB, logf logger.Logf, l nettype.PacketListen
tsTun: tsTun,
dev: dev,
wgLogger: wgLogger,
netMon: netMon,
}
}
@ -228,6 +237,7 @@ func (s *magicStack) String() string {
func (s *magicStack) Close() {
s.dev.Close()
s.conn.Close()
s.netMon.Close()
}
func (s *magicStack) Public() key.NodePublic {
@ -372,6 +382,12 @@ func TestNewConn(t *testing.T) {
}
}
netMon, err := netmon.New(logger.WithPrefix(t.Logf, "... netmon: "))
if err != nil {
t.Fatalf("netmon.New: %v", err)
}
defer netMon.Close()
stunAddr, stunCleanupFn := stuntest.Serve(t)
defer stunCleanupFn()
@ -381,6 +397,7 @@ func TestNewConn(t *testing.T) {
DisablePortMapper: true,
EndpointsFunc: epFunc,
Logf: t.Logf,
NetMon: netMon,
})
if err != nil {
t.Fatal(err)
@ -497,9 +514,16 @@ func TestDeviceStartStop(t *testing.T) {
tstest.PanicOnLog()
tstest.ResourceCheck(t)
netMon, err := netmon.New(logger.WithPrefix(t.Logf, "... netmon: "))
if err != nil {
t.Fatalf("netmon.New: %v", err)
}
defer netMon.Close()
conn, err := NewConn(Options{
EndpointsFunc: func(eps []tailcfg.Endpoint) {},
Logf: t.Logf,
NetMon: netMon,
})
if err != nil {
t.Fatal(err)
@ -1243,7 +1267,15 @@ func Test32bitAlignment(t *testing.T) {
func newTestConn(t testing.TB) *Conn {
t.Helper()
port := pickPort(t)
netMon, err := netmon.New(logger.WithPrefix(t.Logf, "... netmon: "))
if err != nil {
t.Fatalf("netmon.New: %v", err)
}
t.Cleanup(func() { netMon.Close() })
conn, err := NewConn(Options{
NetMon: netMon,
DisablePortMapper: true,
Logf: t.Logf,
Port: port,
@ -3145,48 +3177,24 @@ func TestMaybeRebindOnError(t *testing.T) {
tstest.PanicOnLog()
tstest.ResourceCheck(t)
t.Run("darwin should rebind", func(t *testing.T) {
conn, err := NewConn(Options{
EndpointsFunc: func(eps []tailcfg.Endpoint) {},
Logf: t.Logf,
})
if err != nil {
t.Fatal(err)
}
defer conn.Close()
conn := newTestConn(t)
defer conn.Close()
t.Run("darwin-rebind", func(t *testing.T) {
rebound := conn.maybeRebindOnError("darwin", syscall.EPERM)
if !rebound {
t.Errorf("darwin should rebind on syscall.EPERM")
}
})
t.Run("linux should not rebind", func(t *testing.T) {
conn, err := NewConn(Options{
EndpointsFunc: func(eps []tailcfg.Endpoint) {},
Logf: t.Logf,
})
if err != nil {
t.Fatal(err)
}
defer conn.Close()
t.Run("linux-not-rebind", func(t *testing.T) {
rebound := conn.maybeRebindOnError("linux", syscall.EPERM)
if rebound {
t.Errorf("linux should not rebind on syscall.EPERM")
}
})
t.Run("should not rebind if recently rebind recently performed", func(t *testing.T) {
conn, err := NewConn(Options{
EndpointsFunc: func(eps []tailcfg.Endpoint) {},
Logf: t.Logf,
})
if err != nil {
t.Fatal(err)
}
defer conn.Close()
t.Run("no-frequent-rebind", func(t *testing.T) {
conn.lastEPERMRebind.Store(time.Now().Add(-1 * time.Second))
rebound := conn.maybeRebindOnError("darwin", syscall.EPERM)
if rebound {