diff --git a/drivers/bridge/bridge.go b/drivers/bridge/bridge.go index 57a7f57..8fc05ae 100644 --- a/drivers/bridge/bridge.go +++ b/drivers/bridge/bridge.go @@ -596,21 +596,18 @@ func (d *driver) CreateNetwork(id types.UUID, option map[string]interface{}) err // networks. This step is needed now because driver might have now set the bridge // name on this config struct. And because we need to check for possible address // conflicts, so we need to check against operationa lnetworks. - if err := config.conflictsWithNetworks(id, networkList); err != nil { + if err = config.conflictsWithNetworks(id, networkList); err != nil { return err } setupNetworkIsolationRules := func(config *networkConfiguration, i *bridgeInterface) error { - defer func() { - if err != nil { - if err := network.isolateNetwork(networkList, false); err != nil { - logrus.Warnf("Failed on removing the inter-network iptables rules on cleanup: %v", err) - } + if err := network.isolateNetwork(networkList, true); err != nil { + if err := network.isolateNetwork(networkList, false); err != nil { + logrus.Warnf("Failed on removing the inter-network iptables rules on cleanup: %v", err) } - }() - - err := network.isolateNetwork(networkList, true) - return err + return err + } + return nil } // Prepare the bridge setup configuration @@ -766,17 +763,26 @@ func (d *driver) DeleteNetwork(nid types.UUID) error { } func addToBridge(ifaceName, bridgeName string) error { - iface, err := net.InterfaceByName(ifaceName) + link, err := netlink.LinkByName(ifaceName) if err != nil { return fmt.Errorf("could not find interface %s: %v", ifaceName, err) } + if err = netlink.LinkSetMaster(link, + &netlink.Bridge{LinkAttrs: netlink.LinkAttrs{Name: bridgeName}}); err != nil { + logrus.Debugf("Failed to add %s to bridge via netlink.Trying ioctl: %v", ifaceName, err) + iface, err := net.InterfaceByName(ifaceName) + if err != nil { + return fmt.Errorf("could not find network interface %s: %v", ifaceName, err) + } - master, err := net.InterfaceByName(bridgeName) - if err != nil { - return fmt.Errorf("could not find bridge %s: %v", bridgeName, err) + master, err := net.InterfaceByName(bridgeName) + if err != nil { + return fmt.Errorf("could not find bridge %s: %v", bridgeName, err) + } + + return ioctlAddToBridge(iface, master) } - - return ioctlAddToBridge(iface, master) + return nil } func setHairpinMode(link netlink.Link, enable bool) error { @@ -947,15 +953,14 @@ func (d *driver) CreateEndpoint(nid, eid types.UUID, epInfo driverapi.EndpointIn } // v4 address for the sandbox side pipe interface - sub := types.GetIPNetCanonical(n.bridge.bridgeIPv4) - ip4, err := ipAllocator.RequestIP(sub, nil) + ip4, err := ipAllocator.RequestIP(n.bridge.bridgeIPv4, nil) if err != nil { return err } ipv4Addr := &net.IPNet{IP: ip4, Mask: n.bridge.bridgeIPv4.Mask} // Down the interface before configuring mac address. - if err := netlink.LinkSetDown(sbox); err != nil { + if err = netlink.LinkSetDown(sbox); err != nil { return fmt.Errorf("could not set link down for container interface %s: %v", containerIfName, err) } @@ -968,7 +973,7 @@ func (d *driver) CreateEndpoint(nid, eid types.UUID, epInfo driverapi.EndpointIn endpoint.macAddress = mac // Up the host interface after finishing all netlink configuration - if err := netlink.LinkSetUp(host); err != nil { + if err = netlink.LinkSetUp(host); err != nil { return fmt.Errorf("could not set link up for host interface %s: %v", hostIfName, err) } @@ -1074,8 +1079,7 @@ func (d *driver) DeleteEndpoint(nid, eid types.UUID) error { n.releasePorts(ep) // Release the v4 address allocated to this endpoint's sandbox interface - sub := types.GetIPNetCanonical(n.bridge.bridgeIPv4) - err = ipAllocator.ReleaseIP(sub, ep.addr.IP) + err = ipAllocator.ReleaseIP(n.bridge.bridgeIPv4, ep.addr.IP) if err != nil { return err } diff --git a/drivers/bridge/bridge_test.go b/drivers/bridge/bridge_test.go index 760e3f6..ffd2c94 100644 --- a/drivers/bridge/bridge_test.go +++ b/drivers/bridge/bridge_test.go @@ -54,6 +54,17 @@ func TestCreateFullOptions(t *testing.T) { if err != nil { t.Fatalf("Failed to create bridge: %v", err) } + + // Verify the IP address allocated for the endpoint belongs to the container network + epOptions := make(map[string]interface{}) + te := &testEndpoint{ifaces: []*testInterface{}} + err = d.CreateEndpoint("dummy", "ep1", te, epOptions) + if err != nil { + t.Fatalf("Failed to create an endpoint : %s", err.Error()) + } + if !cnw.Contains(te.Interfaces()[0].Address().IP) { + t.Fatalf("endpoint got assigned address outside of container network(%s): %s", cnw.String(), te.Interfaces()[0].Address()) + } } func TestCreate(t *testing.T) { diff --git a/drivers/bridge/setup_device.go b/drivers/bridge/setup_device.go index 96eeee5..22bf64b 100644 --- a/drivers/bridge/setup_device.go +++ b/drivers/bridge/setup_device.go @@ -1,7 +1,11 @@ package bridge import ( + "fmt" + + "github.com/Sirupsen/logrus" "github.com/docker/docker/pkg/parsers/kernel" + "github.com/docker/libnetwork/netutils" "github.com/vishvananda/netlink" ) @@ -25,11 +29,25 @@ func setupDevice(config *networkConfiguration, i *bridgeInterface) error { // Only set the bridge's MAC address if the kernel version is > 3.3, as it // was not supported before that. kv, err := kernel.GetKernelVersion() - if err == nil && (kv.Kernel >= 3 && kv.Major >= 3) { - setMac = true + if err != nil { + logrus.Errorf("Failed to check kernel versions: %v. Will not assign a MAC address to the bridge interface", err) + } else { + setMac = kv.Kernel > 3 || (kv.Kernel == 3 && kv.Major >= 3) } - return ioctlCreateBridge(config.BridgeName, setMac) + if err = netlink.LinkAdd(i.Link); err != nil { + logrus.Debugf("Failed to create bridge %s via netlink. Trying ioctl", config.BridgeName) + return ioctlCreateBridge(config.BridgeName, setMac) + } + + if setMac { + hwAddr := netutils.GenerateRandomMAC() + if err = netlink.LinkSetHardwareAddr(i.Link, hwAddr); err != nil { + return fmt.Errorf("failed to set bridge mac-address %s : %s", hwAddr, err.Error()) + } + logrus.Debugf("Setting bridge mac address to %s", hwAddr) + } + return err } // SetupDeviceUp ups the given bridge interface. diff --git a/drivers/bridge/setup_ipv4.go b/drivers/bridge/setup_ipv4.go index cca715e..ac4535a 100644 --- a/drivers/bridge/setup_ipv4.go +++ b/drivers/bridge/setup_ipv4.go @@ -8,7 +8,6 @@ import ( log "github.com/Sirupsen/logrus" "github.com/docker/libnetwork/netutils" - "github.com/docker/libnetwork/types" "github.com/vishvananda/netlink" ) @@ -76,8 +75,12 @@ func setupBridgeIPv4(config *networkConfiguration, i *bridgeInterface) error { } func allocateBridgeIP(config *networkConfiguration, i *bridgeInterface) error { - sub := types.GetIPNetCanonical(i.bridgeIPv4) - ipAllocator.RequestIP(sub, i.bridgeIPv4.IP) + // Because of the way ipallocator manages the container address space, + // reserve bridge address only if it belongs to the container network + // (if defined), no need otherwise + if config.FixedCIDR == nil || config.FixedCIDR.Contains(i.bridgeIPv4.IP) { + ipAllocator.RequestIP(i.bridgeIPv4, i.bridgeIPv4.IP) + } return nil } @@ -112,10 +115,13 @@ func setupGatewayIPv4(config *networkConfiguration, i *bridgeInterface) error { return &ErrInvalidGateway{} } - // Pass the real network subnet to ip allocator (no host bits set) - sub := types.GetIPNetCanonical(i.bridgeIPv4) - if _, err := ipAllocator.RequestIP(sub, config.DefaultGatewayIPv4); err != nil { - return err + // Because of the way ipallocator manages the container address space, + // reserve default gw address only if it belongs to the container network + // (if defined), no need otherwise + if config.FixedCIDR == nil || config.FixedCIDR.Contains(config.DefaultGatewayIPv4) { + if _, err := ipAllocator.RequestIP(i.bridgeIPv4, config.DefaultGatewayIPv4); err != nil { + return err + } } // Store requested default gateway diff --git a/drivers/bridge/setup_ipv4_test.go b/drivers/bridge/setup_ipv4_test.go index 0d26a79..8564737 100644 --- a/drivers/bridge/setup_ipv4_test.go +++ b/drivers/bridge/setup_ipv4_test.go @@ -82,7 +82,7 @@ func TestSetupGatewayIPv4(t *testing.T) { ip, nw, _ := net.ParseCIDR("192.168.0.24/16") nw.IP = ip - gw := net.ParseIP("192.168.0.254") + gw := net.ParseIP("192.168.2.254") config := &networkConfiguration{ BridgeName: DefaultBridgeName, diff --git a/ipallocator/allocator.go b/ipallocator/allocator.go index 1560099..06bc051 100644 --- a/ipallocator/allocator.go +++ b/ipallocator/allocator.go @@ -66,7 +66,8 @@ func (a *IPAllocator) RegisterSubnet(network *net.IPNet, subnet *net.IPNet) erro a.mutex.Lock() defer a.mutex.Unlock() - key := network.String() + nw := &net.IPNet{IP: network.IP.Mask(network.Mask), Mask: network.Mask} + key := nw.String() if _, ok := a.allocatedIPs[key]; ok { return ErrNetworkAlreadyRegistered } @@ -90,10 +91,11 @@ func (a *IPAllocator) RequestIP(network *net.IPNet, ip net.IP) (net.IP, error) { a.mutex.Lock() defer a.mutex.Unlock() - key := network.String() + nw := &net.IPNet{IP: network.IP.Mask(network.Mask), Mask: network.Mask} + key := nw.String() allocated, ok := a.allocatedIPs[key] if !ok { - allocated = newAllocatedMap(network) + allocated = newAllocatedMap(nw) a.allocatedIPs[key] = allocated } @@ -109,7 +111,8 @@ func (a *IPAllocator) ReleaseIP(network *net.IPNet, ip net.IP) error { a.mutex.Lock() defer a.mutex.Unlock() - if allocated, exists := a.allocatedIPs[network.String()]; exists { + nw := &net.IPNet{IP: network.IP.Mask(network.Mask), Mask: network.Mask} + if allocated, exists := a.allocatedIPs[nw.String()]; exists { delete(allocated.p, ip.String()) } return nil diff --git a/libnetwork_test.go b/libnetwork_test.go index a139331..3bf62bf 100644 --- a/libnetwork_test.go +++ b/libnetwork_test.go @@ -1919,14 +1919,30 @@ func createGlobalInstance(t *testing.T) { "AllowNonDefaultBridge": true, }, } - net, err := createTestNetwork(bridgeNetType, "network", netOption) + + net1, err := controller.NetworkByName("testhost") if err != nil { - t.Fatal("new network") + t.Fatal(err) } - _, err = net.CreateEndpoint("ep1") + net2, err := createTestNetwork("bridge", "network2", netOption) if err != nil { - t.Fatal("createendpoint") + t.Fatal(err) + } + + _, err = net1.CreateEndpoint("pep1") + if err != nil { + t.Fatal(err) + } + + _, err = net2.CreateEndpoint("pep2") + if err != nil { + t.Fatal(err) + } + + _, err = net2.CreateEndpoint("pep3") + if err != nil { + t.Fatal(err) } } @@ -1940,12 +1956,18 @@ func debugf(format string, a ...interface{}) (int, error) { func parallelJoin(t *testing.T, ep libnetwork.Endpoint, thrNumber int) { debugf("J%d.", thrNumber) - err := ep.Join("racing_container") + var err error + if thrNumber == first { + err = ep.Join(fmt.Sprintf("%drace", thrNumber), libnetwork.JoinOptionUseDefaultSandbox()) + } else { + err = ep.Join(fmt.Sprintf("%drace", thrNumber)) + } + runtime.LockOSThread() if err != nil { if _, ok := err.(libnetwork.ErrNoContainer); !ok { if _, ok := err.(libnetwork.ErrInvalidJoin); !ok { - t.Fatal(err) + t.Fatalf("thread %d: %v", thrNumber, err) } } debugf("JE%d(%v).", thrNumber, err) @@ -1955,12 +1977,18 @@ func parallelJoin(t *testing.T, ep libnetwork.Endpoint, thrNumber int) { func parallelLeave(t *testing.T, ep libnetwork.Endpoint, thrNumber int) { debugf("L%d.", thrNumber) - err := ep.Leave("racing_container") + var err error + if thrNumber == first { + err = ep.Leave(fmt.Sprintf("%drace", thrNumber)) + } else { + err = controller.LeaveAll(fmt.Sprintf("%drace", thrNumber)) + } + runtime.LockOSThread() if err != nil { if _, ok := err.(libnetwork.ErrNoContainer); !ok { if _, ok := err.(libnetwork.ErrInvalidJoin); !ok { - t.Fatal(err) + t.Fatalf("thread %d: %v", thrNumber, err) } } debugf("LE%d(%v).", thrNumber, err) @@ -2012,15 +2040,33 @@ func runParallelTests(t *testing.T, thrNumber int) { } defer netns.Set(origns) - net, err := controller.NetworkByName("network") + net1, err := controller.NetworkByName("testhost") if err != nil { t.Fatal(err) } - if net == nil { - t.Fatal("Could not find network") + if net1 == nil { + t.Fatal("Could not find network1") + } + + net2, err := controller.NetworkByName("network2") + if err != nil { + t.Fatal(err) + } + if net2 == nil { + t.Fatal("Could not find network2") + } + + epName := fmt.Sprintf("pep%d", thrNumber) + + //var err error + var ep libnetwork.Endpoint + + if thrNumber == first { + ep, err = net1.EndpointByName(epName) + } else { + ep, err = net2.EndpointByName(epName) } - ep, err := net.EndpointByName("ep1") if err != nil { t.Fatal(err) } @@ -2035,6 +2081,11 @@ func runParallelTests(t *testing.T, thrNumber int) { debugf("\n") + err = ep.Delete() + if err != nil { + t.Fatal(err) + } + if thrNumber == first { for thrdone := range done { select { @@ -2043,12 +2094,8 @@ func runParallelTests(t *testing.T, thrNumber int) { } testns.Close() - err = ep.Delete() - if err != nil { - t.Fatal(err) - } - if err := net.Delete(); err != nil { + if err := net2.Delete(); err != nil { t.Fatal(err) } } diff --git a/netutils/utils.go b/netutils/utils.go index 0ef357e..cb430eb 100644 --- a/netutils/utils.go +++ b/netutils/utils.go @@ -74,20 +74,22 @@ func NetworkOverlaps(netX *net.IPNet, netY *net.IPNet) bool { // NetworkRange calculates the first and last IP addresses in an IPNet func NetworkRange(network *net.IPNet) (net.IP, net.IP) { - var netIP net.IP - if network.IP.To4() != nil { - netIP = network.IP.To4() - } else if network.IP.To16() != nil { - netIP = network.IP.To16() - } else { + if network == nil { return nil, nil } - lastIP := make([]byte, len(netIP), len(netIP)) - for i := 0; i < len(netIP); i++ { - lastIP[i] = netIP[i] | ^network.Mask[i] + firstIP := network.IP.Mask(network.Mask) + lastIP := types.GetIPCopy(firstIP) + for i := 0; i < len(firstIP); i++ { + lastIP[i] = firstIP[i] | ^network.Mask[i] } - return netIP.Mask(network.Mask), net.IP(lastIP) + + if network.IP.To4() != nil { + firstIP = firstIP.To4() + lastIP = lastIP.To4() + } + + return firstIP, lastIP } // GetIfaceAddr returns the first IPv4 address and slice of IPv6 addresses for the specified network interface diff --git a/sandboxdata.go b/sandboxdata.go index 6b217a8..9b0d8ea 100644 --- a/sandboxdata.go +++ b/sandboxdata.go @@ -139,10 +139,15 @@ func (s *sandboxData) rmEndpoint(ep *endpoint) { } } - // We don't check if s.endpoints is empty here because - // it should never be empty during a rmEndpoint call and - // if it is we will rightfully panic here s.Lock() + if len(s.endpoints) == 0 { + // s.endpoints should never be empty and this is unexpected error condition + // We log an error message to note this down for debugging purposes. + logrus.Errorf("No endpoints in sandbox while trying to remove endpoint %s", ep.Name()) + s.Unlock() + return + } + highEpBefore := s.endpoints[0] var ( i int @@ -245,7 +250,10 @@ func (c *controller) LeaveAll(id string) error { } sData.sandbox().Destroy() + + c.Lock() delete(c.sandboxes, sandbox.GenerateKey(id)) + c.Unlock() return nil }