From 646105752117e8a7079e5e308f521a9a267c332f Mon Sep 17 00:00:00 2001 From: Alessandro Boch Date: Mon, 27 Jul 2015 16:48:30 -0700 Subject: [PATCH] Misc fixes to ipallocator & bridge driver about FixedCIDR - NetworkRange() function on which ipallocatore relies to compute the subnet limits has a bug in computing the upper limit IP - in case container subnet is specified (fixedCIDR), bridge driver to reserve bridge and gateway addresses only if they belong to the container subnet - Make ipallocator more robust in using converting the passed network to a canonical one before using it as a key in its public APIs Signed-off-by: Alessandro Boch --- libnetwork/drivers/bridge/bridge.go | 6 ++---- libnetwork/drivers/bridge/bridge_test.go | 11 ++++++++++ libnetwork/drivers/bridge/setup_ipv4.go | 20 +++++++++++------- libnetwork/drivers/bridge/setup_ipv4_test.go | 2 +- libnetwork/ipallocator/allocator.go | 11 ++++++---- libnetwork/netutils/utils.go | 22 +++++++++++--------- 6 files changed, 46 insertions(+), 26 deletions(-) diff --git a/libnetwork/drivers/bridge/bridge.go b/libnetwork/drivers/bridge/bridge.go index 57a7f575d6..22bdf37a98 100644 --- a/libnetwork/drivers/bridge/bridge.go +++ b/libnetwork/drivers/bridge/bridge.go @@ -947,8 +947,7 @@ 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 } @@ -1074,8 +1073,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/libnetwork/drivers/bridge/bridge_test.go b/libnetwork/drivers/bridge/bridge_test.go index 760e3f699d..ffd2c94c90 100644 --- a/libnetwork/drivers/bridge/bridge_test.go +++ b/libnetwork/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/libnetwork/drivers/bridge/setup_ipv4.go b/libnetwork/drivers/bridge/setup_ipv4.go index cca715e392..ac4535adb9 100644 --- a/libnetwork/drivers/bridge/setup_ipv4.go +++ b/libnetwork/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/libnetwork/drivers/bridge/setup_ipv4_test.go b/libnetwork/drivers/bridge/setup_ipv4_test.go index 0d26a790c8..85647373c7 100644 --- a/libnetwork/drivers/bridge/setup_ipv4_test.go +++ b/libnetwork/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/libnetwork/ipallocator/allocator.go b/libnetwork/ipallocator/allocator.go index 1560099937..06bc051c55 100644 --- a/libnetwork/ipallocator/allocator.go +++ b/libnetwork/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/netutils/utils.go b/libnetwork/netutils/utils.go index 0ef357ec44..cb430eb03f 100644 --- a/libnetwork/netutils/utils.go +++ b/libnetwork/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