Ensure purging neighbor cache for stale deletes

When stale delete notifications are received, we still need to make sure
to purge sandbox neighbor cache because these stale deletes are most
typically out of order delete notifications and if an add for the
peermac was received before the delete of the old peermac,vtep pair then
we process that and replace the kernel state but the old neighbor state
in the sandbox cache remains. That needs to be purged when we finally
get the out of order delete notification.

Signed-off-by: Jana Radhakrishnan <mrjana@docker.com>

Author: mrjana

drivers/overlay/peerdb.go

@@ -168,14 +168,14 @@ func (d *driver) peerDbAdd(nid, eid string, peerIP net.IP, peerIPMask net.IPMask
 }
 
 func (d *driver) peerDbDelete(nid, eid string, peerIP net.IP, peerIPMask net.IPMask,
-	peerMac net.HardwareAddr, vtep net.IP) bool {
+	peerMac net.HardwareAddr, vtep net.IP) peerEntry {
 	peerDbWg.Wait()
 
 	d.peerDb.Lock()
 	pMap, ok := d.peerDb.mp[nid]
 	if !ok {
 		d.peerDb.Unlock()
-		return false
+		return peerEntry{}
 	}
 	d.peerDb.Unlock()
 
@@ -186,19 +186,20 @@ func (d *driver) peerDbDelete(nid, eid string, peerIP net.IP, peerIPMask net.IPM
 
 	pMap.Lock()
 
-	if pEntry, ok := pMap.mp[pKey.String()]; ok {
+	pEntry, ok := pMap.mp[pKey.String()]
+	if ok {
 		// Mismatched endpoint ID(possibly outdated). Do not
 		// delete peerdb
 		if pEntry.eid != eid {
 			pMap.Unlock()
-			return false
+			return pEntry
 		}
 	}
 
 	delete(pMap.mp, pKey.String())
 	pMap.Unlock()
 
-	return true
+	return pEntry
 }
 
 func (d *driver) peerDbUpdateSandbox(nid string) {
@@ -312,10 +313,9 @@ func (d *driver) peerDelete(nid, eid string, peerIP net.IP, peerIPMask net.IPMas
 		return err
 	}
 
+	var pEntry peerEntry
 	if updateDb {
-		if !d.peerDbDelete(nid, eid, peerIP, peerIPMask, peerMac, vtep) {
-			return nil
-		}
+		pEntry = d.peerDbDelete(nid, eid, peerIP, peerIPMask, peerMac, vtep)
 	}
 
 	n := d.network(nid)
@@ -328,14 +328,24 @@ func (d *driver) peerDelete(nid, eid string, peerIP net.IP, peerIPMask net.IPMas
 		return nil
 	}
 
-	// Delete fdb entry to the bridge for the peer mac
-	if err := sbox.DeleteNeighbor(vtep, peerMac); err != nil {
-		return fmt.Errorf("could not delete fdb entry into the sandbox: %v", err)
+	// Delete fdb entry to the bridge for the peer mac only if the
+	// entry existed in local peerdb. If it is a stale delete
+	// request, still call DeleteNeighbor but only to cleanup any
+	// leftover sandbox neighbor cache and not actually delete the
+	// kernel state.
+	if (eid == pEntry.eid && vtep.Equal(pEntry.vtep)) ||
+		(eid != pEntry.eid && !vtep.Equal(pEntry.vtep)) {
+		if err := sbox.DeleteNeighbor(vtep, peerMac,
+			eid == pEntry.eid && vtep.Equal(pEntry.vtep)); err != nil {
+			return fmt.Errorf("could not delete fdb entry into the sandbox: %v", err)
+		}
 	}
 
 	// Delete neighbor entry for the peer IP
-	if err := sbox.DeleteNeighbor(peerIP, peerMac); err != nil {
-		return fmt.Errorf("could not delete neighbor entry into the sandbox: %v", err)
+	if eid == pEntry.eid {
+		if err := sbox.DeleteNeighbor(peerIP, peerMac, true); err != nil {
+			return fmt.Errorf("could not delete neighbor entry into the sandbox: %v", err)
+		}
 	}
 
 	if err := d.checkEncryption(nid, vtep, 0, false, false); err != nil {

osl/neigh_linux.go

@@ -32,7 +32,7 @@ func (n *networkNamespace) findNeighbor(dstIP net.IP, dstMac net.HardwareAddr) *
 	return nil
 }
 
-func (n *networkNamespace) DeleteNeighbor(dstIP net.IP, dstMac net.HardwareAddr) error {
+func (n *networkNamespace) DeleteNeighbor(dstIP net.IP, dstMac net.HardwareAddr, osDelete bool) error {
 	var (
 		iface netlink.Link
 		err   error
@@ -43,42 +43,46 @@ func (n *networkNamespace) DeleteNeighbor(dstIP net.IP, dstMac net.HardwareAddr)
 		return fmt.Errorf("could not find the neighbor entry to delete")
 	}
 
-	n.Lock()
-	nlh := n.nlHandle
-	n.Unlock()
+	if osDelete {
+		n.Lock()
+		nlh := n.nlHandle
+		n.Unlock()
 
-	if nh.linkDst != "" {
-		iface, err = nlh.LinkByName(nh.linkDst)
-		if err != nil {
-			return fmt.Errorf("could not find interface with destination name %s: %v",
-				nh.linkDst, err)
+		if nh.linkDst != "" {
+			iface, err = nlh.LinkByName(nh.linkDst)
+			if err != nil {
+				return fmt.Errorf("could not find interface with destination name %s: %v",
+					nh.linkDst, err)
+			}
 		}
-	}
 
-	nlnh := &netlink.Neigh{
-		IP:     dstIP,
-		State:  netlink.NUD_PERMANENT,
-		Family: nh.family,
-	}
+		nlnh := &netlink.Neigh{
+			IP:     dstIP,
+			State:  netlink.NUD_PERMANENT,
+			Family: nh.family,
+		}
 
-	if nlnh.Family > 0 {
-		nlnh.HardwareAddr = dstMac
-		nlnh.Flags = netlink.NTF_SELF
-	}
+		if nlnh.Family > 0 {
+			nlnh.HardwareAddr = dstMac
+			nlnh.Flags = netlink.NTF_SELF
+		}
 
-	if nh.linkDst != "" {
-		nlnh.LinkIndex = iface.Attrs().Index
-	}
+		if nh.linkDst != "" {
+			nlnh.LinkIndex = iface.Attrs().Index
+		}
 
-	if err := nlh.NeighDel(nlnh); err != nil {
-		return fmt.Errorf("could not delete neighbor entry: %v", err)
+		if err := nlh.NeighDel(nlnh); err != nil {
+			return fmt.Errorf("could not delete neighbor entry: %v", err)
+		}
 	}
 
+	n.Lock()
 	for i, nh := range n.neighbors {
 		if nh.dstIP.Equal(dstIP) && bytes.Equal(nh.dstMac, dstMac) {
 			n.neighbors = append(n.neighbors[:i], n.neighbors[i+1:]...)
 		}
 	}
+	n.Unlock()
 
 	return nil
 }

osl/sandbox.go

@@ -42,7 +42,7 @@ type Sandbox interface {
 	AddNeighbor(dstIP net.IP, dstMac net.HardwareAddr, option ...NeighOption) error
 
 	// DeleteNeighbor deletes neighbor entry from the sandbox.
-	DeleteNeighbor(dstIP net.IP, dstMac net.HardwareAddr) error
+	DeleteNeighbor(dstIP net.IP, dstMac net.HardwareAddr, osDelete bool) error
 
 	// Returns an interface with methods to set neighbor options.
 	NeighborOptions() NeighborOptionSetter