From a6136cf91d656e86909cc07850e655f92a59de05 Mon Sep 17 00:00:00 2001 From: Milan Lenco Date: Thu, 26 Sep 2024 12:23:18 +0200 Subject: [PATCH] Publish NI routes in a deterministic and easy-to-read order First IPv4 routes will be listed, then IPv6 routes. Inside the set of routes of the same IP version, default routes appear first for clarity, followed by other routes ordered by prefix length, with more specific routes (longer prefixes) listed before broader ones. This is at least how Linux lists the routes of a routing table. We also want to avoid returning as destination network for default routes, and instead show destination as 0.0.0.0/0 or ::/0 Signed-off-by: Milan Lenco --- pkg/pillar/nireconciler/linux_reconciler.go | 54 ++++++++++++++++++++- pkg/pillar/nireconciler/linux_test.go | 46 ++++++++++++++++-- pkg/pillar/types/zedroutertypes.go | 4 +- 3 files changed, 97 insertions(+), 7 deletions(-) diff --git a/pkg/pillar/nireconciler/linux_reconciler.go b/pkg/pillar/nireconciler/linux_reconciler.go index 5d705e3871..f0eaedfdc4 100644 --- a/pkg/pillar/nireconciler/linux_reconciler.go +++ b/pkg/pillar/nireconciler/linux_reconciler.go @@ -4,10 +4,13 @@ package nireconciler import ( + "bytes" "context" "fmt" + "net" "net/http" "path" + "sort" "strconv" "strings" "sync" @@ -43,7 +46,12 @@ const ( namedNsDir = "/var/run/netns" ) -var emptyUUID = uuid.UUID{} // used as a constant +// Used as constants. +var ( + emptyUUID = uuid.UUID{} + _, ipv4Any, _ = net.ParseCIDR("0.0.0.0/0") + _, ipv6Any, _ = net.ParseCIDR("::/0") +) // LinuxNIReconciler is a network instance reconciler for Linux network stack, // i.e. it configures and uses Linux networking to provide application connectivity. @@ -1270,12 +1278,54 @@ func (r *LinuxNIReconciler) getNIRouteInfo(niID uuid.UUID) (routes []types.IPRou } } } + var ipVer types.AddressType + switch route.Type() { + case generic.IPv4RouteTypename: + ipVer = types.AddressTypeIPV4 + case generic.IPv6RouteTypename: + ipVer = types.AddressTypeIPV6 + } + // Avoid returning nil destination network. + // nil is used for route destination by netlink for default routes. + dstNet := route.Dst + if dstNet == nil { + switch route.Type() { + case generic.IPv4RouteTypename: + dstNet = ipv4Any + case generic.IPv6RouteTypename: + dstNet = ipv6Any + } + } routes = append(routes, types.IPRouteInfo{ - DstNetwork: route.Dst, + IPVersion: ipVer, + DstNetwork: dstNet, Gateway: route.Gw, OutputPort: portLL, GatewayApp: appGW, }) } + // Return routes in a deterministic and easy-to-read order. + // First IPv4 routes will be listed, then IPv6 routes. + // Inside the set of routes of the same IP version, default routes appear + // first for clarity, followed by other routes ordered by prefix length, + // with more specific routes (longer prefixes) listed before broader ones. + // This is at least how Linux lists the routes of a routing table. + sort.Slice(routes, func(i, j int) bool { + if routes[i].IPVersion != routes[j].IPVersion { + return routes[i].IPVersion < routes[j].IPVersion + } + if routes[i].IsDefaultRoute() { + return true + } + if routes[j].IsDefaultRoute() { + return false + } + iPrefixLen, _ := routes[i].DstNetwork.Mask.Size() + jPrefixLen, _ := routes[j].DstNetwork.Mask.Size() + if iPrefixLen == jPrefixLen { + return bytes.Compare(routes[i].DstNetwork.IP, routes[j].DstNetwork.IP) == -1 + } + return iPrefixLen > jPrefixLen + }) return routes } diff --git a/pkg/pillar/nireconciler/linux_test.go b/pkg/pillar/nireconciler/linux_test.go index 8f050ee3ac..4c6ac12332 100644 --- a/pkg/pillar/nireconciler/linux_test.go +++ b/pkg/pillar/nireconciler/linux_test.go @@ -2752,15 +2752,52 @@ func TestStaticAndConnectedRoutes(test *testing.T) { } _, err = niReconciler.UpdateNI(ctx, ni1Config, ni1Bridge) t.Expect(err).ToNot(HaveOccurred()) + ni5Bridge.StaticRoutes = []nirec.IPRoute{ {DstNetwork: ipAddressWithPrefix("10.50.1.0/24"), Gateway: ipAddress("172.20.1.1")}, + {DstNetwork: ipAddressWithPrefix("10.50.14.0/26"), Gateway: ipAddress("172.30.30.15")}, + // This one uses app2 as GW: + {DstNetwork: ipAddressWithPrefix("10.50.19.0/30"), Gateway: ipAddress("10.10.20.2")}, + // This one also uses app2 as GW: + {DstNetwork: ipAddressWithPrefix("10.50.5.0/30"), Gateway: ipAddress("10.10.20.2")}, // This one has GW outside eth1 and eth3 subnets and will be skipped: {DstNetwork: ipAddressWithPrefix("10.50.2.0/24"), Gateway: ipAddress("172.21.1.1")}, // Override default route: {DstNetwork: ipAddressWithPrefix("0.0.0.0/0"), OutputPort: "ethernet1"}, } - _, err = niReconciler.UpdateNI(ctx, ni5Config, ni5Bridge) - t.Expect(err).ToNot(HaveOccurred()) + recStatus, err := niReconciler.UpdateNI(ctx, ni5Config, ni5Bridge) + t.Expect(err).ToNot(HaveOccurred()) + t.Expect(recStatus.Routes).To(HaveLen(5)) + t.Expect(recStatus.Routes[0].Equal(types.IPRouteInfo{ + IPVersion: types.AddressTypeIPV4, + DstNetwork: ipAddressWithPrefix("0.0.0.0/0"), + Gateway: ipAddress("172.20.0.1"), + OutputPort: "ethernet1", + })).To(BeTrue()) + t.Expect(recStatus.Routes[1].Equal(types.IPRouteInfo{ + IPVersion: types.AddressTypeIPV4, + DstNetwork: ipAddressWithPrefix("10.50.5.0/30"), + Gateway: ipAddress("10.10.20.2"), + GatewayApp: app2UUID.UUID, + })).To(BeTrue()) + t.Expect(recStatus.Routes[2].Equal(types.IPRouteInfo{ + IPVersion: types.AddressTypeIPV4, + DstNetwork: ipAddressWithPrefix("10.50.19.0/30"), + Gateway: ipAddress("10.10.20.2"), + GatewayApp: app2UUID.UUID, + })).To(BeTrue()) + t.Expect(recStatus.Routes[3].Equal(types.IPRouteInfo{ + IPVersion: types.AddressTypeIPV4, + DstNetwork: ipAddressWithPrefix("10.50.14.0/26"), + Gateway: ipAddress("172.30.30.15"), + OutputPort: "ethernet3", + })).To(BeTrue()) + t.Expect(recStatus.Routes[4].Equal(types.IPRouteInfo{ + IPVersion: types.AddressTypeIPV4, + DstNetwork: ipAddressWithPrefix("10.50.1.0/24"), + Gateway: ipAddress("172.20.1.1"), + OutputPort: "ethernet1", + })).To(BeTrue()) t.Expect(itemDescription(dg.Reference(dnsmasqNI1))).To(ContainSubstring( "propagateRoutes: [{10.50.1.0/24 10.10.10.100}]")) @@ -2768,7 +2805,8 @@ func TestStaticAndConnectedRoutes(test *testing.T) { "withDefaultRoute: true")) t.Expect(itemDescription(dg.Reference(dnsmasqNI5))).To(ContainSubstring( "propagateRoutes: [{132.163.96.5/32 10.10.20.1} {128.138.140.211/32 10.10.20.1} " + - "{1.1.1.1/32 10.10.20.1} {10.50.1.0/24 10.10.20.1} {172.20.0.0/16 10.10.20.1} " + + "{1.1.1.1/32 10.10.20.1} {10.50.1.0/24 10.10.20.1} {10.50.14.0/26 10.10.20.1} " + + "{10.50.19.0/30 10.10.20.2} {10.50.5.0/30 10.10.20.2} {172.20.0.0/16 10.10.20.1} " + "{172.30.30.0/24 10.10.20.1}]")) // Check routing tables @@ -2817,7 +2855,7 @@ func TestStaticAndConnectedRoutes(test *testing.T) { return false } return route.Table == 805 - })).To(Equal(2 + 2)) // + 2 static routes + })).To(Equal(2 + 5)) // + 5 static routes // Disconnect the application. appStatus, err = niReconciler.DelAppConn(ctx, app2UUID.UUID) diff --git a/pkg/pillar/types/zedroutertypes.go b/pkg/pillar/types/zedroutertypes.go index 24bf3722da..92dcc9f0e1 100644 --- a/pkg/pillar/types/zedroutertypes.go +++ b/pkg/pillar/types/zedroutertypes.go @@ -1018,6 +1018,7 @@ type IPRouteStatus struct { // IPRouteInfo contains info about a single IP route from the NI routing table. // It is published to the controller as part of ZInfoNetworkInstance. type IPRouteInfo struct { + IPVersion AddressType DstNetwork *net.IPNet // Nil for connected route. Gateway net.IP @@ -1042,7 +1043,8 @@ func (r IPRouteInfo) IsDefaultRoute() bool { // Equal compares two IP routes for equality. func (r IPRouteInfo) Equal(r2 IPRouteInfo) bool { - return netutils.EqualIPs(r.Gateway, r2.Gateway) && + return r.IPVersion == r2.IPVersion && + netutils.EqualIPs(r.Gateway, r2.Gateway) && netutils.EqualIPNets(r.DstNetwork, r2.DstNetwork) && r.OutputPort == r2.OutputPort && r.GatewayApp == r2.GatewayApp