-
Notifications
You must be signed in to change notification settings - Fork 12
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
Only process traffic impacted by network policies #39
Conversation
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: aojea The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
0bfd312
to
0390d5e
Compare
f2b8372
to
6bfb5e7
Compare
/assign @danwinship Dan please take a look |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This looks like it will work (though you lose all the optimization once you add even a single ANP)...
Another possibility would be to just add "local detector" options like kube-proxy (--cluster-cidr
, use node podCIDRs
, etc)
if networkPolicy == nil { | ||
return nil | ||
} | ||
podSelector, err := metav1.LabelSelectorAsSelector(&networkPolicy.Spec.PodSelector) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So FTR, with the existing code, if you have Pod A on Node 1 that denies all ingress, and Pod B on Node 2 tries to send to it, then Node 2 will drop the packets. With this new code, Node 2 would instead forward the packet to Node 1, and then Node 1 would drop it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yeah, is the trade off, but at least the rules are only evaluated once in one node, previously allowed rules were evaluated twice, one in each node
Cluster Wide Policies are hard to implement in the dataplane
Since the controller already has the pod information this looks simpler, kube-proxy does not watch pods and need to depend on those heuristics |
Instead of sending all traffic to user space, only process the traffic that is impacted by network policies. If admin network policies are enabled then we process all traffic.
It would still be better for it to be a command-line option, even if it's required. Required environment variables are terrible. |
I just want to fix the flake, let me open an issue to not forget and do the change |
optimize the datapath not having to send all packets to user space, only the ones that are subject of network policies
Fixes: #10, #31, #12