WIP on tests.

felix/netlinkshim/mocknetlink/netlink.go

@@ -912,6 +912,10 @@ func addNeighs(neighMap map[NeighKey]*netlink.Neigh, neighs []netlink.Neigh) {
 }
 
 func (d *MockNetlinkDataplane) ExpectNeighs(family int, neighs ...netlink.Neigh) {
+	if len(neighs) == 0 {
+		ExpectWithOffset(1, d.NeighsByFamily[family]).To(HaveLen(0))
+		return
+	}
 	nm := map[NeighKey]*netlink.Neigh{}
 	addNeighs(nm, neighs)
 	ExpectWithOffset(1, d.NeighsByFamily[family]).To(Equal(nm))

felix/routetable/conntrack_owner_tracker_test.go

@@ -78,6 +78,32 @@ func TestConntrackCleanupManager_MovedRoute(t *testing.T) {
 
 	expectInSyncAtEnd(h.ccm)
 }
+func TestConntrackCleanupManager_MovedRouteRemoteToRemote(t *testing.T) {
+	h := setupConntrackTrackerTest(t)
+
+	// Call methods in the same order as the RouteTable would.
+
+	// Initially, the route is owned by interface 10.
+	t.Log("Setting initial owner.")
+	h.ccm.UpdateCIDROwner(cidr1, 10, RouteClassVXLANTunnel)
+	// Commit that change.
+	h.ccm.StartConntrackCleanupAndReset()
+
+	// Then, the exact same route moves to interface 11.
+	t.Log("Setting new owner.")
+	h.ccm.UpdateCIDROwner(cidr1, 11, RouteClassVXLANSameSubnet)
+
+	// RouteTable won't have any deletions to do, so move on to the first
+	// pass over the updated routes.
+	Expect(h.ccm.CIDRNeedsEarlyCleanup(cidr1, 10)).To(BeFalse(),
+		"Remote to remote moves houldn't trigger cleanup.")
+
+	h.ccm.StartConntrackCleanupAndReset()
+	Consistently(h.conntrack.NumPendingRemovals, "10ms").Should(Equal(0))
+	expectWaitForPendingDeletionToReturnImmediately(h.ccm, cidr1)
+
+	expectInSyncAtEnd(h.ccm)
+}
 
 func TestConntrackCleanupManager_ChangeOfPrioritySameInterface(t *testing.T) {
 	h := setupConntrackTrackerTest(t)
@@ -139,6 +165,20 @@ func TestConntrackCleanupManager_DeletedRoute(t *testing.T) {
 	expectWaitForPendingDeletionToDelay(h.ccm, h.conntrack, cidr1)
 }
 
+func TestConntrackCleanupManager_DeletedStaleRoute(t *testing.T) {
+	h := setupConntrackTrackerTest(t)
+
+	// Tell the CCM about the route.
+	h.ccm.UpdateCIDROwner(cidr1, 10, RouteClassLocalWorkload)
+	h.ccm.StartConntrackCleanupAndReset() // Commit to the delta tracker.
+	Consistently(h.conntrack.NumPendingRemovals, "10ms").Should(Equal(0))
+
+	// Then, the route is deleted from a different interface.
+	h.ccm.OnDataplaneRouteDeleted(cidr1, 11)
+	h.ccm.StartConntrackCleanupAndReset()
+	Consistently(h.conntrack.NumPendingRemovals, "10ms").Should(Equal(0))
+}
+
 type conntrackTrackerHarness struct {
 	ccm       *ConntrackCleanupManager
 	conntrack *mockConntrack

felix/routetable/route_table.go

@@ -432,13 +432,8 @@ func (r *RouteTable) SetRoutes(routeClass RouteClass, ifaceName string, targets
 	if len(newTargets) == 0 {
 		r.logCxt.Debug("No routes for this interface, removing from map.")
 		delete(r.ifaceToRoutes[routeClass], ifaceName)
-		delete(r.pendingARPs, ifaceName)
 	} else {
 		r.ifaceToRoutes[routeClass][ifaceName] = newTargets
-		if r.makeARPEntries {
-			r.logCxt.Debug("Cleaning ARP map for interface.")
-			delete(r.pendingARPs, ifaceName)
-		}
 	}
 
 	// Clean up the old CIDRs.
@@ -448,6 +443,8 @@ func (r *RouteTable) SetRoutes(routeClass RouteClass, ifaceName string, targets
 		r.recalculateDesiredKernelRoute(cidr)
 	}
 
+	// Clean out the pending ARP list, then recalculate it below.
+	delete(r.pendingARPs, ifaceName)
 	for cidr, target := range newTargets {
 		r.recalculateDesiredKernelRoute(cidr)
 		r.updatePendingARP(ifaceName, cidr.Addr(), target.DestMAC)
@@ -746,6 +743,8 @@ func (r *RouteTable) maybeCleanUpGracePeriods() {
 			continue // Iface still exists, don't want to reset its grace period.
 		}
 		delete(r.ifaceIndexToGraceInfo, k)
+
+		r.livenessCallback()
 	}
 }
 
@@ -804,13 +803,15 @@ func (r *RouteTable) doFullResync(nl netlinkshim.Interface) error {
 		}
 		r.kernelRoutes.Dataplane().Set(kernKey, kernRoute)
 		seenKeys.Add(kernKey)
+		r.livenessCallback()
 	}
 
 	r.kernelRoutes.Dataplane().Iter(func(kernKey kernelRouteKey, kernRoute kernelRoute) {
 		if !seenKeys.Contains(kernKey) {
 			r.kernelRoutes.Dataplane().Delete(kernKey)
 			r.conntrackTracker.OnDataplaneRouteDeleted(kernKey.CIDR, kernRoute.Ifindex)
 		}
+		r.livenessCallback()
 	})
 
 	// We're now in sync.
@@ -826,6 +827,7 @@ func (r *RouteTable) resyncIndividualInterfaces(nl netlinkshim.Interface) error
 	}
 	r.opReporter.RecordOperation(fmt.Sprint("partial-resync-routes-v", r.ipVersion))
 	r.ifacesToRescan.Iter(func(ifaceName string) error {
+		r.livenessCallback()
 		err := r.resyncIface(nl, ifaceName)
 		if err != nil {
 			r.nl.MarkHandleForReopen()
@@ -963,6 +965,7 @@ func (r *RouteTable) refreshAllIfaceStates(nl netlinkshim.Interface) error {
 			}).Info("Spotted interface had changed name during resync.")
 			r.OnIfaceStateChanged(oldName, link.Attrs().Index, ifacemonitor.StateNotPresent)
 		}
+		r.livenessCallback()
 	}
 
 	// Second pass, update the state of any changed interfaces.
@@ -992,6 +995,7 @@ func (r *RouteTable) refreshAllIfaceStates(nl netlinkshim.Interface) error {
 			}
 			r.OnIfaceStateChanged(link.Attrs().Name, link.Attrs().Index, newState)
 		}
+		r.livenessCallback()
 	}
 
 	// Third pass, remove any interfaces that have disappeared.
@@ -1003,6 +1007,7 @@ func (r *RouteTable) refreshAllIfaceStates(nl netlinkshim.Interface) error {
 			r.logCxt.WithField("ifaceName", name).Info("Spotted interface not present during full resync.  Cleaning up.")
 		}
 		r.OnIfaceStateChanged(name, 0, ifacemonitor.StateNotPresent)
+		r.livenessCallback()
 	}
 	return nil
 }
@@ -1122,6 +1127,7 @@ func (r *RouteTable) applyUpdates(attempt int) error {
 	// First clean up any old routes.
 	deletionErrs := map[kernelRouteKey]error{}
 	r.kernelRoutes.PendingDeletions().Iter(func(kernKey kernelRouteKey) deltatracker.IterAction {
+		r.livenessCallback()
 		kernRoute, _ := r.kernelRoutes.PendingDeletions().Get(kernKey)
 		if r.ifaceInGracePeriod(kernRoute.Ifindex) {
 			// Don't remove unexpected routes from interfaces created recently.
@@ -1147,6 +1153,7 @@ func (r *RouteTable) applyUpdates(attempt int) error {
 	// Now do a first pass of the routes that we want to create/update and
 	// trigger any necessary conntrack cleanups for moved routes.
 	r.kernelRoutes.PendingUpdates().Iter(func(kernKey kernelRouteKey, kernRoute kernelRoute) deltatracker.IterAction {
+		r.livenessCallback()
 		cidr := kernKey.CIDR
 		dataplaneRoute, dataplaneExists := r.kernelRoutes.Dataplane().Get(kernKey)
 		if dataplaneExists && r.conntrackTracker.CIDRNeedsEarlyCleanup(cidr, dataplaneRoute.Ifindex) {
@@ -1167,6 +1174,7 @@ func (r *RouteTable) applyUpdates(attempt int) error {
 
 	updateErrs := map[kernelRouteKey]error{}
 	r.kernelRoutes.PendingUpdates().Iter(func(kernKey kernelRouteKey, kRoute kernelRoute) deltatracker.IterAction {
+		r.livenessCallback()
 		dst := kernKey.CIDR.ToIPNet()
 		flags := 0
 		if kRoute.OnLink {
@@ -1243,6 +1251,7 @@ func (r *RouteTable) applyUpdates(attempt int) error {
 			continue
 		}
 		for addr, mac := range addrToMAC {
+			r.livenessCallback()
 			err := r.addStaticARPEntry(nl, addr, mac, ifaceIdx)
 			if err != nil {
 				err = r.filterErrorByIfaceState(
@@ -1362,21 +1371,21 @@ func (r *RouteTable) filterErrorByIfaceState(
 	logCxt := r.logCxt.WithFields(log.Fields{"ifaceName": ifaceName, "error": currentErr})
 	if ifaceName == InterfaceNone {
 		// Short circuit the no-OIF interface name.
-		logCxt.Info("No interface on route.")
+		logCxt.Debug("No interface on route.")
 		return defaultErr
 	}
 
 	if isNotFoundError(currentErr) {
 		// Current error already tells us that the link was not present.  If we re-check
 		// the status in this case, we open a race where the interface gets created and
 		// we log an error when we're about to re-trigger programming anyway.
-		logCxt.Info("Interface doesn't exist, perhaps workload is being torn down?")
+		logCxt.Debug("Interface doesn't exist, perhaps workload is being torn down?")
 		return IfaceNotPresent
 	}
 
 	if errors.Is(currentErr, syscall.ENETDOWN) {
 		// Another clear error: interface is down.
-		logCxt.Info("Interface down, perhaps workload is being torn down?")
+		logCxt.Debug("Interface down, perhaps workload is being torn down?")
 		return IfaceDown
 	}
 

felix/routetable/route_table_test.go

@@ -85,6 +85,26 @@ var _ = Describe("RouteTable v6", func() {
 		Expect(rt).ToNot(BeNil())
 	})
 
+	It("should use interface index 1 for no-iface routes", func() {
+		rt.RouteUpdate(RouteClassWireguard, InterfaceNone, Target{
+			CIDR: ip.MustParseCIDROrIP("f00f::/128"),
+			Type: TargetTypeThrow,
+		})
+		err := rt.Apply()
+		Expect(err).ToNot(HaveOccurred())
+		Expect(dataplane.RouteKeyToRoute["254-f00f::/128"]).To(Equal(
+			netlink.Route{
+				Family:    netlink.FAMILY_V6,
+				LinkIndex: 1,
+				Dst:       mustParseCIDR("f00f::/128"),
+				Type:      syscall.RTN_THROW,
+				Protocol:  syscall.RTPROT_BOOT,
+				Scope:     netlink.SCOPE_UNIVERSE,
+				Table:     unix.RT_TABLE_MAIN,
+			},
+		))
+	})
+
 	It("should not remove the IPv6 link local route", func() {
 		// Route that should be left alone
 		noopLink := dataplane.AddIface(4, "cali4", true, true)
@@ -338,6 +358,33 @@ var _ = Describe("RouteTable", func() {
 					HardwareAddr: mac1,
 				})
 			})
+			It("Should skip adding an ARP entry if route is deleted via SetRoutes before sync", func() {
+				// Route that needs to be added
+				link := dataplane.AddIface(6, "cali6", true, true)
+				rt.SetRoutes(RouteClassLocalWorkload, link.LinkAttrs.Name, []Target{
+					{CIDR: ip.MustParseCIDROrIP("10.0.0.6"), DestMAC: mac1},
+				})
+				rt.SetRoutes(RouteClassLocalWorkload, link.LinkAttrs.Name, nil)
+				err := rt.Apply()
+
+				Expect(err).ToNot(HaveOccurred())
+				Expect(dataplane.RouteKeyToRoute).NotTo(HaveKey("254-10.0.0.6/32"))
+				dataplane.ExpectNeighs(unix.AF_INET)
+			})
+			It("Should skip adding an ARP entry if route is deleted via RouteRemove before sync", func() {
+				// Route that needs to be added
+				link := dataplane.AddIface(6, "cali6", true, true)
+				cidr := ip.MustParseCIDROrIP("10.0.0.6")
+				rt.SetRoutes(RouteClassLocalWorkload, link.LinkAttrs.Name, []Target{
+					{CIDR: cidr, DestMAC: mac1},
+				})
+				rt.RouteRemove(RouteClassLocalWorkload, link.LinkAttrs.Name, cidr)
+				err := rt.Apply()
+
+				Expect(err).ToNot(HaveOccurred())
+				Expect(dataplane.RouteKeyToRoute).NotTo(HaveKey("254-10.0.0.6/32"))
+				dataplane.ExpectNeighs(unix.AF_INET)
+			})
 			It("Should not remove routes with a source address", func() {
 				// Route that should be left alone
 				noopLink := dataplane.AddIface(6, "cali4", true, true)

felix/routetable/routetable_suite_test.go

@@ -29,7 +29,7 @@ func init() {
 	testutils.HookLogrusForGinkgo()
 }
 
-func TestRules(t *testing.T) {
+func TestRouteTable(t *testing.T) {
 	RegisterFailHandler(Fail)
 	junitReporter := reporters.NewJUnitReporter("../report/routetable_suite.xml")
 	RunSpecsWithDefaultAndCustomReporters(t, "RouteTable Suite", []Reporter{junitReporter})