Merge pull request #1433 from mrjana/overlay

Ensure purging neighbor cache for stale deletes

Author: Santhosh Manohar

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