Support Pod secondary interfaces on VLAN network

cmd/antrea-agent/agent.go

@@ -694,8 +694,6 @@ func run(o *Options) error {
 		if err := secondarynetwork.Initialize(
 			o.config.ClientConnection, o.config.KubeAPIServerOverride,
 			k8sClient, localPodInformer, nodeConfig.Name, cniPodInfoStore,
-			// safe to call given that cniServer.Initialize has been called already.
-			cniServer.GetPodConfigurator(),
 			stopCh,
 			&o.config.SecondaryNetwork, ovsdbConnection); err != nil {
 			return fmt.Errorf("failed to initialize secondary network: %v", err)

pkg/agent/cniserver/interface_configuration_linux.go

@@ -249,7 +249,10 @@ func (ic *ifConfigurator) configureContainerLinkVeth(
 	mtu int,
 	result *current.Result,
 ) error {
-	hostIfaceName := util.GenerateContainerInterfaceName(podName, podNamespace, containerID)
+	// Include the container veth interface name in the name generation, as one Pod can have more
+	// than one interfaces inc. secondary interfaces, while the host interface name must be
+	// be unique.
+	hostIfaceName := util.GenerateContainerHostVethName(podName, podNamespace, containerID, containerIfaceName)
 
 	hostIface := &current.Interface{Name: hostIfaceName}
 	containerIface := &current.Interface{Name: containerIfaceName, Sandbox: containerNetNS}

pkg/agent/cniserver/pod_configuration.go

@@ -84,9 +84,9 @@ func newPodConfigurator(
 	gatewayMAC net.HardwareAddr,
 	ovsDatapathType ovsconfig.OVSDatapathType,
 	isOvsHardwareOffloadEnabled bool,
+	disableTXChecksumOffload bool,
 	podUpdateNotifier channel.Notifier,
 	podInfoStore cnipodcache.CNIPodInfoStore,
-	disableTXChecksumOffload bool,
 ) (*podConfigurator, error) {
 	ifConfigurator, err := newInterfaceConfigurator(ovsDatapathType, isOvsHardwareOffloadEnabled, disableTXChecksumOffload)
 	if err != nil {
@@ -259,7 +259,8 @@ func (pc *podConfigurator) configureInterfaces(
 	// Note that the IP address should be advertised after Pod OpenFlow entries are installed, otherwise the packet might
 	// be dropped by OVS.
 	if err = pc.ifConfigurator.advertiseContainerAddr(containerNetNS, containerIface.Name, &result.Result); err != nil {
-		klog.Errorf("Failed to advertise IP address for container %s: %v", containerID, err)
+		// Do not return an error and fail the interface creation.
+		klog.ErrorS(err, "Failed to advertise IP address for container", "container ID", containerID)
 	}
 	// Mark the manipulation as success to cancel deferred operations.
 	success = true

pkg/agent/cniserver/pod_configuration_linux_test.go

@@ -476,7 +476,9 @@ func TestConfigureSriovSecondaryInterface(t *testing.T) {
 			name:               "advertise-failure",
 			podSriovVFDeviceID: "vf2",
 			advertiseErr:       fmt.Errorf("unable to advertise on the sriov link"),
-			expectedErr:        fmt.Errorf("failed to advertise IP address for container %s: unable to advertise on the sriov link", containerID),
+			// When advertiseContainerAddr returns an error, it is logged, but does not
+			// cause ConfigureSriovSecondaryInterface to also return an error.
+
 		}, {
 			name:               "success",
 			podSriovVFDeviceID: "vf3",
@@ -499,7 +501,7 @@ func createPodConfigurator(controller *gomock.Controller, testIfaceConfigurator
 	mockOFClient = openflowtest.NewMockClient(controller)
 	ifaceStore = interfacestore.NewInterfaceStore()
 	mockRoute = routetest.NewMockInterface(controller)
-	configurator, _ := newPodConfigurator(mockOVSBridgeClient, mockOFClient, mockRoute, ifaceStore, gwMAC, "system", false, channel.NewSubscribableChannel("PodUpdate", 100), nil, false)
+	configurator, _ := newPodConfigurator(mockOVSBridgeClient, mockOFClient, mockRoute, ifaceStore, gwMAC, "system", false, false, channel.NewSubscribableChannel("PodUpdate", 100), nil)
 	configurator.ifConfigurator = testIfaceConfigurator
 	return configurator
 }

pkg/agent/cniserver/secondary.go

@@ -0,0 +1,103 @@
+// Copyright 2021 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 cniserver
+
+import (
+	"fmt"
+
+	current "github.com/containernetworking/cni/pkg/types/100"
+	"k8s.io/klog/v2"
+
+	"antrea.io/antrea/pkg/ovs/ovsconfig"
+)
+
+func NewSecondaryInterfaceConfigurator(ovsBridgeClient ovsconfig.OVSBridgeClient) (*podConfigurator, error) {
+	return newPodConfigurator(ovsBridgeClient, nil, nil, nil, nil, ovsconfig.OVSDatapathSystem, false, false, nil, nil)
+}
+
+// ConfigureSriovSecondaryInterface configures a SR-IOV secondary interface for a Pod.
+func (pc *podConfigurator) ConfigureSriovSecondaryInterface(
+	podName, podNamespace string,
+	containerID, containerNetNS, containerInterfaceName string,
+	mtu int,
+	podSriovVFDeviceID string,
+	result *current.Result) error {
+	if podSriovVFDeviceID == "" {
+		return fmt.Errorf("error getting the Pod SR-IOV VF device ID")
+	}
+
+	err := pc.ifConfigurator.configureContainerLink(podName, podNamespace, containerID, containerNetNS, containerInterfaceName, mtu, "", podSriovVFDeviceID, result, nil)
+	if err != nil {
+		return err
+	}
+	hostIface := result.Interfaces[0]
+	containerIface := result.Interfaces[1]
+	klog.InfoS("Configured SR-IOV interface", "Pod", klog.KRef(podNamespace, podName), "interface", containerInterfaceName, "hostInterface", hostIface)
+
+	if err = pc.ifConfigurator.advertiseContainerAddr(containerNetNS, containerIface.Name, result); err != nil {
+		klog.ErrorS(err, "Failed to advertise IP address for SR-IOV interface", "container ID", containerID, "interface", containerInterfaceName)
+	}
+	return nil
+}
+
+// ConfigureVLANSecondaryInterface configures a VLAN secondary interface on the secondary network
+// OVS bridge, and returns the OVS port UUID.
+func (pc *podConfigurator) ConfigureVLANSecondaryInterface(
+	podName, podNamespace string,
+	containerID, containerNetNS, containerInterfaceName string,
+	mtu int, vlanID uint16,
+	result *current.Result) (string, error) {
+	// TODO: revisit the possibility of reusing configureInterfaces(), connectInterfaceToOVS()
+	// removeInterfaces() code, and using InterfaceStore to store secondary interface info.
+	if err := pc.ifConfigurator.configureContainerLink(podName, podNamespace, containerID, containerNetNS, containerInterfaceName, mtu, "", "", result, nil); err != nil {
+		return "", err
+	}
+	hostIface := result.Interfaces[0]
+	containerIface := result.Interfaces[1]
+
+	success := false
+	defer func() {
+		if !success {
+			if err := pc.ifConfigurator.removeContainerLink(containerID, hostIface.Name); err != nil {
+				klog.ErrorS(err, "failed to roll back veth creation", "container ID", containerID, "interface", containerInterfaceName)
+			}
+		}
+	}()
+
+	// Use the outer veth interface name as the OVS port name.
+	ovsPortName := hostIface.Name
+	ovsPortUUID, err := pc.createOVSPort(ovsPortName, nil, vlanID)
+	if err != nil {
+		return "", fmt.Errorf("failed to add OVS port for container %s: %v", containerID, err)
+	}
+	klog.InfoS("Configured VLAN interface", "Pod", klog.KRef(podNamespace, podName), "interface", containerInterfaceName, "hostInterface", hostIface)
+
+	if err := pc.ifConfigurator.advertiseContainerAddr(containerNetNS, containerIface.Name, result); err != nil {
+		klog.ErrorS(err, "Failed to advertise IP address for VLAN interface", "container ID", containerID, "interface", containerInterfaceName)
+	}
+	success = true
+	return ovsPortUUID, nil
+}
+
+// DeleteVLANSecondaryInterface deletes a VLAN secondary interface.
+func (pc *podConfigurator) DeleteVLANSecondaryInterface(containerID, hostInterfaceName, ovsPortUUID string) error {
+	if err := pc.ovsBridgeClient.DeletePort(ovsPortUUID); err != nil {
+		return fmt.Errorf("failed to delete OVS port for container %s: %v", containerID, err)
+	}
+	if err := pc.ifConfigurator.removeContainerLink(containerID, hostInterfaceName); err != nil {
+		return err
+	}
+	return nil
+}

pkg/agent/cniserver/server.go

@@ -387,10 +387,6 @@ func (s *CNIServer) validatePrevResult(cfgArgs *cnipb.CniCmdArgs, prevResult *cu
 	return nil
 }
 
-func (s *CNIServer) GetPodConfigurator() *podConfigurator {
-	return s.podConfigurator
-}
-
 // Declared variables for testing
 var (
 	ipamSecondaryNetworkAdd   = ipam.SecondaryNetworkAdd
@@ -531,8 +527,7 @@ func (s *CNIServer) CmdAdd(ctx context.Context, request *cnipb.CniCmdRequest) (*
 	if s.secondaryNetworkEnabled {
 		// Go cache the CNI server info at CNIConfigInfo cache, for podWatch usage
 		cniInfo := &cnipodcache.CNIConfigInfo{CNIVersion: cniVersion, PodName: podName, PodNamespace: podNamespace,
-			ContainerID: cniConfig.ContainerId, ContainerNetNS: netNS, PodCNIDeleted: false,
-			MTU: cniConfig.MTU}
+			ContainerID: cniConfig.ContainerId, ContainerNetNS: netNS, PodCNIDeleted: false}
 		s.podConfigurator.podInfoStore.AddCNIConfigInfo(cniInfo)
 	}
 
@@ -668,9 +663,9 @@ func (s *CNIServer) Initialize(
 
 	s.podConfigurator, err = newPodConfigurator(
 		ovsBridgeClient, ofClient, s.routeClient, ifaceStore, s.nodeConfig.GatewayConfig.MAC,
-		ovsBridgeClient.GetOVSDatapathType(), ovsBridgeClient.IsHardwareOffloadEnabled(), podUpdateNotifier,
-		podInfoStore, s.disableTXChecksumOffload,
-	)
+		ovsBridgeClient.GetOVSDatapathType(), ovsBridgeClient.IsHardwareOffloadEnabled(),
+		s.disableTXChecksumOffload,
+		podUpdateNotifier, podInfoStore)
 	if err != nil {
 		return fmt.Errorf("error during initialize podConfigurator: %v", err)
 	}

pkg/agent/cniserver/server_linux_test.go

@@ -98,7 +98,7 @@ func TestValidatePrevResult(t *testing.T) {
 		cniConfig.Ifname = ifname
 		cniConfig.Netns = "invalid_netns"
 		sriovVFDeviceID := ""
-		cniServer.podConfigurator, _ = newPodConfigurator(nil, nil, nil, nil, nil, "", false, channel.NewSubscribableChannel("PodUpdate", 100), nil, false)
+		cniServer.podConfigurator, _ = newPodConfigurator(nil, nil, nil, nil, nil, "", false, false, channel.NewSubscribableChannel("PodUpdate", 100), nil)
 		response := cniServer.validatePrevResult(cniConfig.CniCmdArgs, prevResult, sriovVFDeviceID)
 		checkErrorResponse(t, response, cnipb.ErrorCode_CHECK_INTERFACE_FAILURE, "")
 	})
@@ -109,7 +109,7 @@ func TestValidatePrevResult(t *testing.T) {
 		cniConfig.Netns = "invalid_netns"
 		sriovVFDeviceID := "0000:03:00.6"
 		prevResult.Interfaces = []*current.Interface{hostIface, containerIface}
-		cniServer.podConfigurator, _ = newPodConfigurator(nil, nil, nil, nil, nil, "", true, channel.NewSubscribableChannel("PodUpdate", 100), nil, false)
+		cniServer.podConfigurator, _ = newPodConfigurator(nil, nil, nil, nil, nil, "", true, false, channel.NewSubscribableChannel("PodUpdate", 100), nil)
 		response := cniServer.validatePrevResult(cniConfig.CniCmdArgs, prevResult, sriovVFDeviceID)
 		checkErrorResponse(t, response, cnipb.ErrorCode_CHECK_INTERFACE_FAILURE, "")
 	})
@@ -122,7 +122,7 @@ func TestRemoveInterface(t *testing.T) {
 	ifaceStore = interfacestore.NewInterfaceStore()
 	mockRoute = routetest.NewMockInterface(controller)
 	gwMAC, _ := net.ParseMAC("00:00:11:11:11:11")
-	podConfigurator, err := newPodConfigurator(mockOVSBridgeClient, mockOFClient, mockRoute, ifaceStore, gwMAC, "system", false, channel.NewSubscribableChannel("PodUpdate", 100), nil, false)
+	podConfigurator, err := newPodConfigurator(mockOVSBridgeClient, mockOFClient, mockRoute, ifaceStore, gwMAC, "system", false, false, channel.NewSubscribableChannel("PodUpdate", 100), nil)
 	require.Nil(t, err, "No error expected in podConfigurator constructor")
 
 	containerMAC, _ := net.ParseMAC("aa:bb:cc:dd:ee:ff")
@@ -203,7 +203,7 @@ func newMockCNIServer(t *testing.T, controller *gomock.Controller, ipamDriver ip
 	gwMAC, _ := net.ParseMAC("00:00:11:11:11:11")
 	gateway := &config.GatewayConfig{Name: "", IPv4: gwIPv4, MAC: gwMAC}
 	cniServer.nodeConfig = &config.NodeConfig{Name: "node1", PodIPv4CIDR: nodePodCIDRv4, GatewayConfig: gateway}
-	cniServer.podConfigurator, _ = newPodConfigurator(mockOVSBridgeClient, mockOFClient, mockRoute, ifaceStore, gwMAC, "system", false, channel.NewSubscribableChannel("PodUpdate", 100), nil, false)
+	cniServer.podConfigurator, _ = newPodConfigurator(mockOVSBridgeClient, mockOFClient, mockRoute, ifaceStore, gwMAC, "system", false, false, channel.NewSubscribableChannel("PodUpdate", 100), nil)
 	cniServer.enableSecondaryNetworkIPAM = enableSecondaryNetworkIPAM
 	cniServer.isChaining = isChaining
 	cniServer.secondaryNetworkEnabled = secondaryNetworkEnabled
@@ -494,8 +494,7 @@ func TestCmdDel(t *testing.T) {
 			cniserver.podConfigurator.ifConfigurator = testIfaceConfigurator
 			if tc.secondaryNetworkEnabled {
 				cniInfo := &cnipodcache.CNIConfigInfo{CNIVersion: supportedCNIVersion, PodName: tc.podName, PodNamespace: testPodNamespace,
-					ContainerID: containerID, ContainerNetNS: netns, PodCNIDeleted: false,
-					MTU: 1450}
+					ContainerID: containerID, ContainerNetNS: netns, PodCNIDeleted: false}
 				cniserver.podConfigurator.podInfoStore.AddCNIConfigInfo(cniInfo)
 			}
 			if tc.ipamDel {
@@ -639,7 +638,7 @@ func TestReconcile(t *testing.T) {
 	cniServer := newCNIServer(t)
 	cniServer.routeClient = mockRoute
 	gwMAC, _ := net.ParseMAC("00:00:11:11:11:11")
-	cniServer.podConfigurator, _ = newPodConfigurator(mockOVSBridgeClient, mockOFClient, mockRoute, ifaceStore, gwMAC, "system", false, channel.NewSubscribableChannel("PodUpdate", 100), nil, false)
+	cniServer.podConfigurator, _ = newPodConfigurator(mockOVSBridgeClient, mockOFClient, mockRoute, ifaceStore, gwMAC, "system", false, false, channel.NewSubscribableChannel("PodUpdate", 100), nil)
 	cniServer.podConfigurator.ifConfigurator = newTestInterfaceConfigurator()
 	cniServer.nodeConfig = &config.NodeConfig{
 		Name: nodeName,

pkg/agent/cniserver/server_windows_test.go

@@ -293,7 +293,7 @@ func newMockCNIServer(t *testing.T, controller *gomock.Controller, podUpdateNoti
 	gwMAC, _ := net.ParseMAC("00:00:11:11:11:11")
 	gateway := &config.GatewayConfig{Name: "", IPv4: gwIPv4, MAC: gwMAC}
 	cniServer.nodeConfig = &config.NodeConfig{Name: "node1", PodIPv4CIDR: nodePodCIDRv4, GatewayConfig: gateway}
-	cniServer.podConfigurator, _ = newPodConfigurator(mockOVSBridgeClient, mockOFClient, mockRoute, ifaceStore, gwMAC, "system", false, podUpdateNotifier, nil, false)
+	cniServer.podConfigurator, _ = newPodConfigurator(mockOVSBridgeClient, mockOFClient, mockRoute, ifaceStore, gwMAC, "system", false, false, podUpdateNotifier, nil)
 	return cniServer
 }
 
@@ -947,7 +947,7 @@ func TestReconcile(t *testing.T) {
 	pod4IfaceName := "iface4"
 	pod4Iface := containerIfaces["iface4"]
 	waiter := newAsyncWaiter(pod4Iface.PodName, pod4Iface.ContainerID)
-	cniServer.podConfigurator, _ = newPodConfigurator(mockOVSBridgeClient, mockOFClient, mockRoute, ifaceStore, gwMAC, "system", false, waiter.notifier, nil, false)
+	cniServer.podConfigurator, _ = newPodConfigurator(mockOVSBridgeClient, mockOFClient, mockRoute, ifaceStore, gwMAC, "system", false, false, waiter.notifier, nil)
 	cniServer.nodeConfig = &config.NodeConfig{Name: nodeName}
 
 	// Re-install Pod1 flows