diff --git a/derp/derp_server.go b/derp/derp_server.go index b0affe3dc..846f83175 100644 --- a/derp/derp_server.go +++ b/derp/derp_server.go @@ -77,9 +77,13 @@ type Server struct { closed bool netConns map[Conn]chan struct{} // chan is closed when conn closes 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 + clientsEver map[key.Public]bool // never deleted from, for stats; fine for now + watchers map[*sclient]bool // mesh peer -> true + // 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,12 +1012,16 @@ 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. - fwd = multiForwarder{ - prev: 1, // existed 1st, higher priority - fwd: 2, // the passed in fwd is in 2nd place + 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.multiForwarderCreated.Add(1) } s.clientsMesh[dst] = fwd } diff --git a/derp/derp_test.go b/derp/derp_test.go index e22d53c31..d35280151 100644 --- a/derp/derp_test.go +++ b/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), + }) }