allow primary nic for secondary bridge

If primary nic is configured with secondary bridge antrea-agent will
move primary nic configuration(IPs and Routes) to the secondary bridge,
and will revert the change on shutdown.

Signed-off-by: Daman Arora <aroradaman@gmail.com>

cmd/antrea-agent/agent.go

@@ -728,6 +728,7 @@ func run(o *Options) error {
 	}
 
 	if features.DefaultFeatureGate.Enabled(features.SecondaryNetwork) {
+		defer secondarynetwork.RestoreHostInterfaceConfiguration(&o.config.SecondaryNetwork)
 		if err := secondarynetwork.Initialize(
 			o.config.ClientConnection, o.config.KubeAPIServerOverride,
 			k8sClient, localPodInformer.Get(), nodeConfig.Name,

pkg/agent/agent_linux.go

@@ -23,7 +23,6 @@ import (
 	"net"
 	"time"
 
-	"github.com/vishvananda/netlink"
 	"k8s.io/apimachinery/pkg/util/wait"
 	"k8s.io/klog/v2"
 
@@ -38,9 +37,6 @@ var (
 	// getInterfaceByName is meant to be overridden for testing.
 	getInterfaceByName = net.InterfaceByName
 
-	// getAllIPNetsByName is meant to be overridden for testing.
-	getAllIPNetsByName = util.GetAllIPNetsByName
-
 	// setInterfaceARPAnnounce is meant to be overridden for testing.
 	setInterfaceARPAnnounce = util.EnsureARPAnnounceOnInterface
 )
@@ -70,22 +66,10 @@ func (i *Initializer) prepareOVSBridgeForK8sNode() error {
 	uplinkNetConfig := i.nodeConfig.UplinkNetConfig
 	uplinkNetConfig.Name = adapter.Name
 	uplinkNetConfig.MAC = adapter.HardwareAddr
-	uplinkIPs, err := getAllIPNetsByName(adapter.Name)
-	if err != nil {
-		return fmt.Errorf("failed to get uplink IPs: %w", err)
-	}
-	uplinkNetConfig.IPs = uplinkIPs
-	uplinkNetConfig.Index = adapter.Index
 	// Gateway and DNSServers are not configured at adapter in Linux
 	// Limitation: dynamic DNS servers will be lost after DHCP lease expired
 	uplinkNetConfig.Gateway = ""
 	uplinkNetConfig.DNSServers = ""
-	// Save routes which are configured on the uplink interface.
-	// The routes on the host will be lost when moving the network configuration of the uplink interface
-	// to the OVS bridge local interface. The saved routes will be restored on host after that.
-	if err = i.saveHostRoutes(); err != nil {
-		return err
-	}
 
 	// Set datapathID of OVS bridge.
 	// If no datapathID configured explicitly, the reconfiguration operation will change OVS bridge datapathID
@@ -140,56 +124,6 @@ func getTransportIPNetDeviceByName(ifaceName string, ovsBridgeName string) (*net
 	return util.GetIPNetDeviceByName(ifaceName)
 }
 
-// saveHostRoutes saves the routes which were configured on the uplink interface
-// before the interface is configured as the OVS brdige uplink. These routes
-// will be moved to the bridge interface together with the interface IP
-// configuration.
-func (i *Initializer) saveHostRoutes() error {
-	routes, err := netlink.RouteList(nil, netlink.FAMILY_V4)
-	if err != nil {
-		return err
-	}
-	for _, route := range routes {
-		if route.LinkIndex != i.nodeConfig.UplinkNetConfig.Index {
-			klog.V(2).Infof("Skipped host route not on uplink: %+v", route)
-			continue
-		}
-		// Skip IPv6 routes until we support IPv6 stack.
-		// TODO(gran): support IPv6
-		if route.Gw.To4() == nil {
-			klog.V(2).Infof("Skipped IPv6 host route: %+v", route)
-			continue
-		}
-		klog.Infof("Got host route=%+v", route)
-		i.nodeConfig.UplinkNetConfig.Routes = append(i.nodeConfig.UplinkNetConfig.Routes, route)
-	}
-	return nil
-}
-
-// restoreHostRoutes restores the host routes which are lost when moving the IP
-// configuration of uplink interface to the OVS bridge interface during
-// the Antrea bridge initialization stage.
-// The backup routes are restored after the IP configuration changes.
-func (i *Initializer) restoreHostRoutes() error {
-	return i.restoreHostRoutesToInterface(i.nodeConfig.UplinkNetConfig.Name)
-}
-
-func (i *Initializer) restoreHostRoutesToInterface(ifaceName string) error {
-	iface, err := net.InterfaceByName(ifaceName)
-	if err != nil {
-		return nil
-	}
-	for _, routeInterface := range i.nodeConfig.UplinkNetConfig.Routes {
-		route := routeInterface.(netlink.Route)
-		newRoute := route
-		newRoute.LinkIndex = iface.Index
-		if err := netlink.RouteReplace(&newRoute); err != nil {
-			return err
-		}
-	}
-	return nil
-}
-
 func (i *Initializer) ConnectUplinkToOVSBridge() error {
 	// Return immediately on Linux if connectUplinkToBridge is false.
 	if !i.connectUplinkToBridge {
@@ -198,18 +132,21 @@ func (i *Initializer) ConnectUplinkToOVSBridge() error {
 	klog.InfoS("Bridging uplink to OVS bridge")
 	var err error
 	uplinkNetConfig := i.nodeConfig.UplinkNetConfig
-	uplinkName := uplinkNetConfig.Name
-	bridgedUplinkName := util.GenerateUplinkInterfaceName(uplinkNetConfig.Name)
-	uplinkIPs := uplinkNetConfig.IPs
 
-	// If the uplink port already exists, just return.
-	if uplinkOFPort, err := i.ovsBridgeClient.GetOFPort(bridgedUplinkName, false); err == nil {
-		klog.InfoS("Uplink already exists, skip the configuration", "uplink", bridgedUplinkName, "port", uplinkOFPort)
-		return nil
+	externalIDs := map[string]interface{}{
+		interfacestore.AntreaInterfaceTypeKey: interfacestore.AntreaHost,
 	}
-
-	if err := util.RenameInterface(uplinkName, bridgedUplinkName); err != nil {
-		return fmt.Errorf("failed to change uplink interface name: err=%w", err)
+	bridgedUplinkName, exists, err := util.PrepareHostInterfaceConnection(
+		i.ovsBridgeClient,
+		uplinkNetConfig.Name,
+		int32(i.nodeConfig.HostInterfaceOFPort),
+		externalIDs,
+	)
+	if err != nil {
+		return err
+	}
+	if exists {
+		return nil
 	}
 
 	// Create uplink port.
@@ -226,64 +163,6 @@ func (i *Initializer) ConnectUplinkToOVSBridge() error {
 	klog.InfoS("Allocated OpenFlow port for uplink interface", "port", bridgedUplinkName, "ofPort", uplinkOFPort)
 	uplinkInterface.OVSPortConfig = &interfacestore.OVSPortConfig{uplinkPortUUID, uplinkOFPort} //nolint: govet
 	i.ifaceStore.AddInterface(uplinkInterface)
-
-	// Create local port.
-	externalIDs := map[string]interface{}{
-		interfacestore.AntreaInterfaceTypeKey: interfacestore.AntreaHost,
-	}
-	if _, err = i.ovsBridgeClient.CreateInternalPort(uplinkName, int32(i.nodeConfig.HostInterfaceOFPort), uplinkNetConfig.MAC.String(), externalIDs); err != nil {
-		return fmt.Errorf("cannot create host interface port %s: err=%w", uplinkName, err)
-	}
-
-	// Move network configuration of uplink interface to OVS bridge local interface.
-	// The net configuration of uplink will be restored by RestoreOVSBridge when shutting down.
-	wait.PollUntilContextTimeout(context.TODO(), 100*time.Millisecond, 10000*time.Millisecond, true,
-		func(ctx context.Context) (bool, error) {
-			// Wait a few seconds for OVS bridge local port.
-			link, err := netlink.LinkByName(uplinkName)
-			if err != nil {
-				klog.V(4).InfoS("OVS bridge local port is not ready", "port", uplinkName, "err", err)
-				return false, nil
-			}
-			klog.InfoS("OVS bridge local port is ready", "type", link.Type(), "attrs", link.Attrs())
-			return true, nil
-		})
-	localLink, err := netlink.LinkByName(uplinkName)
-	if err != nil {
-		return err
-	}
-	if _, _, err = util.SetLinkUp(uplinkName); err != nil {
-		return err
-	}
-
-	// Check if uplink is configured with an IPv6 address: if it is, we need to ensure that IPv6
-	// is enabled on the OVS internal port as we need to move all IP addresses over.
-	uplinkHasIPv6Address := false
-	for _, ip := range uplinkIPs {
-		if ip.IP.To4() == nil {
-			uplinkHasIPv6Address = true
-			break
-		}
-	}
-	if uplinkHasIPv6Address {
-		klog.InfoS("Uplink has IPv6 address, ensuring that IPv6 is enabled on bridge local port", "port", uplinkName)
-		if err := util.EnsureIPv6EnabledOnInterface(uplinkName); err != nil {
-			klog.ErrorS(err, "Failed to ensure that IPv6 is enabled on bridge local port, moving uplink IPs to bridge is likely to fail", "port", uplinkName)
-		}
-	}
-
-	if err = util.ConfigureLinkAddresses(localLink.Attrs().Index, uplinkIPs); err != nil {
-		return err
-	}
-	if err = util.ConfigureLinkAddresses(uplinkNetConfig.Index, nil); err != nil {
-		return err
-	}
-	// Restore the host routes which are lost when moving the network configuration of the
-	// uplink interface to OVS bridge interface.
-	if err = i.restoreHostRoutes(); err != nil {
-		return err
-	}
-
 	return nil
 }
 
@@ -294,34 +173,11 @@ func (i *Initializer) RestoreOVSBridge() {
 		return
 	}
 	klog.InfoS("Restoring bridge config to uplink...")
-	uplinkNetConfig := i.nodeConfig.UplinkNetConfig
-	uplinkName := ""
-	bridgedUplinkName := ""
-	if uplinkNetConfig != nil {
-		uplinkName = uplinkNetConfig.Name
-		bridgedUplinkName = util.GenerateUplinkInterfaceName(uplinkName)
-	}
-	brName := i.ovsBridge
 
-	if uplinkName != "" {
-		uplinkIPs := uplinkNetConfig.IPs
-		if err := util.DeleteOVSPort(brName, uplinkName); err != nil {
-			klog.ErrorS(err, "Delete OVS port failed", "port", uplinkName)
-		}
-		if err := util.DeleteOVSPort(brName, bridgedUplinkName); err != nil {
-			klog.ErrorS(err, "Delete OVS port failed", "port", bridgedUplinkName)
-		}
-		if err := util.RenameInterface(bridgedUplinkName, uplinkName); err != nil {
-			klog.ErrorS(err, "Restore uplink name failed", "uplink", bridgedUplinkName)
-		}
-		if err := util.ConfigureLinkAddresses(uplinkNetConfig.Index, uplinkIPs); err != nil {
-			klog.ErrorS(err, "Configure IP to uplink failed", "uplink", uplinkName)
-		}
-		if err := i.restoreHostRoutesToInterface(uplinkName); err != nil {
-			klog.ErrorS(err, "Configure route to uplink interface failed", "uplink", uplinkName)
-		}
+	if i.nodeConfig.UplinkNetConfig.Name != "" {
+		util.RestoreHostInterfaceConfiguration(i.ovsBridge, i.nodeConfig.UplinkNetConfig.Name)
+		klog.InfoS("Finished restoring bridge config to uplink...")
 	}
-	klog.InfoS("Finished to restore bridge config to uplink...")
 }
 
 func (i *Initializer) setInterfaceMTU(iface string, mtu int) error {

pkg/agent/agent_linux_test.go

@@ -41,14 +41,6 @@ func mockGetInterfaceByName(t *testing.T, ipDevice *net.Interface) {
 	t.Cleanup(func() { getInterfaceByName = prevGetInterfaceByName })
 }
 
-func mockGetAllIPNetsByName(t *testing.T, ips []*net.IPNet) {
-	prevGetAllIPNetsByName := getAllIPNetsByName
-	getAllIPNetsByName = func(name string) ([]*net.IPNet, error) {
-		return ips, nil
-	}
-	t.Cleanup(func() { getAllIPNetsByName = prevGetAllIPNetsByName })
-}
-
 func TestPrepareOVSBridgeForK8sNode(t *testing.T) {
 	macAddr, _ := net.ParseMAC("00:00:5e:00:53:01")
 	_, nodeIPNet, _ := net.ParseCIDR("192.168.10.10/24")
@@ -119,7 +111,6 @@ func TestPrepareOVSBridgeForK8sNode(t *testing.T) {
 			initializer.nodeConfig = nodeConfig
 			mockGetIPNetDeviceFromIP(t, nodeIPNet, ipDevice)
 			mockGetInterfaceByName(t, ipDevice)
-			mockGetAllIPNetsByName(t, []*net.IPNet{nodeIPNet})
 			if tt.expectedCalls != nil {
 				tt.expectedCalls(mockOVSBridgeClient)
 			}

pkg/agent/secondarynetwork/init.go

@@ -27,6 +27,7 @@ import (
 
 	"antrea.io/antrea/pkg/agent/interfacestore"
 	"antrea.io/antrea/pkg/agent/secondarynetwork/podwatch"
+	"antrea.io/antrea/pkg/agent/util"
 	agentconfig "antrea.io/antrea/pkg/config/agent"
 	"antrea.io/antrea/pkg/ovs/ovsconfig"
 	"antrea.io/antrea/pkg/util/channel"
@@ -48,13 +49,36 @@ func Initialize(
 	nodeName string,
 	podUpdateSubscriber channel.Subscriber,
 	stopCh <-chan struct{},
-	config *agentconfig.SecondaryNetworkConfig, ovsdb *ovsdb.OVSDB) error {
+	secNetConfig *agentconfig.SecondaryNetworkConfig, ovsdb *ovsdb.OVSDB) error {
 
-	ovsBridgeClient, err := createOVSBridge(config.OVSBridges, ovsdb)
+	ovsBridgeClient, err := createOVSBridge(secNetConfig.OVSBridges, ovsdb)
 	if err != nil {
 		return err
 	}
 
+	// We only support moving and restoring of interface configuration to OVS Bridge for the single physical interface case.
+	phyInterfaces := make([]string, len(secNetConfig.OVSBridges[0].PhysicalInterfaces))
+	copy(phyInterfaces, secNetConfig.OVSBridges[0].PhysicalInterfaces)
+	if len(phyInterfaces) == 1 {
+
+		bridgedName, _, err := util.PrepareHostInterfaceConnection(
+			ovsBridgeClient,
+			phyInterfaces[0],
+			0,
+			map[string]interface{}{
+				interfacestore.AntreaInterfaceTypeKey: interfacestore.AntreaHost,
+			},
+		)
+		if err != nil {
+			return err
+		}
+		phyInterfaces[0] = bridgedName
+	}
+
+	if err = connectPhyInterfacesToOVSBridge(ovsBridgeClient, phyInterfaces); err != nil {
+		return err
+	}
+
 	// Create the NetworkAttachmentDefinition client, which handles access to secondary network object
 	// definition from the API Server.
 	netAttachDefClient, err := createNetworkAttachDefClient(clientConnectionConfig, kubeAPIServerOverride)
@@ -74,38 +98,49 @@ func Initialize(
 	return nil
 }
 
-// TODO: check and update bridge configuration.
+// RestoreHostInterfaceConfiguration restores interface configuration from secondary-bridge back to host-interface.
+func RestoreHostInterfaceConfiguration(secNetConfig *agentconfig.SecondaryNetworkConfig) {
+	if len(secNetConfig.OVSBridges[0].PhysicalInterfaces) == 1 {
+		util.RestoreHostInterfaceConfiguration(secNetConfig.OVSBridges[0].BridgeName, secNetConfig.OVSBridges[0].PhysicalInterfaces[0])
+	}
+}
+
 func createOVSBridge(bridges []agentconfig.OVSBridgeConfig, ovsdb *ovsdb.OVSDB) (ovsconfig.OVSBridgeClient, error) {
 	if len(bridges) == 0 {
 		return nil, nil
 	}
 	// Only one OVS bridge is supported.
 	bridgeConfig := bridges[0]
-
-	for _, phyInterface := range bridgeConfig.PhysicalInterfaces {
-		if _, err := interfaceByNameFn(phyInterface); err != nil {
-			return nil, fmt.Errorf("failed to get interface %s: %v", phyInterface, err)
-		}
-	}
-
 	ovsBridgeClient := newOVSBridgeFn(bridgeConfig.BridgeName, ovsconfig.OVSDatapathSystem, ovsdb)
 	if err := ovsBridgeClient.Create(); err != nil {
 		return nil, fmt.Errorf("failed to create OVS bridge %s: %v", bridgeConfig.BridgeName, err)
 	}
 	klog.InfoS("OVS bridge created", "bridge", bridgeConfig.BridgeName)
+	return ovsBridgeClient, nil
+}
 
-	for i, phyInterface := range bridgeConfig.PhysicalInterfaces {
+func connectPhyInterfacesToOVSBridge(ovsBridgeClient ovsconfig.OVSBridgeClient, phyInterfaces []string) error {
+	for _, phyInterface := range phyInterfaces {
+		if _, err := interfaceByNameFn(phyInterface); err != nil {
+			return fmt.Errorf("failed to get interface %s: %v", phyInterface, err)
+		}
+	}
+
+	externalIDs := map[string]interface{}{
+		interfacestore.AntreaInterfaceTypeKey: interfacestore.AntreaUplink,
+	}
+	for i, phyInterface := range phyInterfaces {
 		if _, err := ovsBridgeClient.GetOFPort(phyInterface, false); err == nil {
-			klog.V(2).InfoS("Physical interface already connected to OVS bridge, skip the configuration", "device", phyInterface, "bridge", bridgeConfig.BridgeName)
+			klog.V(2).InfoS("Physical interface already connected to secondary OVS bridge, skip the configuration", "device", phyInterface)
 			continue
 		}
 
-		if _, err := ovsBridgeClient.CreateUplinkPort(phyInterface, int32(i), map[string]interface{}{interfacestore.AntreaInterfaceTypeKey: interfacestore.AntreaUplink}); err != nil {
-			return nil, fmt.Errorf("failed to create OVS uplink port %s: %v", phyInterface, err)
+		if _, err := ovsBridgeClient.CreateUplinkPort(phyInterface, int32(i), externalIDs); err != nil {
+			return fmt.Errorf("failed to create OVS uplink port %s: %v", phyInterface, err)
 		}
-		klog.InfoS("Physical interface added to OVS bridge", "device", phyInterface, "bridge", bridgeConfig.BridgeName)
+		klog.InfoS("Physical interface added to secondary OVS bridge", "device", phyInterface)
 	}
-	return ovsBridgeClient, nil
+	return nil
 }
 
 // CreateNetworkAttachDefClient creates net-attach-def client handle from the given config.