Skip to content

Commit 6bdaeb3

Browse files
HadrienPattepippolo84
authored andcommitted
iptables: Compare the SNAT exclusion CIDR as a netip.Prefix
`snatDstExclusionCIDR` was passed around as a string, so both consumers re-parsed it to a `*net.IPNet` with `net.ParseCIDR` once per route in `installMasqueradeRouteSourceRules`. Pass the existing `netip.Prefix` instead and compare values. Signed-off-by: Hadrien Patte <hadrien.patte@datadoghq.com>
1 parent 1e9f94f commit 6bdaeb3

2 files changed

Lines changed: 29 additions & 21 deletions

File tree

pkg/datapath/iptables/iptables.go

Lines changed: 27 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,6 @@ import (
88
"context"
99
"fmt"
1010
"log/slog"
11-
"net"
1211
"net/netip"
1312
"os"
1413
"regexp"
@@ -22,11 +21,11 @@ import (
2221
"github.com/cilium/statedb"
2322
"github.com/mattn/go-shellwords"
2423
"github.com/vishvananda/netlink"
24+
"go4.org/netipx"
2525
"k8s.io/utils/clock"
2626

2727
"github.com/cilium/cilium/daemon/cmd/cni"
2828
"github.com/cilium/cilium/pkg/byteorder"
29-
"github.com/cilium/cilium/pkg/cidr"
3029
"github.com/cilium/cilium/pkg/command/exec"
3130
"github.com/cilium/cilium/pkg/container/set"
3231
"github.com/cilium/cilium/pkg/datapath/iptables/ipset"
@@ -1484,22 +1483,24 @@ func (m *manager) installForwardChainRulesIpX(prog runnable, ifName, localDelive
14841483
return nil
14851484
}
14861485

1486+
// isDefaultRoutePrefix reports whether p is the default route of its family,
1487+
// i.e. 0.0.0.0/0 or ::/0. The zero Prefix is not a default route.
1488+
func isDefaultRoutePrefix(p netip.Prefix) bool {
1489+
return p.Addr().IsUnspecified() && p.Bits() == 0
1490+
}
1491+
14871492
func (m *manager) installMasqueradeRules(
14881493
prog iptablesInterface, nativeDevices []string,
1489-
localDeliveryInterface, snatDstExclusionCIDR, allocRange, hostMasqueradeIP string,
1494+
localDeliveryInterface string, snatDstExclusionCIDR netip.Prefix,
1495+
allocRange, hostMasqueradeIP string,
14901496
) error {
14911497
devices := nativeDevices
14921498

1493-
if prog.getMode() == "nft" {
1494-
if _, exclusionCIDR, err := net.ParseCIDR(snatDstExclusionCIDR); err == nil {
1495-
maskSize, _ := exclusionCIDR.Mask.Size()
1496-
if exclusionCIDR.IP.IsUnspecified() && maskSize == 0 {
1497-
if prog == m.ip6tables {
1498-
return fmt.Errorf("nf_tables does not support ::/0 exclusion, set --%s=false", option.EnableIPv6Masquerade)
1499-
}
1500-
return fmt.Errorf("nf_tables does not support 0.0.0.0/0 exclusion, set --%s=false", option.EnableIPv4Masquerade)
1501-
}
1499+
if prog.getMode() == "nft" && isDefaultRoutePrefix(snatDstExclusionCIDR) {
1500+
if prog == m.ip6tables {
1501+
return fmt.Errorf("nf_tables does not support ::/0 exclusion, set --%s=false", option.EnableIPv6Masquerade)
15021502
}
1503+
return fmt.Errorf("nf_tables does not support 0.0.0.0/0 exclusion, set --%s=false", option.EnableIPv4Masquerade)
15031504
}
15041505

15051506
if m.sharedCfg.NodeIpsetNeeded {
@@ -1544,7 +1545,7 @@ func (m *manager) installMasqueradeRules(
15441545
// range
15451546
// * Non-tunnel mode:
15461547
// * May not be targeted to an IP in the cluster range
1547-
cmds := allEgressMasqueradeCmds(allocRange, snatDstExclusionCIDR, m.sharedCfg.MasqueradeInterfaces,
1548+
cmds := allEgressMasqueradeCmds(allocRange, snatDstExclusionCIDR.String(), m.sharedCfg.MasqueradeInterfaces,
15481549
m.cfg.IPTablesRandomFully)
15491550
for _, cmd := range cmds {
15501551
if err := prog.runProg(cmd); err != nil {
@@ -1644,7 +1645,7 @@ func (m *manager) installMasqueradeRules(
16441645

16451646
func (m *manager) installMasqueradeRouteSourceRules(
16461647
prog runnable, routes []netlink.Route, linkByIndex func(int) (netlink.Link, error),
1647-
devices []string, snatDstExclusionCIDR, allocRange string,
1648+
devices []string, snatDstExclusionCIDR netip.Prefix, allocRange string,
16481649
) error {
16491650
slices.SortFunc(routes, func(a, b netlink.Route) int {
16501651
aPfx, bPfx := 0, 0
@@ -1687,19 +1688,25 @@ func (m *manager) installMasqueradeRouteSourceRules(
16871688
// -o device.
16881689
match = true
16891690
}
1690-
_, exclusionCIDR, err := net.ParseCIDR(snatDstExclusionCIDR)
1691-
if !match || r.Src == nil || (err == nil && cidr.Equal(r.Dst, exclusionCIDR)) {
1691+
// dst is the zero Prefix for a route without a destination (the
1692+
// kernel reports the default route that way), which never compares
1693+
// equal to a valid exclusion CIDR and is not a default-route prefix.
1694+
var dst netip.Prefix
1695+
if r.Dst != nil {
1696+
dst, _ = netipx.FromStdIPNet(r.Dst)
1697+
}
1698+
if !match || r.Src == nil || (dst.IsValid() && dst == snatDstExclusionCIDR) {
16921699
continue
16931700
}
16941701
progArgs := []string{
16951702
"-t", "nat",
16961703
"-A", ciliumPostNatChain,
16971704
"-s", allocRange,
16981705
}
1699-
if cidr.Equal(r.Dst, cidr.ZeroNet(r.Family)) {
1706+
if isDefaultRoutePrefix(dst) {
17001707
progArgs = append(
17011708
progArgs,
1702-
"!", "-d", snatDstExclusionCIDR)
1709+
"!", "-d", snatDstExclusionCIDR.String())
17031710
} else {
17041711
progArgs = append(
17051712
progArgs,
@@ -1853,7 +1860,7 @@ func (m *manager) installRules(state desiredState) error {
18531860

18541861
if m.sharedCfg.IptablesMasqueradingIPv4Enabled && state.localNodeInfo.internalIPv4.IsValid() {
18551862
if err := m.installMasqueradeRules(m.ip4tables, state.devices.UnsortedList(), localDeliveryInterface,
1856-
m.remoteSNATDstAddrExclusionCIDR(state.localNodeInfo.ipv4NativeRoutingCIDR, state.localNodeInfo.ipv4AllocCIDR).String(),
1863+
m.remoteSNATDstAddrExclusionCIDR(state.localNodeInfo.ipv4NativeRoutingCIDR, state.localNodeInfo.ipv4AllocCIDR),
18571864
state.localNodeInfo.ipv4AllocCIDR.String(),
18581865
state.localNodeInfo.internalIPv4.String(),
18591866
); err != nil {
@@ -1869,7 +1876,7 @@ func (m *manager) installRules(state desiredState) error {
18691876

18701877
if m.sharedCfg.IptablesMasqueradingIPv6Enabled && state.localNodeInfo.internalIPv6.IsValid() {
18711878
if err := m.installMasqueradeRules(m.ip6tables, state.devices.UnsortedList(), localDeliveryInterface,
1872-
m.remoteSNATDstAddrExclusionCIDR(state.localNodeInfo.ipv6NativeRoutingCIDR, state.localNodeInfo.ipv6AllocCIDR).String(),
1879+
m.remoteSNATDstAddrExclusionCIDR(state.localNodeInfo.ipv6NativeRoutingCIDR, state.localNodeInfo.ipv6AllocCIDR),
18731880
state.localNodeInfo.ipv6AllocCIDR.String(),
18741881
state.localNodeInfo.internalIPv6.String(),
18751882
); err != nil {

pkg/datapath/iptables/iptables_test.go

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ package iptables
66
import (
77
"fmt"
88
"net"
9+
"net/netip"
910
"strings"
1011
"testing"
1112

@@ -1289,7 +1290,7 @@ func TestInstallMasqueradeRouteSourceRules(t *testing.T) {
12891290
mgr := &manager{}
12901291
err := mgr.installMasqueradeRouteSourceRules(
12911292
mockProg, routes, linkByIndex,
1292-
[]string{"eth0"}, "11.0.0.0/24", "11.0.0.0/24",
1293+
[]string{"eth0"}, netip.MustParsePrefix("11.0.0.0/24"), "11.0.0.0/24",
12931294
)
12941295
require.NoError(t, err)
12951296
require.NoError(t, mockProg.checkExpectations())

0 commit comments

Comments
 (0)