Some Network code cleanup.

3 files changed, 94 insertions, 81 deletions

diff --git a/node/IncomingPacket.hpp b/node/IncomingPacket.hpp
index 5697c792..b89e259c 100644
--- a/node/IncomingPacket.hpp
+++ b/node/IncomingPacket.hpp
@@ -124,7 +124,13 @@ private:
 	bool _doNETWORK_MEMBERSHIP_CERTIFICATE(const RuntimeEnvironment *RR,const SharedPtr<Peer> &peer);
 	bool _doNETWORK_CONFIG_REQUEST(const RuntimeEnvironment *RR,const SharedPtr<Peer> &peer);
 	bool _doNETWORK_CONFIG_REFRESH(const RuntimeEnvironment *RR,const SharedPtr<Peer> &peer);
+	bool _doMULTICAST_GATHER(const RuntimeEnvironment *RR,const SharedPtr<Peer> &peer);
+	bool _doMULTICAST_FRAME(const RuntimeEnvironment *RR,const SharedPtr<Peer> &peer);
 
+	// Both OK(MULTICAST_GATHER) and OK(MULTICAST_FRAME) can carry this payload
+	void _handleMulticastGatherResponse(const RuntimeEnvironment *RR,const SharedPtr<Peer> &peer,unsigned int startIdx);
+
+	// Send an ERROR_NEED_MEMBERSHIP_CERTIFICATE to a peer indicating that an updated cert is needed to join
 	void _sendErrorNeedCertificate(const RuntimeEnvironment *RR,const SharedPtr<Peer> &peer,uint64_t nwid);
 
 	uint64_t _receiveTime;
diff --git a/node/Network.cpp b/node/Network.cpp
index 6968fb0d..18fbb624 100644
--- a/node/Network.cpp
+++ b/node/Network.cpp
@@ -81,7 +81,7 @@ Network::~Network()
 		Utils::rm(std::string(RR->homePath + ZT_PATH_SEPARATOR_S + "networks.d" + ZT_PATH_SEPARATOR_S + idString() + ".mcerts"));
 	} else {
 		clean();
-		_dumpMulticastCerts();
+		_dumpMembershipCerts();
 	}
 }
 
@@ -338,45 +338,58 @@ Network::Status Network::status() const
 	}
 }
 
-void Network::_CBhandleTapData(void *arg,const MAC &from,const MAC &to,unsigned int etherType,const Buffer<4096> &data)
+void Network::learnBridgeRoute(const MAC &mac,const Address &addr)
 {
-	if ((!((Network *)arg)->_enabled)||(((Network *)arg)->status() != NETWORK_OK))
-		return;
-
-	const RuntimeEnvironment *RR = ((Network *)arg)->RR;
-	if (RR->shutdownInProgress)
-		return;
+	Mutex::Lock _l(_lock);
+	_remoteBridgeRoutes[mac] = addr;
 
-	try {
-		RR->sw->onLocalEthernet(SharedPtr<Network>((Network *)arg),from,to,etherType,data);
-	} catch (std::exception &exc) {
-		TRACE("unexpected exception handling local packet: %s",exc.what());
-	} catch ( ... ) {
-		TRACE("unexpected exception handling local packet");
+	// If _remoteBridgeRoutes exceeds sanity limit, trim worst offenders until below -- denial of service circuit breaker
+	while (_remoteBridgeRoutes.size() > ZT_MAX_BRIDGE_ROUTES) {
+		std::map<Address,unsigned long> counts;
+		Address maxAddr;
+		unsigned long maxCount = 0;
+		for(std::map<MAC,Address>::iterator br(_remoteBridgeRoutes.begin());br!=_remoteBridgeRoutes.end();++br) {
+			unsigned long c = ++counts[br->second];
+			if (c > maxCount) {
+				maxCount = c;
+				maxAddr = br->second;
+			}
+		}
+		for(std::map<MAC,Address>::iterator br(_remoteBridgeRoutes.begin());br!=_remoteBridgeRoutes.end();) {
+			if (br->second == maxAddr)
+				_remoteBridgeRoutes.erase(br++);
+			else ++br;
+		}
 	}
 }
 
-void Network::_pushMembershipCertificate(const Address &peer,bool force,uint64_t now)
+void Network::setEnabled(bool enabled)
 {
-	uint64_t pushTimeout = _config->com().timestampMaxDelta() / 2;
-	if (!pushTimeout)
-		return; // still waiting on my own cert
-	if (pushTimeout > 1000)
-		pushTimeout -= 1000;
+	Mutex::Lock _l(_lock);
+	_enabled = enabled;
+	if (_tap)
+		_tap->setEnabled(enabled);
+}
 
-	uint64_t &lastPushed = _lastPushedMembershipCertificate[peer];
-	if ((force)||((now - lastPushed) > pushTimeout)) {
-		lastPushed = now;
-		TRACE("pushing membership cert for %.16llx to %s",(unsigned long long)_id,peer.toString().c_str());
+void Network::destroy()
+{
+	Mutex::Lock _l(_lock);
 
-		Packet outp(peer,RR->identity.address(),Packet::VERB_NETWORK_MEMBERSHIP_CERTIFICATE);
-		_config->com().serialize(outp);
-		RR->sw->send(outp,true);
-	}
+	_enabled = false;
+	_destroyed = true;
+
+	if (_setupThread)
+		Thread::join(_setupThread);
+	_setupThread = Thread();
+
+	if (_tap)
+		RR->tapFactory->close(_tap,true);
+	_tap = (EthernetTap *)0;
 }
 
 // Ethernet tap creation thread -- required on some platforms where tap
-// creation may be time consuming (e.g. Windows).
+// creation may be time consuming (e.g. Windows). Thread exits after tap
+// device setup.
 void Network::threadMain()
 	throw()
 {
@@ -420,53 +433,46 @@ void Network::threadMain()
 	}
 }
 
-void Network::learnBridgeRoute(const MAC &mac,const Address &addr)
+void Network::_CBhandleTapData(void *arg,const MAC &from,const MAC &to,unsigned int etherType,const Buffer<4096> &data)
 {
-	Mutex::Lock _l(_lock);
-	_remoteBridgeRoutes[mac] = addr;
+	if ((!((Network *)arg)->_enabled)||(((Network *)arg)->status() != NETWORK_OK))
+		return;
 
-	// If _remoteBridgeRoutes exceeds sanity limit, trim worst offenders until below -- denial of service circuit breaker
-	while (_remoteBridgeRoutes.size() > ZT_MAX_BRIDGE_ROUTES) {
-		std::map<Address,unsigned long> counts;
-		Address maxAddr;
-		unsigned long maxCount = 0;
-		for(std::map<MAC,Address>::iterator br(_remoteBridgeRoutes.begin());br!=_remoteBridgeRoutes.end();++br) {
-			unsigned long c = ++counts[br->second];
-			if (c > maxCount) {
-				maxCount = c;
-				maxAddr = br->second;
-			}
-		}
-		for(std::map<MAC,Address>::iterator br(_remoteBridgeRoutes.begin());br!=_remoteBridgeRoutes.end();) {
-			if (br->second == maxAddr)
-				_remoteBridgeRoutes.erase(br++);
-			else ++br;
-		}
+	const RuntimeEnvironment *RR = ((Network *)arg)->RR;
+	if (RR->shutdownInProgress)
+		return;
+
+	try {
+		RR->sw->onLocalEthernet(SharedPtr<Network>((Network *)arg),from,to,etherType,data);
+	} catch (std::exception &exc) {
+		TRACE("unexpected exception handling local packet: %s",exc.what());
+	} catch ( ... ) {
+		TRACE("unexpected exception handling local packet");
 	}
 }
 
-void Network::setEnabled(bool enabled)
+void Network::_pushMembershipCertificate(const Address &peer,bool force,uint64_t now)
 {
-	Mutex::Lock _l(_lock);
-	_enabled = enabled;
-	if (_tap)
-		_tap->setEnabled(enabled);
-}
+	// assumes _lock is locked
+	uint64_t pushTimeout = _config->com().timestampMaxDelta() / 2;
 
-void Network::destroy()
-{
-	Mutex::Lock _l(_lock);
+	// Zero means we're still waiting on our own cert
+	if (!pushTimeout)
+		return;
 
-	_enabled = false;
-	_destroyed = true;
+	// Give a 1s margin around +/- 1/2 max delta to account for latency
+	if (pushTimeout > 1000)
+		pushTimeout -= 1000;
 
-	if (_setupThread)
-		Thread::join(_setupThread);
-	_setupThread = Thread();
+	uint64_t &lastPushed = _lastPushedMembershipCertificate[peer];
+	if ((force)||((now - lastPushed) > pushTimeout)) {
+		lastPushed = now;
+		TRACE("pushing membership cert for %.16llx to %s",(unsigned long long)_id,peer.toString().c_str());
 
-	if (_tap)
-		RR->tapFactory->close(_tap,true);
-	_tap = (EthernetTap *)0;
+		Packet outp(peer,RR->identity.address(),Packet::VERB_NETWORK_MEMBERSHIP_CERTIFICATE);
+		_config->com().serialize(outp);
+		RR->sw->send(outp,true);
+	}
 }
 
 void Network::_restoreState()
@@ -537,7 +543,7 @@ void Network::_restoreState()
 	}
 }
 
-void Network::_dumpMulticastCerts()
+void Network::_dumpMembershipCerts()
 {
 	Buffer<ZT_NETWORK_CERT_WRITE_BUF_SIZE> buf;
 	std::string mcdbPath(RR->homePath + ZT_PATH_SEPARATOR_S + "networks.d" + ZT_PATH_SEPARATOR_S + idString() + ".mcerts");
diff --git a/node/Network.hpp b/node/Network.hpp
index d6e86958..8d4092bb 100644
--- a/node/Network.hpp
+++ b/node/Network.hpp
@@ -249,7 +249,7 @@ public:
 	Status status() const;
 
 	/**
-	 * Update and check multicast rate balance for a group
+	 * Update and check multicast rate balance for a multicast group
 	 *
 	 * @param mg Multicast group
 	 * @param bytes Size of packet
@@ -302,12 +302,6 @@ public:
 	}
 
 	/**
-	 * Thread main method; do not call elsewhere
-	 */
-	void threadMain()
-		throw();
-
-	/**
 	 * Inject a frame into tap (if it's created and network is enabled)
 	 *
 	 * @param from Origin MAC
@@ -318,6 +312,7 @@ public:
 	 */
 	inline void tapPut(const MAC &from,const MAC &to,unsigned int etherType,const void *data,unsigned int len)
 	{
+		Mutex::Lock _l(_lock);
 		if (!_enabled)
 			return;
 		EthernetTap *t = _tap;
@@ -330,6 +325,7 @@ public:
 	 */
 	inline std::string tapDeviceName() const
 	{
+		Mutex::Lock _l(_lock);
 		EthernetTap *t = _tap;
 		if (t)
 			return t->deviceName();
@@ -339,17 +335,14 @@ public:
 	/**
 	 * @return Ethernet MAC address for this network's local interface
 	 */
-	inline const MAC &mac() const
-		throw()
-	{
-		return _mac;
-	}
+	inline const MAC &mac() const throw() { return _mac; }
 
 	/**
 	 * @return Set of IPs currently assigned to interface
 	 */
 	inline std::set<InetAddress> ips() const
 	{
+		Mutex::Lock _l(_lock);
 		EthernetTap *t = _tap;
 		if (t)
 			return t->ips();
@@ -371,8 +364,10 @@ public:
 	}
 
 	/**
+	 * Find the node on this network that has this MAC behind it (if any)
+	 *
 	 * @param mac MAC address
-	 * @return ZeroTier address of bridge to this MAC or null address if not found (also check result for self, since this can happen)
+	 * @return ZeroTier address of bridge to this MAC
 	 */
 	inline Address findBridgeTo(const MAC &mac) const
 	{
@@ -422,12 +417,18 @@ public:
 	 */
 	void destroy();
 
+	/**
+	 * Thread main method; do not call elsewhere
+	 */
+	void threadMain()
+		throw();
+
 private:
 	static void _CBhandleTapData(void *arg,const MAC &from,const MAC &to,unsigned int etherType,const Buffer<4096> &data);
 
 	void _pushMembershipCertificate(const Address &peer,bool force,uint64_t now);
 	void _restoreState();
-	void _dumpMulticastCerts();
+	void _dumpMembershipCerts();
 
 	inline void _mkNetworkFriendlyName(char *buf,unsigned int len)
 	{