Fix for multicast propagation -- supernodes must always keep propagating. Also fix mac-tap build on new version of Xcode CL tools. Must use old llvm-g++ instead of clang for i686 -mkernel.

5 files changed, 52 insertions, 37 deletions

diff --git a/mac-tap/tuntap/src/tap/Makefile b/mac-tap/tuntap/src/tap/Makefile
index 8207101d..e814e01c 100644
--- a/mac-tap/tuntap/src/tap/Makefile
+++ b/mac-tap/tuntap/src/tap/Makefile
@@ -27,7 +27,7 @@ CFLAGS = -Wall -mkernel -force_cpusubtype_ALL \
 CCFLAGS = $(CFLAGS)
 LDFLAGS = -Wall -mkernel -nostdlib -r -lcc_kext -arch i386 -arch x86_64 -Xlinker -kext
 
-CCP = g++
+CCP = llvm-g++
 CC = gcc
 
 all: $(KMOD_BIN) bundle
diff --git a/node/Packet.hpp b/node/Packet.hpp
index 821d617a..899b2517 100644
--- a/node/Packet.hpp
+++ b/node/Packet.hpp
@@ -175,7 +175,7 @@
 #define ZT_PROTO_VERB_MULTICAST_FRAME_IDX_PROPAGATION_PREFIX_BITS (ZT_PROTO_VERB_MULTICAST_FRAME_IDX_PROPAGATION_BLOOM_NONCE + ZT_PROTO_VERB_MULTICAST_FRAME_LEN_PROPAGATION_BLOOM_NONCE)
 #define ZT_PROTO_VERB_MULTICAST_FRAME_LEN_PROPAGATION_PREFIX_BITS 1
 #define ZT_PROTO_VERB_MULTICAST_FRAME_IDX_PROPAGATION_PREFIX (ZT_PROTO_VERB_MULTICAST_FRAME_IDX_PROPAGATION_PREFIX_BITS + ZT_PROTO_VERB_MULTICAST_FRAME_LEN_PROPAGATION_PREFIX_BITS)
-#define ZT_PROTO_VERB_MULTICAST_FRAME_LEN_PROPAGATION_PREFIX 2
+#define ZT_PROTO_VERB_MULTICAST_FRAME_LEN_PROPAGATION_PREFIX 1
 #define ZT_PROTO_VERB_MULTICAST_FRAME_IDX_ORIGIN (ZT_PROTO_VERB_MULTICAST_FRAME_IDX_PROPAGATION_PREFIX + ZT_PROTO_VERB_MULTICAST_FRAME_LEN_PROPAGATION_PREFIX)
 #define ZT_PROTO_VERB_MULTICAST_FRAME_LEN_ORIGIN 5
 #define ZT_PROTO_VERB_MULTICAST_FRAME_IDX_ORIGIN_MCID (ZT_PROTO_VERB_MULTICAST_FRAME_IDX_ORIGIN + ZT_PROTO_VERB_MULTICAST_FRAME_LEN_ORIGIN)
@@ -477,7 +477,7 @@ public:
 		VERB_PROXY_FRAME = 7,
 
 		/* A multicast frame:
-		 *   <[2] 16-bit propagation depth>
+		 *   <[2] 16-bit propagation depth or 0xffff for "do not forward">
 		 *   <[320] propagation FIFO>
 		 *   <[1024] propagation bloom filter>
 		 *   [... begin signed portion ...]
@@ -485,7 +485,7 @@ public:
 		 *   <[8] 64-bit network ID>
 		 *   <[2] 16-bit random propagation bloom filter nonce>
 		 *   <[1] number of significant bits in propagation restrict prefix>
-		 *   <[2] 16-bit propagation restriction prefix (sig bits right to left)>
+		 *   <[1] propagation restriction prefix (sig bits right to left)>
 		 *   <[5] ZeroTier address of node of origin>
 		 *   <[3] 24-bit multicast ID, together with origin forms GUID>
 		 *   <[6] source MAC address>
diff --git a/node/PacketDecoder.cpp b/node/PacketDecoder.cpp
index 6bd7265e..1ec087c7 100644
--- a/node/PacketDecoder.cpp
+++ b/node/PacketDecoder.cpp
@@ -451,6 +451,7 @@ bool PacketDecoder::_doMULTICAST_FRAME(const RuntimeEnvironment *_r,const Shared
 		Address origin(Address(field(ZT_PROTO_VERB_MULTICAST_FRAME_IDX_ORIGIN,ZT_PROTO_VERB_MULTICAST_FRAME_LEN_ORIGIN),ZT_ADDRESS_LENGTH));
 		SharedPtr<Peer> originPeer(_r->topology->getPeer(origin));
 		if (!originPeer) {
+			// We must have the origin's identity in order to authenticate a multicast
 			_r->sw->requestWhois(origin);
 			_step = DECODE_WAITING_FOR_MULTICAST_FRAME_ORIGINAL_SENDER_LOOKUP; // causes processing to come back here
 			return false;
@@ -462,7 +463,7 @@ bool PacketDecoder::_doMULTICAST_FRAME(const RuntimeEnvironment *_r,const Shared
 		uint64_t nwid = at<uint64_t>(ZT_PROTO_VERB_MULTICAST_FRAME_IDX_NETWORK_ID);
 		uint16_t bloomNonce = at<uint16_t>(ZT_PROTO_VERB_MULTICAST_FRAME_IDX_PROPAGATION_BLOOM_NONCE);
 		unsigned int prefixBits = (*this)[ZT_PROTO_VERB_MULTICAST_FRAME_IDX_PROPAGATION_PREFIX_BITS];
-		unsigned int prefix = at<uint16_t>(ZT_PROTO_VERB_MULTICAST_FRAME_IDX_PROPAGATION_PREFIX);
+		unsigned int prefix = (*this)[ZT_PROTO_VERB_MULTICAST_FRAME_IDX_PROPAGATION_PREFIX];
 		uint64_t guid = at<uint64_t>(ZT_PROTO_VERB_MULTICAST_FRAME_IDX_GUID);
 		MAC sourceMac(field(ZT_PROTO_VERB_MULTICAST_FRAME_IDX_SOURCE_MAC,ZT_PROTO_VERB_MULTICAST_FRAME_LEN_SOURCE_MAC));
 		MulticastGroup dest(MAC(field(ZT_PROTO_VERB_MULTICAST_FRAME_IDX_DEST_MAC,ZT_PROTO_VERB_MULTICAST_FRAME_LEN_DEST_MAC)),at<uint32_t>(ZT_PROTO_VERB_MULTICAST_FRAME_IDX_DEST_ADI));
@@ -483,37 +484,47 @@ bool PacketDecoder::_doMULTICAST_FRAME(const RuntimeEnvironment *_r,const Shared
 			return true;
 		}
 
-		if ((origin == _r->identity.address())||(_r->mc->deduplicate(nwid,guid))) {
-			TRACE("dropped MULTICAST_FRAME from %s(%s): duplicate",source().toString().c_str(),_remoteAddress.toString().c_str());
-			return true;
-		}
-
 		bool rateLimitsExceeded = false;
 
-		SharedPtr<Network> network(_r->nc->network(nwid));
-		if (network) {
-			if (!network->isAllowed(origin)) {
-				TRACE("didn't inject MULTICAST_FRAME from %s(%s) into %.16llx: sender %s not allowed or we don't have a certificate",source().toString().c_str(),nwid,_remoteAddress.toString().c_str(),origin.toString().c_str());
-
-				Packet outp(source(),_r->identity.address(),Packet::VERB_ERROR);
-				outp.append((unsigned char)Packet::VERB_FRAME);
-				outp.append(packetId());
-				outp.append((unsigned char)Packet::ERROR_NO_MEMBER_CERTIFICATE);
-				outp.append(nwid);
-				outp.armor(peer->key(),true);
-				_r->demarc->send(_localPort,_remoteAddress,outp.data(),outp.size(),-1);
-
-				// We do not terminate here, since if the member just has an out of
-				// date cert or hasn't sent us a cert yet we still want to propagate
-				// the message so multicast works.
-			} else if ((!network->permitsBridging())&&(!origin.wouldHaveMac(sourceMac))) {
-				TRACE("didn't inject MULTICAST_FRAME from %s(%s) into %.16llx: source mac %s doesn't belong to %s, and bridging is not supported on network",source().toString().c_str(),nwid,_remoteAddress.toString().c_str(),sourceMac.toString().c_str(),origin.toString().c_str());
-			} else if (!network->permitsEtherType(etherType)) {
-				TRACE("didn't inject MULTICAST_FRAME from %s(%s) into %.16llx: ethertype %u is not allowed",source().toString().c_str(),nwid,_remoteAddress.toString().c_str(),etherType);
-			} else if (network->updateAndCheckMulticastBalance(origin,dest,frameLen)) {
-				network->tap().put(sourceMac,dest.mac(),etherType,frame,frameLen);
-			} else {
-				rateLimitsExceeded = true;
+		if ((origin == _r->identity.address())||(_r->mc->deduplicate(nwid,guid))) {
+			// Ordinary frames will drop duplicates. Supernodes keep propagating
+			// them since they're used as hubs to link disparate clusters of
+			// members of the same multicast group.
+			if (!_r->topology->amSupernode()) {
+				TRACE("dropped MULTICAST_FRAME from %s(%s): duplicate",source().toString().c_str(),_remoteAddress.toString().c_str());
+				return true;
+			}
+		} else {
+			// Supernodes however won't do this more than once. If the supernode
+			// does happen to be a member of the network -- which is usually not
+			// true -- we don't want to see a ton of copies of the same frame on
+			// its tap device. Also double or triple counting bandwidth metrics
+			// for the same frame would not be fair.
+			SharedPtr<Network> network(_r->nc->network(nwid));
+			if (network) {
+				if (!network->isAllowed(origin)) {
+					TRACE("didn't inject MULTICAST_FRAME from %s(%s) into %.16llx: sender %s not allowed or we don't have a certificate",source().toString().c_str(),nwid,_remoteAddress.toString().c_str(),origin.toString().c_str());
+
+					Packet outp(source(),_r->identity.address(),Packet::VERB_ERROR);
+					outp.append((unsigned char)Packet::VERB_FRAME);
+					outp.append(packetId());
+					outp.append((unsigned char)Packet::ERROR_NO_MEMBER_CERTIFICATE);
+					outp.append(nwid);
+					outp.armor(peer->key(),true);
+					_r->demarc->send(_localPort,_remoteAddress,outp.data(),outp.size(),-1);
+
+					// We do not terminate here, since if the member just has an out of
+					// date cert or hasn't sent us a cert yet we still want to propagate
+					// the message so multicast works.
+				} else if ((!network->permitsBridging())&&(!origin.wouldHaveMac(sourceMac))) {
+					TRACE("didn't inject MULTICAST_FRAME from %s(%s) into %.16llx: source mac %s doesn't belong to %s, and bridging is not supported on network",source().toString().c_str(),nwid,_remoteAddress.toString().c_str(),sourceMac.toString().c_str(),origin.toString().c_str());
+				} else if (!network->permitsEtherType(etherType)) {
+					TRACE("didn't inject MULTICAST_FRAME from %s(%s) into %.16llx: ethertype %u is not allowed",source().toString().c_str(),nwid,_remoteAddress.toString().c_str(),etherType);
+				} else if (!network->updateAndCheckMulticastBalance(origin,dest,frameLen)) {
+					rateLimitsExceeded = true;
+				} else {
+					network->tap().put(sourceMac,dest.mac(),etherType,frame,frameLen);
+				}
 			}
 		}
 
@@ -526,8 +537,12 @@ bool PacketDecoder::_doMULTICAST_FRAME(const RuntimeEnvironment *_r,const Shared
 			return true;
 		}
 
+		if (depth == 0xffff) {
+			TRACE("not forwarding MULTICAST_FRAME from %s(%s): depth == 0xffff (do not forward)",source().toString().c_str(),_remoteAddress.toString().c_str());
+			return true;
+		}
 		if (++depth > ZT_MULTICAST_MAX_PROPAGATION_DEPTH) {
-			TRACE("dropped MULTICAST_FRAME from %s(%s): max propagation depth reached",source().toString().c_str(),_remoteAddress.toString().c_str());
+			TRACE("not forwarding MULTICAST_FRAME from %s(%s): max propagation depth reached",source().toString().c_str(),_remoteAddress.toString().c_str());
 			return true;
 		}
 		setAt(ZT_PROTO_VERB_MULTICAST_FRAME_IDX_PROPAGATION_DEPTH,(uint16_t)depth);
diff --git a/node/Switch.cpp b/node/Switch.cpp
index 7bece5f2..90f0b2e5 100644
--- a/node/Switch.cpp
+++ b/node/Switch.cpp
@@ -137,7 +137,7 @@ void Switch::onLocalEthernet(const SharedPtr<Network> &network,const MAC &from,c
 			outp.append(network->id());
 			outp.append(bloomNonce);
 			outp.append((unsigned char)ZT_MULTICAST_NUM_PROPAGATION_PREFIX_BITS);
-			outp.append((uint16_t)prefix);
+			outp.append((unsigned char)prefix);
 			_r->identity.address().appendTo(outp);
 			outp.append((unsigned char)((mcid >> 16) & 0xff));
 			outp.append((unsigned char)((mcid >> 8) & 0xff));
diff --git a/node/Utils.hpp b/node/Utils.hpp
index 991ae31b..09e81ee6 100644
--- a/node/Utils.hpp
+++ b/node/Utils.hpp
@@ -572,7 +572,7 @@ public:
 		char *end = dest + len;
 		while ((*dest++ = *src++)) {
 			if (dest == end) {
-				--dest = (char)0;
+				*(--dest) = (char)0;
 				return false;
 			}
 		}