libnetwork/osl: stop tracking neighbor entries

The Namespace keeps some state for each inserted neighbor-table entry
which is used to delete the entry (and any related entries) given only
the IP and MAC address of the entry to delete. This state is not
strictly required as the retained data is a pure function of the
parameters passed to AddNeighbor(), and the kernel can inform us whether
an attempt to add a neighbor entry would conflict with an existing
entry. Get rid of the neighbor state in Namespace. It's just one more
piece of state that can cause lots of grief if it falls out of sync with
ground truth. Require callers to call DeleteNeighbor() with the same
aguments as they had passed to AddNeighbor(). Push the responsibility
for detecting attempts to insert conflicting entries into the neighbor
table onto the kernel by using (*netlink.Handle).NeighAdd() instead of
NeighSet().

Modernize the error messages and logging in DeleteNeighbor() and
AddNeighbor().

Signed-off-by: Cory Snider <csnider@mirantis.com>
This commit is contained in:
Cory Snider
2025-05-07 18:11:53 -04:00
parent 9866738736
commit 0d6e7cd983
3 changed files with 101 additions and 101 deletions

View File

@@ -375,8 +375,16 @@ 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); err != nil {
if err := sbox.DeleteNeighbor(vtep, peerMac, osl.WithLinkName(s.vxlanName)); 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)
@@ -386,7 +394,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); err != nil {
if err := sbox.DeleteNeighbor(peerIP, peerMac, osl.WithLinkName(s.vxlanName), osl.WithFamily(syscall.AF_BRIDGE)); err != nil {
return fmt.Errorf("could not delete neighbor entry for nid:%s eid:%s into the sandbox:%v", nid, eid, err)
}
}

View File

@@ -232,7 +232,6 @@ 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

View File

@@ -1,12 +1,12 @@
package osl
import (
"bytes"
"context"
"errors"
"fmt"
"net"
"os"
"strings"
"github.com/containerd/log"
"github.com/vishvananda/netlink"
@@ -14,110 +14,113 @@ import (
// NeighborSearchError indicates that the neighbor is already present
type NeighborSearchError struct {
ip net.IP
mac net.HardwareAddr
present bool
ip net.IP
mac net.HardwareAddr
linkName string
present bool
}
func (n NeighborSearchError) Error() string {
return fmt.Sprintf("Search neighbor failed for IP %v, mac %v, present in db:%t", n.ip, n.mac, n.present)
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()
}
type neigh struct {
dstIP net.IP
dstMac net.HardwareAddr
linkName string
linkDst string
family int
}
// 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
}
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)
}
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)
return fmt.Errorf("could not delete neighbor %+v: %w", nlnh, err)
}
// Delete the dynamic entry in the bridge
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")
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")
}
}
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)
log.G(context.TODO()).WithFields(log.Fields{
"ip": dstIP,
"mac": dstMac,
"ifc": linkName,
}).Debug("Neighbor entry deleted")
return nil
}
// AddNeighbor adds a neighbor entry into the sandbox.
func (n *Namespace) AddNeighbor(dstIP net.IP, dstMac net.HardwareAddr, options ...NeighOption) error {
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}
nlnh, linkName, err := n.nlNeigh(dstIP, dstMac, options...)
if err != nil {
return err
}
nh = &neigh{
dstIP: dstIP,
dstMac: dstMac,
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)
}
}
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{
@@ -132,26 +135,16 @@ func (n *Namespace) AddNeighbor(dstIP net.IP, dstMac net.HardwareAddr, options .
}
if 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)
linkDst := n.findDst(nh.linkName, false)
if linkDst == "" {
return nil, nh.linkName, fmt.Errorf("could not find the interface with name %s", nh.linkName)
}
iface, err := n.nlHandle.LinkByName(nh.linkDst)
iface, err := n.nlHandle.LinkByName(linkDst)
if err != nil {
return fmt.Errorf("could not find interface with destination name %s: %v", nh.linkDst, err)
return nil, nh.linkName, fmt.Errorf("could not find interface with destination name %s: %w", linkDst, err)
}
nlnh.LinkIndex = iface.Attrs().Index
}
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
return nlnh, nh.linkName, nil
}