Browse Source

libnetwork/iptables: make some vars local, and move bestEffortLock lock

Make some variables local to the if-branches to be slightly more iodiomatic,
and to make clear it's only used in that branch.

Move the bestEffortLock locking later in IPtable.raw(), because that function'
could return before the lock was even needed.

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
Sebastiaan van Stijn 2 years ago
parent
commit
9c2cd65c0d
1 changed files with 12 additions and 20 deletions
  1. 12 20
      libnetwork/iptables/iptables.go

+ 12 - 20
libnetwork/iptables/iptables.go

@@ -488,9 +488,8 @@ const (
 )
 )
 
 
 func filterOutput(start time.Time, output []byte, args ...string) []byte {
 func filterOutput(start time.Time, output []byte, args ...string) []byte {
-	// Flag operations that have taken a long time to complete
-	opTime := time.Since(start)
-	if opTime > opWarnTime {
+	if opTime := time.Since(start); opTime > opWarnTime {
+		// Flag operations that have taken a long time to complete
 		log.G(context.TODO()).Warnf("xtables contention detected while running [%s]: Waited for %.2f seconds and received %q", strings.Join(args, " "), float64(opTime)/float64(time.Second), string(output))
 		log.G(context.TODO()).Warnf("xtables contention detected while running [%s]: Waited for %.2f seconds and received %q", strings.Join(args, " "), float64(opTime)/float64(time.Second), string(output))
 	}
 	}
 	// ignore iptables' message about xtables lock:
 	// ignore iptables' message about xtables lock:
@@ -524,13 +523,6 @@ func (iptable IPTable) raw(args ...string) ([]byte, error) {
 	if err := initCheck(); err != nil {
 	if err := initCheck(); err != nil {
 		return nil, err
 		return nil, err
 	}
 	}
-	if supportsXlock {
-		args = append([]string{"--wait"}, args...)
-	} else {
-		bestEffortLock.Lock()
-		defer bestEffortLock.Unlock()
-	}
-
 	path := iptablesPath
 	path := iptablesPath
 	commandName := "iptables"
 	commandName := "iptables"
 	if iptable.Version == IPv6 {
 	if iptable.Version == IPv6 {
@@ -541,6 +533,13 @@ func (iptable IPTable) raw(args ...string) ([]byte, error) {
 		commandName = "ip6tables"
 		commandName = "ip6tables"
 	}
 	}
 
 
+	if supportsXlock {
+		args = append([]string{"--wait"}, args...)
+	} else {
+		bestEffortLock.Lock()
+		defer bestEffortLock.Unlock()
+	}
+
 	log.G(context.TODO()).Debugf("%s, %v", path, args)
 	log.G(context.TODO()).Debugf("%s, %v", path, args)
 
 
 	startTime := time.Now()
 	startTime := time.Now()
@@ -589,28 +588,21 @@ func (iptable IPTable) AddReturnRule(chain string) error {
 	if iptable.Exists(Filter, chain, "-j", "RETURN") {
 	if iptable.Exists(Filter, chain, "-j", "RETURN") {
 		return nil
 		return nil
 	}
 	}
-
-	err := iptable.RawCombinedOutput("-A", chain, "-j", "RETURN")
-	if err != nil {
+	if err := iptable.RawCombinedOutput("-A", chain, "-j", "RETURN"); err != nil {
 		return fmt.Errorf("unable to add return rule in %s chain: %v", chain, err)
 		return fmt.Errorf("unable to add return rule in %s chain: %v", chain, err)
 	}
 	}
-
 	return nil
 	return nil
 }
 }
 
 
 // EnsureJumpRule ensures the jump rule is on top
 // EnsureJumpRule ensures the jump rule is on top
 func (iptable IPTable) EnsureJumpRule(fromChain, toChain string) error {
 func (iptable IPTable) EnsureJumpRule(fromChain, toChain string) error {
 	if iptable.Exists(Filter, fromChain, "-j", toChain) {
 	if iptable.Exists(Filter, fromChain, "-j", toChain) {
-		err := iptable.RawCombinedOutput("-D", fromChain, "-j", toChain)
-		if err != nil {
+		if err := iptable.RawCombinedOutput("-D", fromChain, "-j", toChain); err != nil {
 			return fmt.Errorf("unable to remove jump to %s rule in %s chain: %v", toChain, fromChain, err)
 			return fmt.Errorf("unable to remove jump to %s rule in %s chain: %v", toChain, fromChain, err)
 		}
 		}
 	}
 	}
-
-	err := iptable.RawCombinedOutput("-I", fromChain, "-j", toChain)
-	if err != nil {
+	if err := iptable.RawCombinedOutput("-I", fromChain, "-j", toChain); err != nil {
 		return fmt.Errorf("unable to insert jump to %s rule in %s chain: %v", toChain, fromChain, err)
 		return fmt.Errorf("unable to insert jump to %s rule in %s chain: %v", toChain, fromChain, err)
 	}
 	}
-
 	return nil
 	return nil
 }
 }