node: Handle arpinging when remote node is in different L2

pkg/datapath/linux/node.go

@@ -21,6 +21,7 @@ import (
 	"os"
 
 	"github.com/cilium/cilium/pkg/cidr"
+	"github.com/cilium/cilium/pkg/counter"
 	"github.com/cilium/cilium/pkg/datapath"
 	"github.com/cilium/cilium/pkg/datapath/linux/ipsec"
 	"github.com/cilium/cilium/pkg/datapath/linux/linux_defaults"
@@ -51,17 +52,21 @@ type linuxNodeHandler struct {
 	datapathConfig       DatapathConfiguration
 	nodes                map[nodeTypes.Identity]*nodeTypes.Node
 	enableNeighDiscovery bool
-	neighByNode          map[nodeTypes.Identity]*netlink.Neigh
+	neighNextHopByNode   map[nodeTypes.Identity]string // val = string(net.IP)
+	neighNextHopRefCount counter.StringCounter
+	neighByNextHop       map[string]*netlink.Neigh // key = string(net.IP)
 }
 
 // NewNodeHandler returns a new node handler to handle node events and
 // implement the implications in the Linux datapath
 func NewNodeHandler(datapathConfig DatapathConfiguration, nodeAddressing datapath.NodeAddressing) datapath.NodeHandler {
 	return &linuxNodeHandler{
-		nodeAddressing: nodeAddressing,
-		datapathConfig: datapathConfig,
-		nodes:          map[nodeTypes.Identity]*nodeTypes.Node{},
-		neighByNode:    map[nodeTypes.Identity]*netlink.Neigh{},
+		nodeAddressing:       nodeAddressing,
+		datapathConfig:       datapathConfig,
+		nodes:                map[nodeTypes.Identity]*nodeTypes.Node{},
+		neighNextHopByNode:   map[nodeTypes.Identity]string{},
+		neighNextHopRefCount: counter.StringCounter{},
+		neighByNextHop:       map[string]*netlink.Neigh{},
 	}
 }
 
@@ -550,73 +555,107 @@ func (n *linuxNodeHandler) insertNeighbor(newNode *nodeTypes.Node, ifaceName str
 		return
 	}
 
-	ciliumIPv4 := newNode.GetNodeIP(false)
+	newNodeIP := newNode.GetNodeIP(false).To4()
+	nextHopIPv4 := make(net.IP, len(newNodeIP))
+	copy(nextHopIPv4, newNodeIP)
+
 	scopedLog := log.WithFields(logrus.Fields{
 		logfields.Interface: ifaceName,
-		logfields.IPAddr:    ciliumIPv4,
+		logfields.IPAddr:    nextHopIPv4,
 	})
 
-	iface, err := net.InterfaceByName(ifaceName)
+	// Figure out whether newNode is directly reachable (i.e. in the same L2)
+	routes, err := netlink.RouteGet(nextHopIPv4)
 	if err != nil {
-		scopedLog.WithError(err).Error("Failed to retrieve iface by name")
+		scopedLog.WithError(err).Error("Failed to retrieve route for remote node IP")
 		return
 	}
-
-	_, err = arping.FindIPInNetworkFromIface(ciliumIPv4, *iface)
-	if err != nil {
-		scopedLog.WithError(err).Error("IP is not L2 reachable")
-		return
+	for _, route := range routes {
+		if route.Gw != nil {
+			// newNode is in a different L2 subnet, so it must be reachable through
+			// a gateway. Send arping to the gw IP addr instead of newNode IP addr.
+			// NOTE: we currently don't handle multipath, so only one gw can be used.
+			copy(nextHopIPv4, route.Gw)
+			break
+		}
 	}
 
-	linkAttr, err := netlink.LinkByName(ifaceName)
-	if err != nil {
-		scopedLog.WithError(err).Error("Failed to retrieve iface by name (netlink)")
-		return
-	}
-	link := linkAttr.Attrs().Index
+	nextHopStr := nextHopIPv4.String()
+	n.neighNextHopByNode[newNode.Identity()] = nextHopStr
+	_, found := n.neighByNextHop[nextHopStr]
 
-	hwAddr, _, err := arping.PingOverIface(ciliumIPv4, *iface)
-	if err != nil {
-		scopedLog.WithError(err).Error("arping failed")
-		return
-	}
-	scopedLog = scopedLog.WithField(logfields.HardwareAddr, hwAddr)
+	// nextHop hasn't been arpinged before OR the arping failed
+	if n.neighNextHopRefCount.Add(nextHopStr) || !found {
+		iface, err := net.InterfaceByName(ifaceName)
+		if err != nil {
+			scopedLog.WithError(err).Error("Failed to retrieve iface by name")
+			return
+		}
 
-	neigh := netlink.Neigh{
-		LinkIndex:    link,
-		IP:           ciliumIPv4,
-		HardwareAddr: hwAddr,
-		State:        netlink.NUD_PERMANENT,
-	}
-	if err := netlink.NeighSet(&neigh); err != nil {
-		scopedLog.WithError(err).Error("Failed to insert neighbor")
-		return
-	}
+		_, err = arping.FindIPInNetworkFromIface(nextHopIPv4, *iface)
+		if err != nil {
+			scopedLog.WithError(err).Error("IP is not L2 reachable")
+			return
+		}
 
-	n.neighByNode[newNode.Identity()] = &neigh
-	if option.Config.NodePortHairpin {
-		neighborsmap.NeighRetire(ciliumIPv4)
+		linkAttr, err := netlink.LinkByName(ifaceName)
+		if err != nil {
+			scopedLog.WithError(err).Error("Failed to retrieve iface by name (netlink)")
+			return
+		}
+		link := linkAttr.Attrs().Index
+
+		hwAddr, _, err := arping.PingOverIface(nextHopIPv4, *iface)
+		if err != nil {
+			scopedLog.WithError(err).Error("arping failed")
+			return
+		}
+		scopedLog = scopedLog.WithField(logfields.HardwareAddr, hwAddr)
+
+		neigh := netlink.Neigh{
+			LinkIndex:    link,
+			IP:           nextHopIPv4,
+			HardwareAddr: hwAddr,
+			State:        netlink.NUD_PERMANENT,
+		}
+		if err := netlink.NeighSet(&neigh); err != nil {
+			scopedLog.WithError(err).Error("Failed to insert neighbor")
+			return
+		}
+
+		n.neighByNextHop[nextHopStr] = &neigh
+		if option.Config.NodePortHairpin {
+			neighborsmap.NeighRetire(nextHopIPv4)
+		}
 	}
 }
 
 // Must be called with linuxNodeHandler.mutex held.
 func (n *linuxNodeHandler) deleteNeighbor(oldNode *nodeTypes.Node) {
-	neigh, ok := n.neighByNode[oldNode.Identity()]
-	if !ok {
+	nextHopStr, found := n.neighNextHopByNode[oldNode.Identity()]
+	if !found {
 		return
 	}
+	defer func() { delete(n.neighNextHopByNode, oldNode.Identity()) }()
 
-	if err := netlink.NeighDel(neigh); err != nil {
-		log.WithFields(logrus.Fields{
-			logfields.IPAddr:       neigh.IP,
-			logfields.HardwareAddr: neigh.HardwareAddr,
-			logfields.LinkIndex:    neigh.LinkIndex,
-		}).WithError(err).Warn("Failed to remove neighbor entry")
-		return
-	}
+	if n.neighNextHopRefCount.Delete(nextHopStr) {
+		neigh, found := n.neighByNextHop[nextHopStr]
+		delete(n.neighByNextHop, nextHopStr)
+
+		if found {
+			if err := netlink.NeighDel(neigh); err != nil {
+				log.WithFields(logrus.Fields{
+					logfields.IPAddr:       neigh.IP,
+					logfields.HardwareAddr: neigh.HardwareAddr,
+					logfields.LinkIndex:    neigh.LinkIndex,
+				}).WithError(err).Warn("Failed to remove neighbor entry")
+				return
+			}
 
-	if option.Config.NodePortHairpin {
-		neighborsmap.NeighRetire(neigh.IP)
+			if option.Config.NodePortHairpin {
+				neighborsmap.NeighRetire(neigh.IP)
+			}
+		}
 	}
 }