egressgw: improve installation of IP rules

route.ReplaceRule() internally fetches the whole set of IP rules in the system. So calling addEgressIpRule() for every EgressGW-eligible endpoint causes quite a bit of churn. Instead fetch the rules just once per EgressGW policy (filtered for the policy's routing table). Then check for each of the policy's endpoints whether its IP rule already exists, and insert any rule that is missing. Note that there is further potential for improvement here - ideally we would fetch the whole rule set just once, dynamically filter it down to each policy's routing table, and only match the policy's endpoints against those specific rules. Signed-off-by: Julian Wiedmann <jwi@isovalent.com>
cilium · Jul 17, 2023 · fabbc5d · fabbc5d
1 parent d5ee4e7
commit fabbc5d
Show file tree

Hide file tree

Showing 2 changed files with 78 additions and 38 deletions.
diff --git a/pkg/egressgateway/manager.go b/pkg/egressgateway/manager.go
@@ -656,45 +656,63 @@ func (manager *Manager) addMissingIpRulesAndRoutes(isRetry bool) (shouldRetry bo
 		return false
 	}
 
-	addIPRulesForConfig := func(endpointIP net.IP, dstCIDR *net.IPNet, gwc *gatewayConfig) {
-		logger := log.WithFields(logrus.Fields{
-			logfields.SourceIP:        endpointIP,
-			logfields.DestinationCIDR: dstCIDR.String(),
-			logfields.EgressIP:        gwc.egressIP.IP,
-			logfields.LinkIndex:       gwc.ifaceIndex,
-		})
+	for _, policyConfig := range manager.policyConfigs {
+		gwc := &policyConfig.gatewayConfig
 
-		if err := addEgressIpRule(endpointIP, dstCIDR, gwc.egressIP.IP, gwc.ifaceIndex); err != nil {
-			if isRetry {
-				logger.WithError(err).Warn("Can't add IP rule")
-			} else {
-				logger.WithError(err).Debug("Can't add IP rule, will retry")
-				shouldRetry = true
-			}
-		} else {
-			logger.Debug("Added IP rule")
+		if !gwc.localNodeConfiguredAsGateway || len(policyConfig.matchedEndpoints) == 0 {
+			continue
 		}
-	}
 
-	for _, policyConfig := range manager.policyConfigs {
-		gwc := &policyConfig.gatewayConfig
+		logger := log.WithFields(logrus.Fields{
+			logfields.EgressIP:  gwc.egressIP.IP,
+			logfields.LinkIndex: gwc.ifaceIndex,
+		})
 
-		if gwc.localNodeConfiguredAsGateway &&
-			len(policyConfig.matchedEndpoints) > 0 {
+		routingTableIdx := egressGatewayRoutingTableIdx(gwc.ifaceIndex)
 
-			policyConfig.forEachEndpointAndDestination(addIPRulesForConfig)
+		ipRules, err := listEgressIpRulesForRoutingTable(routingTableIdx)
+		if err != nil {
+			logger.WithError(err).Warn("Can't fetch IP rules")
+			continue
+		}
 
+		addIPRulesForConfig := func(endpointIP net.IP, dstCIDR *net.IPNet, gwc *gatewayConfig) {
 			logger := log.WithFields(logrus.Fields{
-				logfields.EgressIP:  gwc.egressIP.IP,
-				logfields.LinkIndex: gwc.ifaceIndex,
+				logfields.SourceIP:        endpointIP,
+				logfields.DestinationCIDR: dstCIDR.String(),
+				logfields.EgressIP:        gwc.egressIP.IP,
+				logfields.LinkIndex:       gwc.ifaceIndex,
 			})
 
-			if err := addEgressIpRoutes(gwc.egressIP, gwc.ifaceIndex); err != nil {
-				logger.WithError(err).Warn("Can't add IP routes")
+			// check if the corresponding IP rule already exists
+			for _, ipRule := range ipRules {
+				if egressIPRuleMatches(&ipRule, endpointIP, dstCIDR) {
+					return
+				}
+			}
+
+			// insert the missing rule
+			newRule := newEgressIpRule(endpointIP, dstCIDR, routingTableIdx)
+
+			if err := netlink.RuleAdd(newRule); err != nil {
+				if isRetry {
+					logger.WithError(err).Warn("Can't add IP rule")
+				} else {
+					logger.WithError(err).Debug("Can't add IP rule, will retry")
+					shouldRetry = true
+				}
 			} else {
-				logger.Debug("Added IP routes")
+				logger.Debug("Added IP rule")
 			}
 		}
+
+		policyConfig.forEachEndpointAndDestination(addIPRulesForConfig)
+
+		if err := addEgressIpRoutes(gwc.egressIP, gwc.ifaceIndex); err != nil {
+			logger.WithError(err).Warn("Can't add IP routes")
+		} else {
+			logger.Debug("Added IP routes")
+		}
 	}
 
 	return

diff --git a/pkg/egressgateway/net.go b/pkg/egressgateway/net.go
@@ -76,7 +76,20 @@ func listEgressIpRules() ([]netlink.Rule, error) {
 		Priority: linux_defaults.RulePriorityEgressGateway,
 	}
 
-	rules, err := route.ListRules(netlink.FAMILY_V4, &filter)
+	return listFilteredEgressIpRules(&filter)
+}
+
+func listEgressIpRulesForRoutingTable(RoutingTableIdx int) ([]netlink.Rule, error) {
+	filter := route.Rule{
+		Priority: linux_defaults.RulePriorityEgressGateway,
+		Table:    RoutingTableIdx,
+	}
+
+	return listFilteredEgressIpRules(&filter)
+}
+
+func listFilteredEgressIpRules(filter *route.Rule) ([]netlink.Rule, error) {
+	rules, err := route.ListRules(netlink.FAMILY_V4, filter)
 	if err != nil {
 		return nil, err
 	}
@@ -90,18 +103,27 @@ func listEgressIpRules() ([]netlink.Rule, error) {
 	return rules, nil
 }
 
-func addEgressIpRule(endpointIP net.IP, dstCIDR *net.IPNet, egressIP net.IP, ifaceIndex int) error {
-	routingTableIdx := egressGatewayRoutingTableIdx(ifaceIndex)
+func newEgressIpRule(endpointIP net.IP, dstCIDR *net.IPNet, routingTableIdx int) *netlink.Rule {
+	rule := netlink.NewRule()
 
-	ipRule := route.Rule{
-		Priority: linux_defaults.RulePriorityEgressGateway,
-		From:     &net.IPNet{IP: endpointIP, Mask: net.CIDRMask(32, 32)},
-		To:       dstCIDR,
-		Table:    routingTableIdx,
-		Protocol: linux_defaults.RTProto,
-	}
+	rule.Family = netlink.FAMILY_V4
+	rule.Table = routingTableIdx
+	rule.Priority = linux_defaults.RulePriorityEgressGateway
+	rule.Src = &net.IPNet{IP: endpointIP, Mask: net.CIDRMask(32, 32)}
+	rule.Dst = dstCIDR
+	rule.Protocol = linux_defaults.RTProto
+
+	return rule
+}
+
+func egressIPRuleMatches(ipRule *netlink.Rule, endpointIP net.IP, dstCIDR *net.IPNet) bool {
+	ipRuleMaskSize, _ := ipRule.Dst.Mask.Size()
+	dstCIDRMaskSize, _ := dstCIDR.Mask.Size()
 
-	return route.ReplaceRule(ipRule)
+	// We already filtered for .Family, .Priority and .Table
+	return ipRule.Src.IP.Equal(endpointIP) &&
+		ipRuleMaskSize == dstCIDRMaskSize &&
+		ipRule.Dst.IP.Equal(dstCIDR.IP)
 }
 
 func getFirstIPInHostRange(ip net.IPNet) net.IP {