Merge pull request #5093 from ricky-rav/SDN-5345

SDN-5345: allow localnet -> host network on the same node

Author: trozet

go-controller/pkg/node/gateway_shared_intf.go

@@ -1850,9 +1850,10 @@ func commonFlows(hostSubnets []*net.IPNet, bridge *bridgeConfiguration) ([]strin
 							defaultOpenFlowCookie, netConfig.ofPortPatch, bridgeMacAddress, config.Default.ConntrackZone,
 							netConfig.masqCTMark, ofPortPhys))
 
-					// Allow OVN->Host traffic on the same node
+					// Allow (a) OVN->host traffic on the same node
+					// (b) host->host traffic on the same node
 					if config.Gateway.Mode == config.GatewayModeShared || config.Gateway.Mode == config.GatewayModeLocal {
-						dftFlows = append(dftFlows, ovnToHostNetworkNormalActionFlows(netConfig, bridgeMacAddress, hostSubnets, false)...)
+						dftFlows = append(dftFlows, hostNetworkNormalActionFlows(netConfig, bridgeMacAddress, hostSubnets, false)...)
 					}
 				} else {
 					//  for UDN we additionally SNAT the packet from masquerade IP -> node IP
@@ -1946,9 +1947,10 @@ func commonFlows(hostSubnets []*net.IPNet, bridge *bridgeConfiguration) ([]strin
 							"actions=ct(commit, zone=%d, exec(set_field:%s->ct_mark)), output:%s",
 							defaultOpenFlowCookie, netConfig.ofPortPatch, bridgeMacAddress, config.Default.ConntrackZone, netConfig.masqCTMark, ofPortPhys))
 
-					// Allow OVN->Host traffic on the same node
+					// Allow (a) OVN->host traffic on the same node
+					// (b) host->host traffic on the same node
 					if config.Gateway.Mode == config.GatewayModeShared || config.Gateway.Mode == config.GatewayModeLocal {
-						dftFlows = append(dftFlows, ovnToHostNetworkNormalActionFlows(netConfig, bridgeMacAddress, hostSubnets, true)...)
+						dftFlows = append(dftFlows, hostNetworkNormalActionFlows(netConfig, bridgeMacAddress, hostSubnets, true)...)
 					}
 				} else {
 					//  for UDN we additionally SNAT the packet from masquerade IP -> node IP
@@ -2128,23 +2130,15 @@ func commonFlows(hostSubnets []*net.IPNet, bridge *bridgeConfiguration) ([]strin
 	return dftFlows, nil
 }
 
-// ovnToHostNetworkNormalActionFlows returns the flows that allow IP{v4,v6} traffic from the OVN network to the host network
-// when the destination is on the same node as the sender. This is necessary for pods in the default network to reach
-// localnet pods on the same node, when the localnet is mapped to breth0. The expected srcMAC is the MAC address of breth0
-// and the expected hostSubnets is the host subnets found on the node primary interface.
-func ovnToHostNetworkNormalActionFlows(netConfig *bridgeUDNConfiguration, srcMAC string, hostSubnets []*net.IPNet, isV6 bool) []string {
-	var inPort, ctMark, ipFamily, ipFamilyDest string
+// hostNetworkNormalActionFlows returns the flows that allow IP{v4,v6} traffic:
+// a. from pods in the OVN network to pods in a localnet network, on the same node
+// b. from pods on the host to pods in a localnet network, on the same node
+// when the localnet is mapped to breth0.
+// The expected srcMAC is the MAC address of breth0 and the expected hostSubnets is the host subnets found on the node
+// primary interface.
+func hostNetworkNormalActionFlows(netConfig *bridgeUDNConfiguration, srcMAC string, hostSubnets []*net.IPNet, isV6 bool) []string {
 	var flows []string
-
-	if config.Gateway.Mode == config.GatewayModeShared {
-		inPort = netConfig.ofPortPatch
-		ctMark = netConfig.masqCTMark
-	} else if config.Gateway.Mode == config.GatewayModeLocal {
-		inPort = "LOCAL"
-		ctMark = ctMarkHost
-	} else {
-		return nil
-	}
+	var ipFamily, ipFamilyDest string
 
 	if isV6 {
 		ipFamily = "ipv6"
@@ -2154,38 +2148,69 @@ func ovnToHostNetworkNormalActionFlows(netConfig *bridgeUDNConfiguration, srcMAC
 		ipFamilyDest = "nw_dst"
 	}
 
+	formatFlow := func(inPort, destIP, ctMark string) string {
+		// Matching IP traffic will be handled by the bridge instead of being output directly
+		// to the NIC by the existing flow at prio=100.
+		flowTemplate := "cookie=%s, priority=102, in_port=%s, dl_src=%s, %s, %s=%s, " +
+			"actions=ct(commit, zone=%d, exec(set_field:%s->ct_mark)), output:NORMAL"
+		return fmt.Sprintf(flowTemplate,
+			defaultOpenFlowCookie,
+			inPort,
+			srcMAC,
+			ipFamily,
+			ipFamilyDest,
+			destIP,
+			config.Default.ConntrackZone,
+			ctMark)
+	}
+
+	// Traffic path (a): OVN->localnet for shared gw mode
+	if config.Gateway.Mode == config.GatewayModeShared {
+		for _, hostSubnet := range hostSubnets {
+			if utilnet.IsIPv6(hostSubnet.IP) != isV6 {
+				continue
+			}
+			flows = append(flows, formatFlow(netConfig.ofPortPatch, hostSubnet.String(), netConfig.masqCTMark))
+		}
+	}
+
+	// Traffic path (a): OVN->localnet for local gw mode
+	// Traffic path (b): host->localnet for both gw modes
 	for _, hostSubnet := range hostSubnets {
-		if (hostSubnet.IP.To4() == nil) != isV6 {
+		if utilnet.IsIPv6(hostSubnet.IP) != isV6 {
 			continue
 		}
-		// IP traffic from the OVN network to the host network should be handled normally by the bridge instead of
-		// being output directly to the NIC by the existing flow at prio=100.
-		flows = append(flows,
-			fmt.Sprintf("cookie=%s, priority=102, in_port=%s, dl_src=%s, %s, %s=%s, "+
-				"actions=ct(commit, zone=%d, exec(set_field:%s->ct_mark)), output:NORMAL",
+		flows = append(flows, formatFlow("LOCAL", hostSubnet.String(), ctMarkHost))
+	}
+
+	if isV6 {
+		// IPv6 neighbor discovery uses ICMPv6 messages sent to a special destination (ff02::1:ff00:0/104)
+		// that is unrelated to the host subnets matched in the prio=102 flow above.
+		// Allow neighbor discovery by matching against ICMP type and ingress port.
+		formatICMPFlow := func(inPort, ctMark string, icmpType int) string {
+			icmpFlowTemplate := "cookie=%s, priority=102, in_port=%s, dl_src=%s, icmp6, icmpv6_type=%d, " +
+				"actions=ct(commit, zone=%d, exec(set_field:%s->ct_mark)), output:NORMAL"
+			return fmt.Sprintf(icmpFlowTemplate,
 				defaultOpenFlowCookie,
 				inPort,
 				srcMAC,
-				ipFamily,
-				ipFamilyDest,
-				hostSubnet.String(),
+				icmpType,
 				config.Default.ConntrackZone,
-				ctMark))
-	}
+				ctMark)
+		}
 
-	if isV6 {
-		// Neighbor discovery in IPv6 happens through ICMPv6 messages to a special destination (ff02::1:ff00:0/104),
-		// which has nothing to do with the host subnets we're matching against in the flow above at prio=102.
-		// Let's allow neighbor discovery by matching against icmp type and in_port.
 		for _, icmpType := range []int{types.NeighborSolicitationICMPType, types.NeighborAdvertisementICMPType} {
-			flows = append(flows,
-				fmt.Sprintf("cookie=%s, priority=102, in_port=%s, dl_src=%s, icmp6, icmpv6_type=%d, "+
-					"actions=ct(commit, zone=%d, exec(set_field:%s->ct_mark)), output:NORMAL",
-					defaultOpenFlowCookie, inPort, srcMAC, icmpType,
-					config.Default.ConntrackZone, ctMark))
+			// Traffic path (a) for ICMP: OVN-> localnet for shared gw mode
+			if config.Gateway.Mode == config.GatewayModeShared {
+				flows = append(flows,
+					formatICMPFlow(netConfig.ofPortPatch, netConfig.masqCTMark, icmpType))
+			}
+
+			// Traffic path (a) for ICMP: OVN->localnet for local gw mode
+			// Traffic path (b) for ICMP: host->localnet for both gw modes
+			flows = append(flows, formatICMPFlow("LOCAL", ctMarkHost, icmpType))
 		}
 	}
-
 	return flows
 }
 

test/e2e/multihoming.go

@@ -325,17 +325,31 @@ var _ = Describe("Multi Homing", func() {
 				kickstartPod(cs, clientPodConfig)
 
 				// Check that the client pod can reach the server pod on the server localnet interface
-				serverIPs, err := podIPsForAttachment(cs, f.Namespace.Name, serverPod.GetName(), netConfig.name)
+				var serverIPs []string
+				if serverPodConfig.hostNetwork {
+					serverIPs, err = podIPsFromStatus(cs, serverPodConfig.namespace, serverPodConfig.name)
+				} else {
+					serverIPs, err = podIPsForAttachment(cs, serverPod.Namespace, serverPod.Name, netConfig.name)
+
+				}
 				Expect(err).NotTo(HaveOccurred())
+
 				for _, serverIP := range serverIPs {
 					By(fmt.Sprintf("asserting the *client* can contact the server pod exposed endpoint: %q on port %q", serverIP, port))
+					curlArgs := []string{}
+					pingArgs := []string{}
+					if clientPodConfig.attachments != nil {
+						// When the client is attached to a localnet, send probes from the localnet interface
+						curlArgs = []string{"--interface", "net1"}
+						pingArgs = []string{"-I", "net1"}
+					}
 					Eventually(func() error {
-						return reachServerPodFromClient(cs, serverPodConfig, clientPodConfig, serverIP, port)
+						return reachServerPodFromClient(cs, serverPodConfig, clientPodConfig, serverIP, port, curlArgs...)
 					}, 2*time.Minute, 6*time.Second).Should(Succeed())
 
 					By(fmt.Sprintf("asserting the *client* can ping the server pod exposed endpoint: %q", serverIP))
 					Eventually(func() error {
-						return pingServerPodFromClient(cs, serverPodConfig, clientPodConfig, serverIP)
+						return pingServerPodFromClient(cs, serverPodConfig, clientPodConfig, serverIP, pingArgs...)
 					}, 2*time.Minute, 6*time.Second).Should(Succeed())
 				}
 			},
@@ -383,6 +397,52 @@ var _ = Describe("Multi Homing", func() {
 				},
 				Label("BUG", "OCPBUGS-43004"),
 			),
+			ginkgo.Entry(
+				"can reach a host-networked pod on a different node",
+				networkAttachmentConfigParams{
+					name:     secondaryNetworkName,
+					topology: "localnet",
+				},
+				podConfiguration{ // client on localnet
+					attachments: []nadapi.NetworkSelectionElement{{
+						Name: secondaryNetworkName,
+					}},
+					name:                         clientPodName,
+					nodeSelector:                 map[string]string{nodeHostnameKey: workerOneNodeName},
+					isPrivileged:                 true,
+					needsIPRequestFromHostSubnet: true,
+				},
+				podConfiguration{ // server on default network, pod is host-networked
+					name:         podName,
+					containerCmd: httpServerContainerCmd(port),
+					nodeSelector: map[string]string{nodeHostnameKey: workerTwoNodeName},
+					hostNetwork:  true,
+				},
+				Label("STORY", "SDN-5345"),
+			),
+			ginkgo.Entry(
+				"can reach a host-networked pod on the same node",
+				networkAttachmentConfigParams{
+					name:     secondaryNetworkName,
+					topology: "localnet",
+				},
+				podConfiguration{ // client on localnet
+					attachments: []nadapi.NetworkSelectionElement{{
+						Name: secondaryNetworkName,
+					}},
+					name:                         clientPodName,
+					nodeSelector:                 map[string]string{nodeHostnameKey: workerTwoNodeName},
+					isPrivileged:                 true,
+					needsIPRequestFromHostSubnet: true,
+				},
+				podConfiguration{ // server on default network, pod is host-networked
+					name:         podName,
+					containerCmd: httpServerContainerCmd(port),
+					nodeSelector: map[string]string{nodeHostnameKey: workerTwoNodeName},
+					hostNetwork:  true,
+				},
+				Label("STORY", "SDN-5345"),
+			),
 		)
 	})
 
@@ -842,7 +902,6 @@ var _ = Describe("Multi Homing", func() {
 		Context("localnet OVN-K secondary network", func() {
 			const (
 				clientPodName          = "client-pod"
-				nodeHostnameKey        = "kubernetes.io/hostname"
 				servicePort            = 9000
 				dockerNetworkName      = "underlay"
 				underlayServiceIP      = "60.128.0.1"

test/e2e/multihoming_utils.go

@@ -161,6 +161,7 @@ type podConfiguration struct {
 	isPrivileged                 bool
 	labels                       map[string]string
 	requiresExtraNamespace       bool
+	hostNetwork                  bool
 	needsIPRequestFromHostSubnet bool
 }
 
@@ -171,6 +172,7 @@ func generatePodSpec(config podConfiguration) *v1.Pod {
 	}
 	podSpec.Spec.NodeSelector = config.nodeSelector
 	podSpec.Labels = config.labels
+	podSpec.Spec.HostNetwork = config.hostNetwork
 	if config.isPrivileged {
 		podSpec.Spec.Containers[0].SecurityContext.Privileged = ptr.To(true)
 	} else {
@@ -253,17 +255,19 @@ func inRange(cidr string, ip string) error {
 	return fmt.Errorf("ip [%s] is NOT in range %s", ip, cidr)
 }
 
-func connectToServer(clientPodConfig podConfiguration, serverIP string, port int) error {
-	_, err := e2ekubectl.RunKubectl(
-		clientPodConfig.namespace,
+func connectToServer(clientPodConfig podConfiguration, serverIP string, port int, args ...string) error {
+	target := net.JoinHostPort(serverIP, fmt.Sprintf("%d", port))
+	baseArgs := []string{
 		"exec",
 		clientPodConfig.name,
 		"--",
 		"curl",
 		"--connect-timeout",
 		"2",
-		net.JoinHostPort(serverIP, fmt.Sprintf("%d", port)),
-	)
+	}
+	baseArgs = append(baseArgs, args...)
+
+	_, err := e2ekubectl.RunKubectl(clientPodConfig.namespace, append(baseArgs, target)...)
 	return err
 }
 
@@ -308,16 +312,19 @@ func getSecondaryInterfaceMTU(clientPodConfig podConfiguration) (int, error) {
 	return mtu, nil
 }
 
-func pingServer(clientPodConfig podConfiguration, serverIP string) error {
-	_, err := e2ekubectl.RunKubectl(
-		clientPodConfig.namespace,
+func pingServer(clientPodConfig podConfiguration, serverIP string, args ...string) error {
+	baseArgs := []string{
 		"exec",
 		clientPodConfig.name,
 		"--",
 		"ping",
 		"-c", "1", // send one ICMP echo request
 		"-W", "2", // timeout after 2 seconds if no response
-		serverIP)
+	}
+	baseArgs = append(baseArgs, args...)
+
+	_, err := e2ekubectl.RunKubectl(clientPodConfig.namespace, append(baseArgs, serverIP)...)
+
 	return err
 }
 
@@ -381,6 +388,18 @@ func podIPForAttachment(k8sClient clientset.Interface, podNamespace string, podN
 	return ips[ipIndex], nil
 }
 
+func podIPsFromStatus(k8sClient clientset.Interface, podNamespace string, podName string) ([]string, error) {
+	pod, err := k8sClient.CoreV1().Pods(podNamespace).Get(context.Background(), podName, metav1.GetOptions{})
+	if err != nil {
+		return nil, err
+	}
+	podIPs := make([]string, 0, len(pod.Status.PodIPs))
+	for _, podIP := range pod.Status.PodIPs {
+		podIPs = append(podIPs, podIP.IP)
+	}
+	return podIPs, nil
+}
+
 func allowedClient(podName string) string {
 	return "allowed-" + podName
 }
@@ -610,27 +629,27 @@ func allowedTCPPortsForPolicy(allowPorts ...int) []mnpapi.MultiNetworkPolicyPort
 	return portAllowlist
 }
 
-func reachServerPodFromClient(cs clientset.Interface, serverConfig podConfiguration, clientConfig podConfiguration, serverIP string, serverPort int) error {
+func reachServerPodFromClient(cs clientset.Interface, serverConfig podConfiguration, clientConfig podConfiguration, serverIP string, serverPort int, args ...string) error {
 	updatedPod, err := cs.CoreV1().Pods(serverConfig.namespace).Get(context.Background(), serverConfig.name, metav1.GetOptions{})
 	if err != nil {
 		return err
 	}
 
 	if updatedPod.Status.Phase == v1.PodRunning {
-		return connectToServer(clientConfig, serverIP, serverPort)
+		return connectToServer(clientConfig, serverIP, serverPort, args...)
 	}
 
 	return fmt.Errorf("pod not running. /me is sad")
 }
 
-func pingServerPodFromClient(cs clientset.Interface, serverConfig podConfiguration, clientConfig podConfiguration, serverIP string) error {
+func pingServerPodFromClient(cs clientset.Interface, serverConfig podConfiguration, clientConfig podConfiguration, serverIP string, args ...string) error {
 	updatedPod, err := cs.CoreV1().Pods(serverConfig.namespace).Get(context.Background(), serverConfig.name, metav1.GetOptions{})
 	if err != nil {
 		return err
 	}
 
 	if updatedPod.Status.Phase == v1.PodRunning {
-		return pingServer(clientConfig, serverIP)
+		return pingServer(clientConfig, serverIP, args...)
 	}
 
 	return fmt.Errorf("pod not running. /me is sad")