More cleanup to direct path push, comment fixes, etc.

8 files changed, 71 insertions, 85 deletions

diff --git a/node/IncomingPacket.cpp b/node/IncomingPacket.cpp
index 7e883221..d5b4e9e6 100644
--- a/node/IncomingPacket.cpp
+++ b/node/IncomingPacket.cpp
@@ -661,21 +661,8 @@ bool IncomingPacket::_doNETWORK_MEMBERSHIP_CERTIFICATE(const RuntimeEnvironment
 			ptr += com.deserialize(*this,ptr);
 			if (com.hasRequiredFields()) {
 				SharedPtr<Network> network(RR->node->network(com.networkId()));
-				if (network) {
-					if (network->validateAndAddMembershipCertificate(com)) {
-						if ((network->isAllowed(peer->address()))&&(network->peerNeedsOurMembershipCertificate(peer->address(),RR->node->now()))) {
-							// If peer passed our check and we haven't sent it our cert yet, respond
-							// and push our cert as well for instant authorization setup.
-							SharedPtr<NetworkConfig> nconf(network->config2());
-							if ((nconf)&&(nconf->com())) {
-								Packet outp(peer->address(),RR->identity.address(),Packet::VERB_NETWORK_MEMBERSHIP_CERTIFICATE);
-								nconf->com().serialize(outp);
-								outp.armor(peer->key(),true);
-								RR->node->putPacket(_remoteAddress,outp.data(),outp.size());
-							}
-						}
-					}
-				}
+				if (network)
+					network->validateAndAddMembershipCertificate(com);
 			}
 		}
 
diff --git a/node/Network.hpp b/node/Network.hpp
index 53a22dcd..d7320d46 100644
--- a/node/Network.hpp
+++ b/node/Network.hpp
@@ -184,7 +184,7 @@ public:
 	bool validateAndAddMembershipCertificate(const CertificateOfMembership &cert);
 
 	/**
-	 * Check if we should push membership certificate to a peer, and update last pushed
+	 * Check if we should push membership certificate to a peer, AND update last pushed
 	 *
 	 * If we haven't pushed a cert to this peer in a long enough time, this returns
 	 * true and updates the last pushed time. Otherwise it returns false.
diff --git a/node/OutboundMulticast.cpp b/node/OutboundMulticast.cpp
index 5809504a..46116c07 100644
--- a/node/OutboundMulticast.cpp
+++ b/node/OutboundMulticast.cpp
@@ -102,13 +102,9 @@ void OutboundMulticast::init(
 
 void OutboundMulticast::sendOnly(const RuntimeEnvironment *RR,const Address &toAddr)
 {
-	SharedPtr<Network> network(RR->node->network(_nwid));
-
-	if (!network)
-		return;
-
 	if (_haveCom) {
-		if (network->peerNeedsOurMembershipCertificate(toAddr,RR->node->now())) {
+		SharedPtr<Network> network(RR->node->network(_nwid));
+		if ((network)&&(network->peerNeedsOurMembershipCertificate(toAddr,RR->node->now()))) {
 			_packetWithCom.newInitializationVector();
 			_packetWithCom.setDestination(toAddr);
 			//TRACE(">>MC %.16llx -> %s (with COM)",(unsigned long long)this,toAddr.toString().c_str());
diff --git a/node/Packet.hpp b/node/Packet.hpp
index 9787edb7..e84306c2 100644
--- a/node/Packet.hpp
+++ b/node/Packet.hpp
@@ -678,10 +678,10 @@ public:
 		 *   <[...] serialized certificate of membership>
 		 *   [ ... additional certificates may follow ...]
 		 *
-		 * OK/ERROR are not generated, but the recipient should push its network
-		 * membership certificate if the certificate the peer pushed is valid
-		 * and agrees and if it hasn't done so in too long. This ensures instant
-		 * network authentication setup between valid and authorized peers.
+		 * This is sent in response to ERROR_NEED_MEMBERSHIP_CERTIFICATE and may
+		 * be pushed at any other time to keep exchanged certificates up to date.
+		 *
+		 * OK/ERROR are not generated.
 		 */
 		VERB_NETWORK_MEMBERSHIP_CERTIFICATE = 10,
 
diff --git a/node/Peer.cpp b/node/Peer.cpp
index 7c936d4a..84aa8bef 100644
--- a/node/Peer.cpp
+++ b/node/Peer.cpp
@@ -208,12 +208,13 @@ void Peer::doPingAndKeepalive(const RuntimeEnvironment *RR,uint64_t now)
 	}
 }
 
-void Peer::pushDirectPaths(const RuntimeEnvironment *RR,const std::vector<Path> &dps,uint64_t now,bool force)
+void Peer::pushDirectPaths(const RuntimeEnvironment *RR,RemotePath *path,uint64_t now,bool force)
 {
-	if ((!dps.empty())&&(((now - _lastDirectPathPush) >= ZT_DIRECT_PATH_PUSH_INTERVAL)||(force))) {
+	if ((((now - _lastDirectPathPush) >= ZT_DIRECT_PATH_PUSH_INTERVAL)||(force))) {
 		_lastDirectPathPush = now;
 
-		TRACE("pushing %u direct paths to %s",(unsigned int)dps.size(),_id.address().toString().c_str());
+		std::vector<Path> dps(RR->node->directPaths());
+		TRACE("pushing %u direct paths (local interface addresses) to %s",(unsigned int)dps.size(),_id.address().toString().c_str());
 
 		std::vector<Path>::const_iterator p(dps.begin());
 		while (p != dps.end()) {
@@ -266,7 +267,8 @@ void Peer::pushDirectPaths(const RuntimeEnvironment *RR,const std::vector<Path>
 
 			if (count) {
 				outp.setAt(ZT_PACKET_IDX_PAYLOAD,(uint16_t)count);
-				RR->sw->send(outp,true,0);
+				outp.armor(_key,true);
+				path->send(RR,outp.data(),outp.size(),now);
 			}
 		}
 	}
diff --git a/node/Peer.hpp b/node/Peer.hpp
index 2af3b9e6..f5118794 100644
--- a/node/Peer.hpp
+++ b/node/Peer.hpp
@@ -179,14 +179,14 @@ public:
 	void doPingAndKeepalive(const RuntimeEnvironment *RR,uint64_t now);
 
 	/**
-	 * Push direct paths (if within rate limit)
+	 * Push direct paths if we haven't done so in [rate limit] milliseconds
 	 *
 	 * @param RR Runtime environment
-	 * @param dps Direct paths to me to push to this peer
+	 * @param path Remote path to use to send the push
 	 * @param now Current time
-	 * @param force If true, force regardless of when we pushed direct paths last
+	 * @param force If true, push regardless of rate limit
 	 */
-	void pushDirectPaths(const RuntimeEnvironment *RR,const std::vector<Path> &dps,uint64_t now,bool force);
+	void pushDirectPaths(const RuntimeEnvironment *RR,RemotePath *path,uint64_t now,bool force);
 
 	/**
 	 * @return All known direct paths to this peer
diff --git a/node/Switch.cpp b/node/Switch.cpp
index 201f36d1..13070be1 100644
--- a/node/Switch.cpp
+++ b/node/Switch.cpp
@@ -162,38 +162,34 @@ void Switch::onLocalEthernet(const SharedPtr<Network> &network,const MAC &from,c
 	if (to[0] == MAC::firstOctetForNetwork(network->id())) {
 		// Destination is another ZeroTier peer on the same network
 
-		Address toZT(to.toAddress(network->id()));
-		if (network->isAllowed(toZT)) {
-			const bool includeCom = network->peerNeedsOurMembershipCertificate(toZT,RR->node->now());
-			if ((fromBridged)||(includeCom)) {
-				Packet outp(toZT,RR->identity.address(),Packet::VERB_EXT_FRAME);
-				outp.append(network->id());
-				if (includeCom) {
-					outp.append((unsigned char)0x01); // 0x01 -- COM included
-					nconf->com().serialize(outp);
-				} else {
-					outp.append((unsigned char)0x00);
-				}
-				to.appendTo(outp);
-				from.appendTo(outp);
-				outp.append((uint16_t)etherType);
-				outp.append(data,len);
-				outp.compress();
-				send(outp,true,network->id());
+		Address toZT(to.toAddress(network->id())); // since in-network MACs are derived from addresses and network IDs, we can reverse this
+		const bool includeCom = network->peerNeedsOurMembershipCertificate(toZT,RR->node->now());
+		if ((fromBridged)||(includeCom)) {
+			Packet outp(toZT,RR->identity.address(),Packet::VERB_EXT_FRAME);
+			outp.append(network->id());
+			if (includeCom) {
+				outp.append((unsigned char)0x01); // 0x01 -- COM included
+				nconf->com().serialize(outp);
 			} else {
-				Packet outp(toZT,RR->identity.address(),Packet::VERB_FRAME);
-				outp.append(network->id());
-				outp.append((uint16_t)etherType);
-				outp.append(data,len);
-				outp.compress();
-				send(outp,true,network->id());
+				outp.append((unsigned char)0x00);
 			}
-
-			//TRACE("%.16llx: UNICAST: %s -> %s etherType==%s(%.4x) vlanId==%u len==%u fromBridged==%d includeCom==%d",network->id(),from.toString().c_str(),to.toString().c_str(),etherTypeName(etherType),etherType,vlanId,len,(int)fromBridged,(int)includeCom);
+			to.appendTo(outp);
+			from.appendTo(outp);
+			outp.append((uint16_t)etherType);
+			outp.append(data,len);
+			outp.compress();
+			send(outp,true,network->id());
 		} else {
-			TRACE("%.16llx: UNICAST: %s -> %s etherType==%s dropped, destination not a member of private network",network->id(),from.toString().c_str(),to.toString().c_str(),etherTypeName(etherType));
+			Packet outp(toZT,RR->identity.address(),Packet::VERB_FRAME);
+			outp.append(network->id());
+			outp.append((uint16_t)etherType);
+			outp.append(data,len);
+			outp.compress();
+			send(outp,true,network->id());
 		}
 
+		//TRACE("%.16llx: UNICAST: %s -> %s etherType==%s(%.4x) vlanId==%u len==%u fromBridged==%d includeCom==%d",network->id(),from.toString().c_str(),to.toString().c_str(),etherTypeName(etherType),etherType,vlanId,len,(int)fromBridged,(int)includeCom);
+
 		return;
 	}
 
@@ -205,7 +201,7 @@ void Switch::onLocalEthernet(const SharedPtr<Network> &network,const MAC &from,c
 
 		/* Create an array of up to ZT_MAX_BRIDGE_SPAM recipients for this bridged frame. */
 		bridges[0] = network->findBridgeTo(to);
-		if ((bridges[0])&&(bridges[0] != RR->identity.address())&&(network->isAllowed(bridges[0]))&&(network->permitsBridging(bridges[0]))) {
+		if ((bridges[0])&&(bridges[0] != RR->identity.address())&&(network->permitsBridging(bridges[0]))) {
 			/* We have a known bridge route for this MAC, send it there. */
 			++numBridges;
 		} else if (!nconf->activeBridges().empty()) {
@@ -215,8 +211,7 @@ void Switch::onLocalEthernet(const SharedPtr<Network> &network,const MAC &from,c
 			if (nconf->activeBridges().size() <= ZT_MAX_BRIDGE_SPAM) {
 				// If there are <= ZT_MAX_BRIDGE_SPAM active bridges, spam them all
 				while (ab != nconf->activeBridges().end()) {
-					if (network->isAllowed(*ab)) // config sanity check
-						bridges[numBridges++] = *ab;
+					bridges[numBridges++] = *ab;
 					++ab;
 				}
 			} else {
@@ -225,8 +220,7 @@ void Switch::onLocalEthernet(const SharedPtr<Network> &network,const MAC &from,c
 					if (ab == nconf->activeBridges().end())
 						ab = nconf->activeBridges().begin();
 					if (((unsigned long)RR->prng->next32() % (unsigned long)nconf->activeBridges().size()) == 0) {
-						if (network->isAllowed(*ab)) // config sanity check
-							bridges[numBridges++] = *ab;
+						bridges[numBridges++] = *ab;
 						++ab;
 					} else ++ab;
 				}
@@ -703,29 +697,28 @@ bool Switch::_trySend(const Packet &packet,bool encrypt,uint64_t nwid)
 	if (peer) {
 		const uint64_t now = RR->node->now();
 
+		SharedPtr<Network> network;
+		SharedPtr<NetworkConfig> nconf;
 		if (nwid) {
-			// If this packet has an associated network, give the peer additional hints for direct connectivity
-			peer->pushDirectPaths(RR,RR->node->directPaths(),now,false);
+			network = RR->node->network(nwid);
+			if (!network)
+				return false; // we probably just left this network, let its packets die
+			nconf = network->config2();
+			if (!nconf)
+				return false; // sanity check: unconfigured network? why are we trying to talk to it?
 		}
 
 		RemotePath *viaPath = peer->getBestPath(now);
+		SharedPtr<Peer> relay;
 		if (!viaPath) {
-			SharedPtr<Peer> relay;
-
 			// See if this network has a preferred relay (if packet has an associated network)
-			if (nwid) {
-				SharedPtr<Network> network(RR->node->network(nwid));
-				if (network) {
-					SharedPtr<NetworkConfig> nconf(network->config2());
-					if (nconf) {
-						unsigned int latency = ~((unsigned int)0);
-						for(std::vector< std::pair<Address,InetAddress> >::const_iterator r(nconf->relays().begin());r!=nconf->relays().end();++r) {
-							if (r->first != peer->address()) {
-								SharedPtr<Peer> rp(RR->topology->getPeer(r->first));
-								if ((rp)&&(rp->hasActiveDirectPath(now))&&(rp->latency() <= latency))
-									rp.swap(relay);
-							}
-						}
+			if (nconf) {
+				unsigned int latency = ~((unsigned int)0);
+				for(std::vector< std::pair<Address,InetAddress> >::const_iterator r(nconf->relays().begin());r!=nconf->relays().end();++r) {
+					if (r->first != peer->address()) {
+						SharedPtr<Peer> rp(RR->topology->getPeer(r->first));
+						if ((rp)&&(rp->hasActiveDirectPath(now))&&(rp->latency() <= latency))
+							rp.swap(relay);
 					}
 				}
 			}
@@ -735,7 +728,12 @@ bool Switch::_trySend(const Packet &packet,bool encrypt,uint64_t nwid)
 				relay = RR->topology->getBestRoot();
 
 			if (!(relay)||(!(viaPath = relay->getBestPath(now))))
-				return false;
+				return false; // no paths, no root servers?
+		}
+
+		if ((network)&&(relay)&&(network->isAllowed(peer->address()))) {
+			// Push hints for direct connectivity to this peer if we are relaying
+			peer->pushDirectPaths(RR,viaPath,now,false);
 		}
 
 		Packet tmp(packet);
diff --git a/node/Switch.hpp b/node/Switch.hpp
index af6e5938..95ca362c 100644
--- a/node/Switch.hpp
+++ b/node/Switch.hpp
@@ -108,10 +108,13 @@ public:
 	 * 
 	 * Needless to say, the packet's source must be this node. Otherwise it
 	 * won't be encrypted right. (This is not used for relaying.)
+	 *
+	 * The network ID should only be specified for frames and other actual
+	 * network traffic.
 	 * 
 	 * @param packet Packet to send
 	 * @param encrypt Encrypt packet payload? (always true except for HELLO)
-	 * @param nwid Network ID or 0 if message is not related to a specific network
+	 * @param nwid Related network ID or 0 if message is not in-network traffic
 	 */
 	void send(const Packet &packet,bool encrypt,uint64_t nwid);