Refactor cluster redirects to move code to push peers out of the actual Cluster function that checks for redirect, and clean up Peer::received() to be a bit more logical.

3 files changed, 66 insertions, 58 deletions

diff --git a/node/Cluster.cpp b/node/Cluster.cpp
index d4e81ff8..a2a99ecd 100644
--- a/node/Cluster.cpp
+++ b/node/Cluster.cpp
@@ -44,11 +44,10 @@
 #include "CertificateOfMembership.hpp"
 #include "Salsa20.hpp"
 #include "Poly1305.hpp"
-#include "Packet.hpp"
 #include "Identity.hpp"
-#include "Peer.hpp"
+#include "Topology.hpp"
+#include "Packet.hpp"
 #include "Switch.hpp"
-#include "Node.hpp"
 
 namespace ZeroTier {
 
@@ -566,17 +565,17 @@ void Cluster::removeMember(uint16_t memberId)
 	_memberIds = newMemberIds;
 }
 
-bool Cluster::redirectPeer(const SharedPtr<Peer> &peer,const InetAddress &localAddress,const InetAddress &peerPhysicalAddress,bool offload)
+InetAddress Cluster::findBetterEndpoint(const Address &peerAddress,const InetAddress &peerPhysicalAddress,bool offload)
 {
 	if (!peerPhysicalAddress) // sanity check
-		return false;
+		return InetAddress();
 
 	if (_addressToLocationFunction) {
 		// Pick based on location if it can be determined
 		int px = 0,py = 0,pz = 0;
 		if (_addressToLocationFunction(_addressToLocationFunctionArg,reinterpret_cast<const struct sockaddr_storage *>(&peerPhysicalAddress),&px,&py,&pz) == 0) {
-			TRACE("NO GEOLOCATION available for %s",peerPhysicalAddress.toIpString().c_str());
-			return false;
+			TRACE("no geolocation data for %s (geo-lookup is lazy/async so it may work next time)",peerPhysicalAddress.toIpString().c_str());
+			return InetAddress();
 		}
 
 		// Find member closest to this peer
@@ -585,7 +584,6 @@ bool Cluster::redirectPeer(const SharedPtr<Peer> &peer,const InetAddress &localA
 		const double currentDistance = _dist3d(_x,_y,_z,px,py,pz);
 		double bestDistance = (offload ? 2147483648.0 : currentDistance);
 		unsigned int bestMember = _id;
-		TRACE("%s is at %d,%d,%d -- looking for anyone closer than %d,%d,%d (%fkm)",peerPhysicalAddress.toString().c_str(),px,py,pz,_x,_y,_z,bestDistance);
 		{
 			Mutex::Lock _l(_memberIds_m);
 			for(std::vector<uint16_t>::const_iterator mid(_memberIds.begin());mid!=_memberIds.end();++mid) {
@@ -604,41 +602,17 @@ bool Cluster::redirectPeer(const SharedPtr<Peer> &peer,const InetAddress &localA
 			}
 		}
 
-		if (best.size() > 0) {
-			TRACE("%s seems closer to %u at %fkm, suggesting redirect...",peer->address().toString().c_str(),bestMember,bestDistance);
-
-			/* if (peer->remoteVersionProtocol() >= 5) {
-				// If it's a newer peer send VERB_PUSH_DIRECT_PATHS which is more idiomatic
-			} else { */
-				// Otherwise send VERB_RENDEZVOUS for ourselves, which will trick peers into trying other endpoints for us even if they're too old for PUSH_DIRECT_PATHS
-				for(std::vector<InetAddress>::const_iterator a(best.begin());a!=best.end();++a) {
-					if (a->ss_family == peerPhysicalAddress.ss_family) {
-						Packet outp(peer->address(),RR->identity.address(),Packet::VERB_RENDEZVOUS);
-						outp.append((uint8_t)0); // no flags
-						RR->identity.address().appendTo(outp); // HACK: rendezvous with ourselves! with really old peers this will only work if I'm a root server!
-						outp.append((uint16_t)a->port());
-						if (a->ss_family == AF_INET) {
-							outp.append((uint8_t)4);
-							outp.append(a->rawIpData(),4);
-						} else {
-							outp.append((uint8_t)16);
-							outp.append(a->rawIpData(),16);
-						}
-						outp.armor(peer->key(),true);
-						RR->antiRec->logOutgoingZT(outp.data(),outp.size());
-						RR->node->putPacket(localAddress,peerPhysicalAddress,outp.data(),outp.size());
-					}
-				}
-			//}
-
-			return true;
-		} else {
-			//TRACE("peer %s is at [%d,%d,%d], distance to us is %f and this seems to be the best",peer->address().toString().c_str(),px,py,pz,currentDistance);
-			return false;
+		for(std::vector<InetAddress>::const_iterator a(best.begin());a!=best.end();++a) {
+			if (a->ss_family == peerPhysicalAddress.ss_family) {
+				TRACE("%s at [%d,%d,%d] is %f from us but %f from %u, can redirect to %s",peerAddress.toString().c_str(),px,py,pz,currentDistance,bestDistance,bestMember,a->toString().c_str());
+				return *a;
+			}
 		}
+		TRACE("%s at [%d,%d,%d] is %f from us, no better endpoints found",peerAddress.toString().c_str(),px,py,pz,currentDistance);
+		return InetAddress();
 	} else {
 		// TODO: pick based on load if no location info?
-		return false;
+		return InetAddress();
 	}
 }
 
diff --git a/node/Cluster.hpp b/node/Cluster.hpp
index b1266f27..080c9310 100644
--- a/node/Cluster.hpp
+++ b/node/Cluster.hpp
@@ -245,15 +245,22 @@ public:
 	void removeMember(uint16_t memberId);
 
 	/**
-	 * Redirect this peer to a better cluster member if needed
+	 * Find a better cluster endpoint for this peer
 	 *
-	 * @param peer Peer to (possibly) redirect
-	 * @param localAddress Local address for path or NULL for none/any
+	 * If this endpoint appears to be the best, a NULL/0 InetAddres is returned.
+	 * Otherwise the InetAddress of a better endpoint is returned and the peer
+	 * can then then be told to contact us there.
+	 *
+	 * Redirection is only done within the same address family, so the returned
+	 * endpoint will always be the same ss_family as the supplied physical
+	 * address.
+	 *
+	 * @param peerAddress Address of peer to (possibly) redirect
 	 * @param peerPhysicalAddress Physical address of peer's current best path (where packet was most recently received or getBestPath()->address())
 	 * @param offload Always redirect if possible -- can be used to offload peers during shutdown
-	 * @return True if peer was redirected
+	 * @return InetAddress or NULL if there does not seem to be a better endpoint
 	 */
-	bool redirectPeer(const SharedPtr<Peer> &peer,const InetAddress &localAddress,const InetAddress &peerPhysicalAddress,bool offload);
+	InetAddress findBetterEndpoint(const Address &peerAddress,const InetAddress &peerPhysicalAddress,bool offload);
 
 	/**
 	 * Fill out ZT_ClusterStatus structure (from core API)
diff --git a/node/Peer.cpp b/node/Peer.cpp
index 4f2fe931..31a20eea 100644
--- a/node/Peer.cpp
+++ b/node/Peer.cpp
@@ -35,6 +35,7 @@
 #include "AntiRecursion.hpp"
 #include "SelfAwareness.hpp"
 #include "Cluster.hpp"
+#include "Packet.hpp"
 
 #include <algorithm>
 
@@ -81,9 +82,28 @@ void Peer::received(
 	Packet::Verb inReVerb)
 {
 #ifdef ZT_ENABLE_CLUSTER
-	if ((RR->cluster)&&(hops == 0)&&(verb != VERB_OK)&&(verb != VERB_ERROR)&&(verb != VERB_RENDEZVOUS)&&(verb != VERB_PUSH_DIRECT_PATHS)) {
-		if (RR->cluster->redirectPeer(SharedPtr<Peer>(this),localAddr,remoteAddr,false))
-			return;
+	bool redirected = false;
+	if ((RR->cluster)&&(hops == 0)&&(verb != Packet::VERB_OK)&&(verb != Packet::VERB_ERROR)&&(verb != Packet::VERB_RENDEZVOUS)&&(verb != Packet::VERB_PUSH_DIRECT_PATHS)) {
+		InetAddress redirectTo(RR->cluster->findBetterEndpoint(_id.address(),remoteAddr,false));
+		if (redirectTo) {
+			// For older peers we send RENDEZVOUS with ourselves. This will only work if we are
+			// a root server.
+			Packet outp(_id.address(),RR->identity.address(),Packet::VERB_RENDEZVOUS);
+			outp.append((uint8_t)0); // no flags
+			RR->identity.address().appendTo(outp);
+			outp.append((uint16_t)redirectTo.port());
+			if (redirectTo.ss_family == AF_INET) {
+				outp.append((uint8_t)4);
+				outp.append(redirectTo.rawIpData(),4);
+			} else {
+				outp.append((uint8_t)16);
+				outp.append(redirectTo.rawIpData(),16);
+			}
+			outp.armor(_key,true);
+			RR->antiRec->logOutgoingZT(outp.data(),outp.size());
+			RR->node->putPacket(localAddr,remoteAddr,outp.data(),outp.size());
+			redirected = true;
+		}
 	}
 #endif
 
@@ -95,6 +115,23 @@ void Peer::received(
 		Mutex::Lock _l(_lock);
 
 		_lastReceive = now;
+		if ((verb == Packet::VERB_FRAME)||(verb == Packet::VERB_EXT_FRAME))
+			_lastUnicastFrame = now;
+		else if (verb == Packet::VERB_MULTICAST_FRAME)
+			_lastMulticastFrame = now;
+
+#ifdef ZT_ENABLE_CLUSTER
+		// If we're in cluster mode and have sent the peer a better endpoint, stop
+		// here and don't confirm paths, replicate multicast info, etc. The new
+		// endpoint should do that.
+		if (redirected)
+			return;
+#endif
+
+		if ((now - _lastAnnouncedTo) >= ((ZT_MULTICAST_LIKE_EXPIRE / 2) - 1000)) {
+			_lastAnnouncedTo = now;
+			needMulticastGroupAnnounce = true;
+		}
 
 		if (hops == 0) {
 			unsigned int np = _numPaths;
@@ -144,16 +181,6 @@ void Peer::received(
 				}
 			}
 		}
-
-		if ((now - _lastAnnouncedTo) >= ((ZT_MULTICAST_LIKE_EXPIRE / 2) - 1000)) {
-			_lastAnnouncedTo = now;
-			needMulticastGroupAnnounce = true;
-		}
-
-		if ((verb == Packet::VERB_FRAME)||(verb == Packet::VERB_EXT_FRAME))
-			_lastUnicastFrame = now;
-		else if (verb == Packet::VERB_MULTICAST_FRAME)
-			_lastMulticastFrame = now;
 	}	// end _lock
 
 #ifdef ZT_ENABLE_CLUSTER