Enable IPv6 on OVS internal port if needed in bridging mode (#5409)

The uplink interface may have an IPv6 address, while the interface
created by OVS for the internal port may not support IPv6. For example,
such a situation has been observed in a Kind cluster, with IPv6 enabled
on the uplink but disabled by default on new interfaces:

```
root@kind-worker:/# sysctl net.ipv6.conf.all.disable_ipv6
net.ipv6.conf.all.disable_ipv6 = 1
root@kind-worker:/# sysctl net.ipv6.conf.default.disable_ipv6
net.ipv6.conf.default.disable_ipv6 = 1
root@kind-worker:/# sysctl net.ipv6.conf.eth0.disable_ipv6
net.ipv6.conf.eth0.disable_ipv6 = 0
```

When we detect that uplink addresses include an IPv6 address, we will
now ensure that IPv6 is enabled on the bridge port (using sysctl),
before attempting to move the addresses over. If it fails, we will
proceed with the rest of the initialization, but moving the IP addresses
to the bridge is very likely to be unsuccessful in that case.

We also make bridge cleanup more robust, by saving all uplink IP
addresses in the uplink config, and using the saved values to restore
the uplink interface. This ensures that cleanup can succeed, even if
bridge configuration failed half-way, as was the case in #5368.

Fixes #5368

Signed-off-by: Antonin Bas <abas@vmware.com>

pkg/agent/agent_linux.go

@@ -36,6 +36,9 @@ import (
 var (
 	// getInterfaceByName is meant to be overridden for testing.
 	getInterfaceByName = net.InterfaceByName
+
+	// getAllIPNetsByName is meant to be overridden for testing.
+	getAllIPNetsByName = util.GetAllIPNetsByName
 )
 
 // prepareHostNetwork returns immediately on Linux.
@@ -58,7 +61,11 @@ func (i *Initializer) prepareOVSBridgeForK8sNode() error {
 	uplinkNetConfig := i.nodeConfig.UplinkNetConfig
 	uplinkNetConfig.Name = adapter.Name
 	uplinkNetConfig.MAC = adapter.HardwareAddr
-	uplinkNetConfig.IPs = []*net.IPNet{i.nodeConfig.NodeIPv4Addr}
+	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
@@ -138,7 +145,7 @@ func (i *Initializer) saveHostRoutes() error {
 			klog.V(2).Infof("Skipped host route not on uplink: %+v", route)
 			continue
 		}
-		// Skip IPv6 routes before we support IPv6 stack.
+		// 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)
@@ -179,21 +186,19 @@ func (i *Initializer) ConnectUplinkToOVSBridge() error {
 	if !i.connectUplinkToBridge {
 		return nil
 	}
-	klog.Infof("Bridging uplink to OVS bridge")
+	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
 	}
-	uplinkIPs, err := util.GetAllIPNetsByName(uplinkName)
-	if err != nil {
-		return fmt.Errorf("failed to get uplink IPs: err=%w", err)
-	}
+
 	if err := util.RenameInterface(uplinkName, bridgedUplinkName); err != nil {
 		return fmt.Errorf("failed to change uplink interface name: err=%w", err)
 	}
@@ -240,13 +245,31 @@ func (i *Initializer) ConnectUplinkToOVSBridge() error {
 	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.
+	// 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
 	}
@@ -260,7 +283,7 @@ func (i *Initializer) RestoreOVSBridge() {
 	if !i.connectUplinkToBridge {
 		return
 	}
-	klog.Infof("Restoring bridge config to uplink...")
+	klog.InfoS("Restoring bridge config to uplink...")
 	uplinkNetConfig := i.nodeConfig.UplinkNetConfig
 	uplinkName := ""
 	bridgedUplinkName := ""
@@ -271,10 +294,7 @@ func (i *Initializer) RestoreOVSBridge() {
 	brName := i.ovsBridge
 
 	if uplinkName != "" {
-		uplinkIPs, err := util.GetAllIPNetsByName(uplinkName)
-		if err != nil {
-			klog.ErrorS(err, "Failed to get uplink IPs")
-		}
+		uplinkIPs := uplinkNetConfig.IPs
 		if err := util.DeleteOVSPort(brName, uplinkName); err != nil {
 			klog.ErrorS(err, "Delete OVS port failed", "port", uplinkName)
 		}
@@ -291,7 +311,7 @@ func (i *Initializer) RestoreOVSBridge() {
 			klog.ErrorS(err, "Configure route to uplink interface failed", "uplink", uplinkName)
 		}
 	}
-	klog.Infof("Finished to restore 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

@@ -30,16 +30,23 @@ import (
 	ovsconfigtest "antrea.io/antrea/pkg/ovs/ovsconfig/testing"
 )
 
-func mockSetInterfaceMTU(returnErr error) func() {
-	return func() {}
+func mockSetInterfaceMTU(t *testing.T, returnErr error) {
 }
 
-func mockGetInterfaceByName(ipDevice *net.Interface) func() {
+func mockGetInterfaceByName(t *testing.T, ipDevice *net.Interface) {
 	prevGetInterfaceByName := getInterfaceByName
 	getInterfaceByName = func(name string) (*net.Interface, error) {
 		return ipDevice, nil
 	}
-	return func() { getInterfaceByName = prevGetInterfaceByName }
+	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) {
@@ -110,8 +117,9 @@ func TestPrepareOVSBridgeForK8sNode(t *testing.T) {
 			initializer.nodeType = config.K8sNode
 			initializer.connectUplinkToBridge = tt.connectUplinkToBridge
 			initializer.nodeConfig = nodeConfig
-			defer mockGetIPNetDeviceFromIP(nodeIPNet, ipDevice)()
-			defer mockGetInterfaceByName(ipDevice)()
+			mockGetIPNetDeviceFromIP(t, nodeIPNet, ipDevice)
+			mockGetInterfaceByName(t, ipDevice)
+			mockGetAllIPNetsByName(t, []*net.IPNet{nodeIPNet})
 			if tt.expectedCalls != nil {
 				tt.expectedCalls(mockOVSBridgeClient)
 			}

pkg/agent/agent_test.go

@@ -443,17 +443,17 @@ func TestInitK8sNodeLocalConfig(t *testing.T) {
 				expectedNodeConfig.NodeTransportInterfaceName = tt.transportInterface.iface.Name
 				expectedNodeConfig.NodeTransportIPv4Addr = tt.transportInterface.ipV4Net
 				expectedNodeConfig.NodeTransportIPv6Addr = tt.transportInterface.ipV6Net
-				defer mockGetTransportIPNetDeviceByName(tt.transportInterface.ipV4Net, tt.transportInterface.ipV6Net, tt.transportInterface.iface)()
+				mockGetTransportIPNetDeviceByName(t, tt.transportInterface.ipV4Net, tt.transportInterface.ipV6Net, tt.transportInterface.iface)
 			} else if len(tt.transportIfCIDRs) > 0 {
 				initializer.networkConfig.TransportIfaceCIDRs = tt.transportIfCIDRs
 				expectedNodeConfig.NodeTransportInterfaceName = tt.transportInterface.iface.Name
 				expectedNodeConfig.NodeTransportIPv4Addr = tt.transportInterface.ipV4Net
 				expectedNodeConfig.NodeTransportIPv6Addr = tt.transportInterface.ipV6Net
-				defer mockGetIPNetDeviceByCIDRs(tt.transportInterface.ipV4Net, tt.transportInterface.ipV6Net, tt.transportInterface.iface)()
+				mockGetIPNetDeviceByCIDRs(t, tt.transportInterface.ipV4Net, tt.transportInterface.ipV6Net, tt.transportInterface.iface)
 			}
-			defer mockGetIPNetDeviceFromIP(nodeIPNet, ipDevice)()
-			defer mockNodeNameEnv(nodeName)()
-			defer mockGetNodeTimeout(100 * time.Millisecond)
+			mockGetIPNetDeviceFromIP(t, nodeIPNet, ipDevice)
+			mockNodeNameEnv(t, nodeName)
+			mockGetNodeTimeout(t, 100*time.Millisecond)
 
 			err := initializer.initK8sNodeLocalConfig(nodeName)
 			if tt.expectedErr == "" {
@@ -469,39 +469,39 @@ func TestInitK8sNodeLocalConfig(t *testing.T) {
 	}
 }
 
-func mockGetIPNetDeviceFromIP(ipNet *net.IPNet, ipDevice *net.Interface) func() {
+func mockGetIPNetDeviceFromIP(t *testing.T, ipNet *net.IPNet, ipDevice *net.Interface) {
 	prevGetIPNetDeviceFromIP := getIPNetDeviceFromIP
 	getIPNetDeviceFromIP = func(localIP *ip.DualStackIPs, ignoredHostInterfaces sets.Set[string]) (*net.IPNet, *net.IPNet, *net.Interface, error) {
 		return ipNet, nil, ipDevice, nil
 	}
-	return func() { getIPNetDeviceFromIP = prevGetIPNetDeviceFromIP }
+	t.Cleanup(func() { getIPNetDeviceFromIP = prevGetIPNetDeviceFromIP })
 }
 
-func mockNodeNameEnv(name string) func() {
+func mockNodeNameEnv(t *testing.T, name string) {
 	_ = os.Setenv(env.NodeNameEnvKey, name)
-	return func() { os.Unsetenv(env.NodeNameEnvKey) }
+	t.Cleanup(func() { os.Unsetenv(env.NodeNameEnvKey) })
 }
 
-func mockGetNodeTimeout(timeout time.Duration) func() {
+func mockGetNodeTimeout(t *testing.T, timeout time.Duration) {
 	prevTimeout := getNodeTimeout
 	getNodeTimeout = timeout
-	return func() { getNodeTimeout = prevTimeout }
+	t.Cleanup(func() { getNodeTimeout = prevTimeout })
 }
 
-func mockGetTransportIPNetDeviceByName(ipV4Net, ipV6Net *net.IPNet, ipDevice *net.Interface) func() {
+func mockGetTransportIPNetDeviceByName(t *testing.T, ipV4Net, ipV6Net *net.IPNet, ipDevice *net.Interface) {
 	prevGetIPNetDeviceByName := getTransportIPNetDeviceByNameFn
 	getTransportIPNetDeviceByNameFn = func(ifName, brName string) (*net.IPNet, *net.IPNet, *net.Interface, error) {
 		return ipV4Net, ipV6Net, ipDevice, nil
 	}
-	return func() { getTransportIPNetDeviceByNameFn = prevGetIPNetDeviceByName }
+	t.Cleanup(func() { getTransportIPNetDeviceByNameFn = prevGetIPNetDeviceByName })
 }
 
-func mockGetIPNetDeviceByCIDRs(ipV4Net, ipV6Net *net.IPNet, ipDevice *net.Interface) func() {
+func mockGetIPNetDeviceByCIDRs(t *testing.T, ipV4Net, ipV6Net *net.IPNet, ipDevice *net.Interface) {
 	prevGetIPNetDeviceByCIDRs := getIPNetDeviceByCIDRs
 	getIPNetDeviceByCIDRs = func(cidr []string) (*net.IPNet, *net.IPNet, *net.Interface, error) {
 		return ipV4Net, ipV6Net, ipDevice, nil
 	}
-	return func() { getIPNetDeviceByCIDRs = prevGetIPNetDeviceByCIDRs }
+	t.Cleanup(func() { getIPNetDeviceByCIDRs = prevGetIPNetDeviceByCIDRs })
 }
 
 func TestSetupDefaultTunnelInterface(t *testing.T) {
@@ -632,9 +632,9 @@ 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)()
+	mockSetLinkUp(t, fakeMAC, 10, nil)
+	mockConfigureLinkAddress(t, nil)
+	mockSetInterfaceMTU(t, nil)
 
 	controller := mock.NewController(t)
 
@@ -678,24 +678,20 @@ func TestSetupGatewayInterface(t *testing.T) {
 	assert.NoError(t, err)
 }
 
-func mockSetLinkUp(returnedMAC net.HardwareAddr, returnIndex int, returnErr error) func() {
+func mockSetLinkUp(t *testing.T, returnedMAC net.HardwareAddr, returnIndex int, returnErr error) {
 	originalSetLinkUp := setLinkUp
 	setLinkUp = func(name string) (net.HardwareAddr, int, error) {
 		return returnedMAC, returnIndex, returnErr
 	}
-	return func() {
-		setLinkUp = originalSetLinkUp
-	}
+	t.Cleanup(func() { setLinkUp = originalSetLinkUp })
 }
 
-func mockConfigureLinkAddress(returnedErr error) func() {
+func mockConfigureLinkAddress(t *testing.T, returnedErr error) {
 	originalConfigureLinkAddresses := configureLinkAddresses
 	configureLinkAddresses = func(idx int, ipNets []*net.IPNet) error {
 		return returnedErr
 	}
-	return func() {
-		configureLinkAddresses = originalConfigureLinkAddresses
-	}
+	t.Cleanup(func() { configureLinkAddresses = originalConfigureLinkAddresses })
 }
 
 func TestRestorePortConfigs(t *testing.T) {
@@ -819,9 +815,9 @@ func TestSetOVSDatapath(t *testing.T) {
 	}
 }
 
-func mockIPsecPSKEnv(name string) func() {
+func mockIPsecPSKEnv(t *testing.T, name string) {
 	os.Setenv(ipsecPSKEnvKey, name)
-	return func() { os.Unsetenv(ipsecPSKEnvKey) }
+	t.Cleanup(func() { os.Unsetenv(ipsecPSKEnvKey) })
 }
 
 func TestReadIPSecPSK(t *testing.T) {
@@ -848,7 +844,7 @@ func TestReadIPSecPSK(t *testing.T) {
 				},
 			}
 			if tt.isIPsecPSK {
-				defer mockIPsecPSKEnv("key")()
+				mockIPsecPSKEnv(t, "key")
 			}
 
 			err := initializer.readIPSecPSK()
@@ -960,7 +956,7 @@ func TestInitVMLocalConfig(t *testing.T) {
 				externalNodeNamespace: "external",
 			}
 			close(stopCh)
-			defer mockGetIPNetDeviceFromIP(nil, ipDevice)()
+			mockGetIPNetDeviceFromIP(t, nil, ipDevice)
 			err := initializer.initVMLocalConfig(tt.nodeName)
 			if tt.expectedErr != "" {
 				assert.ErrorContains(t, err, tt.expectedErr)

pkg/agent/agent_windows_test.go

@@ -14,12 +14,14 @@
 
 package agent
 
-func mockSetInterfaceMTU(returnErr error) func() {
+import (
+	"testing"
+)
+
+func mockSetInterfaceMTU(t *testing.T, returnErr error) {
 	originalSetInterfaceMTU := setInterfaceMTU
 	setInterfaceMTU = func(ifaceName string, mtu int) error {
 		return returnErr
 	}
-	return func() {
-		setInterfaceMTU = originalSetInterfaceMTU
-	}
+	t.Cleanup(func() { setInterfaceMTU = originalSetInterfaceMTU })
 }

pkg/agent/util/net_linux.go

@@ -33,6 +33,7 @@ import (
 	"k8s.io/klog/v2"
 
 	utilnetlink "antrea.io/antrea/pkg/agent/util/netlink"
+	"antrea.io/antrea/pkg/agent/util/sysctl"
 )
 
 var (
@@ -331,6 +332,11 @@ func ConfigureLinkRoutes(link netlink.Link, routes []interface{}) error {
 	return nil
 }
 
+func EnsureIPv6EnabledOnInterface(ifaceName string) error {
+	path := fmt.Sprintf("ipv6/conf/%s/disable_ipv6", ifaceName)
+	return sysctl.EnsureSysctlNetValue(path, 0)
+}
+
 func getRoutesOnInterface(linkIndex int) ([]interface{}, error) {
 	link, err := netlinkUtil.LinkByIndex(linkIndex)
 	if err != nil {

pkg/agent/util/sysctl/sysctl_linux.go

@@ -55,13 +55,13 @@ func EnsureSysctlNetValue(sysctl string, value int) error {
 	val, err := GetSysctlNet(sysctl)
 	if err != nil {
 		// If permission error, please provide access to sysctl setting
-		klog.Errorf("Error when getting %s: %v", sysctl, err)
+		klog.ErrorS(err, "Error when getting sysctl parameter", "path", sysctl)
 		return err
 	} else if val != value {
 		err = SetSysctlNet(sysctl, value)
 		if err != nil {
 			// If permission error, please provide access to sysctl setting
-			klog.Errorf("Error when setting %s: %v", sysctl, err)
+			klog.ErrorS(err, "Error when setting sysctl parameter", "path", sysctl, "value", value)
 			return err
 		}
 	}