cilium-cni: Reserve ports that can conflict with transparent DNS proxy #32128
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
gandro
added
area/cni
3 tasks
gandro
force-pushed
the
pr/gandro/transparent-dns-proxy-reserve-known-ports
branch
from
7a814da
to
c7afe2f
Compare
|
I did some manual testing by forcing WireGuard conflicts (port 51871) by reducing the ephemeral port range via |
|
/test |
chancez
reviewed
Apr 22, 2024
gandro
force-pushed
the
pr/gandro/transparent-dns-proxy-reserve-known-ports
branch
from
c7afe2f
to
d819ae1
Compare
|
Thanks for the feedback! I've replied inline, hope that answers some of the questions. |
|
/test |
tommyp1ckles
approved these changes
Apr 23, 2024
learnitall
approved these changes
Apr 23, 2024
chancez
approved these changes
Apr 23, 2024
gandro
removed
the
needs-backport/1.15
maintainer-s-little-helper
bot
moved this from Needs backport from main
to Backport pending to v1.15
in 1.15.5
1 task
gandro
added
backport-pending/1.13
maintainer-s-little-helper
bot
moved this from Needs backport from main
to Backport pending to v1.14
in 1.14.11
maintainer-s-little-helper
bot
moved this from Needs backport from main
to Backport pending to v1.13
in 1.13.16
1 task
gandro
added a commit
to gandro/cilium
that referenced
this pull request
May 8, 2024
This is a follow-up suggested by Julian in a previous PR: cilium#32128 (comment) Fixes: 11fe7cc ("cilium-cni: Reserve ports that can conflict with transparent DNS proxy") Note: This fix has already been applied to the backported version of the above commit. Suggested-by: Julian Wiedmann <jwi@isovalent.com> Signed-off-by: Sebastian Wicki <sebastian@isovalent.com>
github-actions
bot
added
backport-done/1.14
github-actions
bot
added
backport-done/1.13
maintainer-s-little-helper
bot
moved this from Backport pending to v1.13
to Backport done to v1.13
in 1.13.16
This was referenced May 10, 2024
github-merge-queue bot
pushed a commit
that referenced
this pull request
May 14, 2024
This is a follow-up suggested by Julian in a previous PR: #32128 (comment) Fixes: 11fe7cc ("cilium-cni: Reserve ports that can conflict with transparent DNS proxy") Note: This fix has already been applied to the backported version of the above commit. Suggested-by: Julian Wiedmann <jwi@isovalent.com> Signed-off-by: Sebastian Wicki <sebastian@isovalent.com>
jshr-w
pushed a commit
to jshr-w/cilium
that referenced
this pull request
May 16, 2024
This is a follow-up suggested by Julian in a previous PR: cilium#32128 (comment) Fixes: 11fe7cc ("cilium-cni: Reserve ports that can conflict with transparent DNS proxy") Note: This fix has already been applied to the backported version of the above commit. Suggested-by: Julian Wiedmann <jwi@isovalent.com> Signed-off-by: Sebastian Wicki <sebastian@isovalent.com>
jrajahalme
reviewed
May 24, 2024
| @@ -604,13 +628,12 @@ func (cmd *Cmd) Add(args *skel.CmdArgs) (err error) { | |||
|
|
|||
| if err = ns.Do(func() error { | |||
| if ipv6IsEnabled(ipam) { | |||
There was a problem hiding this comment.
Looks like local IP ports are reserved only if IPv6 is enabled?
julianwiedmann
added
the
area/proxy
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 commit adds the ability for the cilium-cni plugin to reserve IP ports (via the
ip_local_reserved_portssysctl knob) during CNI ADD. Reserving these ports will prevent the container network namespace to use these ports as an ephemeral source port, while still allowing the port to be explicitly allocated (see [1]).This functionality is added as a workaround for #31535 where transparent DNS proxy mode causes conflicts when an ephemeral source port is being chosen that is already being used in the host network namespace. By reserving ports used by Cilium itself (such as WireGuard and VXLAN), we hopefully reduce the number of such conflicts.
The set of reserved ports can be configured via a newly introduced agent flag. By default, it will reserve an auto-generated set of ports. The list of ports is configurable such that users running custom UDP services on ports in the ephemeral port range can provide their own set of ports. The flag may be set to an empty string to disable reservations altogether.
[1] https://www.kernel.org/doc/Documentation/networking/ip-sysctl.txt