Preserve client IP if the selected Endpoint is local regardless of Ex…

…ternalTrafficPolicy (#3604)

When an external client accesses to a NodePort / LoadBalancer Service on a K8s
Node, if the selected Endpoint is just on the K8s Node, then don't SNAT the
connection even the externalTrafficPolicy of the Service is Cluster.

Signed-off-by: Hongliang Liu <lhongliang@vmware.com>

pkg/agent/openflow/client.go

@@ -715,6 +715,7 @@ func (c *client) generatePipelines() {
 	c.featureService = newFeatureService(c.cookieAllocator,
 		c.ipProtocols,
 		c.nodeConfig,
+		c.networkConfig,
 		c.serviceConfig,
 		c.bridge,
 		c.enableProxy,

pkg/agent/openflow/fields.go

@@ -49,11 +49,7 @@ var (
 	PktDestinationField = binding.NewRegField(0, 4, 7, "PacketDestination")
 	ToTunnelRegMark     = binding.NewRegMark(PktDestinationField, tunnelVal)
 	ToGatewayRegMark    = binding.NewRegMark(PktDestinationField, gatewayVal)
-	// reg0[0..7]: Union field of the packet source and destination. It is used to mark hairpin packets. Marks in this
-	// field include:
-	//  - 0x11: the packet sourced from Antrea gateway interface, and destined for local Node via Antrea gateway interface.
-	PktUnionField         = binding.NewRegField(0, 0, 7, "PacketUnion")
-	GatewayHairpinRegMark = binding.NewRegMark(PktUnionField, (ToGatewayRegMark.GetValue()<<ToGatewayRegMark.GetField().GetRange().Offset())|FromGatewayRegMark.GetValue())
+	ToUplinkRegMark     = binding.NewRegMark(PktDestinationField, uplinkVal)
 	// reg0[8]: Mark to indicate the ofPort number of an interface is found.
 	OFPortFoundRegMark = binding.NewOneBitRegMark(0, 8, "OFPortFound")
 	// reg0[9]: Field to indicate whether the packet's source / destination MAC address needs to be rewritten.

pkg/agent/openflow/pipeline.go

@@ -1385,6 +1385,7 @@ func (f *featurePodConnectivity) l3FwdFlowToRemoteViaUplink(remoteGatewayMAC net
 			MatchDstIPNet(peerSubnet).
 			Action().SetSrcMAC(f.nodeConfig.UplinkNetConfig.MAC).
 			Action().SetDstMAC(remoteGatewayMAC).
+			Action().LoadRegMark(ToUplinkRegMark).
 			Action().GotoTable(L3DecTTLTable.GetID()).
 			Done()
 	}
@@ -1398,6 +1399,7 @@ func (f *featurePodConnectivity) l3FwdFlowToRemoteViaUplink(remoteGatewayMAC net
 		MatchRegMark(AntreaFlexibleIPAMRegMark).
 		MatchDstIPNet(peerSubnet).
 		Action().SetDstMAC(remoteGatewayMAC).
+		Action().LoadRegMark(ToUplinkRegMark).
 		Action().GotoTable(L3DecTTLTable.GetID()).
 		Done()
 }
@@ -2180,6 +2182,12 @@ func (f *featureNetworkPolicy) ingressClassifierFlows() []binding.Flow {
 			MatchRegMark(ToTunnelRegMark).
 			Action().GotoTable(IngressMetricTable.GetID()).
 			Done(),
+		// This generates the flow to match the packets to uplink and forward them to IngressMetricTable.
+		IngressSecurityClassifierTable.ofTable.BuildFlow(priorityNormal).
+			Cookie(cookieID).
+			MatchRegMark(ToUplinkRegMark).
+			Action().GotoTable(IngressMetricTable.GetID()).
+			Done(),
 	}
 }
 
@@ -2933,33 +2941,41 @@ func (f *featureService) gatewaySNATFlows() []binding.Flow {
 	cookieID := f.cookieAllocator.Request(f.category).Raw()
 	var flows []binding.Flow
 	for _, ipProtocol := range f.ipProtocols {
-		flows = append(flows,
-			// This generates the flow to match the first packet of hairpin connection initiated through the Antrea gateway.
-			// ConnSNATCTMark and HairpinCTMark will be loaded in DNAT CT zone.
-			ServiceMarkTable.ofTable.BuildFlow(priorityNormal).
-				Cookie(cookieID).
-				MatchProtocol(ipProtocol).
-				MatchCTStateNew(true).
-				MatchCTStateTrk(true).
-				MatchRegMark(GatewayHairpinRegMark).
-				Action().CT(true, ServiceMarkTable.GetNext(), f.dnatCtZones[ipProtocol], f.ctZoneSrcField).
-				LoadToCtMark(ConnSNATCTMark, HairpinCTMark).
-				CTDone().
-				Done(),
+		// This generates the flow to match the first packet of hairpin connection initiated through the Antrea gateway.
+		// ConnSNATCTMark and HairpinCTMark will be loaded in DNAT CT zone.
+		flows = append(flows, ServiceMarkTable.ofTable.BuildFlow(priorityNormal).
+			Cookie(cookieID).
+			MatchProtocol(ipProtocol).
+			MatchCTStateNew(true).
+			MatchCTStateTrk(true).
+			MatchRegMark(FromGatewayRegMark, ToGatewayRegMark).
+			Action().CT(true, ServiceMarkTable.GetNext(), f.dnatCtZones[ipProtocol], f.ctZoneSrcField).
+			LoadToCtMark(ConnSNATCTMark, HairpinCTMark).
+			CTDone().
+			Done())
+
+		var pktDstRegMarks []*binding.RegMark
+		if f.networkConfig.TrafficEncapMode.SupportsEncap() {
+			pktDstRegMarks = append(pktDstRegMarks, ToTunnelRegMark)
+		}
+		if f.networkConfig.TrafficEncapMode.SupportsNoEncap() && runtime.IsWindowsPlatform() {
+			pktDstRegMarks = append(pktDstRegMarks, ToUplinkRegMark)
+		}
+		for _, pktDstRegMark := range pktDstRegMarks {
 			// This generates the flow to match the first packet of NodePort / LoadBalancer connection initiated through the
-			// Antrea gateway and externalTrafficPolicy of the Service is Cluster. ConnSNATCTMark will be loaded in DNAT
-			// CT zone.
-			ServiceMarkTable.ofTable.BuildFlow(priorityLow).
+			// Antrea gateway and externalTrafficPolicy of the Service is Cluster, and the selected Endpoint is on a remote
+			// Node, then ConnSNATCTMark will be loaded in DNAT CT zone, indicating that SNAT is required for the connection.
+			flows = append(flows, ServiceMarkTable.ofTable.BuildFlow(priorityNormal).
 				Cookie(cookieID).
 				MatchProtocol(ipProtocol).
 				MatchCTStateNew(true).
 				MatchCTStateTrk(true).
-				MatchRegMark(FromGatewayRegMark, ToClusterServiceRegMark).
+				MatchRegMark(FromGatewayRegMark, pktDstRegMark, ToClusterServiceRegMark).
 				Action().CT(true, ServiceMarkTable.GetNext(), f.dnatCtZones[ipProtocol], f.ctZoneSrcField).
 				LoadToCtMark(ConnSNATCTMark).
 				CTDone().
-				Done(),
-		)
+				Done())
+		}
 	}
 
 	return flows

pkg/agent/openflow/service.go

@@ -40,6 +40,7 @@ type featureService struct {
 	gatewayMAC        net.HardwareAddr
 	nodePortAddresses map[binding.Protocol][]net.IP
 	serviceCIDRs      map[binding.Protocol]net.IPNet
+	networkConfig     *config.NetworkConfig
 
 	enableProxy           bool
 	proxyAll              bool
@@ -57,6 +58,7 @@ func newFeatureService(
 	cookieAllocator cookie.Allocator,
 	ipProtocols []binding.Protocol,
 	nodeConfig *config.NodeConfig,
+	networkConfig *config.NetworkConfig,
 	serviceConfig *config.ServiceConfig,
 	bridge binding.Bridge,
 	enableProxy,
@@ -103,6 +105,7 @@ func newFeatureService(
 		nodePortAddresses:     nodePortAddresses,
 		serviceCIDRs:          serviceCIDRs,
 		gatewayMAC:            nodeConfig.GatewayConfig.MAC,
+		networkConfig:         networkConfig,
 		enableProxy:           enableProxy,
 		proxyAll:              proxyAll,
 		connectUplinkToBridge: connectUplinkToBridge,

test/integration/agent/openflow_test.go

@@ -1303,7 +1303,7 @@ func prepareNodeFlows(peerSubnet net.IPNet, peerGwIP, peerNodeIP net.IP, vMAC, l
 			[]*ofTestUtils.ExpectFlow{
 				{
 					MatchStr: fmt.Sprintf("priority=200,%s,reg4=0x100000/0x100000,reg8=0/0xfff,%s=%s", ipProtoStr, nwDstFieldName, peerSubnet.String()),
-					ActStr:   fmt.Sprintf("set_field:%s->eth_dst,goto_table:L3DecTTL", peerNodeMAC.String()),
+					ActStr:   fmt.Sprintf("set_field:%s->eth_dst,load:0x4->NXM_NX_REG0[4..7],goto_table:L3DecTTL", peerNodeMAC.String()),
 				},
 			},
 		})
@@ -1416,7 +1416,7 @@ func prepareDefaultFlows(config *testConfig) []expectTableFlows {
 		)
 		tableServiceMarkFlows.flows = append(tableServiceMarkFlows.flows,
 			&ofTestUtils.ExpectFlow{MatchStr: "priority=200,ct_state=+new+trk,ip,reg0=0x22/0xff", ActStr: fmt.Sprintf("ct(commit,table=SNATConntrackCommit,zone=%s,exec(load:0x1->NXM_NX_CT_MARK[5],load:0x1->NXM_NX_CT_MARK[6]))", ctZone)},
-			&ofTestUtils.ExpectFlow{MatchStr: "priority=190,ct_state=+new+trk,ip,reg0=0x2/0xf,reg4=0x200000/0x200000", ActStr: fmt.Sprintf("ct(commit,table=SNATConntrackCommit,zone=%s,exec(load:0x1->NXM_NX_CT_MARK[5]))", ctZone)},
+			&ofTestUtils.ExpectFlow{MatchStr: "priority=200,ct_state=+new+trk,ip,reg0=0x12/0xff,reg4=0x200000/0x200000", ActStr: fmt.Sprintf("ct(commit,table=SNATConntrackCommit,zone=%s,exec(load:0x1->NXM_NX_CT_MARK[5]))", ctZone)},
 		)
 		tableL3DecTTLFlows.flows = append(tableL3DecTTLFlows.flows,
 			&ofTestUtils.ExpectFlow{MatchStr: "priority=210,ip,reg0=0x2/0xf", ActStr: "goto_table:ServiceMark"},
@@ -1461,7 +1461,7 @@ func prepareDefaultFlows(config *testConfig) []expectTableFlows {
 		)
 		tableServiceMarkFlows.flows = append(tableServiceMarkFlows.flows,
 			&ofTestUtils.ExpectFlow{MatchStr: "priority=200,ct_state=+new+trk,ipv6,reg0=0x22/0xff", ActStr: "ct(commit,table=SNATConntrackCommit,zone=65510,exec(load:0x1->NXM_NX_CT_MARK[5],load:0x1->NXM_NX_CT_MARK[6]))"},
-			&ofTestUtils.ExpectFlow{MatchStr: "priority=190,ct_state=+new+trk,ipv6,reg0=0x2/0xf,reg4=0x200000/0x200000", ActStr: "ct(commit,table=SNATConntrackCommit,zone=65510,exec(load:0x1->NXM_NX_CT_MARK[5]))"},
+			&ofTestUtils.ExpectFlow{MatchStr: "priority=200,ct_state=+new+trk,ipv6,reg0=0x12/0xff,reg4=0x200000/0x200000", ActStr: "ct(commit,table=SNATConntrackCommit,zone=65510,exec(load:0x1->NXM_NX_CT_MARK[5]))"},
 		)
 		tableL3DecTTLFlows.flows = append(tableL3DecTTLFlows.flows,
 			&ofTestUtils.ExpectFlow{MatchStr: "priority=210,ipv6,reg0=0x2/0xf", ActStr: "goto_table:ServiceMark"},