Bugfix: Pod or gateway interface use a different MAC from the expecta…

…tion

This issue is found on Ubuntu 22.04: the network interface for antrea-gw0
is different from the one we used in OpenFlow rules.

The reason is systemd-udev has modified the interface's MAC after it watches
a new one is created. So if Antrea Agent reads interface's information before
systemd-udev's modification, Antrea Agent would uses an incorrect value to
install OpenFlow rules.

To resolve the issue,
  1. Agent generates a static MAC for antrea-gw0 or the interface used by Pod
  2. Agent uses the generated MAC to create OVS internal port or veth pair

To implement the logic, some code is copied from containernetworking/plugins/ip/link_linux
latest versions to path thirdparty, this is to avoid unexpected issues introduced
when bumping up the dependent libraries.

Signed-off-by: wenyingd <wenyingd@vmware.com>

pkg/agent/agent.go

@@ -77,6 +77,12 @@ var (
 
 	// getTransportIPNetDeviceByName is meant to be overridden for testing.
 	getTransportIPNetDeviceByName = GetTransportIPNetDeviceByName
+
+	// setLinkUp is meant to be overridden for testing
+	setLinkUp = util.SetLinkUp
+
+	// configureLinkAddresses is meant to be overridden for testing
+	configureLinkAddresses = util.ConfigureLinkAddresses
 )
 
 // otherConfigKeysForIPsecCertificates are configurations added to OVS bridge when AuthenticationMode is "cert" and
@@ -243,6 +249,7 @@ func (i *Initializer) initInterfaceStore() error {
 		intf := &interfacestore.InterfaceConfig{
 			Type:          interfacestore.GatewayInterface,
 			InterfaceName: port.Name,
+			MAC:           port.MAC,
 			OVSPortConfig: ovsPort}
 		if intf.InterfaceName != i.hostGateway {
 			klog.Warningf("The discovered gateway interface name %s is different from the configured value: %s",
@@ -639,7 +646,8 @@ func (i *Initializer) setupGatewayInterface() error {
 		externalIDs := map[string]interface{}{
 			interfacestore.AntreaInterfaceTypeKey: interfacestore.AntreaGateway,
 		}
-		gwPortUUID, err := i.ovsBridgeClient.CreateInternalPort(i.hostGateway, config.HostGatewayOFPort, "", externalIDs)
+		mac := util.GenerateRandomMAC()
+		gwPortUUID, err := i.ovsBridgeClient.CreateInternalPort(i.hostGateway, config.HostGatewayOFPort, mac.String(), externalIDs)
 		if err != nil {
 			klog.ErrorS(err, "Failed to create gateway port on OVS bridge", "port", i.hostGateway)
 			return err
@@ -650,15 +658,15 @@ func (i *Initializer) setupGatewayInterface() error {
 			return err
 		}
 		klog.InfoS("Allocated OpenFlow port for gateway interface", "port", i.hostGateway, "ofPort", gwPort)
-		gatewayIface = interfacestore.NewGatewayInterface(i.hostGateway)
+		gatewayIface = interfacestore.NewGatewayInterface(i.hostGateway, mac)
 		gatewayIface.OVSPortConfig = &interfacestore.OVSPortConfig{PortUUID: gwPortUUID, OFPort: gwPort}
 		i.ifaceStore.AddInterface(gatewayIface)
 	} else {
 		klog.V(2).Infof("Gateway port %s already exists on OVS bridge", i.hostGateway)
 	}
 
 	// Idempotent operation to set the gateway's MTU: we perform this operation regardless of
-	// whether or not the gateway interface already exists, as the desired MTU may change across
+	// whether the gateway interface already exists, as the desired MTU may change across
 	// restarts.
 	klog.V(4).Infof("Setting gateway interface %s MTU to %d", i.hostGateway, i.nodeConfig.NodeMTU)
 
@@ -673,13 +681,12 @@ func (i *Initializer) setupGatewayInterface() error {
 }
 
 func (i *Initializer) configureGatewayInterface(gatewayIface *interfacestore.InterfaceConfig) error {
-	var gwMAC net.HardwareAddr
 	var gwLinkIdx int
 	var err error
 	// Host link might not be queried at once after creating OVS internal port; retry max 5 times with 1s
 	// delay each time to ensure the link is ready.
 	for retry := 0; retry < maxRetryForHostLink; retry++ {
-		gwMAC, gwLinkIdx, err = util.SetLinkUp(i.hostGateway)
+		gwLinkIdx, err = setLinkUp(i.hostGateway)
 		if err == nil {
 			break
 		}
@@ -696,8 +703,7 @@ func (i *Initializer) configureGatewayInterface(gatewayIface *interfacestore.Int
 		return err
 	}
 
-	i.nodeConfig.GatewayConfig = &config.GatewayConfig{Name: i.hostGateway, MAC: gwMAC, OFPort: uint32(gatewayIface.OFPort)}
-	gatewayIface.MAC = gwMAC
+	i.nodeConfig.GatewayConfig = &config.GatewayConfig{Name: i.hostGateway, MAC: gatewayIface.MAC, OFPort: uint32(gatewayIface.OFPort)}
 	gatewayIface.IPs = []net.IP{}
 	if i.networkConfig.TrafficEncapMode.IsNetworkPolicyOnly() {
 		// Assign IP to gw as required by SpoofGuard.
@@ -1146,13 +1152,13 @@ func (i *Initializer) allocateGatewayAddresses(localSubnets []*net.IPNet, gatewa
 	// (i.e. portExists is false). Indeed, it may be possible for the interface to exist even if the OVS bridge does
 	// not exist.
 	// Configure any missing IP address on the interface. Remove any extra IP address that may exist.
-	if err := util.ConfigureLinkAddresses(i.nodeConfig.GatewayConfig.LinkIndex, gwIPs); err != nil {
+	if err := configureLinkAddresses(i.nodeConfig.GatewayConfig.LinkIndex, gwIPs); err != nil {
 		return err
 	}
 	// Periodically check whether IP configuration of the gateway is correct.
 	// Terminate when stopCh is closed.
 	go wait.Until(func() {
-		if err := util.ConfigureLinkAddresses(i.nodeConfig.GatewayConfig.LinkIndex, gwIPs); err != nil {
+		if err := configureLinkAddresses(i.nodeConfig.GatewayConfig.LinkIndex, gwIPs); err != nil {
 			klog.Errorf("Failed to check IP configuration of the gateway: %v", err)
 		}
 	}, 60*time.Second, i.stopCh)

pkg/agent/agent_linux.go

@@ -235,7 +235,7 @@ func (i *Initializer) ConnectUplinkToOVSBridge() error {
 	if err != nil {
 		return err
 	}
-	if _, _, err = util.SetLinkUp(uplinkName); err != nil {
+	if _, err = util.SetLinkUp(uplinkName); err != nil {
 		return err
 	}
 	if err = util.ConfigureLinkAddresses(localLink.Attrs().Index, uplinkIPs); err != nil {

pkg/agent/agent_linux_test.go

@@ -0,0 +1,19 @@
+// Copyright 2022 Antrea Authors
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//     http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package agent
+
+func mockSetInterfaceMTU(returnErr error) func() {
+	return func() {}
+}

pkg/agent/agent_test.go

@@ -578,3 +578,70 @@ func TestSetupDefaultTunnelInterface(t *testing.T) {
 		})
 	}
 }
+
+func TestSetupGatewayInterface(t *testing.T) {
+	defer mockSetLinkUp(10, nil)()
+	defer mockConfigureLinkAddress(nil)()
+	defer mockSetInterfaceMTU(nil)()
+
+	controller := mock.NewController(t)
+	defer controller.Finish()
+
+	podCIDRStr := "172.16.10.0/24"
+	_, podCIDR, _ := net.ParseCIDR(podCIDRStr)
+	nodeConfig := &config.NodeConfig{
+		Name:        "n1",
+		Type:        config.K8sNode,
+		OVSBridge:   "br-int",
+		PodIPv4CIDR: podCIDR,
+		NodeMTU:     1450,
+	}
+	networkConfig := &config.NetworkConfig{
+		TrafficEncapMode: config.TrafficEncapModeEncap,
+		TunnelType:       ovsconfig.GeneveTunnel,
+		TunnelCsum:       false,
+	}
+
+	mockOVSBridgeClient := ovsconfigtest.NewMockOVSBridgeClient(controller)
+	client := fake.NewSimpleClientset()
+	ifaceStore := interfacestore.NewInterfaceStore()
+	stopCh := make(chan struct{})
+	initializer := &Initializer{
+		client:          client,
+		ifaceStore:      ifaceStore,
+		ovsBridgeClient: mockOVSBridgeClient,
+		ovsBridge:       "br-int",
+		networkConfig:   networkConfig,
+		nodeConfig:      nodeConfig,
+		hostGateway:     "antrea-gw0",
+		stopCh:          stopCh,
+	}
+	close(stopCh)
+	portUUID := "123456780a"
+	ofport := int32(config.HostGatewayOFPort)
+	mockOVSBridgeClient.EXPECT().CreateInternalPort(initializer.hostGateway, ofport, mock.Any(), mock.Any()).Return(portUUID, nil)
+	mockOVSBridgeClient.EXPECT().GetOFPort(initializer.hostGateway, false).Return(ofport, nil)
+	mockOVSBridgeClient.EXPECT().SetInterfaceMTU(initializer.hostGateway, nodeConfig.NodeMTU).Return(nil)
+	err := initializer.setupGatewayInterface()
+	assert.NoError(t, err)
+}
+
+func mockSetLinkUp(returnIndex int, returnErr error) func() {
+	originalSetLinkUp := setLinkUp
+	setLinkUp = func(name string) (int, error) {
+		return returnIndex, returnErr
+	}
+	return func() {
+		setLinkUp = originalSetLinkUp
+	}
+}
+
+func mockConfigureLinkAddress(returnedErr error) func() {
+	originalConfigureLinkAddresses := configureLinkAddresses
+	configureLinkAddresses = func(idx int, ipNets []*net.IPNet) error {
+		return returnedErr
+	}
+	return func() {
+		configureLinkAddresses = originalConfigureLinkAddresses
+	}
+}

pkg/agent/agent_windows.go

@@ -34,6 +34,11 @@ import (
 	utilip "antrea.io/antrea/pkg/util/ip"
 )
 
+var (
+	// setInterfaceMTU is meant to be overridden for testing
+	setInterfaceMTU = util.SetInterfaceMTU
+)
+
 func (i *Initializer) prepareHostNetwork() error {
 	if i.nodeConfig.Type == config.K8sNode {
 		return i.prepareHNSNetworkAndOVSExtension()
@@ -419,7 +424,7 @@ func (i *Initializer) setInterfaceMTU(iface string, mtu int) error {
 	if err := i.ovsBridgeClient.SetInterfaceMTU(iface, mtu); err != nil {
 		return err
 	}
-	return util.SetInterfaceMTU(iface, mtu)
+	return setInterfaceMTU(iface, mtu)
 }
 
 func (i *Initializer) setVMNodeConfig(en *v1alpha1.ExternalNode, nodeName string) error {

pkg/agent/agent_windows_test.go

@@ -0,0 +1,25 @@
+// Copyright 2022 Antrea Authors
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//     http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package agent
+
+func mockSetInterfaceMTU(returnErr error) func() {
+	originalSetInterfaceMTU := setInterfaceMTU
+	setInterfaceMTU = func(ifaceName string, mtu int) error {
+		return returnErr
+	}
+	return func() {
+		setInterfaceMTU = originalSetInterfaceMTU
+	}
+}

pkg/agent/cniserver/interface_configuration_linux.go

@@ -37,6 +37,7 @@ import (
 	"antrea.io/antrea/pkg/agent/util/ndp"
 	cnipb "antrea.io/antrea/pkg/apis/cni/v1beta1"
 	"antrea.io/antrea/pkg/ovs/ovsconfig"
+	cniip "antrea.io/antrea/third_party/containernetworking/ip"
 )
 
 // NetDeviceType type Enum
@@ -254,13 +255,14 @@ func (ic *ifConfigurator) configureContainerLinkVeth(
 	containerIface := &current.Interface{Name: containerIfaceName, Sandbox: containerNetNS}
 	result.Interfaces = []*current.Interface{hostIface, containerIface}
 
+	podMAC := util.GenerateRandomMAC()
 	if err := ns.WithNetNSPath(containerNetNS, func(hostNS ns.NetNS) error {
 		klog.V(2).Infof("Creating veth devices (%s, %s) for container %s", containerIfaceName, hostIfaceName, containerID)
-		hostVeth, containerVeth, err := ip.SetupVethWithName(containerIfaceName, hostIfaceName, mtu, hostNS)
+		hostVeth, containerVeth, err := cniip.SetupVethWithName(containerIfaceName, hostIfaceName, mtu, podMAC.String(), hostNS)
 		if err != nil {
 			return fmt.Errorf("failed to create veth devices for container %s: %v", containerID, err)
 		}
-		containerIface.Mac = containerVeth.HardwareAddr.String()
+		containerIface.Mac = podMAC.String()
 		hostIface.Mac = hostVeth.HardwareAddr.String()
 		// Disable TX checksum offloading when it's configured explicitly.
 		if ic.disableTXChecksumOffload {

pkg/agent/controller/trafficcontrol/controller.go

@@ -590,7 +590,7 @@ func (c *Controller) createOVSInternalPort(portName string) (string, error) {
 		return "", err
 	}
 	if pollErr := wait.PollImmediate(time.Second, 5*time.Second, func() (bool, error) {
-		_, _, err := util.SetLinkUp(portName)
+		_, err := util.SetLinkUp(portName)
 		if err == nil {
 			return true, nil
 		}

pkg/agent/interfacestore/interface_cache_test.go

@@ -89,7 +89,7 @@ func testContainerInterface(t *testing.T) {
 }
 
 func testGatewayInterface(t *testing.T) {
-	gatewayInterface := NewGatewayInterface("antrea-gw0")
+	gatewayInterface := NewGatewayInterface("antrea-gw0", util.GenerateRandomMAC())
 	gatewayInterface.IPs = []net.IP{gwIP}
 	gatewayInterface.OVSPortConfig = &OVSPortConfig{
 		OFPort:   13,

pkg/agent/interfacestore/types.go

@@ -146,8 +146,8 @@ func NewContainerInterface(
 }
 
 // NewGatewayInterface creates InterfaceConfig for the host gateway interface.
-func NewGatewayInterface(gatewayName string) *InterfaceConfig {
-	gatewayConfig := &InterfaceConfig{InterfaceName: gatewayName, Type: GatewayInterface}
+func NewGatewayInterface(gatewayName string, gatewayMAC net.HardwareAddr) *InterfaceConfig {
+	gatewayConfig := &InterfaceConfig{InterfaceName: gatewayName, Type: GatewayInterface, MAC: gatewayMAC}
 	return gatewayConfig
 }
 

pkg/agent/util/net.go

@@ -406,8 +406,9 @@ func GenerateRandomMAC() net.HardwareAddr {
 	if _, err := rand.Read(buf); err != nil {
 		klog.ErrorS(err, "Failed to generate a random MAC")
 	}
-	// Set the local bit
-	buf[0] |= 2
+	// Unset the multicast bit.
+	buf[0] &= 0xfe
+	buf[0] |= 0x02
 	return buf
 }
 

pkg/agent/util/net_linux.go

@@ -106,22 +106,21 @@ func GetNSPath(netnsName string) (string, error) {
 	return netNS.Path(), nil
 }
 
-func SetLinkUp(name string) (net.HardwareAddr, int, error) {
+func SetLinkUp(name string) (int, error) {
 	link, err := netlink.LinkByName(name)
 	if err != nil {
 		if _, ok := err.(netlink.LinkNotFoundError); ok {
-			return nil, 0, newLinkNotFoundError(name)
+			return 0, newLinkNotFoundError(name)
 		}
-		return nil, 0, err
+		return 0, err
 	}
 	// Set host gateway interface up.
 	if err := netlink.LinkSetUp(link); err != nil {
 		klog.Errorf("Failed to set host link for %s up: %v", name, err)
-		return nil, 0, err
+		return 0, err
 	}
-	mac := link.Attrs().HardwareAddr
 	index := link.Attrs().Index
-	return mac, index, nil
+	return index, nil
 }
 
 func addrSliceDifference(s1, s2 []netlink.Addr) []*netlink.Addr {

pkg/agent/util/net_test.go

@@ -104,3 +104,15 @@ func TestExtendCIDRWithIP(t *testing.T) {
 		assert.Equal(t, expectedIPNet, gotIPNet)
 	}
 }
+
+func TestGenerateRandomMAC(t *testing.T) {
+	validateBits := func(mac net.HardwareAddr) (byte, byte) {
+		localBit := mac[0] & 0x2 >> 1
+		mcastBit := mac[0] & 0x1
+		return localBit, mcastBit
+	}
+	mac1 := GenerateRandomMAC()
+	localBit, mcastBit := validateBits(mac1)
+	assert.Equal(t, uint8(1), localBit)
+	assert.Equal(t, uint8(0), mcastBit)
+}

pkg/agent/util/net_windows.go

@@ -289,26 +289,25 @@ func EnableHNSNetworkExtension(hnsNetID string, vSwitchExtension string) error {
 	return nil
 }
 
-func SetLinkUp(name string) (net.HardwareAddr, int, error) {
+func SetLinkUp(name string) (int, error) {
 	// Set host gateway interface up.
 	if err := EnableHostInterface(name); err != nil {
 		klog.Errorf("Failed to set host link for %s up: %v", name, err)
 		if strings.Contains(err.Error(), "ObjectNotFound") {
-			return nil, 0, newLinkNotFoundError(name)
+			return 0, newLinkNotFoundError(name)
 		}
-		return nil, 0, err
+		return 0, err
 	}
 
 	iface, err := net.InterfaceByName(name)
 	if err != nil {
 		if opErr, ok := err.(*net.OpError); ok && opErr.Err.Error() == "no such network interface" {
-			return nil, 0, newLinkNotFoundError(name)
+			return 0, newLinkNotFoundError(name)
 		}
-		return nil, 0, err
+		return 0, err
 	}
-	mac := iface.HardwareAddr
 	index := iface.Index
-	return mac, index, nil
+	return index, nil
 }
 
 func addrEqual(addr1, addr2 *net.IPNet) bool {