Fix only rate-limit first packet

Signed-off-by: graysonwu <wgrayson@vmware.com>
antrea-io · Oct 20, 2023 · 49758f2 · 49758f2
1 parent ad455c8
commit 49758f2
Show file tree

Hide file tree

Showing 4 changed files with 21 additions and 17 deletions.
diff --git a/pkg/agent/openflow/client_test.go b/pkg/agent/openflow/client_test.go
@@ -1598,15 +1598,15 @@ func Test_client_InstallSNATMarkFlows(t *testing.T) {
 			snatIP:                net.ParseIP("192.168.77.100"),
 			trafficShapingEnabled: true,
 			expectedFlows: []string{
-				"cookie=0x1040000000000, table=EgressMark, priority=200,ct_state=+new+trk,ip,tun_dst=192.168.77.100 actions=set_field:0x64/0xff->pkt_mark,set_field:0x20/0xf0->reg0,goto_table:EgressQoS",
+				"cookie=0x1040000000000, table=EgressMark, priority=200,ct_state=+trk,ip,tun_dst=192.168.77.100 actions=set_field:0x64/0xff->pkt_mark,set_field:0x20/0xf0->reg0,goto_table:EgressQoS",
 			},
 		},
 		{
 			name:                  "IPv6 SNAT IP trafficShaping",
 			snatIP:                net.ParseIP("fec0:192:168:77::100"),
 			trafficShapingEnabled: true,
 			expectedFlows: []string{
-				"cookie=0x1040000000000, table=EgressMark, priority=200,ct_state=+new+trk,ipv6,tun_ipv6_dst=fec0:192:168:77::100 actions=set_field:0x64/0xff->pkt_mark,set_field:0x20/0xf0->reg0,goto_table:EgressQoS",
+				"cookie=0x1040000000000, table=EgressMark, priority=200,ct_state=+trk,ipv6,tun_ipv6_dst=fec0:192:168:77::100 actions=set_field:0x64/0xff->pkt_mark,set_field:0x20/0xf0->reg0,goto_table:EgressQoS",
 			},
 		},
 	}
@@ -1671,7 +1671,7 @@ func Test_client_InstallPodSNATFlows(t *testing.T) {
 			trafficShapingEnabled: true,
 			snatMark:              uint32(100),
 			expectedFlows: []string{
-				"cookie=0x1040000000000, table=EgressMark, priority=200,ct_state=+new+trk,ip,in_port=100 actions=set_field:0x64/0xff->pkt_mark,set_field:0x20/0xf0->reg0,goto_table:EgressQoS",
+				"cookie=0x1040000000000, table=EgressMark, priority=200,ct_state=+trk,ip,in_port=100 actions=set_field:0x64/0xff->pkt_mark,set_field:0x20/0xf0->reg0,goto_table:EgressQoS",
 			},
 		},
 	}
@@ -2704,7 +2704,7 @@ func testClientReplayFlows(t *testing.T, egressTrafficShaping bool) {
 	addFlowInCache(fc.featureEgress.cachedFlows, "egressFlows", []binding.Flow{fc.featureEgress.snatIPFromTunnelFlow(snatIP, uint32(100))})
 	if egressTrafficShaping {
 		replayedFlows = append(replayedFlows,
-			"cookie=0x1040000000000, table=EgressMark, priority=200,ct_state=+new+trk,ip,tun_dst=192.168.77.100 actions=set_field:0x64/0xff->pkt_mark,set_field:0x20/0xf0->reg0,goto_table:EgressQoS",
+			"cookie=0x1040000000000, table=EgressMark, priority=200,ct_state=+trk,ip,tun_dst=192.168.77.100 actions=set_field:0x64/0xff->pkt_mark,set_field:0x20/0xf0->reg0,goto_table:EgressQoS",
 			"cookie=0x1040000000000, table=EgressQoS, priority=190 actions=goto_table:L2ForwardingCalc",
 		)
 	} else {

diff --git a/pkg/agent/openflow/pipeline.go b/pkg/agent/openflow/pipeline.go
@@ -2316,15 +2316,16 @@ func (f *featureEgress) snatIPFromTunnelFlow(snatIP net.IP, mark uint32) binding
 	fb := EgressMarkTable.ofTable.BuildFlow(priorityNormal).
 		Cookie(f.cookieAllocator.Request(f.category).Raw()).
 		MatchProtocol(ipProtocol).
-		MatchCTStateNew(true).
 		MatchCTStateTrk(true).
 		MatchTunnelDst(snatIP).
 		Action().LoadPktMarkRange(mark, snatPktMarkRange).
 		Action().LoadRegMark(ToGatewayRegMark)
 	if f.enableEgressTrafficShaping {
+		// To apply rate-limit on all traffic, instead of just the first one, remove ct_state=+new.
 		fb = fb.Action().GotoTable(EgressQoSTable.GetID())
 	} else {
-		fb = fb.Action().GotoStage(stageSwitching)
+		fb = fb.MatchCTStateNew(true).
+			Action().GotoStage(stageSwitching)
 	}
 	return fb.Done()
 }
@@ -2340,15 +2341,16 @@ func (f *featureEgress) snatRuleFlow(ofPort uint32, snatIP net.IP, snatMark uint
 		fb := EgressMarkTable.ofTable.BuildFlow(priorityNormal).
 			Cookie(cookieID).
 			MatchProtocol(ipProtocol).
-			MatchCTStateNew(true).
 			MatchCTStateTrk(true).
 			MatchInPort(ofPort).
 			Action().LoadPktMarkRange(snatMark, snatPktMarkRange).
 			Action().LoadRegMark(ToGatewayRegMark)
 		if f.enableEgressTrafficShaping {
+			// To apply rate-limit on all traffic, instead of just the first one, remove ct_state=+new.
 			fb = fb.Action().GotoTable(EgressQoSTable.GetID())
 		} else {
-			fb = fb.Action().GotoStage(stageSwitching)
+			fb = fb.MatchCTStateNew(true).
+				Action().GotoStage(stageSwitching)
 		}
 		return fb.Done()
 	}

diff --git a/test/e2e/egress_test.go b/test/e2e/egress_test.go
@@ -687,8 +687,8 @@ func testEgressUpdateBandwidth(t *testing.T, data *TestData) {
 		}
 	)
 
-	egressNode := controlPlaneNodeName()
-	egressNodeIP := controlPlaneNodeIPv4()
+	egressNode := nodeName(1)
+	egressNodeIP := nodeIP(1)
 
 	// Create another netns to fake an external network on the host network Pod.
 	fakeExternalName := "fake-external"
@@ -730,8 +730,8 @@ func testEgressUpdateBandwidth(t *testing.T, data *TestData) {
 		assert.InEpsilon(t, actualBandwidth, expectedBandwidth, 0.1)
 	}
 
-	runIperf([]string{"bash", "-c", "iperf3 -c 1.1.1.1 -f m -O 1|grep sender|awk '{print $7}'"}, transMap[bandwidth1.Rate]+transMap[bandwidth1.Burst])
-	runIperf([]string{"bash", "-c", "iperf3 -c 1.1.1.1 -f m -O 3|grep sender|awk '{print $7}'"}, transMap[bandwidth1.Rate])
+	runIperf([]string{"bash", "-c", "iperf3 -c 1.1.1.1 -f m -t 1|grep sender|awk '{print $7}'"}, transMap[bandwidth1.Rate]+transMap[bandwidth1.Burst])
+	runIperf([]string{"bash", "-c", "iperf3 -c 1.1.1.1 -f m -O 1|grep sender|awk '{print $7}'"}, transMap[bandwidth1.Rate])
 
 	toUpdate := egress.DeepCopy()
 	err = retry.RetryOnConflict(retry.DefaultRetry, func() error {
@@ -744,8 +744,8 @@ func testEgressUpdateBandwidth(t *testing.T, data *TestData) {
 	})
 	require.NoError(t, err, "Failed to update Egress")
 
-	runIperf([]string{"bash", "-c", "iperf3 -c 1.1.1.1 -f m -O 1|grep sender|awk '{print $7}'"}, transMap[bandwidth2.Rate]+transMap[bandwidth2.Burst])
-	runIperf([]string{"bash", "-c", "iperf3 -c 1.1.1.1 -f m -O 3|grep sender|awk '{print $7}'"}, transMap[bandwidth2.Rate])
+	runIperf([]string{"bash", "-c", "iperf3 -c 1.1.1.1 -f m -t 1|grep sender|awk '{print $7}'"}, transMap[bandwidth2.Rate]+transMap[bandwidth2.Burst])
+	runIperf([]string{"bash", "-c", "iperf3 -c 1.1.1.1 -f m -O 1|grep sender|awk '{print $7}'"}, transMap[bandwidth2.Rate])
 
 	failOnError(data.crdClient.CrdV1beta1().Egresses().Delete(context.TODO(), egress.Name, metav1.DeleteOptions{}), t)
 }

diff --git a/test/integration/agent/openflow_test.go b/test/integration/agent/openflow_test.go
@@ -1705,7 +1705,7 @@ func expectedExternalFlows(ipProtoStr, gwMACStr string) []expectTableFlows {
 }
 
 func prepareEgressMarkFlows(snatIP net.IP, mark, podOFPort, podOFPortRemote uint32, vMAC, localGwMAC net.HardwareAddr, trafficShaping bool) []expectTableFlows {
-	var ipProtoStr, tunDstFieldName, nextTableName string
+	var ipProtoStr, tunDstFieldName, nextTableName, ctStateMatch string
 	if snatIP.To4() != nil {
 		tunDstFieldName = "tun_dst"
 		ipProtoStr = "ip"
@@ -1715,19 +1715,21 @@ func prepareEgressMarkFlows(snatIP net.IP, mark, podOFPort, podOFPortRemote uint
 	}
 	if trafficShaping {
 		nextTableName = "EgressQoS"
+		ctStateMatch = "+trk"
 	} else {
 		nextTableName = "L2ForwardingCalc"
+		ctStateMatch = "+new+trk"
 	}
 	return []expectTableFlows{
 		{
 			"EgressMark",
 			[]*ofTestUtils.ExpectFlow{
 				{
-					MatchStr: fmt.Sprintf("priority=200,ct_state=+new+trk,%s,%s=%s", ipProtoStr, tunDstFieldName, snatIP),
+					MatchStr: fmt.Sprintf("priority=200,ct_state=%s,%s,%s=%s", ctStateMatch, ipProtoStr, tunDstFieldName, snatIP),
 					ActStr:   fmt.Sprintf("set_field:0x%x/0xff->pkt_mark,set_field:0x20/0xf0->reg0,goto_table:%s", mark, nextTableName),
 				},
 				{
-					MatchStr: fmt.Sprintf("priority=200,ct_state=+new+trk,%s,in_port=%d", ipProtoStr, podOFPort),
+					MatchStr: fmt.Sprintf("priority=200,ct_state=%s,%s,in_port=%d", ctStateMatch, ipProtoStr, podOFPort),
 					ActStr:   fmt.Sprintf("set_field:0x%x/0xff->pkt_mark,set_field:0x20/0xf0->reg0,goto_table:%s", mark, nextTableName),
 				},
 				{