Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix error with iptables-nft. #229

Merged
merged 1 commit into from
Nov 21, 2023

Conversation

axel7born
Copy link
Contributor

IPv4NativeRoutingCIDR = "0.0.0.0/0" leads to an invalid iptables rule that never matches. In case of iptables-nft this leads to an error. The intention of choosing "0.0.0.0/0" was to have a CIDR that includes both pod and nodes range. However, cilium takes care not to masquerade traffic to node addresses. So it's save to choose the pod range for IPv4NativeRoutingCIDR. As a result, the iptables masquerade rules, that are added by the sidecars are not needed anymore. SnatOutOfCluster and SnatToUpstreamDNS can be switched off as default.

default nat rules without overlay before the change:

Chain CILIUM_POST_nat (1 references)
 pkts bytes target     prot opt in     out     source               destination
  235 22730 MASQUERADE  all  --  *      !cilium_+  100.80.0.0/12        10.181.0.2           /* cilium masquerade non-cluster */
  277 16620 ACCEPT     all  --  *      *       100.80.0.0/24        0.0.0.0/0            match-set cilium_node_set_v4 dst /* exclude traffic to cluster nodes from masquerade */ <In this case destination is an ipset containing all node ips>
  459 27646 MASQUERADE  all  --  *      !cilium_+  100.80.0.0/24       !0.0.0.0/0        /* cilium masquerade non-cluster */  -< This rules is not excepted by iptables-nft !0.0.0.0/0 would never match. 0.0.0.0/0 is configured via IPv4NativeRoutingCIDR>
    0     0 ACCEPT     all  --  *      *       0.0.0.0/0            0.0.0.0/0            mark match 0xa00/0xe00 /* exclude proxy return traffic from masquerade */
    0     0 SNAT       all  --  *      cilium_host  127.0.0.1            0.0.0.0/0            /* cilium host->cluster from 127.0.0.1 masquerade */ to:100.80.0.101
    0     0 SNAT       all  --  *      cilium_host  0.0.0.0/0            0.0.0.0/0            mark match 0xf00/0xf00 ctstate DNAT /* hairpin traffic that originated from a local pod */ to:100.80.0.101
 3758  278K RETURN     all  --  *      *       100.80.0.0/12        100.80.0.0/12
    0     0 RETURN     all  --  *      *       100.80.0.0/12        10.181.0.0/16
    0     0 MASQUERADE  all  --  *      !cilium_+  100.80.0.0/12        0.0.0.0/0            /* cilium masquerade non-cluster */

Default nat rules with this change:

Chain CILIUM_POST_nat (1 references)
 pkts bytes target     prot opt in     out     source               destination
  277 16620 ACCEPT     all  --  *      *       100.80.0.0/24        0.0.0.0/0            match-set cilium_node_set_v4 dst /* exclude traffic to cluster nodes from masquerade */ <In this case destination is an ipset containing all node ips. Everything coming from pods going to nodes is not masqueraded>
  459 27646 MASQUERADE  all  --  *      !cilium_+  100.80.0.0/24       !100.80.0.0/12        /* cilium masquerade non-cluster */  < Here we jump to MASQERADE for everything not leaving a cilium interface and not going to a cidr in the pod range. Here we would also jump to masquerade for the DNS IP.>
    0     0 ACCEPT     all  --  *      *       0.0.0.0/0            0.0.0.0/0            mark match 0xa00/0xe00 /* exclude proxy return traffic from masquerade */
    0     0 SNAT       all  --  *      cilium_host  127.0.0.1            0.0.0.0/0            /* cilium host->cluster from 127.0.0.1 masquerade */ to:100.80.0.101
    0     0 SNAT       all  --  *      cilium_host  0.0.0.0/0            0.0.0.0/0            mark match 0xf00/0xf00 ctstate DNAT /* hairpin traffic that originated from a local pod */ to:100.80.0.101

How to categorize this PR?

/area networking
/kind bug

What this PR does / why we need it:

Which issue(s) this PR fixes:
Fixes #

Special notes for your reviewer:

Release note:

Fixes an error that occurs when running with iptables-nft.

IPv4NativeRoutingCIDR = "0.0.0.0/0" leads to an invalid iptables rule that never matches. In case of iptables-nft this leads to an error.
The intention of choosing "0.0.0.0/0" was to have a CIDR that includes both pod and nodes range.
However, cilium takes care not to masquerade traffic to node addresses.
So it's save to choose the pod range for IPv4NativeRoutingCIDR.
As a result, our own iptables masquerade rules are not needed anymore. SnatOutOfCluster and SnatToUpstreamDNS can be switched off as default.
@axel7born axel7born requested review from a team as code owners November 20, 2023 14:43
@gardener-robot gardener-robot added area/networking Networking related kind/bug Bug needs/review Needs review size/xs Size of pull request is tiny (see gardener-robot robot/bots/size.py) labels Nov 20, 2023
@gardener-robot-ci-1 gardener-robot-ci-1 added the reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) label Nov 20, 2023
@gardener-robot-ci-2 gardener-robot-ci-2 added needs/ok-to-test Needs approval for testing (check PR in detail before setting this label because PR is run on CI/CD) and removed reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) labels Nov 20, 2023
Copy link
Member

@DockToFuture DockToFuture left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

/lgtm

@gardener-robot gardener-robot added reviewed/lgtm Has approval for merging and removed needs/review Needs review labels Nov 20, 2023
@gardener-robot-ci-3 gardener-robot-ci-3 added the reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) label Nov 20, 2023
@axel7born axel7born merged commit adf63e9 into gardener:master Nov 21, 2023
1 check passed
@gardener-robot gardener-robot added the status/closed Issue is closed (either delivered or triaged) label Nov 21, 2023
@axel7born axel7born deleted the iptables-nat-rules branch November 21, 2023 09:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/networking Networking related kind/bug Bug needs/ok-to-test Needs approval for testing (check PR in detail before setting this label because PR is run on CI/CD) reviewed/lgtm Has approval for merging reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) size/xs Size of pull request is tiny (see gardener-robot robot/bots/size.py) status/closed Issue is closed (either delivered or triaged)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants