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
iptables: Fix wrong use of podCIDR in cluster node NAT exclusion #26397
iptables: Fix wrong use of podCIDR in cluster node NAT exclusion #26397
Conversation
By default, in the iptables-based masquerading mode, Cilium will only masquerade traffic coming from the local pod CIDR (`allocRange` in `installMasqueradeRules`). However, many IPAM modes such as ENI or multi-pool IPAM do not have a single pod CIDR. Instead, those modes rely on the `egress-masquerade-interfaces` setting, which masquerades all traffic if it leaves one of the `egress-masquerade-interfaces` devices. Therefore, the "exclude traffic to cluster nodes from masquerade" `CILIUM_POST_nat` rule should also respect the `egress-masquerade-interfaces` setting and not masquerade traffic regardless of the value of `allocRange` (which will not be valid in settings such as ENI mode). This likely has not manifested in ENI mode as an issue, because in ENI mode we derive the native routing CIDR (`snatDstExclusionCIDR` in `installMasqueradeRules`) from the EC2 VPC CIDR, which usually contains the node IPs too. However, we should not rely on that, since we are adding additional non-podCIDR based IPAM modes such as multi-pool where this will not be true. Related: cilium#22273 Signed-off-by: Sebastian Wicki <sebastian@isovalent.com>
/test |
ci-ginko failed with LVH provisioning errors https://github.com/cilium/cilium/actions/runs/5334904372/jobs/9667395901 https://github.com/cilium/cilium/actions/runs/5334904372/jobs/9667397270 Restarting |
Louis says he does not have time to review |
Marking ready to merge. |
With cilium#26397 merged, iptables-based masquerading can now be used together with Multi-Pool IPAM, as long as `egressMasqueradeInterfaces` is set too. This commit adjusts the documentation to reflect that and improves the wording of that section a bit. Signed-off-by: Sebastian Wicki <sebastian@isovalent.com>
With #26397 merged, iptables-based masquerading can now be used together with Multi-Pool IPAM, as long as `egressMasqueradeInterfaces` is set too. This commit adjusts the documentation to reflect that and improves the wording of that section a bit. Signed-off-by: Sebastian Wicki <sebastian@isovalent.com>
[ upstream commit 970f881 ] With cilium#26397 merged, iptables-based masquerading can now be used together with Multi-Pool IPAM, as long as `egressMasqueradeInterfaces` is set too. This commit adjusts the documentation to reflect that and improves the wording of that section a bit. Signed-off-by: Sebastian Wicki <sebastian@isovalent.com> Signed-off-by: Jussi Maki <jussi@isovalent.com>
[ upstream commit 970f881 ] With #26397 merged, iptables-based masquerading can now be used together with Multi-Pool IPAM, as long as `egressMasqueradeInterfaces` is set too. This commit adjusts the documentation to reflect that and improves the wording of that section a bit. Signed-off-by: Sebastian Wicki <sebastian@isovalent.com> Signed-off-by: Jussi Maki <jussi@isovalent.com>
[ upstream commit 970f881 ] With cilium#26397 merged, iptables-based masquerading can now be used together with Multi-Pool IPAM, as long as `egressMasqueradeInterfaces` is set too. This commit adjusts the documentation to reflect that and improves the wording of that section a bit. Signed-off-by: Sebastian Wicki <sebastian@isovalent.com> Signed-off-by: Jussi Maki <jussi@isovalent.com> Signed-off-by: Gilberto Bertin <jibi@cilium.io>
By default, in the iptables-based masquerading mode, Cilium will only masquerade traffic coming from the local pod CIDR (
allocRange
ininstallMasqueradeRules
). However, many IPAM modes such as ENI or multi-pool IPAM do not have a single pod CIDR. Instead, those modes rely on theegress-masquerade-interfaces
setting, which only masquerades traffic if it leaves one of theegress-masquerade-interfaces
devices.Therefore, the "exclude traffic to cluster nodes from masquerade"
CILIUM_POST_nat
rule should also respect theegress-masquerade-interfaces
setting and not masquerade traffic regardless of the value ofallocRange
(which will not be valid in settings such as ENI mode).This likely has not manifested in ENI mode as an issue, because in ENI mode we derive the native routing CIDR (
snatDstExclusionCIDR
ininstallMasqueradeRules
) from the EC2 VPC CIDR, which usually contains the node IPs too. However, we should not rely on that, since we are adding additional non-podCIDR based IPAM modes such as multi-pool where this will not be true.Related: #22273
Open question: Should this be backported to v1.13? I'm not aware of any actual bugs this has caused so far in ENI mode (see above), but we should fix it nonetheless. I'd particularly would like to have this in v1.14, since it will fix iptables-based masquerading for the newly added Multi-Pool IPAM mode in v1.14.