From b2ff78548a9b8b81c481df8c904c1ab3db8b3ad3 Mon Sep 17 00:00:00 2001 From: Alessandro Boch Date: Wed, 24 Jun 2015 19:11:44 -0700 Subject: [PATCH] Fix preferred ip allocation in ipam - also provided a new utility to compute the host part ip address which is resilient to input passed in different representations. Signed-off-by: Alessandro Boch --- libnetwork/ipam/allocator.go | 67 +++++++++++++++++++------------ libnetwork/ipam/allocator_test.go | 43 +++++++++++++++++++- libnetwork/types/types.go | 53 ++++++++++++++++++++++++ libnetwork/types/types_test.go | 59 +++++++++++++++++++++++++++ 4 files changed, 195 insertions(+), 27 deletions(-) diff --git a/libnetwork/ipam/allocator.go b/libnetwork/ipam/allocator.go index bf52dff731..eea04c8b47 100644 --- a/libnetwork/ipam/allocator.go +++ b/libnetwork/ipam/allocator.go @@ -21,7 +21,7 @@ const ( minNetSizeV6Eff = 96 // The size of the host subnet used internally, it's the most granular sequence addresses defaultInternalHostSize = 16 - // datastore keyes for ipam obkects + // datastore keyes for ipam objects dsConfigKey = "ipam-config" // ipam-config// dsDataKey = "ipam-data" // ipam-data//// ) @@ -80,8 +80,7 @@ func NewAllocator(ds datastore.DataStore) (*Allocator, error) { if err != nil { return fmt.Errorf("failed to load address bitmask for configured subnet %s because of %s", v.Subnet.String(), err.Error()) } - a.insertAddressMasks(k, subnetList) - return nil + return a.insertAddressMasks(k, subnetList) }) } a.Unlock() @@ -417,28 +416,48 @@ func (a *Allocator) request(addrSpace AddressSpace, req *AddressRequest, version // Release allows releasing the address from the specified address space func (a *Allocator) Release(addrSpace AddressSpace, address net.IP) { + var ( + space *bitseq.Handle + sub *net.IPNet + ) + if address == nil { + log.Debugf("Requested to remove nil address from address space %s", addrSpace) return } + ver := getAddressVersion(address) if ver == v4 { address = address.To4() } + + // Find the subnet containing the address for _, subKey := range a.getSubnetList(addrSpace, ver) { - a.Lock() - space := a.addresses[subKey] - a.Unlock() - sub := subKey.canonicalChildSubnet() + sub = subKey.canonicalChildSubnet() if sub.Contains(address) { - // Retrieve correspondent ordinal in the subnet - ordinal := ipToUint32(getHostPortionIP(address, sub)) - // Release it - if err := space.Unset(ordinal); err != nil { - log.Warnf("Failed to release address %s because of internal error: %s", address.String(), err.Error()) - } - return + a.Lock() + space = a.addresses[subKey] + a.Unlock() + break } } + if space == nil { + log.Debugf("Could not find subnet on address space %s containing %s on release", addrSpace, address.String()) + return + } + + // Retrieve correspondent ordinal in the subnet + hostPart, err := types.GetHostPartIP(address, sub.Mask) + if err != nil { + log.Warnf("Failed to release address %s on address space %s because of internal error: %v", address.String(), addrSpace, err) + return + } + ordinal := ipToUint32(hostPart) + + // Release it + if err := space.Unset(ordinal); err != nil { + log.Warnf("Failed to release address %s on address space %s because of internal error: %v", address.String(), addrSpace, err) + } } func (a *Allocator) reserveAddress(addrSpace AddressSpace, subnet *net.IPNet, prefAddress net.IP, ver ipVersion) (net.IP, *net.IPNet, error) { @@ -468,7 +487,7 @@ func (a *Allocator) reserveAddress(addrSpace AddressSpace, subnet *net.IPNet, pr bitmask, ok := a.addresses[key] a.Unlock() if !ok { - fmt.Printf("\nDid not find a bitmask for subnet key: %s", key.String()) + log.Warnf("Did not find a bitmask for subnet key: %s", key.String()) continue } address, err := a.getAddress(key.canonicalChildSubnet(), bitmask, prefAddress, ver) @@ -511,7 +530,12 @@ func (a *Allocator) getAddress(subnet *net.IPNet, bitmask *bitseq.Handle, prefAd if prefAddress == nil { ordinal, err = bitmask.SetAny() } else { - err = bitmask.Set(ipToUint32(getHostPortionIP(prefAddress, subnet))) + hostPart, e := types.GetHostPartIP(prefAddress, subnet.Mask) + if e != nil { + return nil, fmt.Errorf("failed to allocate preferred address %s: %v", prefAddress.String(), e) + } + ordinal = ipToUint32(types.GetMinimalIP(hostPart)) + err = bitmask.Set(ordinal) } if err != nil { return nil, ErrNoAvailableIPs @@ -549,7 +573,7 @@ func generateAddress(ordinal uint32, network *net.IPNet) net.IP { var address [16]byte // Get network portion of IP - if network.IP.To4() != nil { + if getAddressVersion(network.IP) == v4 { copy(address[:], network.IP.To4()) } else { copy(address[:], network.IP) @@ -592,12 +616,3 @@ func ipToUint32(ip []byte) uint32 { } return value } - -// Given an address and subnet, returns the host portion address -func getHostPortionIP(address net.IP, subnet *net.IPNet) net.IP { - hostPortion := make([]byte, len(address)) - for i := 0; i < len(subnet.Mask); i++ { - hostPortion[i] = address[i] &^ subnet.Mask[i] - } - return hostPortion -} diff --git a/libnetwork/ipam/allocator_test.go b/libnetwork/ipam/allocator_test.go index 25bd0b8218..c7b6fddb4a 100644 --- a/libnetwork/ipam/allocator_test.go +++ b/libnetwork/ipam/allocator_test.go @@ -3,15 +3,31 @@ package ipam import ( "fmt" "net" + "os" "testing" "time" "github.com/docker/libnetwork/bitseq" + "github.com/docker/libnetwork/config" + "github.com/docker/libnetwork/datastore" _ "github.com/docker/libnetwork/netutils" ) +var ds datastore.DataStore + +// enable w/ upper case +func testMain(m *testing.M) { + var err error + ds, err = datastore.NewDataStore(&config.DatastoreCfg{Embedded: false, Client: config.DatastoreClientCfg{Provider: "consul", Address: "127.0.0.1:8500"}}) + if err != nil { + fmt.Println(err) + } + + os.Exit(m.Run()) +} + func getAllocator(t *testing.T, subnet *net.IPNet) *Allocator { - a, err := NewAllocator(nil) + a, err := NewAllocator(ds) if err != nil { t.Fatal(err) } @@ -273,7 +289,32 @@ func TestGetInternalSubnets(t *testing.T) { for _, d := range input { assertInternalSubnet(t, d.internalHostSize, d.parentSubnet, d.firstIntSubnet, d.lastIntSubnet) } +} +func TestGetSameAddress(t *testing.T) { + a, err := NewAllocator(nil) + if err != nil { + t.Fatal(err) + } + + addSpace := AddressSpace("giallo") + _, subnet, _ := net.ParseCIDR("192.168.100.0/24") + if err := a.AddSubnet(addSpace, &SubnetInfo{Subnet: subnet}); err != nil { + t.Fatal(err) + } + + ip := net.ParseIP("192.168.100.250") + req := &AddressRequest{Subnet: *subnet, Address: ip} + + _, err = a.Request(addSpace, req) + if err != nil { + t.Fatal(err) + } + + _, err = a.Request(addSpace, req) + if err == nil { + t.Fatal(err) + } } func TestGetAddress(t *testing.T) { diff --git a/libnetwork/types/types.go b/libnetwork/types/types.go index 3f394baa19..176f40d0f1 100644 --- a/libnetwork/types/types.go +++ b/libnetwork/types/types.go @@ -197,6 +197,59 @@ func CompareIPNet(a, b *net.IPNet) bool { return a.IP.Equal(b.IP) && bytes.Equal(a.Mask, b.Mask) } +// GetMinimalIP returns the address in its shortest form +func GetMinimalIP(ip net.IP) net.IP { + if ip != nil && ip.To4() != nil { + return ip.To4() + } + return ip +} + +// GetMinimalIPNet returns a copy of the passed IP Network with congruent ip and mask notation +func GetMinimalIPNet(nw *net.IPNet) *net.IPNet { + if nw == nil { + return nil + } + if len(nw.IP) == 16 && nw.IP.To4() != nil { + m := nw.Mask + if len(m) == 16 { + m = m[12:16] + } + return &net.IPNet{IP: nw.IP.To4(), Mask: m} + } + return nw +} + +var v4inV6MaskPrefix = []byte{0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff} + +// GetHostPartIP returns the host portion of the ip address identified by the mask. +// IP address representation is not modified. If address and mask are not compatible +// an error is returned. +func GetHostPartIP(ip net.IP, mask net.IPMask) (net.IP, error) { + // Find the effective starting of address and mask + is := 0 + ms := 0 + if len(ip) == net.IPv6len && ip.To4() != nil { + is = 12 + } + if len(ip[is:]) == net.IPv4len && len(mask) == net.IPv6len && bytes.Equal(mask[:12], v4inV6MaskPrefix) { + ms = 12 + } + + // Check if address and mask are semantically compatible + if len(ip[is:]) != len(mask[ms:]) { + return nil, fmt.Errorf("cannot compute host portion ip address as ip and mask are not compatible: (%#v, %#v)", ip, mask) + } + + // Compute host portion + out := GetIPCopy(ip) + for i := 0; i < len(mask[ms:]); i++ { + out[is+i] &= ^mask[ms+i] + } + + return out, nil +} + const ( // NEXTHOP indicates a StaticRoute with an IP next hop. NEXTHOP = iota diff --git a/libnetwork/types/types_test.go b/libnetwork/types/types_test.go index efa26d7e2e..ed90915b01 100644 --- a/libnetwork/types/types_test.go +++ b/libnetwork/types/types_test.go @@ -2,6 +2,7 @@ package types import ( "flag" + "net" "testing" ) @@ -109,3 +110,61 @@ func TestErrorConstructors(t *testing.T) { t.Fatal(err) } } + +func TestUtilGetHostPortionIP(t *testing.T) { + input := []struct { + ip net.IP + mask net.IPMask + host net.IP + err error + }{ + { // ip in v4Inv6 representation, mask in v4 representation + ip: net.IPv4(172, 28, 30, 1), + mask: []byte{0xff, 0xff, 0xff, 0}, + host: net.IPv4(0, 0, 0, 1), + }, + { // ip and mask in v4Inv6 representation + ip: net.IPv4(172, 28, 30, 2), + mask: []byte{0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0}, + host: net.IPv4(0, 0, 0, 2), + }, + { // ip in v4 representation, mask in v4Inv6 representation + ip: net.IPv4(172, 28, 30, 3)[12:], + mask: []byte{0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0}, + host: net.IPv4(0, 0, 0, 3)[12:], + }, + { // ip and mask in v4 representation + ip: net.IPv4(172, 28, 30, 4)[12:], + mask: []byte{0xff, 0xff, 0xff, 0}, + host: net.IPv4(0, 0, 0, 4)[12:], + }, + { // ip and mask as v6 + ip: net.ParseIP("2005:2004:2002:2001:FFFF:ABCD:EEAB:00CD"), + mask: []byte{0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0, 0, 0}, + host: net.ParseIP("0::AB:00CD"), + }, + } + + for _, i := range input { + h, err := GetHostPartIP(i.ip, i.mask) + if err != nil { + t.Fatal(err) + } + if !i.host.Equal(h) { + t.Fatalf("Failed to return expected host ip. Expected: %s. Got: %s", i.host, h) + } + } + + // ip as v6 and mask as v4 are not compatible + if _, err := GetHostPartIP(net.ParseIP("2005:2004:2002:2001:FFFF:ABCD:EEAB:00CD"), []byte{0xff, 0xff, 0xff, 0}); err == nil { + t.Fatalf("Unexpected success") + } + // ip as v4 and non conventional mask + if _, err := GetHostPartIP(net.ParseIP("173.32.4.5"), []byte{0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0xff, 0xff, 0xff, 0}); err == nil { + t.Fatalf("Unexpected success") + } + // ip as v4 and non conventional mask + if _, err := GetHostPartIP(net.ParseIP("173.32.4.5"), []byte{0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0xff, 0xff, 0xff, 0xff, 0xff, 0}); err == nil { + t.Fatalf("Unexpected success") + } +}