diff --git a/libnetwork/drivers/overlay/peerdb.go b/libnetwork/drivers/overlay/peerdb.go index 345f1c1aba..fd0a37d7f4 100644 --- a/libnetwork/drivers/overlay/peerdb.go +++ b/libnetwork/drivers/overlay/peerdb.go @@ -321,7 +321,7 @@ func (d *driver) peerAddOp(nid, eid string, peerIP net.IP, peerIPMask net.IPMask } // Add neighbor entry for the peer IP - if err := sbox.AddNeighbor(peerIP, peerMac, false, osl.WithLinkName(s.vxlanName)); err != nil { + if err := sbox.AddNeighbor(peerIP, peerMac, osl.WithLinkName(s.vxlanName)); err != nil { if _, ok := err.(osl.NeighborSearchError); ok && dbEntries > 1 { // We are in the transient case so only the first configuration is programmed into the kernel // Upon deletion if the active configuration is deleted the next one from the database will be restored @@ -332,7 +332,7 @@ func (d *driver) peerAddOp(nid, eid string, peerIP net.IP, peerIPMask net.IPMask } // Add fdb entry to the bridge for the peer mac - if err := sbox.AddNeighbor(vtep, peerMac, false, osl.WithLinkName(s.vxlanName), osl.WithFamily(syscall.AF_BRIDGE)); err != nil { + if err := sbox.AddNeighbor(vtep, peerMac, osl.WithLinkName(s.vxlanName), osl.WithFamily(syscall.AF_BRIDGE)); err != nil { return fmt.Errorf("could not add fdb entry for nid:%s eid:%s into the sandbox:%v", nid, eid, err) } @@ -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 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) } } diff --git a/libnetwork/osl/interface_linux.go b/libnetwork/osl/interface_linux.go index 7ec9cdcf0a..765f7f96be 100644 --- a/libnetwork/osl/interface_linux.go +++ b/libnetwork/osl/interface_linux.go @@ -804,24 +804,20 @@ func (n *Namespace) prepAdvertiseAddrs(ctx context.Context, i *Interface, ifInde // original name and moving it out of the sandbox. func (n *Namespace) RemoveInterface(i *Interface) error { close(i.stopCh) - n.mu.Lock() - isDefault := n.isDefault - nlh := n.nlHandle - n.mu.Unlock() // Find the network interface identified by the DstName attribute. - iface, err := nlh.LinkByName(i.DstName()) + iface, err := n.nlHandle.LinkByName(i.DstName()) if err != nil { return err } // Down the interface before configuring - if err := nlh.LinkSetDown(iface); err != nil { + if err := n.nlHandle.LinkSetDown(iface); err != nil { return err } // TODO(aker): Why are we doing this? This would fail if the initial interface set up failed before the "dest interface" was moved into its own namespace; see https://github.com/moby/moby/pull/46315/commits/108595c2fe852a5264b78e96f9e63cda284990a6#r1331253578 - err = nlh.LinkSetName(iface, i.SrcName()) + err = n.nlHandle.LinkSetName(iface, i.SrcName()) if err != nil { log.G(context.TODO()).Debugf("LinkSetName failed for interface %s: %v", i.SrcName(), err) return err @@ -829,13 +825,13 @@ func (n *Namespace) RemoveInterface(i *Interface) error { // if it is a bridge just delete it. if i.Bridge() { - if err := nlh.LinkDel(iface); err != nil { + if err := n.nlHandle.LinkDel(iface); err != nil { return fmt.Errorf("failed deleting bridge %q: %v", i.SrcName(), err) } - } else if !isDefault { + } else if !n.isDefault { // Move the network interface to caller namespace. // TODO(aker): What's this really doing? There are no calls to LinkDel in this package: is this code really used? (Interface.Remove() has 3 callers); see https://github.com/moby/moby/pull/46315/commits/108595c2fe852a5264b78e96f9e63cda284990a6#r1331265335 - if err := nlh.LinkSetNsFd(iface, ns.ParseHandlerInt()); err != nil { + if err := n.nlHandle.LinkSetNsFd(iface, ns.ParseHandlerInt()); err != nil { log.G(context.TODO()).Debugf("LinkSetNsFd failed for interface %s: %v", i.SrcName(), err) return err } diff --git a/libnetwork/osl/namespace_linux.go b/libnetwork/osl/namespace_linux.go index d73f5ddff5..cbc4f8dcd8 100644 --- a/libnetwork/osl/namespace_linux.go +++ b/libnetwork/osl/namespace_linux.go @@ -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 diff --git a/libnetwork/osl/neigh_linux.go b/libnetwork/osl/neigh_linux.go index c5dab0b8a3..11775a1b33 100644 --- 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,145 +14,115 @@ 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} - } - - n.mu.Lock() - nlh := n.nlHandle - n.mu.Unlock() - - var linkIndex int - 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) - } - 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 := nlh.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 := nlh.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, force bool, options ...NeighOption) error { - var ( - iface netlink.Link - err error - neighborAlreadyPresent bool - ) +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 + } - // If the namespace already has the neighbor entry but the AddNeighbor is called - // because of a miss notification (force flag) program the kernel anyway. - nh := n.findNeighbor(dstIP, dstMac) - if nh != nil { - neighborAlreadyPresent = true - log.G(context.TODO()).Warnf("Neighbor entry already present for IP %v, mac %v neighbor:%+v forceUpdate:%t", dstIP, dstMac, nh, force) - if !force { - 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...) - 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) - } - } - - n.mu.Lock() - nlh := n.nlHandle - n.mu.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) - } - } - nlnh := &netlink.Neigh{ IP: dstIP, HardwareAddr: dstMac, @@ -164,22 +134,17 @@ func (n *Namespace) AddNeighbor(dstIP net.IP, dstMac net.HardwareAddr, force boo nlnh.Flags = netlink.NTF_SELF } - if nh.linkDst != "" { + 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) + } + iface, err := n.nlHandle.LinkByName(linkDst) + if err != nil { + return nil, nh.linkName, fmt.Errorf("could not find interface with destination name %s: %w", linkDst, err) + } nlnh.LinkIndex = iface.Attrs().Index } - if err := nlh.NeighSet(nlnh); err != nil { - return fmt.Errorf("could not add neighbor entry:%+v error:%v", nlnh, err) - } - - if neighborAlreadyPresent { - return nil - } - - 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 }