Skip to content

Commit

Permalink
proxy: Never release proxy port for non-dynamic listeners
Browse files Browse the repository at this point in the history
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>
  • Loading branch information
jrajahalme authored and aanm committed May 29, 2020
1 parent da022e8 commit af9fd59
Showing 1 changed file with 13 additions and 3 deletions.
16 changes: 13 additions & 3 deletions pkg/proxy/proxy.go
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,11 @@ type DatapathUpdater interface {
type ProxyPort struct {
// Listener name (immutable)
name string
// isStatic is true when the listener on the proxy port is incapable
// of stopping and/or being reconfigured with a new proxy port once it has been
// first started. Set 'true' by SetProxyPort(), which is only called for
// static listeners (currently only DNS proxy).
isStatic bool
// parser type this port applies to (immutable)
parserType policy.L7ParserType
// 'true' for ingress, 'false' for egress (immutable)
Expand Down Expand Up @@ -293,6 +298,9 @@ func (p *Proxy) releaseProxyPort(name string) error {

pp.nRedirects--
if pp.nRedirects == 0 {
if pp.isStatic {
return fmt.Errorf("Can't release proxy port: proxy %s on %d has a static listener", name, pp.proxyPort)
}
delete(allocatedPorts, pp.proxyPort)
// Force new port allocation the next time this ProxyPort is used.
pp.configured = false
Expand Down Expand Up @@ -348,7 +356,8 @@ func GetProxyPort(l7Type policy.L7ParserType, ingress bool) (uint16, string, err

// SetProxyPort() marks the proxy 'name' as successfully created with proxy port 'port' and creates
// or updates the datapath rules accordingly.
// May only be called once per proxy.
// This should only be called for proxies that have a static listener that is already listening on
// 'port'. May only be called once per proxy.
func (p *Proxy) SetProxyPort(name string, port uint16) error {
proxyPortsMutex.Lock()
defer proxyPortsMutex.Unlock()
Expand All @@ -360,8 +369,9 @@ func (p *Proxy) SetProxyPort(name string, port uint16) error {
return fmt.Errorf("Can't set proxy port to %d: proxy %s is already configured on %d", port, name, pp.proxyPort)
}
pp.proxyPort = port
pp.configured = true
return p.ackProxyPort(pp)
pp.isStatic = true // prevents release of the proxy port
pp.reservePort() // marks 'port' as reserved, 'pp' as configured
return p.ackProxyPort(pp) // creates datapath rules, increases the reference count
}

// ReinstallRules is called by daemon reconfiguration to re-install proxy ports rules that
Expand Down

0 comments on commit af9fd59

Please sign in to comment.