diff --git a/pkg/registry/service/ip_allocator.go b/pkg/registry/service/ip_allocator.go index f48511803f4..947c0a64534 100644 --- a/pkg/registry/service/ip_allocator.go +++ b/pkg/registry/service/ip_allocator.go @@ -31,6 +31,9 @@ type ipAllocator struct { used []byte // a bitmap of allocated IPs } +// The smallest number of IPs we accept. +const minIPSpace = 8 + // newIPAllocator creates and intializes a new ipAllocator object. func newIPAllocator(subnet *net.IPNet) *ipAllocator { if subnet == nil || subnet.IP == nil || subnet.Mask == nil { @@ -38,7 +41,13 @@ func newIPAllocator(subnet *net.IPNet) *ipAllocator { } ones, bits := subnet.Mask.Size() + // TODO: some settings with IPv6 address could cause this to take + // an excessive amount of memory. numIps := 1 << uint(bits-ones) + if numIps < minIPSpace { + glog.Errorf("IPAllocator requires at least %d IPs", minIPSpace) + return nil + } ipa := &ipAllocator{ subnet: subnet, used: make([]byte, numIps/8), @@ -82,7 +91,7 @@ func (ipa *ipAllocator) AllocateNext() (net.IP, error) { } ipa.used[i] |= 1 << nextBit offset := (i * 8) + int(nextBit) - ip := ipAdd(copyIP(ipa.subnet.IP), offset) + ip := ipAdd(ipa.subnet.IP, offset) return ip, nil } } @@ -102,12 +111,16 @@ func ffs(val byte) (uint, error) { // Add an offset to an IP address - used for joining network addr and host addr parts. func ipAdd(ip net.IP, offset int) net.IP { - for i := 0; offset > 0; i++ { + out := copyIP(simplifyIP(ip)) + // Loop from least-significant to most. + for i := len(out) - 1; i >= 0 && offset > 0; i-- { add := offset % 256 - ip[len(ip)-1-i] += byte(add) + result := int(out[i]) + add + out[i] = byte(result % 256) offset >>= 8 + offset += result / 256 // carry } - return ip + return out } // Subtract two IPs, returning the difference as an offset - used or splitting an IP into @@ -115,18 +128,37 @@ func ipAdd(ip net.IP, offset int) net.IP { func ipSub(lhs, rhs net.IP) int { // If they are not the same length, normalize them. Make copies because net.IP is // a slice underneath. Sneaky sneaky. + lhs = simplifyIP(lhs) + rhs = simplifyIP(rhs) if len(lhs) != len(rhs) { lhs = copyIP(lhs).To16() rhs = copyIP(rhs).To16() } offset := 0 + borrow := 0 + // Loop from most-significant to least. for i := range lhs { - offset *= 256 - offset += int(lhs[i] - rhs[i]) + offset <<= 8 + result := (int(lhs[i]) - borrow) - int(rhs[i]) + if result < 0 { + borrow = 1 + } else { + borrow = 0 + } + offset += result } return offset } +// Get the optimal slice for an IP. IPv4 addresses will come back in a 4 byte slice. IPv6 +// addresses will come back in a 16 byte slice. Non-IP arguments will produce nil. +func simplifyIP(in net.IP) net.IP { + if ip4 := in.To4(); ip4 != nil { + return ip4 + } + return in.To16() +} + // Make a copy of a net.IP. It appears to be a value type, but it is actually defined as a // slice, so value assignment is shallow. Why does a poor dumb user like me need to know // this sort of implementation detail? diff --git a/pkg/registry/service/ip_allocator_test.go b/pkg/registry/service/ip_allocator_test.go index cd02020d5cc..db5147806f1 100644 --- a/pkg/registry/service/ip_allocator_test.go +++ b/pkg/registry/service/ip_allocator_test.go @@ -206,10 +206,14 @@ func TestIPAdd(t *testing.T) { {"1.2.3.0", 0, "1.2.3.0"}, {"1.2.3.0", 1, "1.2.3.1"}, {"1.2.3.0", 255, "1.2.3.255"}, + {"1.2.3.1", 255, "1.2.4.0"}, + {"1.2.3.2", 255, "1.2.4.1"}, {"1.2.3.0", 256, "1.2.4.0"}, {"1.2.3.0", 257, "1.2.4.1"}, {"1.2.3.0", 65536, "1.3.3.0"}, {"1.2.3.4", 1, "1.2.3.5"}, + {"255.255.255.255", 1, "0.0.0.0"}, + {"255.255.255.255", 2, "0.0.0.1"}, } for _, tc := range testCases { r := ipAdd(net.ParseIP(tc.ip), tc.offset) @@ -229,9 +233,12 @@ func TestIPSub(t *testing.T) { {"1.2.3.1", "1.2.3.0", 1}, {"1.2.3.255", "1.2.3.0", 255}, {"1.2.4.0", "1.2.3.0", 256}, + {"1.2.4.0", "1.2.3.255", 1}, {"1.2.4.1", "1.2.3.0", 257}, {"1.3.3.0", "1.2.3.0", 65536}, {"1.2.3.5", "1.2.3.4", 1}, + {"0.0.0.0", "0.0.0.1", -1}, + {"0.0.1.0", "0.0.0.1", 255}, } for _, tc := range testCases { r := ipSub(net.ParseIP(tc.lhs), net.ParseIP(tc.rhs)) @@ -249,3 +256,23 @@ func TestCopyIP(t *testing.T) { t.Errorf("copyIP did not copy") } } + +func TestSimplifyIP(t *testing.T) { + ip4 := net.ParseIP("1.2.3.4") + if len(ip4) != 16 { + t.Errorf("expected 16 bytes") + } + if len(simplifyIP(ip4)) != 4 { + t.Errorf("expected 4 bytes") + } + ip6 := net.ParseIP("::1.2.3.4") + if len(ip6) != 16 { + t.Errorf("expected 16 bytes") + } + if len(simplifyIP(ip6)) != 16 { + t.Errorf("expected 16 bytes") + } + if simplifyIP([]byte{0, 0}) != nil { + t.Errorf("expected nil") + } +}