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 Pod-to-external traffic on EKS in policyOnly mode #3975

Merged
merged 1 commit into from
Jul 13, 2022

Conversation

antoninbas
Copy link
Contributor

@antoninbas antoninbas commented Jul 7, 2022

When using Antrea in policyOnly mode on an EKS cluster, an additional
iptables rule is needed in the PREROUTING chain of the nat table. The
rule ensures that Pod-to-external traffic coming from Pods whose IP
address comes from a secondary network interface (secondary ENI) is
marked correctly, so that it hits the appropriate routing table. Without
this, traffic is SNATed with the source IP address of the primary
network interface, while being sent out of the secondary network
interface, causing the VPC to drop the traffic.

Relevant rules (before the fix):

-A PREROUTING -m comment --comment "kubernetes service portals" -j KUBE-SERVICES
-A PREROUTING -i eni+ -m comment --comment "AWS, outbound connections" -m state --state NEW -j AWS-CONNMARK-CHAIN-0
-A PREROUTING -m comment --comment "AWS, CONNMARK" -j CONNMARK --restore-mark --nfmask 0x80 --ctmask 0x80
-A OUTPUT -m comment --comment "kubernetes service portals" -j KUBE-SERVICES
-A POSTROUTING -m comment --comment "kubernetes postrouting rules" -j KUBE-POSTROUTING
-A POSTROUTING -m comment --comment "AWS SNAT CHAIN" -j AWS-SNAT-CHAIN-0
-A POSTROUTING -m comment --comment "Antrea: jump to Antrea postrouting rules" -j ANTREA-POSTROUTING
-A ANTREA-POSTROUTING -o antrea-gw0 -m comment --comment "Antrea: masquerade LOCAL traffic" -m addrtype ! --src-type LOCAL --limit-iface-out -m addrtype --src-type LOCAL -j MASQUERADE --random-fully
-A AWS-CONNMARK-CHAIN-0 ! -d 192.168.0.0/16 -m comment --comment "AWS CONNMARK CHAIN, VPC CIDR" -j AWS-CONNMARK-CHAIN-1
-A AWS-CONNMARK-CHAIN-1 -m comment --comment "AWS, CONNMARK" -j CONNMARK --set-xmark 0x80/0x80
-A AWS-SNAT-CHAIN-0 ! -d 192.168.0.0/16 -m comment --comment "AWS SNAT CHAIN" -j AWS-SNAT-CHAIN-1
-A AWS-SNAT-CHAIN-1 ! -o vlan+ -m comment --comment "AWS, SNAT" -m addrtype ! --dst-type LOCAL -j SNAT --to-source 192.168.18.153 --random-fully

0:	from all lookup local
512:	from all to 192.168.29.56 lookup main
512:	from all to 192.168.24.134 lookup main
512:	from all to 192.168.31.135 lookup main
512:	from all to 192.168.31.223 lookup main
512:	from all to 192.168.29.27 lookup main
512:	from all to 192.168.16.158 lookup main
512:	from all to 192.168.2.135 lookup main
1024:	from all fwmark 0x80/0x80 lookup main
1536:	from 192.168.31.223 lookup 2
1536:	from 192.168.29.27 lookup 2
1536:	from 192.168.16.158 lookup 2
1536:	from 192.168.2.135 lookup 2
32766:	from all lookup main
32767:	from all lookup default

default via 192.168.0.1 dev eth1
192.168.0.1 dev eth1 scope link

The fix is to add new PREROUTING rules, in the ANTREA-PREROUTING chain:

-A ANTREA-PREROUTING -i antrea-gw0 -m comment --comment "Antrea: AWS, outbound connections" -j AWS-CONNMARK-CHAIN-0
-A ANTREA-PREROUTING -m comment --comment "Antrea: AWS, CONNMARK (first packet)" -j CONNMARK --restore-mark --nfmask 0x80 --ctmask 0x80

Fixes #3946

Signed-off-by: Antonin Bas abas@vmware.com

@antoninbas antoninbas requested review from tnqn and jianjuns July 7, 2022 23:32
@antoninbas antoninbas added area/provider/aws Issues or PRs related to aws provider. kind/bug Categorizes issue or PR as related to a bug. action/release-note Indicates a PR that should be included in release notes. action/backport Indicates a PR that requires backports. labels Jul 7, 2022
@codecov-commenter
Copy link

codecov-commenter commented Jul 7, 2022

Codecov Report

Merging #3975 (200197a) into main (f022dd8) will increase coverage by 3.22%.
The diff coverage is 37.93%.

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #3975      +/-   ##
==========================================
+ Coverage   61.65%   64.88%   +3.22%     
==========================================
  Files         295      295              
  Lines       43740    44115     +375     
==========================================
+ Hits        26968    28623    +1655     
+ Misses      14538    13192    -1346     
- Partials     2234     2300      +66     
Flag Coverage Δ
e2e-tests 41.19% <25.86%> (?)
kind-e2e-tests 51.45% <37.93%> (+6.66%) ⬆️
unit-tests 44.31% <0.00%> (+0.02%) ⬆️
Impacted Files Coverage Δ
pkg/agent/route/route_linux.go 48.30% <36.84%> (+20.20%) ⬆️
pkg/agent/util/iptables/iptables.go 43.52% <100.00%> (ø)
pkg/agent/nodeportlocal/k8s/annotations.go 84.44% <0.00%> (-15.56%) ⬇️
pkg/apiserver/handlers/endpoint/handler.go 56.52% <0.00%> (-13.05%) ⬇️
pkg/agent/controller/networkpolicy/reconciler.go 69.26% <0.00%> (+0.36%) ⬆️
pkg/agent/controller/egress/egress_controller.go 73.93% <0.00%> (+0.51%) ⬆️
pkg/ipam/ipallocator/allocator.go 88.02% <0.00%> (+0.52%) ⬆️
pkg/controller/ipam/validate.go 80.32% <0.00%> (+0.54%) ⬆️
...ntroller/networkpolicy/networkpolicy_controller.go 73.29% <0.00%> (+0.60%) ⬆️
pkg/agent/multicluster/mc_route_controller.go 52.27% <0.00%> (+1.13%) ⬆️
... and 53 more

pkg/agent/route/route_linux.go Outdated Show resolved Hide resolved
@antoninbas antoninbas force-pushed the eks-fix-pod-to-external-traffic branch from 688808f to ee5c0c7 Compare July 8, 2022 23:38
@antoninbas antoninbas requested a review from tnqn July 8, 2022 23:41
When using Antrea in policyOnly mode on an EKS cluster, an additional
iptables rule is needed in the PREROUTING chain of the nat table. The
rule ensures that Pod-to-external traffic coming from Pods whose IP
address comes from a secondary network interface (secondary ENI) is
marked correctly, so that it hits the appropriate routing table. Without
this, traffic is SNATed with the source IP address of the primary
network interface, while being sent out of the secondary network
interface, causing the VPC to drop the traffic.

The fix is to add new PREROUTING rules, in the ANTREA-PREROUTING chain:

```
-A ANTREA-PREROUTING -i antrea-gw0 -m comment --comment "Antrea: AWS, outbound connections" -j AWS-CONNMARK-CHAIN-0
-A ANTREA-PREROUTING -m comment --comment "Antrea: AWS, CONNMARK (first packet)" -j CONNMARK --restore-mark --nfmask 0x80 --ctmask 0x80
```

Fixes antrea-io#3946

Signed-off-by: Antonin Bas <abas@vmware.com>
@antoninbas antoninbas force-pushed the eks-fix-pod-to-external-traffic branch from ee5c0c7 to 200197a Compare July 8, 2022 23:42
Copy link
Member

@tnqn tnqn left a comment

Choose a reason for hiding this comment

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

LGTM, one question

c.writeEKSMangleRule(iptablesData)
// When Antrea is used to enforce NetworkPolicies in EKS, additional iptables
// mangle rules are required. See https://github.com/antrea-io/antrea/issues/678.
// These rules are only needed for IPv4.
Copy link
Member

Choose a reason for hiding this comment

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

Does it have NodePort IPv6 case? If yes, is the rule still needed for IPv6, otherwise reverse path may be different?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yes, I think you're right. This one should always be installed.
Note that we don't test Antrea on EKS with IPv6 enabled, so there may be other issues, but I'll fix that one.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Actually scratch that. The AWS rules are only installed for IPv4: https://github.com/aws/amazon-vpc-cni-k8s/blob/d43309bdfdb5034df86907944e682d78608ba165/pkg/networkutils/network.go#L402-L418. Not entirely sure why, but I would think that routing is configured differently for IPv6, and these rules are not needed.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

There is this comment:

		//Essentially a stub function for now in V6 mode. We will need it when we support v6 in secondary IP and
		//custom networking modes. We don't need to install any SNAT rules in v6 mode and currently there is no need
		//to mark packets entering via Primary ENI as all the pods in v6 mode will be behind primary ENI. Will have to
		//start doing that once we start supporting custom networking mode in v6.

So that explains why there is no need for the rule: only the primary ENI is used, with no secondary route table.

Copy link
Member

Choose a reason for hiding this comment

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

got it, thanks

Comment on lines +383 to +385
// While we do not have access to these environment variables, we could
// look for existing rules installed by the AWS VPC CNI, and determine
// whether we need to install this rule.
Copy link
Member

Choose a reason for hiding this comment

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

Is this still true? I didn't find this code.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is part of the TODO paragraph. We don't implement this as the moment, we assume that aws-node (the AWS DaemonSet) is running with default configuration, which is likely to be the case for 99% of users.

@antoninbas
Copy link
Contributor Author

@tnqn feel free to merge this if you're ok with the change. The EKS CI test is passing for this PR.

@antoninbas
Copy link
Contributor Author

/test-all

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
action/backport Indicates a PR that requires backports. action/release-note Indicates a PR that should be included in release notes. area/provider/aws Issues or PRs related to aws provider. kind/bug Categorizes issue or PR as related to a bug.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

EKS node randomly stop applying netpolicies to pods
3 participants