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

…tion (#4428)

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 use 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

@@ -15,6 +15,7 @@
 package agent
 
 import (
+	"bytes"
 	"context"
 	"encoding/json"
 	"fmt"
@@ -77,6 +78,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 +250,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 +647,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 +659,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)
 
@@ -679,7 +688,7 @@ func (i *Initializer) configureGatewayInterface(gatewayIface *interfacestore.Int
 	// 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)
+		gwMAC, gwLinkIdx, err = setLinkUp(i.hostGateway)
 		if err == nil {
 			break
 		}
@@ -695,9 +704,17 @@ func (i *Initializer) configureGatewayInterface(gatewayIface *interfacestore.Int
 		klog.Errorf("Failed to find host link for gateway %s: %v", i.hostGateway, err)
 		return err
 	}
-
+	// Persist the MAC configured in the network interface when the gatewayIface.MAC is not set. This may
+	// happen in upgrade case.
+	// Note the "mac" field in Windows OVS internal Interface has no impact on the network adapter's actual MAC,
+	// set it to the same value just to keep consistency.
+	if bytes.Compare(gatewayIface.MAC, gwMAC) != 0 {
+		gatewayIface.MAC = gwMAC
+		if err := i.ovsBridgeClient.SetInterfaceMAC(gatewayIface.InterfaceName, gwMAC); err != nil {
+			klog.ErrorS(err, "Failed to persist interface MAC address", "interface", gatewayIface.InterfaceName, "mac", gwMAC)
+		}
+	}
 	i.nodeConfig.GatewayConfig = &config.GatewayConfig{Name: i.hostGateway, MAC: gwMAC, OFPort: uint32(gatewayIface.OFPort)}
-	gatewayIface.MAC = gwMAC
 	gatewayIface.IPs = []net.IP{}
 	if i.networkConfig.TrafficEncapMode.IsNetworkPolicyOnly() {
 		// Assign IP to gw as required by SpoofGuard.
@@ -1146,13 +1163,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_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,72 @@ func TestSetupDefaultTunnelInterface(t *testing.T) {
 		})
 	}
 }
+
+func TestSetupGatewayInterface(t *testing.T) {
+	fakeMAC, _ := net.ParseMAC("12:34:56:78:76:54")
+	defer mockSetLinkUp(fakeMAC, 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().SetInterfaceMAC(initializer.hostGateway, fakeMAC).Return(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(returnedMAC net.HardwareAddr, returnIndex int, returnErr error) func() {
+	originalSetLinkUp := setLinkUp
+	setLinkUp = func(name string) (net.HardwareAddr, int, error) {
+		return returnedMAC, 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/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_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/ovs/ovsconfig/interfaces.go

@@ -14,6 +14,10 @@
 
 package ovsconfig
 
+import (
+	"net"
+)
+
 type TunnelType string
 
 type OVSDatapathType string
@@ -64,4 +68,5 @@ type OVSBridgeClient interface {
 	GetOVSDatapathType() OVSDatapathType
 	SetInterfaceType(name, ifType string) Error
 	SetPortExternalIDs(portName string, externalIDs map[string]interface{}) Error
+	SetInterfaceMAC(name string, mac net.HardwareAddr) Error
 }