bpf: nodeport: consolidate packet rewrite in RevDNAT path #26852
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
julianwiedmann
added
sig/datapath
|
/test |
julianwiedmann
force-pushed
the
1.15-bpf-dsr-revdnat
branch
from
026953b
to
e9dfed5
Compare
|
/test |
julianwiedmann
changed the title
|
#26638 needs a few more cycles until it's ready for primetime. But this part is good on its own already, so let's get started on review. |
aspsk
approved these changes
Jul 17, 2023
bpf/lib/conntrack.h
Outdated
| return true; | ||
| } | ||
|
|
||
| if (check_dsr && entry->dsr) |
There was a problem hiding this comment.
Maybe just return check_dsr && entry->dsr; ?
bpf/lib/conntrack.h
Outdated
| if (check_dsr && entry->dsr) | ||
| return true; | ||
|
|
||
| return false; |
There was a problem hiding this comment.
The code is identical with the v4 case. Maybe you can wrap the error checking and saving the rev_nat_index in an inline function?
There was a problem hiding this comment.
I like it 👍 See the pushed diff - two birds, one stone.
maintainer-s-little-helper
bot
added
ready-to-merge
julianwiedmann
force-pushed
the
1.15-bpf-dsr-revdnat
branch
from
e9dfed5
to
cdb2548
Compare
|
/test |
julianwiedmann
removed
the
ready-to-merge
In a following patch, we need easy access to the RevDNAT info for replies by a local backend (from the LB*_REVERSE_NAT_MAP) and by a DSR backend (from the SNAT_MAPPING_IPV* map). For this we slightly rework the existing ct_has_nodeport_egress_entry*() helpers, so that they return the rev_nat_index for non-DSR entries. Then add a nodeport_rev_dnat_get_info_ipv*() helper that wraps the map lookups for both cases, and returns the info as lb*_reverse_nat struct. No functional change intended, the new helpers are not used yet. Signed-off-by: Julian Wiedmann <jwi@isovalent.com>
Align with lb6_rev_nat() and just pass in the needed parameters from the ct_state struct. This enables a subsequent patch that wants to call __lb4_rev_nat() without a ct_state struct. Signed-off-by: Julian Wiedmann <jwi@isovalent.com>
The RevDNAT path currently uses __lb*_rev_nat() for rewriting replies by local backends, and snat_v*_rewrite_egress() for rewriting DSR replies. Both end up doing pretty much the same thing (we currently don't need the ICMP support in the snat_* version), so switch the DSR path to also use __lb*_rev_nat(). For this we employ the new nodeport_rev_dnat_get_info_*() helpers that convert the DSR SNAT entry to lb*_reverse_nat struct. Signed-off-by: Julian Wiedmann <jwi@isovalent.com>
Now that we have access to the SNAT info which was used to rewrite the inner packet, we no longer need to peek into the L3 header to obtain the new source IP. Signed-off-by: Julian Wiedmann <jwi@isovalent.com>
julianwiedmann
force-pushed
the
1.15-bpf-dsr-revdnat
branch
from
cdb2548
to
aefc0e5
Compare
|
/test |
maintainer-s-little-helper
bot
added
the
ready-to-merge
2 tasks
julianwiedmann
added
the
backport-pending/1.14
michi-covalent
added
backport-done/1.14
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This adds some smarts to the nodeport helpers, so that it's possible to split up the RevDNAT lookup (what IP/port to use for the RevDNAT) and the actual packet rewrite. This allows us to use the same helper code for the rewrite, regardless whether the reply comes from a local or a DSR backend.
It's a nice simplification on its own, but will also enable a follow-on fix in the RevDNAT path (#26638) that would otherwise be rejected by the verifier. So we'll backport this PR accordingly, once that fix is ready to merged.