Skip to content

Commit

Permalink
Publish NI routes in a deterministic and easy-to-read order
Browse files Browse the repository at this point in the history 
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 <[email protected]>
  • Loading branch information
@milan-zededa
milan-zededa committed Sep 26, 2024
1 parent 8822a92 commit 76de04f
Show file tree
Hide file tree
Showing 3 changed files with 89 additions and 7 deletions.
54 changes: 52 additions & 2 deletions pkg/pillar/nireconciler/linux_reconciler.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4,10 +4,13 @@
package nireconciler

import (
"bytes"
"context"
"fmt"
"net"
"net/http"
"path"
"sort"
"strconv"
"strings"
"sync"
Expand Down Expand Up @@ -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.
Expand Down Expand Up @@ -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
}
38 changes: 34 additions & 4 deletions pkg/pillar/nireconciler/linux_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2752,23 +2752,53 @@ 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 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(4))
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.19.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.14.0/26"),
Gateway: ipAddress("172.30.30.15"),
OutputPort: "ethernet3",
})).To(BeTrue())
t.Expect(recStatus.Routes[3].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} {172.20.0.0/16 10.10.20.1} " +
"{172.30.30.0/24 10.10.20.1}]"))

// Check routing tables
Expand Down Expand Up @@ -2817,7 +2847,7 @@ func TestStaticAndConnectedRoutes(test *testing.T) {
return false
}
return route.Table == 805
})).To(Equal(2 + 2)) // + 2 static routes
})).To(Equal(2 + 4)) // + 4 static routes

// Disconnect the application.
appStatus, err = niReconciler.DelAppConn(ctx, app2UUID.UUID)
Expand Down
4 changes: 3 additions & 1 deletion pkg/pillar/types/zedroutertypes.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
Expand Down

0 comments on commit 76de04f

Please sign in to comment.