fqdn proxy: fix data race by using separate sessionUDPFactories #28163
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
mhofstetter
added
kind/bug
|
/test |
mhofstetter
force-pushed
the
pr/mhofstetter/fix-fqdnproxy-data-race
branch
from
f2ea482
to
c222929
Compare
|
/test |
mhofstetter
force-pushed
the
pr/mhofstetter/fix-fqdnproxy-data-race
branch
4 times, most recently
from
26642d8
to
b4f42da
Compare
mhofstetter
added
the
needs-backport/1.14
|
/test |
jrajahalme
reviewed
Sep 15, 2023
PR cilium#25309 introduced a data race by sharing the sessionUDPFactory between the DNS server instances for the different IP families (IPv4 & IPv6). This has been detected by cilium#27979. This commit fixes the data race by using dedicated instances of the sessionUDPFactory. Fixes: cilium#28156 Signed-off-by: Marco Hofstetter <marco.hofstetter@isovalent.com>
This commit replaces the global response UDP connections (IPv4 & IPv6) variables with fields in the SessionUDPFactory. This way the connections can already be setup during construction of the factory and the `sync.Once`s (global vars) can be removed. Signed-off-by: Marco Hofstetter <marco.hofstetter@isovalent.com>
mhofstetter
force-pushed
the
pr/mhofstetter/fix-fqdnproxy-data-race
branch
from
b4f42da
to
e906486
Compare
Historically, the `socketUDPFactory` and the lilsten configuration supported IPv4 and IPv6 on the same socket. Since the change to bind the DNS servers to localhost, a server and socket is created per ip family. Therefore this commit removes the support for multiple ipfamilies on the same socket (SocketUDPFactory & listen configuration). This improves readability quite a lot. Signed-off-by: Marco Hofstetter <marco.hofstetter@isovalent.com>
mhofstetter
force-pushed
the
pr/mhofstetter/fix-fqdnproxy-data-race
branch
2 times, most recently
from
4eb30a8
to
761ef40
Compare
jrajahalme
approved these changes
Sep 15, 2023
mhofstetter
force-pushed
the
pr/mhofstetter/fix-fqdnproxy-data-race
branch
from
761ef40
to
0770a29
Compare
|
/test |
pippolo84
reviewed
Sep 18, 2023
maintainer-s-little-helper
bot
added
the
ready-to-merge
maintainer-s-little-helper
bot
added
the
ready-to-merge
This commit adds the transparent socket options to the IPFamily struct. This way it can be used in the respective functions. Signed-off-by: Marco Hofstetter <marco.hofstetter@isovalent.com>
mhofstetter
force-pushed
the
pr/mhofstetter/fix-fqdnproxy-data-race
branch
from
0770a29
to
cd014c8
Compare
Thanks, i adapted the error messages |
mhofstetter
removed
ready-to-merge
|
/test |
maintainer-s-little-helper
bot
added
the
ready-to-merge
giorio94
added
backport-pending/1.14
aanm
added
backport-done/1.14
jrajahalme
moved this from Needs backport from main
to Backport done to v1.14
in 1.14.3
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.
PR #25309 introduced a data race by sharing the sessionUDPFactory between the DNS server instances for the different IP families (IPv4 & IPv6). This has been detected by #27979.
This commit fixes the data race by using dedicated instances of the sessionUDPFactory.
In addition the global response UDP connections (IPv4 & IPv6) variables have been replaced with fields in the
SessionUDPFactory. This way the connections can already be setup during construction of the factory and thesync.Onces (global vars) can be removed.Fixes: #28156