From bd185abb29d63921301d83969ac8247bc34749b2 Mon Sep 17 00:00:00 2001
From: David Wragg <david@wragg.org>
Date: Wed, 10 Jun 2015 11:16:22 +0100
Subject: [PATCH] Introduce PeerShortIDs

These are needed to support vxlan encapsulation.  In a vxlan packet,
the only place we can straightforwardly hold the source and
destination peers identifiers used by the weave router is in the
24-bit VNI (vxlan network identifier) field.  This gives us 12 bits to
indicate a peer. The resulting limit of 4096 peers on a weave network
does not seem like a problem in the near future.  But it does mean
that decentralized allocation makes collisions fairly likely even in
weave networks of sizes that are common today.  Fortunately, the
problem is much more tractable than IPAM because we can detect when
collisions occur and take action to resolve them.

TODO: We already notify the InterHost when a short id -> peer mapping
changes, but we also need to flush any entries in the router MAC cache
that might be affected.
---
 router/handshake.go  |  14 ++-
 router/interhost.go  |   6 +
 router/local_peer.go |  10 +-
 router/mocks_test.go |   2 +-
 router/peer.go       |  16 ++-
 router/peers.go      | 258 +++++++++++++++++++++++++++++++++++++------
 router/peers_test.go | 238 +++++++++++++++++++++++++++++++++++++++
 router/routes.go     |   2 +-
 router/sleeve.go     |   4 +
 9 files changed, 514 insertions(+), 36 deletions(-)

diff --git a/router/handshake.go b/router/handshake.go
index 850f9a75..fbfc5fe3 100644
--- a/router/handshake.go
+++ b/router/handshake.go
@@ -61,6 +61,7 @@ func (conn *LocalConnection) handshake(enc *gob.Encoder, dec *gob.Decoder, accep
 	nameStr, _ := fv.Value("Name")
 	nickNameStr, _ := fv.Value("NickName")
 	uidStr, _ := fv.Value("UID")
+	shortIDStr, _ := fv.Value("ShortID")
 	remoteConnIDStr, _ := fv.Value("ConnID")
 	if err := fv.Err(); err != nil {
 		return err
@@ -70,18 +71,27 @@ func (conn *LocalConnection) handshake(enc *gob.Encoder, dec *gob.Decoder, accep
 	if err != nil {
 		return err
 	}
+
 	if !acceptNewPeer {
 		if _, found := conn.Router.Peers.Fetch(name); !found {
 			return fmt.Errorf("Found unknown remote name: %s at %s", name, conn.remoteTCPAddr)
 		}
 	}
+
 	if existingConn, found := conn.Router.Ourself.ConnectionTo(name); found && existingConn.Established() {
 		return fmt.Errorf("Already have connection to %s at %s", existingConn.Remote(), existingConn.RemoteTCPAddr())
 	}
+
 	uid, err := ParsePeerUID(uidStr)
 	if err != nil {
 		return err
 	}
+
+	shortID, err := strconv.ParseUint(shortIDStr, 10, PeerShortIDBits)
+	if err != nil {
+		return err
+	}
+
 	remoteConnID, err := strconv.ParseUint(remoteConnIDStr, 10, 64)
 	if err != nil {
 		return err
@@ -110,7 +120,8 @@ func (conn *LocalConnection) handshake(enc *gob.Encoder, dec *gob.Decoder, accep
 		conn.tcpSender = NewSimpleTCPSender(enc)
 	}
 
-	return conn.setRemote(NewPeer(name, nickNameStr, uid, 0))
+	return conn.setRemote(NewPeer(name, nickNameStr, uid, 0,
+		PeerShortID(shortID)))
 }
 
 func (conn *LocalConnection) handshakeSendRecv(localConnID uint64, usingPassword bool, enc *gob.Encoder, dec *gob.Decoder) (*FieldValidator, *[32]byte, error) {
@@ -122,6 +133,7 @@ func (conn *LocalConnection) handshakeSendRecv(localConnID uint64, usingPassword
 		"Name":            conn.local.Name.String(),
 		"NickName":        conn.local.NickName,
 		"UID":             fmt.Sprint(conn.local.UID),
+		"ShortID":         fmt.Sprint(conn.local.ShortID),
 		"ConnID":          fmt.Sprint(localConnID)}
 	handshakeRecv := map[string]string{}
 
diff --git a/router/interhost.go b/router/interhost.go
index 2f3e9bd1..1fba3fe1 100644
--- a/router/interhost.go
+++ b/router/interhost.go
@@ -13,6 +13,9 @@ type InterHost interface {
 	// be discarded.
 	InvalidateRoutes()
 
+	// A mapping of a short id to a peer has changed
+	InvalidateShortIDs()
+
 	// Form a packet-forwarding connection.  The remote UDPAddr
 	// can be nil if unknown (in which case the implementation
 	// needs to discover it).
@@ -65,6 +68,9 @@ func (NullInterHost) ConsumeInterHostPackets(*Peer, *Peers,
 func (NullInterHost) InvalidateRoutes() {
 }
 
+func (NullInterHost) InvalidateShortIDs() {
+}
+
 func (NullInterHost) MakeForwarder(*Peer, net.IP, *net.UDPAddr, uint64,
 	InterHostCrypto, func([]byte) error) (InterHostForwarder, error) {
 	return NullInterHost{}, nil
diff --git a/router/local_peer.go b/router/local_peer.go
index f41a60cb..20975313 100644
--- a/router/local_peer.go
+++ b/router/local_peer.go
@@ -20,7 +20,8 @@ type LocalPeerAction func()
 func NewLocalPeer(name PeerName, nickName string, router *Router) *LocalPeer {
 	actionChan := make(chan LocalPeerAction, ChannelSize)
 	peer := &LocalPeer{
-		Peer:       NewPeer(name, nickName, randomPeerUID(), 0),
+		Peer: NewPeer(name, nickName, randomPeerUID(), 0,
+			randomPeerShortID()),
 		router:     router,
 		actionChan: actionChan,
 	}
@@ -293,3 +294,10 @@ func (peer *LocalPeer) connectionCount() int {
 	defer peer.RUnlock()
 	return len(peer.connections)
 }
+
+func (peer *LocalPeer) setShortID(shortID PeerShortID) {
+	peer.Lock()
+	defer peer.Unlock()
+	peer.ShortID = shortID
+	peer.Version++
+}
diff --git a/router/mocks_test.go b/router/mocks_test.go
index c0308c13..452a4dd7 100644
--- a/router/mocks_test.go
+++ b/router/mocks_test.go
@@ -68,7 +68,7 @@ func checkEqualConns(t *testing.T, ourName PeerName, got, wanted map[PeerName]Co
 // Get all the peers from a Peers in a slice
 func (peers *Peers) allPeers() []*Peer {
 	var res []*Peer
-	for _, peer := range peers.table {
+	for _, peer := range peers.byName {
 		res = append(res, peer)
 	}
 	return res
diff --git a/router/peer.go b/router/peer.go
index 7f79064e..7ee3a889 100644
--- a/router/peer.go
+++ b/router/peer.go
@@ -17,11 +17,24 @@ func ParsePeerUID(s string) (PeerUID, error) {
 	return PeerUID(uid), err
 }
 
+// Short IDs exist for the sake of fast datapath.  They are 12 bits,
+// randomly assigned, but we detect and recover from collisions.  This
+// does limit us to 4096 peers, but that should be sufficient for a
+// while.
+type PeerShortID uint16
+
+const PeerShortIDBits = 12
+
+func randomPeerShortID() PeerShortID {
+	return PeerShortID(randUint64() & (1<<PeerShortIDBits - 1))
+}
+
 type PeerSummary struct {
 	NameByte []byte
 	NickName string
 	UID      PeerUID
 	Version  uint64
+	ShortID  PeerShortID
 }
 
 type Peer struct {
@@ -33,7 +46,7 @@ type Peer struct {
 
 type ConnectionSet map[Connection]struct{}
 
-func NewPeer(name PeerName, nickName string, uid PeerUID, version uint64) *Peer {
+func NewPeer(name PeerName, nickName string, uid PeerUID, version uint64, shortID PeerShortID) *Peer {
 	return &Peer{
 		Name: name,
 		PeerSummary: PeerSummary{
@@ -41,6 +54,7 @@ func NewPeer(name PeerName, nickName string, uid PeerUID, version uint64) *Peer
 			NickName: nickName,
 			UID:      uid,
 			Version:  version,
+			ShortID:  shortID,
 		},
 		connections: make(map[PeerName]Connection)}
 }
diff --git a/router/peers.go b/router/peers.go
index b7fc2442..fcf333e7 100644
--- a/router/peers.go
+++ b/router/peers.go
@@ -5,14 +5,27 @@ import (
 	"encoding/gob"
 	"fmt"
 	"io"
+	"math/rand"
 	"sync"
 )
 
 type Peers struct {
 	sync.RWMutex
-	ourself *LocalPeer
-	table   map[PeerName]*Peer
-	onGC    func(*Peer)
+	ourself                   *LocalPeer
+	onGC                      func(*Peer)
+	byName                    map[PeerName]*Peer
+	byShortID                 map[PeerShortID]ShortIDPeers
+	invalidateShortIDsPending bool
+}
+
+type ShortIDPeers struct {
+	// If we know about a single peer with the short id, this is
+	// that peer.  If there is a coliision, this is the peer with
+	// the lowest Name.
+	peer *Peer
+
+	// In case of a collision, this holds the other ppeers.
+	others []*Peer
 }
 
 type UnknownPeerError struct {
@@ -34,35 +47,201 @@ type ConnectionSummary struct {
 
 func NewPeers(ourself *LocalPeer, onGC func(*Peer)) *Peers {
 	peers := &Peers{
-		ourself: ourself,
-		table:   make(map[PeerName]*Peer),
-		onGC:    onGC}
+		ourself:   ourself,
+		byName:    make(map[PeerName]*Peer),
+		byShortID: make(map[PeerShortID]ShortIDPeers),
+		onGC:      onGC}
 	peers.FetchWithDefault(ourself.Peer)
 	return peers
 }
 
+func (peers *Peers) Fetch(name PeerName) (*Peer, bool) {
+	peers.RLock()
+	defer peers.RUnlock()
+	peer, found := peers.byName[name]
+	return peer, found // GRRR, why can't I inline this!?
+}
+
+func (peers *Peers) FetchByShortID(shortID PeerShortID) *Peer {
+	peers.RLock()
+	defer peers.RUnlock()
+	return peers.byShortID[shortID].peer
+}
+
+func (peers *Peers) addByShortID(peer *Peer) {
+	reassign := false
+	entry, ok := peers.byShortID[peer.ShortID]
+	if !ok {
+		entry = ShortIDPeers{peer: peer}
+	} else if entry.peer == nil {
+		// This short ID is free, but was used in the past.
+		// Because we are reusing it, it's an invalidation
+		// event.
+		entry.peer = peer
+		peers.invalidateShortIDsPending = true
+	} else if peer.Name < entry.peer.Name {
+		// Short ID collision, this peer becomes the principal
+		// peer for the short ID, bumping the previous one
+		// into others.
+
+		if entry.peer == peers.ourself.Peer {
+			// The bumped peer is peers.ourself, so we
+			// need to look foor a new short id
+			reassign = true
+		}
+
+		entry.others = append(entry.others, entry.peer)
+		entry.peer = peer
+		peers.invalidateShortIDsPending = true
+	} else {
+		// Short ID collision, this peer is secondary
+		entry.others = append(entry.others, peer)
+	}
+
+	peers.byShortID[peer.ShortID] = entry
+
+	if reassign {
+		peers.reassignLocalShortID()
+	}
+}
+
+func (peers *Peers) deleteByShortID(peer *Peer) {
+	entry := peers.byShortID[peer.ShortID]
+	var otherIndex int
+
+	if peer != entry.peer {
+		// peer is secondary, find its index in others
+		otherIndex = -1
+
+		for i, other := range entry.others {
+			if peer == other {
+				otherIndex = i
+				break
+			}
+		}
+
+		if otherIndex < 0 {
+			return
+		}
+	} else if len(entry.others) != 0 {
+		// need to find the peer with the lowest name to
+		// become the new principal one
+		otherIndex = 0
+		minName := entry.others[0].Name
+
+		for i := 1; i < len(entry.others); i++ {
+			otherName := entry.others[i].Name
+			if otherName < minName {
+				minName = otherName
+				otherIndex = i
+			}
+		}
+
+		entry.peer = entry.others[otherIndex]
+		peers.invalidateShortIDsPending = true
+	} else {
+		// This is the last peer with the short id.  We clear
+		// the entry, don't delete it, in order to detect when
+		// it gets re-used.
+		peers.byShortID[peer.ShortID] = ShortIDPeers{}
+		return
+	}
+
+	entry.others[otherIndex] = entry.others[len(entry.others)-1]
+	entry.others = entry.others[:len(entry.others)-1]
+	peers.byShortID[peer.ShortID] = entry
+}
+
+func (peers *Peers) reassignLocalShortID() {
+	newShortID, ok := peers.chooseShortID()
+	if ok {
+		peers.setLocalShortID(newShortID)
+	}
+
+	// Otherwise we'll try again in garbageColleect
+}
+
+func (peers *Peers) setLocalShortID(newShortID PeerShortID) {
+	peers.deleteByShortID(peers.ourself.Peer)
+	peers.ourself.setShortID(newShortID)
+	peers.addByShortID(peers.ourself.Peer)
+}
+
+// Choose an available short id at random
+func (peers *Peers) chooseShortID() (PeerShortID, bool) {
+	rng := rand.New(rand.NewSource(int64(randUint64())))
+
+	// First, just try picking some short ids at random, and
+	// seeing if they are available:
+	for i := 0; i < 10; i++ {
+		shortID := PeerShortID(rng.Intn(1 << PeerShortIDBits))
+		if peers.byShortID[shortID].peer == nil {
+			return shortID, true
+		}
+	}
+
+	// Looks like most short ids are used.  So count the number of
+	// unused ones, and pick one at random.
+	available := int(1 << PeerShortIDBits)
+	for _, entry := range peers.byShortID {
+		if entry.peer != nil {
+			available--
+		}
+	}
+
+	if available == 0 {
+		// All short ids are used.
+		return 0, false
+	}
+
+	n := rng.Intn(available)
+	var i PeerShortID
+	for {
+		if peers.byShortID[i].peer == nil {
+			if n == 0 {
+				return PeerShortID(i), true
+			}
+
+			n--
+		}
+
+		i++
+		if i == 0 {
+			panic("chooseShortID broken")
+		}
+	}
+}
+
+func (peers *Peers) unlockAndMaybeInvalidateShortIDs() {
+	pending := peers.invalidateShortIDsPending
+	peers.invalidateShortIDsPending = false
+	peers.Unlock()
+	if pending {
+		//  Some tests have a nil router
+		router := peers.ourself.router
+		if router != nil {
+			router.InterHost.InvalidateShortIDs()
+		}
+	}
+}
+
 func (peers *Peers) FetchWithDefault(peer *Peer) *Peer {
 	peers.Lock()
-	defer peers.Unlock()
-	if existingPeer, found := peers.table[peer.Name]; found {
+	defer peers.unlockAndMaybeInvalidateShortIDs()
+	if existingPeer, found := peers.byName[peer.Name]; found {
 		if existingPeer.UID != peer.UID {
 			return nil
 		}
 		existingPeer.localRefCount++
 		return existingPeer
 	}
-	peers.table[peer.Name] = peer
+
+	peers.byName[peer.Name] = peer
+	peers.addByShortID(peer)
 	peer.localRefCount++
 	return peer
 }
 
-func (peers *Peers) Fetch(name PeerName) (*Peer, bool) {
-	peers.RLock()
-	defer peers.RUnlock()
-	peer, found := peers.table[name]
-	return peer, found // GRRR, why can't I inline this!?
-}
-
 func (peers *Peers) Dereference(peer *Peer) {
 	peers.Lock()
 	defer peers.Unlock()
@@ -72,7 +251,7 @@ func (peers *Peers) Dereference(peer *Peer) {
 func (peers *Peers) ForEach(fun func(*Peer)) {
 	peers.RLock()
 	defer peers.RUnlock()
-	for _, peer := range peers.table {
+	for _, peer := range peers.byName {
 		fun(peer)
 	}
 }
@@ -82,7 +261,7 @@ func (peers *Peers) Names() PeerNameSet {
 	defer peers.RUnlock()
 
 	names := make(PeerNameSet)
-	for name := range peers.table {
+	for name := range peers.byName {
 		names[name] = void
 	}
 
@@ -96,7 +275,7 @@ func (peers *Peers) EncodePeers(names PeerNameSet) []byte {
 	defer peers.RUnlock()
 	peerList := make([]*Peer, 0, len(names))
 	for name := range names {
-		if peer, found := peers.table[name]; found {
+		if peer, found := peers.byName[name]; found {
 			if peer == peers.ourself.Peer {
 				peers.ourself.Peer.Encode(enc)
 			} else {
@@ -110,7 +289,7 @@ func (peers *Peers) EncodePeers(names PeerNameSet) []byte {
 
 func (peers *Peers) GarbageCollect() []*Peer {
 	peers.Lock()
-	defer peers.Unlock()
+	defer peers.unlockAndMaybeInvalidateShortIDs()
 	return peers.garbageCollect()
 }
 
@@ -147,13 +326,22 @@ func (peers *Peers) garbageCollect() []*Peer {
 	peers.ourself.RLock()
 	_, reached := peers.ourself.Routes(nil, false)
 	peers.ourself.RUnlock()
-	for name, peer := range peers.table {
+
+	for name, peer := range peers.byName {
 		if _, found := reached[peer.Name]; !found && peer.localRefCount == 0 {
-			delete(peers.table, name)
+			delete(peers.byName, name)
+			peers.deleteByShortID(peer)
 			peers.onGC(peer)
 			removed = append(removed, peer)
 		}
 	}
+
+	if peers.byShortID[peers.ourself.ShortID].peer != peers.ourself.Peer {
+		// The local peer doesn't own its short id.  Garbage
+		// collection might have freed some up, so try to reassign.
+		peers.reassignLocalShortID()
+	}
+
 	return removed
 }
 
@@ -165,7 +353,7 @@ func (peers *Peers) garbageCollect() []*Peer {
 // "improved" update containing just these new/updated elements.
 func (peers *Peers) ApplyUpdate(update []byte) (PeerNameSet, PeerNameSet, error) {
 	peers.Lock()
-	defer peers.Unlock()
+	defer peers.unlockAndMaybeInvalidateShortIDs()
 
 	newPeers, decodedUpdate, decodedConns, err := peers.decodeUpdate(update)
 	if err != nil {
@@ -176,15 +364,16 @@ func (peers *Peers) ApplyUpdate(update []byte) (PeerNameSet, PeerNameSet, error)
 	// have no knowledge of. We can now apply the update. Start by
 	// adding in any new peers into the cache.
 	for name, newPeer := range newPeers {
-		peers.table[name] = newPeer
+		peers.byName[name] = newPeer
+		peers.addByShortID(newPeer)
 	}
 
 	newUpdate := make(PeerNameSet)
 	for idx, newPeer := range decodedUpdate {
 		connSummaries := decodedConns[idx]
 		name := newPeer.Name
-		// guaranteed to find peer in the peers.table
-		peer := peers.table[name]
+		// guaranteed to find peer in the peers.byName
+		peer := peers.byName[name]
 		if peer != newPeer &&
 			(peer == peers.ourself.Peer || peer.Version >= newPeer.Version) {
 			// Nobody but us updates us. And if we know more about a
@@ -203,7 +392,14 @@ func (peers *Peers) ApplyUpdate(update []byte) (PeerNameSet, PeerNameSet, error)
 		// router.Peers.ApplyUpdate. But ApplyUpdate takes the Lock on
 		// the router.Peers, so there can be no race here.
 		peer.Version = newPeer.Version
-		peer.connections = makeConnsMap(peer, connSummaries, peers.table)
+		peer.connections = makeConnsMap(peer, connSummaries, peers.byName)
+
+		if newPeer.ShortID != peer.ShortID {
+			peers.deleteByShortID(peer)
+			peer.ShortID = newPeer.ShortID
+			peers.addByShortID(peer)
+		}
+
 		newUpdate[name] = void
 	}
 
@@ -239,7 +435,7 @@ func (peers *Peers) decodeUpdate(update []byte) (newPeers map[PeerName]*Peer, de
 		newPeer := NewPeerFromSummary(peerSummary)
 		decodedUpdate = append(decodedUpdate, newPeer)
 		decodedConns = append(decodedConns, connSummaries)
-		existingPeer, found := peers.table[newPeer.Name]
+		existingPeer, found := peers.byName[newPeer.Name]
 		if !found {
 			newPeers[newPeer.Name] = newPeer
 		} else if existingPeer.UID != newPeer.UID {
@@ -254,7 +450,7 @@ func (peers *Peers) decodeUpdate(update []byte) (newPeers map[PeerName]*Peer, de
 			if _, found := newPeers[remoteName]; found {
 				continue
 			}
-			if _, found := peers.table[remoteName]; found {
+			if _, found := peers.byName[remoteName]; found {
 				continue
 			}
 			// Update refers to a peer which we have no knowledge
@@ -299,11 +495,11 @@ func decodePeer(dec *gob.Decoder) (peerSummary PeerSummary, connSummaries []Conn
 	return
 }
 
-func makeConnsMap(peer *Peer, connSummaries []ConnectionSummary, table map[PeerName]*Peer) map[PeerName]Connection {
+func makeConnsMap(peer *Peer, connSummaries []ConnectionSummary, byName map[PeerName]*Peer) map[PeerName]Connection {
 	conns := make(map[PeerName]Connection)
 	for _, connSummary := range connSummaries {
 		name := PeerNameFromBin(connSummary.NameByte)
-		remotePeer := table[name]
+		remotePeer := byName[name]
 		conn := NewRemoteConnection(peer, remotePeer, connSummary.RemoteTCPAddr, connSummary.Outbound, connSummary.Established)
 		conns[name] = conn
 	}
diff --git a/router/peers_test.go b/router/peers_test.go
index f0edaed0..3189c2bd 100644
--- a/router/peers_test.go
+++ b/router/peers_test.go
@@ -5,6 +5,7 @@ import (
 	wt "github.com/weaveworks/weave/testing"
 	"math/rand"
 	"testing"
+	"time"
 )
 
 // TODO we should also test:
@@ -110,3 +111,240 @@ func TestPeersGarbageCollection(t *testing.T) {
 	ps1.DeleteTestConnection(p3)
 	checkPeerArray(t, ps1.GarbageCollect(), p3)
 }
+
+func TestShortIDCollisions(t *testing.T) {
+	rng := rand.New(rand.NewSource(time.Now().UnixNano()))
+	_, peers := newNode(PeerName(1 << PeerShortIDBits))
+
+	// Make enough peers that short id collisions are
+	// overwhelmingly likely
+	ps := make([]*Peer, 1<<PeerShortIDBits)
+	for i := 0; i < 1<<PeerShortIDBits; i++ {
+		ps[i] = NewPeer(PeerName(i), "", PeerUID(i), 0,
+			PeerShortID(rng.Intn(1<<PeerShortIDBits)))
+	}
+
+	shuffle := func() {
+		for i := range ps {
+			j := rng.Intn(i + 1)
+			ps[i], ps[j] = ps[j], ps[i]
+		}
+	}
+
+	// Fill peers
+	shuffle()
+	for _, p := range ps {
+		peers.addByShortID(p)
+	}
+
+	// Check invariants
+	counts := make([]int, 1<<PeerShortIDBits)
+	saw := func(p *Peer) {
+		if p != peers.ourself.Peer {
+			counts[p.UID]++
+		}
+	}
+
+	for shortID, entry := range peers.byShortID {
+		if entry.peer == nil {
+			// no principal peer for this short id, so
+			// others must be empty
+			if len(entry.others) != 0 {
+				t.Fatal()
+			}
+
+			continue
+		}
+
+		if entry.peer.ShortID != shortID {
+			t.Fatal()
+		}
+
+		saw(entry.peer)
+
+		for _, p := range entry.others {
+			saw(p)
+
+			if p.ShortID != shortID {
+				t.Fatal()
+			}
+
+			// the principal peer should have the lowest name
+			if p.Name < entry.peer.Name {
+				t.Fatal()
+			}
+		}
+	}
+
+	for _, n := range counts {
+		if n != 1 {
+			t.Fatal()
+		}
+	}
+
+	// Delete all the peers
+	shuffle()
+	for _, p := range ps {
+		peers.deleteByShortID(p)
+	}
+
+	for _, entry := range peers.byShortID {
+		if entry.peer != nil && entry.peer != peers.ourself.Peer {
+			t.Fatal()
+		}
+
+		if len(entry.others) != 0 {
+			t.Fatal()
+		}
+	}
+}
+
+// Test the easy case of short id reassignment, when few short ids are taken
+func TestShortIDReassignmentEasy(t *testing.T) {
+	rng := rand.New(rand.NewSource(time.Now().UnixNano()))
+	_, peers := newNode(PeerName(0))
+
+	for i := 1; i <= 10; i++ {
+		peers.FetchWithDefault(NewPeer(PeerName(i), "", PeerUID(i), 0,
+			PeerShortID(rng.Intn(1<<PeerShortIDBits))))
+	}
+
+	checkShortIDReassignment(t, peers)
+}
+
+// Test the hard case of short id reassignment, when most short ids are taken
+func TestShortIDReassignmentHard(t *testing.T) {
+	rng := rand.New(rand.NewSource(time.Now().UnixNano()))
+	_, peers := newNode(PeerName(1 << PeerShortIDBits))
+
+	// Take all short ids
+	ps := make([]*Peer, 1<<PeerShortIDBits)
+	for i := 0; i < 1<<PeerShortIDBits; i++ {
+		ps[i] = NewPeer(PeerName(i), "", PeerUID(i), 0,
+			PeerShortID(i))
+		peers.addByShortID(ps[i])
+	}
+
+	// As all short ids are taken, an attempted reassigment won't
+	// do anything
+	oldShortID := peers.ourself.ShortID
+	peers.reassignLocalShortID()
+	if peers.ourself.ShortID != oldShortID {
+		t.Fatal()
+	}
+
+	// Free up a few ids
+	for i := 0; i < 10; i++ {
+		x := rng.Intn(len(ps))
+		if ps[x] != nil {
+			peers.deleteByShortID(ps[x])
+			ps[x] = nil
+		}
+	}
+
+	checkShortIDReassignment(t, peers)
+}
+
+func checkShortIDReassignment(t *testing.T, peers *Peers) {
+	oldShortID := peers.ourself.ShortID
+	peers.reassignLocalShortID()
+	if peers.ourself.ShortID == oldShortID {
+		t.Fatal()
+	}
+
+	if peers.byShortID[peers.ourself.ShortID].peer != peers.ourself.Peer {
+		t.Fatal()
+	}
+}
+
+func TestShortIDInvalidation(t *testing.T) {
+	_, peers := newNode(PeerName(1 << PeerShortIDBits))
+
+	// need to use a short id that is not the local peer's
+	shortID := peers.ourself.ShortID + 1
+
+	assertInvalidateShortIDs := func(expect bool) {
+		if peers.invalidateShortIDsPending != expect {
+			t.Fatal()
+		}
+		peers.invalidateShortIDsPending = false
+	}
+
+	// The use of a fresh short id does not cause invalidation
+	a := NewPeer(PeerName(1), "", PeerUID(1), 0, shortID)
+	peers.addByShortID(a)
+	assertInvalidateShortIDs(false)
+
+	// An addition which does not change the mapping
+	b := NewPeer(PeerName(2), "", PeerUID(2), 0, shortID)
+	peers.addByShortID(b)
+	assertInvalidateShortIDs(false)
+
+	// An addition which does change the mapping
+	c := NewPeer(PeerName(0), "", PeerUID(0), 0, shortID)
+	peers.addByShortID(c)
+	assertInvalidateShortIDs(true)
+
+	// A deletion which does not change the mapping
+	peers.deleteByShortID(b)
+	assertInvalidateShortIDs(false)
+
+	// A deletion which does change the mapping
+	peers.deleteByShortID(c)
+	assertInvalidateShortIDs(true)
+
+	// Deleting the last peer with a short id does not cause invalidation
+	peers.deleteByShortID(a)
+	assertInvalidateShortIDs(false)
+
+	// But subsequent reuse that a short id does cause invalidation
+	peers.addByShortID(a)
+	assertInvalidateShortIDs(true)
+}
+
+func TestShortIDPropagation(t *testing.T) {
+	_, peers1 := newNode(PeerName(0))
+	_, peers2 := newNode(PeerName(1))
+
+	peers1.AddTestConnection(peers2.ourself.Peer)
+	peers1.ApplyUpdate(peers2.EncodePeers(peers2.Names()))
+	peers12, _ := peers1.Fetch(PeerName(1))
+	old := peers12.PeerSummary
+
+	peers2.reassignLocalShortID()
+	peers1.ApplyUpdate(peers2.EncodePeers(peers2.Names()))
+	if peers12.Version == old.Version || peers12.ShortID == old.ShortID {
+		t.Fatal()
+	}
+}
+
+// Test the case where all short ids are taken, but then some peers go
+// away, so the local peer reassigns
+func TestDeferredShortIDReassignment(t *testing.T) {
+	rng := rand.New(rand.NewSource(time.Now().UnixNano()))
+	_, us := newNode(PeerName(1 << PeerShortIDBits))
+
+	// Connect us to other peers occupying all short ids
+	others := make([]*Peers, 1<<PeerShortIDBits)
+	for i := range others {
+		_, others[i] = newNode(PeerName(i))
+		others[i].setLocalShortID(PeerShortID(i))
+		us.AddTestConnection(others[i].ourself.Peer)
+	}
+
+	// Check that, as expected, the local peer does not own its
+	// short id
+	if us.byShortID[us.ourself.ShortID].peer == us.ourself.Peer {
+		t.Fatal()
+	}
+
+	// Disconnect one peer, and we should now be able to claim its
+	// short id
+	other := others[rng.Intn(1<<PeerShortIDBits)]
+	us.DeleteTestConnection(other.ourself.Peer)
+	us.GarbageCollect()
+
+	if us.byShortID[us.ourself.ShortID].peer != us.ourself.Peer {
+		t.Fatal()
+	}
+}
diff --git a/router/routes.go b/router/routes.go
index 2611291f..903eeebb 100644
--- a/router/routes.go
+++ b/router/routes.go
@@ -225,7 +225,7 @@ func (routes *Routes) calculateUnicast(establishedAndSymmetric bool) unicastRout
 // where <= is the subset relationship on keys of the returned map.
 func (routes *Routes) calculateBroadcast(establishedAndSymmetric bool) broadcastRoutes {
 	broadcast := make(broadcastRoutes)
-	for _, peer := range routes.peers.table {
+	for _, peer := range routes.peers.byName {
 		hops := []PeerName{}
 		if found, reached := peer.Routes(routes.ourself.Peer, establishedAndSymmetric); found {
 			routes.ourself.ForEachConnectedPeer(establishedAndSymmetric, reached,
diff --git a/router/sleeve.go b/router/sleeve.go
index 0f0a28c6..009a2d6f 100644
--- a/router/sleeve.go
+++ b/router/sleeve.go
@@ -90,6 +90,10 @@ func (*SleeveInterHost) InvalidateRoutes() {
 	// no cached information, so nothing to do
 }
 
+func (*SleeveInterHost) InvalidateShortIDs() {
+	// no cached information, so nothing to do
+}
+
 func (sleeve *SleeveInterHost) logFrame(string, []byte, *layers.Ethernet) {
 	// XXX
 }