bpf: Don't fail on existing SNAT entries in __snat_create #40971
Conversation
This file contains hidden or 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
|
/ci-e2e-upgrade |
|
/test |
fbea40c to
143b7d7
Compare
snat_v4_new_mapping creates a RevSNAT entry, then a SNAT entry for new connections. However, we also have a fallback mechanism in snat_v4_rev_nat_handle_mapping, which restores the SNAT entry if it was evicted by LRU, but the original RevSNAT entry still exists. There is a hypothesis that the above two functions may race with each other, and the following is possible: 1. snat_v4_new_mapping creates a RevSNAT entry. 2. snat_v4_rev_nat_handle_mapping finds the RevSNAT entry and "restores" the SNAT entry (which has never existed yet). 3. snat_v4_new_mapping goes on to create the SNAT entry, corresponding to the RevSNAT entry, created previously. 4. snat_v4_new_mapping fails because the SNAT entry already exists, created by snat_v4_rev_nat_handle_mapping. 5. snat_v4_new_mapping then removes the RevSNAT entry, trying to revert everything. The right outcome in this case would be to keep the SNAT entry, whoever created it first, so it makes sense not to fail on -EEXIST. It also makes the fallback mechanism more robust, because there is a time gap between __snat_lookup and __snat_create. Ref: #35304 Ref: #37747 Fixes: #38466 Signed-off-by: Maxim Mikityanskiy <maxim@isovalent.com>
Fixes: ac5198f ("bpf: Add unit tests for SNAT port allocation") Signed-off-by: Maxim Mikityanskiy <maxim@isovalent.com>
143b7d7 to
c19f24f
Compare
|
/test |
YutaroHayakawa
approved these changes
Aug 12, 2025
|
@gentoo-root Thanks for the follow-up on #37747. This makes the SNAT LRU fallback path more robust and addresses the race around |
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.
snat_v4_new_mapping creates a RevSNAT entry, then a SNAT entry for new connections. However, we also have a fallback mechanism in snat_v4_rev_nat_handle_mapping, which restores the SNAT entry if it was evicted by LRU, but the original RevSNAT entry still exists.
There is a hypothesis that the above two functions may race with each other, and the following is possible:
The right outcome in this case would be to keep the SNAT entry, whoever created it first, so it makes sense not to fail on -EEXIST.
It also makes the fallback mechanism more robust, because there is a time gap between __snat_lookup and __snat_create.
Ref: #35304
Ref: #37747
Fixes: #38466