From ffa70a617dd0ac29c10f886f70ea47e674f29fb6 Mon Sep 17 00:00:00 2001
From: Brad Fitzpatrick <bradfitz@tailscale.com>
Date: Wed, 3 Mar 2021 20:58:09 -0800
Subject: [PATCH] wgengine{,/monitor}: restore Engine.LinkChange, add
 Mon.InjectEvent

The Engine.LinkChange method was recently removed in
e3df29d488f5ce50ee396b1f05a92e9cf1abb006 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>
---
 wgengine/monitor/monitor.go      | 22 +++++++++++++++-------
 wgengine/monitor/monitor_test.go | 24 ++++++++++++++++++++++++
 wgengine/userspace.go            |  6 ++++++
 wgengine/watchdog.go             |  3 +++
 wgengine/wgengine.go             | 15 +++++++++++++++
 5 files changed, 63 insertions(+), 7 deletions(-)

diff --git a/wgengine/monitor/monitor.go b/wgengine/monitor/monitor.go
index 9f8610a06..da99a9d6c 100644
--- a/wgengine/monitor/monitor.go
+++ b/wgengine/monitor/monitor.go
@@ -148,6 +148,20 @@ func (m *Mon) Close() error {
 	return err
 }
 
+// InjectEvent forces the monitor to pretend there was a network
+// change and re-check the state of the network. Any registered
+// ChangeFunc callbacks will be called within the event coalescing
+// period (under a fraction of a second).
+func (m *Mon) InjectEvent() {
+	select {
+	case m.change <- struct{}{}:
+	default:
+		// Another change signal is already
+		// buffered. Debounce will wake up soon
+		// enough.
+	}
+}
+
 func (m *Mon) stopped() bool {
 	select {
 	case <-m.stop:
@@ -175,13 +189,7 @@ func (m *Mon) pump() {
 		if msg.ignore() {
 			continue
 		}
-		select {
-		case m.change <- struct{}{}:
-		default:
-			// Another change signal is already
-			// buffered. Debounce will wake up soon
-			// enough.
-		}
+		m.InjectEvent()
 	}
 }
 
diff --git a/wgengine/monitor/monitor_test.go b/wgengine/monitor/monitor_test.go
index f1e35439e..9dd012ded 100644
--- a/wgengine/monitor/monitor_test.go
+++ b/wgengine/monitor/monitor_test.go
@@ -7,6 +7,7 @@ package monitor
 import (
 	"flag"
 	"testing"
+	"time"
 
 	"tailscale.com/net/interfaces"
 )
@@ -32,6 +33,29 @@ func TestMonitorJustClose(t *testing.T) {
 	}
 }
 
+func TestMonitorInjectEvent(t *testing.T) {
+	mon, err := New(t.Logf)
+	if err != nil {
+		t.Fatal(err)
+	}
+	defer mon.Close()
+	got := make(chan bool, 1)
+	mon.RegisterChangeCallback(func(changed bool, state *interfaces.State) {
+		select {
+		case got <- true:
+		default:
+		}
+	})
+	mon.Start()
+	mon.InjectEvent()
+	select {
+	case <-got:
+		// Pass.
+	case <-time.After(5 * time.Second):
+		t.Fatal("timeout waiting for callback")
+	}
+}
+
 var monitor = flag.String("monitor", "", `go into monitor mode like 'route monitor'; test never terminates. Value can be either "raw" or "callback"`)
 
 func TestMonitorMode(t *testing.T) {
diff --git a/wgengine/userspace.go b/wgengine/userspace.go
index d61b9645d..8d73e9483 100644
--- a/wgengine/userspace.go
+++ b/wgengine/userspace.go
@@ -1251,6 +1251,12 @@ func (e *userspaceEngine) GetLinkMonitor() *monitor.Mon {
 	return e.linkMon
 }
 
+// LinkChange signals a network change event. It's currently
+// (2021-03-03) only called on Android.
+func (e *userspaceEngine) LinkChange(_ bool) {
+	e.linkMon.InjectEvent()
+}
+
 func (e *userspaceEngine) linkChange(changed bool, cur *interfaces.State) {
 	up := cur.AnyInterfaceUp()
 	if !up {
diff --git a/wgengine/watchdog.go b/wgengine/watchdog.go
index bcef4e160..3b96f2e8e 100644
--- a/wgengine/watchdog.go
+++ b/wgengine/watchdog.go
@@ -99,6 +99,9 @@ func (e *watchdogEngine) SetNetInfoCallback(cb NetInfoCallback) {
 func (e *watchdogEngine) RequestStatus() {
 	e.watchdog("RequestStatus", func() { e.wrap.RequestStatus() })
 }
+func (e *watchdogEngine) LinkChange(isExpensive bool) {
+	e.watchdog("LinkChange", func() { e.wrap.LinkChange(isExpensive) })
+}
 func (e *watchdogEngine) SetDERPMap(m *tailcfg.DERPMap) {
 	e.watchdog("SetDERPMap", func() { e.wrap.SetDERPMap(m) })
 }
diff --git a/wgengine/wgengine.go b/wgengine/wgengine.go
index a72d0a96a..57bc7cb77 100644
--- a/wgengine/wgengine.go
+++ b/wgengine/wgengine.go
@@ -89,6 +89,21 @@ type Engine interface {
 	// TODO: return an error?
 	Wait()
 
+	// LinkChange informs the engine that the system network
+	// link has changed.
+	//
+	// The isExpensive parameter is not used.
+	//
+	// LinkChange should be called whenever something changed with
+	// the network, no matter how minor.
+	//
+	// Deprecated: don't use this method. It was removed shortly
+	// before the Tailscale 1.6 release when we remembered that
+	// Android doesn't use the Linux-based link monitor and has
+	// its own mechanism that uses LinkChange. Android is the only
+	// caller of this method now. Don't add more.
+	LinkChange(isExpensive bool)
+
 	// SetDERPMap controls which (if any) DERP servers are used.
 	// If nil, DERP is disabled. It starts disabled until a DERP map
 	// is configured.