Forget paths to peers if we are handing them off.

3 files changed, 56 insertions, 61 deletions

diff --git a/node/Cluster.cpp b/node/Cluster.cpp
index c18663bc..73ff5846 100644
--- a/node/Cluster.cpp
+++ b/node/Cluster.cpp
@@ -563,17 +563,14 @@ void Cluster::removeMember(uint16_t memberId)
 	_memberIds = newMemberIds;
 }
 
-InetAddress Cluster::findBetterEndpoint(const Address &peerAddress,const InetAddress &peerPhysicalAddress,bool offload)
+bool Cluster::findBetterEndpoint(InetAddress &redirectTo,const Address &peerAddress,const InetAddress &peerPhysicalAddress,bool offload)
 {
-	if (!peerPhysicalAddress) // sanity check
-		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 data for %s (geo-lookup is lazy/async so it may work next time)",peerPhysicalAddress.toIpString().c_str());
-			return InetAddress();
+			return false;
 		}
 
 		// Find member closest to this peer
@@ -603,14 +600,15 @@ InetAddress Cluster::findBetterEndpoint(const Address &peerAddress,const InetAdd
 		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;
+				redirectTo = *a;
+				return true;
 			}
 		}
 		TRACE("%s at [%d,%d,%d] is %f from us, no better endpoints found",peerAddress.toString().c_str(),px,py,pz,currentDistance);
-		return InetAddress();
+		return false;
 	} else {
 		// TODO: pick based on load if no location info?
-		return InetAddress();
+		return false;
 	}
 }
 
diff --git a/node/Cluster.hpp b/node/Cluster.hpp
index 282d8120..45395b0f 100644
--- a/node/Cluster.hpp
+++ b/node/Cluster.hpp
@@ -264,22 +264,15 @@ public:
 	void removeMember(uint16_t memberId);
 
 	/**
-	 * Find a better cluster endpoint for this peer
-	 *
-	 * 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.
+	 * Find a better cluster endpoint for this peer (if any)
 	 *
+	 * @param redirectTo InetAddress to be set to a better endpoint (if there is one)
 	 * @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 InetAddress or NULL if there does not seem to be a better endpoint
+	 * @return True if redirectTo was set to a new address, false if redirectTo was not modified
 	 */
-	InetAddress findBetterEndpoint(const Address &peerAddress,const InetAddress &peerPhysicalAddress,bool offload);
+	bool findBetterEndpoint(InetAddress &redirectTo,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 d5367b17..009e2be5 100644
--- a/node/Peer.cpp
+++ b/node/Peer.cpp
@@ -81,47 +81,49 @@ void Peer::received(
 	Packet::Verb inReVerb)
 {
 #ifdef ZT_ENABLE_CLUSTER
-	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.ss_family == AF_INET)||(redirectTo.ss_family == AF_INET6)) {
-			if (_vProto >= 5) {
-				// For newer peers we can send a more idiomatic verb: PUSH_DIRECT_PATHS.
-				Packet outp(_id.address(),RR->identity.address(),Packet::VERB_PUSH_DIRECT_PATHS);
-				outp.append((uint16_t)1); // count == 1
-				outp.append((uint8_t)0); // no flags
-				outp.append((uint16_t)0); // no extensions
-				if (redirectTo.ss_family == AF_INET) {
-					outp.append((uint8_t)4);
-					outp.append((uint8_t)6);
-					outp.append(redirectTo.rawIpData(),4);
-				} else {
-					outp.append((uint8_t)6);
-					outp.append((uint8_t)18);
-					outp.append(redirectTo.rawIpData(),16);
-				}
-				outp.append((uint16_t)redirectTo.port());
-				outp.armor(_key,true);
-				RR->antiRec->logOutgoingZT(outp.data(),outp.size());
-				RR->node->putPacket(localAddr,remoteAddr,outp.data(),outp.size());
-			} else {
-				// For older peers we use RENDEZVOUS to coax them into contacting us elsewhere.
-				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);
+	InetAddress redirectTo;
+	if ((RR->cluster)&&(hops == 0)) {
+		// Note: findBetterEndpoint() is first since we still want to check
+		// for a better endpoint even if we don't actually send a redirect.
+		if ( (RR->cluster->findBetterEndpoint(redirectTo,_id.address(),remoteAddr,false)) && (verb != Packet::VERB_OK)&&(verb != Packet::VERB_ERROR)&&(verb != Packet::VERB_RENDEZVOUS)&&(verb != Packet::VERB_PUSH_DIRECT_PATHS) ) {
+			if ((redirectTo.ss_family == AF_INET)||(redirectTo.ss_family == AF_INET6)) {
+				if (_vProto >= 5) {
+					// For newer peers we can send a more idiomatic verb: PUSH_DIRECT_PATHS.
+					Packet outp(_id.address(),RR->identity.address(),Packet::VERB_PUSH_DIRECT_PATHS);
+					outp.append((uint16_t)1); // count == 1
+					outp.append((uint8_t)0); // no flags
+					outp.append((uint16_t)0); // no extensions
+					if (redirectTo.ss_family == AF_INET) {
+						outp.append((uint8_t)4);
+						outp.append((uint8_t)6);
+						outp.append(redirectTo.rawIpData(),4);
+					} else {
+						outp.append((uint8_t)6);
+						outp.append((uint8_t)18);
+						outp.append(redirectTo.rawIpData(),16);
+					}
+					outp.append((uint16_t)redirectTo.port());
+					outp.armor(_key,true);
+					RR->antiRec->logOutgoingZT(outp.data(),outp.size());
+					RR->node->putPacket(localAddr,remoteAddr,outp.data(),outp.size());
 				} else {
-					outp.append((uint8_t)16);
-					outp.append(redirectTo.rawIpData(),16);
+					// For older peers we use RENDEZVOUS to coax them into contacting us elsewhere.
+					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());
 				}
-				outp.armor(_key,true);
-				RR->antiRec->logOutgoingZT(outp.data(),outp.size());
-				RR->node->putPacket(localAddr,remoteAddr,outp.data(),outp.size());
 			}
-			redirected = true;
 		}
 	}
 #endif
@@ -140,11 +142,13 @@ void Peer::received(
 			_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)
+		// If we're in cluster mode and there's a better endpoint, stop here and don't
+		// learn or confirm paths. Also reset any existing paths, since they should
+		// go there and no longer talk to us here.
+		if (redirectTo) {
+			_numPaths = 0;
 			return;
+		}
 #endif
 
 		if ((now - _lastAnnouncedTo) >= ((ZT_MULTICAST_LIKE_EXPIRE / 2) - 1000)) {