Skip to content

Commit 08f07c7

Browse files
committed
fix: add proper nil rule src handling
When ip rules are evaluated in the netlink library, default routes for src and dst are equated to nil. This makes it difficult to evaluate them and requires additional handling in order for them. I filed an issue upstream so that this could potentially get fixed: vishvananda/netlink#1080 however if it doesn't get resolved, this should allow us to move forward.
1 parent 060e4a7 commit 08f07c7

File tree

3 files changed

+44
-5
lines changed

3 files changed

+44
-5
lines changed

pkg/controllers/proxy/linux_networking.go

Lines changed: 15 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -570,14 +570,27 @@ func (ln *linuxNetworking) setupRoutesForExternalIPForDSR(serviceInfoMap service
570570
nRule.Src = defaultPrefixCIDR
571571
nRule.Table = externalIPRouteTableID
572572

573+
// It would be better if we could filter by src, but it's not actually set by iproute2 when netlink receives it
574+
// back. Instead, a rule.Src that is set to 0.0.0.0/0 or ::/0 will come back as nil. So if we filter by src, we
575+
// will not find any rules.
573576
rules, err := netlink.RuleListFiltered(nFamily, nRule,
574-
netlink.RT_FILTER_TABLE|netlink.RT_FILTER_SRC|netlink.RT_FILTER_PRIORITY)
577+
netlink.RT_FILTER_TABLE|netlink.RT_FILTER_PRIORITY)
575578
if err != nil {
576579
return fmt.Errorf("failed to list rule for external IP's and verify if `ip rule add prio 32765 from all "+
577580
"lookup external_ip` exists due to: %v", err)
578581
}
579582

580-
if len(rules) < 1 {
583+
klog.V(2).Infof("rules found: %d", len(rules))
584+
defaultRuleFound := false
585+
for _, rule := range rules {
586+
klog.V(2).Infof("rule: %+v", rule)
587+
// If the rule.Src is nil, it means that the rule is a default route rule (0.0.0.0/0 or ::/0)
588+
if rule.Src == nil {
589+
defaultRuleFound = true
590+
}
591+
}
592+
593+
if !defaultRuleFound {
581594
err = netlink.RuleAdd(nRule)
582595
if err != nil {
583596
klog.Infof("Failed to add policy rule (equivalent to `ip rule add prio %d from %s lookup "+

pkg/controllers/proxy/network_services_controller.go

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1752,7 +1752,8 @@ func routeVIPTrafficToDirector(fwmark string, family v1.IPFamily) error {
17521752
nRule.Table = customDSRRouteTableID
17531753
nRule.Priority = defaultTrafficDirectorRulePriority
17541754

1755-
routes, err := netlink.RuleListFiltered(nFamily, nRule, netlink.RT_FILTER_MARK|netlink.RT_FILTER_TABLE)
1755+
routes, err := netlink.RuleListFiltered(nFamily, nRule,
1756+
netlink.RT_FILTER_MARK|netlink.RT_FILTER_TABLE|netlink.RT_FILTER_PRIORITY)
17561757
if err != nil {
17571758
return fmt.Errorf("failed to verify if `ip rule` exists due to: %v", err)
17581759
}

pkg/routes/pbr.go

Lines changed: 27 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -51,9 +51,34 @@ func ipRuleAbstraction(ipFamily int, ipOp int, cidr string) error {
5151
nRule.Src = nSrc
5252
nRule.Table = CustomTableID
5353

54-
rules, err := netlink.RuleListFiltered(ipFamily, nRule, netlink.RT_FILTER_SRC)
54+
// If the rule that we are abstracting has either the src or dst set to a default route, then we need to handle it
55+
// differently. For more information, see: https://github.com/vishvananda/netlink/issues/1080
56+
// TODO: If the above issue is resolved, some of the below logic can be removed
57+
rules := make([]netlink.Rule, 0)
58+
isDefaultRoute, err := utils.IsDefaultRoute(nSrc)
5559
if err != nil {
56-
return fmt.Errorf("failed to list rules: %s", err.Error())
60+
return fmt.Errorf("failed to check if CIDR is a default route: %v", err)
61+
}
62+
63+
if isDefaultRoute {
64+
var tmpRules []netlink.Rule
65+
tmpRules, err = netlink.RuleListFiltered(ipFamily, nRule, netlink.RT_FILTER_TABLE)
66+
if err != nil {
67+
return fmt.Errorf("failed to list rules: %s", err.Error())
68+
}
69+
70+
// Check if one or more of the rules returned are a default route rule
71+
for _, rule := range tmpRules {
72+
// If the rule has no src, then it is a default route rule which is the match criteria for the rule
73+
if rule.Src == nil {
74+
rules = append(rules, rule)
75+
}
76+
}
77+
} else {
78+
rules, err = netlink.RuleListFiltered(ipFamily, nRule, netlink.RT_FILTER_SRC|netlink.RT_FILTER_TABLE)
79+
if err != nil {
80+
return fmt.Errorf("failed to list rules: %s", err.Error())
81+
}
5782
}
5883

5984
if ipOp == PBRRuleDel && len(rules) > 0 {

0 commit comments

Comments
 (0)