Automated cherry pick of #4428: Bugfix: Pod or gateway interface use a different MAC from the

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()
@@ -443,7 +448,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
 }