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 <[email protected]>

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(true)
+		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)
@@ -1261,7 +1267,7 @@ func (i *Initializer) setOVSDatapath() error {
 	if _, exists := otherConfig[ovsconfig.OVSOtherConfigDatapathIDKey]; exists {
 		return nil
 	}
-	randMAC := util.GenerateRandomMAC()
+	randMAC := util.GenerateRandomMAC(false)
 	datapathID := "0000" + strings.Replace(randMAC.String(), ":", "", -1)
 	if err := i.ovsBridgeClient.SetDatapathID(datapathID); err != nil {
 		klog.ErrorS(err, "Failed to set OVS bridge datapath_id", "datapathID", datapathID)

pkg/agent/agent_linux_test.go

@@ -0,0 +1,23 @@
+// 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 mockFunctions() {
+	mockUtilFunctions()
+}
+
+func restoreFunctions() {
+	restoreUtilFunctions()
+}

pkg/agent/agent_test.go

@@ -15,6 +15,7 @@
 package agent
 
 import (
+	"antrea.io/antrea/pkg/agent/util"
 	"context"
 	"fmt"
 	"net"
@@ -578,3 +579,62 @@ func TestSetupDefaultTunnelInterface(t *testing.T) {
 		})
 	}
 }
+
+func TestSetupGatewayInterface(t *testing.T) {
+	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,
+	}
+
+	mockFunctions()
+	defer restoreFunctions()
+	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 mockUtilFunctions() {
+	setLinkUp = func(name string) (net.HardwareAddr, int, error) {
+		return util.GenerateRandomMAC(true), 10, nil
+	}
+	configureLinkAddresses = func(idx int, ipNets []*net.IPNet) error {
+		return nil
+	}
+}
+
+func restoreUtilFunctions() {
+	setLinkUp = util.SetLinkUp
+	configureLinkAddresses = util.ConfigureLinkAddresses
+}

pkg/agent/agent_windows.go

@@ -34,6 +34,11 @@ import (
 	utilip "antrea.io/antrea/pkg/util/ip"
 )
 
+var (
+	// winSetInterfaceMTU is meant to be overridden for testing
+	winSetInterfaceMTU = 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 winSetInterfaceMTU(iface, mtu)
 }
 
 func (i *Initializer) setVMNodeConfig(en *v1alpha1.ExternalNode, nodeName string) error {

pkg/agent/agent_windows_test.go

@@ -0,0 +1,29 @@
+// 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
+
+import "antrea.io/antrea/pkg/agent/util"
+
+func mockFunctions() {
+	mockUtilFunctions()
+	winSetInterfaceMTU = func(ifaceName string, mtu int) error {
+		return nil
+	}
+}
+
+func restoreFunctions() {
+	restoreUtilFunctions()
+	winSetInterfaceMTU = util.SetInterfaceMTU
+}

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(false)
 	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(false))
 	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

@@ -401,13 +401,20 @@ func GenerateUplinkInterfaceName(name string) string {
 	return name + bridgedUplinkSuffix
 }
 
-func GenerateRandomMAC() net.HardwareAddr {
+func GenerateRandomMAC(global bool) net.HardwareAddr {
 	buf := make([]byte, 6)
 	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] <<= 1
+	if !global {
+		// Set the local bit
+		buf[0] |= 2
+	} else {
+		// Unset the local bit
+		buf[0] <<= 1
+	}
 	return buf
 }
 

pkg/agent/util/net_test.go

@@ -104,3 +104,19 @@ 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(false)
+	localBit, mcastBit := validateBits(mac1)
+	assert.Equal(t, uint8(1), localBit)
+	assert.Equal(t, uint8(0), mcastBit)
+	mac2 := GenerateRandomMAC(true)
+	localBit, mcastBit = validateBits(mac2)
+	assert.Equal(t, uint8(0), localBit)
+	assert.Equal(t, uint8(0), mcastBit)
+}

pkg/ovs/ovsconfig/ovs_client.go

@@ -48,6 +48,7 @@ type OVSPortData struct {
 	OFPort      int32
 	ExternalIDs map[string]string
 	Options     map[string]string
+	MAC         net.HardwareAddr
 }
 
 const (
@@ -694,6 +695,9 @@ func buildPortDataCommon(port, intf map[string]interface{}, portData *OVSPortDat
 	} else { // ofport not assigned by OVS yet
 		portData.OFPort = 0
 	}
+	if mac, err := net.ParseMAC(intf["mac_in_use"].(string)); err == nil {
+		portData.MAC = mac
+	}
 }
 
 // GetPortData retrieves port data given the OVS port UUID and interface name.
@@ -709,7 +713,7 @@ func (br *OVSBridge) GetPortData(portUUID, ifName string) (*OVSPortData, Error)
 	})
 	tx.Select(dbtransaction.Select{
 		Table:   "Interface",
-		Columns: []string{"_uuid", "type", "ofport", "options"},
+		Columns: []string{"_uuid", "type", "ofport", "options", "mac_in_use"},
 		Where:   [][]interface{}{{"name", "==", ifName}},
 	})
 
@@ -762,7 +766,7 @@ func (br *OVSBridge) GetPortList() ([]OVSPortData, Error) {
 	})
 	tx.Select(dbtransaction.Select{
 		Table:   "Interface",
-		Columns: []string{"_uuid", "type", "name", "ofport", "options"},
+		Columns: []string{"_uuid", "type", "name", "ofport", "options", "mac_in_use"},
 	})
 
 	res, err, temporary := tx.Commit()