Remove table ServiceHairpinTable and HairpinSNATTable

Since a SNAT ct zone is added in PR antrea-io#2599, hairpin Service traffic can make use of the SNAT ct zone instead of current stateless SNAT by modifying source and destination IPs. By removing hairpin table ServiceHairpinTable and HairpinSNATTable, the OVS pipeline can be simpler. Pipeline modifications: - Remove table serviceHairpinTable #23. - Remove table HairpinSNATTable #108. - Add table hairpinMarkTable #81 after table l2ForwardingCalcTable #80. When a local Endpoint is referenced by a Service, then a flow that matches the packet whose input and output interfaces are the same and makes mark HairpinRegMark, will be installed on table hairpinMarkTable. Packets with mark HairpinRegMark will be performed SNAT with Antrea gateway IP on table snatConntrackCommitTable. Signed-off-by: Hongliang Liu <[email protected]>
hongliangl · Sep 29, 2021 · 2d515eb · 2d515eb
1 parent 5480b68
commit 2d515eb
Show file tree

Hide file tree

Showing 7 changed files with 186 additions and 195 deletions.
diff --git a/cmd/antrea-agent/agent.go b/cmd/antrea-agent/agent.go
@@ -100,10 +100,16 @@ func run(o *Options) error {
 	}
 	defer ovsdbConnection.Close()
 
+	// Create an ifaceStore that caches network interfaces managed by this node.
+	ifaceStore := interfacestore.NewInterfaceStore()
+
 	ovsDatapathType := ovsconfig.OVSDatapathType(o.config.OVSDatapathType)
 	ovsBridgeClient := ovsconfig.NewOVSBridge(o.config.OVSBridge, ovsDatapathType, ovsdbConnection)
 	ovsBridgeMgmtAddr := ofconfig.GetMgmtAddress(o.config.OVSRunDir, o.config.OVSBridge)
-	ofClient := openflow.NewClient(o.config.OVSBridge, ovsBridgeMgmtAddr, ovsDatapathType,
+	ofClient := openflow.NewClient(o.config.OVSBridge,
+		ovsBridgeMgmtAddr,
+		ovsDatapathType,
+		ifaceStore,
 		features.DefaultFeatureGate.Enabled(features.AntreaProxy),
 		features.DefaultFeatureGate.Enabled(features.AntreaPolicy),
 		features.DefaultFeatureGate.Enabled(features.Egress),
@@ -139,9 +145,6 @@ func run(o *Options) error {
 		return fmt.Errorf("error creating route client: %v", err)
 	}
 
-	// Create an ifaceStore that caches network interfaces managed by this node.
-	ifaceStore := interfacestore.NewInterfaceStore()
-
 	// networkReadyCh is used to notify that the Node's network is ready.
 	// Functions that rely on the Node's network should wait for the channel to close.
 	networkReadyCh := make(chan struct{})

diff --git a/pkg/agent/openflow/client.go b/pkg/agent/openflow/client.go
@@ -467,11 +467,15 @@ func (c *client) UninstallNodeFlows(hostname string) error {
 func (c *client) InstallPodFlows(interfaceName string, podInterfaceIPs []net.IP, podInterfaceMAC net.HardwareAddr, ofPort uint32) error {
 	c.replayMutex.RLock()
 	defer c.replayMutex.RUnlock()
-
 	localGatewayMAC := c.nodeConfig.GatewayConfig.MAC
+	skipHairpin := false
+	if !c.enableProxy {
+		// If AntreaProxy is disabled, flows on table l2ForwardingCalcTable should skip table hairpinMarkTable.
+		skipHairpin = true
+	}
 	flows := []binding.Flow{
 		c.podClassifierFlow(ofPort, cookie.Pod),
-		c.l2ForwardCalcFlow(podInterfaceMAC, ofPort, false, cookie.Pod),
+		c.l2ForwardCalcFlow(podInterfaceMAC, ofPort, skipHairpin, cookie.Pod),
 	}
 
 	// Add support for IPv4 ARP responder.
@@ -566,7 +570,11 @@ func (c *client) InstallEndpointFlows(protocol binding.Protocol, endpoints []pro
 		cacheKey := generateEndpointFlowCacheKey(endpoint.IP(), endpointPort, protocol)
 		flows = append(flows, c.endpointDNATFlow(endpointIP, portVal, protocol))
 		if endpoint.GetIsLocal() {
-			flows = append(flows, c.hairpinSNATFlow(endpointIP))
+			ofPort, ok := c.ifaceStore.GetInterfaceByIP(endpointIP.String())
+			if !ok {
+				return fmt.Errorf("there is no OVS port with such IP %s", endpointIP.String())
+			}
+			flows = append(flows, c.hairpinMarkFlow(getIPProtocol(endpointIP), uint32(ofPort.OFPort), false))
 		}
 		keyToFlows[cacheKey] = flows
 	}
@@ -623,26 +631,18 @@ func (c *client) InstallDefaultServiceFlows(nodePortAddressesIPv4, nodePortAddre
 		c.l2ForwardOutputServiceHairpinFlow(),
 	}
 	if c.IsIPv4Enabled() {
-		flows = append(flows, c.serviceHairpinResponseDNATFlow(binding.ProtocolIP))
 		flows = append(flows, c.serviceLBBypassFlows(binding.ProtocolIP)...)
 		flows = append(flows, c.l3FwdServiceDefaultFlowsViaGW(binding.ProtocolIP, cookie.Service)...)
 		if c.proxyAll {
-			// The output interface of a packet is the same as where it is from, and the action of the packet should be
-			// IN_PORT, rather than output. When a packet of Service is from Antrea gateway and its Endpoint is on host
-			// network, it needs hairpin mark (by setting a register, it will be matched at table L2ForwardingOutTable).
-			flows = append(flows, c.serviceHairpinRegSetFlows(binding.ProtocolIP))
 			// These flows are used to match the first packet of NodePort. The flows will set a bit of a register to mark
 			// the Service type of the packet as NodePort. The mark will be consumed in table serviceLBTable to match NodePort
 			flows = append(flows, c.serviceClassifierFlows(nodePortAddressesIPv4, binding.ProtocolIP)...)
 		}
 	}
 	if c.IsIPv6Enabled() {
-		flows = append(flows, c.serviceHairpinResponseDNATFlow(binding.ProtocolIPv6))
 		flows = append(flows, c.serviceLBBypassFlows(binding.ProtocolIPv6)...)
 		flows = append(flows, c.l3FwdServiceDefaultFlowsViaGW(binding.ProtocolIPv6, cookie.Service)...)
 		if c.proxyAll {
-			// As IPv4 above.
-			flows = append(flows, c.serviceHairpinRegSetFlows(binding.ProtocolIPv6))
 			// As IPv4 above.
 			flows = append(flows, c.serviceClassifierFlows(nodePortAddressesIPv6, binding.ProtocolIPv6)...)
 		}
@@ -666,20 +666,30 @@ func (c *client) InstallClusterServiceCIDRFlows(serviceNets []*net.IPNet) error
 func (c *client) InstallGatewayFlows() error {
 	gatewayConfig := c.nodeConfig.GatewayConfig
 	gatewayIPs := []net.IP{}
-
+	skipHairpin := false
+	if !c.enableProxy {
+		// If AntreaProxy is disabled, flows on table l2ForwardingCalcTable should skip table hairpinMarkTable.
+		skipHairpin = true
+	}
 	flows := []binding.Flow{
 		c.gatewayClassifierFlow(cookie.Default),
-		c.l2ForwardCalcFlow(gatewayConfig.MAC, config.HostGatewayOFPort, true, cookie.Default),
+		c.l2ForwardCalcFlow(gatewayConfig.MAC, config.HostGatewayOFPort, skipHairpin, cookie.Default),
 	}
 	flows = append(flows, c.gatewayIPSpoofGuardFlows(cookie.Default)...)
 
 	// Add ARP SpoofGuard flow for local gateway interface.
 	if gatewayConfig.IPv4 != nil {
 		gatewayIPs = append(gatewayIPs, gatewayConfig.IPv4)
 		flows = append(flows, c.gatewayARPSpoofGuardFlow(gatewayConfig.IPv4, gatewayConfig.MAC, cookie.Default))
+		if c.enableProxy {
+			flows = append(flows, c.hairpinMarkFlow(binding.ProtocolIP, uint32(config.HostGatewayOFPort), true))
+		}
 	}
 	if gatewayConfig.IPv6 != nil {
 		gatewayIPs = append(gatewayIPs, gatewayConfig.IPv6)
+		if c.enableProxy {
+			flows = append(flows, c.hairpinMarkFlow(binding.ProtocolIPv6, uint32(config.HostGatewayOFPort), true))
+		}
 	}
 
 	// Add flow to ensure the liveness check packet could be forwarded correctly.

diff --git a/pkg/agent/openflow/client_test.go b/pkg/agent/openflow/client_test.go
@@ -103,7 +103,7 @@ func TestIdempotentFlowInstallation(t *testing.T) {
 			ctrl := gomock.NewController(t)
 			defer ctrl.Finish()
 			m := oftest.NewMockOFEntryOperations(ctrl)
-			ofClient := NewClient(bridgeName, bridgeMgmtAddr, ovsconfig.OVSDatapathSystem, true, false, false, false, false)
+			ofClient := NewClient(bridgeName, bridgeMgmtAddr, ovsconfig.OVSDatapathSystem, nil, true, false, false, false, false)
 			client := ofClient.(*client)
 			client.cookieAllocator = cookie.NewAllocator(0)
 			client.ofEntryOperations = m
@@ -132,7 +132,7 @@ func TestIdempotentFlowInstallation(t *testing.T) {
 			ctrl := gomock.NewController(t)
 			defer ctrl.Finish()
 			m := oftest.NewMockOFEntryOperations(ctrl)
-			ofClient := NewClient(bridgeName, bridgeMgmtAddr, ovsconfig.OVSDatapathSystem, true, false, false, false, false)
+			ofClient := NewClient(bridgeName, bridgeMgmtAddr, ovsconfig.OVSDatapathSystem, nil, true, false, false, false, false)
 			client := ofClient.(*client)
 			client.cookieAllocator = cookie.NewAllocator(0)
 			client.ofEntryOperations = m
@@ -174,7 +174,7 @@ func TestFlowInstallationFailed(t *testing.T) {
 			ctrl := gomock.NewController(t)
 			defer ctrl.Finish()
 			m := oftest.NewMockOFEntryOperations(ctrl)
-			ofClient := NewClient(bridgeName, bridgeMgmtAddr, ovsconfig.OVSDatapathSystem, true, false, false, false, false)
+			ofClient := NewClient(bridgeName, bridgeMgmtAddr, ovsconfig.OVSDatapathSystem, nil, true, false, false, false, false)
 			client := ofClient.(*client)
 			client.cookieAllocator = cookie.NewAllocator(0)
 			client.ofEntryOperations = m
@@ -209,7 +209,7 @@ func TestConcurrentFlowInstallation(t *testing.T) {
 			ctrl := gomock.NewController(t)
 			defer ctrl.Finish()
 			m := oftest.NewMockOFEntryOperations(ctrl)
-			ofClient := NewClient(bridgeName, bridgeMgmtAddr, ovsconfig.OVSDatapathSystem, true, false, false, false, false)
+			ofClient := NewClient(bridgeName, bridgeMgmtAddr, ovsconfig.OVSDatapathSystem, nil, true, false, false, false, false)
 			client := ofClient.(*client)
 			client.cookieAllocator = cookie.NewAllocator(0)
 			client.ofEntryOperations = m
@@ -400,7 +400,7 @@ func Test_client_SendTraceflowPacket(t *testing.T) {
 }
 
 func prepareTraceflowFlow(ctrl *gomock.Controller) *client {
-	ofClient := NewClient(bridgeName, bridgeMgmtAddr, ovsconfig.OVSDatapathSystem, true, true, false, false, false)
+	ofClient := NewClient(bridgeName, bridgeMgmtAddr, ovsconfig.OVSDatapathSystem, nil, true, true, false, false, false)
 	c := ofClient.(*client)
 	c.cookieAllocator = cookie.NewAllocator(0)
 	c.nodeConfig = nodeConfig
@@ -418,7 +418,7 @@ func prepareTraceflowFlow(ctrl *gomock.Controller) *client {
 }
 
 func prepareSendTraceflowPacket(ctrl *gomock.Controller, success bool) *client {
-	ofClient := NewClient(bridgeName, bridgeMgmtAddr, ovsconfig.OVSDatapathSystem, true, true, false, false, false)
+	ofClient := NewClient(bridgeName, bridgeMgmtAddr, ovsconfig.OVSDatapathSystem, nil, true, true, false, false, false)
 	c := ofClient.(*client)
 	c.nodeConfig = nodeConfig
 	m := ovsoftest.NewMockBridge(ctrl)
@@ -506,7 +506,7 @@ func Test_client_setBasePacketOutBuilder(t *testing.T) {
 }
 
 func prepareSetBasePacketOutBuilder(ctrl *gomock.Controller, success bool) *client {
-	ofClient := NewClient(bridgeName, bridgeMgmtAddr, ovsconfig.OVSDatapathSystem, true, true, false, false, false)
+	ofClient := NewClient(bridgeName, bridgeMgmtAddr, ovsconfig.OVSDatapathSystem, nil, true, true, false, false, false)
 	c := ofClient.(*client)
 	m := ovsoftest.NewMockBridge(ctrl)
 	c.bridge = m

diff --git a/pkg/agent/openflow/network_policy_test.go b/pkg/agent/openflow/network_policy_test.go
@@ -507,7 +507,7 @@ func TestBatchInstallPolicyRuleFlows(t *testing.T) {
 			ctrl := gomock.NewController(t)
 			defer ctrl.Finish()
 			mockOperations := oftest.NewMockOFEntryOperations(ctrl)
-			ofClient := NewClient(bridgeName, bridgeMgmtAddr, ovsconfig.OVSDatapathSystem, false, true, false, false, false)
+			ofClient := NewClient(bridgeName, bridgeMgmtAddr, ovsconfig.OVSDatapathSystem, nil, false, true, false, false, false)
 			c = ofClient.(*client)
 			c.cookieAllocator = cookie.NewAllocator(0)
 			c.ofEntryOperations = mockOperations
@@ -575,7 +575,8 @@ func BenchmarkBatchInstallPolicyRuleFlows(b *testing.B) {
 	ctrl := gomock.NewController(b)
 	defer ctrl.Finish()
 	mockOperations := oftest.NewMockOFEntryOperations(ctrl)
-	ofClient := NewClient(bridgeName, bridgeMgmtAddr, ovsconfig.OVSDatapathSystem, false, true, false, false, false)
+	ofClient := NewClient(bridgeName, bridgeMgmtAddr, ovsconfig.OVSDatapathSystem, nil,
+		false, true, false, false, false)
 	c = ofClient.(*client)
 	c.cookieAllocator = cookie.NewAllocator(0)
 	c.ofEntryOperations = mockOperations