spoofcheck: Make use of go-nft's ApplyConfigEcho()

Store the relevant applied config part for later to extract the rule to
delete from there instead of having to list the ruleset. This is much
faster especially with large rulesets.

Signed-off-by: Phil Sutter <psutter@redhat.com>

go.mod

@@ -14,7 +14,7 @@ require (
 	github.com/d2g/dhcp4server v0.0.0-20181031114812-7d4a0a7f59a5
 	github.com/godbus/dbus/v5 v5.1.0
 	github.com/mattn/go-shellwords v1.0.12
-	github.com/networkplumbing/go-nft v0.3.0
+	github.com/networkplumbing/go-nft v0.4.0
 	github.com/onsi/ginkgo/v2 v2.9.2
 	github.com/onsi/gomega v1.27.6
 	github.com/opencontainers/selinux v1.11.0

go.sum

@@ -486,8 +486,8 @@ github.com/munnerz/goautoneg v0.0.0-20191010083416-a7dc8b61c822/go.mod h1:+n7T8m
 github.com/mwitkow/go-conntrack v0.0.0-20161129095857-cc309e4a2223/go.mod h1:qRWi+5nqEBWmkhHvq77mSJWrCKwh8bxhgT7d/eI7P4U=
 github.com/mxk/go-flowrate v0.0.0-20140419014527-cca7078d478f/go.mod h1:ZdcZmHo+o7JKHSa8/e818NopupXU1YMK5fe1lsApnBw=
 github.com/ncw/swift v1.0.47/go.mod h1:23YIA4yWVnGwv2dQlN4bB7egfYX6YLn0Yo/S6zZO/ZM=
-github.com/networkplumbing/go-nft v0.3.0 h1:IIc6yHjN85KyJx21p3ZEsO0iBMYHNXux22rc9Q8TfFw=
-github.com/networkplumbing/go-nft v0.3.0/go.mod h1:HnnM+tYvlGAsMU7yoYwXEVLLiDW9gdMmb5HoGcwpuQs=
+github.com/networkplumbing/go-nft v0.4.0 h1:kExVMwXW48DOAukkBwyI16h4uhE5lN9iMvQd52lpTyU=
+github.com/networkplumbing/go-nft v0.4.0/go.mod h1:HnnM+tYvlGAsMU7yoYwXEVLLiDW9gdMmb5HoGcwpuQs=
 github.com/niemeyer/pretty v0.0.0-20200227124842-a10e7caefd8e h1:fD57ERR4JtEqsWbfPhv4DMiApHyliiK5xCTNVSPiaAs=
 github.com/niemeyer/pretty v0.0.0-20200227124842-a10e7caefd8e/go.mod h1:zD1mROLANZcx1PVRCS0qkT7pwLkGfwJo4zjcN/Tysno=
 github.com/nxadm/tail v1.4.4/go.mod h1:kenIhsEOeOJmVchQTgglprH7qJGnHDVpk1VPCcaMI8A=

pkg/link/spoofcheck.go

@@ -30,7 +30,7 @@ const (
 )
 
 type NftConfigurer interface {
-	Apply(*nft.Config) error
+	Apply(*nft.Config) (*nft.Config, error)
 	Read(filterCommands ...string) (*nft.Config, error)
 }
 
@@ -39,12 +39,16 @@ type SpoofChecker struct {
 	macAddress string
 	refID      string
 	configurer NftConfigurer
+	rulestore  *nft.Config
 }
 
 type defaultNftConfigurer struct{}
 
-func (dnc defaultNftConfigurer) Apply(cfg *nft.Config) error {
-	return nft.ApplyConfig(cfg)
+func (dnc defaultNftConfigurer) Apply(cfg *nft.Config) (*nft.Config, error) {
+	const timeout = 55 * time.Second
+	ctxWithTimeout, cancelFunc := context.WithTimeout(context.Background(), timeout)
+	defer cancelFunc()
+	return nft.ApplyConfigEcho(ctxWithTimeout, cfg)
 }
 
 func (dnc defaultNftConfigurer) Read(filterCommands ...string) (*nft.Config, error) {
@@ -59,7 +63,7 @@ func NewSpoofChecker(iface, macAddress, refID string) *SpoofChecker {
 }
 
 func NewSpoofCheckerWithConfigurer(iface, macAddress, refID string, configurer NftConfigurer) *SpoofChecker {
-	return &SpoofChecker{iface, macAddress, refID, configurer}
+	return &SpoofChecker{iface, macAddress, refID, configurer, nil}
 }
 
 // Setup applies nftables configuration to restrict traffic
@@ -88,7 +92,7 @@ func (sc *SpoofChecker) Setup() error {
 	macChain := sc.macChain(ifaceChain.Name)
 	baseConfig.AddChain(macChain)
 
-	if err := sc.configurer.Apply(baseConfig); err != nil {
+	if _, err := sc.configurer.Apply(baseConfig); err != nil {
 		return fmt.Errorf("failed to setup spoof-check: %v", err)
 	}
 
@@ -102,45 +106,59 @@ func (sc *SpoofChecker) Setup() error {
 	rulesConfig.AddRule(sc.matchMacRule(macChain.Name))
 	rulesConfig.AddRule(sc.dropRule(macChain.Name))
 
-	if err := sc.configurer.Apply(rulesConfig); err != nil {
+	rulestore, err := sc.configurer.Apply(rulesConfig)
+	if err != nil {
 		return fmt.Errorf("failed to setup spoof-check: %v", err)
 	}
+	sc.rulestore = rulestore
 
 	return nil
 }
 
+func (sc *SpoofChecker) findPreroutingRule(ruleToFind *schema.Rule) ([]*schema.Rule, error) {
+	ruleset := sc.rulestore
+	if ruleset == nil {
+		chain, err := sc.configurer.Read(listChainBridgeNatPrerouting()...)
+		if err != nil {
+			return nil, err
+		}
+		ruleset = chain
+	}
+	return ruleset.LookupRule(ruleToFind), nil
+}
+
 // Teardown removes the interface and mac-address specific chains and their rules.
 // The table and base-chain are expected to survive while the base-chain rule that matches the
 // interface is removed.
 func (sc *SpoofChecker) Teardown() error {
 	ifaceChain := sc.ifaceChain()
-	currentConfig, ifaceMatchRuleErr := sc.configurer.Read(listChainBridgeNatPrerouting()...)
-	if ifaceMatchRuleErr == nil {
-		expectedRuleToFind := sc.matchIfaceJumpToChainRule(preRoutingBaseChainName, ifaceChain.Name)
-		// It is safer to exclude the statement matching, avoiding cases where a current statement includes
-		// additional default entries (e.g. counters).
-		ruleToFindExcludingStatements := *expectedRuleToFind
-		ruleToFindExcludingStatements.Expr = nil
-		rules := currentConfig.LookupRule(&ruleToFindExcludingStatements)
-		if len(rules) > 0 {
-			c := nft.NewConfig()
-			for _, rule := range rules {
-				c.DeleteRule(rule)
-			}
-			if err := sc.configurer.Apply(c); err != nil {
-				ifaceMatchRuleErr = fmt.Errorf("failed to delete iface match rule: %v", err)
-			}
-		} else {
-			fmt.Fprintf(os.Stderr, "spoofcheck/teardown: unable to detect iface match rule for deletion: %+v", expectedRuleToFind)
+	expectedRuleToFind := sc.matchIfaceJumpToChainRule(preRoutingBaseChainName, ifaceChain.Name)
+	// It is safer to exclude the statement matching, avoiding cases where a current statement includes
+	// additional default entries (e.g. counters).
+	ruleToFindExcludingStatements := *expectedRuleToFind
+	ruleToFindExcludingStatements.Expr = nil
+
+	rules, ifaceMatchRuleErr := sc.findPreroutingRule(&ruleToFindExcludingStatements)
+	if ifaceMatchRuleErr == nil && len(rules) > 0 {
+		c := nft.NewConfig()
+		for _, rule := range rules {
+			c.DeleteRule(rule)
+		}
+		if _, err := sc.configurer.Apply(c); err != nil {
+			ifaceMatchRuleErr = fmt.Errorf("failed to delete iface match rule: %v", err)
 		}
+		// Drop the cache, it should contain deleted rule(s) now
+		sc.rulestore = nil
+	} else {
+		fmt.Fprintf(os.Stderr, "spoofcheck/teardown: unable to detect iface match rule for deletion: %+v", expectedRuleToFind)
 	}
 
 	regularChainsConfig := nft.NewConfig()
 	regularChainsConfig.DeleteChain(ifaceChain)
 	regularChainsConfig.DeleteChain(sc.macChain(ifaceChain.Name))
 
 	var regularChainsErr error
-	if err := sc.configurer.Apply(regularChainsConfig); err != nil {
+	if _, err := sc.configurer.Apply(regularChainsConfig); err != nil {
 		regularChainsErr = fmt.Errorf("failed to delete regular chains: %v", err)
 	}
 

pkg/link/spoofcheck_test.go

@@ -113,6 +113,25 @@ var _ = Describe("spoofcheck", func() {
 			)))
 		})
 	})
+
+	Context("echo", func() {
+		It("succeeds, no read called", func() {
+			c := configurerStub{}
+			sc := link.NewSpoofCheckerWithConfigurer(iface, mac, id, &c)
+			Expect(sc.Setup()).To(Succeed())
+			Expect(sc.Teardown()).To(Succeed())
+			Expect(c.readCalled).To(BeFalse())
+		})
+
+		It("succeeds, fall back to config read", func() {
+			c := configurerStub{applyReturnNil: true}
+			sc := link.NewSpoofCheckerWithConfigurer(iface, mac, id, &c)
+			Expect(sc.Setup()).To(Succeed())
+			c.readConfig = c.applyConfig[0]
+			Expect(sc.Teardown()).To(Succeed())
+			Expect(c.readCalled).To(BeTrue())
+		})
+	})
 })
 
 func assertExpectedRegularChainsDeletionInTeardownConfig(action configurerStub) {
@@ -274,21 +293,28 @@ type configurerStub struct {
 	failFirstApplyConfig  bool
 	failSecondApplyConfig bool
 	failReadConfig        bool
+
+	applyReturnNil        bool
+	readCalled            bool
 }
 
-func (a *configurerStub) Apply(c *nft.Config) error {
+func (a *configurerStub) Apply(c *nft.Config) (*nft.Config, error) {
 	a.applyCounter++
 	if a.failFirstApplyConfig && a.applyCounter == 1 {
-		return fmt.Errorf(errorFirstApplyText)
+		return nil, fmt.Errorf(errorFirstApplyText)
 	}
 	if a.failSecondApplyConfig && a.applyCounter == 2 {
-		return fmt.Errorf(errorSecondApplyText)
+		return nil, fmt.Errorf(errorSecondApplyText)
 	}
 	a.applyConfig = append(a.applyConfig, c)
-	return nil
+	if a.applyReturnNil {
+		return nil, nil
+	}
+	return c, nil
 }
 
 func (a *configurerStub) Read(_ ...string) (*nft.Config, error) {
+	a.readCalled = true
 	if a.failReadConfig {
 		return nil, fmt.Errorf(errorReadText)
 	}

vendor/modules.txt

@@ -103,7 +103,7 @@ github.com/google/pprof/profile
 # github.com/mattn/go-shellwords v1.0.12
 ## explicit; go 1.13
 github.com/mattn/go-shellwords
-# github.com/networkplumbing/go-nft v0.3.0
+# github.com/networkplumbing/go-nft v0.4.0
 ## explicit; go 1.16
 github.com/networkplumbing/go-nft/nft
 github.com/networkplumbing/go-nft/nft/config