New multicast bug fixes, TRACE improvements, and temporarily disable legacy multicast for debugging purposes.

7 files changed, 60 insertions, 27 deletions

diff --git a/node/IncomingPacket.cpp b/node/IncomingPacket.cpp
index 7cbd620c..eedd0a09 100644
--- a/node/IncomingPacket.cpp
+++ b/node/IncomingPacket.cpp
@@ -79,7 +79,7 @@ bool IncomingPacket::tryDecode(const RuntimeEnvironment *RR)
 				case Packet::VERB_RENDEZVOUS:                     return _doRENDEZVOUS(RR,peer);
 				case Packet::VERB_FRAME:                          return _doFRAME(RR,peer);
 				case Packet::VERB_EXT_FRAME:                      return _doEXT_FRAME(RR,peer);
-				case Packet::VERB_P5_MULTICAST_FRAME:             return _doP5_MULTICAST_FRAME(RR,peer);
+				//case Packet::VERB_P5_MULTICAST_FRAME:             return _doP5_MULTICAST_FRAME(RR,peer);
 				case Packet::VERB_MULTICAST_LIKE:                 return _doMULTICAST_LIKE(RR,peer);
 				case Packet::VERB_NETWORK_MEMBERSHIP_CERTIFICATE: return _doNETWORK_MEMBERSHIP_CERTIFICATE(RR,peer);
 				case Packet::VERB_NETWORK_CONFIG_REQUEST:         return _doNETWORK_CONFIG_REQUEST(RR,peer);
@@ -332,6 +332,7 @@ bool IncomingPacket::_doOK(const RuntimeEnvironment *RR,const SharedPtr<Peer> &p
 			case Packet::VERB_MULTICAST_GATHER: {
 				uint64_t nwid = at<uint64_t>(ZT_PROTO_VERB_MULTICAST_GATHER__OK__IDX_NETWORK_ID);
 				MulticastGroup mg(MAC(field(ZT_PROTO_VERB_MULTICAST_GATHER__OK__IDX_MAC,6),6),at<uint32_t>(ZT_PROTO_VERB_MULTICAST_GATHER__OK__IDX_ADI));
+				TRACE("%s(%s): OK(MULTICAST_GATHER) %.16llx/%s length %u",source().toString().c_str(),_remoteAddress.toString().c_str(),nwid,mg.toString().c_str(),size());
 				_parseGatherResults(RR,peer,nwid,mg,ZT_PROTO_VERB_MULTICAST_GATHER__OK__IDX_GATHER_RESULTS);
 			}	break;
 
@@ -340,6 +341,8 @@ bool IncomingPacket::_doOK(const RuntimeEnvironment *RR,const SharedPtr<Peer> &p
 				uint64_t nwid = at<uint64_t>(ZT_PROTO_VERB_MULTICAST_FRAME__OK__IDX_NETWORK_ID);
 				MulticastGroup mg(MAC(field(ZT_PROTO_VERB_MULTICAST_FRAME__OK__IDX_MAC,6),6),at<uint32_t>(ZT_PROTO_VERB_MULTICAST_FRAME__OK__IDX_ADI));
 
+				TRACE("%s(%s): OK(MULTICAST_FRAME) %.16llx/%s flags %.2x",source().toString().c_str(),_remoteAddress.toString().c_str(),nwid,mg.toString().c_str(),flags);
+
 				unsigned int offset = 0;
 
 				if ((flags & 0x01) != 0) {
@@ -568,7 +571,7 @@ bool IncomingPacket::_doP5_MULTICAST_FRAME(const RuntimeEnvironment *RR,const Sh
 	 * though since there aren't likely to be many older nodes left after
 	 * we do a software update. */
 
-	// Quick and dirty -- this is all condemned code in any case
+	// Quick and dirty dedup -- this is all condemned code in any case
 	static uint64_t p5MulticastDedupBuffer[1024];
 	static unsigned long p5MulticastDedupBufferPtr = 0;
 	static Mutex p5MulticastDedupBuffer_m;
@@ -904,12 +907,11 @@ bool IncomingPacket::_doP5_MULTICAST_FRAME(const RuntimeEnvironment *RR,const Sh
 bool IncomingPacket::_doMULTICAST_LIKE(const RuntimeEnvironment *RR,const SharedPtr<Peer> &peer)
 {
 	try {
-		Address src(source());
 		uint64_t now = Utils::now();
 
 		// Iterate through 18-byte network,MAC,ADI tuples
 		for(unsigned int ptr=ZT_PACKET_IDX_PAYLOAD;ptr<size();ptr+=18)
-			RR->mc->add(now,at<uint64_t>(ptr),MulticastGroup(MAC(field(ptr + 8,6),6),at<uint32_t>(ptr + 14)),Address(),src);
+			RR->mc->add(now,at<uint64_t>(ptr),MulticastGroup(MAC(field(ptr + 8,6),6),at<uint32_t>(ptr + 14)),Address(),peer->address());
 
 		peer->receive(RR,_fromSock,_remoteAddress,hops(),packetId(),Packet::VERB_MULTICAST_LIKE,0,Packet::VERB_NOP,now);
 	} catch (std::exception &ex) {
@@ -1022,8 +1024,10 @@ bool IncomingPacket::_doMULTICAST_GATHER(const RuntimeEnvironment *RR,const Shar
 		MulticastGroup mg(MAC(field(ZT_PROTO_VERB_MULTICAST_GATHER_IDX_MAC,6),6),at<uint32_t>(ZT_PROTO_VERB_MULTICAST_GATHER_IDX_ADI));
 		unsigned int gatherLimit = at<uint32_t>(ZT_PROTO_VERB_MULTICAST_GATHER_IDX_GATHER_LIMIT);
 
+		//TRACE("<<MC %s(%s) GATHER up to %u in %.16llx/%s",source().toString().c_str(),_remoteAddress.toString().c_str(),gatherLimit,nwid,mg.toString().c_str());
+
 		if (gatherLimit) {
-			Packet outp(source(),RR->identity.address(),Packet::VERB_OK);
+			Packet outp(peer->address(),RR->identity.address(),Packet::VERB_OK);
 			outp.append((unsigned char)Packet::VERB_MULTICAST_GATHER);
 			outp.append(packetId());
 			outp.append(nwid);
@@ -1081,13 +1085,15 @@ bool IncomingPacket::_doMULTICAST_FRAME(const RuntimeEnvironment *RR,const Share
 				from.setTo(field(offset + ZT_PROTO_VERB_MULTICAST_FRAME_IDX_SOURCE_MAC,6),6);
 				offset += 6;
 			} else {
-				from.fromAddress(source(),nwid);
+				from.fromAddress(peer->address(),nwid);
 			}
 
 			MulticastGroup to(MAC(field(offset + ZT_PROTO_VERB_MULTICAST_FRAME_IDX_DEST_MAC,6),6),at<uint32_t>(offset + ZT_PROTO_VERB_MULTICAST_FRAME_IDX_DEST_ADI));
 			unsigned int etherType = at<uint16_t>(offset + ZT_PROTO_VERB_MULTICAST_FRAME_IDX_ETHERTYPE);
 			unsigned int payloadLen = size() - (offset + ZT_PROTO_VERB_MULTICAST_FRAME_IDX_FRAME);
 
+			//TRACE("<<MC FRAME %.16llx/%s from %s@%s flags %.2x length %u",nwid,to.toString().c_str(),from.toString().c_str(),peer->address().toString().c_str(),flags,payloadLen);
+
 			if ((payloadLen > 0)&&(payloadLen < ZT_IF_MTU)) {
 				if (!to.mac().isMulticast()) {
 					TRACE("dropped MULTICAST_FRAME from %s@%s(%s) to %s: destination is unicast, must use FRAME or EXT_FRAME",from.toString().c_str(),peer->address().toString().c_str(),_remoteAddress.toString().c_str(),to.toString().c_str());
@@ -1107,7 +1113,7 @@ bool IncomingPacket::_doMULTICAST_FRAME(const RuntimeEnvironment *RR,const Share
 					}
 				}
 
-				network->tapPut(from,to.mac(),etherType,field(offset + ZT_PROTO_VERB_MULTICAST_FRAME_IDX_FRAME,payloadLen),offset);
+				network->tapPut(from,to.mac(),etherType,field(offset + ZT_PROTO_VERB_MULTICAST_FRAME_IDX_FRAME,payloadLen),payloadLen);
 			}
 
 			if (gatherLimit) {
diff --git a/node/Multicaster.cpp b/node/Multicaster.cpp
index 191a1db3..c94ac34b 100644
--- a/node/Multicaster.cpp
+++ b/node/Multicaster.cpp
@@ -28,6 +28,7 @@
 #include <algorithm>
 
 #include "Constants.hpp"
+#include "RuntimeEnvironment.hpp"
 #include "SharedPtr.hpp"
 #include "Multicaster.hpp"
 #include "Topology.hpp"
@@ -37,7 +38,7 @@
 #include "CMWC4096.hpp"
 #include "C25519.hpp"
 #include "CertificateOfMembership.hpp"
-#include "RuntimeEnvironment.hpp"
+#include "Logger.hpp"
 
 namespace ZeroTier {
 
@@ -62,23 +63,23 @@ unsigned int Multicaster::gather(const Address &queryingPeer,uint64_t nwid,const
 	if ((gs == _groups.end())||(gs->second.members.empty())) {
 		appendTo.append((uint32_t)0);
 		appendTo.append((uint16_t)0);
+		//TRACE("..MC Multicaster::gather() attached 0 of 0 peers for %.16llx/%s (1)",nwid,mg.toString().c_str());
 		return 0;
 	}
 
 	if (limit > gs->second.members.size())
 		limit = (unsigned int)gs->second.members.size();
-	if (limit > ((ZT_PROTO_MAX_PACKET_LENGTH / 5) + 1))
-		limit = (ZT_PROTO_MAX_PACKET_LENGTH / 5) + 1;
+	if (limit > ((ZT_PROTO_MAX_PACKET_LENGTH / ZT_ADDRESS_LENGTH) + 1))
+		limit = (ZT_PROTO_MAX_PACKET_LENGTH / ZT_ADDRESS_LENGTH) + 1;
 
 	unsigned int totalAt = appendTo.size();
 	appendTo.addSize(4); // sizeof(uint32_t)
 	unsigned int nAt = appendTo.size();
 	appendTo.addSize(2); // sizeof(uint16_t)
 
+	// Members are returned in random order so that repeated gather queries
+	// will return different subsets of a large multicast group.
 	while ((n < limit)&&((appendTo.size() + ZT_ADDRESS_LENGTH) <= ZT_PROTO_MAX_PACKET_LENGTH)) {
-		// Pick a member at random -- if we've already picked it,
-		// keep circling the buffer until we find one we haven't.
-		// This won't loop forever since limit <= members.size().
 		rptr = (unsigned int)RR->prng->next32();
 restart_member_scan:
 		a = gs->second.members[rptr % (unsigned int)gs->second.members.size()].address.toInt();
@@ -105,8 +106,12 @@ restart_member_scan:
 		}
 	}
 
+	n -= skipped;
+
 	appendTo.setAt(totalAt,(uint32_t)(gs->second.members.size() - skipped));
-	appendTo.setAt(nAt,(uint16_t)(n - skipped));
+	appendTo.setAt(nAt,(uint16_t)n);
+
+	//TRACE("..MC Multicaster::gather() attached %u of %u peers for %.16llx/%s (2)",n,(unsigned int)(gs->second.members.size() - skipped),nwid,mg.toString().c_str());
 
 	return n;
 }
@@ -151,8 +156,8 @@ void Multicaster::send(
 		OutboundMulticast out;
 
 		out.init(
+			RR,
 			now,
-			RR->identity.address(),
 			nwid,
 			com,
 			limit,
@@ -196,6 +201,8 @@ void Multicaster::send(
 			gs.lastExplicitGather = now;
 			SharedPtr<Peer> sn(RR->topology->getBestSupernode());
 			if (sn) {
+				TRACE(">>MC GATHER up to %u in %.16llx/%s",gatherLimit,nwid,mg.toString().c_str());
+
 				Packet outp(sn->address(),RR->identity.address(),Packet::VERB_MULTICAST_GATHER);
 				outp.append(nwid);
 				outp.append((uint8_t)0);
@@ -216,8 +223,8 @@ void Multicaster::send(
 		OutboundMulticast &out = gs.txQueue.back();
 
 		out.init(
+			RR,
 			now,
-			RR->identity.address(),
 			nwid,
 			com,
 			limit,
@@ -351,7 +358,7 @@ void Multicaster::clean(uint64_t now)
 	}
 }
 
-void Multicaster::_add(uint64_t now,uint64_t nwid,MulticastGroupStatus &gs,const Address &learnedFrom,const Address &member)
+void Multicaster::_add(uint64_t now,uint64_t nwid,const MulticastGroup &mg,MulticastGroupStatus &gs,const Address &learnedFrom,const Address &member)
 {
 	// assumes _groups_m is locked
 
@@ -376,6 +383,8 @@ void Multicaster::_add(uint64_t now,uint64_t nwid,MulticastGroupStatus &gs,const
 	// this somewhere else but we'll try this for now.
 	gs.members.push_back(MulticastGroupMember(member,learnedFrom,now));
 
+	TRACE("..MC %s joined multicast group %.16llx/%s via %s",member.toString().c_str(),nwid,mg.toString().c_str(),((learnedFrom) ? learnedFrom.toString().c_str() : "(direct)"));
+
 	// Try to send to any outgoing multicasts that are waiting for more recipients
 	for(std::list<OutboundMulticast>::iterator tx(gs.txQueue.begin());tx!=gs.txQueue.end();) {
 		{ // TODO / LEGACY: don't send new multicast frame to old peers (if we know their version)
diff --git a/node/Multicaster.hpp b/node/Multicaster.hpp
index fd9c32d4..74151251 100644
--- a/node/Multicaster.hpp
+++ b/node/Multicaster.hpp
@@ -96,7 +96,7 @@ public:
 	inline void add(uint64_t now,uint64_t nwid,const MulticastGroup &mg,const Address &learnedFrom,const Address &member)
 	{
 		Mutex::Lock _l(_groups_m);
-		_add(now,nwid,_groups[std::pair<uint64_t,MulticastGroup>(nwid,mg)],learnedFrom,member);
+		_add(now,nwid,mg,_groups[std::pair<uint64_t,MulticastGroup>(nwid,mg)],learnedFrom,member);
 	}
 
 	/**
@@ -164,7 +164,7 @@ public:
 	void clean(uint64_t now);
 
 private:
-	void _add(uint64_t now,uint64_t nwid,MulticastGroupStatus &gs,const Address &learnedFrom,const Address &member);
+	void _add(uint64_t now,uint64_t nwid,const MulticastGroup &mg,MulticastGroupStatus &gs,const Address &learnedFrom,const Address &member);
 
 	const RuntimeEnvironment *RR;
 	std::map< std::pair<uint64_t,MulticastGroup>,MulticastGroupStatus > _groups;
diff --git a/node/OutboundMulticast.cpp b/node/OutboundMulticast.cpp
index 6083f4f4..26072a8a 100644
--- a/node/OutboundMulticast.cpp
+++ b/node/OutboundMulticast.cpp
@@ -33,12 +33,13 @@
 #include "Network.hpp"
 #include "CertificateOfMembership.hpp"
 #include "Utils.hpp"
+#include "Logger.hpp"
 
 namespace ZeroTier {
 
 void OutboundMulticast::init(
+	const RuntimeEnvironment *RR,
 	uint64_t timestamp,
-	const Address &self,
 	uint64_t nwid,
 	const CertificateOfMembership *com,
 	unsigned int limit,
@@ -57,7 +58,18 @@ void OutboundMulticast::init(
 	if (gatherLimit) flags |= 0x02;
 	if (src) flags |= 0x04;
 
-	_packetNoCom.setSource(self);
+	TRACE(">>MC %.16llx INIT %.16llx/%s limit %u gatherLimit %u from %s to %s length %u com==%d",
+		(unsigned long long)this,
+		nwid,
+		dest.toString().c_str(),
+		limit,
+		gatherLimit,
+		(src) ? src.toString().c_str() : MAC(RR->identity.address(),nwid).toString().c_str(),
+		dest.toString().c_str(),
+		len,
+		(com) ? 1 : 0);
+
+	_packetNoCom.setSource(RR->identity.address());
 	_packetNoCom.setVerb(Packet::VERB_MULTICAST_FRAME);
 	_packetNoCom.append((uint64_t)nwid);
 	_packetNoCom.append(flags);
@@ -73,7 +85,7 @@ void OutboundMulticast::init(
 		_haveCom = true;
 		flags |= 0x01;
 
-		_packetWithCom.setSource(self);
+		_packetWithCom.setSource(RR->identity.address());
 		_packetWithCom.setVerb(Packet::VERB_MULTICAST_FRAME);
 		_packetWithCom.append((uint64_t)nwid);
 		_packetWithCom.append(flags);
@@ -95,10 +107,12 @@ void OutboundMulticast::sendOnly(const RuntimeEnvironment *RR,const Address &toA
 		if (network->peerNeedsOurMembershipCertificate(toAddr,Utils::now())) {
 			_packetWithCom.newInitializationVector();
 			_packetWithCom.setDestination(toAddr);
+			TRACE(">>MC %.16llx -> %s (with COM)",(unsigned long long)this,toAddr.toString().c_str());
 			RR->sw->send(_packetWithCom,true);
 			return;
 		}
 	}
+	TRACE(">>MC %.16llx -> %s (without COM)",(unsigned long long)this,toAddr.toString().c_str());
 	_packetNoCom.newInitializationVector();
 	_packetNoCom.setDestination(toAddr);
 	RR->sw->send(_packetNoCom,true);
diff --git a/node/OutboundMulticast.hpp b/node/OutboundMulticast.hpp
index 14d5a5bc..ab0aae55 100644
--- a/node/OutboundMulticast.hpp
+++ b/node/OutboundMulticast.hpp
@@ -62,8 +62,8 @@ public:
 	/**
 	 * Initialize outbound multicast
 	 *
+	 * @param RR Runtime environment
 	 * @param timestamp Creation time
-	 * @param self My ZeroTier address
 	 * @param nwid Network ID
 	 * @param com Certificate of membership or NULL if none available
 	 * @param limit Multicast limit for desired number of packets to send
@@ -76,8 +76,8 @@ public:
 	 * @throws std::out_of_range Data too large to fit in a MULTICAST_FRAME
 	 */
 	void init(
+		const RuntimeEnvironment *RR,
 		uint64_t timestamp,
-		const Address &self,
 		uint64_t nwid,
 		const CertificateOfMembership *com,
 		unsigned int limit,
diff --git a/node/Packet.hpp b/node/Packet.hpp
index 3a0957a3..a6ac9dc6 100644
--- a/node/Packet.hpp
+++ b/node/Packet.hpp
@@ -736,10 +736,14 @@ public:
 
 		/* Request endpoints for multicast distribution:
 		 *   <[8] 64-bit network ID>
-		 *   <[1] flags (unused, must be 0)>
+		 *   <[1] flags>
 		 *   <[6] MAC address of multicast group being queried>
 		 *   <[4] 32-bit ADI for multicast group being queried>
-		 *   <[4] 32-bit (suggested) max number of multicast peers desired or 0 for no limit>
+		 *   <[4] 32-bit requested max number of multicast peers>
+		 *  [<[...] network certificate of membership>]
+		 *
+		 * Flags:
+		 *   0x01 - Network certificate of membership is attached
 		 *
 		 * This message asks a peer for additional known endpoints that have
 		 * LIKEd a given multicast group. It's sent when the sender wishes
diff --git a/node/Switch.cpp b/node/Switch.cpp
index 1eb13501..4819bd56 100644
--- a/node/Switch.cpp
+++ b/node/Switch.cpp
@@ -157,7 +157,7 @@ void Switch::onLocalEthernet(const SharedPtr<Network> &network,const MAC &from,c
 			network->id(),
 			nconf->activeBridges(),
 			mg,
-			from,
+			(fromBridged) ? from : MAC(),
 			etherType,
 			data.data(),
 			data.size());