From 719f60bb914e209d4c15f11e75bfaa58198913a9 Mon Sep 17 00:00:00 2001 From: Phil Sutter Date: Wed, 5 Jul 2023 22:14:38 +0200 Subject: [PATCH 1/2] utils: iptables: Use go-iptables' ChainExists() Starting with v0.5.0, go-iptables exports a fast ChainExists() which does not rely upon listing all chains and searching the results but probes chain existence by listing its first rule. This should make a significant difference in rulesets with thousands of chains. Signed-off-by: Phil Sutter --- pkg/utils/iptables.go | 22 ++-------------------- plugins/meta/portmap/chain.go | 2 +- 2 files changed, 3 insertions(+), 21 deletions(-) diff --git a/pkg/utils/iptables.go b/pkg/utils/iptables.go index 6aec8bca1..b83e6d26c 100644 --- a/pkg/utils/iptables.go +++ b/pkg/utils/iptables.go @@ -29,9 +29,9 @@ func EnsureChain(ipt *iptables.IPTables, table, chain string) error { if ipt == nil { return errors.New("failed to ensure iptable chain: IPTables was nil") } - exists, err := ChainExists(ipt, table, chain) + exists, err := ipt.ChainExists(table, chain) if err != nil { - return fmt.Errorf("failed to list iptables chains: %v", err) + return fmt.Errorf("failed to check iptables chain existence: %v", err) } if !exists { err = ipt.NewChain(table, chain) @@ -45,24 +45,6 @@ func EnsureChain(ipt *iptables.IPTables, table, chain string) error { return nil } -// ChainExists checks whether an iptables chain exists. -func ChainExists(ipt *iptables.IPTables, table, chain string) (bool, error) { - if ipt == nil { - return false, errors.New("failed to check iptable chain: IPTables was nil") - } - chains, err := ipt.ListChains(table) - if err != nil { - return false, err - } - - for _, ch := range chains { - if ch == chain { - return true, nil - } - } - return false, nil -} - // DeleteRule idempotently delete the iptables rule in the specified table/chain. // It does not return an error if the referring chain doesn't exist func DeleteRule(ipt *iptables.IPTables, table, chain string, rulespec ...string) error { diff --git a/plugins/meta/portmap/chain.go b/plugins/meta/portmap/chain.go index d7b10d5a4..5aad9332a 100644 --- a/plugins/meta/portmap/chain.go +++ b/plugins/meta/portmap/chain.go @@ -103,7 +103,7 @@ func (c *chain) teardown(ipt *iptables.IPTables) error { // check the chain. func (c *chain) check(ipt *iptables.IPTables) error { - exists, err := utils.ChainExists(ipt, c.table, c.name) + exists, err := ipt.ChainExists(c.table, c.name) if err != nil { return err } From 3eb775c5e6d3c0c4dee58ce3c5f5ad45c2014a33 Mon Sep 17 00:00:00 2001 From: Phil Sutter Date: Wed, 5 Jul 2023 22:20:07 +0200 Subject: [PATCH 2/2] plugins: meta: portmap: Implement a teardown() fast path Just attempt to delete the known rules referring to the custom chain, then flush and delete it. If the latter succeeds, no referencing rules are left and the job is done. If the final flush'n'delete fails, fall back to the referencing rule search which is slow with large rulesets. Signed-off-by: Phil Sutter --- plugins/meta/portmap/chain.go | 26 ++++++++++++++++++++------ 1 file changed, 20 insertions(+), 6 deletions(-) diff --git a/plugins/meta/portmap/chain.go b/plugins/meta/portmap/chain.go index 5aad9332a..a70a8d22c 100644 --- a/plugins/meta/portmap/chain.go +++ b/plugins/meta/portmap/chain.go @@ -67,13 +67,27 @@ func (c *chain) setup(ipt *iptables.IPTables) error { // teardown idempotently deletes a chain. It will not error if the chain doesn't exist. // It will first delete all references to this chain in the entryChains. func (c *chain) teardown(ipt *iptables.IPTables) error { - // flush the chain - // This will succeed *and create the chain* if it does not exist. - // If the chain doesn't exist, the next checks will fail. - if err := utils.ClearChain(ipt, c.table, c.name); err != nil { - return err + // nothing to do if the custom chain doesn't exist to begin with + exists, err := ipt.ChainExists(c.table, c.name) + if err == nil && !exists { + return nil + } + // delete references created by setup() + for _, entryChain := range c.entryChains { + for _, rule := range c.entryRules { + r := []string{} + r = append(r, rule...) + r = append(r, "-j", c.name) + + ipt.Delete(c.table, entryChain, r...) + } + } + // if chain deletion succeeds now, all references are gone + if err := ipt.ClearAndDeleteChain(c.table, c.name); err == nil { + return nil } + // find references the hard way for _, entryChain := range c.entryChains { entryChainRules, err := ipt.List(c.table, entryChain) if err != nil || len(entryChainRules) < 1 { @@ -98,7 +112,7 @@ func (c *chain) teardown(ipt *iptables.IPTables) error { } } - return utils.DeleteChain(ipt, c.table, c.name) + return ipt.ClearAndDeleteChain(c.table, c.name) } // check the chain.