Always enable global IPv6 forwarding

Global forwarding works differently for IPv6:
  conf/all/forwarding - BOOLEAN
   Enable global IPv6 forwarding between all interfaces.
	  IPv4 and IPv6 work differently here; e.g. netfilter must be used
	  to control which interfaces may forward packets and which not.
https://www.kernel.org/doc/Documentation/networking/ip-sysctl.txt

It is not possible to configure the IPv6 forwarding per interface by
setting the net.ipv6.conf.<ifname>.forwarding sysctl. Instead,
the opposite approach is required where the global forwarding
is enabled and an iptables rule is added to restrict it by default.

Signed-off-by: Patryk Diak <pdiak@redhat.com>

go-controller/pkg/node/default_node_network_controller.go

@@ -11,6 +11,7 @@ import (
 	"sync"
 	"time"
 
+	nodeipt "github.com/ovn-org/ovn-kubernetes/go-controller/pkg/node/iptables"
 	kapi "k8s.io/api/core/v1"
 	discovery "k8s.io/api/discovery/v1"
 	kerrors "k8s.io/apimachinery/pkg/api/errors"
@@ -20,6 +21,7 @@ import (
 	"k8s.io/klog/v2"
 	utilnet "k8s.io/utils/net"
 
+	"github.com/containernetworking/plugins/pkg/ip"
 	v1 "github.com/k8snetworkplumbingwg/network-attachment-definition-client/pkg/apis/k8s.cni.cncf.io/v1"
 	honode "github.com/ovn-org/ovn-kubernetes/go-controller/hybrid-overlay/pkg/controller"
 	"github.com/ovn-org/ovn-kubernetes/go-controller/pkg/cni"
@@ -40,7 +42,6 @@ import (
 	"github.com/ovn-org/ovn-kubernetes/go-controller/pkg/util"
 	utilerrors "github.com/ovn-org/ovn-kubernetes/go-controller/pkg/util/errors"
 
-	"github.com/containernetworking/plugins/pkg/ip"
 	"github.com/vishvananda/netlink"
 )
 
@@ -704,6 +705,10 @@ func (nc *DefaultNodeNetworkController) Start(ctx context.Context) error {
 		nc.routeManager.Run(nc.stopChan, 4*time.Minute)
 	}()
 
+	if err = configureGlobalForwarding(); err != nil {
+		return err
+	}
+
 	// Bootstrap flows in OVS if just normal flow is present
 	if err := bootstrapOVSFlows(); err != nil {
 		return fmt.Errorf("failed to bootstrap OVS flows: %w", err)
@@ -1330,3 +1335,50 @@ func DummyNextHopIPs() []net.IP {
 	}
 	return nextHops
 }
+
+// configureGlobalForwarding configures the global forwarding settings.
+// It adds/removes the `-A FORWARD -j DROP` rule based on config.Gateway.DisableForwarding for all enabled IP families.
+// For IPv6 it additionally always enables the global forwarding.
+func configureGlobalForwarding() error {
+	// Global forwarding works differently for IPv6:
+	//   conf/all/forwarding - BOOLEAN
+	//    Enable global IPv6 forwarding between all interfaces.
+	//	  IPv4 and IPv6 work differently here; e.g. netfilter must be used
+	//	  to control which interfaces may forward packets and which not.
+	// https://www.kernel.org/doc/Documentation/networking/ip-sysctl.txt
+	//
+	// It is not possible to configure the IPv6 forwarding per interface by
+	// setting the net.ipv6.conf.<ifname>.forwarding sysctl. Instead,
+	// the opposite approach is required where the global forwarding
+	// is enabled and an iptables rule is added to restrict it by default.
+	if config.IPv6Mode {
+		if err := ip.EnableIP6Forward(); err != nil {
+			return fmt.Errorf("could not set the correct global forwarding value for ipv6:  %w", err)
+		}
+
+	}
+
+	for _, proto := range clusterIPTablesProtocols() {
+		forwardDropRule := nodeipt.Rule{
+			Table: "filter",
+			Chain: "FORWARD",
+			Args: []string{
+				"-j", "DROP",
+			},
+			Protocol: proto,
+		}
+
+		if config.Gateway.DisableForwarding {
+			// Add the default drop rule to FORWARD
+			if err := nodeipt.AddRules([]nodeipt.Rule{forwardDropRule}, true); err != nil {
+				return fmt.Errorf("failed to add the forwarding block rule: err %w", err)
+			}
+		} else {
+			// Remove the default drop rule from FORWARD
+			if err := nodeipt.DelRules([]nodeipt.Rule{forwardDropRule}); err != nil {
+				return fmt.Errorf("failed to remove the forwarding block rule: err %w", err)
+			}
+		}
+	}
+	return nil
+}

go-controller/pkg/node/gateway_init.go

@@ -21,23 +21,12 @@ import (
 // bridgedGatewayNodeSetup enables forwarding on bridge interface, sets up the physical network name mappings for the bridge,
 // and returns an ifaceID created from the bridge name and the node name
 func bridgedGatewayNodeSetup(nodeName, bridgeName, physicalNetworkName string) (string, error) {
-	// enable forwarding on bridge interface always
-	createForwardingRule := func(family string) error {
-		stdout, stderr, err := util.RunSysctl("-w", fmt.Sprintf("net.%s.conf.%s.forwarding=1", family, bridgeName))
-		if err != nil || stdout != fmt.Sprintf("net.%s.conf.%s.forwarding = 1", family, bridgeName) {
-			return fmt.Errorf("could not set the correct forwarding value for interface %s: stdout: %v, stderr: %v, err: %v",
-				bridgeName, stdout, stderr, err)
-		}
-		return nil
-	}
+	// IPv6 forwarding is enabled globally
 	if config.IPv4Mode {
-		if err := createForwardingRule("ipv4"); err != nil {
-			return "", fmt.Errorf("could not add IPv4 forwarding rule: %v", err)
-		}
-	}
-	if config.IPv6Mode {
-		if err := createForwardingRule("ipv6"); err != nil {
-			return "", fmt.Errorf("could not add IPv6 forwarding rule: %v", err)
+		stdout, stderr, err := util.RunSysctl("-w", fmt.Sprintf("net.ipv4.conf.%s.forwarding=1", bridgeName))
+		if err != nil || stdout != fmt.Sprintf("net.ipv4.conf.%s.forwarding = 1", bridgeName) {
+			return "", fmt.Errorf("could not set the correct forwarding value for interface %s: stdout: %v, stderr: %v, err: %v",
+				bridgeName, stdout, stderr, err)
 		}
 	}
 

go-controller/pkg/node/gateway_init_linux_test.go

@@ -109,12 +109,7 @@ func shareGatewayInterfaceTest(app *cli.App, testNS ns.NetNS,
 				Output: "net.ipv4.conf.breth0.forwarding = 1",
 			})
 		}
-		if config.IPv6Mode {
-			fexec.AddFakeCmd(&ovntest.ExpectedCmd{
-				Cmd:    "sysctl -w net.ipv6.conf.breth0.forwarding=1",
-				Output: "net.ipv6.conf.breth0.forwarding = 1",
-			})
-		}
+
 		fexec.AddFakeCmd(&ovntest.ExpectedCmd{
 			Cmd:    "ovs-vsctl --timeout=15 --if-exists get interface breth0 mac_in_use",
 			Output: eth0MAC,
@@ -909,12 +904,7 @@ OFPT_GET_CONFIG_REPLY (xid=0x4): frags=normal miss_send_len=0`
 				Output: "net.ipv4.conf.breth0.forwarding = 1",
 			})
 		}
-		if config.IPv6Mode {
-			fexec.AddFakeCmd(&ovntest.ExpectedCmd{
-				Cmd:    "sysctl -w net.ipv6.conf.breth0.forwarding=1",
-				Output: "net.ipv6.conf.breth0.forwarding = 1",
-			})
-		}
+
 		fexec.AddFakeCmd(&ovntest.ExpectedCmd{
 			Cmd:    "ovs-vsctl --timeout=15 --if-exists get interface breth0 mac_in_use",
 			Output: eth0MAC,
@@ -1084,6 +1074,8 @@ OFPT_GET_CONFIG_REPLY (xid=0x4): frags=normal miss_send_len=0`
 		})
 		err = testNS.Do(func(ns.NetNS) error {
 			defer GinkgoRecover()
+
+			Expect(configureGlobalForwarding()).To(Succeed())
 			gatewayNextHops, gatewayIntf, err := getGatewayNextHops()
 			Expect(err).NotTo(HaveOccurred())
 			ifAddrs := ovntest.MustParseIPNets(eth0CIDR)
@@ -1183,8 +1175,7 @@ OFPT_GET_CONFIG_REPLY (xid=0x4): frags=normal miss_send_len=0`
 					"-s 10.1.0.0/16 -j ACCEPT",
 					"-i ovn-k8s-mp0 -j ACCEPT",
 					"-o ovn-k8s-mp0 -j ACCEPT",
-					"-i breth0 -j DROP",
-					"-o breth0 -j DROP",
+					"-j DROP",
 				},
 				"INPUT": []string{
 					"-i ovn-k8s-mp0 -m comment --comment from OVN to localhost -j ACCEPT",

go-controller/pkg/node/gateway_iptables.go

@@ -366,9 +366,9 @@ func getGatewayForwardRules(cidrs []*net.IPNet) []nodeipt.Rule {
 	return returnRules
 }
 
-func getGatewayDropRules(ifName string) []nodeipt.Rule {
+func getGatewayIPv4DropRules(ifName string) []nodeipt.Rule {
 	var dropRules []nodeipt.Rule
-	for _, protocol := range clusterIPTablesProtocols() {
+	if config.IPv4Mode {
 		dropRules = append(dropRules, []nodeipt.Rule{
 			{
 				Table: "filter",
@@ -377,7 +377,7 @@ func getGatewayDropRules(ifName string) []nodeipt.Rule {
 					"-i", ifName,
 					"-j", "DROP",
 				},
-				Protocol: protocol,
+				Protocol: iptables.ProtocolIPv4,
 			},
 			{
 				Table: "filter",
@@ -386,7 +386,7 @@ func getGatewayDropRules(ifName string) []nodeipt.Rule {
 					"-o", ifName,
 					"-j", "DROP",
 				},
-				Protocol: protocol,
+				Protocol: iptables.ProtocolIPv4,
 			},
 		}...)
 	}
@@ -408,20 +408,6 @@ func delExternalBridgeServiceForwardingRules(cidrs []*net.IPNet) error {
 	return deleteIptRules(getGatewayForwardRules(cidrs))
 }
 
-// initExternalBridgeDropRules sets up iptables rules to block forwarding
-// in br-* interfaces (also for 2ndary bridge) - we block for v4 and v6 based on clusterStack
-// -A FORWARD -i breth1 -j DROP
-// -A FORWARD -o breth1 -j DROP
-func initExternalBridgeDropForwardingRules(ifName string) error {
-	return appendIptRules(getGatewayDropRules(ifName))
-}
-
-// delExternalBridgeDropForwardingRules removes iptables rules which might
-// have been added to disable forwarding
-func delExternalBridgeDropForwardingRules(ifName string) error {
-	return deleteIptRules(getGatewayDropRules(ifName))
-}
-
 func getLocalGatewayFilterRules(ifname string, cidr *net.IPNet) []nodeipt.Rule {
 	// Allow packets to/from the gateway interface in case defaults deny
 	protocol := getIPTablesProtocol(cidr.IP.String())

go-controller/pkg/node/gateway_localnet.go

@@ -79,15 +79,7 @@ func newLocalGateway(nodeName string, hostSubnets []*net.IPNet, gwNextHops []net
 			if err != nil {
 				return err
 			}
-			if config.Gateway.DisableForwarding {
-				if err := initExternalBridgeDropForwardingRules(exGwBridge.bridgeName); err != nil {
-					return fmt.Errorf("failed to add drop rules in forwarding table for bridge %s: err %v", exGwBridge.bridgeName, err)
-				}
-			} else {
-				if err := delExternalBridgeDropForwardingRules(exGwBridge.bridgeName); err != nil {
-					return fmt.Errorf("failed to delete drop rules in forwarding table for bridge %s: err %v", exGwBridge.bridgeName, err)
-				}
-			}
+
 		}
 
 		gw.nodeIPManager = newAddressManager(nodeName, kube, cfg, watchFactory, gwBridge)

go-controller/pkg/node/gateway_localnet_linux_test.go

@@ -89,16 +89,10 @@ func startNodePortWatcher(n *nodePortWatcher, fakeClient *util.OVNNodeClientset,
 		if err := initExternalBridgeServiceForwardingRules(subnets); err != nil {
 			return fmt.Errorf("failed to add accept rules in forwarding table for bridge %s: err %v", linkName, err)
 		}
-		if err := initExternalBridgeDropForwardingRules(linkName); err != nil {
-			return fmt.Errorf("failed to add drop rules in forwarding table for bridge %s: err %v", linkName, err)
-		}
 	} else {
 		if err := delExternalBridgeServiceForwardingRules(subnets); err != nil {
 			return fmt.Errorf("failed to delete accept rules in forwarding table for bridge %s: err %v", linkName, err)
 		}
-		if err := delExternalBridgeDropForwardingRules(linkName); err != nil {
-			return fmt.Errorf("failed to delete drop rules in forwarding table for bridge %s: err %v", linkName, err)
-		}
 	}
 
 	// set up a controller to handle events on services to mock the nodeportwatcher bits
@@ -2820,8 +2814,7 @@ var _ = Describe("Node Operations", func() {
 							"-s 172.16.1.0/24 -j ACCEPT",
 							"-d 10.1.0.0/16 -j ACCEPT",
 							"-s 10.1.0.0/16 -j ACCEPT",
-							"-i breth0 -j DROP",
-							"-o breth0 -j DROP",
+							"-j DROP",
 						},
 					},
 					"mangle": {
@@ -2847,6 +2840,7 @@ var _ = Describe("Node Operations", func() {
 				// Enable forwarding and test deletion of iptables rules from FORWARD chain
 				config.Gateway.DisableForwarding = false
 				fNPW.watchFactory = fakeOvnNode.watcher
+				Expect(configureGlobalForwarding()).To(Succeed())
 				Expect(startNodePortWatcher(fNPW, fakeOvnNode.fakeClient, &fakeMgmtPortConfig)).To(Succeed())
 				expectedTables = map[string]util.FakeTable{
 					"nat": {

go-controller/pkg/node/gateway_shared_intf.go

@@ -1756,15 +1756,6 @@ func newSharedGateway(nodeName string, subnets []*net.IPNet, gwNextHops []net.IP
 			if err != nil {
 				return err
 			}
-			if config.Gateway.DisableForwarding {
-				if err := initExternalBridgeDropForwardingRules(exGwBridge.bridgeName); err != nil {
-					return fmt.Errorf("failed to add drop rules in forwarding table for bridge %s: err %v", exGwBridge.bridgeName, err)
-				}
-			} else {
-				if err := delExternalBridgeDropForwardingRules(exGwBridge.bridgeName); err != nil {
-					return fmt.Errorf("failed to delete drop rules in forwarding table for bridge %s: err %v", exGwBridge.bridgeName, err)
-				}
-			}
 		}
 		gw.nodeIPManager = newAddressManager(nodeName, kube, cfg, watchFactory, gwBridge)
 		nodeIPs := gw.nodeIPManager.ListAddresses()
@@ -1876,16 +1867,10 @@ func newNodePortWatcher(gwBridge *bridgeConfiguration, ofm *openflowManager,
 		if err := initExternalBridgeServiceForwardingRules(subnets); err != nil {
 			return nil, fmt.Errorf("failed to add accept rules in forwarding table for bridge %s: err %v", gwBridge.bridgeName, err)
 		}
-		if err := initExternalBridgeDropForwardingRules(gwBridge.bridgeName); err != nil {
-			return nil, fmt.Errorf("failed to add drop rules in forwarding table for bridge %s: err %v", gwBridge.bridgeName, err)
-		}
 	} else {
 		if err := delExternalBridgeServiceForwardingRules(subnets); err != nil {
 			return nil, fmt.Errorf("failed to delete accept rules in forwarding table for bridge %s: err %v", gwBridge.bridgeName, err)
 		}
-		if err := delExternalBridgeDropForwardingRules(gwBridge.bridgeName); err != nil {
-			return nil, fmt.Errorf("failed to delete drop rules in forwarding table for bridge %s: err %v", gwBridge.bridgeName, err)
-		}
 	}
 
 	// used to tell addServiceRules which rules to add

go-controller/pkg/node/management-port_linux.go

@@ -233,23 +233,12 @@ func setupManagementPortIPFamilyConfig(routeManager *routemanager.Controller, mp
 		return warnings, err
 	}
 
-	createForwardingRule := func(family string) error {
-		stdout, stderr, err := util.RunSysctl("-w", fmt.Sprintf("net.%s.conf.%s.forwarding=1", family, types.K8sMgmtIntfName))
-		if err != nil || stdout != fmt.Sprintf("net.%s.conf.%s.forwarding = 1", family, types.K8sMgmtIntfName) {
-			return fmt.Errorf("could not set the correct forwarding value for interface %s: stdout: %v, stderr: %v, err: %v",
-				types.K8sMgmtIntfName, stdout, stderr, err)
-		}
-		return nil
-	}
-
+	// IPv6 forwarding is enabled globally
 	if mpcfg.ipv4 != nil && cfg == mpcfg.ipv4 {
-		if err := createForwardingRule("ipv4"); err != nil {
-			return warnings, fmt.Errorf("could not add IPv4 forwarding rule: %v", err)
-		}
-	}
-	if mpcfg.ipv6 != nil && cfg == mpcfg.ipv6 {
-		if err := createForwardingRule("ipv6"); err != nil {
-			return warnings, fmt.Errorf("could not add IPv6 forwarding rule: %v", err)
+		stdout, stderr, err := util.RunSysctl("-w", fmt.Sprintf("net.ipv4.conf.%s.forwarding=1", types.K8sMgmtIntfName))
+		if err != nil || stdout != fmt.Sprintf("net.ipv4.conf.%s.forwarding = 1", types.K8sMgmtIntfName) {
+			return warnings, fmt.Errorf("could not set the correct forwarding value for interface %s: stdout: %v, stderr: %v, err: %v",
+				types.K8sMgmtIntfName, stdout, stderr, err)
 		}
 	}
 

go-controller/pkg/node/management-port_linux_test.go

@@ -221,16 +221,12 @@ func testManagementPort(ctx *cli.Context, fexec *ovntest.FakeExec, testNS ns.Net
 		"ovs-vsctl --timeout=15 set interface " + mgtPort + " " + fmt.Sprintf("mac=%s", strings.ReplaceAll(mgtPortMAC, ":", "\\:")),
 	})
 	for _, cfg := range configs {
+		// We do not enable per-interface forwarding for IPv6
 		if cfg.family == netlink.FAMILY_V4 {
 			fexec.AddFakeCmd(&ovntest.ExpectedCmd{
 				Cmd:    "sysctl -w net.ipv4.conf.ovn-k8s-mp0.forwarding=1",
 				Output: "net.ipv4.conf.ovn-k8s-mp0.forwarding = 1",
 			})
-		} else {
-			fexec.AddFakeCmd(&ovntest.ExpectedCmd{
-				Cmd:    "sysctl -w net.ipv6.conf.ovn-k8s-mp0.forwarding=1",
-				Output: "net.ipv6.conf.ovn-k8s-mp0.forwarding = 1",
-			})
 		}
 	}
 	fexec.AddFakeCmd(&ovntest.ExpectedCmd{
@@ -427,18 +423,13 @@ func testManagementPortDPUHost(ctx *cli.Context, fexec *ovntest.FakeExec, testNS
 	})
 
 	for _, cfg := range configs {
+		// We do not enable per-interface forwarding for IPv6
 		if cfg.family == netlink.FAMILY_V4 {
 			fexec.AddFakeCmd(&ovntest.ExpectedCmd{
 				Cmd:    "sysctl -w net.ipv4.conf.ovn-k8s-mp0.forwarding=1",
 				Output: "net.ipv4.conf.ovn-k8s-mp0.forwarding = 1",
 			})
 		}
-		if cfg.family == netlink.FAMILY_V6 {
-			fexec.AddFakeCmd(&ovntest.ExpectedCmd{
-				Cmd:    "sysctl -w net.ipv6.conf.ovn-k8s-mp0.forwarding=1",
-				Output: "net.ipv6.conf.ovn-k8s-mp0.forwarding = 1",
-			})
-		}
 	}
 
 	err := util.SetExec(fexec)