These aren't handled, but it's not an error to get one.
Fixes#6806
Signed-off-by: Andrew Dunham <andrew@du.nham.ca>
Change-Id: I1fcb9032ac36420aa72a048bf26f58360b9461f9
It's possible for the 'somethingChanged' callback to be registered and
then trigger before the ctx field is assigned; move the assignment
earlier so this can't happen.
Change-Id: Ia7ee8b937299014a083ab40adf31a8b3e0db4ec5
Signed-off-by: Andrew Dunham <andrew@du.nham.ca>
We use this pattern in a number of places (in this repo and elsewhere)
and I was about to add a fourth to this repo which was crossing the line.
Add this type instead so they're all the same.
Also, we have another Set type (SliceSet, which tracks its keys in
order) in another repo we can move to this package later.
Change-Id: Ibbdcdba5443fae9b6956f63990bdb9e9443cefa9
Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>
The //go:build syntax was introduced in Go 1.17:
https://go.dev/doc/go1.17#build-lines
gofmt has kept the +build and go:build lines in sync since
then, but enough time has passed. Time to remove them.
Done with:
perl -i -npe 's,^// \+build.*\n,,' $(git grep -l -F '+build')
Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>
One current theory (among other things) on battery consumption is that
magicsock is resorting to using the IPv6 over LTE even on WiFi.
One thing that could explain this is that we do not get link change updates
for the LTE modem as we ignore them in this list.
This commit makes us not ignore changes to `pdp_ip` as a test.
Updates #3363
Signed-off-by: Maisem Ali <maisem@tailscale.com>
It unfortuantely gets truncated because it's too long, split it into 3
different log lines to circumvent truncation.
Signed-off-by: Maisem Ali <maisem@tailscale.com>
Currently we ignore these interfaces in the darwin osMon but then would consider it
interesting when checking if anything had changed.
Signed-off-by: Maisem Ali <maisem@tailscale.com>
In `(*Mon).Start` we don't run a timer to update `(*Mon).lastWall` on iOS and
Android as their sleep patterns are bespoke. However, in the debounce
goroutine we would notice that the the wall clock hadn't been updated
since the last event would assume that a time jump had occurred. This would
result in non-events being considered as major-change events.
This commit makes it so that `(*Mon).timeJumped` is never set to `true`
on iOS and Android.
Signed-off-by: Maisem Ali <maisem@tailscale.com>
Ignoring the events at this layer is the simpler path for right now, a
broader change should follow to suppress irrelevant change events in a
higher layer so as to avoid related problems with other monitoring paths
on other platforms. This approach may also carry a small risk that it
applies an at-most-once invariant low in the chain that could be assumed
otherwise higher in the code.
I adjusted the newAddrMessage type to include interface index rather
than a label, as labels are not always supplied, and in particular on my
test hosts they were consistently missing for ipv6 address messages.
I adjusted the newAddrMessage.Addr field to be populated from
Attributes.Address rather than Attributes.Local, as again for ipv6
.Local was always empty, and with ipv4 the .Address and .Local contained
the same contents in each of my test environments.
Update #4282
Signed-off-by: James Tucker <james@tailscale.com>
While I trust the test behavior, I also want to assert the behavior in a
reproduction environment, this envknob gives me the log information I
need to do so.
Update #4282
Signed-off-by: James Tucker <james@tailscale.com>
The behavior was changed in March (in 7f174e84e6)
but that change forgot to update these docs.
Change-Id: I79c0301692c1d13a4a26641cc5144baf48ec1360
Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>
One of the most common "unexpected" log lines is:
"network state changed, but stringification didn't"
One way that this can occur is if an interesting interface
(non-Tailscale, has interesting IP address)
gains or loses an uninteresting IP address (link local or loopback).
The fact that the interface is interesting is enough for EqualFiltered
to inspect it. The fact that an IP address changed is enough for
EqualFiltered to declare that the interfaces are not equal.
But the State.String method reasonably declines to print any
uninteresting IP addresses. As a result, the network state appears
to have changed, but the stringification did not.
The String method is correct; nothing interesting happened.
This change fixes this by adding an IP address filter to EqualFiltered
in addition to the interface filter. This lets the network monitor
ignore the addition/removal of uninteresting IP addresses.
Signed-off-by: Josh Bleecher Snyder <josh@tailscale.com>
The earlier 2ba36c294b started listening
for ip rule changes and only cared about DELRULE events, buts its subscription
included all rule events, including new ones, which meant we were then
catching our own ip rule creations and logging about how they were unknown.
Stop that log spam.
Updates #1591
Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>
For debugging & working on #1591 where certain versions of systemd-networkd
delete Tailscale's ip rule entries.
Updates #1591
Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>
Google Cloud Run does not implement NETLINK_ROUTE RTMGRP.
If initialization of the netlink socket or group membership
fails, fall back to a polling implementation.
Signed-off-by: Denton Gentry <dgentry@tailscale.com>
Now callers (wgengine/monitor) don't need to mutate the state to remove
boring interfaces before calling State.Equal. Instead, the methods
to remove boring interfaces from the State are removed, as is
the reflect-using Equal method itself, and in their place is
a new EqualFiltered method that takes a func predicate to match
interfaces to compare.
And then the FilterInteresting predicate is added for use
with EqualFiltered to do the job that that wgengine/monitor
previously wanted.
Now wgengine/monitor can keep the full interface state around,
including the "boring" interfaces, which we'll need for peerapi on
macOS/iOS to bind to the interface index of the utunN device.
Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>
Should help iOS battery life on NEProvider.wake/skip events
with useless route updates that shouldn't cause re-STUNs.
Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>
interfaces.State.String tries to print a concise summary of the
network state, removing any interfaces that don't have any or any
interesting IP addresses. On macOS and iOS, for instance, there are a
ton of misc things.
But the link monitor based its are-there-changes decision on
interfaces.State.Equal, which just used reflect.DeepEqual, including
comparing all the boring interfaces. On macOS, when turning wifi on or off, there
are a ton of misc boring interface changes, resulting in hitting an earlier
check I'd added on suspicion this was happening:
[unexpected] network state changed, but stringification didn't
This fixes that by instead adding a new
interfaces.State.RemoveUninterestingInterfacesAndAddresses method that
does, uh, that. Then use that in the monitor. So then when Equal is
used later, it's DeepEqualing the already-cleaned version with only
interesting interfaces.
This makes cmd/tailscaled debug --monitor much less noisy.
Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>
The Engine.LinkChange method was recently removed in
e3df29d488 while misremembering how
Android's link state mechanism worked.
Rather than do some last minute rearchitecting of link state on
Android before Tailscale 1.6, restore the old Engine.LinkChange hook
for now so the Android client doesn't need any changes. But change how
it's implemented to instead inject an event into the link monitor.
Fixes#1427
Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>
Not great, but lets people working on new ports get going more quickly
without having to do everything up front.
As the link monitor is getting used more, I felt bad having a useless
implementation.
Updates #815
Updates #1427
Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>
We used to allow that, but now it just crashes.
Separately I need to figure out why it got into this path at all,
which is #1416.
Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>
Don't use os.NewFile or (*os.File).Close on the AF_ROUTE socket. It
apparently does weird things to the fd and at least doesn't seem to
close it. Just use the unix package.
The test doesn't actually fail reliably before the fix, though. It
was an attempt. But this fixes the integration tests.
Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>
Gets it out of wgengine so the Engine isn't responsible for being a
callback registration hub for it.
This also removes the Engine.LinkChange method, as it's no longer
necessary. The monitor tells us about changes; it doesn't seem to
need any help. (Currently it was only used by Swift, but as of
14dc790137 we just do the same from Go)
Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>
Previously tailscaled on macOS was running "/sbin/route monitor" as a
child process, but child processes aren't allowed in the Network
Extension / App Store sandbox. Instead, just do what "/sbin/route monitor"
itself does: unix.Socket(unix.AF_ROUTE, unix.SOCK_RAW, 0) and read that.
We also parse it now, but don't do anything with the parsed results yet.
We will over time, as we have with Linux netlink messages over time.
Currently any message is considered a signal to poll and see what changed.
Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>
Currently it assumes exactly 1 registered callback. This changes it to
support 0, 1, or more than 1.
This is a step towards plumbing wgengine/monitor into more places (and
moving some of wgengine's interface state fetching into monitor in a
later step)
Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>
