From 90dab0c1b70320c2a5efc3a80178f219d942e4f7 Mon Sep 17 00:00:00 2001 From: Jana Radhakrishnan Date: Tue, 26 May 2015 21:14:01 +0000 Subject: [PATCH 1/7] Workaround kernel bugs s related to namespaces This PR attempts to work around bugs present in kernel version 3.18-4.0.1 relating to namespace creation and destruction. This fix attempts to avoid certain systemmcalls to not get in the kkernel bug path as well as lazily garbage collecting the name paths when they are removed. Signed-off-by: Jana Radhakrishnan --- sandbox/namespace_linux.go | 135 ++++++++++++++++++++++++++++------ sandbox/sandbox_linux_test.go | 11 +++ sandbox/sandbox_test.go | 11 +++ 3 files changed, 134 insertions(+), 23 deletions(-) diff --git a/sandbox/namespace_linux.go b/sandbox/namespace_linux.go index dd93e83..bb1770d 100644 --- a/sandbox/namespace_linux.go +++ b/sandbox/namespace_linux.go @@ -4,10 +4,15 @@ import ( "fmt" "net" "os" + "os/exec" + "path/filepath" "runtime" "sync" "syscall" + "time" + log "github.com/Sirupsen/logrus" + "github.com/docker/docker/pkg/reexec" "github.com/docker/libnetwork/types" "github.com/vishvananda/netlink" "github.com/vishvananda/netns" @@ -15,7 +20,13 @@ import ( const prefix = "/var/run/docker/netns" -var once sync.Once +var ( + once sync.Once + garbagePathMap = make(map[string]bool) + gpmLock sync.Mutex + gpmWg sync.WaitGroup + gpmCleanupPeriod = 60 +) // The networkNamespace type is the linux implementation of the Sandbox // interface. It represents a linux network namespace, and moves an interface @@ -27,11 +38,56 @@ type networkNamespace struct { sync.Mutex } +func init() { + reexec.Register("netns-create", reexecCreateNamespace) +} + func createBasePath() { err := os.MkdirAll(prefix, 0644) if err != nil && !os.IsExist(err) { panic("Could not create net namespace path directory") } + + // cleanup any stale namespace files if any + cleanupNamespaceFiles() + + // Start the garbage collection go routine + go removeUnusedPaths() +} + +func removeUnusedPaths() { + for { + time.Sleep(time.Duration(gpmCleanupPeriod) * time.Second) + + gpmLock.Lock() + pathList := make([]string, 0, len(garbagePathMap)) + for path := range garbagePathMap { + pathList = append(pathList, path) + } + garbagePathMap = make(map[string]bool) + gpmWg.Add(1) + gpmLock.Unlock() + + for _, path := range pathList { + os.Remove(path) + } + + gpmWg.Done() + } +} + +func addToGarbagePaths(path string) { + gpmLock.Lock() + defer gpmLock.Unlock() + + garbagePathMap[path] = true +} + +func removeFromGarbagePaths(path string) { + gpmLock.Lock() + defer gpmLock.Unlock() + + delete(garbagePathMap, path) } // GenerateKey generates a sandbox key based on the passed @@ -56,6 +112,16 @@ func NewSandbox(key string, osCreate bool) (Sandbox, error) { return &networkNamespace{path: key, sinfo: info}, nil } +func reexecCreateNamespace() { + if len(os.Args) < 2 { + log.Fatal("no namespace path provided") + } + + if err := syscall.Mount("/proc/self/ns/net", os.Args[1], "bind", syscall.MS_BIND, ""); err != nil { + log.Fatal(err) + } +} + func createNetworkNamespace(path string, osCreate bool) (*Info, error) { runtime.LockOSThread() defer runtime.UnlockOSThread() @@ -70,23 +136,18 @@ func createNetworkNamespace(path string, osCreate bool) (*Info, error) { return nil, err } - if osCreate { - defer netns.Set(origns) - newns, err := netns.New() - if err != nil { - return nil, err - } - defer newns.Close() - - if err := loopbackUp(); err != nil { - return nil, err - } + cmd := &exec.Cmd{ + Path: reexec.Self(), + Args: append([]string{"netns-create"}, path), + Stdout: os.Stdout, + Stderr: os.Stderr, } - - procNet := fmt.Sprintf("/proc/%d/task/%d/ns/net", os.Getpid(), syscall.Gettid()) - - if err := syscall.Mount(procNet, path, "bind", syscall.MS_BIND, ""); err != nil { - return nil, err + if osCreate { + cmd.SysProcAttr = &syscall.SysProcAttr{} + cmd.SysProcAttr.Cloneflags = syscall.CLONE_NEWNET + } + if err := cmd.Run(); err != nil { + return nil, fmt.Errorf("namespace creation reexec command failed: %v", err) } interfaces := []*Interface{} @@ -94,10 +155,27 @@ func createNetworkNamespace(path string, osCreate bool) (*Info, error) { return info, nil } -func cleanupNamespaceFile(path string) { +func cleanupNamespaceFiles() { + filepath.Walk(prefix, func(path string, info os.FileInfo, err error) error { + stat, err := os.Stat(path) + if err != nil { + return err + } + + if stat.IsDir() { + return filepath.SkipDir + } + + syscall.Unmount(path, syscall.MNT_DETACH) + os.Remove(path) + + return nil + }) +} + +func unmountNamespaceFile(path string) { if _, err := os.Stat(path); err == nil { - n := &networkNamespace{path: path} - n.Destroy() + syscall.Unmount(path, syscall.MNT_DETACH) } } @@ -105,11 +183,20 @@ func createNamespaceFile(path string) (err error) { var f *os.File once.Do(createBasePath) - // cleanup namespace file if it already exists because of a previous ungraceful exit. - cleanupNamespaceFile(path) + // Remove it from garbage collection list if present + removeFromGarbagePaths(path) + + // If the path is there unmount it first + unmountNamespaceFile(path) + + // wait for garbage collection to complete if it is in progress + // before trying to create the file. + gpmWg.Wait() + if f, err = os.Create(path); err == nil { f.Close() } + return err } @@ -310,5 +397,7 @@ func (n *networkNamespace) Destroy() error { return err } - return os.Remove(n.path) + // Stash it into the garbage collection list + addToGarbagePaths(n.path) + return nil } diff --git a/sandbox/sandbox_linux_test.go b/sandbox/sandbox_linux_test.go index 678cd0d..b550363 100644 --- a/sandbox/sandbox_linux_test.go +++ b/sandbox/sandbox_linux_test.go @@ -6,6 +6,7 @@ import ( "path/filepath" "runtime" "testing" + "time" "github.com/docker/libnetwork/netutils" "github.com/vishvananda/netlink" @@ -31,6 +32,9 @@ func newKey(t *testing.T) (string, error) { return "", err } + // Set the rpmCleanupPeriod to be low to make the test run quicker + gpmCleanupPeriod = 2 + return name, nil } @@ -146,3 +150,10 @@ func verifySandbox(t *testing.T, s Sandbox) { err) } } + +func verifyCleanup(t *testing.T, s Sandbox) { + time.Sleep(time.Duration(gpmCleanupPeriod*2) * time.Second) + if _, err := os.Stat(s.Key()); err == nil { + t.Fatalf("The sandbox path %s is not getting cleanup event after twice the cleanup period", s.Key()) + } +} diff --git a/sandbox/sandbox_test.go b/sandbox/sandbox_test.go index 03b250b..639dc79 100644 --- a/sandbox/sandbox_test.go +++ b/sandbox/sandbox_test.go @@ -2,9 +2,19 @@ package sandbox import ( "net" + "os" "testing" + + "github.com/docker/docker/pkg/reexec" ) +func TestMain(m *testing.M) { + if reexec.Init() { + return + } + os.Exit(m.Run()) +} + func TestSandboxCreate(t *testing.T) { key, err := newKey(t) if err != nil { @@ -44,6 +54,7 @@ func TestSandboxCreate(t *testing.T) { verifySandbox(t, s) s.Destroy() + verifyCleanup(t, s) } func TestSandboxCreateTwice(t *testing.T) { From 3a5abeb1c604bc83f07dbdac0e577a2ba4f14acc Mon Sep 17 00:00:00 2001 From: Jana Radhakrishnan Date: Wed, 27 May 2015 20:20:24 +0000 Subject: [PATCH 2/7] Loopback interface not t brought up Loopback interface was s not brought up when wemoved to clone method of creating namespace. e. Adding it. Also taking care of PR R comments. Signed-off-by: Jana Radhakrishnan --- sandbox/namespace_linux.go | 14 ++++++++------ sandbox/sandbox_linux_test.go | 4 ++-- 2 files changed, 10 insertions(+), 8 deletions(-) diff --git a/sandbox/namespace_linux.go b/sandbox/namespace_linux.go index bb1770d..7ecf4b3 100644 --- a/sandbox/namespace_linux.go +++ b/sandbox/namespace_linux.go @@ -25,7 +25,7 @@ var ( garbagePathMap = make(map[string]bool) gpmLock sync.Mutex gpmWg sync.WaitGroup - gpmCleanupPeriod = 60 + gpmCleanupPeriod = 60 * time.Second ) // The networkNamespace type is the linux implementation of the Sandbox @@ -57,7 +57,7 @@ func createBasePath() { func removeUnusedPaths() { for { - time.Sleep(time.Duration(gpmCleanupPeriod) * time.Second) + time.Sleep(time.Duration(gpmCleanupPeriod)) gpmLock.Lock() pathList := make([]string, 0, len(garbagePathMap)) @@ -78,16 +78,14 @@ func removeUnusedPaths() { func addToGarbagePaths(path string) { gpmLock.Lock() - defer gpmLock.Unlock() - garbagePathMap[path] = true + defer gpmLock.Unlock() } func removeFromGarbagePaths(path string) { gpmLock.Lock() - defer gpmLock.Unlock() - delete(garbagePathMap, path) + defer gpmLock.Unlock() } // GenerateKey generates a sandbox key based on the passed @@ -120,6 +118,10 @@ func reexecCreateNamespace() { if err := syscall.Mount("/proc/self/ns/net", os.Args[1], "bind", syscall.MS_BIND, ""); err != nil { log.Fatal(err) } + + if err := loopbackUp(); err != nil { + log.Fatal(err) + } } func createNetworkNamespace(path string, osCreate bool) (*Info, error) { diff --git a/sandbox/sandbox_linux_test.go b/sandbox/sandbox_linux_test.go index b550363..2700635 100644 --- a/sandbox/sandbox_linux_test.go +++ b/sandbox/sandbox_linux_test.go @@ -33,7 +33,7 @@ func newKey(t *testing.T) (string, error) { } // Set the rpmCleanupPeriod to be low to make the test run quicker - gpmCleanupPeriod = 2 + gpmCleanupPeriod = 2 * time.Second return name, nil } @@ -152,7 +152,7 @@ func verifySandbox(t *testing.T, s Sandbox) { } func verifyCleanup(t *testing.T, s Sandbox) { - time.Sleep(time.Duration(gpmCleanupPeriod*2) * time.Second) + time.Sleep(time.Duration(gpmCleanupPeriod * 2)) if _, err := os.Stat(s.Key()); err == nil { t.Fatalf("The sandbox path %s is not getting cleanup event after twice the cleanup period", s.Key()) } From eeafdab8a1a1129b096598ee48f551a9d1c8cac2 Mon Sep 17 00:00:00 2001 From: Jana Radhakrishnan Date: Wed, 27 May 2015 21:40:02 +0000 Subject: [PATCH 3/7] Reworkkgarbage collection code to use tick Instead of sleeping reworked the code to use recurring ticks. Also cleaned up unnecessary defers. Signed-off-by: Jana Radhakrishnan --- sandbox/namespace_linux.go | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/sandbox/namespace_linux.go b/sandbox/namespace_linux.go index 7ecf4b3..68692b7 100644 --- a/sandbox/namespace_linux.go +++ b/sandbox/namespace_linux.go @@ -56,9 +56,7 @@ func createBasePath() { } func removeUnusedPaths() { - for { - time.Sleep(time.Duration(gpmCleanupPeriod)) - + for range time.Tick(gpmCleanupPeriod) { gpmLock.Lock() pathList := make([]string, 0, len(garbagePathMap)) for path := range garbagePathMap { @@ -79,13 +77,13 @@ func removeUnusedPaths() { func addToGarbagePaths(path string) { gpmLock.Lock() garbagePathMap[path] = true - defer gpmLock.Unlock() + gpmLock.Unlock() } func removeFromGarbagePaths(path string) { gpmLock.Lock() delete(garbagePathMap, path) - defer gpmLock.Unlock() + gpmLock.Unlock() } // GenerateKey generates a sandbox key based on the passed From 8738bf5fb28ec906f47a46008455d31af2a6e6c7 Mon Sep 17 00:00:00 2001 From: Jana Radhakrishnan Date: Wed, 27 May 2015 23:49:32 +0000 Subject: [PATCH 4/7] Removee the init time cleanup of namespace files Removing this as this may cause problems when multiple instances are e running. Signed-off-by: Jana Radhakrishnan --- sandbox/namespace_linux.go | 22 ---------------------- 1 file changed, 22 deletions(-) diff --git a/sandbox/namespace_linux.go b/sandbox/namespace_linux.go index 68692b7..97ea548 100644 --- a/sandbox/namespace_linux.go +++ b/sandbox/namespace_linux.go @@ -5,7 +5,6 @@ import ( "net" "os" "os/exec" - "path/filepath" "runtime" "sync" "syscall" @@ -48,9 +47,6 @@ func createBasePath() { panic("Could not create net namespace path directory") } - // cleanup any stale namespace files if any - cleanupNamespaceFiles() - // Start the garbage collection go routine go removeUnusedPaths() } @@ -155,24 +151,6 @@ func createNetworkNamespace(path string, osCreate bool) (*Info, error) { return info, nil } -func cleanupNamespaceFiles() { - filepath.Walk(prefix, func(path string, info os.FileInfo, err error) error { - stat, err := os.Stat(path) - if err != nil { - return err - } - - if stat.IsDir() { - return filepath.SkipDir - } - - syscall.Unmount(path, syscall.MNT_DETACH) - os.Remove(path) - - return nil - }) -} - func unmountNamespaceFile(path string) { if _, err := os.Stat(path); err == nil { syscall.Unmount(path, syscall.MNT_DETACH) From 0a81ebd7454ccefc80bcebb714c9d730c34f474f Mon Sep 17 00:00:00 2001 From: Jana Radhakrishnan Date: Thu, 28 May 2015 20:01:00 +0000 Subject: [PATCH 5/7] Modprobe bridge driver r specific kernel modules Try too modprobe bridge driverer specic modulein case they are not loaded into the kernel. Signed-off-by: Jana Radhakrishnan --- drivers/bridge/bridge.go | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/drivers/bridge/bridge.go b/drivers/bridge/bridge.go index ceff412..c26da23 100644 --- a/drivers/bridge/bridge.go +++ b/drivers/bridge/bridge.go @@ -3,10 +3,12 @@ package bridge import ( "errors" "net" + "os/exec" "strconv" "strings" "sync" + "github.com/Sirupsen/logrus" "github.com/docker/libnetwork/driverapi" "github.com/docker/libnetwork/ipallocator" "github.com/docker/libnetwork/netlabel" @@ -104,6 +106,12 @@ func newDriver() driverapi.Driver { // Init registers a new instance of bridge driver func Init(dc driverapi.DriverCallback) error { + // try to modprobe bridge first + // see gh#12177 + if out, err := exec.Command("modprobe", "-va", "bridge", "nf_nat", "br_netfilter").Output(); err != nil { + logrus.Warnf("Running modprobe bridge nf_nat failed with message: %s, error: %v", out, err) + } + return dc.RegisterDriver(networkType, newDriver()) } From a031640ea09c4889dbb217c9bc4f0368ea68b909 Mon Sep 17 00:00:00 2001 From: Jana Radhakrishnan Date: Thu, 28 May 2015 23:29:21 +0000 Subject: [PATCH 6/7] Fix miscellaneaus data races Fixed the remaining data races in the libnetwork code. Signed-off-by: Jana Radhakrishnan --- network.go | 9 +++++++++ sandbox/namespace_linux.go | 6 +++++- sandbox/sandbox_linux_test.go | 2 ++ 3 files changed, 16 insertions(+), 1 deletion(-) diff --git a/network.go b/network.go index 7b15ffc..8a25b84 100644 --- a/network.go +++ b/network.go @@ -64,14 +64,23 @@ type network struct { } func (n *network) Name() string { + n.Lock() + defer n.Unlock() + return n.name } func (n *network) ID() string { + n.Lock() + defer n.Unlock() + return string(n.id) } func (n *network) Type() string { + n.Lock() + defer n.Unlock() + if n.driver == nil { return "" } diff --git a/sandbox/namespace_linux.go b/sandbox/namespace_linux.go index 97ea548..3db3a8e 100644 --- a/sandbox/namespace_linux.go +++ b/sandbox/namespace_linux.go @@ -52,7 +52,11 @@ func createBasePath() { } func removeUnusedPaths() { - for range time.Tick(gpmCleanupPeriod) { + gpmLock.Lock() + period := gpmCleanupPeriod + gpmLock.Unlock() + + for range time.Tick(period) { gpmLock.Lock() pathList := make([]string, 0, len(garbagePathMap)) for path := range garbagePathMap { diff --git a/sandbox/sandbox_linux_test.go b/sandbox/sandbox_linux_test.go index 2700635..b00d14f 100644 --- a/sandbox/sandbox_linux_test.go +++ b/sandbox/sandbox_linux_test.go @@ -33,7 +33,9 @@ func newKey(t *testing.T) (string, error) { } // Set the rpmCleanupPeriod to be low to make the test run quicker + gpmLock.Lock() gpmCleanupPeriod = 2 * time.Second + gpmLock.Unlock() return name, nil } From 9669dbaf0b631264a45e5cb91a3c12b8629c01d7 Mon Sep 17 00:00:00 2001 From: Madhu Venugopal Date: Thu, 28 May 2015 16:30:36 -0700 Subject: [PATCH 7/7] Fixes https://github.com/docker/docker/issues/13426 Signed-off-by: Madhu Venugopal --- drivers/bridge/bridge.go | 8 ++++++++ drivers/bridge/setup_fixedcidrv6.go | 13 +++++++++++++ drivers/bridge/setup_ipv6.go | 19 +++++++++++++++++-- 3 files changed, 38 insertions(+), 2 deletions(-) diff --git a/drivers/bridge/bridge.go b/drivers/bridge/bridge.go index c26da23..3cacad1 100644 --- a/drivers/bridge/bridge.go +++ b/drivers/bridge/bridge.go @@ -518,6 +518,11 @@ func (d *driver) CreateNetwork(id types.UUID, option map[string]interface{}) err // Even if a bridge exists try to setup IPv4. bridgeSetup.queueStep(setupBridgeIPv4) + enableIPv6Forwarding := false + if d.config != nil && d.config.EnableIPForwarding && config.FixedCIDRv6 != nil { + enableIPv6Forwarding = true + } + // Conditionally queue setup steps depending on configuration values. for _, step := range []struct { Condition bool @@ -541,6 +546,9 @@ func (d *driver) CreateNetwork(id types.UUID, option map[string]interface{}) err // specified subnet. {config.FixedCIDRv6 != nil, setupFixedCIDRv6}, + // Enable IPv6 Forwarding + {enableIPv6Forwarding, setupIPv6Forwarding}, + // Setup Loopback Adresses Routing {!config.EnableUserlandProxy, setupLoopbackAdressesRouting}, diff --git a/drivers/bridge/setup_fixedcidrv6.go b/drivers/bridge/setup_fixedcidrv6.go index 1b4bb57..b2a949b 100644 --- a/drivers/bridge/setup_fixedcidrv6.go +++ b/drivers/bridge/setup_fixedcidrv6.go @@ -1,7 +1,10 @@ package bridge import ( + "os" + log "github.com/Sirupsen/logrus" + "github.com/vishvananda/netlink" ) func setupFixedCIDRv6(config *networkConfiguration, i *bridgeInterface) error { @@ -10,5 +13,15 @@ func setupFixedCIDRv6(config *networkConfiguration, i *bridgeInterface) error { return &FixedCIDRv6Error{Net: config.FixedCIDRv6, Err: err} } + // Setting route to global IPv6 subnet + log.Debugf("Adding route to IPv6 network %s via device %s", config.FixedCIDRv6.String(), config.BridgeName) + err := netlink.RouteAdd(&netlink.Route{ + Scope: netlink.SCOPE_UNIVERSE, + LinkIndex: i.Link.Attrs().Index, + Dst: config.FixedCIDRv6, + }) + if err != nil && !os.IsExist(err) { + log.Errorf("Could not add route to IPv6 network %s via device %s", config.FixedCIDRv6.String(), config.BridgeName) + } return nil } diff --git a/drivers/bridge/setup_ipv6.go b/drivers/bridge/setup_ipv6.go index b797af8..b534644 100644 --- a/drivers/bridge/setup_ipv6.go +++ b/drivers/bridge/setup_ipv6.go @@ -5,12 +5,16 @@ import ( "io/ioutil" "net" + "github.com/Sirupsen/logrus" "github.com/vishvananda/netlink" ) var bridgeIPv6 *net.IPNet -const bridgeIPv6Str = "fe80::1/64" +const ( + bridgeIPv6Str = "fe80::1/64" + ipv6ForwardConfPerm = 0644 +) func init() { // We allow ourselves to panic in this special case because we indicate a @@ -25,7 +29,7 @@ func init() { func setupBridgeIPv6(config *networkConfiguration, i *bridgeInterface) error { // Enable IPv6 on the bridge procFile := "/proc/sys/net/ipv6/conf/" + config.BridgeName + "/disable_ipv6" - if err := ioutil.WriteFile(procFile, []byte{'0', '\n'}, 0644); err != nil { + if err := ioutil.WriteFile(procFile, []byte{'0', '\n'}, ipv6ForwardConfPerm); err != nil { return fmt.Errorf("Unable to enable IPv6 addresses on bridge: %v", err) } @@ -64,3 +68,14 @@ func setupGatewayIPv6(config *networkConfiguration, i *bridgeInterface) error { return nil } + +func setupIPv6Forwarding(config *networkConfiguration, i *bridgeInterface) error { + // Enable IPv6 forwarding + if err := ioutil.WriteFile("/proc/sys/net/ipv6/conf/default/forwarding", []byte{'1', '\n'}, ipv6ForwardConfPerm); err != nil { + logrus.Warnf("Unable to enable IPv6 default forwarding: %v", err) + } + if err := ioutil.WriteFile("/proc/sys/net/ipv6/conf/all/forwarding", []byte{'1', '\n'}, ipv6ForwardConfPerm); err != nil { + logrus.Warnf("Unable to enable IPv6 all forwarding: %v", err) + } + return nil +}