Merge pull request #573 from astoycos/fix-multicast-4.6

Bug 1966833: BACKPORT Add a cluster-wide group with node ls-to-cluster-router ports.

go-controller/pkg/ovn/master.go

@@ -269,6 +269,15 @@ func (oc *Controller) SetupMaster(masterNodeName string) error {
 		return err
 	}
 
+	// Create a cluster-wide port group with all node-to-cluster router
+	// logical switch ports.  Currently the only user is multicast but it might
+	// be used for other features in the future.
+	oc.clusterRtrPortGroupUUID, err = createPortGroup(clusterRtrPortGroupName, clusterRtrPortGroupName)
+	if err != nil {
+		klog.Errorf("Failed to create cluster port group: %v", err)
+		return err
+	}
+
 	// If supported, enable IGMP relay on the router to forward multicast
 	// traffic between nodes.
 	if oc.multicastSupport {
@@ -286,6 +295,13 @@ func (oc *Controller) SetupMaster(masterNodeName string) error {
 			klog.Errorf("Failed to create default deny multicast policy, error: %v", err)
 			return err
 		}
+
+		// Allow IP multicast from node switch to cluster router and from
+		// cluster router to node switch.
+		if err := oc.createDefaultAllowMulticastPolicy(); err != nil {
+			klog.Errorf("Failed to create default deny multicast policy, error: %v", err)
+			return err
+		}
 	}
 
 	// Create 3 load-balancers for east-west traffic for UDP, TCP, SCTP
@@ -385,6 +401,31 @@ func (oc *Controller) deleteNodeJoinSubnet(nodeName string, subnet *net.IPNet) e
 	return nil
 }
 
+func addNodeLogicalSwitchPort(logicalSwitch, portName, portType, addresses, options string) (string, error) {
+	stdout, stderr, err := util.RunOVNNbctl("--", "--may-exist", "lsp-add", logicalSwitch, portName,
+		"--", "lsp-set-type", portName, portType,
+		"--", "lsp-set-options", portName, options,
+		"--", "lsp-set-addresses", portName, addresses)
+	if err != nil {
+		klog.Errorf("Failed to add logical port %s to switch %s, stdout: %q, stderr: %q, error: %v", portName, logicalSwitch, stdout, stderr, err)
+		return "", err
+	}
+
+	// UUID must be retrieved separately from the lsp-add transaction since
+	// (as of OVN 2.12) a bogus UUID is returned if they are part of the same
+	// transaction.
+	uuid, stderr, err := util.RunOVNNbctl("get", "logical_switch_port", portName, "_uuid")
+	if err != nil {
+		klog.Errorf("Error getting UUID for logical port %s "+
+			"stdout: %q, stderr: %q (%v)", portName, uuid, stderr, err)
+		return "", err
+	}
+	if uuid == "" {
+		return uuid, fmt.Errorf("invalid logical port %s uuid", portName)
+	}
+	return uuid, nil
+}
+
 func (oc *Controller) syncNodeManagementPort(node *kapi.Node, hostSubnets []*net.IPNet) error {
 	macAddress, err := util.ParseNodeManagementPortMACAddress(node)
 	if err != nil {
@@ -426,28 +467,11 @@ func (oc *Controller) syncNodeManagementPort(node *kapi.Node, hostSubnets []*net
 
 	// Create this node's management logical port on the node switch
 	portName := util.K8sPrefix + node.Name
-	stdout, stderr, err := util.RunOVNNbctl(
-		"--", "--may-exist", "lsp-add", node.Name, portName,
-		"--", "lsp-set-addresses", portName, addresses)
+	uuid, err := addNodeLogicalSwitchPort(node.Name, portName, "", addresses, "")
 	if err != nil {
-		klog.Errorf("Failed to add logical port to switch, stdout: %q, stderr: %q, error: %v",
-			stdout, stderr, err)
 		return err
 	}
 
-	// UUID must be retrieved separately from the lsp-add transaction since
-	// (as of OVN 2.12) a bogus UUID is returned if they are part of the same
-	// transaction.
-	uuid, stderr, err := util.RunOVNNbctl("get", "logical_switch_port", portName, "_uuid")
-	if err != nil {
-		klog.Errorf("Error getting UUID for logical port %s "+
-			"stdout: %q, stderr: %q (%v)", portName, uuid, stderr, err)
-		return err
-	}
-	if uuid == "" {
-		return fmt.Errorf("invalid logical port %s uuid %q", portName, uuid)
-	}
-
 	if err := addToPortGroup(clusterPortGroupName, &lpInfo{
 		uuid: uuid,
 		name: portName,
@@ -641,14 +665,21 @@ func (oc *Controller) ensureNodeLogicalNetwork(nodeName string, hostSubnets []*n
 	}
 
 	// Connect the switch to the router.
-	stdout, stderr, err = util.RunOVNNbctl("--", "--may-exist", "lsp-add", nodeName, switchToRouterPrefix+nodeName,
-		"--", "set", "logical_switch_port", switchToRouterPrefix+nodeName, "type=router",
-		"options:router-port="+routerToSwitchPrefix+nodeName, "addresses="+"\""+nodeLRPMAC.String()+"\"")
+	nodeSwToRtrUUID, err := addNodeLogicalSwitchPort(nodeName, util.SwitchToRouterPrefix+nodeName,
+		"router", nodeLRPMAC.String(), "router-port="+util.RouterToSwitchPrefix+nodeName)
 	if err != nil {
 		klog.Errorf("Failed to add logical port to switch, stdout: %q, stderr: %q, error: %v", stdout, stderr, err)
 		return err
 	}
 
+	if err = addToPortGroup(clusterRtrPortGroupName, &lpInfo{
+		uuid: nodeSwToRtrUUID,
+		name: util.SwitchToRouterPrefix + nodeName,
+	}); err != nil {
+		klog.Errorf(err.Error())
+		return err
+	}
+
 	// Add our cluster TCP and UDP load balancers to the node switch
 	if oc.TCPLoadBalancerUUID == "" {
 		return fmt.Errorf("TCP cluster load balancer not created")

go-controller/pkg/ovn/master_test.go

@@ -3,17 +3,19 @@ package ovn
 import (
 	"context"
 	"fmt"
+	"net"
+	"strings"
+
 	goovn "github.com/ebay/go-ovn"
 	"github.com/urfave/cli/v2"
 	v1 "k8s.io/api/core/v1"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/client-go/kubernetes/fake"
 	"k8s.io/client-go/tools/record"
-	"net"
-	"strings"
 
 	egressfirewallfake "github.com/ovn-org/ovn-kubernetes/go-controller/pkg/crd/egressfirewall/v1/apis/clientset/versioned/fake"
 	egressipfake "github.com/ovn-org/ovn-kubernetes/go-controller/pkg/crd/egressip/v1/apis/clientset/versioned/fake"
+	"github.com/ovn-org/ovn-kubernetes/go-controller/pkg/types"
 	apiextensionsfake "k8s.io/apiextensions-apiserver/pkg/client/clientset/clientset/fake"
 
 	"github.com/ovn-org/ovn-kubernetes/go-controller/pkg/config"
@@ -139,6 +141,8 @@ func defaultFakeExec(nodeSubnet, nodeName string, sctpSupport bool) (*ovntest.Fa
 		"ovn-nbctl --timeout=15 -- set logical_router ovn_cluster_router options:mcast_relay=\"true\"",
 		"ovn-nbctl --timeout=15 --data=bare --no-heading --columns=_uuid find port_group name=clusterPortGroup",
 		"ovn-nbctl --timeout=15 create port_group name=clusterPortGroup external-ids:name=clusterPortGroup",
+		"ovn-nbctl --timeout=15 --data=bare --no-heading --columns=_uuid find port_group name=clusterRtrPortGroup",
+		"ovn-nbctl --timeout=15 create port_group name=clusterRtrPortGroup external-ids:name=clusterRtrPortGroup",
 		"ovn-nbctl --timeout=15 --data=bare --no-heading --columns=_uuid find ACL match=\"ip4.mcast\" action=drop external-ids:default-deny-policy-type=Egress",
 		"ovn-nbctl --timeout=15 --id=@acl create acl priority=1011 direction=from-lport match=\"ip4.mcast\" action=drop external-ids:default-deny-policy-type=Egress -- add port_group  acls @acl",
 		"ovn-nbctl --timeout=15 --data=bare --no-heading --columns=_uuid find ACL match=\"ip4.mcast\" action=drop external-ids:default-deny-policy-type=Ingress",
@@ -187,7 +191,15 @@ func defaultFakeExec(nodeSubnet, nodeName string, sctpSupport bool) (*ovntest.Fa
 		"ovn-nbctl --timeout=15 --may-exist ls-add " + nodeName + " -- set logical_switch " + nodeName + " other-config:subnet=" + nodeSubnet + " other-config:exclude_ips=" + nodeMgmtPortIP.String() + ".." + hybridOverlayIP.String(),
 		"ovn-nbctl --timeout=15 set logical_switch " + nodeName + " other-config:mcast_snoop=\"true\"",
 		"ovn-nbctl --timeout=15 set logical_switch " + nodeName + " other-config:mcast_querier=\"true\" other-config:mcast_eth_src=\"" + lrpMAC + "\" other-config:mcast_ip4_src=\"" + gwIP + "\"",
-		"ovn-nbctl --timeout=15 -- --may-exist lsp-add " + nodeName + " " + switchToRouterPrefix + nodeName + " -- set logical_switch_port " + switchToRouterPrefix + nodeName + " type=router options:router-port=" + routerToSwitchPrefix + nodeName + " addresses=\"" + lrpMAC + "\"",
+		"ovn-nbctl --timeout=15 -- --may-exist lsp-add " + nodeName + " " + types.SwitchToRouterPrefix + nodeName + " -- lsp-set-type " + types.SwitchToRouterPrefix + nodeName + " router -- lsp-set-options " + types.SwitchToRouterPrefix + nodeName + " router-port=" + types.RouterToSwitchPrefix + nodeName + " -- lsp-set-addresses " + types.SwitchToRouterPrefix + nodeName + " " + lrpMAC,
+	})
+	fexec.AddFakeCmd(&ovntest.ExpectedCmd{
+		Cmd:    "ovn-nbctl --timeout=15 get logical_switch_port " + types.SwitchToRouterPrefix + nodeName + " _uuid",
+		Output: fakeUUID + "\n",
+	})
+
+	fexec.AddFakeCmdsNoOutputNoError([]string{
+		"ovn-nbctl --timeout=15 --if-exists remove port_group clusterRtrPortGroup ports " + fakeUUID + " -- add port_group clusterRtrPortGroup ports " + fakeUUID,
 		"ovn-nbctl --timeout=15 set logical_switch " + nodeName + " load_balancer=" + tcpLBUUID,
 		"ovn-nbctl --timeout=15 add logical_switch " + nodeName + " load_balancer " + udpLBUUID,
 	})
@@ -198,7 +210,7 @@ func defaultFakeExec(nodeSubnet, nodeName string, sctpSupport bool) (*ovntest.Fa
 	}
 	fexec.AddFakeCmdsNoOutputNoError([]string{
 		"ovn-nbctl --timeout=15 --may-exist acl-add " + nodeName + " to-lport 1001 ip4.src==" + nodeMgmtPortIP.String() + " allow-related",
-		"ovn-nbctl --timeout=15 -- --may-exist lsp-add " + nodeName + " " + util.K8sPrefix + nodeName + " -- lsp-set-addresses " + util.K8sPrefix + nodeName + " " + mgmtMAC + " " + nodeMgmtPortIP.String(),
+		"ovn-nbctl --timeout=15 -- --may-exist lsp-add " + nodeName + " " + types.K8sPrefix + nodeName + " -- lsp-set-type " + types.K8sPrefix + nodeName + "  -- lsp-set-options " + types.K8sPrefix + nodeName + "  -- lsp-set-addresses " + types.K8sPrefix + nodeName + " " + mgmtMAC + " " + nodeMgmtPortIP.String(),
 	})
 	fexec.AddFakeCmd(&ovntest.ExpectedCmd{
 		Cmd:    "ovn-nbctl --timeout=15 get logical_switch_port " + util.K8sPrefix + nodeName + " _uuid",
@@ -1039,12 +1051,21 @@ var _ = Describe("Gateway Init Operations", func() {
 			fexec.AddFakeCmdsNoOutputNoError([]string{
 				"ovn-nbctl --timeout=15 --if-exists lrp-del " + routerToSwitchPrefix + nodeName + " -- lrp-add ovn_cluster_router " + routerToSwitchPrefix + nodeName + " " + nodeLRPMAC + " " + nodeGWIP,
 				"ovn-nbctl --timeout=15 --may-exist ls-add " + nodeName + " -- set logical_switch " + nodeName + " other-config:subnet=" + nodeSubnet + " other-config:exclude_ips=" + nodeMgmtPortIP,
-				"ovn-nbctl --timeout=15 -- --may-exist lsp-add " + nodeName + " " + switchToRouterPrefix + nodeName + " -- set logical_switch_port " + switchToRouterPrefix + nodeName + " type=router options:router-port=" + routerToSwitchPrefix + nodeName + " addresses=\"" + nodeLRPMAC + "\"",
+				"ovn-nbctl --timeout=15 -- --may-exist lsp-add " + nodeName + " " + types.SwitchToRouterPrefix + nodeName + " -- lsp-set-type " + types.SwitchToRouterPrefix + nodeName + " router -- lsp-set-options " + types.SwitchToRouterPrefix + nodeName + " router-port=" + types.RouterToSwitchPrefix + nodeName + " -- lsp-set-addresses " + types.SwitchToRouterPrefix + nodeName + " " + nodeLRPMAC,
+			})
+
+			fexec.AddFakeCmd(&ovntest.ExpectedCmd{
+				Cmd:    "ovn-nbctl --timeout=15 get logical_switch_port " + types.SwitchToRouterPrefix + nodeName + " _uuid",
+				Output: fakeUUID + "\n",
+			})
+
+			fexec.AddFakeCmdsNoOutputNoError([]string{
+				"ovn-nbctl --timeout=15 --if-exists remove port_group clusterRtrPortGroup ports " + fakeUUID + " -- add port_group clusterRtrPortGroup ports " + fakeUUID,
 				"ovn-nbctl --timeout=15 set logical_switch " + nodeName + " load_balancer=" + tcpLBUUID,
 				"ovn-nbctl --timeout=15 add logical_switch " + nodeName + " load_balancer " + udpLBUUID,
 				"ovn-nbctl --timeout=15 add logical_switch " + nodeName + " load_balancer " + sctpLBUUID,
 				"ovn-nbctl --timeout=15 --may-exist acl-add " + nodeName + " to-lport 1001 ip4.src==" + nodeMgmtPortIP + " allow-related",
-				"ovn-nbctl --timeout=15 -- --may-exist lsp-add " + nodeName + " " + util.K8sPrefix + nodeName + " -- lsp-set-addresses " + util.K8sPrefix + nodeName + " " + nodeMgmtPortMAC + " " + nodeMgmtPortIP,
+				"ovn-nbctl --timeout=15 -- --may-exist lsp-add " + nodeName + " " + types.K8sPrefix + nodeName + " -- lsp-set-type " + types.K8sPrefix + nodeName + "  -- lsp-set-options " + types.K8sPrefix + nodeName + "  -- lsp-set-addresses " + types.K8sPrefix + nodeName + " " + nodeMgmtPortMAC + " " + nodeMgmtPortIP,
 			})
 			fexec.AddFakeCmd(&ovntest.ExpectedCmd{
 				Cmd:    "ovn-nbctl --timeout=15 get logical_switch_port " + util.K8sPrefix + nodeName + " _uuid",

go-controller/pkg/ovn/ovn.go

@@ -42,8 +42,10 @@ import (
 )
 
 const (
-	egressfirewallCRD    string = "egressfirewalls.k8s.ovn.org"
-	clusterPortGroupName string = "clusterPortGroup"
+	egressfirewallCRD                string        = "egressfirewalls.k8s.ovn.org"
+	clusterPortGroupName             string        = "clusterPortGroup"
+	clusterRtrPortGroupName          string        = "clusterRtrPortGroup"
+	egressFirewallDNSDefaultDuration time.Duration = 30 * time.Minute
 )
 
 // ServiceVIPKey is used for looking up service namespace information for a
@@ -148,6 +150,10 @@ type Controller struct {
 	// Port group for all cluster logical switch ports
 	clusterPortGroupUUID string
 
+	// Port group for all node logical switch ports connected to the cluster
+	// logical router
+	clusterRtrPortGroupUUID string
+
 	// Port group for ingress deny rule
 	portGroupIngressDeny string
 

go-controller/pkg/ovn/policy.go

@@ -57,6 +57,8 @@ const (
 	defaultMcastDenyPriority = "1011"
 	// Default multicast allow acl rule priority
 	defaultMcastAllowPriority = "1012"
+	// Default routed multicast allow acl rule priority
+	defaultRoutedMcastAllowPriority = "1013"
 )
 
 func (oc *Controller) syncNetworkPolicies(networkPolicies []interface{}) {
@@ -354,6 +356,27 @@ func (oc *Controller) createDefaultDenyMulticastPolicy() error {
 	return nil
 }
 
+// Creates a global default allow multicast policy:
+// - one ACL allowing multicast traffic from cluster router ports
+// - one ACL allowing multicast traffic to cluster router ports.
+// Caller must hold the namespace's namespaceInfo object lock.
+func (oc *Controller) createDefaultAllowMulticastPolicy() error {
+	match := getACLMatch(clusterRtrPortGroupName, "ip4.mcast", knet.PolicyTypeEgress)
+	err := addACLPortGroup(oc.clusterRtrPortGroupUUID, fromLport,
+		defaultRoutedMcastAllowPriority, match, "allow", knet.PolicyTypeEgress)
+	if err != nil {
+		return fmt.Errorf("failed to create default deny multicast egress ACL: %v", err)
+	}
+
+	match = getACLMatch(clusterRtrPortGroupName, "ip4.mcast", knet.PolicyTypeIngress)
+	err = addACLPortGroup(oc.clusterRtrPortGroupUUID, toLport,
+		defaultRoutedMcastAllowPriority, match, "allow", knet.PolicyTypeIngress)
+	if err != nil {
+		return fmt.Errorf("failed to create default deny multicast ingress ACL: %v", err)
+	}
+	return nil
+}
+
 // podAddAllowMulticastPolicy adds the pod's logical switch port to the namespace's
 // multicast port group. Caller must hold the namespace's namespaceInfo object
 // lock.

go-controller/pkg/util/util.go

@@ -23,6 +23,10 @@ const (
 	// access to physical/external network
 	PhysicalNetworkName = "physnet"
 
+	// Constants for Multicast ACLs
+	SwitchToRouterPrefix = "stor-"
+	RouterToSwitchPrefix = "rtos-"
+
 	// LocalNetworkName is the name that maps to an OVS bridge that provides
 	// access to local service
 	LocalNetworkName = "locnet"