Optimize getting routes function for Windows (#5335)

Signed-off-by: Hongliang Liu <lhongliang@vmware.com>

pkg/agent/agent_windows.go

@@ -29,6 +29,7 @@ import (
 	"antrea.io/antrea/pkg/agent/externalnode"
 	"antrea.io/antrea/pkg/agent/interfacestore"
 	"antrea.io/antrea/pkg/agent/util"
+	antreasyscall "antrea.io/antrea/pkg/agent/util/syscall"
 	"antrea.io/antrea/pkg/apis/crd/v1alpha1"
 	"antrea.io/antrea/pkg/ovs/ovsctl"
 	utilip "antrea.io/antrea/pkg/util/ip"
@@ -366,21 +367,18 @@ func (i *Initializer) getTunnelPortLocalIP() net.IP {
 // The routes will be restored on the OVS bridge interface after the IP
 // configuration is moved to the OVS bridge.
 func (i *Initializer) saveHostRoutes() error {
-	routes, err := util.GetNetRoutesAll()
+	// IPv6 is not supported on Windows currently. Please refer to https://github.com/antrea-io/antrea/issues/5162
+	// for more information.
+	family := antreasyscall.AF_INET
+	filter := &util.Route{
+		LinkIndex:      i.nodeConfig.UplinkNetConfig.Index,
+		GatewayAddress: net.ParseIP(i.nodeConfig.UplinkNetConfig.Gateway),
+	}
+	routes, err := util.RouteListFiltered(family, filter, util.RT_FILTER_IF|util.RT_FILTER_GW)
 	if err != nil {
 		return err
 	}
 	for _, route := range routes {
-		if route.LinkIndex != i.nodeConfig.UplinkNetConfig.Index {
-			continue
-		}
-		if route.GatewayAddress.String() != i.nodeConfig.UplinkNetConfig.Gateway {
-			continue
-		}
-		// Skip IPv6 routes before we support IPv6 stack.
-		if route.DestinationSubnet.IP.To4() == nil {
-			continue
-		}
 		// Skip default route. The default route will be added automatically when
 		// configuring IP address on OVS bridge interface.
 		if route.DestinationSubnet.IP.IsUnspecified() {

pkg/agent/route/route_windows.go

@@ -21,7 +21,6 @@ import (
 	"errors"
 	"fmt"
 	"net"
-	"reflect"
 	"sync"
 
 	"k8s.io/apimachinery/pkg/util/sets"
@@ -32,6 +31,7 @@ import (
 	"antrea.io/antrea/pkg/agent/openflow"
 	"antrea.io/antrea/pkg/agent/servicecidr"
 	"antrea.io/antrea/pkg/agent/util"
+	antreasyscall "antrea.io/antrea/pkg/agent/util/syscall"
 	"antrea.io/antrea/pkg/agent/util/winfirewall"
 	binding "antrea.io/antrea/pkg/ovs/openflow"
 	iputil "antrea.io/antrea/pkg/util/ip"
@@ -157,18 +157,37 @@ func (c *Client) Reconcile(podCIDRs []string) error {
 	if err != nil {
 		return err
 	}
-	for dst, rt := range routes {
-		if reflect.DeepEqual(rt.DestinationSubnet, c.nodeConfig.PodIPv4CIDR) {
+	for i := range routes {
+		// Don't remove the route entry that does not use global unicast IP address as destination, like multicast, IPv6
+		// link local or loopback.
+		if !routes[i].DestinationSubnet.IP.IsGlobalUnicast() {
 			continue
 		}
-		if desiredPodCIDRs.Has(dst) {
+		// When configuring an IP address to an interface on Windows, three route entries will be automatically added.
+		// For example, if the IP address is 10.10.0.1/24, the following three routes will be created:
+		// Network Destination   Netmask          Gateway  Interface  Metric
+		// 10.10.0.1             255.255.255.255  On-link  10.10.0.1  281
+		// 10.10.0.0             255.255.255.0    On-link  10.10.0.1  281
+		// 10.10.0.255           255.255.255.255  On-link  10.10.0.1  281
+		// The host (10.10.0.1) and broadcast (10.10.0.255) routes should be ignored since they are not supposed to be
+		// managed by Antrea. We can ignore them by comparing them to the calculated broadcast IP. Don't remove them since
+		// removing those route entries might introduce host networking issues.
+		if routes[i].DestinationSubnet.IP.Equal(iputil.GetLocalBroadcastIP(routes[i].DestinationSubnet)) {
 			continue
 		}
-		// Don't delete the routes which are added by AntreaProxy when proxyAll is enabled.
-		if c.proxyAll && c.isServiceRoute(rt) {
+		// Don't remove the route entry that uses local Pod CIDR as destination.
+		if iputil.IPNetEqual(routes[i].DestinationSubnet, c.nodeConfig.PodIPv4CIDR) {
 			continue
 		}
-		err := util.RemoveNetRoute(rt)
+		// Don't remove the route entry whose destination is included in the desired Pod CIDRs.
+		if desiredPodCIDRs.Has(routes[i].DestinationSubnet.String()) {
+			continue
+		}
+		// Don't remove the route entries which are added by AntreaProxy when proxyAll is enabled.
+		if c.proxyAll && c.isServiceRoute(&routes[i]) {
+			continue
+		}
+		err = util.RemoveNetRoute(&routes[i])
 		if err != nil {
 			return err
 		}
@@ -289,21 +308,21 @@ func (c *Client) addServiceCIDRRoute(serviceCIDR *net.IPNet) error {
 		if err != nil {
 			return fmt.Errorf("error listing ip routes: %w", err)
 		}
-		for _, rt := range routes {
-			if !rt.GatewayAddress.Equal(gw) {
+		for i := range routes {
+			if !routes[i].GatewayAddress.Equal(gw) {
 				continue
 			}
 			// It's the latest route we just installed.
-			if iputil.IPNetEqual(rt.DestinationSubnet, serviceCIDR) {
+			if iputil.IPNetEqual(routes[i].DestinationSubnet, serviceCIDR) {
 				continue
 			}
 			// The route covers the desired route. It was installed when the calculated ServiceCIDR is larger than the current one, which could happen after some Services are deleted.
-			if iputil.IPNetContains(rt.DestinationSubnet, serviceCIDR) {
-				staleRoutes = append(staleRoutes, rt)
+			if iputil.IPNetContains(routes[i].DestinationSubnet, serviceCIDR) {
+				staleRoutes = append(staleRoutes, &routes[i])
 			}
 			// The desired route covers the route. It was either installed when the calculated ServiceCIDR is smaller than the current one, or a per-IP route generated before v1.12.0.
-			if iputil.IPNetContains(serviceCIDR, rt.DestinationSubnet) {
-				staleRoutes = append(staleRoutes, rt)
+			if iputil.IPNetContains(serviceCIDR, routes[i].DestinationSubnet) {
+				staleRoutes = append(staleRoutes, &routes[i])
 			}
 		}
 	}
@@ -423,42 +442,10 @@ func (c *Client) isServiceRoute(route *util.Route) bool {
 	return false
 }
 
-func (c *Client) listIPRoutesOnGW() (map[string]*util.Route, error) {
-	routes, err := util.GetNetRoutesAll()
-	if err != nil {
-		return nil, err
-	}
-	rtMap := make(map[string]*util.Route)
-	for idx := range routes {
-		rt := routes[idx]
-		if rt.LinkIndex != c.nodeConfig.GatewayConfig.LinkIndex {
-			continue
-		}
-		// Only process IPv4 route entries in the loop.
-		if rt.DestinationSubnet.IP.To4() == nil {
-			continue
-		}
-		// Retrieve the route entries that use global unicast IP address as the destination. "GetNetRoutesAll" also
-		// returns the entries of loopback, broadcast, and multicast, which are added by the system when adding a new IP
-		// on the interface. Since removing those route entries might introduce the host networking issues, ignore them
-		// from the list.
-		if !rt.DestinationSubnet.IP.IsGlobalUnicast() {
-			continue
-		}
-		// When configuring an IP address to an interface on Windows, three route entries will be automatically added.
-		// For example, if the IP address is 10.10.0.1/24, the following three routes will be created:
-		// Network Destination   Netmask          Gateway  Interface  Metric
-		// 10.10.0.1             255.255.255.255  On-link  10.10.0.1  281
-		// 10.10.0.0             255.255.255.0    On-link  10.10.0.1  281
-		// 10.10.0.255           255.255.255.255  On-link  10.10.0.1  281
-		// The host (10.10.0.1) and broadcast (10.10.0.255) routes should be ignored since they are not supposed to be
-		// managed by Antrea. We can ignore them by comparing them to the calculated broadcast IP.
-		if !rt.GatewayAddress.Equal(config.VirtualServiceIPv4) && rt.DestinationSubnet.IP.Equal(iputil.GetLocalBroadcastIP(rt.DestinationSubnet)) {
-			continue
-		}
-		rtMap[rt.DestinationSubnet.String()] = &rt
-	}
-	return rtMap, nil
+func (c *Client) listIPRoutesOnGW() ([]util.Route, error) {
+	family := antreasyscall.AF_INET
+	filter := &util.Route{LinkIndex: c.nodeConfig.GatewayConfig.LinkIndex}
+	return util.RouteListFiltered(family, filter, util.RT_FILTER_IF)
 }
 
 // initFwRules adds Windows Firewall rules to accept the traffic that is sent to or from local Pods.

pkg/agent/route/route_windows_test.go

@@ -28,6 +28,7 @@ import (
 
 	"antrea.io/antrea/pkg/agent/config"
 	"antrea.io/antrea/pkg/agent/util"
+	antreasyscall "antrea.io/antrea/pkg/agent/util/syscall"
 )
 
 var (
@@ -71,13 +72,13 @@ func TestRouteOperation(t *testing.T) {
 	// Add initial routes.
 	err = client.AddRoutes(destCIDR1, "node1", peerNodeIP1, gwIP1)
 	require.Nil(t, err)
-	routes1, err := util.GetNetRoutes(gwLink, destCIDR1)
+	routes1, err := util.RouteListFiltered(antreasyscall.AF_INET, &util.Route{LinkIndex: gwLink, DestinationSubnet: destCIDR1}, util.RT_FILTER_IF|util.RT_FILTER_DST)
 	require.Nil(t, err)
 	assert.Equal(t, 1, len(routes1))
 
 	err = client.AddRoutes(destCIDR2, "node2", peerNodeIP2, gwIP2)
 	require.Nil(t, err)
-	routes2, err := util.GetNetRoutes(gwLink, destCIDR2)
+	routes2, err := util.RouteListFiltered(antreasyscall.AF_INET, &util.Route{LinkIndex: gwLink, DestinationSubnet: destCIDR2}, util.RT_FILTER_IF|util.RT_FILTER_DST)
 	require.Nil(t, err)
 	assert.Equal(t, 1, len(routes2))
 
@@ -86,7 +87,7 @@ func TestRouteOperation(t *testing.T) {
 
 	err = client.DeleteRoutes(destCIDR2)
 	require.Nil(t, err)
-	routes7, err := util.GetNetRoutes(gwLink, destCIDR2)
+	routes7, err := util.RouteListFiltered(antreasyscall.AF_INET, &util.Route{LinkIndex: gwLink, DestinationSubnet: destCIDR2}, util.RT_FILTER_IF|util.RT_FILTER_DST)
 	require.Nil(t, err)
 	assert.Equal(t, 0, len(routes7))
 }
@@ -100,7 +101,7 @@ func TestAddAndDeleteExternalIPRoute(t *testing.T) {
 
 	assert.NoError(t, c.AddExternalIPRoute(externalIP))
 	externalIPNet := util.NewIPNet(externalIP)
-	routes, err := util.GetNetRoutes(gwLink, externalIPNet)
+	routes, err := util.RouteListFiltered(antreasyscall.AF_INET, &util.Route{LinkIndex: gwLink, DestinationSubnet: externalIPNet}, util.RT_FILTER_IF|util.RT_FILTER_DST)
 	require.Nil(t, err)
 	assert.Equal(t, 1, len(routes))
 
@@ -109,7 +110,7 @@ func TestAddAndDeleteExternalIPRoute(t *testing.T) {
 	assert.EqualValues(t, routes[0], *route.(*util.Route))
 
 	assert.NoError(t, c.DeleteExternalIPRoute(externalIP))
-	routes, err = util.GetNetRoutes(gwLink, externalIPNet)
+	routes, err = util.RouteListFiltered(antreasyscall.AF_INET, &util.Route{LinkIndex: gwLink, DestinationSubnet: externalIPNet}, util.RT_FILTER_IF|util.RT_FILTER_DST)
 	require.Nil(t, err)
 	assert.Equal(t, 0, len(routes))
 	_, ok = c.serviceRoutes.Load(externalIP.String())

pkg/agent/util/net_windows.go

@@ -42,6 +42,7 @@ import (
 	ps "antrea.io/antrea/pkg/agent/util/powershell"
 	antreasyscall "antrea.io/antrea/pkg/agent/util/syscall"
 	binding "antrea.io/antrea/pkg/ovs/openflow"
+	iputil "antrea.io/antrea/pkg/util/ip"
 )
 
 const (
@@ -59,6 +60,12 @@ const (
 
 	AntreaNatName = "antrea-nat"
 	LocalVMSwitch = "antrea-switch"
+
+	// Filter masks are used to indicate the attributes used for route filtering.
+	RT_FILTER_IF uint64 = 1 << (1 + iota)
+	RT_FILTER_METRIC
+	RT_FILTER_DST
+	RT_FILTER_GW
 )
 
 var (
@@ -88,7 +95,7 @@ func (r *Route) Equal(x Route) bool {
 	return x.LinkIndex == r.LinkIndex &&
 		x.DestinationSubnet != nil &&
 		r.DestinationSubnet != nil &&
-		x.DestinationSubnet.String() == r.DestinationSubnet.String() &&
+		iputil.IPNetEqual(x.DestinationSubnet, r.DestinationSubnet) &&
 		x.GatewayAddress.Equal(r.GatewayAddress)
 }
 
@@ -577,9 +584,11 @@ func EnableRSCOnVSwitch(vSwitch string) error {
 func GetDefaultGatewayByInterfaceIndex(ifIndex int) (string, error) {
 	ip, defaultDestination, _ := net.ParseCIDR("0.0.0.0/0")
 	family := addressFamilyByIP(ip)
-	routes, err := listRoutes(family, func(row antreasyscall.MibIPForwardRow) bool {
-		return row.DestinationPrefix.EqualsTo(defaultDestination) && row.Index == uint32(ifIndex)
-	})
+	filter := &Route{
+		LinkIndex:         ifIndex,
+		DestinationSubnet: defaultDestination,
+	}
+	routes, err := RouteListFiltered(family, filter, RT_FILTER_IF|RT_FILTER_DST)
 	if err != nil {
 		return "", err
 	}
@@ -704,33 +713,29 @@ func ReplaceNetRoute(route *Route) error {
 	return NewNetRoute(route)
 }
 
-func GetNetRoutes(linkIndex int, dstSubnet *net.IPNet) ([]Route, error) {
-	if dstSubnet == nil {
-		return nil, fmt.Errorf("unable to get net routes for %d", linkIndex)
-	}
-	family := addressFamilyByIP(dstSubnet.IP)
-	return listRoutes(family, func(row antreasyscall.MibIPForwardRow) bool {
-		return row.DestinationPrefix.EqualsTo(dstSubnet) && row.Index == uint32(linkIndex)
-	})
-}
-
-func GetNetRoutesAll() ([]Route, error) {
-	return listRoutes(antreasyscall.AF_UNSPEC, nil)
-}
-
-type routeFilter func(row antreasyscall.MibIPForwardRow) bool
-
-func listRoutes(family uint16, filter routeFilter) ([]Route, error) {
+func RouteListFiltered(family uint16, filter *Route, filterMask uint64) ([]Route, error) {
 	rows, err := antreaNetIO.ListIPForwardRows(family)
 	if err != nil {
 		return nil, fmt.Errorf("unable to list Windows IPForward rows: %v", err)
 	}
 	rts := make([]Route, 0, len(rows))
-	for i, r := range rows {
-		if filter == nil || filter(r) {
-			route := routeFromIPForwardRow(&rows[i])
-			rts = append(rts, *route)
+	for i := range rows {
+		route := routeFromIPForwardRow(&rows[i])
+		if filter != nil {
+			if filterMask&RT_FILTER_IF != 0 && filter.LinkIndex != route.LinkIndex {
+				continue
+			}
+			if filterMask&RT_FILTER_DST != 0 && !iputil.IPNetEqual(filter.DestinationSubnet, route.DestinationSubnet) {
+				continue
+			}
+			if filterMask&RT_FILTER_GW != 0 && !filter.GatewayAddress.Equal(route.GatewayAddress) {
+				continue
+			}
+			if filterMask&RT_FILTER_METRIC != 0 && filter.RouteMetric != route.RouteMetric {
+				continue
+			}
 		}
+		rts = append(rts, *route)
 	}
 	return rts, nil
 }
@@ -968,10 +973,18 @@ func GetInterfaceConfig(ifName string) (*net.Interface, []*net.IPNet, []interfac
 	if err != nil {
 		return nil, nil, nil, fmt.Errorf("failed to get address for interface %s: %v", iface.Name, err)
 	}
-	routes, err := getRoutesOnInterface(iface.Index)
+	rts, err := RouteListFiltered(antreasyscall.AF_UNSPEC, &Route{LinkIndex: iface.Index}, RT_FILTER_IF)
 	if err != nil {
 		return nil, nil, nil, fmt.Errorf("failed to get routes for interface index %d: %v", iface.Index, err)
 	}
+	var routes []interface{}
+	for _, rt := range rts {
+		// Skip the routes automatically generated by Windows host when adding IP address on the network adapter.
+		if rt.GatewayAddress != nil && rt.GatewayAddress.IsUnspecified() {
+			continue
+		}
+		routes = append(routes, rt)
+	}
 	return iface, addrs, routes, nil
 }
 
@@ -1156,24 +1169,6 @@ func enableOVSExtension() error {
 	return nil
 }
 
-func getRoutesOnInterface(linkIndex int) ([]interface{}, error) {
-	rts, err := listRoutes(antreasyscall.AF_UNSPEC, func(row antreasyscall.MibIPForwardRow) bool {
-		return row.Index == uint32(linkIndex)
-	})
-	if err != nil {
-		return nil, fmt.Errorf("failed to get routes: %v", err)
-	}
-	var routes []interface{}
-	for _, r := range rts {
-		// Skip the routes automatically generated by Windows host when adding IP address on the network adapter.
-		if r.GatewayAddress != nil && r.GatewayAddress.IsUnspecified() {
-			continue
-		}
-		routes = append(routes, r)
-	}
-	return routes, nil
-}
-
 // parseOVSExtensionOutput parses the VM extension output
 // and returns the value of Enabled field
 func parseOVSExtensionOutput(s string) bool {

pkg/agent/util/net_windows_test.go

@@ -623,17 +623,6 @@ func TestReplaceNetRoute(t *testing.T) {
 	}
 }
 
-func TestGetNetRoutesAll(t *testing.T) {
-	gw, subnet, _ := net.ParseCIDR("192.168.2.0/24")
-	testRow := createTestMibIPForwardRow(0, subnet, gw)
-	listRows := []antreasyscall.MibIPForwardRow{testRow}
-	wantRoutes := []Route{*routeFromIPForwardRow(&testRow)}
-	defer mockAntreaNetIO(&antreasyscalltest.MockNetIO{IPForwardRows: listRows, ListIPForwardRowsErr: nil})()
-	gotRoutes, gotErr := GetNetRoutesAll()
-	assert.Equal(t, wantRoutes, gotRoutes)
-	assert.Nil(t, gotErr)
-}
-
 func TestNewNetNat(t *testing.T) {
 	notFoundErr := fmt.Errorf("received error No MSFT_NetNat objects found")
 	testNetNat := "test-nat"
@@ -901,7 +890,7 @@ func TestGetInterfaceConfig(t *testing.T) {
 			listRows:    []antreasyscall.MibIPForwardRow{testRow},
 			listRowsErr: fmt.Errorf("unable to list IP forward rows"),
 			wantErr: fmt.Errorf("failed to get routes for interface index %d: %v", testNetInterface.Index,
-				fmt.Errorf("failed to get routes: unable to list Windows IPForward rows: unable to list IP forward rows")),
+				fmt.Errorf("unable to list Windows IPForward rows: unable to list IP forward rows")),
 		},
 	}
 	for _, tc := range tests {