We use it a number of places in different repos. Might as well make
one. Another use is coming.
Updates #cleanup
Change-Id: Ib7ce38de0db35af998171edee81ca875102349a4
Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>
I noticed that failed tests were leaving aroudn stray tailscaled processes
on macOS at least.
To repro, add this to tstest/integration:
func TestFailInFewSeconds(t *testing.T) {
t.Parallel()
time.Sleep(3 * time.Second)
os.Exit(1)
t.Fatal("boom")
}
Those three seconds let the other parallel tests (with all their
tailscaled child processes) start up and start running their tests,
but then we violently os.Exit(1) the test driver and all the children
were kept alive (and were spinning away, using all available CPU in
gvisor scheduler code, which is a separate scary issue)
Updates #cleanup
Change-Id: I9c891ed1a1ec639fb2afec2808c04dbb8a460e0e
Co-authored-by: Maisem Ali <maisem@tailscale.com>
Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>
We want the overall state (used only for tests) to be computed from
the individual states of each component, rather than moving the state
around by hand in dozens of places.
In working towards that, we found a lot of things to clean up.
Updates #cleanup
Change-Id: Ieaaae5355dfae789a8ec7a56ce212f1d7e3a92db
Co-authored-by: Maisem Ali <maisem@tailscale.com>
Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>
During Shutdown of an ephemeral node, we called Logout (to best effort
delete the node earlier), which then called back into
resetForProfileChangeLockedOnEntry, which then tried to Start
again. That's all a waste of work during shutdown and complicates
other cleanups coming later.
Updates #cleanup
Change-Id: I0b8648cac492fc70fa97c4ebef919bbe352c5d7b
Co-authored-by: Maisem Ali <maisem@tailscale.com>
Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>
Don't just start goroutines and hope for them to be ordered.
Fixes potential regression from earlier 7074a40c0.
Updates #cleanup
Change-Id: I501a6f3e4e8e6306b958bccdc1e47869991c31f7
Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>
It was too aggressive before, as it only had the ill-defined "Major"
bool to work with. Now it can check more precisely.
Updates #9040
Change-Id: I20967283b64af6a9cad3f8e90cff406de91653b8
Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>
Drops time by several minutes.
Also, on top of that: skip building variant CLIs on the race builder
(29s), and getting qemu (15s).
Updates #9182
Change-Id: I979e02ab8c0daeebf5200459c9e4458a1f62f728
Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>
Then use the Locked variants in Shutdown while we already hold the lock.
Updates #cleanup
Change-Id: I367d53e6be6f37f783c8f43fc9c4d498d0adf501
Co-authored-by: Maisem Ali <maisem@tailscale.com>
Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>
* don't try to re-Start (and thus create a new client) during Shutdown
* in tests, wait for controlclient to fully shut down when replacing it
* log a bit more
Updates tailscale/corp#14139
Updates tailscale/corp#13175 etc
Updates #9178 and its flakes.
Change-Id: I3ed2440644dc157aa6e616fe36fbd29a6056846c
Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>
We have this in another repo and I wanted it here too.
Updates #cleanup
Change-Id: If93dc73f11eaaada5024acf2a885a153b88db5a0
Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>
Don't depend on the server to do it.
Updates #cleanup
Change-Id: I8ff40b02aa877155a71fd4db58cbecb872241ac8
Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>
They were entirely redundant and 1:1 with the status field
so this turns them into methods instead.
Updates #cleanup
Updates #1909
Change-Id: I7d939750749edf7dae4c97566bbeb99f2f75adbc
Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>
I'm trying to remove some stuff from the netmap update path.
Updates #1909
Change-Id: Iad2c728dda160cd52f33ef9cf0b75b4940e0ce64
Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>
This removes a lot of API from net/interfaces (including all the
filter types, EqualFiltered, active Tailscale interface func, etc) and
moves the "major" change detection to net/netmon which knows more
about the world and the previous/new states.
Updates #9040
Change-Id: I7fe66a23039c6347ae5458745b709e7ebdcce245
Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>
De-pointer a *time.Time type, move it after the mutex which guard is,
rename two test-only methods with our conventional "ForTest" suffix.
Updates #cleanup
Change-Id: I4f4d1acd9c2de33d9c3cb6465d7349ed051aa9f9
Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>
Named result meant error paths assigned that variable to nil.
But a goroutine was concurrently using that variable.
Don't use a named result for that first parameter. Then then return
paths don't overwrite it.
Fixes#9129
Change-Id: Ie57f99d40ca8110085097780686d9bd620aaf160
Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>
For now the method has only one interface (the same as the func it's
replacing) but it will grow, eventually with the goal to remove the
controlclient.Status type for most purposes.
Updates #1909
Change-Id: I715c8bf95e3f5943055a94e76af98d988558a2f2
Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>
I didn't see the race builder fail on CI earlier in 590c693b9.
This fixes the test.
Updates #greenci
Change-Id: I9f271bfadfc29b010226b55bf6647f35f03730b1
Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>
Printing out JSON representation things in log output is pretty common.
Updates #cleanup
Change-Id: Ife2d2e321a18e6e1185efa8b699a23061ac5e5a4
Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>
So even if the server doesn't support sending patches (neither the
Tailscale control server nor Headscale yet do), this makes the client
convert a changed node to its diff so the diffs can be processed
individually in a follow-up change.
This lets us make progress on #1909 without adding a dependency on
finishing the server-side part, and also means other control servers
will get the same upcoming optimizations.
And add some clientmetrics while here.
Updates #1909
Change-Id: I9533bcb8bba5227e17389f0b10dff71f33ee54ec
Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>
I'm not saying it works, but it compiles.
Updates #5794
Change-Id: I2f3c99732e67fe57a05edb25b758d083417f083e
Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>
Can write "wasm" instead of js || wasi1p, since there's only two:
$ go tool dist list | grep wasm
js/wasm
wasip1/wasm
Plus, if GOOS=wasip2 is added later, we're already set.
Updates #5794
Change-Id: Ifcfb187c3775c17c9141bc721512dc4577ac4434
Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>
Some OS-specific funcs were defined in init. Another used build tags
and required all other OSes to stub it out. Another one could just be in
the portable file.
Simplify it a bit, removing a file and some stubs in the process.
Updates #5794
Change-Id: I51df8772cc60a9335ac4c1dc0ab59b8a0d236961
Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>
We have a fancy package for doing TLS cert validation even if the machine
doesn't have TLS certs (for LetsEncrypt only) but the CLI's netcheck command
wasn't using it.
Also, update the tlsdial's outdated package docs while here.
Updates #cleanup
Change-Id: I74b3cb645d07af4d8ae230fb39a60c809ec129ad
Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>
This simplifies some netmon code in prep for other changes.
It breaks up Monitor.debounce into a helper method so locking is
easier to read and things unindent, and then it simplifies the polling
netmon implementation to remove the redundant stuff that the caller
(the Monitor.debounce loop) was already basically doing.
Updates #9040
Change-Id: Idcfb45201d00ae64017042a7bdee6ef86ad37a9f
Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>
Actually fixed in 77ff705545 but that was cherry-picked to a branch
and we don't bump capver in branches.
This tells the control plane that UPnP should be re-enabled going
forward.
Updates #8992
Change-Id: I5c4743eb52fdee94175668c368c0f712536dc26b
Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>
Upcoming work on incremental netmap change handling will require some
replumbing of which subsystems get notified about what. Done naively,
it could break "tailscale status --json" visibility later. To make sure
I understood the flow of all the updates I was rereading the status code
and realized parts of ipnstate.Status were being populated by the wrong
subsystems.
The engine (wireguard) and magicsock (data plane, NAT traveral) should
only populate the stuff that they uniquely know. The WireGuard bits
were fine but magicsock was populating stuff stuff that LocalBackend
could've better handled, so move it there.
Updates #1909
Change-Id: I6d1b95d19a2d1b70fbb3c875fac8ea1e169e8cb0
Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>
It was in SelfNode.Hostinfo anyway. The redundant copy was just
costing us an allocation per netmap (a Hostinfo.Clone).
Updates #1909
Change-Id: Ifac568aa5f8054d9419828489442a0f4559bc099
Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>
Now mapSession has a bunch more fields and methods, rather than being
just one massive func with a ton of local variables.
So far there are no major new optimizations, though. It should behave
the same as before.
This has been done with an eye towards testability (so tests can set
all the callback funcs as needed, or not, without a huge Direct client
or long-running HTTP requests), but this change doesn't add new tests
yet. That will follow in the changes which flesh out the NetmapUpdater
interface.
Updates #1909
Change-Id: Iad4e7442d5bbbe2614bd4b1dc4b02e27504898df
Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>
And flesh it out and use idiomatic doc style ("whether" for bools)
and end in a period while there anyway.
Updates #cleanup
Change-Id: Ieb82f13969656e2340c3510e7b102dc8e6932611
Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>
And optimize the Persist setting a bit, allocating later and only mutating
fields when there's been a Node change.
Updates #1909
Change-Id: Iaddfd9e88ef76e1d18e8d0a41926eb44d0955312
Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>
In b987b2ab18 (2021-01-12) when we introduced sharing we mapped
the sharer to the userid at a low layer, mostly to fix the display of
"tailscale status" and the client UIs, but also some tests.
The commit earlier today, 7dec09d169, removed the 2.5yo option
to let clients disable that automatic mapping, as clearly we were never
getting around to it.
This plumbs the Sharer UserID all the way to ipnstatus so the CLI
itself can choose to print out the Sharer's identity over the node's
original owner.
Then we stop mangling Node.User and let clients decide how they want
to render things.
To ease the migration for the Windows GUI (which currently operates on
tailcfg.Node via the NetMap from WatchIPNBus, instead of PeerStatus),
a new method Node.SharerOrUser is added to do the mapping of
Sharer-else-User.
Updates #1909
Updates tailscale/corp#1183
Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>
I screwed this up in 58a4fd43d as I expected. I even looked out for
cases like this (because this always happens) and I still missed
it. Vet doesn't flag these because they're not the standard printf
funcs it knows about. TODO: make our vet recognize all our
"logger.Logf" types.
Updates #8948
Change-Id: Iae267d5f81da49d0876b91c0e6dc451bf7dcd721
Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>
I'd added a test case of deephash against a tailcfg.Node to make sure
it worked at all more than anything. We don't care what the exact
bytes are in this test, just that it doesn't fail. So adjust for that.
Then when we make changes to tailcfg.Node and types under it, we don't
need to keep adjusting this test.
Updates #cleanup
Change-Id: Ibf4fa42820aeab8f5292fe65f9f92ffdb0b4407b
Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>
It was added 2.5 years ago in c1dabd9436 but was never used.
Clearly that migration didn't matter.
We can attempt this again later if/when this matters.
Meanwhile this simplifies the code and thus makes working on other
current efforts in these parts of the code easier.
Updates #1909
Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>
Make it just a views.Slice[netip.Prefix] instead of its own named type.
Having the special case led to circular dependencies in another WIP PR
of mine.
Updates #8948
Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>
This is basically https://github.com/bradfitz/iter which was
a joke but now that Go's adding range over int soonish, might
as well. It simplies our code elsewher that uses slice views.
Updates #8948
Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>
Now a nodeAttr: ForceBackgroundSTUN, DERPRoute, TrimWGConfig,
DisableSubnetsIfPAC, DisableUPnP.
Kept support for, but also now a NodeAttr: RandomizeClientPort.
Removed: SetForceBackgroundSTUN, SetRandomizeClientPort (both never
used, sadly... never got around to them. But nodeAttrs are better
anyway), EnableSilentDisco (will be a nodeAttr later when that effort
resumes).
Updates #8923
Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>
No need to have it on Auto or be behind a mutex; it's only read/written
from a single goroutine. Move it there.
Updates tailscale/corp#5761
Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>
See issue. This is a baby step towards passing through deltas
end-to-end from node to control back to node and down to the various
engine subsystems, not computing diffs from two full netmaps at
various levels. This will then let us support larger netmaps without
burning CPU.
But this change itself changes no behavior. It just changes a func
type to an interface with one method. That paves the way for future
changes to then add new NetmapUpdater methods that do more
fine-grained work than updating the whole world.
Updates #1909
Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>
This removes the unsafe/linkname and only uses the standard library.
It's a bit slower, for now, but https://go.dev/cl/518336 should get us
back.
On darwin/arm64, without https://go.dev/cl/518336
pkg: tailscale.com/tstime/mono
│ before │ after │
│ sec/op │ sec/op vs base │
MonoNow-8 16.20n ± 0% 19.75n ± 0% +21.92% (p=0.000 n=10)
TimeNow-8 39.46n ± 0% 39.40n ± 0% -0.16% (p=0.002 n=10)
geomean 25.28n 27.89n +10.33%
And with it,
MonoNow-8 16.34n ± 1% 16.93n ± 0% +3.67% (p=0.001 n=10)
TimeNow-8 39.55n ± 15% 38.46n ± 1% -2.76% (p=0.000 n=10)
geomean 25.42n 25.52n +0.41%
Updates #8839
Updates tailscale/go#70
Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>
Go style is for error variables to start with "err" (or "Err")
and for error types to end in "Error".
Updates #cleanup
Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>
Rather than make each ipn.StateStore implementation guard against
useless writes (a write of the same value that's already in the
store), do writes via a new wrapper that has a fast path for the
unchanged case.
This then fixes profileManager's flood of useless writes to AWS SSM,
etc.
Updates #8785
Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>
Like net/http.Server.BaseContext, this lets callers specify a base
context for dials.
Updates tailscale/corp#12702
Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>
The util/linuxfw/iptables.go had a bunch of code that wasn't yet used
(in prep for future work) but because of its imports, ended up
initializing code deep within gvisor that panicked on init on arm64
systems not using 4KB pages.
This deletes the unused code to delete the imports and remove the
panic. We can then cherry-pick this back to the branch and restore it
later in a different way.
A new test makes sure we don't regress in the future by depending on
the panicking package in question.
Fixes#8658
Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>
We were storing a lot of "ExitNodeFilteredSet":null in the database.
Updates tailscale/corp#1818 (found in the process)
Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>
The server hasn't sent it in ages.
Updates #cleanup
Change-Id: I9695ab0f074ec6fb006e11faf3cdfc5ca049fbf8
Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>
ScrubbedGoroutineDump previously only returned the stacks of all
goroutines. I also want to be able to use this for only the current
goroutine's stack. Add a bool param to support both ways.
Updates tailscale/corp#5149
Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>
