fqdn: serialize requests per-name #30109
Merged
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
squeed
added
kind/bug
|
Commit 8984191 does not match "(?m)^Signed-off-by:". Please follow instructions provided in https://docs.cilium.io/en/stable/contributing/development/contributing_guide/#developer-s-certificate-of-origin |
maintainer-s-little-helper
bot
added
the
dont-merge/needs-sign-off
|
Commit 8984191 does not match "(?m)^Signed-off-by:". Please follow instructions provided in https://docs.cilium.io/en/stable/contributing/development/contributing_guide/#developer-s-certificate-of-origin |
squeed
removed
the
dont-merge/needs-sign-off
This re-creates similar behavior as in v1.14 - FQDN requests are serialized on a per-name basis. This was removed as part of the larger fqdn refactor but, it turns out, that was premature. The difference between v1.14 and now is that this locks per-name, rather than per-IP. It preserves the prime-sized count of bucketed locks. This is needed because we release the lock on the NameManager before policy updates have been fully pushed out to all endpoints. If another endpoint requests the same name, it may have the DNS response returned to it before policy updates are complete. However, the NameManager will detect the noop case, take a shortcut, and release the DNS response before policy updates are potentially complete. For precise details, see the comment in fqdn.go. Fixes: cilium#30014 Fixes: 507259f Signed-off-by: Casey Callendrello <cdc@isovalent.com>
squeed
added
the
needs-backport/1.15
|
/test |
danehans
reviewed
Jan 8, 2024
maintainer-s-little-helper
bot
added
the
ready-to-merge
jibi
added
backport-pending/1.15
giorio94
added
backport-done/1.15
maintainer-s-little-helper
bot
moved this from Needs backport from main
to Backport done to v1.15
in v1.15.0-rc.1
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 re-creates similar behavior as in v1.14 - FQDN requests are serialized on a per-name basis. This was removed as part of the larger fqdn refactor but, it turns out, that was premature. The difference between v1.14 and now is that this locks per-name, rather than per-IP. It preserves the prime-sized count of bucketed locks.
This is needed because we release the lock on the NameManager before policy updates have been fully pushed out to all endpoints. If another endpoint requests the same name, it may have the DNS response returned to it before policy updates are complete. However, the NameManager will detect the noop case, take a shortcut, and release the DNS response before policy updates are potentially complete.
For precise details, see the comment in fqdn.go.
Fixes: #30014
Fixes: 507259f