diff --git a/router/fastdp.go b/router/fastdp.go index 4cb0fd4933..de3112cff8 100644 --- a/router/fastdp.go +++ b/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 { diff --git a/router/mac_cache.go b/router/mac_cache.go index 31510c6e9a..362468809b 100644 --- a/router/mac_cache.go +++ b/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,11 +51,10 @@ 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 } @@ -63,15 +62,7 @@ func (cache *MacCache) add(mac net.HardwareAddr, peer *mesh.Peer, force bool) (b 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 { diff --git a/router/network_router.go b/router/network_router.go index e9a85e16a3..0b8ade50b3 100644 --- a/router/network_router.go +++ b/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 { diff --git a/test/155_migrate_mac_2_test.sh b/test/155_migrate_mac_2_test.sh new file mode 100755 index 0000000000..f5c86a264b --- /dev/null +++ b/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