wgengine/monitor: ignore adding/removing uninteresting IPs

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>
This commit is contained in:
Josh Bleecher Snyder
2021-11-19 17:04:35 -08:00
committed by Josh Bleecher Snyder
parent 60510a6ae7
commit ad5e04249b
4 changed files with 83 additions and 15 deletions

View File

@ -6,6 +6,7 @@ package interfaces
import (
"encoding/json"
"net"
"testing"
"inet.af/netaddr"
@ -28,7 +29,7 @@ func TestGetState(t *testing.T) {
t.Fatal(err)
}
if !st.EqualFiltered(st2, FilterAll) {
if !st.EqualFiltered(st2, UseAllInterfaces, UseAllIPs) {
// let's assume nobody was changing the system network interfaces between
// the two GetState calls.
t.Fatal("two States back-to-back were not equal")
@ -68,3 +69,38 @@ func TestIsUsableV6(t *testing.T) {
}
}
}
func TestStateEqualFilteredIPFilter(t *testing.T) {
// s1 and s2 are identical, except that an "interesting" interface
// has gained an "uninteresting" IP address.
s1 := &State{
InterfaceIPs: map[string][]netaddr.IPPrefix{"x": {
netaddr.MustParseIPPrefix("42.0.0.0/8"),
netaddr.MustParseIPPrefix("169.254.0.0/16"), // link local unicast
}},
Interface: map[string]Interface{"x": {Interface: &net.Interface{Name: "x"}}},
}
s2 := &State{
InterfaceIPs: map[string][]netaddr.IPPrefix{"x": {
netaddr.MustParseIPPrefix("42.0.0.0/8"),
netaddr.MustParseIPPrefix("169.254.0.0/16"), // link local unicast
netaddr.MustParseIPPrefix("127.0.0.0/8"), // loopback (added)
}},
Interface: map[string]Interface{"x": {Interface: &net.Interface{Name: "x"}}},
}
// s1 and s2 are different...
if s1.EqualFiltered(s2, UseAllInterfaces, UseAllIPs) {
t.Errorf("%+v != %+v", s1, s2)
}
// ...and they look different if you only restrict to interesting interfaces...
if s1.EqualFiltered(s2, UseInterestingInterfaces, UseAllIPs) {
t.Errorf("%+v != %+v when restricting to interesting interfaces _but not_ IPs", s1, s2)
}
// ...but because the additional IP address is uninteresting, we should treat them as the same.
if !s1.EqualFiltered(s2, UseInterestingInterfaces, UseInterestingIPs) {
t.Errorf("%+v == %+v when restricting to interesting interfaces and IPs", s1, s2)
}
}