Deal with moving MACs

router/fastdp.go

@@ -303,13 +303,20 @@ func (fastdp fastDatapathOverlay) InvalidateRoutes() {
 	log.Debug("InvalidateRoutes")
 	fastdp.lock.Lock()
 	defer fastdp.lock.Unlock()
-	checkWarn(fastdp.deleteFlows())
+	fastdp.deleteFlowsAndMACs()
 }
 
 func (fastdp fastDatapathOverlay) InvalidateShortIDs() {
 	log.Debug("InvalidateShortIDs")
 	fastdp.lock.Lock()
 	defer fastdp.lock.Unlock()
+	fastdp.deleteFlowsAndMACs()
+}
+
+// NB: The fastdp lock has to be taken before calling.
+func (fastdp *FastDatapath) deleteFlowsAndMACs() {
+	fastdp.sendToMAC = make(map[MAC]bridgeSender)
+	fastdp.seenMACs = make(map[MAC]struct{})
 	checkWarn(fastdp.deleteFlows())
 }
 
@@ -1040,7 +1047,7 @@ func (fastdp *FastDatapath) makeBridgeVport(vport odp.Vport) {
 
 	// Delete flows, in order to recalculate flows for broadcasts
 	// on the bridge.
-	checkWarn(fastdp.deleteFlows())
+	fastdp.deleteFlowsAndMACs()
 
 	// Packets coming from the netdev are processed by the bridge
 	fastdp.missHandlers[vportID] = func(flowKeys odp.FlowKeys, lock *fastDatapathLock) FlowOp {

router/mac_cache.go

@@ -30,7 +30,7 @@ func NewMacCache(maxAge time.Duration, onExpiry func(net.HardwareAddr, *mesh.Pee
 	return cache
 }
 
-func (cache *MacCache) add(mac net.HardwareAddr, peer *mesh.Peer, force bool) (bool, *mesh.Peer) {
+func (cache *MacCache) Add(mac net.HardwareAddr, peer *mesh.Peer) (bool, *mesh.Peer) {
 	key := macint(mac)
 	now := time.Now()
 
@@ -51,27 +51,18 @@ func (cache *MacCache) add(mac net.HardwareAddr, peer *mesh.Peer, force bool) (b
 		return true, nil
 	}
 
-	if entry.peer != peer {
-		if !force {
-			return false, entry.peer
-		}
+	var conflictPeer *mesh.Peer
 
+	if entry.peer != peer {
+		conflictPeer = entry.peer
 		entry.peer = peer
 	}
 
 	if now.After(entry.lastSeen.Add(cache.maxAge / 10)) {
 		entry.lastSeen = now
 	}
 
-	return false, nil
-}
-
-func (cache *MacCache) Add(mac net.HardwareAddr, peer *mesh.Peer) (bool, *mesh.Peer) {
-	return cache.add(mac, peer, false)
-}
-
-func (cache *MacCache) AddForced(mac net.HardwareAddr, peer *mesh.Peer) (bool, *mesh.Peer) {
-	return cache.add(mac, peer, true)
+	return false, conflictPeer
 }
 
 func (cache *MacCache) Lookup(mac net.HardwareAddr) *mesh.Peer {

router/network_router.go

@@ -81,26 +81,15 @@ func (router *NetworkRouter) Start() {
 
 func (router *NetworkRouter) handleCapturedPacket(key PacketKey) FlowOp {
 	router.PacketLogging.LogPacket("Captured", key)
-	srcMac := net.HardwareAddr(key.SrcMAC[:])
-	dstMac := net.HardwareAddr(key.DstMAC[:])
 
-	switch newSrcMac, conflictPeer := router.Macs.Add(srcMac, router.Ourself.Peer); {
-	case newSrcMac:
-		log.Println("Discovered local MAC", srcMac)
-	case conflictPeer != nil:
-		// The MAC cache has an entry for the source MAC
-		// associated with another peer.  This probably means
-		// we are seeing a frame we injected ourself.  That
-		// shouldn't happen, but discard it just in case.
-		log.Error("Captured frame from MAC (", srcMac, ") to (", dstMac, ") associated with another peer ", conflictPeer)
-		return DiscardingFlowOp{}
-	}
+	router.learnMAC(key, router.Ourself.Peer)
 
 	// Discard STP broadcasts
 	if key.DstMAC == [...]byte{0x01, 0x80, 0xC2, 0x00, 0x00, 0x00} {
 		return DiscardingFlowOp{}
 	}
 
+	dstMac := net.HardwareAddr(key.DstMAC[:])
 	switch dstPeer := router.Macs.Lookup(dstMac); dstPeer {
 	case router.Ourself.Peer:
 		// The packet is destined for a local MAC.  The bridge
@@ -124,6 +113,11 @@ func (router *NetworkRouter) handleCapturedPacket(key PacketKey) FlowOp {
 }
 
 func (router *NetworkRouter) handleForwardedPacket(key ForwardPacketKey) FlowOp {
+	if key.SrcPeer == router.Ourself.Peer {
+		// Might happen when a MAC has moved immediately after sending a packet.
+		log.Warning("Detected loop at ", router.Ourself.Peer, " (", key, ")")
+	}
+
 	if key.DstPeer != router.Ourself.Peer {
 		// it's not for us, we're just relaying it
 		router.PacketLogging.LogForwardPacket("Relaying", key)
@@ -134,21 +128,11 @@ func (router *NetworkRouter) handleForwardedPacket(key ForwardPacketKey) FlowOp
 	// (because the DstPeer on a forwarded broadcast packet is
 	// always set to the peer being forwarded to)
 
-	srcMac := net.HardwareAddr(key.SrcMAC[:])
-	dstMac := net.HardwareAddr(key.DstMAC[:])
-
-	switch newSrcMac, conflictPeer := router.Macs.AddForced(srcMac, key.SrcPeer); {
-	case newSrcMac:
-		log.Print("Discovered remote MAC ", srcMac, " at ", key.SrcPeer)
-	case conflictPeer != nil:
-		log.Print("Discovered remote MAC ", srcMac, " at ", key.SrcPeer, " (was at ", conflictPeer, ")")
-		// We need to clear out any flows destined to the MAC
-		// that forward to the old peer.
-		router.Overlay.(NetworkOverlay).InvalidateRoutes()
-	}
+	router.learnMAC(key.PacketKey, key.SrcPeer)
 
 	router.PacketLogging.LogForwardPacket("Injecting", key)
 	injectFop := router.Bridge.InjectPacket(key.PacketKey)
+	dstMac := net.HardwareAddr(key.DstMAC[:])
 	dstPeer := router.Macs.Lookup(dstMac)
 	if dstPeer == router.Ourself.Peer {
 		return injectFop
@@ -169,6 +153,21 @@ func (router *NetworkRouter) handleForwardedPacket(key ForwardPacketKey) FlowOp
 	}
 }
 
+func (router *NetworkRouter) learnMAC(key PacketKey, srcPeer *mesh.Peer) {
+	srcMac := net.HardwareAddr(key.SrcMAC[:])
+
+	switch newSrcMac, conflictPeer := router.Macs.Add(srcMac, srcPeer); {
+	case newSrcMac:
+		log.Print("Discovered MAC ", srcMac, " at ", srcPeer)
+	case conflictPeer != nil:
+		// If it's a local MAC, then the MAC cache has an entry for the source
+		// MAC associated with another peer. This can happen when a MAC has moved.
+		log.Print("Discovered MAC ", srcMac, " at ", srcPeer, " (was at ", conflictPeer, ")")
+		// We need to clear out any flows with the MAC as the source.
+		router.Overlay.(NetworkOverlay).InvalidateRoutes()
+	}
+}
+
 // Routing
 
 func (router *NetworkRouter) relay(key ForwardPacketKey) FlowOp {

test/155_migrate_mac_2_test.sh

@@ -0,0 +1,42 @@
+#! /bin/bash
+
+. ./config.sh
+
+C1=10.2.1.1
+C2=10.2.1.2
+C3=10.2.1.128
+
+assert_migration() {
+    start_container $HOST1 $C1/24 --name=c1
+    start_container $HOST1 $C2/24 --name=c2
+    start_container $HOST2 $C3/24 --name=c3 --privileged
+
+    assert_raises "exec_on $HOST2 c3 $PING $C2"
+    MAC=$(exec_on $HOST1 c1 ip link show ethwe | sed -n -e 's|^ *link/ether \([0-9a-f:]*\).*|\1|p')
+    docker_on $HOST1 rm -f c1
+    exec_on $HOST2 c3 ip link set ethwe address $MAC
+    assert_raises "exec_on $HOST2 c3 $PING $C2"
+}
+
+start_suite "Container MAC migration"
+
+# Test with fastdp
+
+weave_on $HOST1 launch
+weave_on $HOST2 launch $HOST1
+assert_migration
+
+# Cleanup
+
+docker_on $HOST1 rm -f c2
+docker_on $HOST2 rm -f c3
+weave_on $HOST1 reset
+weave_on $HOST2 reset
+
+# Test with sleeve
+
+WEAVE_NO_FASTDP=1 weave_on $HOST1 launch
+WEAVE_NO_FASTDP=1 weave_on $HOST2 launch $HOST1
+assert_migration
+
+end_suite