Skip to content

Commit

Permalink
datapath: Silent iptables removal on first init
Browse files Browse the repository at this point in the history
On the first initialization, the iptables rules may not exist. Remove
the silently and warn on any attempt to remove rules once they are known
to exist.

Also, don't remove iptables rules on additional calls to Reinitialize()
unless iptables is enabled.

Fixes: #11809

Signed-off-by: Thomas Graf <thomas@cilium.io>
  • Loading branch information
tgraf authored and aanm committed Jun 2, 2020
1 parent e57ae1c commit 601852b
Show file tree
Hide file tree
Showing 4 changed files with 15 additions and 6 deletions.
2 changes: 1 addition & 1 deletion pkg/datapath/fake/datapath.go
Expand Up @@ -80,7 +80,7 @@ func (f *fakeDatapath) SupportsOriginalSourceAddr() bool {
return false
}

func (f *fakeDatapath) RemoveRules() {}
func (f *fakeDatapath) RemoveRules(quiet bool) {}

func (f *fakeDatapath) InstallRules(ifName string) error {
return nil
Expand Down
4 changes: 2 additions & 2 deletions pkg/datapath/iptables/iptables.go
Expand Up @@ -434,7 +434,7 @@ func (m *IptablesManager) SupportsOriginalSourceAddr() bool {
}

// RemoveRules removes iptables rules installed by Cilium.
func (m *IptablesManager) RemoveRules() {
func (m *IptablesManager) RemoveRules(quiet bool) {
// Set of tables that have had iptables rules in any Cilium version
tables := []string{"nat", "mangle", "raw", "filter"}
for _, t := range tables {
Expand All @@ -450,7 +450,7 @@ func (m *IptablesManager) RemoveRules() {
}

for _, c := range ciliumChains {
c.remove(m.waitArgs, false)
c.remove(m.waitArgs, quiet)
}
}

Expand Down
2 changes: 1 addition & 1 deletion pkg/datapath/loader.go
Expand Up @@ -77,7 +77,7 @@ type IptablesManager interface {
// use of original source addresses in proxy upstream
// connections.
SupportsOriginalSourceAddr() bool
RemoveRules()
RemoveRules(quiet bool)
InstallRules(ifName string) error
TransientRulesStart(ifName string) error
TransientRulesEnd(quiet bool)
Expand Down
13 changes: 11 additions & 2 deletions pkg/datapath/loader/base.go
Expand Up @@ -66,6 +66,10 @@ const (
initArgMax
)

// firstInitialization is true when Reinitialize() is called for the first
// time. It can only be accessed when GetCompilationLock() is being held.
var firstInitialization = true

func (l *Loader) writeNetdevHeader(dir string, o datapath.BaseProgramOwner) error {
headerPath := filepath.Join(dir, common.NetdevHeaderFileName)
log.WithField(logfields.Path, headerPath).Debug("writing configuration")
Expand Down Expand Up @@ -130,6 +134,7 @@ func (l *Loader) Reinitialize(ctx context.Context, o datapath.BaseProgramOwner,
// Lock so that endpoints cannot be built while we are compile base programs.
o.GetCompilationLock().Lock()
defer o.GetCompilationLock().Unlock()
defer func() { firstInitialization = false }()

l.init(o.Datapath(), o.LocalConfig())

Expand Down Expand Up @@ -344,8 +349,12 @@ func (l *Loader) Reinitialize(ctx context.Context, o datapath.BaseProgramOwner,
return err
}
}
// Always remove masquerade rule and then re-add it if required
iptMgr.RemoveRules()
// The iptables rules are only removed on the first initialization to
// remove stale rules or when iptables is enabled. The first invocation
// is silent as rules may not exist.
if firstInitialization || option.Config.InstallIptRules {
iptMgr.RemoveRules(firstInitialization)
}
if option.Config.InstallIptRules {
err := iptMgr.InstallRules(option.Config.HostDevice)
iptMgr.TransientRulesEnd(false)
Expand Down

0 comments on commit 601852b

Please sign in to comment.