Revert "libnetwork/osl: stop tracking neighbor entries"

This reverts commit 0d6e7cd983. Signed-off-by: Rob Murray <rob.murray@docker.com>
2026-08-07 16:41:50 +00:00 · 2025-06-11 12:05:36 +01:00
parent ea818a7f6f
commit 5d6ae34753
3 changed files with 100 additions and 100 deletions
--- a/libnetwork/drivers/overlay/peerdb.go
+++ b/libnetwork/drivers/overlay/peerdb.go
@@ -375,16 +375,8 @@ func (d *driver) peerDeleteOp(nid, eid string, peerIP net.IP, peerIPMask net.IPM

 	// Local peers do not have any local configuration to delete
 	if !localPeer {
-		IP := &net.IPNet{
-			IP:   peerIP,
-			Mask: peerIPMask,
-		}
-		s := n.getSubnetforIP(IP)
-		if s == nil {
-			return fmt.Errorf("could not find the subnet %q in network %q", IP.String(), n.id)
-		}
 		// Remove fdb entry to the bridge for the peer mac
-		if err := sbox.DeleteNeighbor(vtep, peerMac, osl.WithLinkName(s.vxlanName)); err != nil {
+		if err := sbox.DeleteNeighbor(vtep, peerMac); err != nil {
 			if _, ok := err.(osl.NeighborSearchError); ok && dbEntries > 0 {
 				// We fall in here if there is a transient state and if the neighbor that is being deleted
 				// was never been configured into the kernel (we allow only 1 configuration at the time per <ip,mac> mapping)
@@ -394,7 +386,7 @@ func (d *driver) peerDeleteOp(nid, eid string, peerIP net.IP, peerIPMask net.IPM
 		}

 		// Delete neighbor entry for the peer IP
-		if err := sbox.DeleteNeighbor(peerIP, peerMac, osl.WithLinkName(s.vxlanName), osl.WithFamily(syscall.AF_BRIDGE)); err != nil {
+		if err := sbox.DeleteNeighbor(peerIP, peerMac); err != nil {
 			return fmt.Errorf("could not delete neighbor entry for nid:%s eid:%s into the sandbox:%v", nid, eid, err)
 		}
 	}
--- a/libnetwork/osl/namespace_linux.go
+++ b/libnetwork/osl/namespace_linux.go
@@ -232,6 +232,7 @@ type Namespace struct {
 	defRoute4SrcName    string
 	defRoute6SrcName    string
 	staticRoutes        []*types.StaticRoute
+	neighbors           []*neigh
 	isDefault           bool // isDefault is true when Namespace represents the host network namespace. It is safe to access it concurrently.
 	ipv6LoEnabledOnce   sync.Once
 	ipv6LoEnabledCached bool
--- a/libnetwork/osl/neigh_linux.go
+++ b/libnetwork/osl/neigh_linux.go
@@ -1,12 +1,12 @@
 package osl

 import (
+	"bytes"
 	"context"
 	"errors"
 	"fmt"
 	"net"
 	"os"
-	"strings"

 	"github.com/containerd/log"
 	"github.com/vishvananda/netlink"
@@ -14,113 +14,110 @@ import (

 // NeighborSearchError indicates that the neighbor is already present
 type NeighborSearchError struct {
-	ip       net.IP
-	mac      net.HardwareAddr
-	linkName string
-	present  bool
+	ip      net.IP
+	mac     net.HardwareAddr
+	present bool
 }

 func (n NeighborSearchError) Error() string {
-	var b strings.Builder
-	b.WriteString("neighbor entry ")
-	if n.present {
-		b.WriteString("already exists ")
-	} else {
-		b.WriteString("not found ")
-	}
-	b.WriteString("for IP ")
-	b.WriteString(n.ip.String())
-	b.WriteString(", mac ")
-	b.WriteString(n.mac.String())
-	if n.linkName != "" {
-		b.WriteString(", link ")
-		b.WriteString(n.linkName)
-	}
-	return b.String()
+	return fmt.Sprintf("Search neighbor failed for IP %v, mac %v, present in db:%t", n.ip, n.mac, n.present)
 }

-// DeleteNeighbor deletes a neighbor entry from the sandbox.
-//
-// To delete an entry inserted by [AddNeighbor] the caller must provide the same
-// parameters used to add it.
-func (n *Namespace) DeleteNeighbor(dstIP net.IP, dstMac net.HardwareAddr, options ...NeighOption) error {
-	nlnh, linkName, err := n.nlNeigh(dstIP, dstMac, options...)
-	if err != nil {
-		return err
+type neigh struct {
+	dstIP    net.IP
+	dstMac   net.HardwareAddr
+	linkName string
+	linkDst  string
+	family   int
+}
+
+func (n *Namespace) findNeighbor(dstIP net.IP, dstMac net.HardwareAddr) *neigh {
+	n.mu.Lock()
+	defer n.mu.Unlock()
+
+	for _, nh := range n.neighbors {
+		if nh.dstIP.Equal(dstIP) && bytes.Equal(nh.dstMac, dstMac) {
+			return nh
+		}
 	}

-	if err := n.nlHandle.NeighDel(nlnh); err != nil {
-		log.G(context.TODO()).WithFields(log.Fields{
-			"ip":    dstIP,
-			"mac":   dstMac,
-			"ifc":   linkName,
-			"error": err,
-		}).Warn("error deleting neighbor entry")
-		if errors.Is(err, os.ErrNotExist) {
-			return NeighborSearchError{dstIP, dstMac, linkName, false}
+	return nil
+}
+
+// DeleteNeighbor deletes neighbor entry from the sandbox.
+func (n *Namespace) DeleteNeighbor(dstIP net.IP, dstMac net.HardwareAddr) error {
+	nh := n.findNeighbor(dstIP, dstMac)
+	if nh == nil {
+		return NeighborSearchError{dstIP, dstMac, false}
+	}
+
+	var linkIndex int
+	if nh.linkDst != "" {
+		iface, err := n.nlHandle.LinkByName(nh.linkDst)
+		if err != nil {
+			return fmt.Errorf("could not find interface with destination name %s: %v", nh.linkDst, err)
 		}
-		return fmt.Errorf("could not delete neighbor %+v: %w", nlnh, err)
+		linkIndex = iface.Attrs().Index
+	}
+
+	nlnh := &netlink.Neigh{
+		LinkIndex: linkIndex,
+		IP:        dstIP,
+		State:     netlink.NUD_PERMANENT,
+		Family:    nh.family,
+	}
+
+	if nh.family > 0 {
+		nlnh.HardwareAddr = dstMac
+		nlnh.Flags = netlink.NTF_SELF
+	}
+
+	// If the kernel deletion fails for the neighbor entry still remove it
+	// from the namespace cache, otherwise kernel update can fail if the
+	// neighbor moves back to the same host again.
+	if err := n.nlHandle.NeighDel(nlnh); err != nil && !errors.Is(err, os.ErrNotExist) {
+		log.G(context.TODO()).Warnf("Deleting neighbor IP %s, mac %s failed, %v", dstIP, dstMac, err)
 	}

 	// Delete the dynamic entry in the bridge
-	if nlnh.Family > 0 {
-		nlnh.Flags = netlink.NTF_MASTER
-		if err := n.nlHandle.NeighDel(nlnh); err != nil && !errors.Is(err, os.ErrNotExist) {
-			log.G(context.TODO()).WithFields(log.Fields{
-				"ip":    dstIP,
-				"mac":   dstMac,
-				"ifc":   linkName,
-				"error": err,
-			}).Warn("error deleting dynamic neighbor entry")
+	if nh.family > 0 {
+		if err := n.nlHandle.NeighDel(&netlink.Neigh{
+			LinkIndex:    linkIndex,
+			IP:           dstIP,
+			Family:       nh.family,
+			HardwareAddr: dstMac,
+			Flags:        netlink.NTF_MASTER,
+		}); err != nil && !errors.Is(err, os.ErrNotExist) {
+			log.G(context.TODO()).WithError(err).Warn("error while deleting neighbor entry")
 		}
 	}

-	log.G(context.TODO()).WithFields(log.Fields{
-		"ip":  dstIP,
-		"mac": dstMac,
-		"ifc": linkName,
-	}).Debug("Neighbor entry deleted")
+	n.mu.Lock()
+	for i, neighbor := range n.neighbors {
+		if neighbor.dstIP.Equal(dstIP) && bytes.Equal(neighbor.dstMac, dstMac) {
+			n.neighbors = append(n.neighbors[:i], n.neighbors[i+1:]...)
+			break
+		}
+	}
+	n.mu.Unlock()
+	log.G(context.TODO()).Debugf("Neighbor entry deleted for IP %v, mac %v", dstIP, dstMac)

 	return nil
 }

 // AddNeighbor adds a neighbor entry into the sandbox.
 func (n *Namespace) AddNeighbor(dstIP net.IP, dstMac net.HardwareAddr, options ...NeighOption) error {
-	nlnh, linkName, err := n.nlNeigh(dstIP, dstMac, options...)
-	if err != nil {
-		return err
+	nh := n.findNeighbor(dstIP, dstMac)
+	if nh != nil {
+		log.G(context.TODO()).Warnf("Neighbor entry already present for IP %v, mac %v neighbor:%+v", dstIP, dstMac, nh)
+		return NeighborSearchError{dstIP, dstMac, true}
 	}

-	if err := n.nlHandle.NeighAdd(nlnh); err != nil {
-		if errors.Is(err, os.ErrExist) {
-			log.G(context.TODO()).WithFields(log.Fields{
-				"ip":    dstIP,
-				"mac":   dstMac,
-				"ifc":   linkName,
-				"neigh": fmt.Sprintf("%+v", nlnh),
-			}).Warn("Neighbor entry already present")
-			return NeighborSearchError{dstIP, dstMac, linkName, true}
-		} else {
-			return fmt.Errorf("could not add neighbor entry %+v: %w", nlnh, err)
-		}
+	nh = &neigh{
+		dstIP:  dstIP,
+		dstMac: dstMac,
 	}

-	log.G(context.TODO()).WithFields(log.Fields{
-		"ip":  dstIP,
-		"mac": dstMac,
-		"ifc": linkName,
-	}).Debug("Neighbor entry added")
-
-	return nil
-}
-
-type neigh struct {
-	linkName string
-	family   int
-}
-
-func (n *Namespace) nlNeigh(dstIP net.IP, dstMac net.HardwareAddr, options ...NeighOption) (*netlink.Neigh, string, error) {
-	var nh neigh
 	nh.processNeighOptions(options...)

 	nlnh := &netlink.Neigh{
@@ -135,16 +132,26 @@ func (n *Namespace) nlNeigh(dstIP net.IP, dstMac net.HardwareAddr, options ...Ne
 	}

 	if nh.linkName != "" {
-		linkDst := n.findDst(nh.linkName, false)
-		if linkDst == "" {
-			return nil, nh.linkName, fmt.Errorf("could not find the interface with name %s", nh.linkName)
+		nh.linkDst = n.findDst(nh.linkName, false)
+		if nh.linkDst == "" {
+			return fmt.Errorf("could not find the interface with name %s", nh.linkName)
 		}
-		iface, err := n.nlHandle.LinkByName(linkDst)
+
+		iface, err := n.nlHandle.LinkByName(nh.linkDst)
 		if err != nil {
-			return nil, nh.linkName, fmt.Errorf("could not find interface with destination name %s: %w", linkDst, err)
+			return fmt.Errorf("could not find interface with destination name %s: %v", nh.linkDst, err)
 		}
 		nlnh.LinkIndex = iface.Attrs().Index
 	}

-	return nlnh, nh.linkName, nil
+	if err := n.nlHandle.NeighSet(nlnh); err != nil {
+		return fmt.Errorf("could not add neighbor entry:%+v error:%v", nlnh, err)
+	}
+
+	n.mu.Lock()
+	n.neighbors = append(n.neighbors, nh)
+	n.mu.Unlock()
+	log.G(context.TODO()).Debugf("Neighbor entry added for IP:%v, mac:%v on ifc:%s", dstIP, dstMac, nh.linkName)
+
+	return nil
 }