libnetwork/osl: Namespace.DeleteNeighbor: assorted cleanups, and ignore "non-exist" warnings
- relates to / addresses https://github.com/moby/moby/issues/46241

### libnetwork/osl: Namespace.DeleteNeighbor: don't warn on non-existing neighbor

The code ignores these errors, but will unconditionally print a warning;

> If the kernel deletion fails for the neighbor entry still remote it
> from the namespace cache. Otherwise if the neighbor moves back to the
> same host again, kernel update can fail.

Let's reduce noise if the neighbor wasn't found, to prevent logs like:

    Aug 16 13:26:35 master1.local dockerd[4019880]: time="2023-08-16T13:26:35.186662370+02:00" level=warning msg="error while deleting neighbor entry" error="no such file or directory"
    Aug 16 13:26:35 master1.local dockerd[4019880]: time="2023-08-16T13:26:35.366585939+02:00" level=warning msg="error while deleting neighbor entry" error="no such file or directory"
    Aug 16 13:26:42 master1.local dockerd[4019880]: time="2023-08-16T13:26:42.366658513+02:00" level=warning msg="error while deleting neighbor entry" error="no such file or directory"

### libnetwork/osl: Namespace.DeleteNeighbor: remove osDelete argument

This argument was originally added in libnetwork:
https://github.com/moby/libnetwork/commit/03f440667f79f33ef3cb1faeb13c0bca1a0f8737

At the time, this argument was conditional, but currently it's always set
to "true", so let's remove it.

### libnetwork/osl: Namespace.DeleteNeighbor: remove intermediate vars

- store linkIndex in a local variable so that it can be reused
- remove / rename some intermediate vars that shadowed existing declaration





**- A picture of a cute animal (not mandatory but encouraged)**


libnetwork/osl/neigh_linux.go
package osl

import (
	"bytes"
	"context"
	"fmt"
	"net"

	"github.com/containerd/containerd/log"
	"github.com/vishvananda/netlink"
)

// NeighborSearchError indicates that the neighbor is already present
type NeighborSearchError struct {
	ip      net.IP
	mac     net.HardwareAddr
	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)
}

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.Lock()
	defer n.Unlock()

	for _, nh := range n.neighbors {
		if nh.dstIP.Equal(dstIP) && bytes.Equal(nh.dstMac, dstMac) {
			return nh
		}
	}

	return nil
}

// DeleteNeighbor deletes neighbor entry from the sandbox.
func (n *Namespace) DeleteNeighbor(dstIP net.IP, dstMac net.HardwareAddr, osDelete bool) error {
	var (
		iface netlink.Link
		err   error
	)

	nh := n.findNeighbor(dstIP, dstMac)
	if nh == nil {
		return NeighborSearchError{dstIP, dstMac, false}
	}

	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)
			}
		}

		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 nh.linkDst != "" {
			nlnh.LinkIndex = iface.Attrs().Index
		}

		// If the kernel deletion fails for the neighbor entry still remote it
		// from the namespace cache. Otherwise if the neighbor moves back to the
		// same host again, kernel update can fail.
		if err := nlh.NeighDel(nlnh); err != nil {
			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 := &netlink.Neigh{
				IP:     dstIP,
				Family: nh.family,
			}

			nlnh.HardwareAddr = dstMac
			nlnh.Flags = netlink.NTF_MASTER
			if nh.linkDst != "" {
				nlnh.LinkIndex = iface.Attrs().Index
			}
			if err := nlh.NeighDel(nlnh); err != nil {
				log.G(context.TODO()).WithError(err).Warn("error while deleting neighbor entry")
			}
		}
	}

	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:]...)
			break
		}
	}
	n.Unlock()
	log.G(context.TODO()).Debugf("Neighbor entry deleted for IP %v, mac %v osDelete:%t", dstIP, dstMac, osDelete)

	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
	)

	// 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}
		}
	}

	nh = &neigh{
		dstIP:  dstIP,
		dstMac: dstMac,
	}

	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.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)
		}
	}

	nlnh := &netlink.Neigh{
		IP:           dstIP,
		HardwareAddr: dstMac,
		State:        netlink.NUD_PERMANENT,
		Family:       nh.family,
	}

	if nlnh.Family > 0 {
		nlnh.Flags = netlink.NTF_SELF
	}

	if nh.linkDst != "" {
		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.Lock()
	n.neighbors = append(n.neighbors, nh)
	n.Unlock()
	log.G(context.TODO()).Debugf("Neighbor entry added for IP:%v, mac:%v on ifc:%s", dstIP, dstMac, nh.linkName)

	return nil
}

package osl

import (
	"bytes"
	"context"
	"errors"
	"fmt"
	"net"
	"os"

	"github.com/containerd/containerd/log"
	"github.com/vishvananda/netlink"
)

// NeighborSearchError indicates that the neighbor is already present
type NeighborSearchError struct {
	ip      net.IP
	mac     net.HardwareAddr
	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)
}

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.Lock()
	defer n.Unlock()

	for _, nh := range n.neighbors {
		if nh.dstIP.Equal(dstIP) && bytes.Equal(nh.dstMac, dstMac) {
			return nh
		}
	}

	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.Lock()
	nlh := n.nlHandle
	n.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)
	}

	// 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")
		}
	}

	n.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.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, force bool, options ...NeighOption) error {
	var (
		iface                  netlink.Link
		err                    error
		neighborAlreadyPresent bool
	)

	// 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}
		}
	}

	nh = &neigh{
		dstIP:  dstIP,
		dstMac: dstMac,
	}

	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.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)
		}
	}

	nlnh := &netlink.Neigh{
		IP:           dstIP,
		HardwareAddr: dstMac,
		State:        netlink.NUD_PERMANENT,
		Family:       nh.family,
	}

	if nlnh.Family > 0 {
		nlnh.Flags = netlink.NTF_SELF
	}

	if nh.linkDst != "" {
		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.Lock()
	n.neighbors = append(n.neighbors, nh)
	n.Unlock()
	log.G(context.TODO()).Debugf("Neighbor entry added for IP:%v, mac:%v on ifc:%s", dstIP, dstMac, nh.linkName)

	return nil
}

Mh I would tend to say: if it doesn't exist anymore, the end state has been reached anyway. But... this code is called by the `peerdb` package, and:

1. I'm wondering if that could be a symptom of some broken internal state;
2. [this comment](https://github.com/moby/moby/blob/2243046bf8e9b677309744863af116932f9aeabe/libnetwork/drivers/overlay/peerdb.go#L232-L242) references some "message", channel and goroutin but I don't see anything like that hanging around in that method, in its caller or in `peerInitOp`. I guess this comment is outdated, which makes me wonder what prevent two "peer delete" / "peer add" to be process concurrently :thinking: I admit the last time I looked into `peerdb` was months ago and I don't recall how it works exactly :woozy_face: 

> // Note also that this method sends a single message on the channel and the go routine on the
> // other side, will atomically loop on the whole table of peers and will program their state
> // in one single atomic operation. This is fundamental to guarantee consistency, and avoid that
> // new peerAdd or peerDelete gets reordered during the sandbox init.