From c1dd2e3a09b563c412f3f8936af3582432d4b617 Mon Sep 17 00:00:00 2001
From: Matthias Radestock <matthias.radestock@gmail.com>
Date: Thu, 29 Jan 2015 12:41:52 +0000
Subject: [PATCH] tear down connections that have no (initial) udp connectivity
 Prior to the introduction of gossip, a tcp connection would not see regular
 communication until it had been marked as 'established', because the topology
 communication scheme would periodically send 'FetchAll' only after that. So
 in the absence of udp connectivity we'd time out. The timeout could of course
 occur for other reasons, namely disrupted tcp connectivity, but the
 combination of a timeout and 'not established' pointed to a lack of udp
 connectivity as a likely cause. So that's what we checked for.

In the new gossip-based topology communication, however, gossip (of
anything, not just topology) occurs as soon as we have tcp
connectivity. Therefore we will no longer encounter a timeout when
just udp connectivity is disrupted. Which makes our check invalid.

So instead we introduce a one-shot timer, created on handshake
completion and cancelled when we mark the connection as
'established'. When the timer fires, and the connection still hasn't
been established, we shut down the connection with an error indicating
lack of udp connectivity.
---
 router/connection.go | 57 +++++++++++++++++++++++++-------------------
 router/consts.go     |  1 +
 2 files changed, 34 insertions(+), 24 deletions(-)

diff --git a/router/connection.go b/router/connection.go
index d268eb0b..b84172bf 100644
--- a/router/connection.go
+++ b/router/connection.go
@@ -38,26 +38,27 @@ type RemoteConnection struct {
 type LocalConnection struct {
 	sync.RWMutex
 	RemoteConnection
-	TCPConn           *net.TCPConn
-	tcpSender         TCPSender
-	remoteUDPAddr     *net.UDPAddr
-	established       bool
-	receivedHeartbeat bool
-	stackFrag         bool
-	effectivePMTU     int
-	SessionKey        *[32]byte
-	heartbeatFrame    *ForwardedFrame
-	heartbeat         *time.Ticker
-	fragTest          *time.Ticker
-	forwardChan       chan<- *ForwardedFrame
-	forwardChanDF     chan<- *ForwardedFrame
-	stopForward       chan<- interface{}
-	stopForwardDF     chan<- interface{}
-	verifyPMTU        chan<- int
-	Decryptor         Decryptor
-	Router            *Router
-	uid               uint64
-	queryChan         chan<- *ConnectionInteraction
+	TCPConn            *net.TCPConn
+	tcpSender          TCPSender
+	remoteUDPAddr      *net.UDPAddr
+	established        bool
+	receivedHeartbeat  bool
+	stackFrag          bool
+	effectivePMTU      int
+	SessionKey         *[32]byte
+	establishedTimeout *time.Timer
+	heartbeatFrame     *ForwardedFrame
+	heartbeat          *time.Ticker
+	fragTest           *time.Ticker
+	forwardChan        chan<- *ForwardedFrame
+	forwardChanDF      chan<- *ForwardedFrame
+	stopForward        chan<- interface{}
+	stopForwardDF      chan<- interface{}
+	verifyPMTU         chan<- int
+	Decryptor          Decryptor
+	Router             *Router
+	uid                uint64
+	queryChan          chan<- *ConnectionInteraction
 }
 
 type ConnectionInteraction struct {
@@ -253,10 +254,9 @@ func (conn *LocalConnection) run(queryChan <-chan *ConnectionInteraction, accept
 		}
 	}
 
-	err := conn.queryLoop(queryChan)
-	if netErr, ok := err.(net.Error); ok && netErr.Timeout() && !conn.established {
-		conn.log("connection shutting down due to timeout; possibly caused by blocked UDP connectivity")
-	} else if err != nil {
+	conn.establishedTimeout = time.NewTimer(EstablishedTimeout)
+
+	if err := conn.queryLoop(queryChan); err != nil {
 		conn.log("connection shutting down due to error:", err)
 	} else {
 		conn.log("connection shutting down")
@@ -278,10 +278,15 @@ func (conn *LocalConnection) queryLoop(queryChan <-chan *ConnectionInteraction)
 			case CReceivedHeartbeat:
 				err = conn.handleReceivedHeartbeat(query.payload.(*net.UDPAddr))
 			case CSetEstablished:
+				conn.establishedTimeout.Stop()
 				err = conn.handleSetEstablished()
 			case CSendProtocolMsg:
 				err = conn.handleSendProtocolMsg(query.payload.(ProtocolMsg))
 			}
+		case <-conn.establishedTimeout.C:
+			if !conn.established {
+				err = fmt.Errorf("failed to establish UDP connectivity")
+			}
 		case <-tickerChan(conn.heartbeat):
 			conn.Forward(true, conn.heartbeatFrame, nil)
 		case <-tickerChan(conn.fragTest):
@@ -368,6 +373,10 @@ func (conn *LocalConnection) handleShutdown() {
 		conn.Router.Ourself.DeleteConnection(conn)
 	}
 
+	if conn.establishedTimeout != nil {
+		conn.establishedTimeout.Stop()
+	}
+
 	stopTicker(conn.heartbeat)
 	stopTicker(conn.fragTest)
 
diff --git a/router/consts.go b/router/consts.go
index 70b558c2..11075638 100644
--- a/router/consts.go
+++ b/router/consts.go
@@ -19,6 +19,7 @@ const (
 	FastHeartbeat      = 500 * time.Millisecond
 	SlowHeartbeat      = 10 * time.Second
 	FragTestInterval   = 5 * time.Minute
+	EstablishedTimeout = 30 * time.Second
 	ReadTimeout        = 1 * time.Minute
 	PMTUVerifyAttempts = 8
 	PMTUVerifyTimeout  = 10 * time.Millisecond // gets doubled with every attempt