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

egressgw: minor bpf refactors #32094

Merged
merged 3 commits into from
Apr 24, 2024

Conversation

julianwiedmann
Copy link
Member

@julianwiedmann julianwiedmann commented Apr 19, 2024

This slightly refactors some of the EGW bpf code, so that we have low-level EGW helpers that are only concerned about the map lookup & interpreting the result. High-level logic (ie. EGW only handles outbound connections) gets applied independently of what the policy mechanism looks like, and error propagation is more straight-forward.

…rect()

Cleanly split up the EgressGW policy lookup, and the subsequent ipcache
lookup.

No functional change.

Signed-off-by: Julian Wiedmann <jwi@isovalent.com>
@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. labels Apr 19, 2024
@julianwiedmann
Copy link
Member Author

/test

Introduce a slightly better separation between
1. interpreting the CT result,
2. performing the actual EGW policy lookup & evaluation,
3. determining whether the selected gateway is local or remote.

Also allow the policy lookup to return a DROP_* reason, without
intermediate transport via the tunnel_endpoint variable.

Signed-off-by: Julian Wiedmann <jwi@isovalent.com>
EGW is incompatible with IPsec, and so __encap_and_redirect_lxc() will
never return CTX_ACT_OK. We currently also *can't* pass the packet to the
stack for encryption, as there's no code in bpf_host that would catch it
and handle the EGW redirect to the gateway node.

Therefore remove the unused & confusing code.

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

/test

@julianwiedmann julianwiedmann added the dont-merge/preview-only Only for preview or testing, don't merge it. label Apr 21, 2024
@julianwiedmann julianwiedmann changed the title egressgw: refactor egressgw: minor bpf refactors Apr 21, 2024
@julianwiedmann julianwiedmann marked this pull request as ready for review April 21, 2024 14:16
@julianwiedmann julianwiedmann requested review from a team as code owners April 21, 2024 14:17
Copy link
Member

@ysksuzuki ysksuzuki left a comment

Choose a reason for hiding this comment

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

LGTM! Thanks

@julianwiedmann julianwiedmann removed the dont-merge/preview-only Only for preview or testing, don't merge it. label Apr 23, 2024
@julianwiedmann julianwiedmann added this pull request to the merge queue Apr 24, 2024
@julianwiedmann julianwiedmann removed the request for review from gentoo-root April 24, 2024 10:24
Merged via the queue into cilium:main with commit fcf8f79 Apr 24, 2024
62 checks passed
@julianwiedmann julianwiedmann deleted the 1.16-bpf-egressgw-refactor branch April 24, 2024 10:31
@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 Apr 24, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature/egress-gateway Impacts the egress IP gateway feature. ready-to-merge This PR has passed all tests and received consensus from code owners to merge. 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

2 participants