Only send redirects to the sending InetAddress and only in response to a set of certain frame types to avoid potential race conditions.

node/Cluster.cpp

@@ -566,7 +566,7 @@ void Cluster::removeMember(uint16_t memberId)
 	_memberIds = newMemberIds;
 }
 
-bool Cluster::redirectPeer(const Address &peerAddress,const InetAddress &peerPhysicalAddress,bool offload)
+bool Cluster::redirectPeer(const SharedPtr<Peer> &peer,const InetAddress &localAddress,const InetAddress &peerPhysicalAddress,bool offload)
 {
 	if (!peerPhysicalAddress) // sanity check
 		return false;
@@ -575,7 +575,7 @@ bool Cluster::redirectPeer(const Address &peerAddress,const InetAddress &peerPhy
 		// 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());
+			TRACE("NO GEOLOCATION available for %s",peerPhysicalAddress.toIpString().c_str());
 			return false;
 		}
 
@@ -585,7 +585,7 @@ bool Cluster::redirectPeer(const Address &peerAddress,const InetAddress &peerPhy
 		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);
+		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) {
@@ -605,7 +605,7 @@ bool Cluster::redirectPeer(const Address &peerAddress,const InetAddress &peerPhy
 		}
 
 		if (best.size() > 0) {
-			TRACE("%s seems closer to %u at %fkm, suggesting redirect...",peerAddress.toString().c_str(),bestMember,bestDistance);
+			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
@@ -613,7 +613,7 @@ bool Cluster::redirectPeer(const Address &peerAddress,const InetAddress &peerPhy
 				// 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(peerAddress,RR->identity.address(),Packet::VERB_RENDEZVOUS);
+						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());
@@ -624,14 +624,16 @@ bool Cluster::redirectPeer(const Address &peerAddress,const InetAddress &peerPhy
 							outp.append((uint8_t)16);
 							outp.append(a->rawIpData(),16);
 						}
-						RR->sw->send(outp,true,0);
+						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",peerAddress.toString().c_str(),px,py,pz,currentDistance);
+			//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;
 		}
 	} else {

node/Cluster.hpp

@@ -247,12 +247,13 @@ public:
 	/**
 	 * Redirect this peer to a better cluster member if needed
 	 *
-	 * @param peerAddress Peer to (possibly) redirect
+	 * @param peer Peer to (possibly) redirect
+	 * @param localAddress Local address for path or NULL for none/any
 	 * @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
 	 */
-	bool redirectPeer(const Address &peerAddress,const InetAddress &peerPhysicalAddress,bool offload);
+	bool redirectPeer(const SharedPtr<Peer> &peer,const InetAddress &localAddress,const InetAddress &peerPhysicalAddress,bool offload);
 
 	/**
 	 * Fill out ZT_ClusterStatus structure (from core API)

node/Peer.cpp

@@ -80,64 +80,67 @@ void Peer::received(
 	uint64_t inRePacketId,
 	Packet::Verb inReVerb)
 {
+#ifdef ZT_ENABLE_CLUSTER
+	if ((RR->cluster)&&(hops == 0)&&((verb == Packet::VERB_HELLO)||(verb == Packet::VERB_FRAME)||(verb == Packet::VERB_EXT_FRAME)||(verb == Packet::VERB_MULTICAST_FRAME))) {
+		if (RR->cluster->redirectPeer(SharedPtr<Peer>(this),localAddr,remoteAddr,false))
+			return;
+	}
+#endif
+
 	const uint64_t now = RR->node->now();
 	bool needMulticastGroupAnnounce = false;
 	bool pathIsConfirmed = false;
 
-	{
+	{	// begin _lock
 		Mutex::Lock _l(_lock);
 
 		_lastReceive = now;
 
-		if (!hops) {
-			/* Learn new paths from direct (hops == 0) packets */
-			{
-				unsigned int np = _numPaths;
-				for(unsigned int p=0;p<np;++p) {
-					if ((_paths[p].address() == remoteAddr)&&(_paths[p].localAddress() == localAddr)) {
-						_paths[p].received(now);
-						pathIsConfirmed = true;
-						break;
-					}
+		if (hops == 0) {
+			unsigned int np = _numPaths;
+			for(unsigned int p=0;p<np;++p) {
+				if ((_paths[p].address() == remoteAddr)&&(_paths[p].localAddress() == localAddr)) {
+					_paths[p].received(now);
+					pathIsConfirmed = true;
+					break;
 				}
+			}
 
-				if (!pathIsConfirmed) {
-					if ((verb == Packet::VERB_OK)&&((inReVerb == Packet::VERB_HELLO)||(inReVerb == Packet::VERB_ECHO))) {
-
-						// Learn paths if they've been confirmed via a HELLO or an ECHO
-						RemotePath *slot = (RemotePath *)0;
-						if (np < ZT_MAX_PEER_NETWORK_PATHS) {
-							slot = &(_paths[np++]);
-						} else {
-							uint64_t slotLRmin = 0xffffffffffffffffULL;
-							for(unsigned int p=0;p<ZT_MAX_PEER_NETWORK_PATHS;++p) {
-								if (_paths[p].lastReceived() <= slotLRmin) {
-									slotLRmin = _paths[p].lastReceived();
-									slot = &(_paths[p]);
-								}
-							}
-						}
-						if (slot) {
-							*slot = RemotePath(localAddr,remoteAddr);
-							slot->received(now);
-							_numPaths = np;
-							pathIsConfirmed = true;
-							_sortPaths(now);
-						}
+			if (!pathIsConfirmed) {
+				if (verb == Packet::VERB_OK) {
 
+					RemotePath *slot = (RemotePath *)0;
+					if (np < ZT_MAX_PEER_NETWORK_PATHS) {
+						slot = &(_paths[np++]);
 					} else {
-
-						/* If this path is not known, send a HELLO. We don't learn
-						 * paths without confirming that a bidirectional link is in
-						 * fact present, but any packet that decodes and authenticates
-						 * correctly is considered valid. */
-						if ((now - _lastPathConfirmationSent) >= ZT_MIN_PATH_CONFIRMATION_INTERVAL) {
-							_lastPathConfirmationSent = now;
-							TRACE("got %s via unknown path %s(%s), confirming...",Packet::verbString(verb),_id.address().toString().c_str(),remoteAddr.toString().c_str());
-							attemptToContactAt(RR,localAddr,remoteAddr,now);
+						uint64_t slotLRmin = 0xffffffffffffffffULL;
+						for(unsigned int p=0;p<ZT_MAX_PEER_NETWORK_PATHS;++p) {
+							if (_paths[p].lastReceived() <= slotLRmin) {
+								slotLRmin = _paths[p].lastReceived();
+								slot = &(_paths[p]);
+							}
 						}
+					}
+					if (slot) {
+						*slot = RemotePath(localAddr,remoteAddr);
+						slot->received(now);
+						_numPaths = np;
+						pathIsConfirmed = true;
+						_sortPaths(now);
+					}
+
+				} else {
 
+					/* If this path is not known, send a HELLO. We don't learn
+					 * paths without confirming that a bidirectional link is in
+					 * fact present, but any packet that decodes and authenticates
+					 * correctly is considered valid. */
+					if ((now - _lastPathConfirmationSent) >= ZT_MIN_PATH_CONFIRMATION_INTERVAL) {
+						_lastPathConfirmationSent = now;
+						TRACE("got %s via unknown path %s(%s), confirming...",Packet::verbString(verb),_id.address().toString().c_str(),remoteAddr.toString().c_str());
+						attemptToContactAt(RR,localAddr,remoteAddr,now);
 					}
+
 				}
 			}
 		}
@@ -151,14 +154,11 @@ void Peer::received(
 			_lastUnicastFrame = now;
 		else if (verb == Packet::VERB_MULTICAST_FRAME)
 			_lastMulticastFrame = now;
-	}
+	}	// end _lock
 
 #ifdef ZT_ENABLE_CLUSTER
-	if ((pathIsConfirmed)&&(RR->cluster)) {
-		// Either shuttle this peer off somewhere else or report to other members that we have it
-		if (!RR->cluster->redirectPeer(_id.address(),remoteAddr,false))
-			RR->cluster->replicateHavePeer(_id);
-	}
+	if ((RR->cluster)&&(pathIsConfirmed))
+		RR->cluster->replicateHavePeer(_id);
 #endif
 
 	if (needMulticastGroupAnnounce) {