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 <nil> as destination network for default
routes, and instead show destination as 0.0.0.0/0 or ::/0

Signed-off-by: Milan Lenco <milan@zededa.com>

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
 }

pkg/pillar/nireconciler/linux_test.go

@@ -2752,23 +2752,61 @@ 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}]"))
 	t.Expect(itemDescription(dg.Reference(dnsmasqNI5))).To(ContainSubstring(
 		"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)

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