[DNM] Ovs cpu part2 saga revert set3

go-controller/pkg/libovsdb/libovsdb.go

@@ -140,10 +140,6 @@ func NewSBClientWithConfig(cfg config.OvnAuthConfig, promRegistry prometheus.Reg
 	enableMetricsOption := client.WithMetricsRegistryNamespaceSubsystem(promRegistry,
 		"ovnkube", "master_libovsdb")
 
-	dbModel.SetIndexes(map[string][]model.ClientIndex{
-		sbdb.EncapTable: {{Columns: []model.ColumnKey{{Column: "chassis_name"}}}},
-	})
-
 	c, err := newClient(cfg, dbModel, stopCh, enableMetricsOption)
 	if err != nil {
 		return nil, err

go-controller/pkg/libovsdb/ops/chassis.go

@@ -138,27 +138,33 @@ func DeleteChassisWithPredicate(sbClient libovsdbclient.Client, p chassisPredica
 }
 
 // CreateOrUpdateChassis creates or updates the chassis record along with the encap record
-func CreateOrUpdateChassis(sbClient libovsdbclient.Client, chassis *sbdb.Chassis, encap *sbdb.Encap) error {
+func CreateOrUpdateChassis(sbClient libovsdbclient.Client, chassis *sbdb.Chassis, encaps ...*sbdb.Encap) error {
 	m := newModelClient(sbClient)
-	opModels := []operationModel{
-		{
+	opModels := make([]operationModel, 0, len(encaps)+1)
+	for i := range encaps {
+		encap := encaps[i]
+		opModel := operationModel{
 			Model: encap,
 			DoAfter: func() {
-				chassis.Encaps = []string{encap.UUID}
+				encapsList := append(chassis.Encaps, encap.UUID)
+				chassis.Encaps = sets.New(encapsList...).UnsortedList()
 			},
-			OnModelUpdates: onModelUpdatesAllNonDefault(),
+			OnModelUpdates: onModelUpdatesNone(),
 			ErrNotFound:    false,
 			BulkOp:         false,
-		},
-		{
-			Model:            chassis,
-			OnModelMutations: []interface{}{&chassis.OtherConfig},
-			OnModelUpdates:   []interface{}{&chassis.Encaps},
-			ErrNotFound:      false,
-			BulkOp:           false,
-		},
+		}
+		opModels = append(opModels, opModel)
+	}
+
+	opModel := operationModel{
+		Model:            chassis,
+		OnModelMutations: []interface{}{&chassis.OtherConfig},
+		OnModelUpdates:   []interface{}{&chassis.Encaps},
+		ErrNotFound:      false,
+		BulkOp:           false,
 	}
 
+	opModels = append(opModels, opModel)
 	if _, err := m.CreateOrUpdate(opModels...); err != nil {
 		return err
 	}

go-controller/pkg/libovsdb/ops/chassis_test.go

@@ -181,3 +181,97 @@ func TestDeleteChassis(t *testing.T) {
 		})
 	}
 }
+
+func TestCreateOrUpdateChassis(t *testing.T) {
+	uuid1 := "b9998337-2498-4d1e-86e6-fc0417abb2f0"
+	uuid2 := "b9998337-2498-4d1e-86e6-fc0417abb2f1"
+	uuid3 := "b9998337-2498-4d1e-86e6-fc0417abb2f2"
+	tests := []struct {
+		desc       string
+		chassis    *sbdb.Chassis
+		encaps     []*sbdb.Encap
+		initialDB  []libovsdbtest.TestData
+		expectedDB []libovsdbtest.TestData
+	}{
+		{
+			desc:    "create new chassis with encap records",
+			chassis: &sbdb.Chassis{Name: "test1"},
+			encaps: []*sbdb.Encap{{ChassisName: "test1", IP: "10.0.0.10", Type: "geneve"},
+				{ChassisName: "test1", IP: "10.0.0.11", Type: "geneve"}},
+			initialDB: []libovsdbtest.TestData{},
+			expectedDB: []libovsdbtest.TestData{
+				&sbdb.Chassis{UUID: uuid1, Name: "test1", Encaps: []string{uuid2, uuid3}},
+				&sbdb.Encap{UUID: uuid2, ChassisName: "test1", IP: "10.0.0.10", Type: "geneve"},
+				&sbdb.Encap{UUID: uuid3, ChassisName: "test1", IP: "10.0.0.11", Type: "geneve"},
+			},
+		},
+		{
+			desc:    "update chassis by inserting new encap record",
+			chassis: &sbdb.Chassis{Name: "test2"},
+			encaps: []*sbdb.Encap{{ChassisName: "test2", IP: "10.0.0.10", Type: "geneve"},
+				{ChassisName: "test2", IP: "10.0.0.11", Type: "geneve"}},
+			initialDB: []libovsdbtest.TestData{
+				&sbdb.Chassis{UUID: uuid1, Name: "test2", Encaps: []string{uuid2}},
+				&sbdb.Encap{UUID: uuid2, ChassisName: "test2", IP: "10.0.0.10", Type: "geneve"},
+			},
+			expectedDB: []libovsdbtest.TestData{
+				&sbdb.Chassis{UUID: uuid1, Name: "test2", Encaps: []string{uuid2, uuid3}},
+				&sbdb.Encap{UUID: uuid2, ChassisName: "test2", IP: "10.0.0.10", Type: "geneve"},
+				&sbdb.Encap{UUID: uuid3, ChassisName: "test2", IP: "10.0.0.11", Type: "geneve"},
+			},
+		},
+		{
+			desc:    "update chassis by removing obsolete encap record",
+			chassis: &sbdb.Chassis{Name: "test3"},
+			encaps:  []*sbdb.Encap{{ChassisName: "test3", IP: "10.0.0.11", Type: "geneve"}},
+			initialDB: []libovsdbtest.TestData{
+				&sbdb.Chassis{UUID: uuid1, Name: "test3", Encaps: []string{uuid2, uuid3}},
+				&sbdb.Encap{UUID: uuid2, ChassisName: "test3", IP: "10.0.0.10", Type: "geneve"},
+				&sbdb.Encap{UUID: uuid3, ChassisName: "test3", IP: "10.0.0.11", Type: "geneve"},
+			},
+			expectedDB: []libovsdbtest.TestData{
+				&sbdb.Chassis{UUID: uuid1, Name: "test3", Encaps: []string{uuid3}},
+				&sbdb.Encap{UUID: uuid3, ChassisName: "test3", IP: "10.0.0.11", Type: "geneve"},
+			},
+		},
+		{
+			desc:    "update chassis by adding new encap record and deleting the old one",
+			chassis: &sbdb.Chassis{Name: "test4"},
+			encaps:  []*sbdb.Encap{{ChassisName: "test4", IP: "10.0.0.11", Type: "geneve"}},
+			initialDB: []libovsdbtest.TestData{
+				&sbdb.Chassis{UUID: uuid1, Name: "test4", Encaps: []string{uuid2}},
+				&sbdb.Encap{UUID: uuid2, ChassisName: "test4", IP: "10.0.0.10", Type: "geneve"},
+			},
+			expectedDB: []libovsdbtest.TestData{
+				&sbdb.Chassis{UUID: uuid1, Name: "test4", Encaps: []string{uuid3}},
+				&sbdb.Encap{UUID: uuid3, ChassisName: "test4", IP: "10.0.0.11", Type: "geneve"},
+			},
+		},
+	}
+	for _, tt := range tests {
+		t.Run(tt.desc, func(t *testing.T) {
+			dbSetup := libovsdbtest.TestSetup{
+				SBData: tt.initialDB,
+			}
+			sbClient, cleanup, err := libovsdbtest.NewSBTestHarness(dbSetup, nil)
+			if err != nil {
+				t.Fatalf("%s: failed to set up test harness: %v", tt.desc, err)
+			}
+			t.Cleanup(cleanup.Cleanup)
+
+			err = CreateOrUpdateChassis(sbClient, tt.chassis, tt.encaps...)
+			if err != nil {
+				t.Fatal(fmt.Errorf("%s: got unexpected error: %v", tt.desc, err))
+			}
+
+			matcher := libovsdbtest.HaveDataIgnoringUUIDs(tt.expectedDB)
+			match, err := matcher.Match(sbClient)
+			if err != nil {
+				t.Fatalf("%s: matcher error: %v", tt.desc, err)
+			}
+			if !match {
+				t.Fatalf("%s: DB state did not match: %s", tt.desc, matcher.FailureMessage(sbClient))
+			}
+		})
+	}
+}

go-controller/pkg/node/default_node_network_controller.go

@@ -20,6 +20,7 @@ import (
 	apierrors "k8s.io/apimachinery/pkg/api/errors"
 	"k8s.io/apimachinery/pkg/labels"
 	"k8s.io/apimachinery/pkg/selection"
+	"k8s.io/apimachinery/pkg/util/sets"
 	"k8s.io/apimachinery/pkg/util/wait"
 	clientset "k8s.io/client-go/kubernetes"
 	"k8s.io/client-go/tools/record"
@@ -925,6 +926,13 @@ func (nc *DefaultNodeNetworkController) Init(ctx context.Context) error {
 	if err := util.SetNodeZone(nodeAnnotator, sbZone); err != nil {
 		return fmt.Errorf("failed to set node zone annotation for node %s: %w", nc.name, err)
 	}
+
+	encapIPList := sets.New[string]()
+	encapIPList.Insert(strings.Split(config.Default.EffectiveEncapIP, ",")...)
+	if err := util.SetNodeEncapIPs(nodeAnnotator, encapIPList); err != nil {
+		return fmt.Errorf("failed to set node-encap-ips annotation for node %s: %w", nc.name, err)
+	}
+
 	if err := nodeAnnotator.Run(); err != nil {
 		return fmt.Errorf("failed to set node %s annotations: %w", nc.name, err)
 	}

go-controller/pkg/node/gateway_shared_intf.go

@@ -1937,10 +1937,9 @@ func commonFlows(hostSubnets []*net.IPNet, bridge *bridgeConfiguration) ([]strin
 							defaultOpenFlowCookie, netConfig.ofPortPatch, bridgeMacAddress, config.Default.ConntrackZone,
 							netConfig.masqCTMark, ofPortPhys))
 
-					// Allow (a) OVN->host traffic on the same node
-					// (b) host->host traffic on the same node
+					// Allow OVN->Host traffic on the same node
 					if config.Gateway.Mode == config.GatewayModeShared || config.Gateway.Mode == config.GatewayModeLocal {
-						dftFlows = append(dftFlows, hostNetworkNormalActionFlows(netConfig, bridgeMacAddress, hostSubnets, false)...)
+						dftFlows = append(dftFlows, ovnToHostNetworkNormalActionFlows(netConfig, bridgeMacAddress, hostSubnets, false)...)
 					}
 				} else {
 					//  for UDN we additionally SNAT the packet from masquerade IP -> node IP
@@ -2034,10 +2033,9 @@ 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 (a) OVN->host traffic on the same node
-					// (b) host->host traffic on the same node
+					// Allow OVN->Host traffic on the same node
 					if config.Gateway.Mode == config.GatewayModeShared || config.Gateway.Mode == config.GatewayModeLocal {
-						dftFlows = append(dftFlows, hostNetworkNormalActionFlows(netConfig, bridgeMacAddress, hostSubnets, true)...)
+						dftFlows = append(dftFlows, ovnToHostNetworkNormalActionFlows(netConfig, bridgeMacAddress, hostSubnets, true)...)
 					}
 				} else {
 					//  for UDN we additionally SNAT the packet from masquerade IP -> node IP
@@ -2217,15 +2215,23 @@ func commonFlows(hostSubnets []*net.IPNet, bridge *bridgeConfiguration) ([]strin
 	return dftFlows, nil
 }
 
-// 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 {
+// 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
 	var flows []string
-	var ipFamily, ipFamilyDest 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
+	}
 
 	if isV6 {
 		ipFamily = "ipv6"
@@ -2235,69 +2241,38 @@ func hostNetworkNormalActionFlows(netConfig *bridgeUDNConfiguration, srcMAC stri
 		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 utilnet.IsIPv6(hostSubnet.IP) != isV6 {
+		if (hostSubnet.IP.To4() == nil) != isV6 {
 			continue
 		}
-		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,
+		// 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",
 				defaultOpenFlowCookie,
 				inPort,
 				srcMAC,
-				icmpType,
+				ipFamily,
+				ipFamilyDest,
+				hostSubnet.String(),
 				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} {
-			// 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))
+			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))
 		}
 	}
+
 	return flows
 }
 

go-controller/pkg/ovn/default_network_controller.go

@@ -943,6 +943,8 @@ func (h *defaultNetworkControllerEventHandler) UpdateResource(oldObj, newObj int
 		newNodeIsLocalZoneNode := h.oc.isLocalZoneNode(newNode)
 		zoneClusterChanged := h.oc.nodeZoneClusterChanged(oldNode, newNode, newNodeIsLocalZoneNode, types.DefaultNetworkName)
 		nodeSubnetChange := nodeSubnetChanged(oldNode, newNode, types.DefaultNetworkName)
+		nodeEncapIPsChanged := util.NodeEncapIPsChanged(oldNode, newNode)
+
 		var aggregatedErrors []error
 		if newNodeIsLocalZoneNode {
 			var nodeSyncsParam *nodeSyncs
@@ -989,7 +991,8 @@ func (h *defaultNetworkControllerEventHandler) UpdateResource(oldObj, newObj int
 			// Check if the node moved from local zone to remote zone and if so syncZoneIC should be set to true.
 			// Also check if node subnet changed, so static routes are properly set
 			// Also check if the node is used to be a hybrid overlay node
-			syncZoneIC = syncZoneIC || h.oc.isLocalZoneNode(oldNode) || nodeSubnetChange || zoneClusterChanged || primaryAddrChanged(oldNode, newNode) || switchToOvnNode
+			syncZoneIC = syncZoneIC || h.oc.isLocalZoneNode(oldNode) || nodeSubnetChange || zoneClusterChanged ||
+				switchToOvnNode || nodeEncapIPsChanged
 			if syncZoneIC {
 				klog.Infof("Node %s in remote zone %s needs interconnect zone sync up. Zone cluster changed: %v",
 					newNode.Name, util.GetNodeZone(newNode), zoneClusterChanged)

go-controller/pkg/ovn/zone_interconnect/chassis_handler.go

@@ -134,9 +134,29 @@ func (zch *ZoneChassisHandler) createOrUpdateNodeChassis(node *corev1.Node, isRe
 			node.Name, parsedErr)
 	}
 
-	nodePrimaryIp, err := util.GetNodePrimaryIP(node)
+	// Get the encap IPs.
+	encapIPs, err := util.ParseNodeEncapIPsAnnotation(node)
 	if err != nil {
-		return fmt.Errorf("failed to parse node %s primary IP %w", node.Name, err)
+		return fmt.Errorf("failed to parse node-encap-ips for node - %s, error: %w",
+			node.Name, err)
+	}
+
+	encaps := make([]*sbdb.Encap, 0, len(encapIPs))
+	encapOptions := map[string]string{}
+	encapOptions["csum"] = "true"
+	// set the geneve port if using something else than default
+	if config.Default.EncapPort != config.DefaultEncapPort {
+		encapOptions["dst_port"] = strconv.FormatUint(uint64(config.Default.EncapPort), 10)
+	}
+
+	for _, ovnEncapIP := range encapIPs {
+		encap := sbdb.Encap{
+			ChassisName: chassisID,
+			IP:          strings.TrimSpace(ovnEncapIP),
+			Type:        "geneve",
+			Options:     encapOptions,
+		}
+		encaps = append(encaps, &encap)
 	}
 
 	chassis := sbdb.Chassis{
@@ -147,17 +167,5 @@ func (zch *ZoneChassisHandler) createOrUpdateNodeChassis(node *corev1.Node, isRe
 		},
 	}
 
-	encap := sbdb.Encap{
-		ChassisName: chassisID,
-		IP:          nodePrimaryIp,
-		Type:        "geneve",
-		Options:     map[string]string{"csum": "true"},
-	}
-
-	// set the geneve port if using something else than default
-	if config.Default.EncapPort != config.DefaultEncapPort {
-		encap.Options["dst_port"] = strconv.FormatUint(uint64(config.Default.EncapPort), 10)
-	}
-
-	return libovsdbops.CreateOrUpdateChassis(zch.sbClient, &chassis, &encap)
+	return libovsdbops.CreateOrUpdateChassis(zch.sbClient, &chassis, encaps...)
 }