From 274a4ec2e89183c93fe3d26b11da1a93b7c87345 Mon Sep 17 00:00:00 2001 From: Bryan Boreham Date: Wed, 23 Dec 2020 11:40:05 +0000 Subject: [PATCH 1/2] Assign a random MAC address to the bridge If we don't do this it will adopt the lowest address of an attached device, hence change over time. Remove the previous work-around of getting and setting the random MAC that Linux assigned, also getting and setting the TUN address since that doesn't change if you set the bridge address. --- pkg/container/network.go | 53 +++++++++++++++++----------------------- 1 file changed, 23 insertions(+), 30 deletions(-) diff --git a/pkg/container/network.go b/pkg/container/network.go index 9f18c15ca..03393c0a8 100644 --- a/pkg/container/network.go +++ b/pkg/container/network.go @@ -1,6 +1,7 @@ package container import ( + "crypto/rand" "fmt" "net" "time" @@ -224,6 +225,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, err + } + 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 +252,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 - } + // 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 - if err = netlink.LinkSetMasterByIndex(link, masterIndex); err != nil { - return err + return net.HardwareAddr(mac), nil } - if err = netlink.LinkSetHardwareAddr(link, mac); err != nil { +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 err } } - return netlink.LinkSetHardwareAddr(master, masterMAC) -} - -// 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 - } - - addr = link.Attrs().HardwareAddr - return + return nil } func maskString(mask net.IPMask) string { From 0b3815e27b6bbadaf2e57d52b977b92cf821a0b0 Mon Sep 17 00:00:00 2001 From: Bryan Boreham Date: Wed, 23 Dec 2020 11:42:33 +0000 Subject: [PATCH 2/2] Add context to netlink errors Otherwise it just says something like 'invalid argument' and you have to guess what the problem is --- pkg/container/network.go | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/pkg/container/network.go b/pkg/container/network.go index 03393c0a8..9a26be0fb 100644 --- a/pkg/container/network.go +++ b/pkg/container/network.go @@ -6,6 +6,7 @@ import ( "net" "time" + "github.com/pkg/errors" log "github.com/sirupsen/logrus" "github.com/vishvananda/netlink" "github.com/weaveworks/ignite/pkg/constants" @@ -118,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 { @@ -229,7 +230,7 @@ func createBridge(bridgeName string) (*netlink.Bridge, error) { // the lowest address of an attached device, hence change over time. mac, err := randomMAC() if err != nil { - return nil, err + return nil, errors.Wrap(err, "creating random MAC") } la.HardwareAddr = mac @@ -263,14 +264,14 @@ func randomMAC() (net.HardwareAddr, error) { mac[0] = (mac[0] & 0xFE) | 0x02 return net.HardwareAddr(mac), nil - } +} 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 err + return errors.Wrapf(err, "setMaster %s %s", master.Attrs().Name, link.Attrs().Name) } }