Restore group announcement on Peer::receive() but centralize packet composition in one place.

3 files changed, 123 insertions, 123 deletions

diff --git a/node/Network.cpp b/node/Network.cpp
index 9c8aabfa..d86145da 100644
--- a/node/Network.cpp
+++ b/node/Network.cpp
@@ -141,6 +141,12 @@ void Network::multicastUnsubscribe(const MulticastGroup &mg)
 		_myMulticastGroups.swap(nmg);
 }
 
+bool Network::tryAnnounceMulticastGroupsTo(const SharedPtr<Peer> &peer)
+{
+	Mutex::Lock _l(_lock);
+	return _tryAnnounceMulticastGroupsTo(RR->topology->rootAddresses(),_allMulticastGroups(),peer,RR->node->now());
+}
+
 bool Network::applyConfiguration(const SharedPtr<NetworkConfig> &conf)
 {
 	if (_destroyed) // sanity check
@@ -394,65 +400,56 @@ bool Network::_isAllowed(const SharedPtr<Peer> &peer) const
 	return false; // default position on any failure
 }
 
-std::vector<MulticastGroup> Network::_allMulticastGroups() const
-{
-	// Assumes _lock is locked
-	std::vector<MulticastGroup> mgs;
-	mgs.reserve(_myMulticastGroups.size() + _multicastGroupsBehindMe.size() + 1);
-	mgs.insert(mgs.end(),_myMulticastGroups.begin(),_myMulticastGroups.end());
-	_multicastGroupsBehindMe.appendKeys(mgs);
-	if ((_config)&&(_config->enableBroadcast()))
-		mgs.push_back(Network::BROADCAST);
-	std::sort(mgs.begin(),mgs.end());
-	mgs.erase(std::unique(mgs.begin(),mgs.end()),mgs.end());
-	return mgs;
-}
-
 // Used in Network::_announceMulticastGroups()
 class _AnnounceMulticastGroupsToPeersWithActiveDirectPaths
 {
 public:
 	_AnnounceMulticastGroupsToPeersWithActiveDirectPaths(const RuntimeEnvironment *renv,Network *nw) :
-		RR(renv),
 		_now(renv->node->now()),
+		RR(renv),
 		_network(nw),
 		_rootAddresses(renv->topology->rootAddresses()),
 		_allMulticastGroups(nw->_allMulticastGroups())
 	{}
 
-	inline void operator()(Topology &t,const SharedPtr<Peer> &p)
-	{
-		if ( ( (p->hasActiveDirectPath(_now)) && ( (_network->_isAllowed(p)) || (p->address() == _network->controller()) ) ) || (std::find(_rootAddresses.begin(),_rootAddresses.end(),p->address()) != _rootAddresses.end()) ) {
-			Packet outp(p->address(),RR->identity.address(),Packet::VERB_MULTICAST_LIKE);
-
-			for(std::vector<MulticastGroup>::iterator mg(_allMulticastGroups.begin());mg!=_allMulticastGroups.end();++mg) {
-				if ((outp.size() + 18) >= ZT_UDP_DEFAULT_PAYLOAD_MTU) {
-					outp.armor(p->key(),true);
-					p->send(RR,outp.data(),outp.size(),_now);
-					outp.reset(p->address(),RR->identity.address(),Packet::VERB_MULTICAST_LIKE);
-				}
-
-				// network ID, MAC, ADI
-				outp.append((uint64_t)_network->id());
-				mg->mac().appendTo(outp);
-				outp.append((uint32_t)mg->adi());
-			}
-
-			if (outp.size() > ZT_PROTO_MIN_PACKET_LENGTH) {
-				outp.armor(p->key(),true);
-				p->send(RR,outp.data(),outp.size(),_now);
-			}
-		}
-	}
+	inline void operator()(Topology &t,const SharedPtr<Peer> &p) { _network->_tryAnnounceMulticastGroupsTo(_rootAddresses,_allMulticastGroups,p,_now); }
 
 private:
-	const RuntimeEnvironment *RR;
 	uint64_t _now;
+	const RuntimeEnvironment *RR;
 	Network *_network;
 	std::vector<Address> _rootAddresses;
 	std::vector<MulticastGroup> _allMulticastGroups;
 };
 
+bool Network::_tryAnnounceMulticastGroupsTo(const std::vector<Address> &alwaysAddresses,const std::vector<MulticastGroup> &allMulticastGroups,const SharedPtr<Peer> &peer,uint64_t now) const
+{
+	if ( ( (peer->hasActiveDirectPath(now)) && ( _isAllowed(peer) || (peer->address() == this->controller()) ) ) || (std::find(alwaysAddresses.begin(),alwaysAddresses.end(),peer->address()) != alwaysAddresses.end()) ) {
+		Packet outp(peer->address(),RR->identity.address(),Packet::VERB_MULTICAST_LIKE);
+
+		for(std::vector<MulticastGroup>::const_iterator mg(allMulticastGroups.begin());mg!=allMulticastGroups.end();++mg) {
+			if ((outp.size() + 18) >= ZT_UDP_DEFAULT_PAYLOAD_MTU) {
+				outp.armor(peer->key(),true);
+				peer->send(RR,outp.data(),outp.size(),now);
+				outp.reset(peer->address(),RR->identity.address(),Packet::VERB_MULTICAST_LIKE);
+			}
+
+			// network ID, MAC, ADI
+			outp.append((uint64_t)_id);
+			mg->mac().appendTo(outp);
+			outp.append((uint32_t)mg->adi());
+		}
+
+		if (outp.size() > ZT_PROTO_MIN_PACKET_LENGTH) {
+			outp.armor(peer->key(),true);
+			peer->send(RR,outp.data(),outp.size(),now);
+		}
+
+		return true;
+	}
+	return false;
+}
+
 void Network::_announceMulticastGroups()
 {
 	// Assumes _lock is locked
@@ -460,4 +457,18 @@ void Network::_announceMulticastGroups()
 	RR->topology->eachPeer<_AnnounceMulticastGroupsToPeersWithActiveDirectPaths &>(afunc);
 }
 
+std::vector<MulticastGroup> Network::_allMulticastGroups() const
+{
+	// Assumes _lock is locked
+	std::vector<MulticastGroup> mgs;
+	mgs.reserve(_myMulticastGroups.size() + _multicastGroupsBehindMe.size() + 1);
+	mgs.insert(mgs.end(),_myMulticastGroups.begin(),_myMulticastGroups.end());
+	_multicastGroupsBehindMe.appendKeys(mgs);
+	if ((_config)&&(_config->enableBroadcast()))
+		mgs.push_back(Network::BROADCAST);
+	std::sort(mgs.begin(),mgs.end());
+	mgs.erase(std::unique(mgs.begin(),mgs.end()),mgs.end());
+	return mgs;
+}
+
 } // namespace ZeroTier
diff --git a/node/Network.hpp b/node/Network.hpp
index 37077650..b942e5f9 100644
--- a/node/Network.hpp
+++ b/node/Network.hpp
@@ -93,7 +93,7 @@ public:
 	/**
 	 * @return Address of network's controller (most significant 40 bits of ID)
 	 */
-	inline Address controller() throw() { return Address(_id >> 24); }
+	inline Address controller() const throw() { return Address(_id >> 24); }
 
 	/**
 	 * @param nwid Network ID
@@ -141,6 +141,14 @@ public:
 	void multicastUnsubscribe(const MulticastGroup &mg);
 
 	/**
+	 * Announce multicast groups to a peer if that peer is authorized on this network
+	 *
+	 * @param peer Peer to try to announce multicast groups to
+	 * @return True if peer was authorized and groups were announced
+	 */
+	bool tryAnnounceMulticastGroupsTo(const SharedPtr<Peer> &peer);
+
+	/**
 	 * Apply a NetworkConfig to this network
 	 *
 	 * @param conf Configuration in NetworkConfig form
@@ -334,6 +342,7 @@ private:
 	ZT_VirtualNetworkStatus _status() const;
 	void _externalConfig(ZT_VirtualNetworkConfig *ec) const; // assumes _lock is locked
 	bool _isAllowed(const SharedPtr<Peer> &peer) const;
+	bool _tryAnnounceMulticastGroupsTo(const std::vector<Address> &rootAddresses,const std::vector<MulticastGroup> &allMulticastGroups,const SharedPtr<Peer> &peer,uint64_t now) const;
 	void _announceMulticastGroups();
 	std::vector<MulticastGroup> _allMulticastGroups() const;
 
diff --git a/node/Peer.cpp b/node/Peer.cpp
index 6203d0b4..d98e0807 100644
--- a/node/Peer.cpp
+++ b/node/Peer.cpp
@@ -96,106 +96,86 @@ void Peer::received(
 	Packet::Verb inReVerb)
 {
 	const uint64_t now = RR->node->now();
-	Mutex::Lock _l(_lock);
+	bool needMulticastGroupAnnounce = false;
 
-	_lastReceive = now;
+	{
+		Mutex::Lock _l(_lock);
 
-	if (!hops) {
-		bool pathIsConfirmed = false;
+		_lastReceive = now;
 
-		/* 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) {
+			bool pathIsConfirmed = false;
 
-			if (!pathIsConfirmed) {
-				if ((verb == Packet::VERB_OK)&&(inReVerb == Packet::VERB_HELLO)) {
+			/* 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;
+					}
+				}
 
-					// Learn paths if they've been confirmed via a HELLO
-					RemotePath *slot = (RemotePath *)0;
-					if (np < ZT_MAX_PEER_NETWORK_PATHS) {
-						// Add new path
-						slot = &(_paths[np++]);
-					} else {
-						// Replace oldest non-fixed path
-						uint64_t slotLRmin = 0xffffffffffffffffULL;
-						for(unsigned int p=0;p<ZT_MAX_PEER_NETWORK_PATHS;++p) {
-							if ((!_paths[p].fixed())&&(_paths[p].lastReceived() <= slotLRmin)) {
-								slotLRmin = _paths[p].lastReceived();
-								slot = &(_paths[p]);
+				if (!pathIsConfirmed) {
+					if ((verb == Packet::VERB_OK)&&(inReVerb == Packet::VERB_HELLO)) {
+
+						// Learn paths if they've been confirmed via a HELLO
+						RemotePath *slot = (RemotePath *)0;
+						if (np < ZT_MAX_PEER_NETWORK_PATHS) {
+							// Add new path
+							slot = &(_paths[np++]);
+						} else {
+							// Replace oldest non-fixed path
+							uint64_t slotLRmin = 0xffffffffffffffffULL;
+							for(unsigned int p=0;p<ZT_MAX_PEER_NETWORK_PATHS;++p) {
+								if ((!_paths[p].fixed())&&(_paths[p].lastReceived() <= slotLRmin)) {
+									slotLRmin = _paths[p].lastReceived();
+									slot = &(_paths[p]);
+								}
 							}
 						}
-					}
-					if (slot) {
-						*slot = RemotePath(localAddr,remoteAddr,false);
-						slot->received(now);
-						_numPaths = np;
-						pathIsConfirmed = true;
-						_sortPaths(now);
-					}
+						if (slot) {
+							*slot = RemotePath(localAddr,remoteAddr,false);
+							slot->received(now);
+							_numPaths = np;
+							pathIsConfirmed = true;
+							_sortPaths(now);
+						}
 
-				} else {
+					} 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);
-					}
+						/* 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);
+						}
 
+					}
 				}
 			}
 		}
 
-		/* Announce multicast groups of interest to direct peers if they are
-		 * considered authorized members of a given network. Also announce to
-		 * root servers and network controllers. */
-		/*
-		if ((pathIsConfirmed)&&((now - _lastAnnouncedTo) >= ((ZT_MULTICAST_LIKE_EXPIRE / 2) - 1000))) {
+		if ((now - _lastAnnouncedTo) >= ((ZT_MULTICAST_LIKE_EXPIRE / 2) - 1000)) {
 			_lastAnnouncedTo = now;
-
-			const bool isRoot = RR->topology->isRoot(_id);
-
-			Packet outp(_id.address(),RR->identity.address(),Packet::VERB_MULTICAST_LIKE);
-			const std::vector< SharedPtr<Network> > networks(RR->node->allNetworks());
-			for(std::vector< SharedPtr<Network> >::const_iterator n(networks.begin());n!=networks.end();++n) {
-				if ( (isRoot) || ((*n)->isAllowed(_id.address())) || (_id.address() == (*n)->controller()) ) {
-					const std::vector<MulticastGroup> mgs((*n)->allMulticastGroups());
-					for(std::vector<MulticastGroup>::const_iterator mg(mgs.begin());mg!=mgs.end();++mg) {
-						if ((outp.size() + 18) > ZT_UDP_DEFAULT_PAYLOAD_MTU) {
-							outp.armor(_key,true);
-							RR->node->putPacket(localAddr,remoteAddr,outp.data(),outp.size());
-							outp.reset(_id.address(),RR->identity.address(),Packet::VERB_MULTICAST_LIKE);
-						}
-
-						// network ID, MAC, ADI
-						outp.append((uint64_t)(*n)->id());
-						mg->mac().appendTo(outp);
-						outp.append((uint32_t)mg->adi());
-					}
-				}
-			}
-			if (outp.size() > ZT_PROTO_MIN_PACKET_LENGTH) {
-				outp.armor(_key,true);
-				RR->node->putPacket(localAddr,remoteAddr,outp.data(),outp.size());
-			}
+			needMulticastGroupAnnounce = true;
 		}
-		*/
+
+		if ((verb == Packet::VERB_FRAME)||(verb == Packet::VERB_EXT_FRAME))
+			_lastUnicastFrame = now;
+		else if (verb == Packet::VERB_MULTICAST_FRAME)
+			_lastMulticastFrame = now;
 	}
 
-	if ((verb == Packet::VERB_FRAME)||(verb == Packet::VERB_EXT_FRAME))
-		_lastUnicastFrame = now;
-	else if (verb == Packet::VERB_MULTICAST_FRAME)
-		_lastMulticastFrame = now;
+	if (needMulticastGroupAnnounce) {
+		const std::vector< SharedPtr<Network> > networks(RR->node->allNetworks());
+		for(std::vector< SharedPtr<Network> >::const_iterator n(networks.begin());n!=networks.end();++n)
+			(*n)->tryAnnounceMulticastGroupsTo(SharedPtr<Peer>(this));
+	}
 }
 
 void Peer::attemptToContactAt(const RuntimeEnvironment *RR,const InetAddress &localAddr,const InetAddress &atAddress,uint64_t now)