Skip to content

bpf: minor SNAT improvements #35531

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 3 commits into from
Dec 2, 2024
Merged

bpf: minor SNAT improvements #35531

merged 3 commits into from
Dec 2, 2024

Conversation

julianwiedmann
Copy link
Member

@julianwiedmann julianwiedmann commented Oct 25, 2024 •
edited
Loading

While looking for something else, I realized that the processing for replies in the Masquerading path is unnecessarily complicated. Clean it up.

@julianwiedmann julianwiedmann added area/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/snat Relates to SNAT or Masquerading of traffic labels Oct 25, 2024
@julianwiedmann
Copy link
Member Author

/test

@julianwiedmann julianwiedmann changed the title 1.17 bpf snat bpf: minor SNAT improvements Oct 25, 2024
@julianwiedmann julianwiedmann marked this pull request as ready for review October 25, 2024 06:32
@julianwiedmann julianwiedmann requested a review from a team as a code owner October 25, 2024 06:32
@julianwiedmann julianwiedmann force-pushed the 1.17-bpf-snat branch 2 times, most recently from aa06712 to c67edb6 Compare October 30, 2024 07:53
gentoo-root
gentoo-root approved these changes Nov 29, 2024
View reviewed changes
Copy link
Contributor

@gentoo-root gentoo-root left a comment

Choose a reason for hiding this comment

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

Looks good to me, sorry for taking too long to review it.

@julianwiedmann
bpf: nat: return early from snat_v*_needs_masquerade() for reply traffic
b160190 
Looking at all the subsequent return paths, we always return
NAT_PUNT_TO_STACK for packets where `is_reply == true`. Simplify things
a bit by returning this value straight away after doing the CT lookup.

Signed-off-by: Julian Wiedmann <jwi@isovalent.com>
@julianwiedmann
bpf: nat: don't require ipcache hit for IP Masq agent lookup
f3b30a6 
The lookup in the IP_MASQ_AGENT_IPV* map doesn't require any information
from the remote_endpoint. Make it unconditional. In practice this doesn't
make a difference, as we expect to always hit the WORLD_ID fallback entry
in the ipcache.

While at it also move the ipcache lookup down to where the first user is.

Signed-off-by: Julian Wiedmann <jwi@isovalent.com>
@julianwiedmann
bpf: nat: remove unused egress_gw_policy variable
7723080 
Cleanup an unused variable.

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

/test

@julianwiedmann julianwiedmann added this pull request to the merge queue Dec 2, 2024
@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 Dec 2, 2024
Merged via the queue into cilium:main with commit 92875dc Dec 2, 2024
64 checks passed
@julianwiedmann julianwiedmann deleted the 1.17-bpf-snat branch December 2, 2024 08:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@gentoo-root gentoo-root gentoo-root approved these changes

Assignees
No one assigned
Labels
area/datapath Impacts bpf/ or low-level forwarding details, including map management and monitor messages. feature/snat Relates to SNAT or Masquerading of traffic 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.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@julianwiedmann @gentoo-root