Skip to content

Conversation

jrajahalme
Copy link
Member

@jrajahalme jrajahalme commented May 22, 2020

Proxy port for DNS becomes unallocated if the proxy port reference count reaches zero. While this should never happen for a DNS proxy port, as it starts with a reference count of 1, it would be better make sure the DNS proxy port is never reallocated as the DNS proxy can't change its listening port.

Fixes: #11637

Check if proxy port has already been configured in reservePort(). This
is a cleanup only, should not change existing behavior.

Signed-off-by: Jarno Rajahalme <jarno@covalent.io>
Make proxy port changes visible in logs at info level. This should not
add too much logging as proxy ports are supposedly relatively static
over time.

Signed-off-by: Jarno Rajahalme <jarno@covalent.io>
Due to an apparent reference counting problem, where DNS redirect
count reaches zero even though the reference count is set to one by
SetProxyPort(), is is possible for the DNS proxy listening port to be
reallocated and the corresponding datapath redirection rules changed
to a new port, while the DNS proxy is incapable of changing it's
listening port. Fix this by marking a proxy post set via
SetProxyPort() as static and adding corresponding conditions that
prevent the release and reallocation of the proxy port even if the
reference count reaches zero.

Fixes: 11637
Signed-off-by: Jarno Rajahalme <jarno@covalent.io>
@jrajahalme jrajahalme added kind/bug This is a bug in the Cilium logic. area/proxy Impacts proxy components, including DNS, Kafka, Envoy and/or XDS servers. release-note/bug This PR fixes an issue in a previous release of Cilium. labels May 22, 2020
@jrajahalme jrajahalme requested a review from a team as a code owner May 22, 2020 21:13
@jrajahalme
Copy link
Member Author

test-me-please

@jrajahalme
Copy link
Member Author

test-me-please

@@ -259,14 +266,14 @@ func (p *Proxy) ackProxyPort(pp *ProxyPort) error {

// Remove old rules, if any and for different port
if pp.rulesPort != 0 && pp.rulesPort != pp.proxyPort {
scopedLog.Debugf("Removing old proxy port rules for %s:%d", pp.name, pp.rulesPort)
scopedLog.Infof("Removing old proxy port rules for %s:%d", pp.name, pp.rulesPort)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Are we planing to keep this as Info?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd like to, for a while at least. It should not be chatty at all.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/proxy Impacts proxy components, including DNS, Kafka, Envoy and/or XDS servers. kind/bug This is a bug in the Cilium logic. release-note/bug This PR fixes an issue in a previous release of Cilium.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

FQDN proxy stops forwarding traffic
4 participants