From c271c61feea7d3ea3fcbb3af9f0d9c1f641a8d82 Mon Sep 17 00:00:00 2001
From: Aaron Davidson <aaron@databricks.com>
Date: Sun, 12 Apr 2015 16:56:05 -0700
Subject: [PATCH] Do our best not to invoke iptables concurrently if --wait is
 unsupported

We encountered a situation where concurrent invocations of the docker daemon on a machine with an older version of iptables led to nondeterministic errors related to simultaenous invocations of iptables.

While this is best resolved by upgrading iptables itself, the particular situation would have been avoided if the docker daemon simply took care not to concurrently invoke iptables. Of course, external processes could also cause iptables to fail in this way, but invoking docker in parallel seems like a pretty common case.

Signed-off-by: Aaron Davidson <aaron@databricks.com>
---
 pkg/iptables/iptables.go      | 10 +++++++--
 pkg/iptables/iptables_test.go | 40 +++++++++++++++++++++++++++++++++++
 2 files changed, 48 insertions(+), 2 deletions(-)

diff --git a/pkg/iptables/iptables.go b/pkg/iptables/iptables.go
index 0cfcca7502..fcedd0f341 100644
--- a/pkg/iptables/iptables.go
+++ b/pkg/iptables/iptables.go
@@ -8,6 +8,7 @@ import (
 	"regexp"
 	"strconv"
 	"strings"
+	"sync"
 
 	"github.com/Sirupsen/logrus"
 )
@@ -25,8 +26,10 @@ const (
 )
 
 var (
-	iptablesPath        string
-	supportsXlock       = false
+	iptablesPath  string
+	supportsXlock = false
+	// used to lock iptables commands if xtables lock is not supported
+	bestEffortLock      sync.Mutex
 	ErrIptablesNotFound = errors.New("Iptables not found")
 )
 
@@ -288,6 +291,9 @@ func Raw(args ...string) ([]byte, error) {
 	}
 	if supportsXlock {
 		args = append([]string{"--wait"}, args...)
+	} else {
+		bestEffortLock.Lock()
+		defer bestEffortLock.Unlock()
 	}
 
 	logrus.Debugf("%s, %v", iptablesPath, args)
diff --git a/pkg/iptables/iptables_test.go b/pkg/iptables/iptables_test.go
index ced4262ce2..840aca14b9 100644
--- a/pkg/iptables/iptables_test.go
+++ b/pkg/iptables/iptables_test.go
@@ -5,6 +5,7 @@ import (
 	"os/exec"
 	"strconv"
 	"strings"
+	"sync"
 	"testing"
 )
 
@@ -169,6 +170,45 @@ func TestOutput(t *testing.T) {
 	}
 }
 
+func TestConcurrencyWithWait(t *testing.T) {
+	RunConcurrencyTest(t, true)
+}
+
+func TestConcurrencyNoWait(t *testing.T) {
+	RunConcurrencyTest(t, false)
+}
+
+// Runs 10 concurrent rule additions. This will fail if iptables
+// is actually invoked simultaneously without --wait.
+// Note that if iptables does not support the xtable lock on this
+// system, then allowXlock has no effect -- it will always be off.
+func RunConcurrencyTest(t *testing.T, allowXlock bool) {
+	var wg sync.WaitGroup
+
+	if !allowXlock && supportsXlock {
+		supportsXlock = false
+		defer func() { supportsXlock = true }()
+	}
+
+	ip := net.ParseIP("192.168.1.1")
+	port := 1234
+	dstAddr := "172.17.0.1"
+	dstPort := 4321
+	proto := "tcp"
+
+	for i := 0; i < 10; i++ {
+		wg.Add(1)
+		go func() {
+			defer wg.Done()
+			err := natChain.Forward(Append, ip, port, proto, dstAddr, dstPort)
+			if err != nil {
+				t.Fatal(err)
+			}
+		}()
+	}
+	wg.Wait()
+}
+
 func TestCleanup(t *testing.T) {
 	var err error
 	var rules []byte