Finished testing IPv6 allocation pool feature

Fixed the bug of improperly calculated remaining address storage capacity Fixed the bug of not recognizing Pool6 CIDR falling outside of Net6 is they had some overlaps
nokia · Feb 7, 2020 · 5d37cd5 · 5d37cd5
1 parent 97e8a7e
commit 5d37cd5
Show file tree

Hide file tree

Showing 3 changed files with 68 additions and 26 deletions.
diff --git a/pkg/admit/validators.go b/pkg/admit/validators.go
@@ -12,6 +12,7 @@ import (
   "github.com/nokia/danm/pkg/danmep"
   "github.com/nokia/danm/pkg/ipam"
   "k8s.io/kubernetes/pkg/kubelet/cm/cpuset"
+  "log"
 )
 
 const (
@@ -64,28 +65,28 @@ func validateAllocationPools(oldManifest, newManifest *danmtypes.DanmNet, opType
      (newManifest.Spec.Options.Alloc != "" || newManifest.Spec.Options.Alloc6 != "") {
     return errors.New("Allocation bitmasks shall not be manually defined upon creation!")
   }
-  v4PoolMask, err := validateAllocV4(newManifest)
+  err := validateAllocV4(newManifest)
   if err != nil {
     return err
   }
-  err = validateAllocV6(newManifest, v4PoolMask)
+  err = validateAllocV6(newManifest)
   if err != nil {
     return err
   }
   return nil
 }
 
-func validateAllocV4(newManifest *danmtypes.DanmNet) (int, error) {
+func validateAllocV4(newManifest *danmtypes.DanmNet) error {
   cidrV4 := newManifest.Spec.Options.Cidr
   if cidrV4 == "" {
     if newManifest.Spec.Options.Pool.Start != "" || newManifest.Spec.Options.Pool.End != "" {
-      return datastructs.MinV4MaskLength, errors.New("V4 Allocation pool cannot be defined without CIDR!")
+      return errors.New("V4 Allocation pool cannot be defined without CIDR!")
     }
-    return datastructs.MinV4MaskLength, nil
+    return nil
   }
   _, ipnet, _ := net.ParseCIDR(cidrV4)
   if ipnet.IP.To4() == nil {
-    return datastructs.MinV4MaskLength, errors.New("Options.CIDR is not a valid V4 subnet!")
+    return errors.New("Options.CIDR is not a valid V4 subnet!")
   }
   if newManifest.Spec.Options.Pool.Start == "" {
     newManifest.Spec.Options.Pool.Start = cidr.Inc(ipnet.IP).String()
@@ -94,22 +95,22 @@ func validateAllocV4(newManifest *danmtypes.DanmNet) (int, error) {
     newManifest.Spec.Options.Pool.End = cidr.Dec(GetBroadcastAddress(ipnet)).String()
   }
   if !ipnet.Contains(net.ParseIP(newManifest.Spec.Options.Pool.Start)) || !ipnet.Contains(net.ParseIP(newManifest.Spec.Options.Pool.End)) {
-    return datastructs.MinV4MaskLength, errors.New("Allocation pool is outside of defined CIDR!")
+    return errors.New("Allocation pool is outside of defined CIDR!")
   }
   if ipam.Ip2int(net.ParseIP(newManifest.Spec.Options.Pool.End)) <= ipam.Ip2int(net.ParseIP(newManifest.Spec.Options.Pool.Start)) {
-    return datastructs.MinV4MaskLength, errors.New("Allocation pool start:" + newManifest.Spec.Options.Pool.Start + " is bigger than or equal to allocation pool end:" + newManifest.Spec.Options.Pool.End)
+    return errors.New("Allocation pool start:" + newManifest.Spec.Options.Pool.Start + " is bigger than or equal to allocation pool end:" + newManifest.Spec.Options.Pool.End)
   }
   netMaskSize, _ := ipnet.Mask.Size()
   if netMaskSize < datastructs.MaxV4MaskLength {
-    return datastructs.MinV4MaskLength, errors.New("Netmask of the IPv4 CIDR is bigger than the maximum allowed /"+ strconv.Itoa(datastructs.MaxV4MaskLength))
+    return errors.New("Netmask of the IPv4 CIDR is bigger than the maximum allowed /"+ strconv.Itoa(datastructs.MaxV4MaskLength))
   }
   if newManifest.Spec.Options.Alloc == "" {
     newManifest.Spec.Options.Alloc = ipam.CreateAllocationArray(ipnet, newManifest.Spec.Options.Routes)
   }
-  return netMaskSize, nil
+  return nil
 }
 
-func validateAllocV6(newManifest *danmtypes.DanmNet, v4PoolMask int) error {
+func validateAllocV6(newManifest *danmtypes.DanmNet) error {
   net6 := newManifest.Spec.Options.Net6
   if net6 == "" {
     if newManifest.Spec.Options.Pool6.Start != "" ||
@@ -120,26 +121,30 @@ func validateAllocV6(newManifest *danmtypes.DanmNet, v4PoolMask int) error {
     return nil
   }
   _, netCidr, _ := net.ParseCIDR(net6)
+  if netCidr.IP.To4() != nil {
+    return errors.New("spec.Options.Net6 is not a valid V6 subnet!")
+  }
   // The limit of the current storage algorithm is 16M addresses per network.
   // This means that the summarized size of the IPv4, and IPv6 allocation pools shall not go over this threshold.
   // Therefore we need to calculate the maximum usable prefix for our V6 pool, discounting the space we have already reserved for the V4 pool.
-  maxV6AllocPrefix := datastructs.MaxV6PrefixLength + (datastructs.MinV4MaskLength - v4PoolMask)
+  maxV6AllocPrefix := ipam.GetMaxUsableV6Prefix(newManifest)
   if newManifest.Spec.Options.Pool6.Cidr == "" {
     baseCidrStart := netCidr.IP
     maskedV6AllocCidrBase := net.CIDRMask(maxV6AllocPrefix, 128)
     maskedV6AllocCidr := net.IPNet{IP:baseCidrStart, Mask:maskedV6AllocCidrBase}
     newManifest.Spec.Options.Pool6.Cidr = maskedV6AllocCidr.String()
   }
+  log.Println("Alloc6 CIDR:" + newManifest.Spec.Options.Pool6.Cidr)
   _, allocCidr, err := net.ParseCIDR(newManifest.Spec.Options.Pool6.Cidr)
   if err != nil {
     return errors.New("spec.Options.Pool6.CIDR is invalid!")
   }
-  if allocCidr.IP.To4() != nil || netCidr.IP.To4() != nil {
-    return errors.New("IPv6 CIDRs are not valid V6 subnets!")
+  if allocCidr.IP.To4() != nil {
+    return errors.New("spec.Options.Allocation_Pool_V6.Cidr is not a valid V6 subnet!")
   }
   netMaskSize, _ := allocCidr.Mask.Size()
   // We don't have enough storage space left for storing IPv6 allocations
-  if netMaskSize < maxV6AllocPrefix {
+  if netMaskSize < maxV6AllocPrefix || netMaskSize == datastructs.MinV6PrefixLength {
     return errors.New("The defined IPv6 allocation pool exceeds the maximum - 16M-size(IPv4 allocation pool) - storage capacity!")
   }
   if (newManifest.Spec.Options.Pool6.Start != "" && !allocCidr.Contains(net.ParseIP(newManifest.Spec.Options.Pool6.Start))) ||
@@ -154,7 +159,7 @@ func validateAllocV6(newManifest *danmtypes.DanmNet, v4PoolMask int) error {
     newManifest.Spec.Options.Pool6.End = cidr.Dec(GetBroadcastAddress(allocCidr)).String()
   }
   if ipam.Ip62int(net.ParseIP(newManifest.Spec.Options.Pool6.End)).Cmp(ipam.Ip62int(net.ParseIP(newManifest.Spec.Options.Pool6.Start))) <=0 {
-    return errors.New("Allocation pool start:" + newManifest.Spec.Options.Pool.Start + " is bigger than or equal to allocation pool end:" + newManifest.Spec.Options.Pool.End)
+    return errors.New("Allocation pool start:" + newManifest.Spec.Options.Pool6.Start + " is bigger than or equal to allocation pool end:" + newManifest.Spec.Options.Pool6.End)
   }
   if newManifest.Spec.Options.Alloc6 == "" {
     newManifest.Spec.Options.Alloc6 = ipam.CreateAllocationArray(allocCidr, newManifest.Spec.Options.Routes6)
@@ -163,10 +168,6 @@ func validateAllocV6(newManifest *danmtypes.DanmNet, v4PoolMask int) error {
 }
 
 func GetBroadcastAddress(subnet *net.IPNet) (net.IP) {
-/*  ip := make(net.IP, len(subnet.IP.To4()))
-  //Don't ask
-  binary.BigEndian.PutUint32(ip, binary.BigEndian.Uint32(subnet.IP.To4())|^binary.BigEndian.Uint32(net.IP(subnet.Mask).To4()))
-*/
   _, lastIp := cidr.AddressRange(subnet)
   return lastIp
 }

diff --git a/pkg/ipam/ipam.go b/pkg/ipam/ipam.go
@@ -3,17 +3,20 @@ package ipam
 import (
   "errors"
   "fmt"
+  "math"
   "net"
   "strconv"
   "strings"
   "time"
   "encoding/binary"
   "math/big"
   "math/rand"
+  "github.com/apparentlymart/go-cidr/cidr"
   danmtypes "github.com/nokia/danm/crd/apis/danm/v1"
   danmclientset "github.com/nokia/danm/crd/client/clientset/versioned"
-  "github.com/nokia/danm/pkg/netcontrol"
   "github.com/nokia/danm/pkg/bitarray"
+  "github.com/nokia/danm/pkg/datastructs"
+  "github.com/nokia/danm/pkg/netcontrol"
 )
 
 const (
@@ -283,12 +286,30 @@ func reserveGatewayIps(routes map[string]string, bitArray *bitarray.BitArray, ip
   }
 }
 
-// Here be Stackoverflow magic
+
 func DoV6CidrsIntersect(masterCidr, subCidr *net.IPNet) bool {
-  for i := range masterCidr.IP {
-    if masterCidr.IP[i] & masterCidr.Mask[i] != subCidr.IP[i] & subCidr.Mask[i] & masterCidr.Mask[i] {
-      return false
+  firstAllocIp, lastAllocIp := cidr.AddressRange(subCidr)
+  //Brute force: if the Alloc6 CIDR's first, and last IP both belongs to Net6, we assume the whole CIDR also does
+  if masterCidr.Contains(firstAllocIp) && masterCidr.Contains(lastAllocIp) {
+    return true
+  }
+  return false
+}
+
+func GetMaxUsableV6Prefix(dnet *danmtypes.DanmNet) int {
+  if dnet.Spec.Options.Cidr == "" {
+    return datastructs.MaxV6PrefixLength
+  }
+  _, v4Cidr, _ := net.ParseCIDR(dnet.Spec.Options.Cidr)
+  sizeOfV4AllocPool := cidr.AddressCount(v4Cidr)
+  maxRemainingCapacity := uint64(math.Pow(2,float64(bitarray.MaxSupportedAllocLength))) - sizeOfV4AllocPool
+  maxUsableV6Prefix := datastructs.MinV6PrefixLength
+  for pref := datastructs.MaxV6PrefixLength; pref <= datastructs.MinV6PrefixLength; pref++ {
+    _, testCidr, _ := net.ParseCIDR("2a00:8a00:a000:1193:f816:3eff:fe24:e348/" + strconv.Itoa(pref))
+    if cidr.AddressCount(testCidr) < maxRemainingCapacity {
+      maxUsableV6Prefix = pref
+      break
     }
   }
-  return true
+  return maxUsableV6Prefix
 }
diff --git a/test/uts/admit_tests/netadmit_test.go b/test/uts/admit_tests/netadmit_test.go
@@ -143,6 +143,10 @@ var validateNetworkTcs = []struct {
   {"CreateV6NetworkWithoutPool6DNet", "", "net6-without-pool6", DnetType, v1beta1.Create, nil, nil, false, v6Allocs, 0},
   {"CreateV6NetworkWithoutPool6TNet", "", "net6-without-pool6", TnetType, v1beta1.Create, randomDev, nil, false, v6AllocsForTnet, 1},
   {"CreateV6NetworkWithoutPool6CNet", "", "net6-without-pool6", CnetType, v1beta1.Create, nil, nil, false, v6Allocs, 0},
+  {"V4PlusV6IsOverCapacity", "", "no-space-for-v6-alloc", DnetType, "", nil, nil, true, nil, 0},
+  {"Pool6CidrBiggerThanNet6", "", "pool6-cidr-outside-net6", DnetType, "", nil, nil, true, nil, 0},
+  {"InvalidPool6StartAddress", "", "invalid-pool6-start", DnetType, "", nil, nil, true, nil, 0},
+  {"Pool6StartAddressMatchesEnd", "", "pool6-end-equals-start", DnetType, "", nil, nil, true, nil, 0},
 }
 
 var (
@@ -346,6 +350,22 @@ var (
       ObjectMeta: meta_v1.ObjectMeta {Name: "net6-without-pool6"},
       Spec: danmtypes.DanmNetSpec{NetworkType: "ipvlan", NetworkID: "nanomsg", Options: danmtypes.DanmNetOption{Net6: "2a00:8a00:a000:1193::/64"}},
     },
+    danmtypes.DanmNet {
+      ObjectMeta: meta_v1.ObjectMeta {Name: "no-space-for-v6-alloc"},
+      Spec: danmtypes.DanmNetSpec{NetworkType: "ipvlan", NetworkID: "nanomsg", Options: danmtypes.DanmNetOption{Cidr: "37.0.0.0/8", Net6: "2001:db8:85a3::8a2e:370:7334/104"}},
+    },
+    danmtypes.DanmNet {
+      ObjectMeta: meta_v1.ObjectMeta {Name: "pool6-cidr-outside-net6"},
+      Spec: danmtypes.DanmNetSpec{NetworkType: "ipvlan", NetworkID: "nanomsg", Options: danmtypes.DanmNetOption{Net6: "2001:db8:85a3::8a2e:370:7334/110", Pool6: danmtypes.IpPoolV6{Cidr: "2001:db8:85a3::8a2e:370:7334/109"}}},
+    },
+    danmtypes.DanmNet {
+      ObjectMeta: meta_v1.ObjectMeta {Name: "invalid-pool6-start"},
+      Spec: danmtypes.DanmNetSpec{NetworkType: "ipvlan", NetworkID: "nanomsg", Options: danmtypes.DanmNetOption{Net6: "2001:db8:85a3::8a2e:370:7334/108", Pool6: danmtypes.IpPoolV6{Cidr: "2001:db8:85a3::8a2e:370:7334/109", IpPool: danmtypes.IpPool{Start: "2001:db8:85a3::8a2e:370:734g"}}}},
+    },
+    danmtypes.DanmNet {
+      ObjectMeta: meta_v1.ObjectMeta {Name: "pool6-end-equals-start"},
+      Spec: danmtypes.DanmNetSpec{NetworkType: "ipvlan", NetworkID: "nanomsg", Options: danmtypes.DanmNetOption{Net6: "2001:db8:85a3::8a2e:370:7334/108", Pool6: danmtypes.IpPoolV6{Cidr: "2001:db8:85a3::8a2e:370:7334/109", IpPool: danmtypes.IpPool{Start: "2001:db8:85a3::8a2e:370:7340", End: "2001:db8:85a3::8a2e:370:7340"}}}},
+    },
   }
 )