From d2dfb0d15d39a2bb690d00ff190e586c7aadda4d Mon Sep 17 00:00:00 2001 From: David Wragg Date: Wed, 21 Oct 2015 13:46:18 +0100 Subject: [PATCH] Do InvalidateRoutes when a forwarded packet updates a MAC cache entry When we get a forwarded packet, we put its source MAC into the MAC cache, overriding any existing entry. But we could have a flow which says to forward packets destined to that MAC based on the old entry. Such a flow could be prolonged indefinitely by attempts to send to the MAC. This is obviously bad, so do InvalidateRoutes in such cases. --- router/mac_cache.go | 5 ++--- router/router.go | 15 ++++++++++++--- 2 files changed, 14 insertions(+), 6 deletions(-) diff --git a/router/mac_cache.go b/router/mac_cache.go index d0eb7684..ecc9afb3 100644 --- a/router/mac_cache.go +++ b/router/mac_cache.go @@ -68,9 +68,8 @@ func (cache *MacCache) Add(mac net.HardwareAddr, peer *Peer) (bool, *Peer) { return cache.add(mac, peer, false) } -func (cache *MacCache) AddForced(mac net.HardwareAddr, peer *Peer) bool { - newMac, _ := cache.add(mac, peer, true) - return newMac +func (cache *MacCache) AddForced(mac net.HardwareAddr, peer *Peer) (bool, *Peer) { + return cache.add(mac, peer, true) } func (cache *MacCache) Lookup(mac net.HardwareAddr) *Peer { diff --git a/router/router.go b/router/router.go index 73ea1673..5e135a6d 100644 --- a/router/router.go +++ b/router/router.go @@ -113,7 +113,7 @@ func (router *Router) handleCapturedPacket(key PacketKey) FlowOp { // 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.Errorln("Captured frame from MAC (", srcMac, ") associated with another peer", conflictPeer) + log.Error("Captured frame from MAC (", srcMac, ") associated with another peer ", conflictPeer) return DiscardingFlowOp{} } @@ -187,8 +187,17 @@ func (router *Router) handleForwardedPacket(key ForwardPacketKey) FlowOp { srcMac := net.HardwareAddr(key.SrcMAC[:]) dstMac := net.HardwareAddr(key.DstMAC[:]) - if router.Macs.AddForced(srcMac, key.SrcPeer) { - log.Println("Discovered remote MAC", srcMac, "at", key.SrcPeer) + + 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.InvalidateRoutes() } router.PacketLogging.LogForwardPacket("Injecting", key)