derp: fix tracking problem if conn starts local, then also joins mesh peer

derp/derp_server.go

@@ -79,7 +79,11 @@ type Server struct {
 	clients     map[key.Public]*sclient
 	clientsEver map[key.Public]bool // never deleted from, for stats; fine for now
 	watchers    map[*sclient]bool   // mesh peer -> true
-	clientsMesh map[key.Public]PacketForwarder // clients connected to mesh peers; nil means only in clients, not remote
+	// clientsMesh tracks all clients in the cluster, both locally
+	// and to mesh peers.  If the value is nil, that means the
+	// peer is only remote (and thus in the clients Map).  If the
+	// value is non-nil, it's only remote.
+	clientsMesh map[key.Public]PacketForwarder
 }
 
 // PacketForwarder is something that can forward packets.
@@ -1008,13 +1012,17 @@ func (s *Server) AddPacketForwarder(dst key.Public, fwd PacketForwarder) {
 			m[fwd] = m.maxVal() + 1
 			return
 		}
-		// Otherwise, the existing value is not a set and not a dup, so make it a set.
+		if prev != nil {
+			// Otherwise, the existing value is not a set,
+			// not a dup, and not local-only (nil) so make
+			// it a set.
 			fwd = multiForwarder{
 				prev: 1, // existed 1st, higher priority
 				fwd:  2, // the passed in fwd is in 2nd place
 			}
 			s.multiForwarderCreated.Add(1)
 		}
+	}
 	s.clientsMesh[dst] = fwd
 }
 

derp/derp_test.go

@@ -732,4 +732,17 @@ func TestForwarderRegistration(t *testing.T) {
 	want(map[key.Public]PacketForwarder{
 		u1: testFwd(2),
 	})
+
+	// Now pretend u1 was already connected locally (so clientsMesh[u1] is nil), and then we heard
+	// that they're also connected to a peer of ours. That sholdn't transition the forwarder
+	// from nil to the new one, not a multiForwarder.
+	s.clients[u1] = u1c
+	s.clientsMesh[u1] = nil
+	want(map[key.Public]PacketForwarder{
+		u1: nil,
+	})
+	s.AddPacketForwarder(u1, testFwd(3))
+	want(map[key.Public]PacketForwarder{
+		u1: testFwd(3),
+	})
 }