From 5041800ac66b7daed8f9fad776e54c426ffa1451 Mon Sep 17 00:00:00 2001
From: Avery Pennarun <apenwarr@tailscale.com>
Date: Mon, 5 Oct 2020 22:36:48 -0400
Subject: [PATCH] wgengine/tstun/faketun: it's a null tunnel, not a loopback.

At some point faketun got implemented as a loopback (put a packet in
from wireguard, the same packet goes back to wireguard) which is not
useful. It's supposed to be an interface that just sinks all packets,
and then wgengine adds *only* and ICMP Echo responder as a layer on
top.

This caused extremely odd bugs on darwin, where the special case that
reinjects packets from local->local was filling the loopback channel
and creating an infinite loop (which became jammed since the reader and
writer were in the same goroutine).

Signed-off-by: Avery Pennarun <apenwarr@tailscale.com>
---
 wgengine/tstun/faketun.go  | 15 +++------
 wgengine/tstun/tun_test.go | 66 ++------------------------------------
 2 files changed, 6 insertions(+), 75 deletions(-)

diff --git a/wgengine/tstun/faketun.go b/wgengine/tstun/faketun.go
index 6afc861e3..50880131a 100644
--- a/wgengine/tstun/faketun.go
+++ b/wgengine/tstun/faketun.go
@@ -12,7 +12,6 @@ import (
 )
 
 type fakeTUN struct {
-	datachan  chan []byte
 	evchan    chan tun.Event
 	closechan chan struct{}
 }
@@ -22,7 +21,6 @@ type fakeTUN struct {
 // It primarily exists for testing.
 func NewFakeTUN() tun.Device {
 	return &fakeTUN{
-		datachan:  make(chan []byte),
 		evchan:    make(chan tun.Event),
 		closechan: make(chan struct{}),
 	}
@@ -39,22 +37,17 @@ func (t *fakeTUN) Close() error {
 }
 
 func (t *fakeTUN) Read(out []byte, offset int) (int, error) {
-	select {
-	case <-t.closechan:
-		return 0, io.EOF
-	case b := <-t.datachan:
-		copy(out[offset:offset+len(b)], b)
-		return len(b), nil
-	}
+	<-t.closechan
+	return 0, io.EOF
 }
 
 func (t *fakeTUN) Write(b []byte, n int) (int, error) {
 	select {
 	case <-t.closechan:
 		return 0, ErrClosed
-	case t.datachan <- b[n:]:
-		return len(b), nil
+	default:
 	}
+	return len(b), nil
 }
 
 func (t *fakeTUN) Flush() error           { return nil }
diff --git a/wgengine/tstun/tun_test.go b/wgengine/tstun/tun_test.go
index 7d7452050..3394a1aba 100644
--- a/wgengine/tstun/tun_test.go
+++ b/wgengine/tstun/tun_test.go
@@ -274,39 +274,13 @@ func TestAllocs(t *testing.T) {
 	ftun, tun := newFakeTUN(t.Logf, false)
 	defer tun.Close()
 
-	go func() {
-		var buf []byte
-		for {
-			select {
-			case <-tun.closed:
-				return
-			case buf = <-ftun.datachan:
-				// continue
-			}
-
-			select {
-			case <-tun.closed:
-				return
-			case ftun.datachan <- buf:
-				// continue
-			}
-		}
-	}()
-
 	buf := []byte{0x00}
 	allocs := testing.AllocsPerRun(100, func() {
-		_, err := tun.Write(buf, 0)
+		_, err := ftun.Write(buf, 0)
 		if err != nil {
 			t.Errorf("write: error: %v", err)
 			return
 		}
-
-		_, err = tun.Read(buf, 0)
-		if err != nil {
-			t.Errorf("read: error: %v", err)
-			return
-		}
-
 	})
 
 	if allocs > 0 {
@@ -318,45 +292,9 @@ func BenchmarkWrite(b *testing.B) {
 	ftun, tun := newFakeTUN(b.Logf, true)
 	defer tun.Close()
 
-	go func() {
-		for {
-			select {
-			case <-tun.closed:
-				return
-			case <-ftun.datachan:
-				// continue
-			}
-		}
-	}()
-
-	packet := udp(0x05060708, 0x01020304, 89, 89)
-	for i := 0; i < b.N; i++ {
-		_, err := tun.Write(packet, 0)
-		if err != nil {
-			b.Errorf("err = %v; want nil", err)
-		}
-	}
-}
-
-func BenchmarkRead(b *testing.B) {
-	ftun, tun := newFakeTUN(b.Logf, true)
-	defer tun.Close()
-
 	packet := udp(0x05060708, 0x01020304, 89, 89)
-	go func() {
-		for {
-			select {
-			case <-tun.closed:
-				return
-			case ftun.datachan <- packet:
-				// continue
-			}
-		}
-	}()
-
-	var buf [128]byte
 	for i := 0; i < b.N; i++ {
-		_, err := tun.Read(buf[:], 0)
+		_, err := ftun.Write(packet, 0)
 		if err != nil {
 			b.Errorf("err = %v; want nil", err)
 		}