libnet/ipam: simplify the data model

The address spaces are orthogonal. There is no shared state between them
logically so there is no reason for them to share any in-memory data
structures. addrSpace is responsible for allocating subnets and
addresses, while Allocator is responsible for implementing the IPAM API.
Lower all the implementation details of allocation into addrSpace.

There is no longer a need to include the name of the address space in
the map keys for subnets now that each addrSpace holds its own state
independently from other addrSpaces. Remove the AddressSpace field from
the struct used for map keys within an addrSpace so that an addrSpace
does not need to know its own name.

Pool allocations were encoded in a tree structure, using parent
references and reference counters. This structure affords for pools
subdivided an arbitrary number of times to be modeled, in theory. In
practice, the max depth is only two: master pools and sub-pools. The
allocator data model has also been heavily influenced by the
requirements and limitations of Datastore persistence, which are no
longer applicable.

Address allocations are always associated with a master pool. Sub-pools
only serve to restrict the range of addresses within the master pool
which could be allocated from. Model pool allocations within an address
space as a two-level hierarchy of maps.

Signed-off-by: Cory Snider <csnider@mirantis.com>

libnetwork/ipam/allocator.go

@@ -3,9 +3,9 @@ package ipam
 import (
 	"fmt"
 	"net"
-	"sync"
+	"strings"
 
-	"github.com/docker/docker/libnetwork/bitseq"
+	"github.com/docker/docker/libnetwork/bitmap"
 	"github.com/docker/docker/libnetwork/ipamapi"
 	"github.com/docker/docker/libnetwork/types"
 	"github.com/sirupsen/logrus"
@@ -20,27 +20,19 @@ const (
 type Allocator struct {
 	// The address spaces
 	local, global *addrSpace
-	// Allocated addresses in each address space's subnet
-	addresses map[SubnetKey]*bitseq.Handle
-	sync.Mutex
 }
 
 // NewAllocator returns an instance of libnetwork ipam
 func NewAllocator(lcAs, glAs []*net.IPNet) (*Allocator, error) {
-	a := &Allocator{
-		addresses: map[SubnetKey]*bitseq.Handle{},
-	}
-
-	a.local = newAddrSpace(a, lcAs)
-	a.global = newAddrSpace(a, glAs)
-
-	return a, nil
+	return &Allocator{
+		local:  newAddrSpace(lcAs),
+		global: newAddrSpace(glAs),
+	}, nil
 }
 
-func newAddrSpace(a *Allocator, predefined []*net.IPNet) *addrSpace {
+func newAddrSpace(predefined []*net.IPNet) *addrSpace {
 	return &addrSpace{
-		subnets:    map[SubnetKey]*PoolData{},
-		alloc:      a,
+		subnets:    map[string]*PoolData{},
 		predefined: predefined,
 	}
 }
@@ -69,45 +61,38 @@ func (a *Allocator) RequestPool(addressSpace, pool, subPool string, options map[
 	if err != nil {
 		return "", nil, nil, err
 	}
-	k := SubnetKey{AddressSpace: addressSpace}
+	k := PoolID{AddressSpace: addressSpace}
 
-	if pool == "" && subPool != "" {
-		return parseErr(ipamapi.ErrInvalidSubPool)
+	if pool == "" {
+		if subPool != "" {
+			return parseErr(ipamapi.ErrInvalidSubPool)
+		}
+		var nw *net.IPNet
+		nw, k.SubnetKey, err = aSpace.allocatePredefinedPool(v6)
+		if err != nil {
+			return "", nil, nil, err
+		}
+		return k.String(), nw, nil, nil
 	}
-	pdf := pool == ""
 
 	var (
-		nw  *net.IPNet
-		ipr *AddressRange
+		nw, sub *net.IPNet
 	)
-	if !pdf {
-		if _, nw, err = net.ParseCIDR(pool); err != nil {
-			return parseErr(ipamapi.ErrInvalidPool)
-		}
-		k.Subnet = nw.String()
-		k.ChildSubnet = subPool
-
-		if subPool != "" {
-			if ipr, err = getAddressRange(subPool, nw); err != nil {
-				return parseErr(err)
-			}
-		}
-
+	if _, nw, err = net.ParseCIDR(pool); err != nil {
+		return parseErr(ipamapi.ErrInvalidPool)
 	}
 
-retry:
-	if pdf {
-		if nw, err = a.getPredefinedPool(addressSpace, v6); err != nil {
-			return "", nil, nil, err
+	if subPool != "" {
+		var err error
+		_, sub, err = net.ParseCIDR(subPool)
+		if err != nil {
+			return parseErr(ipamapi.ErrInvalidSubPool)
 		}
-		k.Subnet = nw.String()
+		k.ChildSubnet = subPool
 	}
 
-	if err := aSpace.allocateSubnet(k, nw, ipr, pdf); err != nil {
-		if _, ok := err.(types.MaskableError); ok {
-			logrus.Debugf("Retrying predefined pool search: %v", err)
-			goto retry
-		}
+	k.SubnetKey, err = aSpace.allocateSubnet(nw, sub)
+	if err != nil {
 		return "", nil, nil, err
 	}
 
@@ -117,7 +102,7 @@ retry:
 // ReleasePool releases the address pool identified by the passed id
 func (a *Allocator) ReleasePool(poolID string) error {
 	logrus.Debugf("ReleasePool(%s)", poolID)
-	k := SubnetKey{}
+	k := PoolID{}
 	if err := k.FromString(poolID); err != nil {
 		return types.BadRequestErrorf("invalid pool id: %s", poolID)
 	}
@@ -127,14 +112,12 @@ func (a *Allocator) ReleasePool(poolID string) error {
 		return err
 	}
 
-	return aSpace.releaseSubnet(k)
+	return aSpace.releaseSubnet(k.SubnetKey)
 }
 
 // Given the address space, returns the local or global PoolConfig based on whether the
 // address space is local or global. AddressSpace locality is registered with IPAM out of band.
 func (a *Allocator) getAddrSpace(as string) (*addrSpace, error) {
-	a.Lock()
-	defer a.Unlock()
 	switch as {
 	case localAddressSpace:
 		return a.local, nil
@@ -144,9 +127,7 @@ func (a *Allocator) getAddrSpace(as string) (*addrSpace, error) {
 	return nil, types.BadRequestErrorf("cannot find address space %s", as)
 }
 
-func (a *Allocator) insertBitMask(key SubnetKey, pool *net.IPNet) error {
-	//logrus.Debugf("Inserting bitmask (%s, %s)", key.String(), pool.String())
-
+func newPoolData(pool *net.IPNet) *PoolData {
 	ipVer := getAddressVersion(pool.IP)
 	ones, bits := pool.Mask.Size()
 	numAddresses := uint64(1 << uint(bits-ones))
@@ -157,10 +138,7 @@ func (a *Allocator) insertBitMask(key SubnetKey, pool *net.IPNet) error {
 	}
 
 	// Generate the new address masks.
-	h, err := bitseq.NewHandle("", nil, "", numAddresses)
-	if err != nil {
-		return err
-	}
+	h := bitmap.New(numAddresses)
 
 	// Pre-reserve the network address on IPv4 networks large
 	// enough to have one (i.e., anything bigger than a /31.
@@ -174,26 +152,7 @@ func (a *Allocator) insertBitMask(key SubnetKey, pool *net.IPNet) error {
 		h.Set(numAddresses - 1)
 	}
 
-	a.Lock()
-	a.addresses[key] = h
-	a.Unlock()
-	return nil
-}
-
-func (a *Allocator) retrieveBitmask(k SubnetKey, n *net.IPNet) (*bitseq.Handle, error) {
-	a.Lock()
-	bm, ok := a.addresses[k]
-	a.Unlock()
-	if !ok {
-		logrus.Debugf("Retrieving bitmask (%s, %s)", k, n)
-		if err := a.insertBitMask(k, n); err != nil {
-			return nil, types.InternalErrorf("could not find bitmask for %s", k.String())
-		}
-		a.Lock()
-		bm = a.addresses[k]
-		a.Unlock()
-	}
-	return bm, nil
+	return &PoolData{Pool: pool, addrs: h, children: map[string]struct{}{}}
 }
 
 // getPredefineds returns the predefined subnets for the address space.
@@ -219,18 +178,13 @@ func (aSpace *addrSpace) updatePredefinedStartIndex(amt int) {
 	aSpace.predefinedStartIndex = i
 }
 
-func (a *Allocator) getPredefinedPool(as string, ipV6 bool) (*net.IPNet, error) {
+func (aSpace *addrSpace) allocatePredefinedPool(ipV6 bool) (*net.IPNet, SubnetKey, error) {
 	var v ipVersion
 	v = v4
 	if ipV6 {
 		v = v6
 	}
 
-	aSpace, _ := a.getAddrSpace(as)
-	if aSpace == nil {
-		return nil, types.NotImplementedErrorf("no default pool available for non-default address spaces")
-	}
-
 	aSpace.Lock()
 	defer aSpace.Unlock()
 
@@ -239,24 +193,28 @@ func (a *Allocator) getPredefinedPool(as string, ipV6 bool) (*net.IPNet, error)
 			continue
 		}
 		// Checks whether pool has already been allocated
-		if _, ok := aSpace.subnets[SubnetKey{AddressSpace: as, Subnet: nw.String()}]; ok {
+		if _, ok := aSpace.subnets[nw.String()]; ok {
 			continue
 		}
 		// Shouldn't be necessary, but check prevents IP collisions should
 		// predefined pools overlap for any reason.
-		if !aSpace.contains(as, nw) {
+		if !aSpace.contains(nw) {
 			aSpace.updatePredefinedStartIndex(i + 1)
-			return nw, nil
+			k, err := aSpace.allocateSubnetL(nw, nil)
+			if err != nil {
+				return nil, SubnetKey{}, err
+			}
+			return nw, k, nil
 		}
 	}
 
-	return nil, types.NotFoundErrorf("could not find an available, non-overlapping IPv%d address pool among the defaults to assign to the network", v)
+	return nil, SubnetKey{}, types.NotFoundErrorf("could not find an available, non-overlapping IPv%d address pool among the defaults to assign to the network", v)
 }
 
 // RequestAddress returns an address from the specified pool ID
 func (a *Allocator) RequestAddress(poolID string, prefAddress net.IP, opts map[string]string) (*net.IPNet, map[string]string, error) {
 	logrus.Debugf("RequestAddress(%s, %v, %v)", poolID, prefAddress, opts)
-	k := SubnetKey{}
+	k := PoolID{}
 	if err := k.FromString(poolID); err != nil {
 		return nil, nil, types.BadRequestErrorf("invalid pool id: %s", poolID)
 	}
@@ -265,40 +223,41 @@ func (a *Allocator) RequestAddress(poolID string, prefAddress net.IP, opts map[s
 	if err != nil {
 		return nil, nil, err
 	}
+	return aSpace.requestAddress(k.SubnetKey, prefAddress, opts)
+}
 
+func (aSpace *addrSpace) requestAddress(k SubnetKey, prefAddress net.IP, opts map[string]string) (*net.IPNet, map[string]string, error) {
 	aSpace.Lock()
-	p, ok := aSpace.subnets[k]
+	defer aSpace.Unlock()
+
+	p, ok := aSpace.subnets[k.Subnet]
 	if !ok {
-		aSpace.Unlock()
-		return nil, nil, types.NotFoundErrorf("cannot find address pool for poolID:%s", poolID)
+		return nil, nil, types.NotFoundErrorf("cannot find address pool for poolID:%+v", k)
 	}
 
 	if prefAddress != nil && !p.Pool.Contains(prefAddress) {
-		aSpace.Unlock()
 		return nil, nil, ipamapi.ErrIPOutOfRange
 	}
 
-	c := p
-	for c.Range != nil {
-		k = c.ParentKey
-		c = aSpace.subnets[k]
+	var ipr *AddressRange
+	if k.ChildSubnet != "" {
+		if _, ok := p.children[k.ChildSubnet]; !ok {
+			return nil, nil, types.NotFoundErrorf("cannot find address pool for poolID:%+v", k)
+		}
+		_, sub, err := net.ParseCIDR(k.ChildSubnet)
+		if err != nil {
+			return nil, nil, types.NotFoundErrorf("cannot find address pool for poolID:%+v: %v", k, err)
+		}
+		ipr, err = getAddressRange(sub, p.Pool)
+		if err != nil {
+			return nil, nil, err
+		}
 	}
-	aSpace.Unlock()
 
-	bm, err := a.retrieveBitmask(k, c.Pool)
-	if err != nil {
-		return nil, nil, types.InternalErrorf("could not find bitmask for %s on address %v request from pool %s: %v",
-			k.String(), prefAddress, poolID, err)
-	}
 	// In order to request for a serial ip address allocation, callers can pass in the option to request
 	// IP allocation serially or first available IP in the subnet
-	var serial bool
-	if opts != nil {
-		if val, ok := opts[ipamapi.AllocSerialPrefix]; ok {
-			serial = (val == "true")
-		}
-	}
-	ip, err := a.getAddress(p.Pool, bm, prefAddress, p.Range, serial)
+	serial := opts[ipamapi.AllocSerialPrefix] == "true"
+	ip, err := getAddress(p.Pool, p.addrs, prefAddress, ipr, serial)
 	if err != nil {
 		return nil, nil, err
 	}
@@ -309,7 +268,7 @@ func (a *Allocator) RequestAddress(poolID string, prefAddress net.IP, opts map[s
 // ReleaseAddress releases the address from the specified pool ID
 func (a *Allocator) ReleaseAddress(poolID string, address net.IP) error {
 	logrus.Debugf("ReleaseAddress(%s, %v)", poolID, address)
-	k := SubnetKey{}
+	k := PoolID{}
 	if err := k.FromString(poolID); err != nil {
 		return types.BadRequestErrorf("invalid pool id: %s", poolID)
 	}
@@ -319,48 +278,44 @@ func (a *Allocator) ReleaseAddress(poolID string, address net.IP) error {
 		return err
 	}
 
+	return aSpace.releaseAddress(k.SubnetKey, address)
+}
+
+func (aSpace *addrSpace) releaseAddress(k SubnetKey, address net.IP) error {
 	aSpace.Lock()
-	p, ok := aSpace.subnets[k]
+	defer aSpace.Unlock()
+
+	p, ok := aSpace.subnets[k.Subnet]
 	if !ok {
-		aSpace.Unlock()
-		return types.NotFoundErrorf("cannot find address pool for poolID:%s", poolID)
+		return types.NotFoundErrorf("cannot find address pool for %+v", k)
+	}
+	if k.ChildSubnet != "" {
+		if _, ok := p.children[k.ChildSubnet]; !ok {
+			return types.NotFoundErrorf("cannot find address pool for poolID:%+v", k)
+		}
 	}
 
 	if address == nil {
-		aSpace.Unlock()
 		return types.BadRequestErrorf("invalid address: nil")
 	}
 
 	if !p.Pool.Contains(address) {
-		aSpace.Unlock()
 		return ipamapi.ErrIPOutOfRange
 	}
 
-	c := p
-	for c.Range != nil {
-		k = c.ParentKey
-		c = aSpace.subnets[k]
-	}
-	aSpace.Unlock()
-
 	mask := p.Pool.Mask
 
 	h, err := types.GetHostPartIP(address, mask)
 	if err != nil {
-		return types.InternalErrorf("failed to release address %s: %v", address.String(), err)
+		return types.InternalErrorf("failed to release address %s: %v", address, err)
 	}
 
-	bm, err := a.retrieveBitmask(k, c.Pool)
-	if err != nil {
-		return types.InternalErrorf("could not find bitmask for %s on address %v release from pool %s: %v",
-			k.String(), address, poolID, err)
-	}
-	defer logrus.Debugf("Released address PoolID:%s, Address:%v Sequence:%s", poolID, address, bm)
+	defer logrus.Debugf("Released address Address:%v Sequence:%s", address, p.addrs)
 
-	return bm.Unset(ipToUint64(h))
+	return p.addrs.Unset(ipToUint64(h))
 }
 
-func (a *Allocator) getAddress(nw *net.IPNet, bitmask *bitseq.Handle, prefAddress net.IP, ipr *AddressRange, serial bool) (net.IP, error) {
+func getAddress(nw *net.IPNet, bitmask *bitmap.Bitmap, prefAddress net.IP, ipr *AddressRange, serial bool) (net.IP, error) {
 	var (
 		ordinal uint64
 		err     error
@@ -390,9 +345,9 @@ func (a *Allocator) getAddress(nw *net.IPNet, bitmask *bitseq.Handle, prefAddres
 	case nil:
 		// Convert IP ordinal for this subnet into IP address
 		return generateAddress(ordinal, base), nil
-	case bitseq.ErrBitAllocated:
+	case bitmap.ErrBitAllocated:
 		return nil, ipamapi.ErrIPAlreadyAllocated
-	case bitseq.ErrNoBitAvailable:
+	case bitmap.ErrNoBitAvailable:
 		return nil, ipamapi.ErrNoAvailableIPs
 	default:
 		return nil, err
@@ -401,33 +356,32 @@ func (a *Allocator) getAddress(nw *net.IPNet, bitmask *bitseq.Handle, prefAddres
 
 // DumpDatabase dumps the internal info
 func (a *Allocator) DumpDatabase() string {
-	a.Lock()
 	aspaces := map[string]*addrSpace{
 		localAddressSpace:  a.local,
 		globalAddressSpace: a.global,
 	}
-	a.Unlock()
 
-	var s string
+	var b strings.Builder
 	for _, as := range []string{localAddressSpace, globalAddressSpace} {
-		aSpace := aspaces[as]
-		s = fmt.Sprintf("\n\n%s Config", as)
-		aSpace.Lock()
-		for k, config := range aSpace.subnets {
-			s += fmt.Sprintf("\n%v: %v", k, config)
-			if config.Range == nil {
-				a.retrieveBitmask(k, config.Pool)
-			}
-		}
-		aSpace.Unlock()
+		fmt.Fprintf(&b, "\n### %s\n", as)
+		b.WriteString(aspaces[as].DumpDatabase())
 	}
+	return b.String()
+}
 
-	s = fmt.Sprintf("%s\n\nBitmasks", s)
-	for k, bm := range a.addresses {
-		s += fmt.Sprintf("\n%s: %s", k, bm)
-	}
+func (aSpace *addrSpace) DumpDatabase() string {
+	aSpace.Lock()
+	defer aSpace.Unlock()
 
-	return s
+	var b strings.Builder
+	for k, config := range aSpace.subnets {
+		fmt.Fprintf(&b, "%v: %v\n", k, config)
+		fmt.Fprintf(&b, "  Bitmap: %v\n", config.addrs)
+		for k := range config.children {
+			fmt.Fprintf(&b, "  - Subpool: %v\n", k)
+		}
+	}
+	return b.String()
 }
 
 // IsBuiltIn returns true for builtin drivers

libnetwork/ipam/allocator_test.go

@@ -12,7 +12,7 @@ import (
 	"testing"
 	"time"
 
-	"github.com/docker/docker/libnetwork/bitseq"
+	"github.com/docker/docker/libnetwork/bitmap"
 	"github.com/docker/docker/libnetwork/ipamapi"
 	"github.com/docker/docker/libnetwork/ipamutils"
 	"github.com/docker/docker/libnetwork/types"
@@ -48,13 +48,13 @@ func TestGetAddressVersion(t *testing.T) {
 }
 
 func TestKeyString(t *testing.T) {
-	k := &SubnetKey{AddressSpace: "default", Subnet: "172.27.0.0/16"}
+	k := &PoolID{AddressSpace: "default", SubnetKey: SubnetKey{Subnet: "172.27.0.0/16"}}
 	expected := "default/172.27.0.0/16"
 	if expected != k.String() {
 		t.Fatalf("Unexpected key string: %s", k.String())
 	}
 
-	k2 := &SubnetKey{}
+	k2 := &PoolID{}
 	err := k2.FromString(expected)
 	if err != nil {
 		t.Fatal(err)
@@ -158,7 +158,7 @@ func TestAddReleasePoolID(t *testing.T) {
 	a, err := NewAllocator(ipamutils.GetLocalScopeDefaultNetworks(), ipamutils.GetGlobalScopeDefaultNetworks())
 	assert.NilError(t, err)
 
-	var k0, k1 SubnetKey
+	var k0, k1 PoolID
 	_, err = a.getAddrSpace(localAddressSpace)
 	if err != nil {
 		t.Fatal(err)
@@ -166,7 +166,7 @@ func TestAddReleasePoolID(t *testing.T) {
 
 	pid0, _, _, err := a.RequestPool(localAddressSpace, "10.0.0.0/8", "", nil, false)
 	if err != nil {
-		t.Fatal("Unexpected failure in adding pool")
+		t.Fatalf("Unexpected failure in adding pool: %v", err)
 	}
 	if err := k0.FromString(pid0); err != nil {
 		t.Fatal(err)
@@ -177,15 +177,13 @@ func TestAddReleasePoolID(t *testing.T) {
 		t.Fatal(err)
 	}
 
-	subnets := aSpace.subnets
-
-	if subnets[k0].RefCount != 1 {
-		t.Fatalf("Unexpected ref count for %s: %d", k0, subnets[k0].RefCount)
+	if got := aSpace.subnets[k0.Subnet].autoRelease; got != false {
+		t.Errorf("Unexpected autoRelease value for %s: %v", k0, got)
 	}
 
 	pid1, _, _, err := a.RequestPool(localAddressSpace, "10.0.0.0/8", "10.0.0.0/16", nil, false)
 	if err != nil {
-		t.Fatal("Unexpected failure in adding sub pool")
+		t.Fatalf("Unexpected failure in adding sub pool: %v", err)
 	}
 	if err := k1.FromString(pid1); err != nil {
 		t.Fatal(err)
@@ -200,14 +198,13 @@ func TestAddReleasePoolID(t *testing.T) {
 		t.Fatal(err)
 	}
 
-	subnets = aSpace.subnets
-	if subnets[k1].RefCount != 1 {
-		t.Fatalf("Unexpected ref count for %s: %d", k1, subnets[k1].RefCount)
+	if got := aSpace.subnets[k1.Subnet].autoRelease; got != false {
+		t.Errorf("Unexpected autoRelease value for %s: %v", k1, got)
 	}
 
 	_, _, _, err = a.RequestPool(localAddressSpace, "10.0.0.0/8", "10.0.0.0/16", nil, false)
 	if err == nil {
-		t.Fatal("Expected failure in adding sub pool")
+		t.Fatalf("Expected failure in adding sub pool: %v", err)
 	}
 
 	aSpace, err = a.getAddrSpace(localAddressSpace)
@@ -215,10 +212,8 @@ func TestAddReleasePoolID(t *testing.T) {
 		t.Fatal(err)
 	}
 
-	subnets = aSpace.subnets
-
-	if subnets[k0].RefCount != 2 {
-		t.Fatalf("Unexpected ref count for %s: %d", k0, subnets[k0].RefCount)
+	if got := aSpace.subnets[k0.Subnet].autoRelease; got != false {
+		t.Errorf("Unexpected autoRelease value for %s: %v", k0, got)
 	}
 
 	if err := a.ReleasePool(pid1); err != nil {
@@ -230,20 +225,23 @@ func TestAddReleasePoolID(t *testing.T) {
 		t.Fatal(err)
 	}
 
-	subnets = aSpace.subnets
-	if subnets[k0].RefCount != 1 {
-		t.Fatalf("Unexpected ref count for %s: %d", k0, subnets[k0].RefCount)
+	if got := aSpace.subnets[k0.Subnet].autoRelease; got != false {
+		t.Errorf("Unexpected autoRelease value for %s: %v", k0, got)
 	}
 	if err := a.ReleasePool(pid0); err != nil {
-		t.Fatal(err)
+		t.Error(err)
+	}
+
+	if _, ok := aSpace.subnets[k0.Subnet]; ok {
+		t.Error("Pool should have been deleted when released")
 	}
 
 	pid00, _, _, err := a.RequestPool(localAddressSpace, "10.0.0.0/8", "", nil, false)
 	if err != nil {
-		t.Fatal("Unexpected failure in adding pool")
+		t.Errorf("Unexpected failure in adding pool: %v", err)
 	}
 	if pid00 != pid0 {
-		t.Fatal("main pool should still exist")
+		t.Errorf("main pool should still exist. Got poolID %q, want %q", pid00, pid0)
 	}
 
 	aSpace, err = a.getAddrSpace(localAddressSpace)
@@ -251,13 +249,12 @@ func TestAddReleasePoolID(t *testing.T) {
 		t.Fatal(err)
 	}
 
-	subnets = aSpace.subnets
-	if subnets[k0].RefCount != 1 {
-		t.Fatalf("Unexpected ref count for %s: %d", k0, subnets[k0].RefCount)
+	if got := aSpace.subnets[k0.Subnet].autoRelease; got != false {
+		t.Errorf("Unexpected autoRelease value for %s: %v", k0, got)
 	}
 
 	if err := a.ReleasePool(pid00); err != nil {
-		t.Fatal(err)
+		t.Error(err)
 	}
 
 	aSpace, err = a.getAddrSpace(localAddressSpace)
@@ -265,14 +262,13 @@ func TestAddReleasePoolID(t *testing.T) {
 		t.Fatal(err)
 	}
 
-	subnets = aSpace.subnets
-	if bp, ok := subnets[k0]; ok {
-		t.Fatalf("Base pool %s is still present: %v", k0, bp)
+	if bp, ok := aSpace.subnets[k0.Subnet]; ok {
+		t.Errorf("Base pool %s is still present: %v", k0, bp)
 	}
 
 	_, _, _, err = a.RequestPool(localAddressSpace, "10.0.0.0/8", "", nil, false)
 	if err != nil {
-		t.Fatal("Unexpected failure in adding pool")
+		t.Errorf("Unexpected failure in adding pool: %v", err)
 	}
 
 	aSpace, err = a.getAddrSpace(localAddressSpace)
@@ -280,9 +276,8 @@ func TestAddReleasePoolID(t *testing.T) {
 		t.Fatal(err)
 	}
 
-	subnets = aSpace.subnets
-	if subnets[k0].RefCount != 1 {
-		t.Fatalf("Unexpected ref count for %s: %d", k0, subnets[k0].RefCount)
+	if got := aSpace.subnets[k0.Subnet].autoRelease; got != false {
+		t.Errorf("Unexpected autoRelease value for %s: %v", k0, got)
 	}
 }
 
@@ -290,19 +285,16 @@ func TestPredefinedPool(t *testing.T) {
 	a, err := NewAllocator(ipamutils.GetLocalScopeDefaultNetworks(), ipamutils.GetGlobalScopeDefaultNetworks())
 	assert.NilError(t, err)
 
-	if _, err := a.getPredefinedPool("blue", false); err == nil {
-		t.Fatal("Expected failure for non default addr space")
-	}
-
 	pid, nw, _, err := a.RequestPool(localAddressSpace, "", "", nil, false)
 	if err != nil {
 		t.Fatal(err)
 	}
 
-	nw2, err := a.getPredefinedPool(localAddressSpace, false)
+	pid2, nw2, _, err := a.RequestPool(localAddressSpace, "", "", nil, false)
 	if err != nil {
 		t.Fatal(err)
 	}
+
 	if types.CompareIPNet(nw, nw2) {
 		t.Fatalf("Unexpected default network returned: %s = %s", nw2, nw)
 	}
@@ -310,6 +302,10 @@ func TestPredefinedPool(t *testing.T) {
 	if err := a.ReleasePool(pid); err != nil {
 		t.Fatal(err)
 	}
+
+	if err := a.ReleasePool(pid2); err != nil {
+		t.Fatal(err)
+	}
 }
 
 func TestRemoveSubnet(t *testing.T) {
@@ -958,7 +954,7 @@ func TestRelease(t *testing.T) {
 	for i, inp := range toRelease {
 		ip0 := net.ParseIP(inp.address)
 		a.ReleaseAddress(pid, ip0)
-		bm := a.addresses[SubnetKey{localAddressSpace, subnet, ""}]
+		bm := a.local.subnets[subnet].addrs
 		if bm.Unselected() != 1 {
 			t.Fatalf("Failed to update free address count after release. Expected %d, Found: %d", i+1, bm.Unselected())
 		}
@@ -978,7 +974,6 @@ func assertGetAddress(t *testing.T, subnet string) {
 	var (
 		err       error
 		printTime = false
-		a         = &Allocator{}
 	)
 
 	_, sub, _ := net.ParseCIDR(subnet)
@@ -986,15 +981,12 @@ func assertGetAddress(t *testing.T, subnet string) {
 	zeroes := bits - ones
 	numAddresses := 1 << uint(zeroes)
 
-	bm, err := bitseq.NewHandle("ipam_test", nil, "default/"+subnet, uint64(numAddresses))
-	if err != nil {
-		t.Fatal(err)
-	}
+	bm := bitmap.New(uint64(numAddresses))
 
 	start := time.Now()
 	run := 0
 	for err != ipamapi.ErrNoAvailableIPs {
-		_, err = a.getAddress(sub, bm, nil, nil, false)
+		_, err = getAddress(sub, bm, nil, nil, false)
 		run++
 	}
 	if printTime {
@@ -1030,6 +1022,9 @@ func assertNRequests(t *testing.T, subnet string, numReq int, lastExpectedIP str
 	start := time.Now()
 	for ; i < numReq; i++ {
 		nw, _, err = a.RequestAddress(pid, nil, nil)
+		if err != nil {
+			t.Fatal(err)
+		}
 	}
 	if printTime {
 		fmt.Printf("\nTaken %v, to allocate %d addresses on %s\n", time.Since(start), numReq, subnet)

libnetwork/ipam/structures.go

@@ -6,29 +6,36 @@ import (
 	"strings"
 	"sync"
 
+	"github.com/docker/docker/libnetwork/bitmap"
 	"github.com/docker/docker/libnetwork/ipamapi"
 	"github.com/docker/docker/libnetwork/types"
 )
 
-// SubnetKey is the pointer to the configured pools in each address space
-type SubnetKey struct {
+// PoolID is the pointer to the configured pools in each address space
+type PoolID struct {
 	AddressSpace string
-	Subnet       string
-	ChildSubnet  string
+	SubnetKey
 }
 
 // PoolData contains the configured pool data
 type PoolData struct {
-	ParentKey SubnetKey
-	Pool      *net.IPNet
-	Range     *AddressRange `json:",omitempty"`
-	RefCount  int
+	Pool     *net.IPNet
+	addrs    *bitmap.Bitmap
+	children map[string]struct{}
+
+	// Whether to implicitly release the pool once it no longer has any children.
+	autoRelease bool
+}
+
+// SubnetKey is the composite key to an address pool within an address space.
+type SubnetKey struct {
+	Subnet, ChildSubnet string
 }
 
 // addrSpace contains the pool configurations for the address space
 type addrSpace struct {
-	subnets map[SubnetKey]*PoolData
-	alloc   *Allocator
+	// Master subnet pools, indexed by the value's stringified PoolData.Pool field.
+	subnets map[string]*PoolData
 
 	// Predefined pool for the address space
 	predefined           []*net.IPNet
@@ -50,7 +57,7 @@ func (r *AddressRange) String() string {
 }
 
 // String returns the string form of the SubnetKey object
-func (s *SubnetKey) String() string {
+func (s *PoolID) String() string {
 	k := fmt.Sprintf("%s/%s", s.AddressSpace, s.Subnet)
 	if s.ChildSubnet != "" {
 		k = fmt.Sprintf("%s/%s", k, s.ChildSubnet)
@@ -59,7 +66,7 @@ func (s *SubnetKey) String() string {
 }
 
 // FromString populates the SubnetKey object reading it from string
-func (s *SubnetKey) FromString(str string) error {
+func (s *PoolID) FromString(str string) error {
 	if str == "" || !strings.Contains(str, "/") {
 		return types.BadRequestErrorf("invalid string form for subnetkey: %s", str)
 	}
@@ -79,98 +86,94 @@ func (s *SubnetKey) FromString(str string) error {
 
 // String returns the string form of the PoolData object
 func (p *PoolData) String() string {
-	return fmt.Sprintf("ParentKey: %s, Pool: %s, Range: %s, RefCount: %d",
-		p.ParentKey.String(), p.Pool.String(), p.Range, p.RefCount)
+	return fmt.Sprintf("Pool: %s, Children: %d",
+		p.Pool.String(), len(p.children))
 }
 
 // allocateSubnet adds the subnet k to the address space.
-func (aSpace *addrSpace) allocateSubnet(k SubnetKey, nw *net.IPNet, ipr *AddressRange, pdf bool) error {
+func (aSpace *addrSpace) allocateSubnet(nw, sub *net.IPNet) (SubnetKey, error) {
 	aSpace.Lock()
 	defer aSpace.Unlock()
 
 	// Check if already allocated
-	if _, ok := aSpace.subnets[k]; ok {
-		if pdf {
-			return types.InternalMaskableErrorf("predefined pool %s is already reserved", nw)
+	if pool, ok := aSpace.subnets[nw.String()]; ok {
+		var childExists bool
+		if sub != nil {
+			_, childExists = pool.children[sub.String()]
+		}
+		if sub == nil || childExists {
+			// This means the same pool is already allocated. allocateSubnet is called when there
+			// is request for a pool/subpool. It should ensure there is no overlap with existing pools
+			return SubnetKey{}, ipamapi.ErrPoolOverlap
 		}
-		// This means the same pool is already allocated. allocateSubnet is called when there
-		// is request for a pool/subpool. It should ensure there is no overlap with existing pools
-		return ipamapi.ErrPoolOverlap
 	}
 
+	return aSpace.allocateSubnetL(nw, sub)
+}
+
+func (aSpace *addrSpace) allocateSubnetL(nw, sub *net.IPNet) (SubnetKey, error) {
 	// If master pool, check for overlap
-	if ipr == nil {
-		if aSpace.contains(k.AddressSpace, nw) {
-			return ipamapi.ErrPoolOverlap
+	if sub == nil {
+		if aSpace.contains(nw) {
+			return SubnetKey{}, ipamapi.ErrPoolOverlap
 		}
+		k := SubnetKey{Subnet: nw.String()}
 		// This is a new master pool, add it along with corresponding bitmask
-		aSpace.subnets[k] = &PoolData{Pool: nw, RefCount: 1}
-		return aSpace.alloc.insertBitMask(k, nw)
+		aSpace.subnets[k.Subnet] = newPoolData(nw)
+		return k, nil
 	}
 
 	// This is a new non-master pool (subPool)
-	p := &PoolData{
-		ParentKey: SubnetKey{AddressSpace: k.AddressSpace, Subnet: k.Subnet},
-		Pool:      nw,
-		Range:     ipr,
-		RefCount:  1,
+
+	_, err := getAddressRange(sub, nw)
+	if err != nil {
+		return SubnetKey{}, err
 	}
-	aSpace.subnets[k] = p
+
+	k := SubnetKey{Subnet: nw.String(), ChildSubnet: sub.String()}
 
 	// Look for parent pool
-	pp, ok := aSpace.subnets[p.ParentKey]
-	if ok {
-		aSpace.incRefCount(pp, 1)
-		return nil
+	pp, ok := aSpace.subnets[k.Subnet]
+	if !ok {
+		// Parent pool does not exist, add it along with corresponding bitmask
+		pp = newPoolData(nw)
+		pp.autoRelease = true
+		aSpace.subnets[k.Subnet] = pp
 	}
-
-	// Parent pool does not exist, add it along with corresponding bitmask
-	aSpace.subnets[p.ParentKey] = &PoolData{Pool: nw, RefCount: 1}
-	return aSpace.alloc.insertBitMask(p.ParentKey, nw)
+	pp.children[k.ChildSubnet] = struct{}{}
+	return k, nil
 }
 
 func (aSpace *addrSpace) releaseSubnet(k SubnetKey) error {
 	aSpace.Lock()
 	defer aSpace.Unlock()
 
-	p, ok := aSpace.subnets[k]
+	p, ok := aSpace.subnets[k.Subnet]
 	if !ok {
 		return ipamapi.ErrBadPool
 	}
 
-	aSpace.incRefCount(p, -1)
-
-	c := p
-	for ok {
-		if c.RefCount == 0 {
-			delete(aSpace.subnets, k)
-			if c.Range == nil {
-				return nil
-			}
+	if k.ChildSubnet != "" {
+		if _, ok := p.children[k.ChildSubnet]; !ok {
+			return ipamapi.ErrBadPool
 		}
-		k = c.ParentKey
-		c, ok = aSpace.subnets[k]
+		delete(p.children, k.ChildSubnet)
+	} else {
+		p.autoRelease = true
 	}
 
-	return nil
-}
-
-func (aSpace *addrSpace) incRefCount(p *PoolData, delta int) {
-	c := p
-	ok := true
-	for ok {
-		c.RefCount += delta
-		c, ok = aSpace.subnets[c.ParentKey]
+	if len(p.children) == 0 && p.autoRelease {
+		delete(aSpace.subnets, k.Subnet)
 	}
+
+	return nil
 }
 
-// Checks whether the passed subnet is a superset or subset of any of the subset in this config db
-func (aSpace *addrSpace) contains(space string, nw *net.IPNet) bool {
-	for k, v := range aSpace.subnets {
-		if space == k.AddressSpace && k.ChildSubnet == "" {
-			if nw.Contains(v.Pool.IP) || v.Pool.Contains(nw.IP) {
-				return true
-			}
+// contains checks whether nw is a superset or subset of any of the existing subnets in this address space.
+func (aSpace *addrSpace) contains(nw *net.IPNet) bool {
+	for _, v := range aSpace.subnets {
+		if nw.Contains(v.Pool.IP) || v.Pool.Contains(nw.IP) {
+			return true
 		}
 	}
 	return false

libnetwork/ipam/utils.go

@@ -4,7 +4,6 @@ import (
 	"fmt"
 	"net"
 
-	"github.com/docker/docker/libnetwork/ipamapi"
 	"github.com/docker/docker/libnetwork/types"
 )
 
@@ -15,11 +14,7 @@ const (
 	v6 = 6
 )
 
-func getAddressRange(pool string, masterNw *net.IPNet) (*AddressRange, error) {
-	ip, nw, err := net.ParseCIDR(pool)
-	if err != nil {
-		return nil, ipamapi.ErrInvalidSubPool
-	}
+func getAddressRange(nw, masterNw *net.IPNet) (*AddressRange, error) {
 	lIP, e := types.GetHostPartIP(nw.IP, masterNw.Mask)
 	if e != nil {
 		return nil, fmt.Errorf("failed to compute range's lowest ip address: %v", e)
@@ -32,7 +27,6 @@ func getAddressRange(pool string, masterNw *net.IPNet) (*AddressRange, error) {
 	if e != nil {
 		return nil, fmt.Errorf("failed to compute range's highest ip address: %v", e)
 	}
-	nw.IP = ip
 	return &AddressRange{nw, ipToUint64(types.GetMinimalIP(lIP)), ipToUint64(types.GetMinimalIP(hIP))}, nil
 }