Skip to content

Commit

Permalink
wgengine/magicsock: disable portmapper in tunchan-faked tests
Browse files Browse the repository at this point in the history
Most of the magicsock tests fake the network, simulating packets going
out and coming in. There's no reason to actually hit your router to do
UPnP/NAT-PMP/PCP during in tests. But while debugging thousands of
iterations of tests to deflake some things, I saw it slamming my
router. This stops that.

Updates #11762

Change-Id: I59b9f48f8f5aff1fa16b4935753d786342e87744
Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>
  • Loading branch information
bradfitz committed Apr 18, 2024
1 parent 22bd506 commit 03d5d1f
Show file tree
Hide file tree
Showing 3 changed files with 14 additions and 5 deletions.
4 changes: 3 additions & 1 deletion tstest/log.go
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,9 @@ func (panicLogWriter) Write(b []byte) (int, error) {
// interfaces.GetState & tshttpproxy code to allow pushing
// down a Logger yet. TODO(bradfitz): do that refactoring once
// 1.2.0 is out.
if bytes.Contains(b, []byte("tshttpproxy: ")) || bytes.Contains(b, []byte("runtime/panic.go:")) {
if bytes.Contains(b, []byte("tshttpproxy: ")) ||
bytes.Contains(b, []byte("runtime/panic.go:")) ||
bytes.Contains(b, []byte("XXX")) {
os.Stderr.Write(b)
return len(b), nil
}
Expand Down
6 changes: 5 additions & 1 deletion wgengine/magicsock/magicsock.go
Original file line number Diff line number Diff line change
Expand Up @@ -384,6 +384,10 @@ type Options struct {
// WireGuard state by its public key. If nil, it's not used.
// In regular use, this will be wgengine.(*userspaceEngine).PeerByKey.
PeerByKeyFunc func(key.NodePublic) (_ wgint.Peer, ok bool)

// DisablePortMapper, if true, disables the portmapper.
// This is primarily useful in tests.
DisablePortMapper bool
}

func (o *Options) logf() logger.Logf {
Expand Down Expand Up @@ -452,7 +456,7 @@ func NewConn(opts Options) (*Conn, error) {
c.testOnlyPacketListener = opts.TestOnlyPacketListener
c.noteRecvActivity = opts.NoteRecvActivity
portMapOpts := &portmapper.DebugKnobs{
DisableAll: func() bool { return c.onlyTCP443.Load() },
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 {
Expand Down
9 changes: 6 additions & 3 deletions wgengine/magicsock/magicsock_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -171,6 +171,7 @@ func newMagicStackWithKey(t testing.TB, logf logger.Logf, l nettype.PacketListen
epCh := make(chan []tailcfg.Endpoint, 100) // arbitrary
conn, err := NewConn(Options{
Logf: logf,
DisablePortMapper: true,
TestOnlyPacketListener: l,
EndpointsFunc: func(eps []tailcfg.Endpoint) {
epCh <- eps
Expand Down Expand Up @@ -376,9 +377,10 @@ func TestNewConn(t *testing.T) {

port := pickPort(t)
conn, err := NewConn(Options{
Port: port,
EndpointsFunc: epFunc,
Logf: t.Logf,
Port: port,
DisablePortMapper: true,
EndpointsFunc: epFunc,
Logf: t.Logf,
})
if err != nil {
t.Fatal(err)
Expand Down Expand Up @@ -1242,6 +1244,7 @@ func newTestConn(t testing.TB) *Conn {
t.Helper()
port := pickPort(t)
conn, err := NewConn(Options{
DisablePortMapper: true,
Logf: t.Logf,
Port: port,
TestOnlyPacketListener: localhostListener{},
Expand Down

0 comments on commit 03d5d1f

Please sign in to comment.