Add MAC Address Management

This adds MAC address management into the existing IPAM functionality
and CRD.  The ViNO CR is augmented to supply a MAC Prefix.  This prefix
is used as the first in a sequence of consecutive MAC addresses that
the IPAM code will generate when needed.

Note, there is no upper bounds checking on MAC addresses
(no End defined for the MAC Range), operating under the assumption
that the MAC addresses are for intents and purposes inexhaustable:
all RFC 1918 private MAC ranges are huge.
    x2-xx-xx-xx-xx-xx
    x6-xx-xx-xx-xx-xx
    xA-xx-xx-xx-xx-xx
    xE-xx-xx-xx-xx-xx

Change-Id: I19eb709019337acfe41acd7091ec43dc08e05648

config/crd/bases/airship.airshipit.org_ippools.yaml

@@ -39,17 +39,27 @@ spec:
           properties:
             allocatedIPs:
               items:
-                description: AllocatedIP Allocates an IP to an entity
+                description: AllocatedIP Allocates an IP and MAC address to an entity
                 properties:
                   allocatedTo:
                     type: string
                   ip:
                     type: string
+                  mac:
+                    type: string
                 required:
                 - allocatedTo
                 - ip
+                - mac
                 type: object
               type: array
+            macPrefix:
+              description: MACPrefix defines the MAC prefix to use for VM mac addresses
+              type: string
+            nextMAC:
+              description: NextMAC indicates the next MAC address (in sequence) that
+                will be provisioned to a VM in this Subnet
+              type: string
             ranges:
               items:
                 description: Range has (inclusive) bounds within a subnet from which
@@ -68,6 +78,8 @@ spec:
               type: string
           required:
           - allocatedIPs
+          - macPrefix
+          - nextMAC
           - ranges
           - subnet
           type: object

config/crd/bases/airship.airshipit.org_vinoes.yaml

@@ -90,6 +90,13 @@ spec:
                     items:
                       type: string
                     type: array
+                  macPrefix:
+                    description: MACPrefix defines the zero-padded MAC prefix to use
+                      for VM mac addresses, and is the first address that will be
+                      allocated sequentially to VMs in this network. If omitted, a
+                      default private MAC prefix will be used. The prefix should be
+                      specified in full MAC notation, e.g. 06:42:42:00:00:00
+                    type: string
                   name:
                     description: Network Parameter defined
                     type: string

config/samples/ippool.yaml

@@ -7,6 +7,8 @@ metadata:
   name: ippool-sample
 spec:
   subnet: 10.0.0.0/16
+  macPrefix: "02:00:00:00:00:00"
+  nextMAC: "02:00:00:00:00:03"
   ranges:
     - start: 10.0.0.1
       stop:  10.0.0.9
@@ -15,7 +17,10 @@ spec:
   allocatedIPs:
     - allocatedTo: default-vino-test-cr-leviathan-worker-0
       ip: 10.0.0.1
+      mac: "02:00:00:00:00:00"
     - allocatedTo: default-vino-test-cr-leviathan-worker-1
       ip: 10.0.0.2
+      mac: "02:00:00:00:00:01"
     - allocatedTo: default-vino-test-cr-leviathan-worker-2
       ip: 10.0.1.1
+      mac: "02:00:00:00:00:02"

config/samples/network-template-secret.yaml

@@ -19,7 +19,7 @@ stringData:
         name: {{ .Name }}
         type: {{ .Type }}
         mtu: {{ .MTU }}
-        #  ethernet_mac_address: ??
+        ethernet_mac_address: {{ index $.Generated.MACAddresses .Name }}
         {{- if .Options -}}
         {{ range $key, $val := .Options }}
         {{ $key }}: {{ $val }}

config/samples/vino_cr.yaml

@@ -31,6 +31,7 @@ spec:
           gateway: 169.0.0.1
       allocationStart: 169.0.0.10
       allocationStop: 169.0.0.254
+      macPrefix: "0A:00:00:00:00:00"
 
   vmBridge: lo
   nodes:

docs/api/vino.md

@@ -15,7 +15,7 @@ Resource Types:
 (<em>Appears on:</em>
 <a href="#airship.airshipit.org/v1.IPPoolSpec">IPPoolSpec</a>)
 </p>
-<p>AllocatedIP Allocates an IP to an entity</p>
+<p>AllocatedIP Allocates an IP and MAC address to an entity</p>
 <div class="md-typeset__scrollwrap">
 <div class="md-typeset__table">
 <table>
@@ -38,6 +38,16 @@ string
 </tr>
 <tr>
 <td>
+<code>mac</code><br>
+<em>
+string
+</em>
+</td>
+<td>
+</td>
+</tr>
+<tr>
+<td>
 <code>allocatedTo</code><br>
 <em>
 string
@@ -375,6 +385,29 @@ string
 <td>
 </td>
 </tr>
+<tr>
+<td>
+<code>macPrefix</code><br>
+<em>
+string
+</em>
+</td>
+<td>
+<p>MACPrefix defines the MAC prefix to use for VM mac addresses</p>
+</td>
+</tr>
+<tr>
+<td>
+<code>nextMAC</code><br>
+<em>
+string
+</em>
+</td>
+<td>
+<p>NextMAC indicates the next MAC address (in sequence) that
+will be provisioned to a VM in this Subnet</p>
+</td>
+</tr>
 </table>
 </td>
 </tr>
@@ -448,6 +481,29 @@ string
 <td>
 </td>
 </tr>
+<tr>
+<td>
+<code>macPrefix</code><br>
+<em>
+string
+</em>
+</td>
+<td>
+<p>MACPrefix defines the MAC prefix to use for VM mac addresses</p>
+</td>
+</tr>
+<tr>
+<td>
+<code>nextMAC</code><br>
+<em>
+string
+</em>
+</td>
+<td>
+<p>NextMAC indicates the next MAC address (in sequence) that
+will be provisioned to a VM in this Subnet</p>
+</td>
+</tr>
 </tbody>
 </table>
 </div>
@@ -591,6 +647,22 @@ string
 <td>
 </td>
 </tr>
+<tr>
+<td>
+<code>macPrefix</code><br>
+<em>
+string
+</em>
+</td>
+<td>
+<p>MACPrefix defines the zero-padded MAC prefix to use for
+VM mac addresses, and is the first address that will be
+allocated sequentially to VMs in this network.
+If omitted, a default private MAC prefix will be used.
+The prefix should be specified in full MAC notation, e.g.
+06:42:42:00:00:00</p>
+</td>
+</tr>
 </tbody>
 </table>
 </div>

pkg/api/v1/ippool_types.go

@@ -29,11 +29,17 @@ type IPPoolSpec struct {
 	Subnet       string        `json:"subnet"`
 	Ranges       []Range       `json:"ranges"`
 	AllocatedIPs []AllocatedIP `json:"allocatedIPs"`
+	// MACPrefix defines the MAC prefix to use for VM mac addresses
+	MACPrefix string `json:"macPrefix"`
+	// NextMAC indicates the next MAC address (in sequence) that
+	// will be provisioned to a VM in this Subnet
+	NextMAC string `json:"nextMAC"`
 }
 
-// AllocatedIP Allocates an IP to an entity
+// AllocatedIP Allocates an IP and MAC address to an entity
 type AllocatedIP struct {
 	IP          string `json:"ip"`
+	MAC         string `json:"mac"`
 	AllocatedTo string `json:"allocatedTo"`
 }
 

pkg/api/v1/vino_types.go

@@ -83,6 +83,13 @@ type Network struct {
 	AllocationStop  string     `json:"allocationStop,omitempty"`
 	DNSServers      []string   `json:"dns_servers,omitempty"`
 	Routes          []VMRoutes `json:"routes,omitempty"`
+	// MACPrefix defines the zero-padded MAC prefix to use for
+	// VM mac addresses, and is the first address that will be
+	// allocated sequentially to VMs in this network.
+	// If omitted, a default private MAC prefix will be used.
+	// The prefix should be specified in full MAC notation, e.g.
+	// 06:42:42:00:00:00
+	MACPrefix string `json:"macPrefix,omitempty"`
 }
 
 // VMRoutes defined

pkg/controllers/bmh.go

@@ -34,6 +34,12 @@ import (
 	"vino/pkg/ipam"
 )
 
+const (
+	// DefaultMACPrefix is a private RFC 1918 MAC range used if
+	// no MACPrefix is specified for a network in the ViNO CR
+	DefaultMACPrefix = "02:00:00:00:00:00"
+)
+
 type networkTemplateValues struct {
 	Node      vinov1.NodeSet // the specific node type to be templated
 	BMHName   string
@@ -42,7 +48,8 @@ type networkTemplateValues struct {
 }
 
 type generatedValues struct {
-	IPAddresses map[string]string // a map of network names to IP addresses
+	IPAddresses  map[string]string // a map of network names to IP addresses
+	MACAddresses map[string]string // a map of network interface (link) names to MACs
 }
 
 func (r *VinoReconciler) ensureBMHs(ctx context.Context, vino *vinov1.Vino) error {
@@ -117,12 +124,18 @@ func (r *VinoReconciler) reconcileBMHs(ctx context.Context, vino *vinov1.Vino) e
 }
 
 func (r *VinoReconciler) createIpamNetworks(ctx context.Context, vino *vinov1.Vino) error {
+	logger := logr.FromContext(ctx)
 	for _, network := range vino.Spec.Networks {
 		subnetRange, err := ipam.NewRange(network.AllocationStart, network.AllocationStop)
 		if err != nil {
 			return err
 		}
-		err = r.Ipam.AddSubnetRange(ctx, network.SubNet, subnetRange)
+		if network.MACPrefix == "" {
+			logger.Info("No MACPrefix provided; using default MACPrefix %s for network %s",
+				DefaultMACPrefix, network.Name)
+			network.MACPrefix = DefaultMACPrefix
+		}
+		err = r.Ipam.AddSubnetRange(ctx, network.SubNet, subnetRange, network.MACPrefix)
 		if err != nil {
 			return err
 		}
@@ -131,8 +144,8 @@ func (r *VinoReconciler) createIpamNetworks(ctx context.Context, vino *vinov1.Vi
 }
 
 func (r *VinoReconciler) createBMHperPod(ctx context.Context, vino *vinov1.Vino, pod corev1.Pod) error {
+	logger := logr.FromContext(ctx)
 	for _, node := range vino.Spec.Nodes {
-		logger := logr.FromContext(ctx)
 		logger.Info("Creating BMHs for vino node", "node name", node.Name, "count", node.Count)
 		prefix := r.getBMHNodePrefix(vino, pod)
 		for i := 0; i < node.Count; i++ {
@@ -146,6 +159,7 @@ func (r *VinoReconciler) createBMHperPod(ctx context.Context, vino *vinov1.Vino,
 
 			// Allocate an IP for each of this BMH's network interfaces
 			ipAddresses := map[string]string{}
+			macAddresses := map[string]string{}
 			for _, iface := range node.NetworkInterfaces {
 				networkName := iface.NetworkName
 				subnet := ""
@@ -165,11 +179,12 @@ func (r *VinoReconciler) createBMHperPod(ctx context.Context, vino *vinov1.Vino,
 					return fmt.Errorf("Interface %s doesn't have a matching network defined", networkName)
 				}
 				ipAllocatedTo := fmt.Sprintf("%s/%s", bmhName, iface.NetworkName)
-				ipAddress, er := r.Ipam.AllocateIP(ctx, subnet, subnetRange, ipAllocatedTo)
+				ipAddress, macAddress, er := r.Ipam.AllocateIP(ctx, subnet, subnetRange, ipAllocatedTo)
 				if er != nil {
 					return er
 				}
 				ipAddresses[networkName] = ipAddress
+				macAddresses[iface.Name] = macAddress
 			}
 
 			values := networkTemplateValues{
@@ -177,7 +192,8 @@ func (r *VinoReconciler) createBMHperPod(ctx context.Context, vino *vinov1.Vino,
 				BMHName:  bmhName,
 				Networks: vino.Spec.Networks,
 				Generated: generatedValues{
-					IPAddresses: ipAddresses,
+					IPAddresses:  ipAddresses,
+					MACAddresses: macAddresses,
 				},
 			}
 			netData, netDataNs, err := r.reconcileBMHNetworkData(ctx, node, vino, values)

pkg/ipam/errors.go

@@ -54,7 +54,13 @@ type ErrInvalidIPAddress struct {
 	IP string
 }
 
-// ErrNotSupported returned if unsupported address types are used
+// ErrInvalidMACAddress returned if a MAC address string is malformed
+type ErrInvalidMACAddress struct {
+	MAC string
+}
+
+// ErrNotSupported returned if unsupported address types are used,
+// or if a change to immutable fields is attempted
 type ErrNotSupported struct {
 	Message string
 }
@@ -87,6 +93,10 @@ func (e ErrInvalidIPAddress) Error() string {
 	return fmt.Sprintf("IP address %s is invalid", e.IP)
 }
 
+func (e ErrInvalidMACAddress) Error() string {
+	return fmt.Sprintf("MAC address %s is invalid", e.MAC)
+}
+
 func (e ErrNotSupported) Error() string {
 	return fmt.Sprintf("%s", e.Message)
 }

pkg/ipam/ipam.go

@@ -46,7 +46,7 @@ func NewIpam(logger logr.Logger, client client.Client, namespace string) *Ipam {
 	}
 }
 
-// Create a new Range, validating its input
+// NewRange creates a new Range, validating its input
 func NewRange(start string, stop string) (vinov1.Range, error) {
 	r := vinov1.Range{Start: start, Stop: stop}
 	a, e := ipStringToInt(start)
@@ -69,8 +69,9 @@ func NewRange(start string, stop string) (vinov1.Range, error) {
 // subnet range than what is already allocated -- i.e. this function should be idempotent
 // against allocating the exact same subnet+range multiple times.
 // TODO error: invalid range for subnet
-func (i *Ipam) AddSubnetRange(ctx context.Context, subnet string, subnetRange vinov1.Range) error {
+func (i *Ipam) AddSubnetRange(ctx context.Context, subnet string, subnetRange vinov1.Range,
-	logger := i.Log.WithValues("subnet", subnet, "subnetRange", subnetRange)
+	macPrefix string) error {
+	logger := i.Log.WithValues("subnet", subnet, "subnetRange", subnetRange, "macPrefix", macPrefix)
 	// Does the subnet already exist? (this is fine)
 	ippools, err := i.getIPPools(ctx)
 	if err != nil {
@@ -80,13 +81,22 @@ func (i *Ipam) AddSubnetRange(ctx context.Context, subnet string, subnetRange vi
 	ippool, exists := ippools[subnet]
 	if !exists {
 		logger.Info("IPAM creating subnet")
+		_, err = macStringToInt(macPrefix) // mac format validation
+		if err != nil {
+			return err
+		}
 		ippool = &vinov1.IPPoolSpec{
 			Subnet:       subnet,
 			Ranges:       []vinov1.Range{},
 			AllocatedIPs: []vinov1.AllocatedIP{},
+			MACPrefix:    macPrefix,
+			NextMAC:      macPrefix,
 		}
 		ippools[subnet] = ippool
+	} else if ippool.MACPrefix != macPrefix {
+		return ErrNotSupported{Message: "Cannot change immutable field `macPrefix`"}
 	}
+
 	// Add the IPAM range to the subnet if it doesn't exist already
 	exists = false
 	for _, existingSubnetRange := range ippools[subnet].Ranges {
@@ -112,14 +122,14 @@ func (i *Ipam) AddSubnetRange(ctx context.Context, subnet string, subnetRange vi
 //              allocated IP.  If the same entity requests another IP, it will be given
 //              the same one.  I.e. this function is idempotent for the same allocatedTo.
 func (i *Ipam) AllocateIP(ctx context.Context, subnet string, subnetRange vinov1.Range,
-	allocatedTo string) (string, error) {
+	allocatedTo string) (allocatedIP string, allocatedMAC string, err error) {
 	ippools, err := i.getIPPools(ctx)
 	if err != nil {
-		return "", err
+		return "", "", err
 	}
 	ippool, exists := ippools[subnet]
 	if !exists {
-		return "", ErrSubnetNotAllocated{Subnet: subnet}
+		return "", "", ErrSubnetNotAllocated{Subnet: subnet}
 	}
 	// Make sure the range has been allocated within the subnet
 	var match bool
@@ -130,39 +140,50 @@ func (i *Ipam) AllocateIP(ctx context.Context, subnet string, subnetRange vinov1
 		}
 	}
 	if !match {
-		return "", ErrSubnetRangeNotAllocated{Subnet: subnet, SubnetRange: subnetRange}
+		return "", "", ErrSubnetRangeNotAllocated{Subnet: subnet, SubnetRange: subnetRange}
 	}
 
 	// If an IP has already been allocated to this entity, return it
-	ip := findAlreadyAllocatedIP(ippool, allocatedTo)
+	ip, mac := findAlreadyAllocatedIP(ippool, allocatedTo)
 
 	// No IP already allocated, so allocate a new IP
 	if ip == "" {
+		// Find an IP
 		ip, err = findFreeIPInRange(ippool, subnetRange)
 		if err != nil {
-			return "", err
+			return "", "", err
 		}
 		i.Log.Info("Allocating IP", "ip", ip, "subnet", subnet, "subnetRange", subnetRange)
 		ippool.AllocatedIPs = append(ippool.AllocatedIPs, vinov1.AllocatedIP{IP: ip, AllocatedTo: allocatedTo})
+
+		// Find a MAC
+		mac = ippool.NextMAC
+		macInt, err := macStringToInt(ippool.NextMAC)
+		if err != nil {
+			return "", "", err
+		}
+		ippool.NextMAC = intToMACString(macInt + 1)
+
+		// Save the updated IPPool
 		err = i.applyIPPool(ctx, *ippool)
 		if err != nil {
-			return "", err
+			return "", "", err
 		}
 	}
 
-	return ip, nil
+	return ip, mac, nil
 }
 
 // This returns an IP already allocated to the entity specified by `allocatedTo`
 // if it exists within the requested ippool/subnet, and a blank string
 // if no IP is already allocated.
-func findAlreadyAllocatedIP(ippool *vinov1.IPPoolSpec, allocatedTo string) string {
+func findAlreadyAllocatedIP(ippool *vinov1.IPPoolSpec, allocatedTo string) (ip string, mac string) {
 	for _, allocatedIP := range ippool.AllocatedIPs {
 		if allocatedIP.AllocatedTo == allocatedTo {
-			return allocatedIP.IP
+			return allocatedIP.IP, allocatedIP.MAC
 		}
 	}
-	return ""
+	return "", ""
 }
 
 // This converts IP ranges/addresses into iterable ints,
@@ -235,6 +256,24 @@ func ipStringToInt(ipString string) (uint64, error) {
 	return byteArrayToInt(bytes), nil
 }
 
+// Convert a MAC address in xx:xx:xx:xx:xx:xx format to an easily iterable uint64.
+func macStringToInt(macString string) (uint64, error) {
+	// ParseMAC parses various flavors of macs; we restrict to vanilla ethernet
+	regex := regexp.MustCompile(`[..:..:..:..:..:..]`)
+	if !regex.MatchString(macString) {
+		return 0, ErrInvalidMACAddress{macString}
+	}
+
+	bytes, err := net.ParseMAC(macString)
+	if err != nil {
+		return 0, ErrInvalidMACAddress{macString}
+	}
+
+	// Pad to 8 bytes for the uint64 conversion
+	bytes = append(make([]byte, 2), bytes...)
+	return byteArrayToInt(bytes), nil
+}
+
 func intToIPv4String(i uint64) string {
 	bytes := intToByteArray(i)
 	ip := net.IPv4(bytes[4], bytes[5], bytes[6], bytes[7])
@@ -249,6 +288,13 @@ func intToIPv6String(i uint64) string {
 	return ip.String()
 }
 
+func intToMACString(i uint64) string {
+	bytes := intToByteArray(i)
+	// lop off the first two bytes to get a 6-byte array
+	var hardwareAddress net.HardwareAddr = bytes[2:]
+	return hardwareAddress.String()
+}
+
 // Convert an uint64 into 8 bytes, with most significant byte first
 // Based on https://gist.github.com/ecoshub/5be18dc63ac64f3792693bb94f00662f
 func intToByteArray(num uint64) []byte {

pkg/ipam/ipam_test.go

@@ -43,6 +43,8 @@ func SetUpMockClient(ctx context.Context, ctrl *gomock.Controller) *test.MockCli
 					Ranges: []vinov1.Range{
 						{Start: "10.0.1.0", Stop: "10.0.1.9"},
 					},
+					MACPrefix: "02:00:00:00:00:00",
+					NextMAC:   "02:00:00:00:00:00",
 				},
 			},
 			{
@@ -51,6 +53,8 @@ func SetUpMockClient(ctx context.Context, ctrl *gomock.Controller) *test.MockCli
 					Ranges: []vinov1.Range{
 						{Start: "2600:1700:b030:0000::", Stop: "2600:1700:b030:0009::"},
 					},
+					MACPrefix: "06:00:00:00:00:00",
+					NextMAC:   "06:00:00:00:00:00",
 				},
 			},
 			{
@@ -60,8 +64,10 @@ func SetUpMockClient(ctx context.Context, ctrl *gomock.Controller) *test.MockCli
 						{Start: "192.168.0.0", Stop: "192.168.0.0"},
 					},
 					AllocatedIPs: []vinov1.AllocatedIP{
-						{IP: "192.168.0.0", AllocatedTo: "old-vm-name"},
+						{IP: "192.168.0.0", MAC: "02:00:00:00:00:00", AllocatedTo: "old-vm-name"},
 					},
+					MACPrefix: "02:00:00:00:00:00",
+					NextMAC:   "02:00:00:00:00:01",
 				},
 			},
 			{
@@ -71,8 +77,10 @@ func SetUpMockClient(ctx context.Context, ctrl *gomock.Controller) *test.MockCli
 						{Start: "2600:1700:b031:0000::", Stop: "2600:1700:b031:0000::"},
 					},
 					AllocatedIPs: []vinov1.AllocatedIP{
-						{IP: "2600:1700:b031:0000::", AllocatedTo: "old-vm-name"},
+						{IP: "2600:1700:b031:0000::", MAC: "06:00:00:00:00:00", AllocatedTo: "old-vm-name"},
 					},
+					MACPrefix: "06:00:00:00:00:00",
+					NextMAC:   "06:00:00:00:00:01",
 				},
 			},
 		},
@@ -87,20 +95,22 @@ func SetUpMockClient(ctx context.Context, ctrl *gomock.Controller) *test.MockCli
 
 func TestAllocateIP(t *testing.T) {
 	tests := []struct {
-		name, subnet, allocatedTo, expectedErr string
+		name, subnet, allocatedTo, expectedErr, expectedMAC string
-		subnetRange                            vinov1.Range
+		subnetRange                                         vinov1.Range
 	}{
 		{
 			name:        "success ipv4",
 			subnet:      "10.0.0.0/16",
 			subnetRange: vinov1.Range{Start: "10.0.1.0", Stop: "10.0.1.9"},
 			allocatedTo: "new-vm-name",
+			expectedMAC: "02:00:00:00:00:00",
 		},
 		{
 			name:        "success ipv6",
 			subnet:      "2600:1700:b030:0000::/72",
 			subnetRange: vinov1.Range{Start: "2600:1700:b030:0000::", Stop: "2600:1700:b030:0009::"},
 			allocatedTo: "new-vm-name",
+			expectedMAC: "06:00:00:00:00:00",
 		},
 		{
 			name:        "error subnet not allocated ipv4",
@@ -136,6 +146,7 @@ func TestAllocateIP(t *testing.T) {
 			subnet:      "192.168.0.0/1",
 			subnetRange: vinov1.Range{Start: "192.168.0.0", Stop: "192.168.0.0"},
 			allocatedTo: "old-vm-name",
+			expectedMAC: "02:00:00:00:00:00",
 		},
 		{
 			name:        "error range exhausted ipv4",
@@ -165,14 +176,16 @@ func TestAllocateIP(t *testing.T) {
 			ipammer := NewIpam(log.Log, m, "vino-system")
 			ipammer.Log = log.Log
 
-			ip, err := ipammer.AllocateIP(ctx, tt.subnet, tt.subnetRange, tt.allocatedTo)
+			ip, mac, err := ipammer.AllocateIP(ctx, tt.subnet, tt.subnetRange, tt.allocatedTo)
 			if tt.expectedErr != "" {
 				require.Error(t, err)
 				assert.Equal(t, "", ip)
+				assert.Equal(t, "", mac)
 				assert.Contains(t, err.Error(), tt.expectedErr)
 			} else {
 				require.NoError(t, err)
 				assert.NotEmpty(t, ip)
+				assert.Equal(t, tt.expectedMAC, mac)
 			}
 		})
 	}
@@ -192,19 +205,19 @@ func TestNewRange(t *testing.T) {
 			name:        "error stop less than start",
 			start:       "10.0.0.2",
 			stop:        "10.0.0.1",
-			expectedErr: "is invalid",
+			expectedErr: "IPAM range",
 		},
 		{
 			name:        "error bad start",
 			start:       "10.0.0.2.x",
 			stop:        "10.0.0.1",
-			expectedErr: "is invalid",
+			expectedErr: "IP address",
 		},
 		{
 			name:        "error bad stop",
 			start:       "10.0.0.2",
 			stop:        "10.0.0.1.x",
-			expectedErr: "is invalid",
+			expectedErr: "IP address",
 		},
 	}
 	for _, tt := range tests {
@@ -226,15 +239,30 @@ func TestNewRange(t *testing.T) {
 // Test some error handling that is not captured by TestAllocateIP
 func TestAddSubnetRange(t *testing.T) {
 	tests := []struct {
-		name, subnet, expectedErr string
+		name, subnet, macPrefix, expectedErr string
-		subnetRange               vinov1.Range
+		subnetRange                          vinov1.Range
 	}{
 		{
 			name:        "success",
-			subnet:      "10.0.0.0/16",
+			subnet:      "20.0.0.0/16",
-			subnetRange: vinov1.Range{Start: "10.0.2.0", Stop: "10.0.2.9"},
+			subnetRange: vinov1.Range{Start: "20.0.2.0", Stop: "20.0.2.9"},
+			macPrefix:   "02:00:00:00:00:00",
 			expectedErr: "",
 		},
+		{
+			name:        "error bad mac",
+			subnet:      "20.0.0.0/16",
+			subnetRange: vinov1.Range{Start: "20.0.2.0", Stop: "20.0.2.9"},
+			macPrefix:   "",
+			expectedErr: "MAC address",
+		},
+		{
+			name:        "error macPrefix is immutable",
+			subnet:      "10.0.0.0/16",
+			subnetRange: vinov1.Range{Start: "10.0.1.0", Stop: "10.0.1.9"},
+			macPrefix:   "02:00:00:00:00:0`",
+			expectedErr: "immutable",
+		},
 		// TODO: check for partially overlapping ranges and subnets
 	}
 
@@ -248,7 +276,7 @@ func TestAddSubnetRange(t *testing.T) {
 			m := SetUpMockClient(ctx, ctrl)
 			ipammer := NewIpam(log.Log, m, "vino-system")
 
-			err := ipammer.AddSubnetRange(ctx, tt.subnet, tt.subnetRange)
+			err := ipammer.AddSubnetRange(ctx, tt.subnet, tt.subnetRange, tt.macPrefix)
 			if tt.expectedErr != "" {
 				require.Error(t, err)
 				assert.Contains(t, err.Error(), tt.expectedErr)
@@ -408,6 +436,48 @@ func TestIPStringToInt(t *testing.T) {
 	}
 }
 
+func TestMACStringToInt(t *testing.T) {
+	tests := []struct {
+		name        string
+		in          string
+		out         uint64
+		expectedErr string
+	}{
+		{
+			name: "valid MAC address",
+			in:   "00:00:00:00:01:01",
+			out:  0x101,
+		},
+		{
+			name:        "invalid MAC address",
+			in:          "00:00:00:00:01:01:00",
+			out:         0,
+			expectedErr: " is invalid",
+		},
+		{
+			name:        "blank MAC address",
+			in:          "",
+			out:         0,
+			expectedErr: " is invalid",
+		},
+	}
+
+	for _, tt := range tests {
+		tt := tt
+		t.Run(tt.name, func(t *testing.T) {
+			actual, err := macStringToInt(tt.in)
+			if tt.expectedErr != "" {
+				require.Error(t, err)
+				assert.Empty(t, tt.out)
+				assert.Contains(t, err.Error(), tt.expectedErr)
+			} else {
+				require.NoError(t, err)
+				assert.Equal(t, tt.out, actual)
+			}
+		})
+	}
+}
+
 func TestIntToByteArray(t *testing.T) {
 	tests := []struct {
 		name string