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

bpf: xdp: use CT tuple hash for tunnel encap's source port #26177

Merged
merged 1 commit into from Jun 16, 2023

Conversation

julianwiedmann
Copy link
Member

@julianwiedmann julianwiedmann commented Jun 13, 2023

(follow-on to #24422)

hash_from_tuple_v*() isn't perfect (as it ignores the .daddr), but much better than what we currently have.

The one path where we might notice the reduced entropy is in a config with EgressGW and native-routing, when processing EgressGW reply traffic with non-service protocol (ICMP or other exotic types). As here we currently don't extract any L4 ports either.

@julianwiedmann julianwiedmann added sig/datapath Impacts bpf/ or low-level forwarding details, including map management and monitor messages. release-note/misc This PR makes changes that have no direct user impact. feature/egress-gateway Impacts the egress IP gateway feature. area/loadbalancing Impacts load-balancing and Kubernetes service implementations labels Jun 13, 2023
@julianwiedmann
Copy link
Member Author

/test

@julianwiedmann
Copy link
Member Author

/test

hash_from_tuple_v*() isn't perfect (as it ignores the .daddr), but much
better than what we currently have.

The one path where we might notice the reduced entropy is in a config with
EgressGW and native-routing, when processing EgressGW reply traffic with
non-service protocol (ICMP or other exotic types). As here we currently
don't extract any L4 ports either.

Signed-off-by: Julian Wiedmann <jwi@isovalent.com>
@julianwiedmann
Copy link
Member Author

Dropped the manual update of the CT tuple after RevDNAT. REV_NAT_F_TUPLE_SADDR should already do that for us :amaze:.

@julianwiedmann
Copy link
Member Author

/test

@julianwiedmann julianwiedmann marked this pull request as ready for review June 14, 2023 13:55
@julianwiedmann julianwiedmann requested a review from a team as a code owner June 14, 2023 13:55
@julianwiedmann julianwiedmann added the release-blocker/1.14 This issue will prevent the release of the next version of Cilium. label Jun 14, 2023
@julianwiedmann
Copy link
Member Author

(marking as blocker so it doesn't fall through the cracks)

@julianwiedmann julianwiedmann added the kind/enhancement This would improve or streamline existing functionality. label Jun 14, 2023
Copy link
Contributor

@ldelossa ldelossa left a comment

Choose a reason for hiding this comment

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

LGTM.

@maintainer-s-little-helper maintainer-s-little-helper bot added the ready-to-merge This PR has passed all tests and received consensus from code owners to merge. label Jun 15, 2023
@joestringer
Copy link
Member

The one path where we might notice the reduced entropy is in a config with EgressGW and native-routing, when processing EgressGW reply traffic with non-service protocol (ICMP or other exotic types). As here we currently don't extract any L4 ports either.

Does this need further discussion before we merge? cc @jibi

@jibi jibi merged commit 6175e23 into cilium:main Jun 16, 2023
65 checks passed
@julianwiedmann julianwiedmann deleted the 1.14-xdp-tunnel-source-port branch June 16, 2023 10:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/loadbalancing Impacts load-balancing and Kubernetes service implementations feature/egress-gateway Impacts the egress IP gateway feature. kind/enhancement This would improve or streamline existing functionality. ready-to-merge This PR has passed all tests and received consensus from code owners to merge. release-blocker/1.14 This issue will prevent the release of the next version of Cilium. release-note/misc This PR makes changes that have no direct user impact. sig/datapath Impacts bpf/ or low-level forwarding details, including map management and monitor messages.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants