From 4f8253dcdb9e6ff2ae18639556811d13729fae2b Mon Sep 17 00:00:00 2001
From: Adam Ierymenko <adam.ierymenko@gmail.com>
Date: Fri, 2 Sep 2016 13:33:56 -0700
Subject: [PATCH] Tweaks to path handling...

---
 node/IncomingPacket.cpp |  4 ++--
 node/Node.cpp           |  2 +-
 node/Peer.cpp           | 41 +++++++++++++++++++++++++----------------
 node/Peer.hpp           |  7 +++----
 node/Switch.cpp         | 21 +++++++++++----------
 5 files changed, 42 insertions(+), 33 deletions(-)

diff --git a/node/IncomingPacket.cpp b/node/IncomingPacket.cpp
index fafd56793..7ba345662 100644
--- a/node/IncomingPacket.cpp
+++ b/node/IncomingPacket.cpp
@@ -1163,8 +1163,8 @@ bool IncomingPacket::_doCIRCUIT_TEST(const RuntimeEnvironment *RR,const SharedPt
 				remainingHopsPtr += ZT_ADDRESS_LENGTH;
 				SharedPtr<Peer> nhp(RR->topology->getPeer(nextHop[h]));
 				if (nhp) {
-					SharedPtr<Path> nhbp(nhp->getBestPath(now,false));
-					if (nhbp)
+					SharedPtr<Path> nhbp(nhp->getBestPath(now));
+					if ((nhbp)&&(nhbp->alive(now)))
 						nextHopBestPathAddress[h] = nhbp->address();
 				}
 			}
diff --git a/node/Node.cpp b/node/Node.cpp
index 39e243252..d2840bd0f 100644
--- a/node/Node.cpp
+++ b/node/Node.cpp
@@ -416,7 +416,7 @@ ZT_PeerList *Node::peers() const
 		p->role = RR->topology->isRoot(pi->second->identity()) ? ZT_PEER_ROLE_ROOT : ZT_PEER_ROLE_LEAF;
 
 		std::vector< SharedPtr<Path> > paths(pi->second->paths());
-		SharedPtr<Path> bestp(pi->second->getBestPath(_now,true));
+		SharedPtr<Path> bestp(pi->second->getBestPath(_now));
 		p->pathCount = 0;
 		for(std::vector< SharedPtr<Path> >::iterator path(paths.begin());path!=paths.end();++path) {
 			memcpy(&(p->paths[p->pathCount].address),&((*path)->address()),sizeof(struct sockaddr_storage));
diff --git a/node/Peer.cpp b/node/Peer.cpp
index 9b5d84fc6..a23d08222 100644
--- a/node/Peer.cpp
+++ b/node/Peer.cpp
@@ -229,7 +229,7 @@ void Peer::makeExclusive(const InetAddress &addr)
 	}
 }
 
-bool Peer::send(const void *data,unsigned int len,uint64_t now,bool forceEvenIfDead)
+bool Peer::sendDirect(const void *data,unsigned int len,uint64_t now,bool forceEvenIfDead)
 {
 	Mutex::Lock _l(_paths_m);
 
@@ -252,19 +252,17 @@ bool Peer::send(const void *data,unsigned int len,uint64_t now,bool forceEvenIfD
 	}
 }
 
-SharedPtr<Path> Peer::getBestPath(uint64_t now,bool forceEvenIfDead)
+SharedPtr<Path> Peer::getBestPath(uint64_t now)
 {
 	Mutex::Lock _l(_paths_m);
 
 	int bestp = -1;
 	uint64_t best = 0ULL;
 	for(unsigned int p=0;p<_numPaths;++p) {
-		if (_paths[p].path->alive(now)||(forceEvenIfDead)) {
-			const uint64_t s = _paths[p].path->score();
-			if (s >= best) {
-				best = s;
-				bestp = (int)p;
-			}
+		const uint64_t s = _paths[p].path->score();
+		if (s >= best) {
+			best = s;
+			bestp = (int)p;
 		}
 	}
 
@@ -293,18 +291,29 @@ void Peer::sendHELLO(const InetAddress &localAddr,const InetAddress &atAddress,u
 
 bool Peer::doPingAndKeepalive(uint64_t now,int inetAddressFamily)
 {
-	bool somethingAlive = false;
 	Mutex::Lock _l(_paths_m);
+
+	int bestp = -1;
+	uint64_t best = 0ULL;
 	for(unsigned int p=0;p<_numPaths;++p) {
-		if ((now - _paths[p].lastReceive) >= ZT_PEER_PING_PERIOD) {
-			sendHELLO(_paths[p].path->localAddress(),_paths[p].path->address(),now);
-		} else if (_paths[p].path->needsHeartbeat(now)) {
-			_natKeepaliveBuf += (uint32_t)((now * 0x9e3779b1) >> 1); // tumble this around to send constantly varying (meaningless) payloads
-			_paths[p].path->send(RR,&_natKeepaliveBuf,sizeof(_natKeepaliveBuf),now);
+		const uint64_t s = _paths[p].path->score();
+		if (s >= best) {
+			best = s;
+			bestp = (int)p;
 		}
-		somethingAlive |= _paths[p].path->alive(now);
 	}
-	return somethingAlive;
+
+	if (bestp >= 0) {
+		if ((now - _paths[bestp].lastReceive) >= ZT_PEER_PING_PERIOD) {
+			sendHELLO(_paths[bestp].path->localAddress(),_paths[bestp].path->address(),now);
+		} else if (_paths[bestp].path->needsHeartbeat(now)) {
+			_natKeepaliveBuf += (uint32_t)((now * 0x9e3779b1) >> 1); // tumble this around to send constantly varying (meaningless) payloads
+			_paths[bestp].path->send(RR,&_natKeepaliveBuf,sizeof(_natKeepaliveBuf),now);
+		}
+		return true;
+	} else {
+		return false;
+	}
 }
 
 bool Peer::hasActiveDirectPath(uint64_t now) const
diff --git a/node/Peer.hpp b/node/Peer.hpp
index ff32184fa..87aea486a 100644
--- a/node/Peer.hpp
+++ b/node/Peer.hpp
@@ -128,7 +128,7 @@ public:
 	void makeExclusive(const InetAddress &addr);
 
 	/**
-	 * Send via best path
+	 * Send via best direct path
 	 *
 	 * @param data Packet data
 	 * @param len Packet length
@@ -136,16 +136,15 @@ public:
 	 * @param forceEvenIfDead If true, send even if the path is not 'alive'
 	 * @return True if we actually sent something
 	 */
-	bool send(const void *data,unsigned int len,uint64_t now,bool forceEvenIfDead);
+	bool sendDirect(const void *data,unsigned int len,uint64_t now,bool forceEvenIfDead);
 
 	/**
 	 * Get the best current direct path
 	 *
 	 * @param now Current time
-	 * @param forceEvenIfDead If true, pick even if path is not alive
 	 * @return Best current path or NULL if none
 	 */
-	SharedPtr<Path> getBestPath(uint64_t now,bool forceEvenIfDead);
+	SharedPtr<Path> getBestPath(uint64_t now);
 
 	/**
 	 * Send a HELLO to this peer at a specified physical address
diff --git a/node/Switch.cpp b/node/Switch.cpp
index ab07d3536..125c4b699 100644
--- a/node/Switch.cpp
+++ b/node/Switch.cpp
@@ -75,6 +75,7 @@ void Switch::onRemotePacket(const InetAddress &localAddr,const InetAddress &from
 
 		SharedPtr<Path> path(RR->topology->getPath(localAddr,fromAddr));
 		path->received(now);
+		printf("<< %s %u\n",fromAddr.toString().c_str(),len);
 
 		if (len == 13) {
 			/* LEGACY: before VERB_PUSH_DIRECT_PATHS, peers used broadcast
@@ -112,7 +113,7 @@ void Switch::onRemotePacket(const InetAddress &localAddr,const InetAddress &from
 						// Note: we don't bother initiating NAT-t for fragments, since heads will set that off.
 						// It wouldn't hurt anything, just redundant and unnecessary.
 						SharedPtr<Peer> relayTo = RR->topology->getPeer(destination);
-						if ((!relayTo)||(!relayTo->send(fragment.data(),fragment.size(),now,true))) {
+						if ((!relayTo)||(!relayTo->sendDirect(fragment.data(),fragment.size(),now,false))) {
 #ifdef ZT_ENABLE_CLUSTER
 							if (RR->cluster) {
 								RR->cluster->sendViaCluster(Address(),destination,fragment.data(),fragment.size(),false);
@@ -123,7 +124,7 @@ void Switch::onRemotePacket(const InetAddress &localAddr,const InetAddress &from
 							// Don't know peer or no direct path -- so relay via root server
 							relayTo = RR->topology->getBestRoot();
 							if (relayTo)
-								relayTo->send(fragment.data(),fragment.size(),now,true);
+								relayTo->sendDirect(fragment.data(),fragment.size(),now,true);
 						}
 					} else {
 						TRACE("dropped relay [fragment](%s) -> %s, max hops exceeded",fromAddr.toString().c_str(),destination.toString().c_str());
@@ -210,7 +211,7 @@ void Switch::onRemotePacket(const InetAddress &localAddr,const InetAddress &from
 						packet.incrementHops();
 
 						SharedPtr<Peer> relayTo = RR->topology->getPeer(destination);
-						if ((relayTo)&&((relayTo->send(packet.data(),packet.size(),now,true)))) {
+						if ((relayTo)&&((relayTo->sendDirect(packet.data(),packet.size(),now,false)))) {
 							Mutex::Lock _l(_lastUniteAttempt_m);
 							uint64_t &luts = _lastUniteAttempt[_LastUniteKey(source,destination)];
 							if ((now - luts) >= ZT_MIN_UNITE_INTERVAL) {
@@ -234,7 +235,7 @@ void Switch::onRemotePacket(const InetAddress &localAddr,const InetAddress &from
 #endif
 							relayTo = RR->topology->getBestRoot(&source,1,true);
 							if (relayTo)
-								relayTo->send(packet.data(),packet.size(),now,true);
+								relayTo->sendDirect(packet.data(),packet.size(),now,true);
 						}
 					} else {
 						TRACE("dropped relay %s(%s) -> %s, max hops exceeded",packet.source().toString().c_str(),fromAddr.toString().c_str(),destination.toString().c_str());
@@ -607,7 +608,7 @@ bool Switch::unite(const Address &p1,const Address &p2)
 				outp.append(cg.first.rawIpData(),4);
 			}
 			outp.armor(p1p->key(),true);
-			p1p->send(outp.data(),outp.size(),now,true);
+			p1p->sendDirect(outp.data(),outp.size(),now,true);
 		} else {
 			// Tell p2 where to find p1.
 			Packet outp(p2,RR->identity.address(),Packet::VERB_RENDEZVOUS);
@@ -622,7 +623,7 @@ bool Switch::unite(const Address &p1,const Address &p2)
 				outp.append(cg.second.rawIpData(),4);
 			}
 			outp.armor(p2p->key(),true);
-			p2p->send(outp.data(),outp.size(),now,true);
+			p2p->sendDirect(outp.data(),outp.size(),now,true);
 		}
 		++alt; // counts up and also flips LSB
 	}
@@ -739,7 +740,7 @@ Address Switch::_sendWhoisRequest(const Address &addr,const Address *peersAlread
 		Packet outp(root->address(),RR->identity.address(),Packet::VERB_WHOIS);
 		addr.appendTo(outp);
 		outp.armor(root->key(),true);
-		if (root->send(outp.data(),outp.size(),RR->node->now(),true))
+		if (root->sendDirect(outp.data(),outp.size(),RR->node->now(),true))
 			return root->address();
 	}
 	return Address();
@@ -752,10 +753,10 @@ bool Switch::_trySend(const Packet &packet,bool encrypt)
 	if (peer) {
 		const uint64_t now = RR->node->now();
 
-		SharedPtr<Path> viaPath(peer->getBestPath(now,false));
-		if (!viaPath) {
+		SharedPtr<Path> viaPath(peer->getBestPath(now));
+		if ( (!viaPath) || ((!viaPath->alive(now))&&(!RR->topology->isRoot(peer->identity()))) ) {
 			SharedPtr<Peer> relay(RR->topology->getBestRoot());
-			if ( (!relay) || (!(viaPath = relay->getBestPath(now,true))) )
+			if ( (!relay) || (!(viaPath = relay->getBestPath(now))) )
 				return false;
 		}