-
Notifications
You must be signed in to change notification settings - Fork 3.4k
bpf: avoid race when selecting the RevSNAT port #33115
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
Conversation
The logic to allocate SNAT mapping contains a race condition. At a high level it does the following: if (!revsnat_exists(port)) { if (!create_revsnat(port) return error; ... } Two concurrent executions of the datapath may succeed the revsnat_exists check, which then leads to one of them bailing out since create_revsnat fails. Instead simply try to create the RevSNAT entry. If that fails we retry with another port. Signed-off-by: Lorenz Bauer <lmb@isovalent.com>
/test |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you, nice catch!
I slightly word-smithed the release note - but imho this is even a release-note/bug
, and we should queue up a backport?
/test |
I had a look, this is present even in the first version 0e785f9 which appeared in 1.5.0. There were a couple of changes in between, so the backport would have to be manual. I'll leave it up to you to decide - maybe it makes sense to see whether this helps the user with their symptoms? |
Ack, sounds good. Let's release-note it as bug, but hold off on backports until we see if it helps. |
The logic to allocate SNAT mapping contains a race condition. At a high level it does the following:
Two concurrent executions of the datapath may succeed the revsnat_exists check, which then leads to one of them bailing out since create_revsnat fails.
Instead simply try to create the RevSNAT entry. If that fails we retry with another port.
Updates: #33074