Assign a random MAC address to the bridge

pkg/container/network.go

@@ -1,10 +1,12 @@
 package container
 
 import (
+	"crypto/rand"
 	"fmt"
 	"net"
 	"time"
 
+	"github.com/pkg/errors"
 	log "github.com/sirupsen/logrus"
 	"github.com/vishvananda/netlink"
 	"github.com/weaveworks/ignite/pkg/constants"
@@ -117,17 +119,17 @@ func bridge(iface *net.Interface) (*DHCPInterface, error) {
 
 	eth, err := netlink.LinkByIndex(iface.Index)
 	if err != nil {
-		return nil, err
+		return nil, errors.Wrap(err, "LinkByIndex")
 	}
 
 	tuntap, err := createTAPAdapter(tapName)
 	if err != nil {
-		return nil, err
+		return nil, errors.Wrap(err, "createTAPAdapter")
 	}
 
 	bridge, err := createBridge(bridgeName)
 	if err != nil {
-		return nil, err
+		return nil, errors.Wrap(err, "createBridge")
 	}
 
 	if err := setMaster(bridge, tuntap, eth); err != nil {
@@ -224,6 +226,14 @@ func createBridge(bridgeName string) (*netlink.Bridge, error) {
 	la := netlink.NewLinkAttrs()
 	la.Name = bridgeName
 
+	// Assign a specific mac to the bridge - if we don't do this it will adopt
+	// the lowest address of an attached device, hence change over time.
+	mac, err := randomMAC()
+	if err != nil {
+		return nil, errors.Wrap(err, "creating random MAC")
+	}
+	la.HardwareAddr = mac
+
 	// Disable MAC address age tracking. This causes issues in the container,
 	// the bridge is unable to resolve MACs from outside resulting in it never
 	// establishing the internal routes. This "optimization" is only really useful
@@ -243,45 +253,29 @@ func addLink(link netlink.Link) (err error) {
 	return
 }
 
-// This is a MAC address persistence workaround, netlink.LinkSetMaster{,ByIndex}()
-// has a bug that arbitrarily changes the MAC addresses of the bridge and virtual
-// device to be bound to it. TODO: Remove when fixed upstream
-func setMaster(master netlink.Link, links ...netlink.Link) error {
-	masterIndex := master.Attrs().Index
-	masterMAC, err := getMAC(master)
-	if err != nil {
-		return err
+func randomMAC() (net.HardwareAddr, error) {
+	mac := make([]byte, 6)
+	if _, err := rand.Read(mac); err != nil {
+		return nil, err
 	}
 
-	for _, link := range links {
-		mac, err := getMAC(link)
-		if err != nil {
-			return err
-		}
-
-		if err = netlink.LinkSetMasterByIndex(link, masterIndex); err != nil {
-			return err
-		}
-
-		if err = netlink.LinkSetHardwareAddr(link, mac); err != nil {
-			return err
-		}
-	}
+	// In the first byte of the MAC, the 'multicast' bit should be
+	// clear and 'locally administered' bit should be set.
+	mac[0] = (mac[0] & 0xFE) | 0x02
 
-	return netlink.LinkSetHardwareAddr(master, masterMAC)
+	return net.HardwareAddr(mac), nil
 }
 
-// getMAC fetches the generated MAC address for the given link
-func getMAC(link netlink.Link) (addr net.HardwareAddr, err error) {
-	// The attributes of the netlink.Link passed to this function do not contain HardwareAddr
-	// as it is expected to be generated by the networking subsystem. Thus, "reload" the Link
-	// by querying it to retrieve the generated attributes after the link has been created.
-	if link, err = netlink.LinkByIndex(link.Attrs().Index); err != nil {
-		return
+func setMaster(master netlink.Link, links ...netlink.Link) error {
+	masterIndex := master.Attrs().Index
+
+	for _, link := range links {
+		if err := netlink.LinkSetMasterByIndex(link, masterIndex); err != nil {
+			return errors.Wrapf(err, "setMaster %s %s", master.Attrs().Name, link.Attrs().Name)
+		}
 	}
 
-	addr = link.Attrs().HardwareAddr
-	return
+	return nil
 }
 
 func maskString(mask net.IPMask) string {