From d855b187d1a493a05aa97b61159e1572a34f677c Mon Sep 17 00:00:00 2001 From: Nell Jerram Date: Fri, 23 Jan 2026 10:24:52 +0000 Subject: [PATCH 1/8] Enhance Felix route table for elevated priority programming JIRA ticket: CORE-12272 On its own, this PR does not yet change how Felix programs local routes. `SetRoutes` callers do not currently set the new `Priority` field, so programmed `Priority` will be 0 for IPv4 routes and 1024 for IPv6 routes, as was the case before this PR. Upcoming, but separate, live migration work will: - add Felix configuration fields for "normal" and "elevated" priority values - change route programming outside of live migration to use the "normal" priority value - program routes with the "elevated" priority value during a live migration, as part of ensuring the best possible handover --- felix/routetable/defs.go | 1 + felix/routetable/route_table.go | 50 ++++--- felix/routetable/route_table_test.go | 186 +++++++++++++++++++-------- 3 files changed, 160 insertions(+), 77 deletions(-) diff --git a/felix/routetable/defs.go b/felix/routetable/defs.go index 3151cb43295..1eb845d12e4 100644 --- a/felix/routetable/defs.go +++ b/felix/routetable/defs.go @@ -78,6 +78,7 @@ type Target struct { Protocol netlink.RouteProtocol MultiPath []NextHop MTU int + Priority int } func (t Target) Equal(t2 Target) bool { diff --git a/felix/routetable/route_table.go b/felix/routetable/route_table.go index c4b3f269752..38d97037197 100644 --- a/felix/routetable/route_table.go +++ b/felix/routetable/route_table.go @@ -310,8 +310,8 @@ func New( ifaceToRoutes: map[RouteClass]map[string]map[ip.CIDR]Target{}, cidrToIfaces: map[RouteClass]map[ip.CIDR]set.Set[string]{}, - kernelRoutes: deltatracker.New[kernelRouteKey, kernelRoute]( - deltatracker.WithValuesEqualFn[kernelRouteKey, kernelRoute](func(a, b kernelRoute) bool { + kernelRoutes: deltatracker.New( + deltatracker.WithValuesEqualFn[kernelRouteKey](func(a, b kernelRoute) bool { return a.Equals(b) }), ), @@ -479,17 +479,17 @@ func (r *RouteTable) SetRoutes(routeClass RouteClass, ifaceName string, targets } // Clean up the old CIDRs. - for cidr := range oldTargetsToCleanUp { + for cidr, target := range oldTargetsToCleanUp { r.logCxt.WithField("cidr", cidr).Debug("Cleaning up old route.") // removeOwningIface() calls recalculateDesiredKernelRoute. - r.removeOwningIface(routeClass, ifaceName, cidr) + r.removeOwningIface(routeClass, ifaceName, cidr, target) } // Clean out the pending ARP list, then recalculate it below. delete(r.permanentARPs, ifaceName) for cidr, target := range newTargets { // addOwningIface() calls recalculateDesiredKernelRoute. - r.addOwningIface(routeClass, ifaceName, cidr) + r.addOwningIface(routeClass, ifaceName, cidr, target) r.updatePermanentARP(ifaceName, cidr.Addr(), target.DestMAC) } } @@ -514,7 +514,7 @@ func (r *RouteTable) RouteUpdate(routeClass RouteClass, ifaceName string, target r.ifaceToRoutes[routeClass][ifaceName] = routesByCIDR } routesByCIDR[target.CIDR] = target - r.addOwningIface(routeClass, ifaceName, target.CIDR) + r.addOwningIface(routeClass, ifaceName, target.CIDR, target) r.updatePermanentARP(ifaceName, target.CIDR.Addr(), target.DestMAC) } @@ -527,11 +527,15 @@ func (r *RouteTable) RouteRemove(routeClass RouteClass, ifaceName string, cidr i return } + target, exists := r.ifaceToRoutes[routeClass][ifaceName][cidr] + if !exists { + return + } delete(r.ifaceToRoutes[routeClass][ifaceName], cidr) if len(r.ifaceToRoutes[routeClass][ifaceName]) == 0 { delete(r.ifaceToRoutes[routeClass], ifaceName) } - r.removeOwningIface(routeClass, ifaceName, cidr) + r.removeOwningIface(routeClass, ifaceName, cidr, target) r.removePermanentARP(ifaceName, cidr.Addr()) } @@ -562,7 +566,7 @@ func (r *RouteTable) removePermanentARP(ifaceName string, addr ip.Addr) { } } -func (r *RouteTable) addOwningIface(class RouteClass, ifaceName string, cidr ip.CIDR) { +func (r *RouteTable) addOwningIface(class RouteClass, ifaceName string, cidr ip.CIDR, target Target) { if r.cidrToIfaces[class] == nil { r.cidrToIfaces[class] = map[ip.CIDR]set.Set[string]{} } @@ -572,10 +576,10 @@ func (r *RouteTable) addOwningIface(class RouteClass, ifaceName string, cidr ip. r.cidrToIfaces[class][cidr] = ifaceNames } ifaceNames.Add(ifaceName) - r.recalculateDesiredKernelRoute(cidr) + r.recalculateDesiredKernelRoute(cidr, target) } -func (r *RouteTable) removeOwningIface(class RouteClass, ifaceName string, cidr ip.CIDR) { +func (r *RouteTable) removeOwningIface(class RouteClass, ifaceName string, cidr ip.CIDR, target Target) { ifaceNames, ok := r.cidrToIfaces[class][cidr] if !ok { return @@ -584,7 +588,7 @@ func (r *RouteTable) removeOwningIface(class RouteClass, ifaceName string, cidr if ifaceNames.Len() == 0 { delete(r.cidrToIfaces[class], cidr) } - r.recalculateDesiredKernelRoute(cidr) + r.recalculateDesiredKernelRoute(cidr, target) } // recheckRouteOwnershipsByIface reruns conflict resolution for all @@ -592,11 +596,11 @@ func (r *RouteTable) removeOwningIface(class RouteClass, ifaceName string, cidr func (r *RouteTable) recheckRouteOwnershipsByIface(name string) { seen := set.New[ip.CIDR]() for _, ifaceToRoutes := range r.ifaceToRoutes { - for cidr := range ifaceToRoutes[name] { + for cidr, target := range ifaceToRoutes[name] { if seen.Contains(cidr) { continue } - r.recalculateDesiredKernelRoute(cidr) + r.recalculateDesiredKernelRoute(cidr, target) seen.Add(cidr) } } @@ -625,9 +629,9 @@ func (r *RouteTable) ifaceNameForIndex(ifindex int) (string, bool) { return name, ok } -func (r *RouteTable) recalculateDesiredKernelRoute(cidr ip.CIDR) { +func (r *RouteTable) recalculateDesiredKernelRoute(cidr ip.CIDR, target Target) { defer r.updateGauges() - kernKey := r.routeKeyForCIDR(cidr) + kernKey := r.routeKeyForCIDR(cidr, target) oldDesiredRoute, _ := r.kernelRoutes.Desired().Get(kernKey) var bestTarget Target @@ -804,6 +808,7 @@ func (r *RouteTable) ReadRoutesFromKernel(ifaceName string) ([]Target, error) { CIDR: key.CIDR, Src: kernRoute.Src, Protocol: kernRoute.Protocol, + Priority: key.Priority, } switch kernRoute.Type { @@ -1117,8 +1122,8 @@ func (r *RouteTable) resyncIface(nl netlinkshim.Interface, ifaceName string) err // Look for routes that the tracker says are there but are actually missing. for _, ifaceToRoutes := range r.ifaceToRoutes { - for cidr := range ifaceToRoutes[ifaceName] { - kernKey := r.routeKeyForCIDR(cidr) + for cidr, target := range ifaceToRoutes[ifaceName] { + kernKey := r.routeKeyForCIDR(cidr, target) if seenRoutes.Contains(kernKey) { // Route still there; handled above. continue @@ -1236,12 +1241,15 @@ func (r *RouteTable) refreshIfaceStateBestEffort(nl netlinkshim.Interface, iface return nil } -func (r *RouteTable) routeKeyForCIDR(cidr ip.CIDR) kernelRouteKey { - key := kernelRouteKey{CIDR: cidr} - // For IPv6, set priority to 1024. The kernel treats priority 0 as a sigil +func (r *RouteTable) routeKeyForCIDR(cidr ip.CIDR, target Target) kernelRouteKey { + key := kernelRouteKey{ + CIDR: cidr, + Priority: target.Priority, + } + // If IPv6 and Priority is 0, set it to 1024. The kernel treats priority 0 as a sigil // meaning "use the default value", which is 1024 for IPv6. We need to set // an explicit priority so that routes round trip cleanly. - if r.ipVersion == 6 { + if r.ipVersion == 6 && key.Priority == 0 { key.Priority = 1024 } return key diff --git a/felix/routetable/route_table_test.go b/felix/routetable/route_table_test.go index e64ee7a7b03..8f7db7f4733 100644 --- a/felix/routetable/route_table_test.go +++ b/felix/routetable/route_table_test.go @@ -35,6 +35,8 @@ import ( "github.com/projectcalico/calico/felix/timeshim/mocktime" ) +const routePriorityForTest int = 48931 + var ( FelixRouteProtocol = netlink.RouteProtocol(syscall.RTPROT_BOOT) @@ -117,11 +119,11 @@ var _ = Describe("RouteTable v6", func() { Table: unix.RT_TABLE_MAIN, } rt.SetRoutes(RouteClassLocalWorkload, noopLink.LinkAttrs.Name, []Target{ - {CIDR: ip.MustParseCIDROrIP("10.0.0.4/32"), DestMAC: mac1}, + {CIDR: ip.MustParseCIDROrIP("10.0.0.4/32"), DestMAC: mac1, Priority: routePriorityForTest}, }) dataplane.AddMockRoute(&noopRoute) - // Route that should be deleted. + // Routes that should be deleted. deleteLink := dataplane.AddIface(5, "cali5", true, true) deleteRoute := netlink.Route{ LinkIndex: deleteLink.LinkAttrs.Index, @@ -132,12 +134,23 @@ var _ = Describe("RouteTable v6", func() { Table: unix.RT_TABLE_MAIN, } dataplane.AddMockRoute(&deleteRoute) + deleteRoute2 := netlink.Route{ + LinkIndex: deleteLink.LinkAttrs.Index, + Dst: mustParseCIDR("10.0.0.2/32"), + Type: syscall.RTN_UNICAST, + Protocol: FelixRouteProtocol, + Scope: netlink.SCOPE_LINK, + Table: unix.RT_TABLE_MAIN, + Priority: routePriorityForTest, + } + dataplane.AddMockRoute(&deleteRoute2) err := rt.Apply() Expect(err).ToNot(HaveOccurred()) Expect(dataplane.DeletedRouteKeys).ToNot(HaveKey(mocknetlink.KeyForRoute(&noopRoute))) Expect(dataplane.UpdatedRouteKeys).ToNot(HaveKey(mocknetlink.KeyForRoute(&noopRoute))) Expect(dataplane.DeletedRouteKeys).To(HaveKey(mocknetlink.KeyForRoute(&deleteRoute))) + Expect(dataplane.DeletedRouteKeys).To(HaveKey(mocknetlink.KeyForRoute(&deleteRoute2))) }) }) @@ -209,6 +222,7 @@ var _ = Describe("RouteTable", func() { Protocol: FelixRouteProtocol, Scope: netlink.SCOPE_LINK, Table: unix.RT_TABLE_MAIN, + Priority: routePriorityForTest, } dataplane.AddMockRoute(&cali1Route) cali3Route = netlink.Route{ @@ -219,6 +233,7 @@ var _ = Describe("RouteTable", func() { Protocol: FelixRouteProtocol, Scope: netlink.SCOPE_LINK, Table: unix.RT_TABLE_MAIN, + Priority: routePriorityForTest, } dataplane.AddMockRoute(&cali3Route) gatewayRoute = netlink.Route{ @@ -286,11 +301,12 @@ var _ = Describe("RouteTable", func() { } dataplane.AddMockRoute(&updateRoute) rt.SetRoutes(RouteClassLocalWorkload, updateLink.LinkAttrs.Name, []Target{ - {CIDR: ip.MustParseCIDROrIP("10.0.0.5"), DestMAC: mac1}, + {CIDR: ip.MustParseCIDROrIP("10.0.0.5"), DestMAC: mac1, Priority: routePriorityForTest}, }) fixedRoute := updateRoute fixedRoute.Src = nil + fixedRoute.Priority = routePriorityForTest err := rt.Apply() Expect(err).ToNot(HaveOccurred()) @@ -333,7 +349,7 @@ var _ = Describe("RouteTable", func() { // Route that needs to be added addLink := dataplane.AddIface(6, "cali6", true, true) rt.SetRoutes(RouteClassLocalWorkload, addLink.LinkAttrs.Name, []Target{ - {CIDR: ip.MustParseCIDROrIP("10.0.0.6"), DestMAC: mac1}, + {CIDR: ip.MustParseCIDROrIP("10.0.0.6"), DestMAC: mac1, Priority: routePriorityForTest}, }) err := rt.Apply() Expect(err).ToNot(HaveOccurred()) @@ -347,6 +363,7 @@ var _ = Describe("RouteTable", func() { Scope: netlink.SCOPE_LINK, Src: deviceRouteSourceAddress, Table: unix.RT_TABLE_MAIN, + Priority: routePriorityForTest, })) dataplane.ExpectNeighs(unix.AF_INET, netlink.Neigh{ Family: unix.AF_INET, @@ -365,7 +382,7 @@ var _ = Describe("RouteTable", func() { addLink := dataplane.AddIface(6, "cali6", true, true) linkIndex = addLink.LinkAttrs.Index rt.SetRoutes(RouteClassLocalWorkload, addLink.LinkAttrs.Name, []Target{ - {CIDR: ip.MustParseCIDROrIP("10.0.0.6"), DestMAC: mac1}, + {CIDR: ip.MustParseCIDROrIP("10.0.0.6"), DestMAC: mac1, Priority: routePriorityForTest}, }) err := rt.Apply() Expect(err).ToNot(HaveOccurred()) @@ -407,7 +424,7 @@ var _ = Describe("RouteTable", 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}, + {CIDR: ip.MustParseCIDROrIP("10.0.0.6"), DestMAC: mac1, Priority: routePriorityForTest}, }) rt.SetRoutes(RouteClassLocalWorkload, link.LinkAttrs.Name, nil) err := rt.Apply() @@ -421,7 +438,7 @@ var _ = Describe("RouteTable", func() { 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}, + {CIDR: cidr, DestMAC: mac1, Priority: routePriorityForTest}, }) rt.RouteRemove(RouteClassLocalWorkload, link.LinkAttrs.Name, cidr) err := rt.Apply() @@ -443,9 +460,10 @@ var _ = Describe("RouteTable", func() { Scope: netlink.SCOPE_LINK, Src: deviceRouteSourceAddress, Table: unix.RT_TABLE_MAIN, + Priority: routePriorityForTest, } rt.SetRoutes(RouteClassLocalWorkload, noopLink.LinkAttrs.Name, []Target{ - {CIDR: ip.MustParseCIDROrIP("10.0.0.4/32"), DestMAC: mac1}, + {CIDR: ip.MustParseCIDROrIP("10.0.0.4/32"), DestMAC: mac1, Priority: routePriorityForTest}, }) dataplane.AddMockRoute(&noopRoute) @@ -474,9 +492,10 @@ var _ = Describe("RouteTable", func() { Protocol: FelixRouteProtocol, Scope: netlink.SCOPE_LINK, Table: unix.RT_TABLE_MAIN, + Priority: routePriorityForTest, } rt.SetRoutes(RouteClassLocalWorkload, updateLink.LinkAttrs.Name, []Target{ - {CIDR: ip.MustParseCIDROrIP("10.0.0.5"), DestMAC: mac1}, + {CIDR: ip.MustParseCIDROrIP("10.0.0.5"), DestMAC: mac1, Priority: routePriorityForTest}, }) dataplane.AddMockRoute(&updateRoute) @@ -511,9 +530,10 @@ var _ = Describe("RouteTable", func() { Scope: netlink.SCOPE_LINK, Src: net.ParseIP("192.168.0.2"), Table: unix.RT_TABLE_MAIN, + Priority: routePriorityForTest, } rt.SetRoutes(RouteClassLocalWorkload, updateLink.LinkAttrs.Name, []Target{ - {CIDR: ip.MustParseCIDROrIP("10.0.0.5"), DestMAC: mac1}, + {CIDR: ip.MustParseCIDROrIP("10.0.0.5"), DestMAC: mac1, Priority: routePriorityForTest}, }) dataplane.AddMockRoute(&updateRoute) @@ -545,9 +565,10 @@ var _ = Describe("RouteTable", func() { Scope: netlink.SCOPE_LINK, Src: net.ParseIP("192.168.0.2"), Table: unix.RT_TABLE_MAIN, + Priority: routePriorityForTest, } rt.SetRoutes(RouteClassLocalWorkload, noopLink.LinkAttrs.Name, []Target{ - {CIDR: ip.MustParseCIDROrIP("10.0.0.5"), DestMAC: mac1, Src: ip.FromString("192.168.0.2")}, + {CIDR: ip.MustParseCIDROrIP("10.0.0.5"), DestMAC: mac1, Src: ip.FromString("192.168.0.2"), Priority: routePriorityForTest}, }) dataplane.AddMockRoute(&noopRoute) err := rt.Apply() @@ -566,9 +587,10 @@ var _ = Describe("RouteTable", func() { Scope: netlink.SCOPE_LINK, Src: net.ParseIP("192.168.0.2"), Table: unix.RT_TABLE_MAIN, + Priority: routePriorityForTest, } rt.SetRoutes(RouteClassLocalWorkload, noopLink.LinkAttrs.Name, []Target{ - {CIDR: ip.MustParseCIDROrIP("10.0.0.5"), DestMAC: mac1, Src: ip.FromString("192.168.0.3")}, + {CIDR: ip.MustParseCIDROrIP("10.0.0.5"), DestMAC: mac1, Src: ip.FromString("192.168.0.3"), Priority: routePriorityForTest}, }) dataplane.AddMockRoute(&noopRoute) err := rt.Apply() @@ -612,7 +634,7 @@ var _ = Describe("RouteTable", func() { // Route that needs to be added addLink := dataplane.AddIface(6, "cali6", true, true) rt.SetRoutes(RouteClassLocalWorkload, addLink.LinkAttrs.Name, []Target{ - {CIDR: ip.MustParseCIDROrIP("10.0.0.6"), DestMAC: mac1}, + {CIDR: ip.MustParseCIDROrIP("10.0.0.6"), DestMAC: mac1, Priority: routePriorityForTest}, }) err := rt.Apply() Expect(err).ToNot(HaveOccurred()) @@ -624,6 +646,7 @@ var _ = Describe("RouteTable", func() { Protocol: deviceRouteProtocol, Scope: netlink.SCOPE_LINK, Table: unix.RT_TABLE_MAIN, + Priority: routePriorityForTest, })) By("Reading back the route") @@ -632,6 +655,7 @@ var _ = Describe("RouteTable", func() { Type: TargetTypeLinkLocalUnicast, CIDR: ip.MustParseCIDROrIP("10.0.0.6"), Protocol: deviceRouteProtocol, + Priority: routePriorityForTest, }), ) }) @@ -653,6 +677,7 @@ var _ = Describe("RouteTable", func() { Gw: ip.FromString("10.0.0.7"), }, }, + Priority: routePriorityForTest, }, }) err := rt.Apply() @@ -678,6 +703,7 @@ var _ = Describe("RouteTable", func() { Flags: syscall.RTNH_F_ONLINK, }, }, + Priority: routePriorityForTest, })) By("Reading back the route") @@ -696,6 +722,7 @@ var _ = Describe("RouteTable", func() { Gw: ip.FromString("10.0.0.7"), }, }, + Priority: routePriorityForTest, })) }) It("Should add/remove multi-path routes when interface goes up/down", func() { @@ -719,6 +746,7 @@ var _ = Describe("RouteTable", func() { Gw: ip.FromString("10.0.0.7"), }, }, + Priority: routePriorityForTest, }, }) @@ -757,6 +785,7 @@ var _ = Describe("RouteTable", func() { Flags: syscall.RTNH_F_ONLINK, }, }, + Priority: routePriorityForTest, } Expect(dataplane.RouteKeyToRoute["254-10.0.0.0/24"]).To(Equal(expectedRoute)) @@ -800,6 +829,7 @@ var _ = Describe("RouteTable", func() { Gw: ip.FromString("10.0.0.7"), }, }, + Priority: routePriorityForTest, }, }) @@ -847,6 +877,7 @@ var _ = Describe("RouteTable", func() { Flags: syscall.RTNH_F_ONLINK, }, }, + Priority: routePriorityForTest, } Expect(dataplane.RouteKeyToRoute["254-10.0.0.0/24"]).To(Equal(expectedRoute)) @@ -866,8 +897,8 @@ var _ = Describe("RouteTable", func() { // Route that needs to be added addLink := dataplane.AddIface(6, "cali6", true, true) rt.SetRoutes(RouteClassLocalWorkload, addLink.LinkAttrs.Name, []Target{ - {CIDR: ip.MustParseCIDROrIP("10.0.0.6"), DestMAC: mac1}, - {CIDR: ip.MustParseCIDROrIP("10.0.0.7"), DestMAC: mac1}, + {CIDR: ip.MustParseCIDROrIP("10.0.0.6"), DestMAC: mac1, Priority: routePriorityForTest}, + {CIDR: ip.MustParseCIDROrIP("10.0.0.7"), DestMAC: mac1, Priority: routePriorityForTest}, }) err := rt.Apply() Expect(err).ToNot(HaveOccurred()) @@ -879,6 +910,7 @@ var _ = Describe("RouteTable", func() { Protocol: deviceRouteProtocol, Scope: netlink.SCOPE_LINK, Table: unix.RT_TABLE_MAIN, + Priority: routePriorityForTest, })) Expect(dataplane.RouteKeyToRoute["254-10.0.0.7/32"]).To(Equal(netlink.Route{ Family: unix.AF_INET, @@ -888,14 +920,15 @@ var _ = Describe("RouteTable", func() { Protocol: deviceRouteProtocol, Scope: netlink.SCOPE_LINK, Table: unix.RT_TABLE_MAIN, + Priority: routePriorityForTest, })) }) It("Should add multiple routes with a protocol after persistent failures", func() { // Route that needs to be added addLink := dataplane.AddIface(6, "cali6", true, true) rt.SetRoutes(RouteClassLocalWorkload, addLink.LinkAttrs.Name, []Target{ - {CIDR: ip.MustParseCIDROrIP("10.0.0.6"), DestMAC: mac1}, - {CIDR: ip.MustParseCIDROrIP("10.0.0.7"), DestMAC: mac1}, + {CIDR: ip.MustParseCIDROrIP("10.0.0.6"), DestMAC: mac1, Priority: routePriorityForTest}, + {CIDR: ip.MustParseCIDROrIP("10.0.0.7"), DestMAC: mac1, Priority: routePriorityForTest}, }) // Persist failures, this will apply the deltas to the cache but will be out of sync with the dataplane. dataplane.FailuresToSimulate = mocknetlink.FailNextRouteAdd | mocknetlink.FailNextRouteReplace @@ -916,6 +949,7 @@ var _ = Describe("RouteTable", func() { Protocol: deviceRouteProtocol, Scope: netlink.SCOPE_LINK, Table: unix.RT_TABLE_MAIN, + Priority: routePriorityForTest, })) Expect(dataplane.RouteKeyToRoute["254-10.0.0.7/32"]).To(Equal(netlink.Route{ Family: unix.AF_INET, @@ -925,6 +959,7 @@ var _ = Describe("RouteTable", func() { Protocol: deviceRouteProtocol, Scope: netlink.SCOPE_LINK, Table: unix.RT_TABLE_MAIN, + Priority: routePriorityForTest, })) }) It("Should not remove routes with a protocol", func() { @@ -938,9 +973,10 @@ var _ = Describe("RouteTable", func() { Protocol: deviceRouteProtocol, Scope: netlink.SCOPE_LINK, Table: unix.RT_TABLE_MAIN, + Priority: routePriorityForTest, } rt.SetRoutes(RouteClassLocalWorkload, noopLink.LinkAttrs.Name, []Target{ - {CIDR: ip.MustParseCIDROrIP("10.0.0.4/32"), DestMAC: mac1}, + {CIDR: ip.MustParseCIDROrIP("10.0.0.4/32"), DestMAC: mac1, Priority: routePriorityForTest}, }) dataplane.AddMockRoute(&noopRoute) @@ -959,9 +995,10 @@ var _ = Describe("RouteTable", func() { Type: syscall.RTN_UNICAST, Scope: netlink.SCOPE_LINK, Table: unix.RT_TABLE_MAIN, + Priority: routePriorityForTest, } rt.SetRoutes(RouteClassLocalWorkload, updateLink.LinkAttrs.Name, []Target{ - {CIDR: ip.MustParseCIDROrIP("10.0.0.5"), DestMAC: mac1}, + {CIDR: ip.MustParseCIDROrIP("10.0.0.5"), DestMAC: mac1, Priority: routePriorityForTest}, }) dataplane.AddMockRoute(&updateRoute) @@ -985,9 +1022,10 @@ var _ = Describe("RouteTable", func() { Protocol: 64, Scope: netlink.SCOPE_LINK, Table: unix.RT_TABLE_MAIN, + Priority: routePriorityForTest, } rt.SetRoutes(RouteClassLocalWorkload, updateLink.LinkAttrs.Name, []Target{ - {CIDR: ip.MustParseCIDROrIP("10.0.0.5"), DestMAC: mac1}, + {CIDR: ip.MustParseCIDROrIP("10.0.0.5"), DestMAC: mac1, Priority: routePriorityForTest}, }) dataplane.AddMockRoute(&updateRoute) @@ -1013,7 +1051,7 @@ var _ = Describe("RouteTable", func() { Expect(err).ToNot(HaveOccurred()) // We try to add 10.0.0.1 back in. rt.SetRoutes(RouteClassLocalWorkload, "cali1", []Target{ - {CIDR: ip.MustParseCIDROrIP("10.0.0.1/32"), DestMAC: mac1}, + {CIDR: ip.MustParseCIDROrIP("10.0.0.1/32"), DestMAC: mac1, Priority: routePriorityForTest}, }) start := time.Now() err = rt.Apply() @@ -1027,7 +1065,7 @@ var _ = Describe("RouteTable", func() { Expect(err).ToNot(HaveOccurred()) // We try to add 10.0.0.10, which hasn't been seen before. rt.SetRoutes(RouteClassLocalWorkload, "cali1", []Target{ - {CIDR: ip.MustParseCIDROrIP("10.0.0.10/32"), DestMAC: mac1}, + {CIDR: ip.MustParseCIDROrIP("10.0.0.10/32"), DestMAC: mac1, Priority: routePriorityForTest}, }) start := time.Now() err = rt.Apply() @@ -1055,7 +1093,8 @@ var _ = Describe("RouteTable", func() { dataplane.FailuresToSimulate = mocknetlink.FailNextRouteAdd | mocknetlink.FailNextRouteReplace dataplane.PersistFailures = true rt.RouteUpdate(RouteClassLocalWorkload, "cali3", Target{ - CIDR: ip.MustParseCIDROrIP("10.20.30.40"), + CIDR: ip.MustParseCIDROrIP("10.20.30.40"), + Priority: routePriorityForTest, }) err = rt.Apply() Expect(err).To(HaveOccurred()) @@ -1074,6 +1113,7 @@ var _ = Describe("RouteTable", func() { Protocol: FelixRouteProtocol, Scope: netlink.SCOPE_LINK, Table: unix.RT_TABLE_MAIN, + Priority: routePriorityForTest, })) }) @@ -1089,6 +1129,7 @@ var _ = Describe("RouteTable", func() { Protocol: FelixRouteProtocol, Scope: netlink.SCOPE_LINK, Table: unix.RT_TABLE_MAIN, + Priority: routePriorityForTest, })) }) }) @@ -1096,12 +1137,14 @@ var _ = Describe("RouteTable", func() { Describe("after adding two routes to cali3", func() { JustBeforeEach(func() { rt.RouteUpdate(RouteClassLocalWorkload, "cali3", Target{ - CIDR: ip.MustParseCIDROrIP("10.20.30.40"), + CIDR: ip.MustParseCIDROrIP("10.20.30.40"), + Priority: routePriorityForTest, }) err := rt.Apply() Expect(err).ToNot(HaveOccurred()) rt.RouteUpdate(RouteClassLocalWorkload, "cali3", Target{ - CIDR: ip.MustParseCIDROrIP("10.0.20.0/24"), + CIDR: ip.MustParseCIDROrIP("10.0.20.0/24"), + Priority: routePriorityForTest, }) err = rt.Apply() Expect(err).ToNot(HaveOccurred()) @@ -1116,6 +1159,7 @@ var _ = Describe("RouteTable", func() { Protocol: FelixRouteProtocol, Scope: netlink.SCOPE_LINK, Table: unix.RT_TABLE_MAIN, + Priority: routePriorityForTest, })) Expect(dataplane.RouteKeyToRoute).To(ContainElement(netlink.Route{ Family: unix.AF_INET, @@ -1125,17 +1169,20 @@ var _ = Describe("RouteTable", func() { Protocol: FelixRouteProtocol, Scope: netlink.SCOPE_LINK, Table: unix.RT_TABLE_MAIN, + Priority: routePriorityForTest, })) }) It("should make no dataplane updates when deleting, creating and updating back to the same target before the next apply", func() { rt.RouteRemove(RouteClassLocalWorkload, "cali3", ip.MustParseCIDROrIP("10.0.20.0/24")) rt.RouteUpdate(RouteClassLocalWorkload, "cali3", Target{ - CIDR: ip.MustParseCIDROrIP("10.0.20.0/24"), - GW: ip.FromString("1.2.3.4"), + CIDR: ip.MustParseCIDROrIP("10.0.20.0/24"), + GW: ip.FromString("1.2.3.4"), + Priority: routePriorityForTest, }) rt.RouteUpdate(RouteClassLocalWorkload, "cali3", Target{ - CIDR: ip.MustParseCIDROrIP("10.0.20.0/24"), + CIDR: ip.MustParseCIDROrIP("10.0.20.0/24"), + Priority: routePriorityForTest, }) dataplane.ResetDeltas() @@ -1153,6 +1200,7 @@ var _ = Describe("RouteTable", func() { Protocol: FelixRouteProtocol, Scope: netlink.SCOPE_LINK, Table: unix.RT_TABLE_MAIN, + Priority: routePriorityForTest, })) Expect(dataplane.RouteKeyToRoute).To(ContainElement(netlink.Route{ Family: unix.AF_INET, @@ -1162,15 +1210,18 @@ var _ = Describe("RouteTable", func() { Protocol: FelixRouteProtocol, Scope: netlink.SCOPE_LINK, Table: unix.RT_TABLE_MAIN, + Priority: routePriorityForTest, })) }) It("should make no dataplane updates when deleting and then setting back to the same target before the next apply", func() { rt.RouteRemove(RouteClassLocalWorkload, "cali3", ip.MustParseCIDROrIP("10.0.20.0/24")) rt.SetRoutes(RouteClassLocalWorkload, "cali3", []Target{{ - CIDR: ip.MustParseCIDROrIP("10.0.20.0/24"), + CIDR: ip.MustParseCIDROrIP("10.0.20.0/24"), + Priority: routePriorityForTest, }, { - CIDR: ip.MustParseCIDROrIP("10.20.30.40"), + CIDR: ip.MustParseCIDROrIP("10.20.30.40"), + Priority: routePriorityForTest, }}) dataplane.ResetDeltas() @@ -1189,6 +1240,7 @@ var _ = Describe("RouteTable", func() { Protocol: FelixRouteProtocol, Scope: netlink.SCOPE_LINK, Table: unix.RT_TABLE_MAIN, + Priority: routePriorityForTest, })) Expect(dataplane.RouteKeyToRoute).To(ContainElement(netlink.Route{ Family: unix.AF_INET, @@ -1198,6 +1250,7 @@ var _ = Describe("RouteTable", func() { Protocol: FelixRouteProtocol, Scope: netlink.SCOPE_LINK, Table: unix.RT_TABLE_MAIN, + Priority: routePriorityForTest, })) }) }) @@ -1205,10 +1258,10 @@ var _ = Describe("RouteTable", func() { Describe("delete interface", func() { BeforeEach(func() { rt.SetRoutes(RouteClassLocalWorkload, "cali1", []Target{ - {CIDR: ip.MustParseCIDROrIP("10.0.0.1/32")}, + {CIDR: ip.MustParseCIDROrIP("10.0.0.1/32"), Priority: routePriorityForTest}, }) rt.SetRoutes(RouteClassLocalWorkload, "cali3", []Target{ - {CIDR: ip.MustParseCIDROrIP("10.0.0.3/32")}, + {CIDR: ip.MustParseCIDROrIP("10.0.0.3/32"), Priority: routePriorityForTest}, }) // Apply the changes. err := rt.Apply() @@ -1239,13 +1292,13 @@ var _ = Describe("RouteTable", func() { Describe(desc, func() { BeforeEach(func() { rt.SetRoutes(RouteClassLocalWorkload, "cali1", []Target{ - {CIDR: ip.MustParseCIDROrIP("10.0.0.1/32"), DestMAC: mac1}, + {CIDR: ip.MustParseCIDROrIP("10.0.0.1/32"), DestMAC: mac1, Priority: routePriorityForTest}, }) rt.SetRoutes(RouteClassLocalWorkload, "cali2", []Target{ - {CIDR: ip.MustParseCIDROrIP("10.0.0.2/32"), DestMAC: mac2}, + {CIDR: ip.MustParseCIDROrIP("10.0.0.2/32"), DestMAC: mac2, Priority: routePriorityForTest}, }) rt.SetRoutes(RouteClassLocalWorkload, "cali3", []Target{ - {CIDR: ip.MustParseCIDROrIP("10.0.1.3/32")}, + {CIDR: ip.MustParseCIDROrIP("10.0.1.3/32"), Priority: routePriorityForTest}, }) dataplane.FailuresToSimulate = testFailFlags }) @@ -1295,6 +1348,7 @@ var _ = Describe("RouteTable", func() { Protocol: FelixRouteProtocol, Scope: netlink.SCOPE_LINK, Table: unix.RT_TABLE_MAIN, + Priority: routePriorityForTest, })) Expect(dataplane.AddedRouteKeys.Contains("254-10.0.0.1/32")).To(BeFalse()) }) @@ -1309,6 +1363,7 @@ var _ = Describe("RouteTable", func() { Protocol: FelixRouteProtocol, Scope: netlink.SCOPE_LINK, Table: unix.RT_TABLE_MAIN, + Priority: routePriorityForTest, })) }) It("should update changed route", func() { @@ -1321,6 +1376,7 @@ var _ = Describe("RouteTable", func() { Protocol: FelixRouteProtocol, Scope: netlink.SCOPE_LINK, Table: unix.RT_TABLE_MAIN, + Priority: routePriorityForTest, })) Expect(dataplane.DeletedRouteKeys.Contains("254-10.0.0.3/32")).To(BeTrue()) Eventually(dataplane.GetDeletedConntrackEntries).Should(ContainElement(net.ParseIP("10.0.0.3").To4())) @@ -1473,7 +1529,8 @@ var _ = Describe("RouteTable", func() { // Trigger a per-interface sync. rt.OnIfaceStateChanged("cali1", 2, ifacemonitor.StateUp) rt.RouteUpdate(RouteClassLocalWorkload, "cali1", Target{ - CIDR: ip.MustParseCIDROrIP("10.0.20.0/24"), + CIDR: ip.MustParseCIDROrIP("10.0.20.0/24"), + Priority: routePriorityForTest, }) err := rt.Apply() Expect(err).NotTo(HaveOccurred()) @@ -1664,6 +1721,7 @@ var _ = Describe("RouteTable (table 100)", func() { Protocol: FelixRouteProtocol, Scope: netlink.SCOPE_UNIVERSE, Table: 100, + Priority: routePriorityForTest, } dataplane.AddMockRoute(&throwRoute) @@ -1676,6 +1734,7 @@ var _ = Describe("RouteTable (table 100)", func() { Protocol: FelixRouteProtocol, Scope: netlink.SCOPE_LINK, Table: 100, + Priority: routePriorityForTest, } }) It("should tidy up non-link routes immediately and wait for the route cleanup delay for interface routes", func() { @@ -1714,8 +1773,9 @@ var _ = Describe("RouteTable (table 100)", func() { Describe("after configuring a throw route", func() { JustBeforeEach(func() { rt.RouteUpdate(RouteClassWireguard, InterfaceNone, Target{ - CIDR: ip.MustParseCIDROrIP("10.10.10.10/32"), - Type: TargetTypeThrow, + CIDR: ip.MustParseCIDROrIP("10.10.10.10/32"), + Type: TargetTypeThrow, + Priority: routePriorityForTest, }) err := rt.Apply() Expect(err).ToNot(HaveOccurred()) @@ -1731,8 +1791,9 @@ var _ = Describe("RouteTable (table 100)", func() { Describe("after configuring a throw route", func() { JustBeforeEach(func() { rt.RouteUpdate(RouteClassWireguard, InterfaceNone, Target{ - CIDR: ip.MustParseCIDROrIP("10.10.10.10/32"), - Type: TargetTypeThrow, + CIDR: ip.MustParseCIDROrIP("10.10.10.10/32"), + Type: TargetTypeThrow, + Priority: routePriorityForTest, }) err := rt.Apply() Expect(err).ToNot(HaveOccurred()) @@ -1743,7 +1804,8 @@ var _ = Describe("RouteTable (table 100)", func() { for range 3 { rt.RouteRemove(RouteClassWireguard, InterfaceNone, ip.MustParseCIDROrIP("10.10.10.10/32")) rt.RouteUpdate(RouteClassLocalWorkload, "cali", Target{ - CIDR: ip.MustParseCIDROrIP("10.10.10.10/32"), + CIDR: ip.MustParseCIDROrIP("10.10.10.10/32"), + Priority: routePriorityForTest, }) err := rt.Apply() Expect(err).ToNot(HaveOccurred()) @@ -1751,8 +1813,9 @@ var _ = Describe("RouteTable (table 100)", func() { rt.RouteRemove(RouteClassLocalWorkload, "cali", ip.MustParseCIDROrIP("10.10.10.10/32")) rt.RouteUpdate(RouteClassWireguard, InterfaceNone, Target{ - CIDR: ip.MustParseCIDROrIP("10.10.10.10/32"), - Type: TargetTypeThrow, + CIDR: ip.MustParseCIDROrIP("10.10.10.10/32"), + Type: TargetTypeThrow, + Priority: routePriorityForTest, }) err = rt.Apply() Expect(err).ToNot(HaveOccurred()) @@ -1762,7 +1825,8 @@ var _ = Describe("RouteTable (table 100)", func() { It("should prioritise a workload route over the throw route", func() { rt.RouteUpdate(RouteClassLocalWorkload, "cali", Target{ - CIDR: ip.MustParseCIDROrIP("10.10.10.10/32"), + CIDR: ip.MustParseCIDROrIP("10.10.10.10/32"), + Priority: routePriorityForTest, }) err := rt.Apply() Expect(err).ToNot(HaveOccurred()) @@ -1778,7 +1842,8 @@ var _ = Describe("RouteTable (table 100)", func() { Describe("throw route configured in dataplane, actual route is via cali", func() { It("the throw route should be removed and the interface route added", func() { rt.RouteUpdate(RouteClassLocalWorkload, "cali", Target{ - CIDR: ip.MustParseCIDROrIP("10.10.10.10/32"), + CIDR: ip.MustParseCIDROrIP("10.10.10.10/32"), + Priority: routePriorityForTest, }) err := rt.Apply() Expect(err).ToNot(HaveOccurred()) @@ -1789,8 +1854,9 @@ var _ = Describe("RouteTable (table 100)", func() { Describe("after configuring an existing throw route and then deleting it", func() { JustBeforeEach(func() { rt.RouteUpdate(RouteClassWireguard, InterfaceNone, Target{ - CIDR: ip.MustParseCIDROrIP("10.10.10.10/32"), - Type: TargetTypeThrow, + CIDR: ip.MustParseCIDROrIP("10.10.10.10/32"), + Type: TargetTypeThrow, + Priority: routePriorityForTest, }) err := rt.Apply() Expect(err).ToNot(HaveOccurred()) @@ -1809,14 +1875,16 @@ var _ = Describe("RouteTable (table 100)", func() { Describe("after configuring a throw route and then replacing it with a blackhole route", func() { JustBeforeEach(func() { rt.RouteUpdate(RouteClassIPAMBlockDrop, InterfaceNone, Target{ - CIDR: ip.MustParseCIDROrIP("10.10.10.10/32"), - Type: TargetTypeThrow, + CIDR: ip.MustParseCIDROrIP("10.10.10.10/32"), + Type: TargetTypeThrow, + Priority: routePriorityForTest, }) err := rt.Apply() Expect(err).ToNot(HaveOccurred()) rt.RouteUpdate(RouteClassIPAMBlockDrop, InterfaceNone, Target{ - CIDR: ip.MustParseCIDROrIP("10.10.10.10/32"), - Type: TargetTypeBlackhole, + CIDR: ip.MustParseCIDROrIP("10.10.10.10/32"), + Type: TargetTypeBlackhole, + Priority: routePriorityForTest, }) err = rt.Apply() Expect(err).ToNot(HaveOccurred()) @@ -1831,6 +1899,7 @@ var _ = Describe("RouteTable (table 100)", func() { Protocol: FelixRouteProtocol, Scope: netlink.SCOPE_UNIVERSE, Table: 100, + Priority: routePriorityForTest, })) Expect(dataplane.UpdatedRouteKeys.Contains("100-10.10.10.10/32")).To(BeTrue()) }) @@ -1839,14 +1908,16 @@ var _ = Describe("RouteTable (table 100)", func() { Describe("after configuring a blackhole route and then replacing it with a prohibit route", func() { JustBeforeEach(func() { rt.RouteUpdate(RouteClassIPAMBlockDrop, InterfaceNone, Target{ - CIDR: ip.MustParseCIDROrIP("10.10.10.10/32"), - Type: TargetTypeBlackhole, + CIDR: ip.MustParseCIDROrIP("10.10.10.10/32"), + Type: TargetTypeBlackhole, + Priority: routePriorityForTest, }) err := rt.Apply() Expect(err).ToNot(HaveOccurred()) rt.RouteUpdate(RouteClassIPAMBlockDrop, InterfaceNone, Target{ - CIDR: ip.MustParseCIDROrIP("10.10.10.10/32"), - Type: TargetTypeProhibit, + CIDR: ip.MustParseCIDROrIP("10.10.10.10/32"), + Type: TargetTypeProhibit, + Priority: routePriorityForTest, }) err = rt.Apply() Expect(err).ToNot(HaveOccurred()) @@ -1861,6 +1932,7 @@ var _ = Describe("RouteTable (table 100)", func() { Protocol: FelixRouteProtocol, Scope: netlink.SCOPE_UNIVERSE, Table: 100, + Priority: routePriorityForTest, })) Expect(dataplane.UpdatedRouteKeys.Contains("100-10.10.10.10/32")).To(BeTrue()) }) @@ -1880,13 +1952,15 @@ var _ = Describe("RouteTable (table 100)", func() { Protocol: FelixRouteProtocol, Scope: netlink.SCOPE_LINK, Table: 100, + Priority: routePriorityForTest, } }) It("should create the table as needed", func() { // In "strict" mode, RouteListFiltered returns an error if the routing table doesn't exist. // Check that is handled and that we proceed to create the route (and thus create the routing table). rt.RouteUpdate(RouteClassLocalWorkload, "cali", Target{ - CIDR: ip.MustParseCIDROrIP("10.0.0.1/32"), + CIDR: ip.MustParseCIDROrIP("10.0.0.1/32"), + Priority: routePriorityForTest, }) err := rt.Apply() Expect(err).ToNot(HaveOccurred()) From a1ddb458665a5119d840f6d769a0d94c2ec7bb6e Mon Sep 17 00:00:00 2001 From: Nell Jerram Date: Wed, 25 Feb 2026 17:52:19 +0000 Subject: [PATCH 2/8] Index routetable structs on kernelRouteKey instead of just CIDR Relatedly: - Change `RouteRemove` to take a `Target` (like `RouteUpdate` and `SetRoutes`) instead of just CIDR. (Only the key fields are significant in `RouteRemove` calls.) Todo: - Test adding/removing routes with same CIDR and different priorities at the same time. - Do we need to handle different priorities in `conntrackTracker` calls? --- felix/dataplane/linux/bpf_ep_mgr.go | 4 +- felix/dataplane/linux/endpoint_mgr_test.go | 2 +- felix/routetable/dummy_table.go | 3 +- felix/routetable/interface.go | 7 +- felix/routetable/route_table.go | 104 +++++++++++---------- felix/routetable/route_table_test.go | 14 +-- felix/wireguard/wireguard.go | 8 +- 7 files changed, 71 insertions(+), 71 deletions(-) diff --git a/felix/dataplane/linux/bpf_ep_mgr.go b/felix/dataplane/linux/bpf_ep_mgr.go index 7749a19fc2a..b32d2994068 100644 --- a/felix/dataplane/linux/bpf_ep_mgr.go +++ b/felix/dataplane/linux/bpf_ep_mgr.go @@ -4472,10 +4472,10 @@ func (m *bpfEndpointManager) setRoute(cidr ip.CIDR) { func (m *bpfEndpointManager) delRoute(cidr ip.CIDR) { if m.v6 != nil && cidr.Version() == 6 { - m.routeTableV6.RouteRemove(dataplanedefs.BPFInDev, cidr) + m.routeTableV6.RouteRemove(dataplanedefs.BPFInDev, routetable.Target{CIDR: cidr}) } if m.v4 != nil && cidr.Version() == 4 { - m.routeTableV4.RouteRemove(dataplanedefs.BPFInDev, cidr) + m.routeTableV4.RouteRemove(dataplanedefs.BPFInDev, routetable.Target{CIDR: cidr}) } logrus.WithFields(logrus.Fields{ "cidr": cidr, diff --git a/felix/dataplane/linux/endpoint_mgr_test.go b/felix/dataplane/linux/endpoint_mgr_test.go index 14dac979497..43cca781d2d 100644 --- a/felix/dataplane/linux/endpoint_mgr_test.go +++ b/felix/dataplane/linux/endpoint_mgr_test.go @@ -722,7 +722,7 @@ func (t *mockRouteTable) SetRoutes(routeClass routetable.RouteClass, ifaceName s t.currentRoutes[ifaceName] = targets } -func (t *mockRouteTable) RouteRemove(routeClass routetable.RouteClass, ifaceName string, cidr ip.CIDR) { +func (t *mockRouteTable) RouteRemove(routeClass routetable.RouteClass, ifaceName string, target routetable.Target) { } func (t *mockRouteTable) RouteUpdate(routeClass routetable.RouteClass, ifaceName string, target routetable.Target) { diff --git a/felix/routetable/dummy_table.go b/felix/routetable/dummy_table.go index 38db593cb75..67c91703e3a 100644 --- a/felix/routetable/dummy_table.go +++ b/felix/routetable/dummy_table.go @@ -2,7 +2,6 @@ package routetable import ( "github.com/projectcalico/calico/felix/ifacemonitor" - "github.com/projectcalico/calico/felix/ip" ) type DummyTable struct { @@ -24,7 +23,7 @@ func (*DummyTable) Apply() error { func (*DummyTable) SetRoutes(routeClass RouteClass, ifaceName string, targets []Target) { } -func (*DummyTable) RouteRemove(routeClass RouteClass, ifaceName string, cidr ip.CIDR) { +func (*DummyTable) RouteRemove(routeClass RouteClass, ifaceName string, target Target) { } func (*DummyTable) RouteUpdate(routeClass RouteClass, ifaceName string, target Target) { diff --git a/felix/routetable/interface.go b/felix/routetable/interface.go index 1f5c6bc4500..cf1bda2e80e 100644 --- a/felix/routetable/interface.go +++ b/felix/routetable/interface.go @@ -16,7 +16,6 @@ package routetable import ( "github.com/projectcalico/calico/felix/ifacemonitor" - "github.com/projectcalico/calico/felix/ip" ) // SyncerInterface is the interface used to manage data-sync of route table managers. This includes notification of @@ -31,7 +30,7 @@ type SyncerInterface interface { type Interface interface { SyncerInterface SetRoutes(routeClass RouteClass, ifaceName string, targets []Target) - RouteRemove(routeClass RouteClass, ifaceName string, cidr ip.CIDR) + RouteRemove(routeClass RouteClass, ifaceName string, target Target) RouteUpdate(routeClass RouteClass, ifaceName string, target Target) Index() int QueueResyncIface(ifaceName string) @@ -68,8 +67,8 @@ func (cv *ClassView) SetRoutes(ifaceName string, targets []Target) { cv.routeTable.SetRoutes(cv.class, ifaceName, targets) } -func (cv *ClassView) RouteRemove(ifaceName string, cidr ip.CIDR) { - cv.routeTable.RouteRemove(cv.class, ifaceName, cidr) +func (cv *ClassView) RouteRemove(ifaceName string, target Target) { + cv.routeTable.RouteRemove(cv.class, ifaceName, target) } func (cv *ClassView) RouteUpdate(ifaceName string, target Target) { diff --git a/felix/routetable/route_table.go b/felix/routetable/route_table.go index 38d97037197..4d19f9d9043 100644 --- a/felix/routetable/route_table.go +++ b/felix/routetable/route_table.go @@ -149,8 +149,8 @@ type RouteTable struct { // ifaceToRoutes and cidrToIfaces are our inputs, updated // eagerly when something in the manager layer tells us to change the // routes. - ifaceToRoutes map[RouteClass]map[string]map[ip.CIDR]Target - cidrToIfaces map[RouteClass]map[ip.CIDR]set.Set[string] + ifaceToRoutes map[RouteClass]map[string]map[kernelRouteKey]Target + cidrToIfaces map[RouteClass]map[kernelRouteKey]set.Set[string] // kernelRoutes tracks the relationship between the route that we want // to program for a given CIDR (i.e. the route selected after conflict @@ -307,8 +307,8 @@ func New( ifacesToARP: set.New[string](), ownershipPolicy: ownershipPolicy, - ifaceToRoutes: map[RouteClass]map[string]map[ip.CIDR]Target{}, - cidrToIfaces: map[RouteClass]map[ip.CIDR]set.Set[string]{}, + ifaceToRoutes: map[RouteClass]map[string]map[kernelRouteKey]Target{}, + cidrToIfaces: map[RouteClass]map[kernelRouteKey]set.Set[string]{}, kernelRoutes: deltatracker.New( deltatracker.WithValuesEqualFn[kernelRouteKey](func(a, b kernelRoute) bool { @@ -459,15 +459,16 @@ func (r *RouteTable) SetRoutes(routeClass RouteClass, ifaceName string, targets r.checkTargets(ifaceName, targets...) if r.ifaceToRoutes[routeClass] == nil { - r.ifaceToRoutes[routeClass] = map[string]map[ip.CIDR]Target{} + r.ifaceToRoutes[routeClass] = map[string]map[kernelRouteKey]Target{} } // Figure out what has changed. oldTargetsToCleanUp := r.ifaceToRoutes[routeClass][ifaceName] - newTargets := map[ip.CIDR]Target{} + newTargets := map[kernelRouteKey]Target{} for _, t := range targets { - delete(oldTargetsToCleanUp, t.CIDR) - newTargets[t.CIDR] = t + kernKey := r.routeKeyForTarget(&t) + delete(oldTargetsToCleanUp, kernKey) + newTargets[kernKey] = t } // Record the new desired state. @@ -479,18 +480,18 @@ func (r *RouteTable) SetRoutes(routeClass RouteClass, ifaceName string, targets } // Clean up the old CIDRs. - for cidr, target := range oldTargetsToCleanUp { - r.logCxt.WithField("cidr", cidr).Debug("Cleaning up old route.") + for kernKey := range oldTargetsToCleanUp { + r.logCxt.WithField("kernKey", kernKey).Debug("Cleaning up old route.") // removeOwningIface() calls recalculateDesiredKernelRoute. - r.removeOwningIface(routeClass, ifaceName, cidr, target) + r.removeOwningIface(routeClass, ifaceName, kernKey) } // Clean out the pending ARP list, then recalculate it below. delete(r.permanentARPs, ifaceName) - for cidr, target := range newTargets { + for kernKey, target := range newTargets { // addOwningIface() calls recalculateDesiredKernelRoute. - r.addOwningIface(routeClass, ifaceName, cidr, target) - r.updatePermanentARP(ifaceName, cidr.Addr(), target.DestMAC) + r.addOwningIface(routeClass, ifaceName, kernKey) + r.updatePermanentARP(ifaceName, kernKey.CIDR.Addr(), target.DestMAC) } } @@ -505,38 +506,40 @@ func (r *RouteTable) RouteUpdate(routeClass RouteClass, ifaceName string, target r.checkTargets(ifaceName, target) if r.ifaceToRoutes[routeClass] == nil { - r.ifaceToRoutes[routeClass] = map[string]map[ip.CIDR]Target{} + r.ifaceToRoutes[routeClass] = map[string]map[kernelRouteKey]Target{} } routesByCIDR := r.ifaceToRoutes[routeClass][ifaceName] if routesByCIDR == nil { - routesByCIDR = map[ip.CIDR]Target{} + routesByCIDR = map[kernelRouteKey]Target{} r.ifaceToRoutes[routeClass][ifaceName] = routesByCIDR } - routesByCIDR[target.CIDR] = target - r.addOwningIface(routeClass, ifaceName, target.CIDR, target) - r.updatePermanentARP(ifaceName, target.CIDR.Addr(), target.DestMAC) + kernKey := r.routeKeyForTarget(&target) + routesByCIDR[kernKey] = target + r.addOwningIface(routeClass, ifaceName, kernKey) + r.updatePermanentARP(ifaceName, kernKey.CIDR.Addr(), target.DestMAC) } // RouteRemove removes the route with the specified CIDR. These deltas will // be applied to any routes set using SetRoute. -func (r *RouteTable) RouteRemove(routeClass RouteClass, ifaceName string, cidr ip.CIDR) { +func (r *RouteTable) RouteRemove(routeClass RouteClass, ifaceName string, target Target) { if !r.ownershipPolicy.IfaceIsOurs(ifaceName) { r.logCxt.WithField("ifaceName", ifaceName).Error( "Cannot set route for interface not managed by this routetable.") return } - target, exists := r.ifaceToRoutes[routeClass][ifaceName][cidr] + kernKey := r.routeKeyForTarget(&target) + target, exists := r.ifaceToRoutes[routeClass][ifaceName][kernKey] if !exists { return } - delete(r.ifaceToRoutes[routeClass][ifaceName], cidr) + delete(r.ifaceToRoutes[routeClass][ifaceName], kernKey) if len(r.ifaceToRoutes[routeClass][ifaceName]) == 0 { delete(r.ifaceToRoutes[routeClass], ifaceName) } - r.removeOwningIface(routeClass, ifaceName, cidr, target) - r.removePermanentARP(ifaceName, cidr.Addr()) + r.removeOwningIface(routeClass, ifaceName, kernKey) + r.removePermanentARP(ifaceName, kernKey.CIDR.Addr()) } func (r *RouteTable) updatePermanentARP(ifaceName string, addr ip.Addr, mac net.HardwareAddr) { @@ -566,42 +569,42 @@ func (r *RouteTable) removePermanentARP(ifaceName string, addr ip.Addr) { } } -func (r *RouteTable) addOwningIface(class RouteClass, ifaceName string, cidr ip.CIDR, target Target) { +func (r *RouteTable) addOwningIface(class RouteClass, ifaceName string, kernKey kernelRouteKey) { if r.cidrToIfaces[class] == nil { - r.cidrToIfaces[class] = map[ip.CIDR]set.Set[string]{} + r.cidrToIfaces[class] = map[kernelRouteKey]set.Set[string]{} } - ifaceNames := r.cidrToIfaces[class][cidr] + ifaceNames := r.cidrToIfaces[class][kernKey] if ifaceNames == nil { ifaceNames = set.New[string]() - r.cidrToIfaces[class][cidr] = ifaceNames + r.cidrToIfaces[class][kernKey] = ifaceNames } ifaceNames.Add(ifaceName) - r.recalculateDesiredKernelRoute(cidr, target) + r.recalculateDesiredKernelRoute(kernKey) } -func (r *RouteTable) removeOwningIface(class RouteClass, ifaceName string, cidr ip.CIDR, target Target) { - ifaceNames, ok := r.cidrToIfaces[class][cidr] +func (r *RouteTable) removeOwningIface(class RouteClass, ifaceName string, kernKey kernelRouteKey) { + ifaceNames, ok := r.cidrToIfaces[class][kernKey] if !ok { return } ifaceNames.Discard(ifaceName) if ifaceNames.Len() == 0 { - delete(r.cidrToIfaces[class], cidr) + delete(r.cidrToIfaces[class], kernKey) } - r.recalculateDesiredKernelRoute(cidr, target) + r.recalculateDesiredKernelRoute(kernKey) } // recheckRouteOwnershipsByIface reruns conflict resolution for all // the interface's routes. func (r *RouteTable) recheckRouteOwnershipsByIface(name string) { - seen := set.New[ip.CIDR]() + seen := set.New[kernelRouteKey]() for _, ifaceToRoutes := range r.ifaceToRoutes { - for cidr, target := range ifaceToRoutes[name] { - if seen.Contains(cidr) { + for kernKey := range ifaceToRoutes[name] { + if seen.Contains(kernKey) { continue } - r.recalculateDesiredKernelRoute(cidr, target) - seen.Add(cidr) + r.recalculateDesiredKernelRoute(kernKey) + seen.Add(kernKey) } } } @@ -629,9 +632,8 @@ func (r *RouteTable) ifaceNameForIndex(ifindex int) (string, bool) { return name, ok } -func (r *RouteTable) recalculateDesiredKernelRoute(cidr ip.CIDR, target Target) { +func (r *RouteTable) recalculateDesiredKernelRoute(kernKey kernelRouteKey) { defer r.updateGauges() - kernKey := r.routeKeyForCIDR(cidr, target) oldDesiredRoute, _ := r.kernelRoutes.Desired().Get(kernKey) var bestTarget Target @@ -641,7 +643,7 @@ func (r *RouteTable) recalculateDesiredKernelRoute(cidr ip.CIDR, target Target) var candidates []string for routeClass, cidrToIface := range r.cidrToIfaces { - ifaces := cidrToIface[cidr] + ifaces := cidrToIface[kernKey] if ifaces == nil { continue } @@ -659,11 +661,11 @@ func (r *RouteTable) recalculateDesiredKernelRoute(cidr ip.CIDR, target Target) } someUp := false - target, ok := r.ifaceToRoutes[routeClass][ifaceName][cidr] + target, ok := r.ifaceToRoutes[routeClass][ifaceName][kernKey] if !ok { log.WithFields(log.Fields{ "ifaceName": ifaceName, - "cidr": cidr, + "kernKey": kernKey, }).Warn("Bug? No route for iface/CIDR (recalculateDesiredKernelRoute called too early?).") continue } @@ -709,18 +711,18 @@ func (r *RouteTable) recalculateDesiredKernelRoute(cidr ip.CIDR, target Target) if bestIfaceIdx == -1 { if len(candidates) == 0 { r.logCxt.WithFields(log.Fields{ - "cidr": cidr, + "kernKey": kernKey, }).Debug("CIDR no longer has any associated routes.") } else { r.logCxt.WithFields(log.Fields{ - "cidr": cidr, + "kernKey": kernKey, "candidates": candidates, }).Debug("No valid route for this CIDR (all candidate routes missing iface index).") } // Clean up the old entries. r.kernelRoutes.Desired().Delete(kernKey) - r.conntrackTracker.RemoveCIDROwner(cidr) + r.conntrackTracker.RemoveCIDROwner(kernKey.CIDR) return } @@ -773,7 +775,7 @@ func (r *RouteTable) recalculateDesiredKernelRoute(cidr ip.CIDR, target Target) } r.kernelRoutes.Desired().Set(kernKey, kernRoute) - r.conntrackTracker.UpdateCIDROwner(cidr, bestIfaceIdx, bestRouteClass) + r.conntrackTracker.UpdateCIDROwner(kernKey.CIDR, bestIfaceIdx, bestRouteClass) } func (r *RouteTable) QueueResync() { @@ -1122,8 +1124,8 @@ func (r *RouteTable) resyncIface(nl netlinkshim.Interface, ifaceName string) err // Look for routes that the tracker says are there but are actually missing. for _, ifaceToRoutes := range r.ifaceToRoutes { - for cidr, target := range ifaceToRoutes[ifaceName] { - kernKey := r.routeKeyForCIDR(cidr, target) + for _, target := range ifaceToRoutes[ifaceName] { + kernKey := r.routeKeyForTarget(&target) if seenRoutes.Contains(kernKey) { // Route still there; handled above. continue @@ -1241,9 +1243,9 @@ func (r *RouteTable) refreshIfaceStateBestEffort(nl netlinkshim.Interface, iface return nil } -func (r *RouteTable) routeKeyForCIDR(cidr ip.CIDR, target Target) kernelRouteKey { +func (r *RouteTable) routeKeyForTarget(target *Target) kernelRouteKey { key := kernelRouteKey{ - CIDR: cidr, + CIDR: target.CIDR, Priority: target.Priority, } // If IPv6 and Priority is 0, set it to 1024. The kernel treats priority 0 as a sigil diff --git a/felix/routetable/route_table_test.go b/felix/routetable/route_table_test.go index 8f7db7f4733..81d4fcde6a3 100644 --- a/felix/routetable/route_table_test.go +++ b/felix/routetable/route_table_test.go @@ -440,7 +440,7 @@ var _ = Describe("RouteTable", func() { rt.SetRoutes(RouteClassLocalWorkload, link.LinkAttrs.Name, []Target{ {CIDR: cidr, DestMAC: mac1, Priority: routePriorityForTest}, }) - rt.RouteRemove(RouteClassLocalWorkload, link.LinkAttrs.Name, cidr) + rt.RouteRemove(RouteClassLocalWorkload, link.LinkAttrs.Name, Target{CIDR: cidr, Priority: routePriorityForTest}) err := rt.Apply() Expect(err).ToNot(HaveOccurred()) @@ -1174,7 +1174,7 @@ var _ = Describe("RouteTable", func() { }) It("should make no dataplane updates when deleting, creating and updating back to the same target before the next apply", func() { - rt.RouteRemove(RouteClassLocalWorkload, "cali3", ip.MustParseCIDROrIP("10.0.20.0/24")) + rt.RouteRemove(RouteClassLocalWorkload, "cali3", Target{CIDR: ip.MustParseCIDROrIP("10.0.20.0/24"), Priority: routePriorityForTest}) rt.RouteUpdate(RouteClassLocalWorkload, "cali3", Target{ CIDR: ip.MustParseCIDROrIP("10.0.20.0/24"), GW: ip.FromString("1.2.3.4"), @@ -1215,7 +1215,7 @@ var _ = Describe("RouteTable", func() { }) It("should make no dataplane updates when deleting and then setting back to the same target before the next apply", func() { - rt.RouteRemove(RouteClassLocalWorkload, "cali3", ip.MustParseCIDROrIP("10.0.20.0/24")) + rt.RouteRemove(RouteClassLocalWorkload, "cali3", Target{CIDR: ip.MustParseCIDROrIP("10.0.20.0/24"), Priority: routePriorityForTest}) rt.SetRoutes(RouteClassLocalWorkload, "cali3", []Target{{ CIDR: ip.MustParseCIDROrIP("10.0.20.0/24"), Priority: routePriorityForTest, @@ -1802,7 +1802,7 @@ var _ = Describe("RouteTable (table 100)", func() { It("should be able to toggle between throw and local iface routes", func() { // Modify the action associated with a particular destination. for range 3 { - rt.RouteRemove(RouteClassWireguard, InterfaceNone, ip.MustParseCIDROrIP("10.10.10.10/32")) + rt.RouteRemove(RouteClassWireguard, InterfaceNone, Target{CIDR: ip.MustParseCIDROrIP("10.10.10.10/32"), Priority: routePriorityForTest}) rt.RouteUpdate(RouteClassLocalWorkload, "cali", Target{ CIDR: ip.MustParseCIDROrIP("10.10.10.10/32"), Priority: routePriorityForTest, @@ -1811,7 +1811,7 @@ var _ = Describe("RouteTable (table 100)", func() { Expect(err).ToNot(HaveOccurred()) Expect(dataplane.RouteKeyToRoute).To(ConsistOf(caliRoute, gatewayRoute, caliRouteTable100SameAsThrow)) - rt.RouteRemove(RouteClassLocalWorkload, "cali", ip.MustParseCIDROrIP("10.10.10.10/32")) + rt.RouteRemove(RouteClassLocalWorkload, "cali", Target{CIDR: ip.MustParseCIDROrIP("10.10.10.10/32"), Priority: routePriorityForTest}) rt.RouteUpdate(RouteClassWireguard, InterfaceNone, Target{ CIDR: ip.MustParseCIDROrIP("10.10.10.10/32"), Type: TargetTypeThrow, @@ -1832,7 +1832,7 @@ var _ = Describe("RouteTable (table 100)", func() { Expect(err).ToNot(HaveOccurred()) Expect(dataplane.RouteKeyToRoute).To(ConsistOf(caliRoute, gatewayRoute, caliRouteTable100SameAsThrow)) - rt.RouteRemove(RouteClassLocalWorkload, "cali", ip.MustParseCIDROrIP("10.10.10.10/32")) + rt.RouteRemove(RouteClassLocalWorkload, "cali", Target{CIDR: ip.MustParseCIDROrIP("10.10.10.10/32"), Priority: routePriorityForTest}) err = rt.Apply() Expect(err).ToNot(HaveOccurred()) Expect(dataplane.RouteKeyToRoute).To(ConsistOf(caliRoute, gatewayRoute, throwRoute)) @@ -1860,7 +1860,7 @@ var _ = Describe("RouteTable (table 100)", func() { }) err := rt.Apply() Expect(err).ToNot(HaveOccurred()) - rt.RouteRemove(RouteClassWireguard, InterfaceNone, ip.MustParseCIDROrIP("10.10.10.10/32")) + rt.RouteRemove(RouteClassWireguard, InterfaceNone, Target{CIDR: ip.MustParseCIDROrIP("10.10.10.10/32"), Priority: routePriorityForTest}) err = rt.Apply() Expect(err).ToNot(HaveOccurred()) }) diff --git a/felix/wireguard/wireguard.go b/felix/wireguard/wireguard.go index 6cae17d3e94..bceaca9cf9d 100644 --- a/felix/wireguard/wireguard.go +++ b/felix/wireguard/wireguard.go @@ -1096,8 +1096,8 @@ func (w *Wireguard) updateRouteTableFromNodeUpdates() { // not programmed. for cidr := range update.cidrsDeleted.All() { w.logCtx.WithField("cidr", cidr).Debug("Removing CIDR from routetable interface") - w.routetable.RouteRemove(w.interfaceName, cidr) - w.routetable.RouteRemove(routetable.InterfaceNone, cidr) + w.routetable.RouteRemove(w.interfaceName, routetable.Target{CIDR: cidr}) + w.routetable.RouteRemove(routetable.InterfaceNone, routetable.Target{CIDR: cidr}) } } @@ -1147,8 +1147,8 @@ func (w *Wireguard) updateRouteTableFromNodeUpdates() { // information to decide which route we need to remove - however we have also had bugs related to state // tracking so deleting both is reasonable - routetable ignores the one that is not programmed. updateLogCtx.Debug("Wireguard routing has changed - delete previous route") - w.routetable.RouteRemove(routetable.InterfaceNone, cidr) - w.routetable.RouteRemove(w.interfaceName, cidr) + w.routetable.RouteRemove(routetable.InterfaceNone, routetable.Target{CIDR: cidr}) + w.routetable.RouteRemove(w.interfaceName, routetable.Target{CIDR: cidr}) } w.routetable.RouteUpdate(ifaceName, routetable.Target{ Type: targetType, From 336461cc551e116ac0cc0b137820e9a7f5eb2be0 Mon Sep 17 00:00:00 2001 From: Nell Jerram Date: Fri, 27 Feb 2026 10:20:43 +0000 Subject: [PATCH 3/8] Use RouteKey in RouteTable API - Rename kernelRouteKey to RouteKey and make it part of the API. - In Target use an embedded RouteKey instead of equivalent individual fields. - Update RouteRemove to take RouteKey instead of Target. --- felix/dataplane/linux/bpf_ep_mgr.go | 16 +- felix/dataplane/linux/endpoint_mgr.go | 5 +- felix/dataplane/linux/endpoint_mgr_test.go | 50 +- felix/dataplane/linux/ipip_mgr.go | 6 +- felix/dataplane/linux/ipip_mgr_test.go | 6 +- felix/dataplane/linux/noencap_mgr_test.go | 24 +- felix/dataplane/linux/route_mgr.go | 12 +- felix/dataplane/linux/vxlan_mgr.go | 14 +- felix/dataplane/linux/vxlan_mgr_test.go | 12 +- felix/routetable/bench_test.go | 4 +- felix/routetable/defs.go | 25 +- felix/routetable/dummy_table.go | 2 +- felix/routetable/interface.go | 6 +- felix/routetable/route_table.go | 242 +++++----- felix/routetable/route_table_test.go | 525 ++++++++++++++------- felix/wireguard/wireguard.go | 12 +- 16 files changed, 593 insertions(+), 368 deletions(-) diff --git a/felix/dataplane/linux/bpf_ep_mgr.go b/felix/dataplane/linux/bpf_ep_mgr.go index b32d2994068..5e431943e24 100644 --- a/felix/dataplane/linux/bpf_ep_mgr.go +++ b/felix/dataplane/linux/bpf_ep_mgr.go @@ -3727,13 +3727,17 @@ func (m *bpfEndpointManager) ensureBPFDevices() error { if m.v4 != nil { m.routeTableV4.RouteUpdate(dataplanedefs.BPFInDev, routetable.Target{ Type: routetable.TargetTypeLinkLocalUnicast, - CIDR: bpfnatGWCIDR, + RouteKey: routetable.RouteKey{ + CIDR: bpfnatGWCIDR, + }, }) } if m.v6 != nil { m.routeTableV6.RouteUpdate(dataplanedefs.BPFInDev, routetable.Target{ Type: routetable.TargetTypeLinkLocalUnicast, - CIDR: bpfnatGWCIDRv6, + RouteKey: routetable.RouteKey{ + CIDR: bpfnatGWCIDRv6, + }, }) } @@ -4450,7 +4454,9 @@ var ( func (m *bpfEndpointManager) setRoute(cidr ip.CIDR) { target := routetable.Target{ Type: routetable.TargetTypeGlobalUnicast, - CIDR: cidr, + RouteKey: routetable.RouteKey{ + CIDR: cidr, + }, } if cidr.Version() == 6 { @@ -4472,10 +4478,10 @@ func (m *bpfEndpointManager) setRoute(cidr ip.CIDR) { func (m *bpfEndpointManager) delRoute(cidr ip.CIDR) { if m.v6 != nil && cidr.Version() == 6 { - m.routeTableV6.RouteRemove(dataplanedefs.BPFInDev, routetable.Target{CIDR: cidr}) + m.routeTableV6.RouteRemove(dataplanedefs.BPFInDev, routetable.RouteKey{CIDR: cidr}) } if m.v4 != nil && cidr.Version() == 4 { - m.routeTableV4.RouteRemove(dataplanedefs.BPFInDev, routetable.Target{CIDR: cidr}) + m.routeTableV4.RouteRemove(dataplanedefs.BPFInDev, routetable.RouteKey{CIDR: cidr}) } logrus.WithFields(logrus.Fields{ "cidr": cidr, diff --git a/felix/dataplane/linux/endpoint_mgr.go b/felix/dataplane/linux/endpoint_mgr.go index a72f02dae81..7d1b24320fd 100644 --- a/felix/dataplane/linux/endpoint_mgr.go +++ b/felix/dataplane/linux/endpoint_mgr.go @@ -825,7 +825,10 @@ func (m *endpointManager) resolveWorkloadEndpoints() { logCxt.Debug("Endpoint up, adding routes") for _, s := range ipStrings { routeTargets = append(routeTargets, routetable.Target{ - CIDR: ip.MustParseCIDROrIP(s), + RouteKey: routetable.RouteKey{ + + CIDR: ip.MustParseCIDROrIP(s), + }, DestMAC: mac, }) } diff --git a/felix/dataplane/linux/endpoint_mgr_test.go b/felix/dataplane/linux/endpoint_mgr_test.go index 43cca781d2d..743ba9e3dd6 100644 --- a/felix/dataplane/linux/endpoint_mgr_test.go +++ b/felix/dataplane/linux/endpoint_mgr_test.go @@ -722,7 +722,7 @@ func (t *mockRouteTable) SetRoutes(routeClass routetable.RouteClass, ifaceName s t.currentRoutes[ifaceName] = targets } -func (t *mockRouteTable) RouteRemove(routeClass routetable.RouteClass, ifaceName string, target routetable.Target) { +func (t *mockRouteTable) RouteRemove(routeClass routetable.RouteClass, ifaceName string, routeKey routetable.RouteKey) { } func (t *mockRouteTable) RouteUpdate(routeClass routetable.RouteClass, ifaceName string, target routetable.Target) { @@ -1870,12 +1870,16 @@ func endpointManagerTests(ipVersion uint8, flowlogs bool) func() { It("should set routes", func() { if ipVersion == 6 { routeTable.checkRoutes("cali12345-ab", []routetable.Target{{ - CIDR: ip.MustParseCIDROrIP("2001:db8:2::2/128"), + RouteKey: routetable.RouteKey{ + CIDR: ip.MustParseCIDROrIP("2001:db8:2::2/128"), + }, DestMAC: testutils.MustParseMAC("01:02:03:04:05:06"), }}) } else { routeTable.checkRoutes("cali12345-ab", []routetable.Target{{ - CIDR: ip.MustParseCIDROrIP("10.0.240.0/24"), + RouteKey: routetable.RouteKey{ + CIDR: ip.MustParseCIDROrIP("10.0.240.0/24"), + }, DestMAC: testutils.MustParseMAC("01:02:03:04:05:06"), }}) } @@ -1976,30 +1980,42 @@ func endpointManagerTests(ipVersion uint8, flowlogs bool) func() { if ipVersion == 6 { routeTable.checkRoutes("cali12345-ab", []routetable.Target{ { - CIDR: ip.MustParseCIDROrIP("2001:db8:2::2/128"), + RouteKey: routetable.RouteKey{ + CIDR: ip.MustParseCIDROrIP("2001:db8:2::2/128"), + }, DestMAC: testutils.MustParseMAC("01:02:03:04:05:06"), }, { - CIDR: ip.MustParseCIDROrIP("2001:db8:3::2/128"), + RouteKey: routetable.RouteKey{ + CIDR: ip.MustParseCIDROrIP("2001:db8:3::2/128"), + }, DestMAC: testutils.MustParseMAC("01:02:03:04:05:06"), }, { - CIDR: ip.MustParseCIDROrIP("2001:db8:4::2/128"), + RouteKey: routetable.RouteKey{ + CIDR: ip.MustParseCIDROrIP("2001:db8:4::2/128"), + }, DestMAC: testutils.MustParseMAC("01:02:03:04:05:06"), }, }) } else { routeTable.checkRoutes("cali12345-ab", []routetable.Target{ { - CIDR: ip.MustParseCIDROrIP("10.0.240.0/24"), + RouteKey: routetable.RouteKey{ + CIDR: ip.MustParseCIDROrIP("10.0.240.0/24"), + }, DestMAC: testutils.MustParseMAC("01:02:03:04:05:06"), }, { - CIDR: ip.MustParseCIDROrIP("172.16.1.3/32"), + RouteKey: routetable.RouteKey{ + CIDR: ip.MustParseCIDROrIP("172.16.1.3/32"), + }, DestMAC: testutils.MustParseMAC("01:02:03:04:05:06"), }, { - CIDR: ip.MustParseCIDROrIP("172.18.1.4/32"), + RouteKey: routetable.RouteKey{ + CIDR: ip.MustParseCIDROrIP("172.18.1.4/32"), + }, DestMAC: testutils.MustParseMAC("01:02:03:04:05:06"), }, }) @@ -2045,14 +2061,18 @@ func endpointManagerTests(ipVersion uint8, flowlogs bool) func() { if ipVersion == 6 { routeTable.checkRoutes("cali12345-ab", []routetable.Target{ { - CIDR: ip.MustParseCIDROrIP("2001:db8:2::2/128"), + RouteKey: routetable.RouteKey{ + CIDR: ip.MustParseCIDROrIP("2001:db8:2::2/128"), + }, DestMAC: testutils.MustParseMAC("01:02:03:04:05:06"), }, }) } else { routeTable.checkRoutes("cali12345-ab", []routetable.Target{ { - CIDR: ip.MustParseCIDROrIP("10.0.240.0/24"), + RouteKey: routetable.RouteKey{ + CIDR: ip.MustParseCIDROrIP("10.0.240.0/24"), + }, DestMAC: testutils.MustParseMAC("01:02:03:04:05:06"), }, }) @@ -2117,12 +2137,16 @@ func endpointManagerTests(ipVersion uint8, flowlogs bool) func() { It("should have set routes for new iface", func() { if ipVersion == 6 { routeTable.checkRoutes("cali12345-cd", []routetable.Target{{ - CIDR: ip.MustParseCIDROrIP("2001:db8:2::2/128"), + RouteKey: routetable.RouteKey{ + CIDR: ip.MustParseCIDROrIP("2001:db8:2::2/128"), + }, DestMAC: testutils.MustParseMAC("01:02:03:04:05:06"), }}) } else { routeTable.checkRoutes("cali12345-cd", []routetable.Target{{ - CIDR: ip.MustParseCIDROrIP("10.0.240.0/24"), + RouteKey: routetable.RouteKey{ + CIDR: ip.MustParseCIDROrIP("10.0.240.0/24"), + }, DestMAC: testutils.MustParseMAC("01:02:03:04:05:06"), }}) } diff --git a/felix/dataplane/linux/ipip_mgr.go b/felix/dataplane/linux/ipip_mgr.go index 6eee3fb4042..2a6850e02a7 100644 --- a/felix/dataplane/linux/ipip_mgr.go +++ b/felix/dataplane/linux/ipip_mgr.go @@ -166,8 +166,10 @@ func (m *ipipManager) tunnelRoute(cidr ip.CIDR, r *proto.RouteUpdate) *routetabl } return &routetable.Target{ - Type: routetable.TargetTypeOnLink, - CIDR: cidr, + Type: routetable.TargetTypeOnLink, + RouteKey: routetable.RouteKey{ + CIDR: cidr, + }, GW: ip.FromString(remoteAddr), Protocol: m.routeProtocol, MTU: m.dpConfig.IPIPMTU, diff --git a/felix/dataplane/linux/ipip_mgr_test.go b/felix/dataplane/linux/ipip_mgr_test.go index 0f7e82a4368..84a99394754 100644 --- a/felix/dataplane/linux/ipip_mgr_test.go +++ b/felix/dataplane/linux/ipip_mgr_test.go @@ -286,8 +286,10 @@ var _ = Describe("IPIPManager", func() { Expect(rt.currentRoutes[dataplanedefs.IPIPIfaceName]).To(HaveLen(1)) Expect(rt.currentRoutes[dataplanedefs.IPIPIfaceName][0]).To(Equal( routetable.Target{ - Type: "onlink", - CIDR: ip.MustParseCIDROrIP("10.0.1.1/32"), + Type: "onlink", + RouteKey: routetable.RouteKey{ + CIDR: ip.MustParseCIDROrIP("10.0.1.1/32"), + }, GW: ip.FromString("172.0.2.2"), Protocol: 80, })) diff --git a/felix/dataplane/linux/noencap_mgr_test.go b/felix/dataplane/linux/noencap_mgr_test.go index 011fa1b74ce..4b00cfd8c28 100644 --- a/felix/dataplane/linux/noencap_mgr_test.go +++ b/felix/dataplane/linux/noencap_mgr_test.go @@ -142,16 +142,20 @@ var _ = Describe("NoEncap Manager", func() { Expect(rt.currentRoutes[routetable.InterfaceNone]).To(HaveLen(1)) Expect(rt.currentRoutes[routetable.InterfaceNone][0]).To(Equal( routetable.Target{ - Type: "blackhole", - CIDR: ip.MustParseCIDROrIP("192.168.0.100/26"), + Type: "blackhole", + RouteKey: routetable.RouteKey{ + CIDR: ip.MustParseCIDROrIP("192.168.0.100/26"), + }, Protocol: 80, })) Expect(rt.currentRoutes["eth0"]).To(HaveLen(1)) Expect(rt.currentRoutes["eth0"][0]).To(Equal( routetable.Target{ - Type: "noencap", - CIDR: ip.MustParseCIDROrIP("192.168.0.0/26"), + Type: "noencap", + RouteKey: routetable.RouteKey{ + CIDR: ip.MustParseCIDROrIP("192.168.0.0/26"), + }, GW: ip.FromString("172.0.2.2"), Protocol: 80, })) @@ -225,16 +229,20 @@ var _ = Describe("NoEncap Manager", func() { Expect(rt.currentRoutes[routetable.InterfaceNone]).To(HaveLen(1)) Expect(rt.currentRoutes[routetable.InterfaceNone][0]).To(Equal( routetable.Target{ - Type: "blackhole", - CIDR: ip.MustParseCIDROrIP("dead:beef::1:30/112"), + Type: "blackhole", + RouteKey: routetable.RouteKey{ + CIDR: ip.MustParseCIDROrIP("dead:beef::1:30/112"), + }, Protocol: 80, })) Expect(rt.currentRoutes["eth0"]).To(HaveLen(1)) Expect(rt.currentRoutes["eth0"][0]).To(Equal( routetable.Target{ - Type: "noencap", - CIDR: ip.MustParseCIDROrIP("dead:beef::2:10/112"), + Type: "noencap", + RouteKey: routetable.RouteKey{ + CIDR: ip.MustParseCIDROrIP("dead:beef::2:10/112"), + }, GW: ip.FromString("fc00:10:10::1"), Protocol: 80, })) diff --git a/felix/dataplane/linux/route_mgr.go b/felix/dataplane/linux/route_mgr.go index e918efbe75c..39acc49213b 100644 --- a/felix/dataplane/linux/route_mgr.go +++ b/felix/dataplane/linux/route_mgr.go @@ -402,8 +402,10 @@ func blackholeRoutes(localIPAMBlocks map[string]*proto.RouteUpdate, proto netlin continue } rtt = append(rtt, routetable.Target{ - Type: routetable.TargetTypeBlackhole, - CIDR: cidr, + Type: routetable.TargetTypeBlackhole, + RouteKey: routetable.RouteKey{ + CIDR: cidr, + }, Protocol: proto, }) } @@ -421,8 +423,10 @@ func (m *routeManager) noEncapRoute(cidr ip.CIDR, r *proto.RouteUpdate) *routeta return nil } noEncapRoute := routetable.Target{ - Type: routetable.TargetTypeNoEncap, - CIDR: cidr, + Type: routetable.TargetTypeNoEncap, + RouteKey: routetable.RouteKey{ + CIDR: cidr, + }, GW: ip.FromString(r.DstNodeIp), Protocol: m.routeProtocol, } diff --git a/felix/dataplane/linux/vxlan_mgr.go b/felix/dataplane/linux/vxlan_mgr.go index 8ac6e99f6f7..1897e304707 100644 --- a/felix/dataplane/linux/vxlan_mgr.go +++ b/felix/dataplane/linux/vxlan_mgr.go @@ -269,8 +269,10 @@ func (m *vxlanManager) tunnelRoute(cidr ip.CIDR, r *proto.RouteUpdate) *routetab // We treat remote tunnel routes as directly connected. They don't have a gateway of // the VTEP because they ARE the VTEP! return &routetable.Target{ - CIDR: cidr, - MTU: m.mtu, + RouteKey: routetable.RouteKey{ + CIDR: cidr, + }, + MTU: m.mtu, } } @@ -286,9 +288,11 @@ func (m *vxlanManager) tunnelRoute(cidr ip.CIDR, r *proto.RouteUpdate) *routetab } return &routetable.Target{ Type: routetable.TargetTypeVXLAN, - CIDR: cidr, - GW: ip.FromString(vtepAddr), - MTU: m.mtu, + RouteKey: routetable.RouteKey{ + CIDR: cidr, + }, + GW: ip.FromString(vtepAddr), + MTU: m.mtu, } } diff --git a/felix/dataplane/linux/vxlan_mgr_test.go b/felix/dataplane/linux/vxlan_mgr_test.go index cff81b66983..bbd3fbc4785 100644 --- a/felix/dataplane/linux/vxlan_mgr_test.go +++ b/felix/dataplane/linux/vxlan_mgr_test.go @@ -432,8 +432,10 @@ var _ = Describe("VXLANManager", func() { Expect(rt.currentRoutes[dataplanedefs.VXLANIfaceNameV4]).To(HaveLen(1)) Expect(rt.currentRoutes[dataplanedefs.VXLANIfaceNameV4][0]).To(Equal( routetable.Target{ - CIDR: ip.MustParseCIDROrIP("10.0.1.1/32"), - MTU: 4444, + RouteKey: routetable.RouteKey{ + CIDR: ip.MustParseCIDROrIP("10.0.1.1/32"), + }, + MTU: 4444, })) // Delete the route. @@ -466,8 +468,10 @@ var _ = Describe("VXLANManager", func() { Expect(rt.currentRoutes[dataplanedefs.VXLANIfaceNameV6]).To(HaveLen(1)) Expect(rt.currentRoutes[dataplanedefs.VXLANIfaceNameV6][0]).To(Equal( routetable.Target{ - CIDR: ip.MustParseCIDROrIP("fc00:10:244::1/112"), - MTU: 6666, + RouteKey: routetable.RouteKey{ + CIDR: ip.MustParseCIDROrIP("fc00:10:244::1/112"), + }, + MTU: 6666, })) // Delete the route. diff --git a/felix/routetable/bench_test.go b/felix/routetable/bench_test.go index 64bf0b48658..3455d7ea310 100644 --- a/felix/routetable/bench_test.go +++ b/felix/routetable/bench_test.go @@ -90,7 +90,9 @@ outer: for i := range 256 { for j := range 256 { rt.RouteUpdate(RouteClassLocalWorkload, ifaceName, Target{ - CIDR: ip.MustParseCIDROrIP(fmt.Sprintf("10.0.%d.%d/32", i, j)), + RouteKey: RouteKey{ + CIDR: ip.MustParseCIDROrIP(fmt.Sprintf("10.0.%d.%d/32", i, j)), + }, }) n++ if n == numRoutes { diff --git a/felix/routetable/defs.go b/felix/routetable/defs.go index 1eb845d12e4..c9159ba42c9 100644 --- a/felix/routetable/defs.go +++ b/felix/routetable/defs.go @@ -16,6 +16,7 @@ package routetable import ( "errors" + "fmt" "net" "reflect" @@ -69,16 +70,36 @@ var ( ErrIfaceDown = errors.New("interface down") ) +// RouteKey represents the kernel's FIB key, and is also what we use on the RouteTable API to +// identify each route. The kernel and RouteTable API allow routes to coexist as long as they have +// different keys. +type RouteKey struct { + // Destination CIDR; route matches traffic to this destination. + CIDR ip.CIDR + // TOS is the Type-of-Service field. For example, one app may mark its + // packets as "high importance" and that will take a different route to + // another app. + // + // Kernel uses the TOS=0 route if there isn't a more precise match. + TOS int + // Priority is the routing metric / distance. Given two routes with the + // same CIDR, the kernel prefers the route with the _lower_ priority. + Priority int +} + +func (k RouteKey) String() string { + return fmt.Sprintf("%s(tos=%x metric=%d)", k.CIDR.String(), k.TOS, k.Priority) +} + type Target struct { + RouteKey Type TargetType - CIDR ip.CIDR GW ip.Addr Src ip.Addr DestMAC net.HardwareAddr Protocol netlink.RouteProtocol MultiPath []NextHop MTU int - Priority int } func (t Target) Equal(t2 Target) bool { diff --git a/felix/routetable/dummy_table.go b/felix/routetable/dummy_table.go index 67c91703e3a..52091de002a 100644 --- a/felix/routetable/dummy_table.go +++ b/felix/routetable/dummy_table.go @@ -23,7 +23,7 @@ func (*DummyTable) Apply() error { func (*DummyTable) SetRoutes(routeClass RouteClass, ifaceName string, targets []Target) { } -func (*DummyTable) RouteRemove(routeClass RouteClass, ifaceName string, target Target) { +func (*DummyTable) RouteRemove(routeClass RouteClass, ifaceName string, routeKey RouteKey) { } func (*DummyTable) RouteUpdate(routeClass RouteClass, ifaceName string, target Target) { diff --git a/felix/routetable/interface.go b/felix/routetable/interface.go index cf1bda2e80e..057bdc222d2 100644 --- a/felix/routetable/interface.go +++ b/felix/routetable/interface.go @@ -30,7 +30,7 @@ type SyncerInterface interface { type Interface interface { SyncerInterface SetRoutes(routeClass RouteClass, ifaceName string, targets []Target) - RouteRemove(routeClass RouteClass, ifaceName string, target Target) + RouteRemove(routeClass RouteClass, ifaceName string, routeKey RouteKey) RouteUpdate(routeClass RouteClass, ifaceName string, target Target) Index() int QueueResyncIface(ifaceName string) @@ -67,8 +67,8 @@ func (cv *ClassView) SetRoutes(ifaceName string, targets []Target) { cv.routeTable.SetRoutes(cv.class, ifaceName, targets) } -func (cv *ClassView) RouteRemove(ifaceName string, target Target) { - cv.routeTable.RouteRemove(cv.class, ifaceName, target) +func (cv *ClassView) RouteRemove(ifaceName string, routeKey RouteKey) { + cv.routeTable.RouteRemove(cv.class, ifaceName, routeKey) } func (cv *ClassView) RouteUpdate(ifaceName string, target Target) { diff --git a/felix/routetable/route_table.go b/felix/routetable/route_table.go index 4d19f9d9043..62460324721 100644 --- a/felix/routetable/route_table.go +++ b/felix/routetable/route_table.go @@ -1,4 +1,4 @@ -// Copyright (c) 2016-2023 Tigera, Inc. All rights reserved. +// Copyright (c) 2016-2026 Tigera, Inc. All rights reserved. // // Licensed under the Apache License, Version 2.0 (the "License"); // you may not use this file except in compliance with the License. @@ -149,14 +149,14 @@ type RouteTable struct { // ifaceToRoutes and cidrToIfaces are our inputs, updated // eagerly when something in the manager layer tells us to change the // routes. - ifaceToRoutes map[RouteClass]map[string]map[kernelRouteKey]Target - cidrToIfaces map[RouteClass]map[kernelRouteKey]set.Set[string] + ifaceToRoutes map[RouteClass]map[string]map[RouteKey]Target + cidrToIfaces map[RouteClass]map[RouteKey]set.Set[string] // kernelRoutes tracks the relationship between the route that we want // to program for a given CIDR (i.e. the route selected after conflict // resolution if there are multiple routes) and the route that's actually // in the kernel. - kernelRoutes *deltatracker.DeltaTracker[kernelRouteKey, kernelRoute] + kernelRoutes *deltatracker.DeltaTracker[RouteKey, kernelRoute] permanentARPs map[string]map[ip.Addr]net.HardwareAddr ifaceNameToIndex map[string]int @@ -307,11 +307,11 @@ func New( ifacesToARP: set.New[string](), ownershipPolicy: ownershipPolicy, - ifaceToRoutes: map[RouteClass]map[string]map[kernelRouteKey]Target{}, - cidrToIfaces: map[RouteClass]map[kernelRouteKey]set.Set[string]{}, + ifaceToRoutes: map[RouteClass]map[string]map[RouteKey]Target{}, + cidrToIfaces: map[RouteClass]map[RouteKey]set.Set[string]{}, kernelRoutes: deltatracker.New( - deltatracker.WithValuesEqualFn[kernelRouteKey](func(a, b kernelRoute) bool { + deltatracker.WithValuesEqualFn[RouteKey](func(a, b kernelRoute) bool { return a.Equals(b) }), ), @@ -459,16 +459,16 @@ func (r *RouteTable) SetRoutes(routeClass RouteClass, ifaceName string, targets r.checkTargets(ifaceName, targets...) if r.ifaceToRoutes[routeClass] == nil { - r.ifaceToRoutes[routeClass] = map[string]map[kernelRouteKey]Target{} + r.ifaceToRoutes[routeClass] = map[string]map[RouteKey]Target{} } // Figure out what has changed. oldTargetsToCleanUp := r.ifaceToRoutes[routeClass][ifaceName] - newTargets := map[kernelRouteKey]Target{} + newTargets := map[RouteKey]Target{} for _, t := range targets { - kernKey := r.routeKeyForTarget(&t) - delete(oldTargetsToCleanUp, kernKey) - newTargets[kernKey] = t + routeKey := r.routeKeyForTarget(&t) + delete(oldTargetsToCleanUp, routeKey) + newTargets[routeKey] = t } // Record the new desired state. @@ -480,18 +480,18 @@ func (r *RouteTable) SetRoutes(routeClass RouteClass, ifaceName string, targets } // Clean up the old CIDRs. - for kernKey := range oldTargetsToCleanUp { - r.logCxt.WithField("kernKey", kernKey).Debug("Cleaning up old route.") + for routeKey := range oldTargetsToCleanUp { + r.logCxt.WithField("routeKey", routeKey).Debug("Cleaning up old route.") // removeOwningIface() calls recalculateDesiredKernelRoute. - r.removeOwningIface(routeClass, ifaceName, kernKey) + r.removeOwningIface(routeClass, ifaceName, routeKey) } // Clean out the pending ARP list, then recalculate it below. delete(r.permanentARPs, ifaceName) - for kernKey, target := range newTargets { + for routeKey, target := range newTargets { // addOwningIface() calls recalculateDesiredKernelRoute. - r.addOwningIface(routeClass, ifaceName, kernKey) - r.updatePermanentARP(ifaceName, kernKey.CIDR.Addr(), target.DestMAC) + r.addOwningIface(routeClass, ifaceName, routeKey) + r.updatePermanentARP(ifaceName, routeKey.CIDR.Addr(), target.DestMAC) } } @@ -506,40 +506,39 @@ func (r *RouteTable) RouteUpdate(routeClass RouteClass, ifaceName string, target r.checkTargets(ifaceName, target) if r.ifaceToRoutes[routeClass] == nil { - r.ifaceToRoutes[routeClass] = map[string]map[kernelRouteKey]Target{} + r.ifaceToRoutes[routeClass] = map[string]map[RouteKey]Target{} } routesByCIDR := r.ifaceToRoutes[routeClass][ifaceName] if routesByCIDR == nil { - routesByCIDR = map[kernelRouteKey]Target{} + routesByCIDR = map[RouteKey]Target{} r.ifaceToRoutes[routeClass][ifaceName] = routesByCIDR } - kernKey := r.routeKeyForTarget(&target) - routesByCIDR[kernKey] = target - r.addOwningIface(routeClass, ifaceName, kernKey) - r.updatePermanentARP(ifaceName, kernKey.CIDR.Addr(), target.DestMAC) + routeKey := r.routeKeyForTarget(&target) + routesByCIDR[routeKey] = target + r.addOwningIface(routeClass, ifaceName, routeKey) + r.updatePermanentARP(ifaceName, routeKey.CIDR.Addr(), target.DestMAC) } // RouteRemove removes the route with the specified CIDR. These deltas will // be applied to any routes set using SetRoute. -func (r *RouteTable) RouteRemove(routeClass RouteClass, ifaceName string, target Target) { +func (r *RouteTable) RouteRemove(routeClass RouteClass, ifaceName string, routeKey RouteKey) { if !r.ownershipPolicy.IfaceIsOurs(ifaceName) { r.logCxt.WithField("ifaceName", ifaceName).Error( "Cannot set route for interface not managed by this routetable.") return } - kernKey := r.routeKeyForTarget(&target) - target, exists := r.ifaceToRoutes[routeClass][ifaceName][kernKey] + _, exists := r.ifaceToRoutes[routeClass][ifaceName][routeKey] if !exists { return } - delete(r.ifaceToRoutes[routeClass][ifaceName], kernKey) + delete(r.ifaceToRoutes[routeClass][ifaceName], routeKey) if len(r.ifaceToRoutes[routeClass][ifaceName]) == 0 { delete(r.ifaceToRoutes[routeClass], ifaceName) } - r.removeOwningIface(routeClass, ifaceName, kernKey) - r.removePermanentARP(ifaceName, kernKey.CIDR.Addr()) + r.removeOwningIface(routeClass, ifaceName, routeKey) + r.removePermanentARP(ifaceName, routeKey.CIDR.Addr()) } func (r *RouteTable) updatePermanentARP(ifaceName string, addr ip.Addr, mac net.HardwareAddr) { @@ -569,42 +568,42 @@ func (r *RouteTable) removePermanentARP(ifaceName string, addr ip.Addr) { } } -func (r *RouteTable) addOwningIface(class RouteClass, ifaceName string, kernKey kernelRouteKey) { +func (r *RouteTable) addOwningIface(class RouteClass, ifaceName string, routeKey RouteKey) { if r.cidrToIfaces[class] == nil { - r.cidrToIfaces[class] = map[kernelRouteKey]set.Set[string]{} + r.cidrToIfaces[class] = map[RouteKey]set.Set[string]{} } - ifaceNames := r.cidrToIfaces[class][kernKey] + ifaceNames := r.cidrToIfaces[class][routeKey] if ifaceNames == nil { ifaceNames = set.New[string]() - r.cidrToIfaces[class][kernKey] = ifaceNames + r.cidrToIfaces[class][routeKey] = ifaceNames } ifaceNames.Add(ifaceName) - r.recalculateDesiredKernelRoute(kernKey) + r.recalculateDesiredKernelRoute(routeKey) } -func (r *RouteTable) removeOwningIface(class RouteClass, ifaceName string, kernKey kernelRouteKey) { - ifaceNames, ok := r.cidrToIfaces[class][kernKey] +func (r *RouteTable) removeOwningIface(class RouteClass, ifaceName string, routeKey RouteKey) { + ifaceNames, ok := r.cidrToIfaces[class][routeKey] if !ok { return } ifaceNames.Discard(ifaceName) if ifaceNames.Len() == 0 { - delete(r.cidrToIfaces[class], kernKey) + delete(r.cidrToIfaces[class], routeKey) } - r.recalculateDesiredKernelRoute(kernKey) + r.recalculateDesiredKernelRoute(routeKey) } // recheckRouteOwnershipsByIface reruns conflict resolution for all // the interface's routes. func (r *RouteTable) recheckRouteOwnershipsByIface(name string) { - seen := set.New[kernelRouteKey]() + seen := set.New[RouteKey]() for _, ifaceToRoutes := range r.ifaceToRoutes { - for kernKey := range ifaceToRoutes[name] { - if seen.Contains(kernKey) { + for routeKey := range ifaceToRoutes[name] { + if seen.Contains(routeKey) { continue } - r.recalculateDesiredKernelRoute(kernKey) - seen.Add(kernKey) + r.recalculateDesiredKernelRoute(routeKey) + seen.Add(routeKey) } } } @@ -632,9 +631,9 @@ func (r *RouteTable) ifaceNameForIndex(ifindex int) (string, bool) { return name, ok } -func (r *RouteTable) recalculateDesiredKernelRoute(kernKey kernelRouteKey) { +func (r *RouteTable) recalculateDesiredKernelRoute(routeKey RouteKey) { defer r.updateGauges() - oldDesiredRoute, _ := r.kernelRoutes.Desired().Get(kernKey) + oldDesiredRoute, _ := r.kernelRoutes.Desired().Get(routeKey) var bestTarget Target bestRouteClass := RouteClassMax @@ -643,7 +642,7 @@ func (r *RouteTable) recalculateDesiredKernelRoute(kernKey kernelRouteKey) { var candidates []string for routeClass, cidrToIface := range r.cidrToIfaces { - ifaces := cidrToIface[kernKey] + ifaces := cidrToIface[routeKey] if ifaces == nil { continue } @@ -661,11 +660,11 @@ func (r *RouteTable) recalculateDesiredKernelRoute(kernKey kernelRouteKey) { } someUp := false - target, ok := r.ifaceToRoutes[routeClass][ifaceName][kernKey] + target, ok := r.ifaceToRoutes[routeClass][ifaceName][routeKey] if !ok { log.WithFields(log.Fields{ "ifaceName": ifaceName, - "kernKey": kernKey, + "routeKey": routeKey, }).Warn("Bug? No route for iface/CIDR (recalculateDesiredKernelRoute called too early?).") continue } @@ -711,18 +710,18 @@ func (r *RouteTable) recalculateDesiredKernelRoute(kernKey kernelRouteKey) { if bestIfaceIdx == -1 { if len(candidates) == 0 { r.logCxt.WithFields(log.Fields{ - "kernKey": kernKey, + "routeKey": routeKey, }).Debug("CIDR no longer has any associated routes.") } else { r.logCxt.WithFields(log.Fields{ - "kernKey": kernKey, + "routeKey": routeKey, "candidates": candidates, }).Debug("No valid route for this CIDR (all candidate routes missing iface index).") } // Clean up the old entries. - r.kernelRoutes.Desired().Delete(kernKey) - r.conntrackTracker.RemoveCIDROwner(kernKey.CIDR) + r.kernelRoutes.Desired().Delete(routeKey) + r.conntrackTracker.RemoveCIDROwner(routeKey.CIDR) return } @@ -761,21 +760,21 @@ func (r *RouteTable) recalculateDesiredKernelRoute(kernKey kernelRouteKey) { } if log.IsLevelEnabled(log.DebugLevel) && !reflect.DeepEqual(oldDesiredRoute, kernRoute) { r.logCxt.WithFields(log.Fields{ - "dst": kernKey, + "dst": routeKey, "oldRoute": oldDesiredRoute, "newRoute": kernRoute, "iface": bestIface, }).Debug("Preferred kernel route for this dest has changed.") } else if log.IsLevelEnabled(log.DebugLevel) { r.logCxt.WithFields(log.Fields{ - "dst": kernKey, + "dst": routeKey, "route": kernRoute, "iface": bestIface, }).Debug("Preferred kernel route for this dest still the same.") } - r.kernelRoutes.Desired().Set(kernKey, kernRoute) - r.conntrackTracker.UpdateCIDROwner(kernKey.CIDR, bestIfaceIdx, bestRouteClass) + r.kernelRoutes.Desired().Set(routeKey, kernRoute) + r.conntrackTracker.UpdateCIDROwner(routeKey.CIDR, bestIfaceIdx, bestRouteClass) } func (r *RouteTable) QueueResync() { @@ -801,16 +800,15 @@ func (r *RouteTable) ReadRoutesFromKernel(ifaceName string) ([]Target, error) { } var allTargets []Target - r.kernelRoutes.Dataplane().Iter(func(key kernelRouteKey, kernRoute kernelRoute) { + r.kernelRoutes.Dataplane().Iter(func(key RouteKey, kernRoute kernelRoute) { if kernRoute.Ifindex != ifaceIndex { return } target := Target{ - CIDR: key.CIDR, + RouteKey: key, Src: kernRoute.Src, Protocol: kernRoute.Protocol, - Priority: key.Priority, } switch kernRoute.Type { @@ -955,7 +953,7 @@ func (r *RouteTable) doFullResync(nl netlinkshim.Interface) error { routeFilterFlags := netlink.RT_FILTER_TABLE var err error - seenKeys := set.NewSize[kernelRouteKey](r.kernelRoutes.Dataplane().Len()) + seenKeys := set.NewSize[RouteKey](r.kernelRoutes.Dataplane().Len()) for range routeListFilterAttempts { // Using the Iter version here saves allocating a large slice of netlink.Route, // which we immediately discard. @@ -971,11 +969,11 @@ func (r *RouteTable) doFullResync(nl netlinkshim.Interface) error { return true } - kernKey, kernRoute := r.netlinkRouteToKernelRoute(&scratchRoute) - if oldRoute, ok := r.kernelRoutes.Dataplane().Get(kernKey); !ok || oldRoute.Equals(kernRoute) { - r.kernelRoutes.Dataplane().Set(kernKey, kernRoute) + routeKey, kernRoute := r.netlinkRouteToKernelRoute(&scratchRoute) + if oldRoute, ok := r.kernelRoutes.Dataplane().Get(routeKey); !ok || oldRoute.Equals(kernRoute) { + r.kernelRoutes.Dataplane().Set(routeKey, kernRoute) } - seenKeys.Add(kernKey) + seenKeys.Add(routeKey) r.livenessCallback() return true }) @@ -998,10 +996,10 @@ func (r *RouteTable) doFullResync(nl netlinkshim.Interface) error { return fmt.Errorf("failed to list all routes for resync: %w", err) } - 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.kernelRoutes.Dataplane().Iter(func(routeKey RouteKey, kernRoute kernelRoute) { + if !seenKeys.Contains(routeKey) { + r.kernelRoutes.Dataplane().Delete(routeKey) + r.conntrackTracker.OnDataplaneRouteDeleted(routeKey.CIDR, kernRoute.Ifindex) } r.livenessCallback() }) @@ -1066,7 +1064,7 @@ func (r *RouteTable) resyncIface(nl netlinkshim.Interface, ifaceName string) err } routeFilterFlags := netlink.RT_FILTER_OIF | netlink.RT_FILTER_TABLE - seenRoutes := set.New[kernelRouteKey]() + seenRoutes := set.New[RouteKey]() for range routeListFilterAttempts { // Using the Iter version here saves allocating a large slice of netlink.Route, // which we immediately discard. @@ -1081,11 +1079,11 @@ func (r *RouteTable) resyncIface(nl netlinkshim.Interface, ifaceName string) err return true } - kernKey, kernRoute := r.netlinkRouteToKernelRoute(&scratchRoute) - if oldRoute, ok := r.kernelRoutes.Dataplane().Get(kernKey); !ok || oldRoute.Equals(kernRoute) { - r.kernelRoutes.Dataplane().Set(kernKey, kernRoute) + routeKey, kernRoute := r.netlinkRouteToKernelRoute(&scratchRoute) + if oldRoute, ok := r.kernelRoutes.Dataplane().Get(routeKey); !ok || oldRoute.Equals(kernRoute) { + r.kernelRoutes.Dataplane().Set(routeKey, kernRoute) } - seenRoutes.Add(kernKey) + seenRoutes.Add(routeKey) return true }) if errors.Is(err, unix.EINTR) { @@ -1125,18 +1123,18 @@ func (r *RouteTable) resyncIface(nl netlinkshim.Interface, ifaceName string) err // Look for routes that the tracker says are there but are actually missing. for _, ifaceToRoutes := range r.ifaceToRoutes { for _, target := range ifaceToRoutes[ifaceName] { - kernKey := r.routeKeyForTarget(&target) - if seenRoutes.Contains(kernKey) { + routeKey := r.routeKeyForTarget(&target) + if seenRoutes.Contains(routeKey) { // Route still there; handled above. continue } - desKernRoute, ok := r.kernelRoutes.Desired().Get(kernKey) + desKernRoute, ok := r.kernelRoutes.Desired().Get(routeKey) if !ok || desKernRoute.Ifindex != ifIndex { // The interface we're syncing doesn't own this route // so the fact that it's missing is expected. continue } - r.kernelRoutes.Dataplane().Delete(kernKey) + r.kernelRoutes.Dataplane().Delete(routeKey) } } partialResyncTimeSummary.Observe(r.time.Since(startTime).Seconds()) @@ -1243,11 +1241,9 @@ func (r *RouteTable) refreshIfaceStateBestEffort(nl netlinkshim.Interface, iface return nil } -func (r *RouteTable) routeKeyForTarget(target *Target) kernelRouteKey { - key := kernelRouteKey{ - CIDR: target.CIDR, - Priority: target.Priority, - } +func (r *RouteTable) routeKeyForTarget(target *Target) RouteKey { + key := target.RouteKey + // If IPv6 and Priority is 0, set it to 1024. The kernel treats priority 0 as a sigil // meaning "use the default value", which is 1024 for IPv6. We need to set // an explicit priority so that routes round trip cleanly. @@ -1290,7 +1286,7 @@ func (r *RouteTable) routeIsOurs(route *netlink.Route) bool { return true } -func (r *RouteTable) netlinkRouteToKernelRoute(route *netlink.Route) (kernKey kernelRouteKey, kernRoute kernelRoute) { +func (r *RouteTable) netlinkRouteToKernelRoute(route *netlink.Route) (routeKey RouteKey, kernRoute kernelRoute) { // Defensive; recent versions of netlink always return a CIDR, but just // in case that gets regressed... cidr := ip.CIDRFromIPNet(route.Dst) @@ -1302,7 +1298,7 @@ func (r *RouteTable) netlinkRouteToKernelRoute(route *netlink.Route) (kernKey ke } } - kernKey = kernelRouteKey{ + routeKey = RouteKey{ CIDR: cidr, Priority: route.Priority, TOS: route.Tos, @@ -1336,7 +1332,7 @@ func (r *RouteTable) netlinkRouteToKernelRoute(route *netlink.Route) (kernKey ke if log.IsLevelEnabled(log.DebugLevel) { r.logCxt.WithFields(log.Fields{ "kernRoute": kernRoute, - "kernKey": kernKey, + "routeKey": routeKey, }).Debug("Loaded route from kernel.") } return @@ -1376,45 +1372,45 @@ 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 { + deletionErrs := map[RouteKey]error{} + r.kernelRoutes.PendingDeletions().Iter(func(routeKey RouteKey) deltatracker.IterAction { r.livenessCallback() - kernRoute, _ := r.kernelRoutes.PendingDeletions().Get(kernKey) + kernRoute, _ := r.kernelRoutes.PendingDeletions().Get(routeKey) if r.ifaceInGracePeriod(kernRoute.Ifindex) { // Don't remove unexpected routes from interfaces created recently. r.logCxt.WithFields(log.Fields{ "route": kernRoute, - "dest": kernKey, + "dest": routeKey, }).Debug("Found unexpected route; ignoring due to grace period.") return deltatracker.IterActionNoOp } - err := r.deleteRoute(nl, kernKey) + err := r.deleteRoute(nl, routeKey) if err != nil { - deletionErrs[kernKey] = err + deletionErrs[routeKey] = err return deltatracker.IterActionNoOp } - r.conntrackTracker.OnDataplaneRouteDeleted(kernKey.CIDR, kernRoute.Ifindex) + r.conntrackTracker.OnDataplaneRouteDeleted(routeKey.CIDR, kernRoute.Ifindex) // Route is gone, clean up the dataplane side of the tracker. - r.logCxt.WithField("route", kernKey).Debug("Deleted route.") + r.logCxt.WithField("route", routeKey).Debug("Deleted route.") return deltatracker.IterActionUpdateDataplane }) // 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.kernelRoutes.PendingUpdates().Iter(func(routeKey RouteKey, kernRoute kernelRoute) deltatracker.IterAction { r.livenessCallback() - cidr := kernKey.CIDR - dataplaneRoute, dataplaneExists := r.kernelRoutes.Dataplane().Get(kernKey) + cidr := routeKey.CIDR + dataplaneRoute, dataplaneExists := r.kernelRoutes.Dataplane().Get(routeKey) if dataplaneExists && r.conntrackTracker.CIDRNeedsEarlyCleanup(cidr, dataplaneRoute.Ifindex) { - err := r.deleteRoute(nl, kernKey) + err := r.deleteRoute(nl, routeKey) if err != nil { - deletionErrs[kernKey] = err + deletionErrs[routeKey] = err return deltatracker.IterActionNoOp } // This will queue the route for conntrack cleanup. - r.conntrackTracker.OnDataplaneRouteDeleted(kernKey.CIDR, dataplaneRoute.Ifindex) + r.conntrackTracker.OnDataplaneRouteDeleted(routeKey.CIDR, dataplaneRoute.Ifindex) } return deltatracker.IterActionNoOp }) @@ -1423,25 +1419,25 @@ func (r *RouteTable) applyUpdates(attempt int) error { // time. r.conntrackTracker.StartConntrackCleanupAndReset() - updateErrs := map[kernelRouteKey]error{} - r.kernelRoutes.PendingUpdates().Iter(func(kernKey kernelRouteKey, kRoute kernelRoute) deltatracker.IterAction { + updateErrs := map[RouteKey]error{} + r.kernelRoutes.PendingUpdates().Iter(func(routeKey RouteKey, kRoute kernelRoute) deltatracker.IterAction { r.livenessCallback() - dst := kernKey.CIDR.ToIPNet() + dst := routeKey.CIDR.ToIPNet() flags := 0 if kRoute.OnLink { flags = unix.RTNH_F_ONLINK } // In case we're moving a route, wait for the cleanup to finish. - r.conntrackTracker.WaitForPendingDeletion(kernKey.CIDR) + r.conntrackTracker.WaitForPendingDeletion(routeKey.CIDR) nlRoute := &netlink.Route{ Family: r.netlinkFamily, Table: r.tableIndex, Dst: &dst, - Tos: kernKey.TOS, - Priority: int(kernKey.Priority), + Tos: routeKey.TOS, + Priority: int(routeKey.Priority), Type: kRoute.Type, Scope: kRoute.Scope, @@ -1461,7 +1457,7 @@ func (r *RouteTable) applyUpdates(attempt int) error { } r.logCxt.WithFields(log.Fields{ "nlRoute": nlRoute, - "ourKey": kernKey, + "ourKey": routeKey, "ourRoute": kRoute, }).Debug("Replacing route") err := nl.RouteReplace(nlRoute) @@ -1487,7 +1483,7 @@ func (r *RouteTable) applyUpdates(attempt int) error { err = fmt.Errorf("%v(%s): %w", kRoute, name, err) } - updateErrs[kernKey] = err + updateErrs[routeKey] = err return deltatracker.IterActionNoOp } @@ -1610,19 +1606,19 @@ func (r *RouteTable) ifaceInGracePeriod(ifindex int) bool { return r.time.Since(graceInf.FirstSeen) < r.routeCleanupGracePeriod } -func (r *RouteTable) deleteRoute(nl netlinkshim.Interface, kernKey kernelRouteKey) error { +func (r *RouteTable) deleteRoute(nl netlinkshim.Interface, routeKey RouteKey) error { // Template route for deletion. The family, table, TOS, Priority uniquely // identify the route, but we also need to set some fields to their "wildcard" // values (found via code reading the kernel and running "ip route del" under // strace). - dst := kernKey.CIDR.ToIPNet() + dst := routeKey.CIDR.ToIPNet() nlRoute := &netlink.Route{ Family: r.netlinkFamily, Table: r.tableIndex, Dst: &dst, - Tos: kernKey.TOS, - Priority: kernKey.Priority, + Tos: routeKey.TOS, + Priority: routeKey.Priority, Protocol: unix.RTPROT_UNSPEC, // Wildcard (but also zero value). Scope: unix.RT_SCOPE_NOWHERE, // Wildcard. Note: non-zero value! @@ -1630,7 +1626,7 @@ func (r *RouteTable) deleteRoute(nl netlinkshim.Interface, kernKey kernelRouteKe } err := nl.RouteDel(nlRoute) if errors.Is(err, unix.ESRCH) { - r.logCxt.WithField("route", kernKey).Debug("Tried to delete route but it wasn't found.") + r.logCxt.WithField("route", routeKey).Debug("Tried to delete route but it wasn't found.") err = nil // Already gone (we hope). } return err @@ -1768,26 +1764,6 @@ func (r *RouteTable) checkTargets(ifaceName string, targets ...Target) { } } -// kernelRouteKey represents the kernel's FIB key. The kernel allows routes -// to coexist as long as they have different keys. -type kernelRouteKey struct { - // Destination CIDR; route matches traffic to this destination. - CIDR ip.CIDR - // TOS is the Type-of-Service field. For example, one app may mark its - // packets as "high importance" and that will take a different route to - // another app. - // - // Kernel uses the TOS=0 route if there isn't a more precise match. - TOS int - // Priority is the routing metric / distance. Given two routes with the - // same CIDR, the kernel prefers the route with the _lower_ priority. - Priority int -} - -func (k kernelRouteKey) String() string { - return fmt.Sprintf("%s(tos=%x metric=%d)", k.CIDR.String(), k.TOS, k.Priority) -} - // kernelRoute is our low-level representation of the parts of a route that // we care to program. It contains fields that we can easily read back from the // kernel for comparison. In particular, we track the interface index instead diff --git a/felix/routetable/route_table_test.go b/felix/routetable/route_table_test.go index 81d4fcde6a3..45d05a2d139 100644 --- a/felix/routetable/route_table_test.go +++ b/felix/routetable/route_table_test.go @@ -87,7 +87,9 @@ var _ = Describe("RouteTable v6", func() { It("should use interface index 1 for no-iface routes", func() { rt.RouteUpdate(RouteClassWireguard, InterfaceNone, Target{ - CIDR: ip.MustParseCIDROrIP("f00f::/128"), + RouteKey: RouteKey{ + CIDR: ip.MustParseCIDROrIP("f00f::/128"), + }, Type: TargetTypeThrow, }) err := rt.Apply() @@ -118,9 +120,13 @@ var _ = Describe("RouteTable v6", func() { Scope: netlink.SCOPE_LINK, Table: unix.RT_TABLE_MAIN, } - rt.SetRoutes(RouteClassLocalWorkload, noopLink.LinkAttrs.Name, []Target{ - {CIDR: ip.MustParseCIDROrIP("10.0.0.4/32"), DestMAC: mac1, Priority: routePriorityForTest}, - }) + rt.SetRoutes(RouteClassLocalWorkload, noopLink.LinkAttrs.Name, []Target{{ + RouteKey: RouteKey{ + CIDR: ip.MustParseCIDROrIP("10.0.0.4/32"), + Priority: routePriorityForTest, + }, + DestMAC: mac1, + }}) dataplane.AddMockRoute(&noopRoute) // Routes that should be deleted. @@ -300,9 +306,13 @@ var _ = Describe("RouteTable", func() { Table: unix.RT_TABLE_MAIN, } dataplane.AddMockRoute(&updateRoute) - rt.SetRoutes(RouteClassLocalWorkload, updateLink.LinkAttrs.Name, []Target{ - {CIDR: ip.MustParseCIDROrIP("10.0.0.5"), DestMAC: mac1, Priority: routePriorityForTest}, - }) + rt.SetRoutes(RouteClassLocalWorkload, updateLink.LinkAttrs.Name, []Target{{ + RouteKey: RouteKey{ + CIDR: ip.MustParseCIDROrIP("10.0.0.5"), + Priority: routePriorityForTest, + }, + DestMAC: mac1, + }}) fixedRoute := updateRoute fixedRoute.Src = nil @@ -348,9 +358,13 @@ var _ = Describe("RouteTable", func() { It("Should add routes with a source address", func() { // Route that needs to be added addLink := dataplane.AddIface(6, "cali6", true, true) - rt.SetRoutes(RouteClassLocalWorkload, addLink.LinkAttrs.Name, []Target{ - {CIDR: ip.MustParseCIDROrIP("10.0.0.6"), DestMAC: mac1, Priority: routePriorityForTest}, - }) + rt.SetRoutes(RouteClassLocalWorkload, addLink.LinkAttrs.Name, []Target{{ + RouteKey: RouteKey{ + CIDR: ip.MustParseCIDROrIP("10.0.0.6"), + Priority: routePriorityForTest, + }, + DestMAC: mac1, + }}) err := rt.Apply() Expect(err).ToNot(HaveOccurred()) cidr := mustParseCIDR("10.0.0.6/32") @@ -381,9 +395,13 @@ var _ = Describe("RouteTable", func() { // Initial route programming... addLink := dataplane.AddIface(6, "cali6", true, true) linkIndex = addLink.LinkAttrs.Index - rt.SetRoutes(RouteClassLocalWorkload, addLink.LinkAttrs.Name, []Target{ - {CIDR: ip.MustParseCIDROrIP("10.0.0.6"), DestMAC: mac1, Priority: routePriorityForTest}, - }) + rt.SetRoutes(RouteClassLocalWorkload, addLink.LinkAttrs.Name, []Target{{ + RouteKey: RouteKey{ + CIDR: ip.MustParseCIDROrIP("10.0.0.6"), + Priority: routePriorityForTest, + }, + DestMAC: mac1, + }}) err := rt.Apply() Expect(err).ToNot(HaveOccurred()) cidr = mustParseCIDR("10.0.0.6/32") @@ -423,9 +441,13 @@ var _ = Describe("RouteTable", func() { 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, Priority: routePriorityForTest}, - }) + rt.SetRoutes(RouteClassLocalWorkload, link.LinkAttrs.Name, []Target{{ + RouteKey: RouteKey{ + CIDR: ip.MustParseCIDROrIP("10.0.0.6"), + Priority: routePriorityForTest, + }, + DestMAC: mac1, + }}) rt.SetRoutes(RouteClassLocalWorkload, link.LinkAttrs.Name, nil) err := rt.Apply() @@ -437,10 +459,14 @@ var _ = Describe("RouteTable", 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, Priority: routePriorityForTest}, - }) - rt.RouteRemove(RouteClassLocalWorkload, link.LinkAttrs.Name, Target{CIDR: cidr, Priority: routePriorityForTest}) + rt.SetRoutes(RouteClassLocalWorkload, link.LinkAttrs.Name, []Target{{ + RouteKey: RouteKey{ + CIDR: cidr, + Priority: routePriorityForTest, + }, + DestMAC: mac1, + }}) + rt.RouteRemove(RouteClassLocalWorkload, link.LinkAttrs.Name, RouteKey{CIDR: cidr, Priority: routePriorityForTest}) err := rt.Apply() Expect(err).ToNot(HaveOccurred()) @@ -462,9 +488,13 @@ var _ = Describe("RouteTable", func() { Table: unix.RT_TABLE_MAIN, Priority: routePriorityForTest, } - rt.SetRoutes(RouteClassLocalWorkload, noopLink.LinkAttrs.Name, []Target{ - {CIDR: ip.MustParseCIDROrIP("10.0.0.4/32"), DestMAC: mac1, Priority: routePriorityForTest}, - }) + rt.SetRoutes(RouteClassLocalWorkload, noopLink.LinkAttrs.Name, []Target{{ + RouteKey: RouteKey{ + CIDR: ip.MustParseCIDROrIP("10.0.0.4/32"), + Priority: routePriorityForTest, + }, + DestMAC: mac1, + }}) dataplane.AddMockRoute(&noopRoute) err := rt.Apply() @@ -494,9 +524,13 @@ var _ = Describe("RouteTable", func() { Table: unix.RT_TABLE_MAIN, Priority: routePriorityForTest, } - rt.SetRoutes(RouteClassLocalWorkload, updateLink.LinkAttrs.Name, []Target{ - {CIDR: ip.MustParseCIDROrIP("10.0.0.5"), DestMAC: mac1, Priority: routePriorityForTest}, - }) + rt.SetRoutes(RouteClassLocalWorkload, updateLink.LinkAttrs.Name, []Target{{ + RouteKey: RouteKey{ + CIDR: ip.MustParseCIDROrIP("10.0.0.5"), + Priority: routePriorityForTest, + }, + DestMAC: mac1, + }}) dataplane.AddMockRoute(&updateRoute) fixedRoute := updateRoute @@ -532,9 +566,13 @@ var _ = Describe("RouteTable", func() { Table: unix.RT_TABLE_MAIN, Priority: routePriorityForTest, } - rt.SetRoutes(RouteClassLocalWorkload, updateLink.LinkAttrs.Name, []Target{ - {CIDR: ip.MustParseCIDROrIP("10.0.0.5"), DestMAC: mac1, Priority: routePriorityForTest}, - }) + rt.SetRoutes(RouteClassLocalWorkload, updateLink.LinkAttrs.Name, []Target{{ + RouteKey: RouteKey{ + CIDR: ip.MustParseCIDROrIP("10.0.0.5"), + Priority: routePriorityForTest, + }, + DestMAC: mac1, + }}) dataplane.AddMockRoute(&updateRoute) fixedRoute := updateRoute @@ -567,9 +605,14 @@ var _ = Describe("RouteTable", func() { Table: unix.RT_TABLE_MAIN, Priority: routePriorityForTest, } - rt.SetRoutes(RouteClassLocalWorkload, noopLink.LinkAttrs.Name, []Target{ - {CIDR: ip.MustParseCIDROrIP("10.0.0.5"), DestMAC: mac1, Src: ip.FromString("192.168.0.2"), Priority: routePriorityForTest}, - }) + rt.SetRoutes(RouteClassLocalWorkload, noopLink.LinkAttrs.Name, []Target{{ + RouteKey: RouteKey{ + CIDR: ip.MustParseCIDROrIP("10.0.0.5"), + Priority: routePriorityForTest, + }, + DestMAC: mac1, + Src: ip.FromString("192.168.0.2"), + }}) dataplane.AddMockRoute(&noopRoute) err := rt.Apply() Expect(err).ToNot(HaveOccurred()) @@ -589,9 +632,14 @@ var _ = Describe("RouteTable", func() { Table: unix.RT_TABLE_MAIN, Priority: routePriorityForTest, } - rt.SetRoutes(RouteClassLocalWorkload, noopLink.LinkAttrs.Name, []Target{ - {CIDR: ip.MustParseCIDROrIP("10.0.0.5"), DestMAC: mac1, Src: ip.FromString("192.168.0.3"), Priority: routePriorityForTest}, - }) + rt.SetRoutes(RouteClassLocalWorkload, noopLink.LinkAttrs.Name, []Target{{ + RouteKey: RouteKey{ + CIDR: ip.MustParseCIDROrIP("10.0.0.5"), + Priority: routePriorityForTest, + }, + DestMAC: mac1, + Src: ip.FromString("192.168.0.3"), + }}) dataplane.AddMockRoute(&noopRoute) err := rt.Apply() Expect(err).ToNot(HaveOccurred()) @@ -633,9 +681,13 @@ var _ = Describe("RouteTable", func() { It("Should add routes with a protocol", func() { // Route that needs to be added addLink := dataplane.AddIface(6, "cali6", true, true) - rt.SetRoutes(RouteClassLocalWorkload, addLink.LinkAttrs.Name, []Target{ - {CIDR: ip.MustParseCIDROrIP("10.0.0.6"), DestMAC: mac1, Priority: routePriorityForTest}, - }) + rt.SetRoutes(RouteClassLocalWorkload, addLink.LinkAttrs.Name, []Target{{ + RouteKey: RouteKey{ + CIDR: ip.MustParseCIDROrIP("10.0.0.6"), + Priority: routePriorityForTest, + }, + DestMAC: mac1, + }}) err := rt.Apply() Expect(err).ToNot(HaveOccurred()) Expect(dataplane.RouteKeyToRoute["254-10.0.0.6/32"]).To(Equal(netlink.Route{ @@ -652,10 +704,12 @@ var _ = Describe("RouteTable", func() { By("Reading back the route") Expect(rt.ReadRoutesFromKernel(addLink.LinkAttrs.Name)).To(ConsistOf( Target{ - Type: TargetTypeLinkLocalUnicast, - CIDR: ip.MustParseCIDROrIP("10.0.0.6"), + Type: TargetTypeLinkLocalUnicast, + RouteKey: RouteKey{ + CIDR: ip.MustParseCIDROrIP("10.0.0.6"), + Priority: routePriorityForTest, + }, Protocol: deviceRouteProtocol, - Priority: routePriorityForTest, }), ) }) @@ -663,23 +717,23 @@ var _ = Describe("RouteTable", func() { // Route that needs to be added addLink := dataplane.AddIface(6, "cali6", true, true) addLink2 := dataplane.AddIface(7, "cali7", true, true) - rt.SetRoutes(RouteClassLocalWorkload, InterfaceNone, []Target{ - { - Type: TargetTypeVXLAN, - CIDR: ip.MustParseCIDROrIP("10.0.0.0/24"), - MultiPath: []NextHop{ - { - IfaceName: addLink.LinkAttrs.Name, - Gw: ip.FromString("10.0.0.6"), - }, - { - IfaceName: addLink2.LinkAttrs.Name, - Gw: ip.FromString("10.0.0.7"), - }, - }, + rt.SetRoutes(RouteClassLocalWorkload, InterfaceNone, []Target{{ + Type: TargetTypeVXLAN, + RouteKey: RouteKey{ + CIDR: ip.MustParseCIDROrIP("10.0.0.0/24"), Priority: routePriorityForTest, }, - }) + MultiPath: []NextHop{ + { + IfaceName: addLink.LinkAttrs.Name, + Gw: ip.FromString("10.0.0.6"), + }, + { + IfaceName: addLink2.LinkAttrs.Name, + Gw: ip.FromString("10.0.0.7"), + }, + }, + }}) err := rt.Apply() Expect(err).ToNot(HaveOccurred()) Expect(dataplane.RouteKeyToRoute["254-10.0.0.0/24"]).To(Equal(netlink.Route{ @@ -709,8 +763,11 @@ var _ = Describe("RouteTable", func() { By("Reading back the route") Expect(rt.ReadRoutesFromKernel(InterfaceNone)).To(ConsistOf( Target{ - Type: TargetTypeVXLAN, - CIDR: ip.MustParseCIDROrIP("10.0.0.0/24"), + Type: TargetTypeVXLAN, + RouteKey: RouteKey{ + CIDR: ip.MustParseCIDROrIP("10.0.0.0/24"), + Priority: routePriorityForTest, + }, Protocol: deviceRouteProtocol, MultiPath: []NextHop{ { @@ -722,7 +779,6 @@ var _ = Describe("RouteTable", func() { Gw: ip.FromString("10.0.0.7"), }, }, - Priority: routePriorityForTest, })) }) It("Should add/remove multi-path routes when interface goes up/down", func() { @@ -732,23 +788,23 @@ var _ = Describe("RouteTable", func() { addLink2 := dataplane.AddIface(7, "cali7", false, false) By("Setting routes") - rt.SetRoutes(RouteClassLocalWorkload, InterfaceNone, []Target{ - { - Type: TargetTypeVXLAN, - CIDR: ip.MustParseCIDROrIP("10.0.0.0/24"), - MultiPath: []NextHop{ - { - IfaceName: addLink.LinkAttrs.Name, - Gw: ip.FromString("10.0.0.6"), - }, - { - IfaceName: addLink2.LinkAttrs.Name, - Gw: ip.FromString("10.0.0.7"), - }, - }, + rt.SetRoutes(RouteClassLocalWorkload, InterfaceNone, []Target{{ + Type: TargetTypeVXLAN, + RouteKey: RouteKey{ + CIDR: ip.MustParseCIDROrIP("10.0.0.0/24"), Priority: routePriorityForTest, }, - }) + MultiPath: []NextHop{ + { + IfaceName: addLink.LinkAttrs.Name, + Gw: ip.FromString("10.0.0.6"), + }, + { + IfaceName: addLink2.LinkAttrs.Name, + Gw: ip.FromString("10.0.0.7"), + }, + }, + }}) By("Apply") err := rt.Apply() @@ -815,23 +871,23 @@ var _ = Describe("RouteTable", func() { }) It("Should add/remove multi-path routes when interface creted/deleted", func() { By("Setting routes") - rt.SetRoutes(RouteClassLocalWorkload, InterfaceNone, []Target{ - { - Type: TargetTypeVXLAN, - CIDR: ip.MustParseCIDROrIP("10.0.0.0/24"), - MultiPath: []NextHop{ - { - IfaceName: "cali6", - Gw: ip.FromString("10.0.0.6"), - }, - { - IfaceName: "cali7", - Gw: ip.FromString("10.0.0.7"), - }, - }, + rt.SetRoutes(RouteClassLocalWorkload, InterfaceNone, []Target{{ + Type: TargetTypeVXLAN, + RouteKey: RouteKey{ + CIDR: ip.MustParseCIDROrIP("10.0.0.0/24"), Priority: routePriorityForTest, }, - }) + MultiPath: []NextHop{ + { + IfaceName: "cali6", + Gw: ip.FromString("10.0.0.6"), + }, + { + IfaceName: "cali7", + Gw: ip.FromString("10.0.0.7"), + }, + }, + }}) By("Apply") err := rt.Apply() @@ -896,10 +952,19 @@ var _ = Describe("RouteTable", func() { It("Should add multiple routes with a protocol", func() { // Route that needs to be added addLink := dataplane.AddIface(6, "cali6", true, true) - rt.SetRoutes(RouteClassLocalWorkload, addLink.LinkAttrs.Name, []Target{ - {CIDR: ip.MustParseCIDROrIP("10.0.0.6"), DestMAC: mac1, Priority: routePriorityForTest}, - {CIDR: ip.MustParseCIDROrIP("10.0.0.7"), DestMAC: mac1, Priority: routePriorityForTest}, - }) + rt.SetRoutes(RouteClassLocalWorkload, addLink.LinkAttrs.Name, []Target{{ + RouteKey: RouteKey{ + CIDR: ip.MustParseCIDROrIP("10.0.0.6"), + Priority: routePriorityForTest, + }, + DestMAC: mac1, + }, { + RouteKey: RouteKey{ + CIDR: ip.MustParseCIDROrIP("10.0.0.7"), + Priority: routePriorityForTest, + }, + DestMAC: mac1, + }}) err := rt.Apply() Expect(err).ToNot(HaveOccurred()) Expect(dataplane.RouteKeyToRoute["254-10.0.0.6/32"]).To(Equal(netlink.Route{ @@ -926,10 +991,19 @@ var _ = Describe("RouteTable", func() { It("Should add multiple routes with a protocol after persistent failures", func() { // Route that needs to be added addLink := dataplane.AddIface(6, "cali6", true, true) - rt.SetRoutes(RouteClassLocalWorkload, addLink.LinkAttrs.Name, []Target{ - {CIDR: ip.MustParseCIDROrIP("10.0.0.6"), DestMAC: mac1, Priority: routePriorityForTest}, - {CIDR: ip.MustParseCIDROrIP("10.0.0.7"), DestMAC: mac1, Priority: routePriorityForTest}, - }) + rt.SetRoutes(RouteClassLocalWorkload, addLink.LinkAttrs.Name, []Target{{ + RouteKey: RouteKey{ + CIDR: ip.MustParseCIDROrIP("10.0.0.6"), + Priority: routePriorityForTest, + }, + DestMAC: mac1, + }, { + RouteKey: RouteKey{ + CIDR: ip.MustParseCIDROrIP("10.0.0.7"), + Priority: routePriorityForTest, + }, + DestMAC: mac1, + }}) // Persist failures, this will apply the deltas to the cache but will be out of sync with the dataplane. dataplane.FailuresToSimulate = mocknetlink.FailNextRouteAdd | mocknetlink.FailNextRouteReplace dataplane.PersistFailures = true @@ -975,9 +1049,13 @@ var _ = Describe("RouteTable", func() { Table: unix.RT_TABLE_MAIN, Priority: routePriorityForTest, } - rt.SetRoutes(RouteClassLocalWorkload, noopLink.LinkAttrs.Name, []Target{ - {CIDR: ip.MustParseCIDROrIP("10.0.0.4/32"), DestMAC: mac1, Priority: routePriorityForTest}, - }) + rt.SetRoutes(RouteClassLocalWorkload, noopLink.LinkAttrs.Name, []Target{{ + RouteKey: RouteKey{ + CIDR: ip.MustParseCIDROrIP("10.0.0.4/32"), + Priority: routePriorityForTest, + }, + DestMAC: mac1, + }}) dataplane.AddMockRoute(&noopRoute) err := rt.Apply() @@ -997,9 +1075,13 @@ var _ = Describe("RouteTable", func() { Table: unix.RT_TABLE_MAIN, Priority: routePriorityForTest, } - rt.SetRoutes(RouteClassLocalWorkload, updateLink.LinkAttrs.Name, []Target{ - {CIDR: ip.MustParseCIDROrIP("10.0.0.5"), DestMAC: mac1, Priority: routePriorityForTest}, - }) + rt.SetRoutes(RouteClassLocalWorkload, updateLink.LinkAttrs.Name, []Target{{ + RouteKey: RouteKey{ + CIDR: ip.MustParseCIDROrIP("10.0.0.5"), + Priority: routePriorityForTest, + }, + DestMAC: mac1, + }}) dataplane.AddMockRoute(&updateRoute) fixedRoute := updateRoute @@ -1024,9 +1106,13 @@ var _ = Describe("RouteTable", func() { Table: unix.RT_TABLE_MAIN, Priority: routePriorityForTest, } - rt.SetRoutes(RouteClassLocalWorkload, updateLink.LinkAttrs.Name, []Target{ - {CIDR: ip.MustParseCIDROrIP("10.0.0.5"), DestMAC: mac1, Priority: routePriorityForTest}, - }) + rt.SetRoutes(RouteClassLocalWorkload, updateLink.LinkAttrs.Name, []Target{{ + RouteKey: RouteKey{ + CIDR: ip.MustParseCIDROrIP("10.0.0.5"), + Priority: routePriorityForTest, + }, + DestMAC: mac1, + }}) dataplane.AddMockRoute(&updateRoute) fixedRoute := updateRoute @@ -1050,9 +1136,13 @@ var _ = Describe("RouteTable", func() { err := rt.Apply() Expect(err).ToNot(HaveOccurred()) // We try to add 10.0.0.1 back in. - rt.SetRoutes(RouteClassLocalWorkload, "cali1", []Target{ - {CIDR: ip.MustParseCIDROrIP("10.0.0.1/32"), DestMAC: mac1, Priority: routePriorityForTest}, - }) + rt.SetRoutes(RouteClassLocalWorkload, "cali1", []Target{{ + RouteKey: RouteKey{ + CIDR: ip.MustParseCIDROrIP("10.0.0.1/32"), + Priority: routePriorityForTest, + }, + DestMAC: mac1, + }}) start := time.Now() err = rt.Apply() Expect(err).ToNot(HaveOccurred()) @@ -1064,9 +1154,13 @@ var _ = Describe("RouteTable", func() { err := rt.Apply() Expect(err).ToNot(HaveOccurred()) // We try to add 10.0.0.10, which hasn't been seen before. - rt.SetRoutes(RouteClassLocalWorkload, "cali1", []Target{ - {CIDR: ip.MustParseCIDROrIP("10.0.0.10/32"), DestMAC: mac1, Priority: routePriorityForTest}, - }) + rt.SetRoutes(RouteClassLocalWorkload, "cali1", []Target{{ + RouteKey: RouteKey{ + CIDR: ip.MustParseCIDROrIP("10.0.0.10/32"), + Priority: routePriorityForTest, + }, + DestMAC: mac1, + }}) start := time.Now() err = rt.Apply() Expect(err).ToNot(HaveOccurred()) @@ -1093,8 +1187,10 @@ var _ = Describe("RouteTable", func() { dataplane.FailuresToSimulate = mocknetlink.FailNextRouteAdd | mocknetlink.FailNextRouteReplace dataplane.PersistFailures = true rt.RouteUpdate(RouteClassLocalWorkload, "cali3", Target{ - CIDR: ip.MustParseCIDROrIP("10.20.30.40"), - Priority: routePriorityForTest, + RouteKey: RouteKey{ + CIDR: ip.MustParseCIDROrIP("10.20.30.40"), + Priority: routePriorityForTest, + }, }) err = rt.Apply() Expect(err).To(HaveOccurred()) @@ -1137,14 +1233,18 @@ var _ = Describe("RouteTable", func() { Describe("after adding two routes to cali3", func() { JustBeforeEach(func() { rt.RouteUpdate(RouteClassLocalWorkload, "cali3", Target{ - CIDR: ip.MustParseCIDROrIP("10.20.30.40"), - Priority: routePriorityForTest, + RouteKey: RouteKey{ + CIDR: ip.MustParseCIDROrIP("10.20.30.40"), + Priority: routePriorityForTest, + }, }) err := rt.Apply() Expect(err).ToNot(HaveOccurred()) rt.RouteUpdate(RouteClassLocalWorkload, "cali3", Target{ - CIDR: ip.MustParseCIDROrIP("10.0.20.0/24"), - Priority: routePriorityForTest, + RouteKey: RouteKey{ + CIDR: ip.MustParseCIDROrIP("10.0.20.0/24"), + Priority: routePriorityForTest, + }, }) err = rt.Apply() Expect(err).ToNot(HaveOccurred()) @@ -1174,15 +1274,22 @@ var _ = Describe("RouteTable", func() { }) It("should make no dataplane updates when deleting, creating and updating back to the same target before the next apply", func() { - rt.RouteRemove(RouteClassLocalWorkload, "cali3", Target{CIDR: ip.MustParseCIDROrIP("10.0.20.0/24"), Priority: routePriorityForTest}) - rt.RouteUpdate(RouteClassLocalWorkload, "cali3", Target{ + rt.RouteRemove(RouteClassLocalWorkload, "cali3", RouteKey{ CIDR: ip.MustParseCIDROrIP("10.0.20.0/24"), - GW: ip.FromString("1.2.3.4"), Priority: routePriorityForTest, }) rt.RouteUpdate(RouteClassLocalWorkload, "cali3", Target{ - CIDR: ip.MustParseCIDROrIP("10.0.20.0/24"), - Priority: routePriorityForTest, + RouteKey: RouteKey{ + CIDR: ip.MustParseCIDROrIP("10.0.20.0/24"), + Priority: routePriorityForTest, + }, + GW: ip.FromString("1.2.3.4"), + }) + rt.RouteUpdate(RouteClassLocalWorkload, "cali3", Target{ + RouteKey: RouteKey{ + CIDR: ip.MustParseCIDROrIP("10.0.20.0/24"), + Priority: routePriorityForTest, + }, }) dataplane.ResetDeltas() @@ -1215,13 +1322,20 @@ var _ = Describe("RouteTable", func() { }) It("should make no dataplane updates when deleting and then setting back to the same target before the next apply", func() { - rt.RouteRemove(RouteClassLocalWorkload, "cali3", Target{CIDR: ip.MustParseCIDROrIP("10.0.20.0/24"), Priority: routePriorityForTest}) - rt.SetRoutes(RouteClassLocalWorkload, "cali3", []Target{{ + rt.RouteRemove(RouteClassLocalWorkload, "cali3", RouteKey{ CIDR: ip.MustParseCIDROrIP("10.0.20.0/24"), Priority: routePriorityForTest, + }) + rt.SetRoutes(RouteClassLocalWorkload, "cali3", []Target{{ + RouteKey: RouteKey{ + CIDR: ip.MustParseCIDROrIP("10.0.20.0/24"), + Priority: routePriorityForTest, + }, }, { - CIDR: ip.MustParseCIDROrIP("10.20.30.40"), - Priority: routePriorityForTest, + RouteKey: RouteKey{ + CIDR: ip.MustParseCIDROrIP("10.20.30.40"), + Priority: routePriorityForTest, + }, }}) dataplane.ResetDeltas() @@ -1257,12 +1371,18 @@ var _ = Describe("RouteTable", func() { Describe("delete interface", func() { BeforeEach(func() { - rt.SetRoutes(RouteClassLocalWorkload, "cali1", []Target{ - {CIDR: ip.MustParseCIDROrIP("10.0.0.1/32"), Priority: routePriorityForTest}, - }) - rt.SetRoutes(RouteClassLocalWorkload, "cali3", []Target{ - {CIDR: ip.MustParseCIDROrIP("10.0.0.3/32"), Priority: routePriorityForTest}, - }) + rt.SetRoutes(RouteClassLocalWorkload, "cali1", []Target{{ + RouteKey: RouteKey{ + CIDR: ip.MustParseCIDROrIP("10.0.0.1/32"), + Priority: routePriorityForTest, + }, + }}) + rt.SetRoutes(RouteClassLocalWorkload, "cali3", []Target{{ + RouteKey: RouteKey{ + CIDR: ip.MustParseCIDROrIP("10.0.0.3/32"), + Priority: routePriorityForTest, + }, + }}) // Apply the changes. err := rt.Apply() Expect(err).NotTo(HaveOccurred()) @@ -1291,15 +1411,26 @@ var _ = Describe("RouteTable", func() { desc := fmt.Sprintf("with some routes added and failures: %v", testFailFlags) Describe(desc, func() { BeforeEach(func() { - rt.SetRoutes(RouteClassLocalWorkload, "cali1", []Target{ - {CIDR: ip.MustParseCIDROrIP("10.0.0.1/32"), DestMAC: mac1, Priority: routePriorityForTest}, - }) - rt.SetRoutes(RouteClassLocalWorkload, "cali2", []Target{ - {CIDR: ip.MustParseCIDROrIP("10.0.0.2/32"), DestMAC: mac2, Priority: routePriorityForTest}, - }) - rt.SetRoutes(RouteClassLocalWorkload, "cali3", []Target{ - {CIDR: ip.MustParseCIDROrIP("10.0.1.3/32"), Priority: routePriorityForTest}, - }) + rt.SetRoutes(RouteClassLocalWorkload, "cali1", []Target{{ + RouteKey: RouteKey{ + CIDR: ip.MustParseCIDROrIP("10.0.0.1/32"), + Priority: routePriorityForTest, + }, + DestMAC: mac1, + }}) + rt.SetRoutes(RouteClassLocalWorkload, "cali2", []Target{{ + RouteKey: RouteKey{ + CIDR: ip.MustParseCIDROrIP("10.0.0.2/32"), + Priority: routePriorityForTest, + }, + DestMAC: mac2, + }}) + rt.SetRoutes(RouteClassLocalWorkload, "cali3", []Target{{ + RouteKey: RouteKey{ + CIDR: ip.MustParseCIDROrIP("10.0.1.3/32"), + Priority: routePriorityForTest, + }, + }}) dataplane.FailuresToSimulate = testFailFlags }) JustBeforeEach(func() { @@ -1529,8 +1660,10 @@ var _ = Describe("RouteTable", func() { // Trigger a per-interface sync. rt.OnIfaceStateChanged("cali1", 2, ifacemonitor.StateUp) rt.RouteUpdate(RouteClassLocalWorkload, "cali1", Target{ - CIDR: ip.MustParseCIDROrIP("10.0.20.0/24"), - Priority: routePriorityForTest, + RouteKey: RouteKey{ + CIDR: ip.MustParseCIDROrIP("10.0.20.0/24"), + Priority: routePriorityForTest, + }, }) err := rt.Apply() Expect(err).NotTo(HaveOccurred()) @@ -1773,9 +1906,11 @@ var _ = Describe("RouteTable (table 100)", func() { Describe("after configuring a throw route", func() { JustBeforeEach(func() { rt.RouteUpdate(RouteClassWireguard, InterfaceNone, Target{ - CIDR: ip.MustParseCIDROrIP("10.10.10.10/32"), - Type: TargetTypeThrow, - Priority: routePriorityForTest, + RouteKey: RouteKey{ + CIDR: ip.MustParseCIDROrIP("10.10.10.10/32"), + Priority: routePriorityForTest, + }, + Type: TargetTypeThrow, }) err := rt.Apply() Expect(err).ToNot(HaveOccurred()) @@ -1791,9 +1926,11 @@ var _ = Describe("RouteTable (table 100)", func() { Describe("after configuring a throw route", func() { JustBeforeEach(func() { rt.RouteUpdate(RouteClassWireguard, InterfaceNone, Target{ - CIDR: ip.MustParseCIDROrIP("10.10.10.10/32"), - Type: TargetTypeThrow, - Priority: routePriorityForTest, + RouteKey: RouteKey{ + CIDR: ip.MustParseCIDROrIP("10.10.10.10/32"), + Priority: routePriorityForTest, + }, + Type: TargetTypeThrow, }) err := rt.Apply() Expect(err).ToNot(HaveOccurred()) @@ -1802,21 +1939,31 @@ var _ = Describe("RouteTable (table 100)", func() { It("should be able to toggle between throw and local iface routes", func() { // Modify the action associated with a particular destination. for range 3 { - rt.RouteRemove(RouteClassWireguard, InterfaceNone, Target{CIDR: ip.MustParseCIDROrIP("10.10.10.10/32"), Priority: routePriorityForTest}) - rt.RouteUpdate(RouteClassLocalWorkload, "cali", Target{ + rt.RouteRemove(RouteClassWireguard, InterfaceNone, RouteKey{ CIDR: ip.MustParseCIDROrIP("10.10.10.10/32"), Priority: routePriorityForTest, }) + rt.RouteUpdate(RouteClassLocalWorkload, "cali", Target{ + RouteKey: RouteKey{ + CIDR: ip.MustParseCIDROrIP("10.10.10.10/32"), + Priority: routePriorityForTest, + }, + }) err := rt.Apply() Expect(err).ToNot(HaveOccurred()) Expect(dataplane.RouteKeyToRoute).To(ConsistOf(caliRoute, gatewayRoute, caliRouteTable100SameAsThrow)) - rt.RouteRemove(RouteClassLocalWorkload, "cali", Target{CIDR: ip.MustParseCIDROrIP("10.10.10.10/32"), Priority: routePriorityForTest}) - rt.RouteUpdate(RouteClassWireguard, InterfaceNone, Target{ + rt.RouteRemove(RouteClassLocalWorkload, "cali", RouteKey{ CIDR: ip.MustParseCIDROrIP("10.10.10.10/32"), - Type: TargetTypeThrow, Priority: routePriorityForTest, }) + rt.RouteUpdate(RouteClassWireguard, InterfaceNone, Target{ + RouteKey: RouteKey{ + CIDR: ip.MustParseCIDROrIP("10.10.10.10/32"), + Priority: routePriorityForTest, + }, + Type: TargetTypeThrow, + }) err = rt.Apply() Expect(err).ToNot(HaveOccurred()) Expect(dataplane.RouteKeyToRoute).To(ConsistOf(caliRoute, gatewayRoute, throwRoute)) @@ -1825,14 +1972,19 @@ var _ = Describe("RouteTable (table 100)", func() { It("should prioritise a workload route over the throw route", func() { rt.RouteUpdate(RouteClassLocalWorkload, "cali", Target{ - CIDR: ip.MustParseCIDROrIP("10.10.10.10/32"), - Priority: routePriorityForTest, + RouteKey: RouteKey{ + CIDR: ip.MustParseCIDROrIP("10.10.10.10/32"), + Priority: routePriorityForTest, + }, }) err := rt.Apply() Expect(err).ToNot(HaveOccurred()) Expect(dataplane.RouteKeyToRoute).To(ConsistOf(caliRoute, gatewayRoute, caliRouteTable100SameAsThrow)) - rt.RouteRemove(RouteClassLocalWorkload, "cali", Target{CIDR: ip.MustParseCIDROrIP("10.10.10.10/32"), Priority: routePriorityForTest}) + rt.RouteRemove(RouteClassLocalWorkload, "cali", RouteKey{ + CIDR: ip.MustParseCIDROrIP("10.10.10.10/32"), + Priority: routePriorityForTest, + }) err = rt.Apply() Expect(err).ToNot(HaveOccurred()) Expect(dataplane.RouteKeyToRoute).To(ConsistOf(caliRoute, gatewayRoute, throwRoute)) @@ -1842,8 +1994,10 @@ var _ = Describe("RouteTable (table 100)", func() { Describe("throw route configured in dataplane, actual route is via cali", func() { It("the throw route should be removed and the interface route added", func() { rt.RouteUpdate(RouteClassLocalWorkload, "cali", Target{ - CIDR: ip.MustParseCIDROrIP("10.10.10.10/32"), - Priority: routePriorityForTest, + RouteKey: RouteKey{ + CIDR: ip.MustParseCIDROrIP("10.10.10.10/32"), + Priority: routePriorityForTest, + }, }) err := rt.Apply() Expect(err).ToNot(HaveOccurred()) @@ -1854,13 +2008,18 @@ var _ = Describe("RouteTable (table 100)", func() { Describe("after configuring an existing throw route and then deleting it", func() { JustBeforeEach(func() { rt.RouteUpdate(RouteClassWireguard, InterfaceNone, Target{ - CIDR: ip.MustParseCIDROrIP("10.10.10.10/32"), - Type: TargetTypeThrow, - Priority: routePriorityForTest, + RouteKey: RouteKey{ + CIDR: ip.MustParseCIDROrIP("10.10.10.10/32"), + Priority: routePriorityForTest, + }, + Type: TargetTypeThrow, }) err := rt.Apply() Expect(err).ToNot(HaveOccurred()) - rt.RouteRemove(RouteClassWireguard, InterfaceNone, Target{CIDR: ip.MustParseCIDROrIP("10.10.10.10/32"), Priority: routePriorityForTest}) + rt.RouteRemove(RouteClassWireguard, InterfaceNone, RouteKey{ + CIDR: ip.MustParseCIDROrIP("10.10.10.10/32"), + Priority: routePriorityForTest, + }) err = rt.Apply() Expect(err).ToNot(HaveOccurred()) }) @@ -1875,16 +2034,20 @@ var _ = Describe("RouteTable (table 100)", func() { Describe("after configuring a throw route and then replacing it with a blackhole route", func() { JustBeforeEach(func() { rt.RouteUpdate(RouteClassIPAMBlockDrop, InterfaceNone, Target{ - CIDR: ip.MustParseCIDROrIP("10.10.10.10/32"), - Type: TargetTypeThrow, - Priority: routePriorityForTest, + RouteKey: RouteKey{ + CIDR: ip.MustParseCIDROrIP("10.10.10.10/32"), + Priority: routePriorityForTest, + }, + Type: TargetTypeThrow, }) err := rt.Apply() Expect(err).ToNot(HaveOccurred()) rt.RouteUpdate(RouteClassIPAMBlockDrop, InterfaceNone, Target{ - CIDR: ip.MustParseCIDROrIP("10.10.10.10/32"), - Type: TargetTypeBlackhole, - Priority: routePriorityForTest, + RouteKey: RouteKey{ + CIDR: ip.MustParseCIDROrIP("10.10.10.10/32"), + Priority: routePriorityForTest, + }, + Type: TargetTypeBlackhole, }) err = rt.Apply() Expect(err).ToNot(HaveOccurred()) @@ -1908,16 +2071,20 @@ var _ = Describe("RouteTable (table 100)", func() { Describe("after configuring a blackhole route and then replacing it with a prohibit route", func() { JustBeforeEach(func() { rt.RouteUpdate(RouteClassIPAMBlockDrop, InterfaceNone, Target{ - CIDR: ip.MustParseCIDROrIP("10.10.10.10/32"), - Type: TargetTypeBlackhole, - Priority: routePriorityForTest, + RouteKey: RouteKey{ + CIDR: ip.MustParseCIDROrIP("10.10.10.10/32"), + Priority: routePriorityForTest, + }, + Type: TargetTypeBlackhole, }) err := rt.Apply() Expect(err).ToNot(HaveOccurred()) rt.RouteUpdate(RouteClassIPAMBlockDrop, InterfaceNone, Target{ - CIDR: ip.MustParseCIDROrIP("10.10.10.10/32"), - Type: TargetTypeProhibit, - Priority: routePriorityForTest, + RouteKey: RouteKey{ + CIDR: ip.MustParseCIDROrIP("10.10.10.10/32"), + Priority: routePriorityForTest, + }, + Type: TargetTypeProhibit, }) err = rt.Apply() Expect(err).ToNot(HaveOccurred()) @@ -1959,8 +2126,10 @@ var _ = Describe("RouteTable (table 100)", func() { // In "strict" mode, RouteListFiltered returns an error if the routing table doesn't exist. // Check that is handled and that we proceed to create the route (and thus create the routing table). rt.RouteUpdate(RouteClassLocalWorkload, "cali", Target{ - CIDR: ip.MustParseCIDROrIP("10.0.0.1/32"), - Priority: routePriorityForTest, + RouteKey: RouteKey{ + CIDR: ip.MustParseCIDROrIP("10.0.0.1/32"), + Priority: routePriorityForTest, + }, }) err := rt.Apply() Expect(err).ToNot(HaveOccurred()) diff --git a/felix/wireguard/wireguard.go b/felix/wireguard/wireguard.go index bceaca9cf9d..3963872d62f 100644 --- a/felix/wireguard/wireguard.go +++ b/felix/wireguard/wireguard.go @@ -1096,8 +1096,8 @@ func (w *Wireguard) updateRouteTableFromNodeUpdates() { // not programmed. for cidr := range update.cidrsDeleted.All() { w.logCtx.WithField("cidr", cidr).Debug("Removing CIDR from routetable interface") - w.routetable.RouteRemove(w.interfaceName, routetable.Target{CIDR: cidr}) - w.routetable.RouteRemove(routetable.InterfaceNone, routetable.Target{CIDR: cidr}) + w.routetable.RouteRemove(w.interfaceName, routetable.RouteKey{CIDR: cidr}) + w.routetable.RouteRemove(routetable.InterfaceNone, routetable.RouteKey{CIDR: cidr}) } } @@ -1147,12 +1147,12 @@ func (w *Wireguard) updateRouteTableFromNodeUpdates() { // information to decide which route we need to remove - however we have also had bugs related to state // tracking so deleting both is reasonable - routetable ignores the one that is not programmed. updateLogCtx.Debug("Wireguard routing has changed - delete previous route") - w.routetable.RouteRemove(routetable.InterfaceNone, routetable.Target{CIDR: cidr}) - w.routetable.RouteRemove(w.interfaceName, routetable.Target{CIDR: cidr}) + w.routetable.RouteRemove(routetable.InterfaceNone, routetable.RouteKey{CIDR: cidr}) + w.routetable.RouteRemove(w.interfaceName, routetable.RouteKey{CIDR: cidr}) } w.routetable.RouteUpdate(ifaceName, routetable.Target{ - Type: targetType, - CIDR: cidr, + RouteKey: routetable.RouteKey{CIDR: cidr}, + Type: targetType, }) } node.routingToWireguard = shouldRouteToWireguard From 2a048ad150b55f2a21c7880e3d65e808c7febc2d Mon Sep 17 00:00:00 2001 From: Nell Jerram Date: Fri, 27 Feb 2026 15:50:22 +0000 Subject: [PATCH 4/8] Fix IPv6 RouteRemove not normalizing Priority 0 to 1024 RouteUpdate and SetRoutes normalize IPv6 Priority 0 to 1024 (since the kernel treats 0 as "use default" which is 1024 for IPv6), but RouteRemove did not. This meant RouteRemove with Priority 0 would fail to find and remove IPv6 routes that were stored with the normalized key Priority 1024. Extract normalizeRouteKey from routeKeyForTarget and call it in RouteRemove as well. Co-Authored-By: Claude Opus 4.6 --- felix/routetable/route_table.go | 17 ++++++++--------- 1 file changed, 8 insertions(+), 9 deletions(-) diff --git a/felix/routetable/route_table.go b/felix/routetable/route_table.go index 62460324721..5c577053694 100644 --- a/felix/routetable/route_table.go +++ b/felix/routetable/route_table.go @@ -466,7 +466,7 @@ func (r *RouteTable) SetRoutes(routeClass RouteClass, ifaceName string, targets oldTargetsToCleanUp := r.ifaceToRoutes[routeClass][ifaceName] newTargets := map[RouteKey]Target{} for _, t := range targets { - routeKey := r.routeKeyForTarget(&t) + routeKey := r.normalizeRouteKey(t.RouteKey) delete(oldTargetsToCleanUp, routeKey) newTargets[routeKey] = t } @@ -514,7 +514,7 @@ func (r *RouteTable) RouteUpdate(routeClass RouteClass, ifaceName string, target routesByCIDR = map[RouteKey]Target{} r.ifaceToRoutes[routeClass][ifaceName] = routesByCIDR } - routeKey := r.routeKeyForTarget(&target) + routeKey := r.normalizeRouteKey(target.RouteKey) routesByCIDR[routeKey] = target r.addOwningIface(routeClass, ifaceName, routeKey) r.updatePermanentARP(ifaceName, routeKey.CIDR.Addr(), target.DestMAC) @@ -529,6 +529,7 @@ func (r *RouteTable) RouteRemove(routeClass RouteClass, ifaceName string, routeK return } + routeKey = r.normalizeRouteKey(routeKey) _, exists := r.ifaceToRoutes[routeClass][ifaceName][routeKey] if !exists { return @@ -1123,7 +1124,7 @@ func (r *RouteTable) resyncIface(nl netlinkshim.Interface, ifaceName string) err // Look for routes that the tracker says are there but are actually missing. for _, ifaceToRoutes := range r.ifaceToRoutes { for _, target := range ifaceToRoutes[ifaceName] { - routeKey := r.routeKeyForTarget(&target) + routeKey := r.normalizeRouteKey(target.RouteKey) if seenRoutes.Contains(routeKey) { // Route still there; handled above. continue @@ -1241,12 +1242,10 @@ func (r *RouteTable) refreshIfaceStateBestEffort(nl netlinkshim.Interface, iface return nil } -func (r *RouteTable) routeKeyForTarget(target *Target) RouteKey { - key := target.RouteKey - - // If IPv6 and Priority is 0, set it to 1024. The kernel treats priority 0 as a sigil - // meaning "use the default value", which is 1024 for IPv6. We need to set - // an explicit priority so that routes round trip cleanly. +// normalizeRouteKey normalizes a RouteKey for the current IP version. For IPv6, +// the kernel treats priority 0 as a sigil meaning "use the default value", +// which is 1024. We normalize here so that routes round trip cleanly. +func (r *RouteTable) normalizeRouteKey(key RouteKey) RouteKey { if r.ipVersion == 6 && key.Priority == 0 { key.Priority = 1024 } From 18164b3ce28bd9826d3d22e6e434338ff3430166 Mon Sep 17 00:00:00 2001 From: Nell Jerram Date: Fri, 27 Feb 2026 15:50:42 +0000 Subject: [PATCH 5/8] Add unit tests for routes with different priorities sharing the same CIDR Include Priority in mock netlink KeyForRoute so routes with the same CIDR but different priorities get distinct keys in the mock dataplane. Update all existing route key assertions accordingly. Add 11 new tests covering multi-priority route scenarios: adding and removing routes at different priorities, resync behavior, and stale route cleanup. Add a test for IPv6 RouteRemove Priority normalization. Co-Authored-By: Claude Opus 4.6 --- felix/netlinkshim/mocknetlink/netlink.go | 2 +- felix/routetable/route_table_test.go | 416 +++++++++++++++++++++-- felix/wireguard/wireguard_test.go | 56 +-- 3 files changed, 417 insertions(+), 57 deletions(-) diff --git a/felix/netlinkshim/mocknetlink/netlink.go b/felix/netlinkshim/mocknetlink/netlink.go index c672bfc60a6..46d8b412346 100644 --- a/felix/netlinkshim/mocknetlink/netlink.go +++ b/felix/netlinkshim/mocknetlink/netlink.go @@ -1163,7 +1163,7 @@ func KeyForRoute(route *netlink.Route) string { if table == 0 { table = unix.RT_TABLE_MAIN } - key := fmt.Sprintf("%v-%v", table, route.Dst) + key := fmt.Sprintf("%v-%v-%v", table, route.Dst, route.Priority) log.WithField("routeKey", key).Debug("Calculated route key") return key } diff --git a/felix/routetable/route_table_test.go b/felix/routetable/route_table_test.go index 45d05a2d139..23362e4674a 100644 --- a/felix/routetable/route_table_test.go +++ b/felix/routetable/route_table_test.go @@ -37,6 +37,16 @@ import ( const routePriorityForTest int = 48931 +// routeKeyStr builds a mock dataplane route key string that includes table, CIDR, and priority. +func routeKeyStr(table int, cidr string, priority int) string { + return fmt.Sprintf("%d-%s-%d", table, cidr, priority) +} + +// mainRouteKey builds a route key string for the main routing table with the standard test priority. +func mainRouteKey(cidr string) string { + return routeKeyStr(254, cidr, routePriorityForTest) +} + var ( FelixRouteProtocol = netlink.RouteProtocol(syscall.RTPROT_BOOT) @@ -94,7 +104,7 @@ var _ = Describe("RouteTable v6", func() { }) err := rt.Apply() Expect(err).ToNot(HaveOccurred()) - Expect(dataplane.RouteKeyToRoute["254-f00f::/128"]).To(Equal( + Expect(dataplane.RouteKeyToRoute[routeKeyStr(254, "f00f::/128", 1024)]).To(Equal( netlink.Route{ Family: netlink.FAMILY_V6, LinkIndex: 1, @@ -158,6 +168,32 @@ var _ = Describe("RouteTable v6", func() { Expect(dataplane.DeletedRouteKeys).To(HaveKey(mocknetlink.KeyForRoute(&deleteRoute))) Expect(dataplane.DeletedRouteKeys).To(HaveKey(mocknetlink.KeyForRoute(&deleteRoute2))) }) + + It("should normalize IPv6 Priority 0 to 1024 in RouteRemove", func() { + // Add a route with Priority 0 — internally, routeKeyForTarget normalizes this to 1024 for IPv6. + link := dataplane.AddIface(10, "cali10", true, true) + rt.OnIfaceStateChanged(link.LinkAttrs.Name, link.LinkAttrs.Index, ifacemonitor.StateUp) + + rt.RouteUpdate(RouteClassLocalWorkload, link.LinkAttrs.Name, Target{ + RouteKey: RouteKey{ + CIDR: ip.MustParseCIDROrIP("fd00::1/128"), + // Priority 0 — will be normalized to 1024. + }, + Type: TargetTypeVXLAN, + }) + err := rt.Apply() + Expect(err).ToNot(HaveOccurred()) + Expect(dataplane.RouteKeyToRoute).To(HaveKey(routeKeyStr(254, "fd00::1/128", 1024))) + + // Now remove using Priority 0 again — RouteRemove should normalize it to 1024. + rt.RouteRemove(RouteClassLocalWorkload, link.LinkAttrs.Name, RouteKey{ + CIDR: ip.MustParseCIDROrIP("fd00::1/128"), + // Priority 0 — should be normalized to 1024 to match the stored route. + }) + err = rt.Apply() + Expect(err).ToNot(HaveOccurred()) + Expect(dataplane.RouteKeyToRoute).NotTo(HaveKey(routeKeyStr(254, "fd00::1/128", 1024))) + }) }) var _ = Describe("RouteTable", func() { @@ -304,6 +340,7 @@ var _ = Describe("RouteTable", func() { Scope: netlink.SCOPE_LINK, Src: net.ParseIP("192.168.0.1"), Table: unix.RT_TABLE_MAIN, + Priority: routePriorityForTest, } dataplane.AddMockRoute(&updateRoute) rt.SetRoutes(RouteClassLocalWorkload, updateLink.LinkAttrs.Name, []Target{{ @@ -316,7 +353,6 @@ var _ = Describe("RouteTable", func() { fixedRoute := updateRoute fixedRoute.Src = nil - fixedRoute.Priority = routePriorityForTest err := rt.Apply() Expect(err).ToNot(HaveOccurred()) @@ -368,7 +404,7 @@ var _ = Describe("RouteTable", func() { err := rt.Apply() Expect(err).ToNot(HaveOccurred()) cidr := mustParseCIDR("10.0.0.6/32") - Expect(dataplane.RouteKeyToRoute["254-10.0.0.6/32"]).To(Equal(netlink.Route{ + Expect(dataplane.RouteKeyToRoute[mainRouteKey("10.0.0.6/32")]).To(Equal(netlink.Route{ Family: unix.AF_INET, LinkIndex: addLink.LinkAttrs.Index, Dst: cidr, @@ -452,7 +488,7 @@ var _ = Describe("RouteTable", func() { err := rt.Apply() Expect(err).ToNot(HaveOccurred()) - Expect(dataplane.RouteKeyToRoute).NotTo(HaveKey("254-10.0.0.6/32")) + Expect(dataplane.RouteKeyToRoute).NotTo(HaveKey(mainRouteKey("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() { @@ -470,7 +506,7 @@ var _ = Describe("RouteTable", func() { err := rt.Apply() Expect(err).ToNot(HaveOccurred()) - Expect(dataplane.RouteKeyToRoute).NotTo(HaveKey("254-10.0.0.6/32")) + Expect(dataplane.RouteKeyToRoute).NotTo(HaveKey(mainRouteKey("10.0.0.6/32"))) dataplane.ExpectNeighs(unix.AF_INET) }) It("Should not remove routes with a source address", func() { @@ -690,7 +726,7 @@ var _ = Describe("RouteTable", func() { }}) err := rt.Apply() Expect(err).ToNot(HaveOccurred()) - Expect(dataplane.RouteKeyToRoute["254-10.0.0.6/32"]).To(Equal(netlink.Route{ + Expect(dataplane.RouteKeyToRoute[mainRouteKey("10.0.0.6/32")]).To(Equal(netlink.Route{ Family: unix.AF_INET, LinkIndex: addLink.LinkAttrs.Index, Dst: mustParseCIDR("10.0.0.6/32"), @@ -736,7 +772,7 @@ var _ = Describe("RouteTable", func() { }}) err := rt.Apply() Expect(err).ToNot(HaveOccurred()) - Expect(dataplane.RouteKeyToRoute["254-10.0.0.0/24"]).To(Equal(netlink.Route{ + Expect(dataplane.RouteKeyToRoute[mainRouteKey("10.0.0.0/24")]).To(Equal(netlink.Route{ Family: unix.AF_INET, LinkIndex: 0, Dst: mustParseCIDR("10.0.0.0/24"), @@ -809,7 +845,7 @@ var _ = Describe("RouteTable", func() { By("Apply") err := rt.Apply() Expect(err).ToNot(HaveOccurred()) - Expect(dataplane.RouteKeyToRoute["254-10.0.0.0/24"]).To(BeZero()) + Expect(dataplane.RouteKeyToRoute[mainRouteKey("10.0.0.0/24")]).To(BeZero()) By("Bringing interfaces up") dataplane.SetIface("cali6", true, true) @@ -843,7 +879,7 @@ var _ = Describe("RouteTable", func() { }, Priority: routePriorityForTest, } - Expect(dataplane.RouteKeyToRoute["254-10.0.0.0/24"]).To(Equal(expectedRoute)) + Expect(dataplane.RouteKeyToRoute[mainRouteKey("10.0.0.0/24")]).To(Equal(expectedRoute)) By("Bringing one interface down") dataplane.SetIface("cali6", false, false) @@ -854,7 +890,7 @@ var _ = Describe("RouteTable", func() { Expect(err).ToNot(HaveOccurred()) // It's ok to have one interface down on a multi-path route. // Kernel keeps the route in place. - Expect(dataplane.RouteKeyToRoute["254-10.0.0.0/24"]).To(Equal(expectedRoute), + Expect(dataplane.RouteKeyToRoute[mainRouteKey("10.0.0.0/24")]).To(Equal(expectedRoute), "Route should not be removed when only one interface is down") By("Bringing other interface down") @@ -866,7 +902,7 @@ var _ = Describe("RouteTable", func() { Expect(err).ToNot(HaveOccurred()) // The kernel will remove the route once all interfaces go down // so we do the same to stay in sync. - Expect(dataplane.RouteKeyToRoute["254-10.0.0.0/24"]).To(BeZero(), + Expect(dataplane.RouteKeyToRoute[mainRouteKey("10.0.0.0/24")]).To(BeZero(), "Route should be removed when all interfaces are down") }) It("Should add/remove multi-path routes when interface creted/deleted", func() { @@ -892,7 +928,7 @@ var _ = Describe("RouteTable", func() { By("Apply") err := rt.Apply() Expect(err).ToNot(HaveOccurred()) - Expect(dataplane.RouteKeyToRoute["254-10.0.0.0/24"]).To(BeZero()) + Expect(dataplane.RouteKeyToRoute[mainRouteKey("10.0.0.0/24")]).To(BeZero()) By("Creating one interface") addLink := dataplane.AddIface(6, "cali6", false, false) @@ -902,7 +938,7 @@ var _ = Describe("RouteTable", func() { By("Apply") err = rt.Apply() Expect(err).ToNot(HaveOccurred()) - Expect(dataplane.RouteKeyToRoute["254-10.0.0.0/24"]).To(BeZero()) + Expect(dataplane.RouteKeyToRoute[mainRouteKey("10.0.0.0/24")]).To(BeZero()) By("Creating other interface") addLink2 := dataplane.AddIface(7, "cali7", false, false) @@ -935,7 +971,7 @@ var _ = Describe("RouteTable", func() { }, Priority: routePriorityForTest, } - Expect(dataplane.RouteKeyToRoute["254-10.0.0.0/24"]).To(Equal(expectedRoute)) + Expect(dataplane.RouteKeyToRoute[mainRouteKey("10.0.0.0/24")]).To(Equal(expectedRoute)) By("Deleting one interface") dataplane.DelIface("cali6") @@ -946,7 +982,7 @@ var _ = Describe("RouteTable", func() { Expect(err).ToNot(HaveOccurred()) // Can't have a route with a deleted interface. The ifindex becomes // invalid. - Expect(dataplane.RouteKeyToRoute["254-10.0.0.0/24"]).To(BeZero(), + Expect(dataplane.RouteKeyToRoute[mainRouteKey("10.0.0.0/24")]).To(BeZero(), "Route should be removed when one interface deleted") }) It("Should add multiple routes with a protocol", func() { @@ -967,7 +1003,7 @@ var _ = Describe("RouteTable", func() { }}) err := rt.Apply() Expect(err).ToNot(HaveOccurred()) - Expect(dataplane.RouteKeyToRoute["254-10.0.0.6/32"]).To(Equal(netlink.Route{ + Expect(dataplane.RouteKeyToRoute[mainRouteKey("10.0.0.6/32")]).To(Equal(netlink.Route{ Family: unix.AF_INET, LinkIndex: addLink.LinkAttrs.Index, Dst: mustParseCIDR("10.0.0.6/32"), @@ -977,7 +1013,7 @@ var _ = Describe("RouteTable", func() { Table: unix.RT_TABLE_MAIN, Priority: routePriorityForTest, })) - Expect(dataplane.RouteKeyToRoute["254-10.0.0.7/32"]).To(Equal(netlink.Route{ + Expect(dataplane.RouteKeyToRoute[mainRouteKey("10.0.0.7/32")]).To(Equal(netlink.Route{ Family: unix.AF_INET, LinkIndex: addLink.LinkAttrs.Index, Dst: mustParseCIDR("10.0.0.7/32"), @@ -1015,7 +1051,7 @@ var _ = Describe("RouteTable", func() { dataplane.PersistFailures = false err = rt.Apply() Expect(err).NotTo(HaveOccurred()) - Expect(dataplane.RouteKeyToRoute["254-10.0.0.6/32"]).To(Equal(netlink.Route{ + Expect(dataplane.RouteKeyToRoute[mainRouteKey("10.0.0.6/32")]).To(Equal(netlink.Route{ Family: unix.AF_INET, LinkIndex: addLink.LinkAttrs.Index, Dst: mustParseCIDR("10.0.0.6/32"), @@ -1025,7 +1061,7 @@ var _ = Describe("RouteTable", func() { Table: unix.RT_TABLE_MAIN, Priority: routePriorityForTest, })) - Expect(dataplane.RouteKeyToRoute["254-10.0.0.7/32"]).To(Equal(netlink.Route{ + Expect(dataplane.RouteKeyToRoute[mainRouteKey("10.0.0.7/32")]).To(Equal(netlink.Route{ Family: unix.AF_INET, LinkIndex: addLink.LinkAttrs.Index, Dst: mustParseCIDR("10.0.0.7/32"), @@ -1471,7 +1507,7 @@ var _ = Describe("RouteTable", func() { // resolution determines that no routes are eligible for programming. if testFailFlags != mocknetlink.FailNextLinkByNameNotFound { It("should keep correct route", func() { - Expect(dataplane.RouteKeyToRoute["254-10.0.0.1/32"]).To(Equal(netlink.Route{ + Expect(dataplane.RouteKeyToRoute[mainRouteKey("10.0.0.1/32")]).To(Equal(netlink.Route{ Family: unix.AF_INET, LinkIndex: cali1.LinkAttrs.Index, Dst: &ip1, @@ -1481,12 +1517,12 @@ var _ = Describe("RouteTable", func() { Table: unix.RT_TABLE_MAIN, Priority: routePriorityForTest, })) - Expect(dataplane.AddedRouteKeys.Contains("254-10.0.0.1/32")).To(BeFalse()) + Expect(dataplane.AddedRouteKeys.Contains(mainRouteKey("10.0.0.1/32"))).To(BeFalse()) }) } It("should add new route", func() { - Expect(dataplane.RouteKeyToRoute).To(HaveKey("254-10.0.0.2/32")) - Expect(dataplane.RouteKeyToRoute["254-10.0.0.2/32"]).To(Equal(netlink.Route{ + Expect(dataplane.RouteKeyToRoute).To(HaveKey(mainRouteKey("10.0.0.2/32"))) + Expect(dataplane.RouteKeyToRoute[mainRouteKey("10.0.0.2/32")]).To(Equal(netlink.Route{ Family: unix.AF_INET, LinkIndex: cali2.LinkAttrs.Index, Dst: &ip2, @@ -1498,8 +1534,8 @@ var _ = Describe("RouteTable", func() { })) }) It("should update changed route", func() { - Expect(dataplane.RouteKeyToRoute).To(HaveKey("254-10.0.1.3/32")) - Expect(dataplane.RouteKeyToRoute["254-10.0.1.3/32"]).To(Equal(netlink.Route{ + Expect(dataplane.RouteKeyToRoute).To(HaveKey(mainRouteKey("10.0.1.3/32"))) + Expect(dataplane.RouteKeyToRoute[mainRouteKey("10.0.1.3/32")]).To(Equal(netlink.Route{ Family: unix.AF_INET, LinkIndex: cali3.LinkAttrs.Index, Dst: &ip13, @@ -1509,7 +1545,7 @@ var _ = Describe("RouteTable", func() { Table: unix.RT_TABLE_MAIN, Priority: routePriorityForTest, })) - Expect(dataplane.DeletedRouteKeys.Contains("254-10.0.0.3/32")).To(BeTrue()) + Expect(dataplane.DeletedRouteKeys.Contains(mainRouteKey("10.0.0.3/32"))).To(BeTrue()) Eventually(dataplane.GetDeletedConntrackEntries).Should(ContainElement(net.ParseIP("10.0.0.3").To4())) }) It("should have expected number of routes at the end", func() { @@ -1674,6 +1710,330 @@ var _ = Describe("RouteTable", func() { }) }) +var _ = Describe("RouteTable with multiple priorities", func() { + var dataplane *mocknetlink.MockNetlinkDataplane + var t *mocktime.MockTime + var rt *RouteTable + + const ( + lowPriority = 100 + highPriority = 200 + ) + + BeforeEach(func() { + dataplane = mocknetlink.New() + t = mocktime.New() + t.SetAutoIncrement(11 * time.Second) + rt = New( + &defaultOwnershipPolicy, + 4, + 10*time.Second, + nil, + FelixRouteProtocol, + true, + 0, + logutils.NewSummarizer("test"), + dataplane, + WithRouteCleanupGracePeriod(10*time.Second), + WithTimeShim(t), + WithConntrackShim(dataplane), + WithNetlinkHandleShim(dataplane.NewMockNetlink), + ) + }) + + Describe("with an interface", func() { + var cali1 *mocknetlink.MockLink + BeforeEach(func() { + cali1 = dataplane.AddIface(3, "cali1", true, true) + }) + + It("should program two routes with the same CIDR but different priorities", func() { + rt.SetRoutes(RouteClassLocalWorkload, "cali1", []Target{ + { + RouteKey: RouteKey{ + CIDR: ip.MustParseCIDROrIP("10.0.0.1/32"), + Priority: lowPriority, + }, + }, + { + RouteKey: RouteKey{ + CIDR: ip.MustParseCIDROrIP("10.0.0.1/32"), + Priority: highPriority, + }, + }, + }) + err := rt.Apply() + Expect(err).ToNot(HaveOccurred()) + + lowKey := routeKeyStr(254, "10.0.0.1/32", lowPriority) + highKey := routeKeyStr(254, "10.0.0.1/32", highPriority) + + Expect(dataplane.RouteKeyToRoute).To(HaveKey(lowKey)) + Expect(dataplane.RouteKeyToRoute).To(HaveKey(highKey)) + Expect(dataplane.RouteKeyToRoute[lowKey]).To(Equal(netlink.Route{ + Family: unix.AF_INET, + LinkIndex: cali1.LinkAttrs.Index, + Dst: mustParseCIDR("10.0.0.1/32"), + Type: syscall.RTN_UNICAST, + Protocol: FelixRouteProtocol, + Scope: netlink.SCOPE_LINK, + Table: unix.RT_TABLE_MAIN, + Priority: lowPriority, + })) + Expect(dataplane.RouteKeyToRoute[highKey]).To(Equal(netlink.Route{ + Family: unix.AF_INET, + LinkIndex: cali1.LinkAttrs.Index, + Dst: mustParseCIDR("10.0.0.1/32"), + Type: syscall.RTN_UNICAST, + Protocol: FelixRouteProtocol, + Scope: netlink.SCOPE_LINK, + Table: unix.RT_TABLE_MAIN, + Priority: highPriority, + })) + }) + + It("should add two routes with different priorities via RouteUpdate", func() { + rt.RouteUpdate(RouteClassLocalWorkload, "cali1", Target{ + RouteKey: RouteKey{ + CIDR: ip.MustParseCIDROrIP("10.0.0.1/32"), + Priority: lowPriority, + }, + }) + rt.RouteUpdate(RouteClassLocalWorkload, "cali1", Target{ + RouteKey: RouteKey{ + CIDR: ip.MustParseCIDROrIP("10.0.0.1/32"), + Priority: highPriority, + }, + }) + err := rt.Apply() + Expect(err).ToNot(HaveOccurred()) + + Expect(dataplane.RouteKeyToRoute).To(HaveKey(routeKeyStr(254, "10.0.0.1/32", lowPriority))) + Expect(dataplane.RouteKeyToRoute).To(HaveKey(routeKeyStr(254, "10.0.0.1/32", highPriority))) + }) + + Context("after programming two routes with different priorities", func() { + BeforeEach(func() { + rt.SetRoutes(RouteClassLocalWorkload, "cali1", []Target{ + { + RouteKey: RouteKey{ + CIDR: ip.MustParseCIDROrIP("10.0.0.1/32"), + Priority: lowPriority, + }, + }, + { + RouteKey: RouteKey{ + CIDR: ip.MustParseCIDROrIP("10.0.0.1/32"), + Priority: highPriority, + }, + }, + }) + err := rt.Apply() + Expect(err).ToNot(HaveOccurred()) + }) + + It("should remove only the low-priority route when it is removed", func() { + rt.RouteRemove(RouteClassLocalWorkload, "cali1", RouteKey{ + CIDR: ip.MustParseCIDROrIP("10.0.0.1/32"), + Priority: lowPriority, + }) + dataplane.ResetDeltas() + err := rt.Apply() + Expect(err).ToNot(HaveOccurred()) + + lowKey := routeKeyStr(254, "10.0.0.1/32", lowPriority) + highKey := routeKeyStr(254, "10.0.0.1/32", highPriority) + + Expect(dataplane.RouteKeyToRoute).NotTo(HaveKey(lowKey)) + Expect(dataplane.RouteKeyToRoute).To(HaveKey(highKey)) + Expect(dataplane.DeletedRouteKeys).To(HaveKey(lowKey)) + }) + + It("should remove only the high-priority route when it is removed", func() { + rt.RouteRemove(RouteClassLocalWorkload, "cali1", RouteKey{ + CIDR: ip.MustParseCIDROrIP("10.0.0.1/32"), + Priority: highPriority, + }) + dataplane.ResetDeltas() + err := rt.Apply() + Expect(err).ToNot(HaveOccurred()) + + lowKey := routeKeyStr(254, "10.0.0.1/32", lowPriority) + highKey := routeKeyStr(254, "10.0.0.1/32", highPriority) + + Expect(dataplane.RouteKeyToRoute).To(HaveKey(lowKey)) + Expect(dataplane.RouteKeyToRoute).NotTo(HaveKey(highKey)) + Expect(dataplane.DeletedRouteKeys).To(HaveKey(highKey)) + }) + + It("should remove both routes when SetRoutes is called with nil", func() { + rt.SetRoutes(RouteClassLocalWorkload, "cali1", nil) + dataplane.ResetDeltas() + err := rt.Apply() + Expect(err).ToNot(HaveOccurred()) + + lowKey := routeKeyStr(254, "10.0.0.1/32", lowPriority) + highKey := routeKeyStr(254, "10.0.0.1/32", highPriority) + + Expect(dataplane.RouteKeyToRoute).NotTo(HaveKey(lowKey)) + Expect(dataplane.RouteKeyToRoute).NotTo(HaveKey(highKey)) + Expect(dataplane.DeletedRouteKeys).To(HaveKey(lowKey)) + Expect(dataplane.DeletedRouteKeys).To(HaveKey(highKey)) + }) + + It("should replace both routes when SetRoutes is called with new targets at same priorities", func() { + rt.SetRoutes(RouteClassLocalWorkload, "cali1", []Target{ + { + RouteKey: RouteKey{ + CIDR: ip.MustParseCIDROrIP("10.0.0.1/32"), + Priority: lowPriority, + }, + GW: ip.FromString("10.0.0.254"), + }, + { + RouteKey: RouteKey{ + CIDR: ip.MustParseCIDROrIP("10.0.0.1/32"), + Priority: highPriority, + }, + GW: ip.FromString("10.0.0.253"), + }, + }) + dataplane.ResetDeltas() + err := rt.Apply() + Expect(err).ToNot(HaveOccurred()) + + lowKey := routeKeyStr(254, "10.0.0.1/32", lowPriority) + highKey := routeKeyStr(254, "10.0.0.1/32", highPriority) + + Expect(dataplane.RouteKeyToRoute[lowKey].Gw).To(Equal(net.ParseIP("10.0.0.254").To4())) + Expect(dataplane.RouteKeyToRoute[highKey].Gw).To(Equal(net.ParseIP("10.0.0.253").To4())) + }) + + It("should handle removing one priority and keeping the other via SetRoutes", func() { + rt.SetRoutes(RouteClassLocalWorkload, "cali1", []Target{ + { + RouteKey: RouteKey{ + CIDR: ip.MustParseCIDROrIP("10.0.0.1/32"), + Priority: highPriority, + }, + }, + }) + dataplane.ResetDeltas() + err := rt.Apply() + Expect(err).ToNot(HaveOccurred()) + + lowKey := routeKeyStr(254, "10.0.0.1/32", lowPriority) + highKey := routeKeyStr(254, "10.0.0.1/32", highPriority) + + Expect(dataplane.RouteKeyToRoute).NotTo(HaveKey(lowKey)) + Expect(dataplane.RouteKeyToRoute).To(HaveKey(highKey)) + Expect(dataplane.DeletedRouteKeys).To(HaveKey(lowKey)) + }) + + It("should survive a resync and keep both routes", func() { + rt.QueueResync() + dataplane.ResetDeltas() + err := rt.Apply() + Expect(err).ToNot(HaveOccurred()) + + lowKey := routeKeyStr(254, "10.0.0.1/32", lowPriority) + highKey := routeKeyStr(254, "10.0.0.1/32", highPriority) + + Expect(dataplane.RouteKeyToRoute).To(HaveKey(lowKey)) + Expect(dataplane.RouteKeyToRoute).To(HaveKey(highKey)) + // No changes should be needed. + Expect(dataplane.AddedRouteKeys).To(BeEmpty()) + Expect(dataplane.DeletedRouteKeys).To(BeEmpty()) + }) + + It("should add a third priority for the same CIDR", func() { + const thirdPriority = 300 + rt.RouteUpdate(RouteClassLocalWorkload, "cali1", Target{ + RouteKey: RouteKey{ + CIDR: ip.MustParseCIDROrIP("10.0.0.1/32"), + Priority: thirdPriority, + }, + }) + err := rt.Apply() + Expect(err).ToNot(HaveOccurred()) + + Expect(dataplane.RouteKeyToRoute).To(HaveKey(routeKeyStr(254, "10.0.0.1/32", lowPriority))) + Expect(dataplane.RouteKeyToRoute).To(HaveKey(routeKeyStr(254, "10.0.0.1/32", highPriority))) + Expect(dataplane.RouteKeyToRoute).To(HaveKey(routeKeyStr(254, "10.0.0.1/32", thirdPriority))) + }) + }) + + It("should clean up stale kernel routes at different priorities during resync", func() { + // Pre-populate the kernel with two routes at different priorities + // that we don't know about. + staleRouteLow := netlink.Route{ + Family: unix.AF_INET, + LinkIndex: cali1.LinkAttrs.Index, + Dst: mustParseCIDR("10.0.0.1/32"), + Type: syscall.RTN_UNICAST, + Protocol: FelixRouteProtocol, + Scope: netlink.SCOPE_LINK, + Table: unix.RT_TABLE_MAIN, + Priority: lowPriority, + } + staleRouteHigh := netlink.Route{ + Family: unix.AF_INET, + LinkIndex: cali1.LinkAttrs.Index, + Dst: mustParseCIDR("10.0.0.1/32"), + Type: syscall.RTN_UNICAST, + Protocol: FelixRouteProtocol, + Scope: netlink.SCOPE_LINK, + Table: unix.RT_TABLE_MAIN, + Priority: highPriority, + } + dataplane.AddMockRoute(&staleRouteLow) + dataplane.AddMockRoute(&staleRouteHigh) + + err := rt.Apply() + Expect(err).ToNot(HaveOccurred()) + + // Both stale routes should be deleted. + lowKey := routeKeyStr(254, "10.0.0.1/32", lowPriority) + highKey := routeKeyStr(254, "10.0.0.1/32", highPriority) + Expect(dataplane.RouteKeyToRoute).NotTo(HaveKey(lowKey)) + Expect(dataplane.RouteKeyToRoute).NotTo(HaveKey(highKey)) + }) + + It("should clean up stale priority and keep desired priority during resync", func() { + // Pre-populate the kernel with a stale route at one priority. + staleRoute := netlink.Route{ + Family: unix.AF_INET, + LinkIndex: cali1.LinkAttrs.Index, + Dst: mustParseCIDR("10.0.0.1/32"), + Type: syscall.RTN_UNICAST, + Protocol: FelixRouteProtocol, + Scope: netlink.SCOPE_LINK, + Table: unix.RT_TABLE_MAIN, + Priority: lowPriority, + } + dataplane.AddMockRoute(&staleRoute) + + // Tell the routetable we want a route at a different priority. + rt.SetRoutes(RouteClassLocalWorkload, "cali1", []Target{{ + RouteKey: RouteKey{ + CIDR: ip.MustParseCIDROrIP("10.0.0.1/32"), + Priority: highPriority, + }, + }}) + + err := rt.Apply() + Expect(err).ToNot(HaveOccurred()) + + lowKey := routeKeyStr(254, "10.0.0.1/32", lowPriority) + highKey := routeKeyStr(254, "10.0.0.1/32", highPriority) + + // Stale route should be cleaned up, desired route should be programmed. + Expect(dataplane.RouteKeyToRoute).NotTo(HaveKey(lowKey)) + Expect(dataplane.RouteKeyToRoute).To(HaveKey(highKey)) + }) + }) +}) + var _ = Describe("RouteTable (main table)", func() { var dataplane *mocknetlink.MockNetlinkDataplane var t *mocktime.MockTime @@ -2064,7 +2424,7 @@ var _ = Describe("RouteTable (table 100)", func() { Table: 100, Priority: routePriorityForTest, })) - Expect(dataplane.UpdatedRouteKeys.Contains("100-10.10.10.10/32")).To(BeTrue()) + Expect(dataplane.UpdatedRouteKeys.Contains(routeKeyStr(100, "10.10.10.10/32", routePriorityForTest))).To(BeTrue()) }) }) @@ -2101,7 +2461,7 @@ var _ = Describe("RouteTable (table 100)", func() { Table: 100, Priority: routePriorityForTest, })) - Expect(dataplane.UpdatedRouteKeys.Contains("100-10.10.10.10/32")).To(BeTrue()) + Expect(dataplane.UpdatedRouteKeys.Contains(routeKeyStr(100, "10.10.10.10/32", routePriorityForTest))).To(BeTrue()) }) }) }) diff --git a/felix/wireguard/wireguard_test.go b/felix/wireguard/wireguard_test.go index 47c69c40bee..9ed9bf5ce4e 100644 --- a/felix/wireguard/wireguard_test.go +++ b/felix/wireguard/wireguard_test.go @@ -79,13 +79,13 @@ var ( ipnet_4 = cidr_4.ToIPNet() // ipnet_5 = cidr_5.ToIPNet() // ipnet_6 = cidr_6.ToIPNet() - routekey_cidr_local = fmt.Sprintf("%d-%s", tableIndex, cidr_local) - // routekey_1 = fmt.Sprintf("%d-%s", tableIndex, cidr_1) - // routekey_2 = fmt.Sprintf("%d-%s", tableIndex, cidr_2) - // routekey_3 = fmt.Sprintf("%d-%s", tableIndex, cidr_3) - routekey_4 = fmt.Sprintf("%d-%s", tableIndex, cidr_4) - // routekey_5 = fmt.Sprintf("%d-%s", tableIndex, cidr_5) - routekey_6 = fmt.Sprintf("%d-%s", tableIndex, cidr_6) + routekey_cidr_local = fmt.Sprintf("%d-%s-0", tableIndex, cidr_local) + // routekey_1 = fmt.Sprintf("%d-%s-0", tableIndex, cidr_1) + // routekey_2 = fmt.Sprintf("%d-%s-0", tableIndex, cidr_2) + // routekey_3 = fmt.Sprintf("%d-%s-0", tableIndex, cidr_3) + routekey_4 = fmt.Sprintf("%d-%s-0", tableIndex, cidr_4) + // routekey_5 = fmt.Sprintf("%d-%s-0", tableIndex, cidr_5) + routekey_6 = fmt.Sprintf("%d-%s-0", tableIndex, cidr_6) ipv6_int1 = ip.FromString("2001:db8::192:168:0:0") ipv6_int2 = ip.FromString("2001:db8::192:168:10:0") @@ -110,13 +110,13 @@ var ( ipnetV6_4 = cidrV6_4.ToIPNet() // ipnetV6_5 = cidrV6_5.ToIPNet() // ipnetV6_6 = cidrV6_6.ToIPNet() - routekey_cidrV6_local = fmt.Sprintf("%d-%s", tableIndex, cidrV6_local) - // routekeyV6_1 = fmt.Sprintf("%d-%s", tableIndex, cidrV6_1) - // routekeyV6_2 = fmt.Sprintf("%d-%s", tableIndex, cidrV6_2) - // routekeyV6_3 = fmt.Sprintf("%d-%s", tableIndex, cidrV6_3) - routekeyV6_4 = fmt.Sprintf("%d-%s", tableIndex, cidrV6_4) - // routekeyV6_5 = fmt.Sprintf("%d-%s", tableIndex, cidr_5) - routekeyV6_6 = fmt.Sprintf("%d-%s", tableIndex, cidrV6_6) + routekey_cidrV6_local = fmt.Sprintf("%d-%s-1024", tableIndex, cidrV6_local) + // routekeyV6_1 = fmt.Sprintf("%d-%s-1024", tableIndex, cidrV6_1) + // routekeyV6_2 = fmt.Sprintf("%d-%s-1024", tableIndex, cidrV6_2) + // routekeyV6_3 = fmt.Sprintf("%d-%s-1024", tableIndex, cidrV6_3) + routekeyV6_4 = fmt.Sprintf("%d-%s-1024", tableIndex, cidrV6_4) + // routekeyV6_5 = fmt.Sprintf("%d-%s-0", tableIndex, cidr_5) + routekeyV6_6 = fmt.Sprintf("%d-%s-1024", tableIndex, cidrV6_6) ) func mustGeneratePrivateKey() wgtypes.Key { @@ -1388,9 +1388,9 @@ func describeEnableTests(enableV4, enableV6 bool) { if enableV4 { // Update the mock routing table dataplane so that it knows about the wireguard interface. rtDataplane.NameToLink[ifaceName] = link - routekey_1 = fmt.Sprintf("%d-%s", tableIndex, cidr_1) - routekey_2 = fmt.Sprintf("%d-%s", tableIndex, cidr_2) - routekey_3 = fmt.Sprintf("%d-%s", tableIndex, cidr_3) + routekey_1 = fmt.Sprintf("%d-%s-0", tableIndex, cidr_1) + routekey_2 = fmt.Sprintf("%d-%s-0", tableIndex, cidr_2) + routekey_3 = fmt.Sprintf("%d-%s-0", tableIndex, cidr_3) wg.RouteUpdate(hostname, cidr_local) wg.RouteUpdate(peer1, cidr_1) @@ -1403,9 +1403,9 @@ func describeEnableTests(enableV4, enableV6 bool) { if enableV6 { // Update the mock routing table dataplane so that it knows about the wireguard interface. rtDataplaneV6.NameToLink[ifaceNameV6] = linkV6 - routekeyV6_1 = fmt.Sprintf("%d-%s", tableIndex, cidrV6_1) - routekeyV6_2 = fmt.Sprintf("%d-%s", tableIndex, cidrV6_2) - routekeyV6_3 = fmt.Sprintf("%d-%s", tableIndex, cidrV6_3) + routekeyV6_1 = fmt.Sprintf("%d-%s-1024", tableIndex, cidrV6_1) + routekeyV6_2 = fmt.Sprintf("%d-%s-1024", tableIndex, cidrV6_2) + routekeyV6_3 = fmt.Sprintf("%d-%s-1024", tableIndex, cidrV6_3) wgV6.RouteUpdate(hostname, cidrV6_local) wgV6.RouteUpdate(peer1, cidrV6_1) @@ -2515,7 +2515,7 @@ func describeEnableTests(enableV4, enableV6 bool) { It("should reprogram the route to peer3 only", func() { if enableV4 { - routekey_4 := fmt.Sprintf("%d-%s", tableIndex, cidr_4) + routekey_4 := fmt.Sprintf("%d-%s-0", tableIndex, cidr_4) Expect(rtDataplane.UpdatedRouteKeys).To(HaveLen(1)) Expect(rtDataplane.DeletedRouteKeys).To(HaveLen(0)) Expect(rtDataplane.UpdatedRouteKeys).To(HaveKey(routekey_4)) @@ -2530,7 +2530,7 @@ func describeEnableTests(enableV4, enableV6 bool) { })) } if enableV6 { - routekeyV6_4 := fmt.Sprintf("%d-%s", tableIndex, cidrV6_4) + routekeyV6_4 := fmt.Sprintf("%d-%s-1024", tableIndex, cidrV6_4) Expect(rtDataplaneV6.UpdatedRouteKeys).To(HaveLen(1)) Expect(rtDataplaneV6.DeletedRouteKeys).To(HaveLen(0)) Expect(rtDataplaneV6.UpdatedRouteKeys).To(HaveKey(routekeyV6_4)) @@ -2763,9 +2763,9 @@ func describeEnableTests(enableV4, enableV6 bool) { // We expect the link to exist. link = wgDataplane.NameToLink[ifaceName] Expect(link).ToNot(BeNil()) - routekey_1 = fmt.Sprintf("%d-%s", tableIndex, cidr_1) - routekey_2 = fmt.Sprintf("%d-%s", tableIndex, cidr_2) - routekey_3 = fmt.Sprintf("%d-%s", tableIndex, cidr_3) + routekey_1 = fmt.Sprintf("%d-%s-0", tableIndex, cidr_1) + routekey_2 = fmt.Sprintf("%d-%s-0", tableIndex, cidr_2) + routekey_3 = fmt.Sprintf("%d-%s-0", tableIndex, cidr_3) // Set the interface to be up wgDataplane.SetIface(ifaceName, true, true) @@ -2819,9 +2819,9 @@ func describeEnableTests(enableV4, enableV6 bool) { // We expect the link to exist. linkV6 = wgDataplaneV6.NameToLink[ifaceNameV6] Expect(linkV6).ToNot(BeNil()) - routekeyV6_1 = fmt.Sprintf("%d-%s", tableIndex, cidrV6_1) - routekeyV6_2 = fmt.Sprintf("%d-%s", tableIndex, cidrV6_2) - routekeyV6_3 = fmt.Sprintf("%d-%s", tableIndex, cidrV6_3) + routekeyV6_1 = fmt.Sprintf("%d-%s-1024", tableIndex, cidrV6_1) + routekeyV6_2 = fmt.Sprintf("%d-%s-1024", tableIndex, cidrV6_2) + routekeyV6_3 = fmt.Sprintf("%d-%s-1024", tableIndex, cidrV6_3) // Set the interface to be up wgDataplaneV6.SetIface(ifaceNameV6, true, true) From b4365503dbdfd06bc510d23b2cdcfce3ddf1ca3e Mon Sep 17 00:00:00 2001 From: Nell Jerram Date: Mon, 2 Mar 2026 10:30:48 +0000 Subject: [PATCH 6/8] Add live migration routing sequence unit tests Test the two key live migration subcases where Felix-managed routes coexist with external BIRD routes at different priorities for the same VM IP: (a) Source host: Felix local route at normal priority, BIRD remote route appears at elevated priority, Felix removes its route, BIRD reverts to normal priority. (b) Destination host: BIRD remote route at normal priority, Felix programs local route at elevated priority, BIRD route removed, Felix reverts to normal priority. Verifies that resync never disturbs the external BIRD routes. Co-Authored-By: Claude Opus 4.6 --- felix/routetable/route_table_test.go | 187 +++++++++++++++++++++++++++ 1 file changed, 187 insertions(+) diff --git a/felix/routetable/route_table_test.go b/felix/routetable/route_table_test.go index 23362e4674a..6135f6ea65c 100644 --- a/felix/routetable/route_table_test.go +++ b/felix/routetable/route_table_test.go @@ -2031,6 +2031,193 @@ var _ = Describe("RouteTable with multiple priorities", func() { Expect(dataplane.RouteKeyToRoute).NotTo(HaveKey(lowKey)) Expect(dataplane.RouteKeyToRoute).To(HaveKey(highKey)) }) + + Describe("live migration: source host", func() { + // Simulates the sequence of events when a VM is live-migrated + // AWAY from this host. Felix manages the local workload route + // on cali1; BIRD programs a remote route on eth0 that Felix + // should not touch. + + const ( + normalPriority = 200 // Normal routing priority. + elevatedPriority = 100 // Lower number = higher kernel preference. + birdProto = 12 // RTPROT_BIRD + vmCIDR = "10.0.0.42/32" + ) + + var eth0 *mocknetlink.MockLink + + BeforeEach(func() { + eth0 = dataplane.AddIface(10, "eth0", true, true) + }) + + It("should handle the full migration sequence", func() { + // Step 1: Felix programs a local route for the VM at normal priority. + rt.SetRoutes(RouteClassLocalWorkload, "cali1", []Target{{ + RouteKey: RouteKey{ + CIDR: ip.MustParseCIDROrIP(vmCIDR), + Priority: normalPriority, + }, + }}) + err := rt.Apply() + Expect(err).ToNot(HaveOccurred()) + + felixKey := routeKeyStr(254, vmCIDR, normalPriority) + Expect(dataplane.RouteKeyToRoute).To(HaveKey(felixKey)) + + // Step 2: BIRD programs a remote route at elevated priority via eth0. + birdRoute := netlink.Route{ + Family: unix.AF_INET, + LinkIndex: eth0.LinkAttrs.Index, + Dst: mustParseCIDR(vmCIDR), + Type: syscall.RTN_UNICAST, + Protocol: birdProto, + Scope: netlink.SCOPE_UNIVERSE, + Table: unix.RT_TABLE_MAIN, + Priority: elevatedPriority, + } + dataplane.AddMockRoute(&birdRoute) + birdKey := routeKeyStr(254, vmCIDR, elevatedPriority) + + // Resync: Felix should see the BIRD route but leave it alone. + rt.QueueResync() + dataplane.ResetDeltas() + err = rt.Apply() + Expect(err).ToNot(HaveOccurred()) + + Expect(dataplane.RouteKeyToRoute).To(HaveKey(felixKey)) + Expect(dataplane.RouteKeyToRoute).To(HaveKey(birdKey)) + Expect(dataplane.DeletedRouteKeys).NotTo(HaveKey(birdKey)) + + // Step 3: Source cleanup — Felix removes the local route. + rt.RouteRemove(RouteClassLocalWorkload, "cali1", RouteKey{ + CIDR: ip.MustParseCIDROrIP(vmCIDR), + Priority: normalPriority, + }) + dataplane.ResetDeltas() + err = rt.Apply() + Expect(err).ToNot(HaveOccurred()) + + Expect(dataplane.RouteKeyToRoute).NotTo(HaveKey(felixKey)) + Expect(dataplane.RouteKeyToRoute).To(HaveKey(birdKey)) + Expect(dataplane.DeletedRouteKeys).To(HaveKey(felixKey)) + Expect(dataplane.DeletedRouteKeys).NotTo(HaveKey(birdKey)) + + // Step 4: BIRD updates its route to normal priority. + dataplane.RemoveMockRoute(&birdRoute) + birdRouteNormal := netlink.Route{ + Family: unix.AF_INET, + LinkIndex: eth0.LinkAttrs.Index, + Dst: mustParseCIDR(vmCIDR), + Type: syscall.RTN_UNICAST, + Protocol: birdProto, + Scope: netlink.SCOPE_UNIVERSE, + Table: unix.RT_TABLE_MAIN, + Priority: normalPriority, + } + dataplane.AddMockRoute(&birdRouteNormal) + birdNormalKey := routeKeyStr(254, vmCIDR, normalPriority) + + // Resync: Felix should leave BIRD's route at the new priority alone. + rt.QueueResync() + dataplane.ResetDeltas() + err = rt.Apply() + Expect(err).ToNot(HaveOccurred()) + + Expect(dataplane.RouteKeyToRoute).To(HaveKey(birdNormalKey)) + Expect(dataplane.DeletedRouteKeys).NotTo(HaveKey(birdNormalKey)) + }) + }) + + Describe("live migration: destination host", func() { + // Simulates the sequence of events when a VM is live-migrated + // TO this host. BIRD has a remote route for the VM on eth0; + // Felix then programs a local workload route on cali1 with + // elevated priority. + + const ( + normalPriority = 200 // Normal routing priority. + elevatedPriority = 100 // Lower number = higher kernel preference. + birdProto = 12 // RTPROT_BIRD + vmCIDR = "10.0.0.42/32" + ) + + var eth0 *mocknetlink.MockLink + + BeforeEach(func() { + eth0 = dataplane.AddIface(10, "eth0", true, true) + }) + + It("should handle the full migration sequence", func() { + // Step 1: BIRD remote route exists at normal priority. + birdRoute := netlink.Route{ + Family: unix.AF_INET, + LinkIndex: eth0.LinkAttrs.Index, + Dst: mustParseCIDR(vmCIDR), + Type: syscall.RTN_UNICAST, + Protocol: birdProto, + Scope: netlink.SCOPE_UNIVERSE, + Table: unix.RT_TABLE_MAIN, + Priority: normalPriority, + } + dataplane.AddMockRoute(&birdRoute) + birdKey := routeKeyStr(254, vmCIDR, normalPriority) + + // Initial resync: BIRD route should be left alone. + rt.QueueResync() + dataplane.ResetDeltas() + err := rt.Apply() + Expect(err).ToNot(HaveOccurred()) + + Expect(dataplane.RouteKeyToRoute).To(HaveKey(birdKey)) + Expect(dataplane.DeletedRouteKeys).NotTo(HaveKey(birdKey)) + + // Step 2: VM is now live on this host — Felix programs a + // local route with elevated priority. + rt.SetRoutes(RouteClassLocalWorkload, "cali1", []Target{{ + RouteKey: RouteKey{ + CIDR: ip.MustParseCIDROrIP(vmCIDR), + Priority: elevatedPriority, + }, + }}) + dataplane.ResetDeltas() + err = rt.Apply() + Expect(err).ToNot(HaveOccurred()) + + felixKey := routeKeyStr(254, vmCIDR, elevatedPriority) + Expect(dataplane.RouteKeyToRoute).To(HaveKey(felixKey)) + Expect(dataplane.RouteKeyToRoute).To(HaveKey(birdKey)) + + // Step 3: Source cleanup — BIRD removes the remote route. + dataplane.RemoveMockRoute(&birdRoute) + + // Resync: Felix route should survive, BIRD route gone. + rt.QueueResync() + dataplane.ResetDeltas() + err = rt.Apply() + Expect(err).ToNot(HaveOccurred()) + + Expect(dataplane.RouteKeyToRoute).To(HaveKey(felixKey)) + Expect(dataplane.RouteKeyToRoute).NotTo(HaveKey(birdKey)) + + // Step 4: Reversion to normal — Felix updates its route + // to normal priority. + rt.SetRoutes(RouteClassLocalWorkload, "cali1", []Target{{ + RouteKey: RouteKey{ + CIDR: ip.MustParseCIDROrIP(vmCIDR), + Priority: normalPriority, + }, + }}) + dataplane.ResetDeltas() + err = rt.Apply() + Expect(err).ToNot(HaveOccurred()) + + normalKey := routeKeyStr(254, vmCIDR, normalPriority) + Expect(dataplane.RouteKeyToRoute).To(HaveKey(normalKey)) + Expect(dataplane.RouteKeyToRoute).NotTo(HaveKey(felixKey)) + Expect(dataplane.DeletedRouteKeys).To(HaveKey(felixKey)) + }) + }) }) }) From 91a6e4307251ac4f2a92189b3d1143349607466b Mon Sep 17 00:00:00 2001 From: Nell Jerram Date: Mon, 2 Mar 2026 10:44:14 +0000 Subject: [PATCH 7/8] Document conntrack tracker limitation with multiple route priorities Add a detailed comment explaining the interaction between conntrack cleanup and multiple route priorities. The ConntrackCleanupManager is keyed on CIDR (one owner per CIDR) while routes are keyed on RouteKey (CIDR + Priority). For the live migration use case this is safe: Felix only manages one route per CIDR at a time, with the coexisting BIRD route being external to the tracker. On the source host, the conntrack flush when Felix removes its local route is correct: the VM is leaving, so stale conntrack entries should be flushed to force policy re-evaluation (the return path may now traverse different HostEndpoints on a different host). On the destination host, no flush is triggered because BIRD's pre-existing route was never tracked. Co-Authored-By: Claude Opus 4.6 --- felix/routetable/route_table.go | 54 +++++++++++++++++++++++++++++++++ 1 file changed, 54 insertions(+) diff --git a/felix/routetable/route_table.go b/felix/routetable/route_table.go index 5c577053694..7c37862c7bf 100644 --- a/felix/routetable/route_table.go +++ b/felix/routetable/route_table.go @@ -347,6 +347,60 @@ func New( handlemgr.WithSocketTimeout(netlinkTimeout), ) + // Interaction between conntrack cleanup and multiple route priorities + // ================================================================ + // + // The conntrack tracker (ConntrackCleanupManager) tracks ownership of + // CIDRs for conntrack cleanup purposes. It is keyed on ip.Addr (one + // owner per CIDR), whereas routes are keyed on RouteKey (CIDR + TOS + + // Priority). recalculateDesiredKernelRoute is called per RouteKey and + // calls UpdateCIDROwner/RemoveCIDROwner on the tracker; when multiple + // RouteKeys share the same CIDR (i.e. different priorities), the last + // call wins. + // + // The primary motivation for multiple priorities is VM live migration. + // During live migration, the kernel has two routes for the VM's IP at + // different priorities: one managed by Felix (the local workload route) + // and one managed by BIRD (the remote route via the network). Only + // Felix's route passes through the RouteTable; BIRD's route is external + // and invisible to the tracker (it fails the routeIsOurs() check during + // resync because it has a non-Calico protocol on a non-workload + // interface). + // + // Because of this, Felix only manages ONE route per CIDR at any given + // time, and the tracker's one-owner-per-CIDR model is sufficient: + // + // - Source host (VM migrating away): Felix does RouteRemove of its + // local workload route. RemoveCIDROwner is called, and when the + // route is deleted from the kernel, OnDataplaneRouteDeleted queues a + // conntrack cleanup. This is the right behaviour: the VM is leaving + // this host, so conntrack entries for its IP are stale. Calico uses + // conntrack state to avoid re-evaluating policy for every packet in + // an established flow; flushing forces re-evaluation, which is + // appropriate because the return path may now traverse different + // HostEndpoints or policies on a different host. The downside is + // minor: a return-path packet may be dropped, but TCP will + // retransmit and UDP applications are typically tolerant of loss. + // + // - Destination host (VM arriving): Felix does RouteUpdate to add a + // local workload route at elevated priority. UpdateCIDROwner is + // called. BIRD's pre-existing remote route was never tracked (it's + // not "ours"), so the tracker sees this as a brand new entry — no + // previous owner, no cleanup triggered. This is also correct: the + // VM is arriving on this host, so there are no stale conntrack + // entries to flush. + // + // Limitation: if we later add use cases where Felix programs TWO routes + // for the same CIDR at different priorities simultaneously (both managed + // by Felix, not one Felix + one BIRD), the tracker would give incorrect + // results. Specifically, removing one priority's route would call + // RemoveCIDROwner for the CIDR, erasing the tracker's knowledge of the + // still-active route at the other priority. OnDataplaneRouteDeleted + // would then see no desired owner and queue a spurious conntrack flush. + // Fixing this would require extending the tracker with reference + // counting or a lookup across all priorities for a given CIDR. See + // also the comment on ConntrackCleanupManager in + // conntrack_owner_tracker.go. if rt.conntrackCleanupEnabled { rt.conntrackTracker = NewConntrackCleanupManager(ipVersion, rt.conntrack) } else { From a7f808182eb903c651aa27757d6675f57c0e61fb Mon Sep 17 00:00:00 2001 From: Nell Jerram Date: Mon, 2 Mar 2026 17:08:29 +0000 Subject: [PATCH 8/8] Improve comment about live migration and conntrack state cleanup --- felix/routetable/route_table.go | 75 ++++++++++++--------------------- 1 file changed, 27 insertions(+), 48 deletions(-) diff --git a/felix/routetable/route_table.go b/felix/routetable/route_table.go index 7c37862c7bf..e155be5b3b1 100644 --- a/felix/routetable/route_table.go +++ b/felix/routetable/route_table.go @@ -347,60 +347,39 @@ func New( handlemgr.WithSocketTimeout(netlinkTimeout), ) - // Interaction between conntrack cleanup and multiple route priorities - // ================================================================ + // The conntrack tracker (ConntrackCleanupManager) cleans up conntrack state for a workload + // IP - i.e. for flows going to or from that IP, on this node - when Felix stops programming + // a local route for that workload. The latter happens during live migration when the VM + // was originally on this node: a remote BIRD-programmed route appears for the IP, then + // Felix removes the old local route for the IP, which means that conntrack state for that + // IP is cleaned up; yet we want existing connections to the migrating IP to be impacted as + // little as possible. // - // The conntrack tracker (ConntrackCleanupManager) tracks ownership of - // CIDRs for conntrack cleanup purposes. It is keyed on ip.Addr (one - // owner per CIDR), whereas routes are keyed on RouteKey (CIDR + TOS + - // Priority). recalculateDesiredKernelRoute is called per RouteKey and - // calls UpdateCIDROwner/RemoveCIDROwner on the tracker; when multiple - // RouteKeys share the same CIDR (i.e. different priorities), the last - // call wins. + // Note that this isn't related to Priority, so it's irrelevant that the conntrack tracker + // doesn't yet handle that. We need the route table to handle Priority, to get the correct + // interaction between Felix- and BIRD-programmed routes with different priorities, and we + // include Priority in RouteKey because that matches what the kernel does. But Felix itself + // doesn't program multiple routes with the same IP and different priorities; at least, not + // for live migration. // - // The primary motivation for multiple priorities is VM live migration. - // During live migration, the kernel has two routes for the VM's IP at - // different priorities: one managed by Felix (the local workload route) - // and one managed by BIRD (the remote route via the network). Only - // Felix's route passes through the RouteTable; BIRD's route is external - // and invisible to the tracker (it fails the routeIsOurs() check during - // resync because it has a non-Calico protocol on a non-workload - // interface). + // The conntrack issue is about moving an existing flow from one node to another. The + // concerns are: // - // Because of this, Felix only manages ONE route per CIDR at any given - // time, and the tracker's one-owner-per-CIDR model is sufficient: + // 1. If TCP loose is disabled (`nf_conntrack_tcp_loose`), a non-SYN packet on the new node + // will not be able to re-establish the conntrack state there. // - // - Source host (VM migrating away): Felix does RouteRemove of its - // local workload route. RemoveCIDROwner is called, and when the - // route is deleted from the kernel, OnDataplaneRouteDeleted queues a - // conntrack cleanup. This is the right behaviour: the VM is leaving - // this host, so conntrack entries for its IP are stale. Calico uses - // conntrack state to avoid re-evaluating policy for every packet in - // an established flow; flushing forces re-evaluation, which is - // appropriate because the return path may now traverse different - // HostEndpoints or policies on a different host. The downside is - // minor: a return-path packet may be dropped, but TCP will - // retransmit and UDP applications are typically tolerant of loss. + // 2. If the next packet on a migrated flow is in the reverse direction (i.e. from the + // original connection's destination IP), policy might not allow it, whereas it would allow + // a packet in the forwards direction. // - // - Destination host (VM arriving): Felix does RouteUpdate to add a - // local workload route at elevated priority. UpdateCIDROwner is - // called. BIRD's pre-existing remote route was never tracked (it's - // not "ours"), so the tracker sees this as a brand new entry — no - // previous owner, no cleanup triggered. This is also correct: the - // VM is arriving on this host, so there are no stale conntrack - // entries to flush. + // 3. Re-evaluating policy for a migrated flow might require retransmitting a packet, and + // for UDP retransmission depends on application layer behaviour, instead of being baked + // into the protocol like with TCP; so that might not happen. // - // Limitation: if we later add use cases where Felix programs TWO routes - // for the same CIDR at different priorities simultaneously (both managed - // by Felix, not one Felix + one BIRD), the tracker would give incorrect - // results. Specifically, removing one priority's route would call - // RemoveCIDROwner for the CIDR, erasing the tracker's knowledge of the - // still-active route at the other priority. OnDataplaneRouteDeleted - // would then see no desired owner and queue a spurious conntrack flush. - // Fixing this would require extending the tracker with reference - // counting or a lookup across all priorities for a given CIDR. See - // also the comment on ConntrackCleanupManager in - // conntrack_owner_tracker.go. + // For the current increment of live migration support we have decided to live with these + // issues. In future we may address them by migrating the conntrack state to the new node. + // For now, TCP loose is effectively required for (1), and we hope that (2) and (3) will not + // have significant impacts in practice. if rt.conntrackCleanupEnabled { rt.conntrackTracker = NewConntrackCleanupManager(ipVersion, rt.conntrack) } else {