From 45c091a385dd21c33702bf39ece93bd77f88ac80 Mon Sep 17 00:00:00 2001 From: Hongliang Liu Date: Thu, 16 Nov 2023 20:08:39 +0800 Subject: [PATCH] Prioritize L7NP flows over TrafficControl When applying an L7NP to a Pod, there's a potential issue where creating a TrafficControl CR with a redirect action to the same Pod could bypass the L7 engine. This is due to the fact that both the ct mark `L7NPRedirectCTMark` for identifying L7NP packets and the reg mark `TrafficControlRedirectRegMark` for identifying TrafficControl redirect packets can be set together. In `OutputTable`, the priorities of flows to match the ct mark and the reg mark are the same. Without an additional condition to distinguish between them, packets with both the reg mark and ct mark may be matched by either flow with an equal chance. To rectify this and ensure proper L7NP enforcement, it is crucial to assign a higher priority to the flow that matches the ct mark L7NPRedirectCTMark. This patch also adds an extra parameter `priority` to InstallTrafficControlMarkFlows, which is used to set the priority of the related flows. Signed-off-by: Hongliang Liu --- .../controller/trafficcontrol/controller.go | 7 ++- .../trafficcontrol/controller_test.go | 60 +++++++++---------- pkg/agent/openflow/client.go | 29 ++++++++- pkg/agent/openflow/client_test.go | 4 +- pkg/agent/openflow/network_policy.go | 2 +- pkg/agent/openflow/network_policy_test.go | 2 +- pkg/agent/openflow/pod_connectivity.go | 10 +++- pkg/agent/openflow/testing/mock_openflow.go | 8 +-- pkg/agent/types/trafficcontrol.go | 28 +++++++++ test/integration/agent/openflow_test.go | 2 +- 10 files changed, 106 insertions(+), 46 deletions(-) create mode 100644 pkg/agent/types/trafficcontrol.go diff --git a/pkg/agent/controller/trafficcontrol/controller.go b/pkg/agent/controller/trafficcontrol/controller.go index 1c0d8720a77..e084b0e162f 100644 --- a/pkg/agent/controller/trafficcontrol/controller.go +++ b/pkg/agent/controller/trafficcontrol/controller.go @@ -920,7 +920,12 @@ func (c *Controller) syncTrafficControl(tcName string) error { for _, port := range sets.List(newOfPorts) { ofPorts = append(ofPorts, uint32(port)) } - if err = c.ofClient.InstallTrafficControlMarkFlows(tc.Name, ofPorts, targetOFPort, tc.Spec.Direction, tc.Spec.Action); err != nil { + if err = c.ofClient.InstallTrafficControlMarkFlows(tc.Name, + ofPorts, + targetOFPort, + tc.Spec.Direction, + tc.Spec.Action, + types.TrafficControlFlowPriorityMedium); err != nil { return err } } diff --git a/pkg/agent/controller/trafficcontrol/controller_test.go b/pkg/agent/controller/trafficcontrol/controller_test.go index 3dab23a1fac..477b88268eb 100644 --- a/pkg/agent/controller/trafficcontrol/controller_test.go +++ b/pkg/agent/controller/trafficcontrol/controller_test.go @@ -332,7 +332,7 @@ func TestTrafficControlAdd(t *testing.T) { mockOVSBridgeClient.EXPECT().CreatePort(networkDeviceName, networkDeviceName, externalIDs) mockOVSBridgeClient.EXPECT().GetOFPort(networkDeviceName, false) mockOVSCtlClient.EXPECT().SetPortNoFlood(0) - mockOFClient.EXPECT().InstallTrafficControlMarkFlows(tc1Name, gomock.InAnyOrder([]uint32{pod1OFPort, pod3OFPort}), gomock.Any(), directionIngress, actionMirror) + mockOFClient.EXPECT().InstallTrafficControlMarkFlows(tc1Name, gomock.InAnyOrder([]uint32{pod1OFPort, pod3OFPort}), gomock.Any(), directionIngress, actionMirror, types.TrafficControlFlowPriorityMedium) }, }, { @@ -346,7 +346,7 @@ func TestTrafficControlAdd(t *testing.T) { mockOVSBridgeClient.EXPECT().CreateTunnelPortExt(gomock.Any(), ovsconfig.TunnelType(ovsconfig.VXLANTunnel), int32(0), false, "", remoteIP, "", "", extraOptions, externalIDs) mockOVSBridgeClient.EXPECT().GetOFPort(gomock.Any(), false) mockOVSCtlClient.EXPECT().SetPortNoFlood(gomock.Any()) - mockOFClient.EXPECT().InstallTrafficControlMarkFlows(tc1Name, gomock.InAnyOrder([]uint32{pod1OFPort, pod3OFPort}), gomock.Any(), directionIngress, actionMirror) + mockOFClient.EXPECT().InstallTrafficControlMarkFlows(tc1Name, gomock.InAnyOrder([]uint32{pod1OFPort, pod3OFPort}), gomock.Any(), directionIngress, actionMirror, types.TrafficControlFlowPriorityMedium) }, }, { @@ -360,7 +360,7 @@ func TestTrafficControlAdd(t *testing.T) { mockOVSBridgeClient.EXPECT().CreateTunnelPortExt(gomock.Any(), ovsconfig.TunnelType(ovsconfig.GeneveTunnel), int32(0), false, "", remoteIP, "", "", extraOptions, externalIDs) mockOVSBridgeClient.EXPECT().GetOFPort(gomock.Any(), false) mockOVSCtlClient.EXPECT().SetPortNoFlood(gomock.Any()) - mockOFClient.EXPECT().InstallTrafficControlMarkFlows(tc1Name, gomock.InAnyOrder([]uint32{pod1OFPort, pod3OFPort}), gomock.Any(), directionIngress, actionMirror) + mockOFClient.EXPECT().InstallTrafficControlMarkFlows(tc1Name, gomock.InAnyOrder([]uint32{pod1OFPort, pod3OFPort}), gomock.Any(), directionIngress, actionMirror, types.TrafficControlFlowPriorityMedium) }, }, { @@ -374,7 +374,7 @@ func TestTrafficControlAdd(t *testing.T) { mockOVSBridgeClient.EXPECT().CreateTunnelPortExt(gomock.Any(), ovsconfig.TunnelType(ovsconfig.GRETunnel), int32(0), false, "", remoteIP, "", "", extraOptions, externalIDs) mockOVSBridgeClient.EXPECT().GetOFPort(gomock.Any(), false) mockOVSCtlClient.EXPECT().SetPortNoFlood(gomock.Any()) - mockOFClient.EXPECT().InstallTrafficControlMarkFlows(tc1Name, gomock.InAnyOrder([]uint32{pod1OFPort, pod3OFPort}), gomock.Any(), directionIngress, actionMirror) + mockOFClient.EXPECT().InstallTrafficControlMarkFlows(tc1Name, gomock.InAnyOrder([]uint32{pod1OFPort, pod3OFPort}), gomock.Any(), directionIngress, actionMirror, types.TrafficControlFlowPriorityMedium) }, }, { @@ -388,7 +388,7 @@ func TestTrafficControlAdd(t *testing.T) { mockOVSBridgeClient.EXPECT().CreateTunnelPortExt(gomock.Any(), ovsconfig.TunnelType(ovsconfig.ERSPANTunnel), int32(0), false, "", remoteIP, "", "", extraOptions, externalIDs) mockOVSBridgeClient.EXPECT().GetOFPort(gomock.Any(), false) mockOVSCtlClient.EXPECT().SetPortNoFlood(gomock.Any()) - mockOFClient.EXPECT().InstallTrafficControlMarkFlows(tc1Name, gomock.InAnyOrder([]uint32{pod1OFPort, pod3OFPort}), gomock.Any(), directionIngress, actionMirror) + mockOFClient.EXPECT().InstallTrafficControlMarkFlows(tc1Name, gomock.InAnyOrder([]uint32{pod1OFPort, pod3OFPort}), gomock.Any(), directionIngress, actionMirror, types.TrafficControlFlowPriorityMedium) }, }, { @@ -402,7 +402,7 @@ func TestTrafficControlAdd(t *testing.T) { expectedCalls: func(mockOFClient *openflowtest.MockClient, mockOVSBridgeClient *ovsconfigtest.MockOVSBridgeClient, mockOVSCtlClient *ovsctltest.MockOVSCtlClient) { - mockOFClient.EXPECT().InstallTrafficControlMarkFlows(tc1Name, gomock.InAnyOrder([]uint32{pod1OFPort, pod3OFPort}), targetPort2OFPort, directionIngress, actionRedirect) + mockOFClient.EXPECT().InstallTrafficControlMarkFlows(tc1Name, gomock.InAnyOrder([]uint32{pod1OFPort, pod3OFPort}), targetPort2OFPort, directionIngress, actionRedirect, types.TrafficControlFlowPriorityMedium) }, }, { @@ -415,7 +415,7 @@ func TestTrafficControlAdd(t *testing.T) { expectedCalls: func(mockOFClient *openflowtest.MockClient, mockOVSBridgeClient *ovsconfigtest.MockOVSBridgeClient, mockOVSCtlClient *ovsctltest.MockOVSCtlClient) { - mockOFClient.EXPECT().InstallTrafficControlMarkFlows(tc1Name, gomock.InAnyOrder([]uint32{pod1OFPort, pod3OFPort}), targetPort1OFPort, directionIngress, actionMirror) + mockOFClient.EXPECT().InstallTrafficControlMarkFlows(tc1Name, gomock.InAnyOrder([]uint32{pod1OFPort, pod3OFPort}), targetPort1OFPort, directionIngress, actionMirror, types.TrafficControlFlowPriorityMedium) }, }, { @@ -428,7 +428,7 @@ func TestTrafficControlAdd(t *testing.T) { expectedCalls: func(mockOFClient *openflowtest.MockClient, mockOVSBridgeClient *ovsconfigtest.MockOVSBridgeClient, mockOVSCtlClient *ovsctltest.MockOVSCtlClient) { - mockOFClient.EXPECT().InstallTrafficControlMarkFlows(tc1Name, gomock.InAnyOrder([]uint32{pod1OFPort, pod2OFPort}), targetPort1OFPort, directionIngress, actionMirror) + mockOFClient.EXPECT().InstallTrafficControlMarkFlows(tc1Name, gomock.InAnyOrder([]uint32{pod1OFPort, pod2OFPort}), targetPort1OFPort, directionIngress, actionMirror, types.TrafficControlFlowPriorityMedium) }, }, { @@ -441,7 +441,7 @@ func TestTrafficControlAdd(t *testing.T) { expectedCalls: func(mockOFClient *openflowtest.MockClient, mockOVSBridgeClient *ovsconfigtest.MockOVSBridgeClient, mockOVSCtlClient *ovsctltest.MockOVSCtlClient) { - mockOFClient.EXPECT().InstallTrafficControlMarkFlows(tc1Name, []uint32{pod2OFPort}, targetPort1OFPort, directionIngress, actionRedirect) + mockOFClient.EXPECT().InstallTrafficControlMarkFlows(tc1Name, []uint32{pod2OFPort}, targetPort1OFPort, directionIngress, actionRedirect, types.TrafficControlFlowPriorityMedium) }, }, { @@ -454,7 +454,7 @@ func TestTrafficControlAdd(t *testing.T) { expectedCalls: func(mockOFClient *openflowtest.MockClient, mockOVSBridgeClient *ovsconfigtest.MockOVSBridgeClient, mockOVSCtlClient *ovsctltest.MockOVSCtlClient) { - mockOFClient.EXPECT().InstallTrafficControlMarkFlows(tc1Name, nil, targetPort1OFPort, directionIngress, actionRedirect) + mockOFClient.EXPECT().InstallTrafficControlMarkFlows(tc1Name, nil, targetPort1OFPort, directionIngress, actionRedirect, types.TrafficControlFlowPriorityMedium) }, }, { @@ -467,7 +467,7 @@ func TestTrafficControlAdd(t *testing.T) { expectedCalls: func(mockOFClient *openflowtest.MockClient, mockOVSBridgeClient *ovsconfigtest.MockOVSBridgeClient, mockOVSCtlClient *ovsctltest.MockOVSCtlClient) { - mockOFClient.EXPECT().InstallTrafficControlMarkFlows(tc1Name, []uint32{pod1OFPort, pod2OFPort, pod3OFPort, pod4OFPort}, targetPort1OFPort, directionIngress, actionRedirect) + mockOFClient.EXPECT().InstallTrafficControlMarkFlows(tc1Name, []uint32{pod1OFPort, pod2OFPort, pod3OFPort, pod4OFPort}, targetPort1OFPort, directionIngress, actionRedirect, types.TrafficControlFlowPriorityMedium) }, }, } @@ -520,7 +520,7 @@ func TestTrafficControlUpdate(t *testing.T) { mockOVSBridgeClient.EXPECT().CreatePort(targetPort2Name, targetPort2Name, externalIDs) mockOVSBridgeClient.EXPECT().GetOFPort(targetPort2Name, false) mockOVSCtlClient.EXPECT().SetPortNoFlood(gomock.Any()) - mockOFClient.EXPECT().InstallTrafficControlMarkFlows(tc1Name, gomock.InAnyOrder([]uint32{pod1OFPort, pod3OFPort}), gomock.Any(), directionIngress, actionMirror) + mockOFClient.EXPECT().InstallTrafficControlMarkFlows(tc1Name, gomock.InAnyOrder([]uint32{pod1OFPort, pod3OFPort}), gomock.Any(), directionIngress, actionMirror, types.TrafficControlFlowPriorityMedium) }, }, { @@ -534,7 +534,7 @@ func TestTrafficControlUpdate(t *testing.T) { mockOVSBridgeClient.EXPECT().GetOFPort(returnPort1Name, false) mockOVSCtlClient.EXPECT().SetPortNoFlood(gomock.Any()) mockOFClient.EXPECT().InstallTrafficControlReturnPortFlow(gomock.Any()) - mockOFClient.EXPECT().InstallTrafficControlMarkFlows(tc1Name, gomock.InAnyOrder([]uint32{pod1OFPort, pod3OFPort}), targetPort1OFPort, directionIngress, actionRedirect) + mockOFClient.EXPECT().InstallTrafficControlMarkFlows(tc1Name, gomock.InAnyOrder([]uint32{pod1OFPort, pod3OFPort}), targetPort1OFPort, directionIngress, actionRedirect, types.TrafficControlFlowPriorityMedium) }, }, { @@ -544,7 +544,7 @@ func TestTrafficControlUpdate(t *testing.T) { expectedCalls: func(mockOFClient *openflowtest.MockClient, mockOVSBridgeClient *ovsconfigtest.MockOVSBridgeClient, mockOVSCtlClient *ovsctltest.MockOVSCtlClient) { - mockOFClient.EXPECT().InstallTrafficControlMarkFlows(tc1Name, gomock.InAnyOrder([]uint32{pod1OFPort, pod3OFPort}), targetPort1OFPort, directionEgress, actionMirror) + mockOFClient.EXPECT().InstallTrafficControlMarkFlows(tc1Name, gomock.InAnyOrder([]uint32{pod1OFPort, pod3OFPort}), targetPort1OFPort, directionEgress, actionMirror, types.TrafficControlFlowPriorityMedium) }, }, { @@ -554,7 +554,7 @@ func TestTrafficControlUpdate(t *testing.T) { expectedCalls: func(mockOFClient *openflowtest.MockClient, mockOVSBridgeClient *ovsconfigtest.MockOVSBridgeClient, mockOVSCtlClient *ovsctltest.MockOVSCtlClient) { - mockOFClient.EXPECT().InstallTrafficControlMarkFlows(tc1Name, gomock.InAnyOrder([]uint32{pod2OFPort, pod4OFPort}), targetPort1OFPort, directionIngress, actionMirror) + mockOFClient.EXPECT().InstallTrafficControlMarkFlows(tc1Name, gomock.InAnyOrder([]uint32{pod2OFPort, pod4OFPort}), targetPort1OFPort, directionIngress, actionMirror, types.TrafficControlFlowPriorityMedium) }, }, { @@ -564,7 +564,7 @@ func TestTrafficControlUpdate(t *testing.T) { expectedCalls: func(mockOFClient *openflowtest.MockClient, mockOVSBridgeClient *ovsconfigtest.MockOVSBridgeClient, mockOVSCtlClient *ovsctltest.MockOVSCtlClient) { - mockOFClient.EXPECT().InstallTrafficControlMarkFlows(tc1Name, []uint32{pod3OFPort}, targetPort1OFPort, directionIngress, actionMirror) + mockOFClient.EXPECT().InstallTrafficControlMarkFlows(tc1Name, []uint32{pod3OFPort}, targetPort1OFPort, directionIngress, actionMirror, types.TrafficControlFlowPriorityMedium) }, }, } @@ -637,8 +637,8 @@ func TestSharedTargetPort(t *testing.T) { c.mockOVSBridgeClient.EXPECT().GetOFPort(targetPort1Name, false).Times(1) c.mockOVSCtlClient.EXPECT().SetPortNoFlood(gomock.Any()) // Mark flows for TrafficControl tc1 and tc2 are expected to be installed. - c.mockOFClient.EXPECT().InstallTrafficControlMarkFlows(tc1Name, gomock.InAnyOrder([]uint32{pod1OFPort, pod3OFPort}), gomock.Any(), directionIngress, actionMirror) - c.mockOFClient.EXPECT().InstallTrafficControlMarkFlows(tc2Name, gomock.InAnyOrder([]uint32{pod2OFPort, pod4OFPort}), gomock.Any(), directionIngress, actionMirror) + c.mockOFClient.EXPECT().InstallTrafficControlMarkFlows(tc1Name, gomock.InAnyOrder([]uint32{pod1OFPort, pod3OFPort}), gomock.Any(), directionIngress, actionMirror, types.TrafficControlFlowPriorityMedium) + c.mockOFClient.EXPECT().InstallTrafficControlMarkFlows(tc2Name, gomock.InAnyOrder([]uint32{pod2OFPort, pod4OFPort}), gomock.Any(), directionIngress, actionMirror, types.TrafficControlFlowPriorityMedium) // Process the TrafficControl ADD events for TrafficControl tc1 and tc2. waitEvents(t, 2, c) @@ -721,7 +721,7 @@ func TestPodUpdateFromCNIServer(t *testing.T) { require.Equal(t, expectedState, c.tcStates[tc1Name]) // Mark flows are expected to be installed after the interface of the Pod is ready. - c.mockOFClient.EXPECT().InstallTrafficControlMarkFlows(tc1Name, []uint32{pod1OFPort}, targetPort1OFPort, directionIngress, actionMirror) + c.mockOFClient.EXPECT().InstallTrafficControlMarkFlows(tc1Name, []uint32{pod1OFPort}, targetPort1OFPort, directionIngress, actionMirror, types.TrafficControlFlowPriorityMedium) // Add the interface information of the test Pod to interface store to mock the interface of the Pod is ready, then // add an update event to podUpdateChannel to trigger a TrafficControl event. @@ -772,7 +772,7 @@ func TestPodLabelsUpdate(t *testing.T) { eventsTriggeredByPodLabelsUpdate: 2, expectedPodBinding: nil, expectedCalls: func(mockOFClient *openflowtest.MockClient) { - mockOFClient.EXPECT().InstallTrafficControlMarkFlows(tc1Name, nil, targetPort1OFPort, directionIngress, actionMirror) + mockOFClient.EXPECT().InstallTrafficControlMarkFlows(tc1Name, nil, targetPort1OFPort, directionIngress, actionMirror, types.TrafficControlFlowPriorityMedium) }, }, { @@ -790,8 +790,8 @@ func TestPodLabelsUpdate(t *testing.T) { eventsTriggeredByPodEffectiveTCUpdate: 1, expectedPodBinding: &podToTCBinding{effectiveTC: tc2Name, alternativeTCs: sets.New[string]()}, expectedCalls: func(mockOFClient *openflowtest.MockClient) { - mockOFClient.EXPECT().InstallTrafficControlMarkFlows(tc1Name, nil, targetPort1OFPort, directionIngress, actionMirror) - mockOFClient.EXPECT().InstallTrafficControlMarkFlows(tc2Name, []uint32{pod1OFPort}, targetPort2OFPort, directionIngress, actionMirror) + mockOFClient.EXPECT().InstallTrafficControlMarkFlows(tc1Name, nil, targetPort1OFPort, directionIngress, actionMirror, types.TrafficControlFlowPriorityMedium) + mockOFClient.EXPECT().InstallTrafficControlMarkFlows(tc2Name, []uint32{pod1OFPort}, targetPort2OFPort, directionIngress, actionMirror, types.TrafficControlFlowPriorityMedium) }, }, { @@ -802,8 +802,8 @@ func TestPodLabelsUpdate(t *testing.T) { eventsTriggeredByPodEffectiveTCUpdate: 1, expectedPodBinding: &podToTCBinding{effectiveTC: tc2Name, alternativeTCs: sets.New[string](tc3Name)}, expectedCalls: func(mockOFClient *openflowtest.MockClient) { - mockOFClient.EXPECT().InstallTrafficControlMarkFlows(tc1Name, nil, targetPort1OFPort, directionIngress, actionMirror) - mockOFClient.EXPECT().InstallTrafficControlMarkFlows(tc2Name, []uint32{pod1OFPort}, targetPort2OFPort, directionIngress, actionMirror) + mockOFClient.EXPECT().InstallTrafficControlMarkFlows(tc1Name, nil, targetPort1OFPort, directionIngress, actionMirror, types.TrafficControlFlowPriorityMedium) + mockOFClient.EXPECT().InstallTrafficControlMarkFlows(tc2Name, []uint32{pod1OFPort}, targetPort2OFPort, directionIngress, actionMirror, types.TrafficControlFlowPriorityMedium) }, }, { @@ -947,7 +947,7 @@ func TestNamespaceLabelsUpdate(t *testing.T) { updatedNS: newNamespace("ns1", nil), eventsTriggeredByNSLabelsUpdate: 2, expectedCalls: func(mockOFClient *openflowtest.MockClient) { - mockOFClient.EXPECT().InstallTrafficControlMarkFlows(tc1Name, nil, targetPort1OFPort, directionIngress, actionMirror) + mockOFClient.EXPECT().InstallTrafficControlMarkFlows(tc1Name, nil, targetPort1OFPort, directionIngress, actionMirror, types.TrafficControlFlowPriorityMedium) }, }, { @@ -965,8 +965,8 @@ func TestNamespaceLabelsUpdate(t *testing.T) { eventsTriggeredByPodEffectiveTCUpdate: 1, expectedPodBinding: &podToTCBinding{effectiveTC: tc2Name, alternativeTCs: sets.New[string]()}, expectedCalls: func(mockOFClient *openflowtest.MockClient) { - mockOFClient.EXPECT().InstallTrafficControlMarkFlows(tc1Name, nil, targetPort1OFPort, directionIngress, actionMirror) - mockOFClient.EXPECT().InstallTrafficControlMarkFlows(tc2Name, []uint32{pod1OFPort}, targetPort2OFPort, directionIngress, actionMirror) + mockOFClient.EXPECT().InstallTrafficControlMarkFlows(tc1Name, nil, targetPort1OFPort, directionIngress, actionMirror, types.TrafficControlFlowPriorityMedium) + mockOFClient.EXPECT().InstallTrafficControlMarkFlows(tc2Name, []uint32{pod1OFPort}, targetPort2OFPort, directionIngress, actionMirror, types.TrafficControlFlowPriorityMedium) }, }, { @@ -977,8 +977,8 @@ func TestNamespaceLabelsUpdate(t *testing.T) { eventsTriggeredByPodEffectiveTCUpdate: 1, expectedPodBinding: &podToTCBinding{effectiveTC: tc2Name, alternativeTCs: sets.New[string](tc3Name)}, expectedCalls: func(mockOFClient *openflowtest.MockClient) { - mockOFClient.EXPECT().InstallTrafficControlMarkFlows(tc1Name, nil, targetPort1OFPort, directionIngress, actionMirror) - mockOFClient.EXPECT().InstallTrafficControlMarkFlows(tc2Name, []uint32{pod1OFPort}, targetPort2OFPort, directionIngress, actionMirror) + mockOFClient.EXPECT().InstallTrafficControlMarkFlows(tc1Name, nil, targetPort1OFPort, directionIngress, actionMirror, types.TrafficControlFlowPriorityMedium) + mockOFClient.EXPECT().InstallTrafficControlMarkFlows(tc2Name, []uint32{pod1OFPort}, targetPort2OFPort, directionIngress, actionMirror, types.TrafficControlFlowPriorityMedium) }, }, { @@ -1156,7 +1156,7 @@ func TestPodDelete(t *testing.T) { c.queue.Done(item) } - c.mockOFClient.EXPECT().InstallTrafficControlMarkFlows(tc1Name, []uint32{pod3OFPort}, targetPort1OFPort, directionIngress, actionMirror) + c.mockOFClient.EXPECT().InstallTrafficControlMarkFlows(tc1Name, []uint32{pod3OFPort}, targetPort1OFPort, directionIngress, actionMirror, types.TrafficControlFlowPriorityMedium) expectedPod3Binding := &podToTCBinding{ effectiveTC: tc1Name, alternativeTCs: sets.New[string](tc2Name, tc3Name), diff --git a/pkg/agent/openflow/client.go b/pkg/agent/openflow/client.go index 45acf46aac5..bbd558e77d1 100644 --- a/pkg/agent/openflow/client.go +++ b/pkg/agent/openflow/client.go @@ -39,6 +39,19 @@ import ( const maxRetryForOFSwitch = 5 +func tcPriorityToOFPriority(p types.TrafficControlFlowPriority) uint16 { + switch p { + case types.TrafficControlFlowPriorityHigh: + return priorityHigh + case types.TrafficControlFlowPriorityMedium: + return priorityNormal + case types.TrafficControlFlowPriorityLow: + return priorityLow + default: + return 0 + } +} + // Client is the interface to program OVS flows for entity connectivity of Antrea. type Client interface { // Initialize sets up all basic flows on the specific OVS bridge. It returns a channel which @@ -323,7 +336,12 @@ type Client interface { igmp ofutil.Message) error // InstallTrafficControlMarkFlows installs the flows to mark the packets for a traffic control rule. - InstallTrafficControlMarkFlows(name string, sourceOFPorts []uint32, targetOFPort uint32, direction crdv1alpha2.Direction, action crdv1alpha2.TrafficControlAction) error + InstallTrafficControlMarkFlows(name string, + sourceOFPorts []uint32, + targetOFPort uint32, + direction crdv1alpha2.Direction, + action crdv1alpha2.TrafficControlAction, + priority types.TrafficControlFlowPriority) error // UninstallTrafficControlMarkFlows removes the flows for a traffic control rule. UninstallTrafficControlMarkFlows(name string) error @@ -1449,8 +1467,13 @@ func (c *client) SendIGMPQueryPacketOut( return c.bridge.SendPacketOut(packetOutObj) } -func (c *client) InstallTrafficControlMarkFlows(name string, sourceOFPorts []uint32, targetOFPort uint32, direction crdv1alpha2.Direction, action crdv1alpha2.TrafficControlAction) error { - flows := c.featurePodConnectivity.trafficControlMarkFlows(sourceOFPorts, targetOFPort, direction, action) +func (c *client) InstallTrafficControlMarkFlows(name string, + sourceOFPorts []uint32, + targetOFPort uint32, + direction crdv1alpha2.Direction, + action crdv1alpha2.TrafficControlAction, + priority types.TrafficControlFlowPriority) error { + flows := c.featurePodConnectivity.trafficControlMarkFlows(sourceOFPorts, targetOFPort, direction, action, tcPriorityToOFPriority(priority)) cacheKey := fmt.Sprintf("tc_%s", name) c.replayMutex.RLock() defer c.replayMutex.RUnlock() diff --git a/pkg/agent/openflow/client_test.go b/pkg/agent/openflow/client_test.go index 8ce11c86263..1f7f8ab0a54 100644 --- a/pkg/agent/openflow/client_test.go +++ b/pkg/agent/openflow/client_test.go @@ -2425,7 +2425,7 @@ func Test_client_InstallTrafficControlMarkFlows(t *testing.T) { cacheKey := fmt.Sprintf("tc_%s", tcName) - assert.NoError(t, fc.InstallTrafficControlMarkFlows(tcName, sourceOFPorts, targetOFPort, tc.direction, tc.action)) + assert.NoError(t, fc.InstallTrafficControlMarkFlows(tcName, sourceOFPorts, targetOFPort, tc.direction, tc.action, types.TrafficControlFlowPriorityMedium)) fCacheI, ok := fc.featurePodConnectivity.tcCachedFlows.Load(cacheKey) require.True(t, ok) assert.ElementsMatch(t, tc.expectedFlows, getFlowStrings(fCacheI)) @@ -2768,7 +2768,7 @@ func Test_client_ReplayFlows(t *testing.T) { ) sourceOFPorts := []uint32{50, 100} targetOFPort := uint32(200) - addFlowInCache(fc.featurePodConnectivity.tcCachedFlows, "tcFlows", fc.featurePodConnectivity.trafficControlMarkFlows(sourceOFPorts, targetOFPort, v1alpha2.DirectionEgress, v1alpha2.ActionMirror)) + addFlowInCache(fc.featurePodConnectivity.tcCachedFlows, "tcFlows", fc.featurePodConnectivity.trafficControlMarkFlows(sourceOFPorts, targetOFPort, v1alpha2.DirectionEgress, v1alpha2.ActionMirror, priorityNormal)) replayedFlows = append(replayedFlows, "cookie=0x1010000000000, table=TrafficControl, priority=200,in_port=50 actions=set_field:0xc8->reg9,set_field:0x400000/0xc00000->reg4,goto_table:IngressSecurityClassifier", "cookie=0x1010000000000, table=TrafficControl, priority=200,in_port=100 actions=set_field:0xc8->reg9,set_field:0x400000/0xc00000->reg4,goto_table:IngressSecurityClassifier", diff --git a/pkg/agent/openflow/network_policy.go b/pkg/agent/openflow/network_policy.go index 8a8c96a344d..5b56e6f8cb8 100644 --- a/pkg/agent/openflow/network_policy.go +++ b/pkg/agent/openflow/network_policy.go @@ -2218,7 +2218,7 @@ func (f *featureNetworkPolicy) l7NPTrafficControlFlows() []binding.Flow { // This generates the flow to output the packets marked with L7NPRedirectCTMark to an application-aware engine // via the target ofPort. Note that, before outputting the packets, VLAN ID stored on field L7NPRuleVlanIDCTMarkField // will be copied to VLAN ID register (OXM_OF_VLAN_VID) to set VLAN ID of the packets. - OutputTable.ofTable.BuildFlow(priorityHigh+1). + OutputTable.ofTable.BuildFlow(priorityHigh+2). Cookie(cookieID). MatchRegMark(OutputToOFPortRegMark). MatchCTMark(L7NPRedirectCTMark). diff --git a/pkg/agent/openflow/network_policy_test.go b/pkg/agent/openflow/network_policy_test.go index 898b178f7b8..e8da28c1f2b 100644 --- a/pkg/agent/openflow/network_policy_test.go +++ b/pkg/agent/openflow/network_policy_test.go @@ -1404,7 +1404,7 @@ func networkPolicyInitFlows(ovsMeterSupported, externalNodeEnabled, l7NetworkPol initFlows = append(initFlows, "cookie=0x1020000000000, table=Classifier, priority=200,in_port=11,vlan_tci=0x1000/0x1000 actions=pop_vlan,set_field:0x6/0xf->reg0,goto_table:L3Forwarding", "cookie=0x1020000000000, table=TrafficControl, priority=210,reg0=0x200006/0x60000f actions=goto_table:Output", - "cookie=0x1020000000000, table=Output, priority=211,ct_mark=0x80/0x80,reg0=0x200000/0x600000 actions=push_vlan:0x8100,move:NXM_NX_CT_LABEL[64..75]->OXM_OF_VLAN_VID[0..11],output:10", + "cookie=0x1020000000000, table=Output, priority=212,ct_mark=0x80/0x80,reg0=0x200000/0x600000 actions=push_vlan:0x8100,move:NXM_NX_CT_LABEL[64..75]->OXM_OF_VLAN_VID[0..11],output:10", ) } return initFlows diff --git a/pkg/agent/openflow/pod_connectivity.go b/pkg/agent/openflow/pod_connectivity.go index 7a073b5bdbd..ec9b366e65c 100644 --- a/pkg/agent/openflow/pod_connectivity.go +++ b/pkg/agent/openflow/pod_connectivity.go @@ -200,7 +200,11 @@ func (f *featurePodConnectivity) replayFlows() []*openflow15.FlowMod { } // trafficControlMarkFlows generates the flows to mark the packets that need to be redirected or mirrored. -func (f *featurePodConnectivity) trafficControlMarkFlows(sourceOFPorts []uint32, targetOFPort uint32, direction v1alpha2.Direction, action v1alpha2.TrafficControlAction) []binding.Flow { +func (f *featurePodConnectivity) trafficControlMarkFlows(sourceOFPorts []uint32, + targetOFPort uint32, + direction v1alpha2.Direction, + action v1alpha2.TrafficControlAction, + priority uint16) []binding.Flow { cookieID := f.cookieAllocator.Request(f.category).Raw() var actionRegMark *binding.RegMark if action == v1alpha2.ActionRedirect { @@ -212,7 +216,7 @@ func (f *featurePodConnectivity) trafficControlMarkFlows(sourceOFPorts []uint32, for _, port := range sourceOFPorts { if direction == v1alpha2.DirectionIngress || direction == v1alpha2.DirectionBoth { // This generates the flow to mark the packets destined for a provided port. - flows = append(flows, TrafficControlTable.ofTable.BuildFlow(priorityNormal). + flows = append(flows, TrafficControlTable.ofTable.BuildFlow(priority). Cookie(cookieID). MatchRegFieldWithValue(TargetOFPortField, port). Action().LoadToRegField(TrafficControlTargetOFPortField, targetOFPort). @@ -222,7 +226,7 @@ func (f *featurePodConnectivity) trafficControlMarkFlows(sourceOFPorts []uint32, } // This generates the flow to mark the packets sourced from a provided port. if direction == v1alpha2.DirectionEgress || direction == v1alpha2.DirectionBoth { - flows = append(flows, TrafficControlTable.ofTable.BuildFlow(priorityNormal). + flows = append(flows, TrafficControlTable.ofTable.BuildFlow(priority). Cookie(cookieID). MatchInPort(port). Action().LoadToRegField(TrafficControlTargetOFPortField, targetOFPort). diff --git a/pkg/agent/openflow/testing/mock_openflow.go b/pkg/agent/openflow/testing/mock_openflow.go index af1e9a34cc2..8137fda4ef3 100644 --- a/pkg/agent/openflow/testing/mock_openflow.go +++ b/pkg/agent/openflow/testing/mock_openflow.go @@ -532,17 +532,17 @@ func (mr *MockClientMockRecorder) InstallTraceflowFlows(arg0, arg1, arg2, arg3, } // InstallTrafficControlMarkFlows mocks base method. -func (m *MockClient) InstallTrafficControlMarkFlows(arg0 string, arg1 []uint32, arg2 uint32, arg3 v1alpha2.Direction, arg4 v1alpha2.TrafficControlAction) error { +func (m *MockClient) InstallTrafficControlMarkFlows(arg0 string, arg1 []uint32, arg2 uint32, arg3 v1alpha2.Direction, arg4 v1alpha2.TrafficControlAction, arg5 types.TrafficControlFlowPriority) error { m.ctrl.T.Helper() - ret := m.ctrl.Call(m, "InstallTrafficControlMarkFlows", arg0, arg1, arg2, arg3, arg4) + ret := m.ctrl.Call(m, "InstallTrafficControlMarkFlows", arg0, arg1, arg2, arg3, arg4, arg5) ret0, _ := ret[0].(error) return ret0 } // InstallTrafficControlMarkFlows indicates an expected call of InstallTrafficControlMarkFlows. -func (mr *MockClientMockRecorder) InstallTrafficControlMarkFlows(arg0, arg1, arg2, arg3, arg4 any) *gomock.Call { +func (mr *MockClientMockRecorder) InstallTrafficControlMarkFlows(arg0, arg1, arg2, arg3, arg4, arg5 any) *gomock.Call { mr.mock.ctrl.T.Helper() - return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "InstallTrafficControlMarkFlows", reflect.TypeOf((*MockClient)(nil).InstallTrafficControlMarkFlows), arg0, arg1, arg2, arg3, arg4) + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "InstallTrafficControlMarkFlows", reflect.TypeOf((*MockClient)(nil).InstallTrafficControlMarkFlows), arg0, arg1, arg2, arg3, arg4, arg5) } // InstallTrafficControlReturnPortFlow mocks base method. diff --git a/pkg/agent/types/trafficcontrol.go b/pkg/agent/types/trafficcontrol.go new file mode 100644 index 00000000000..2bc2e5658d9 --- /dev/null +++ b/pkg/agent/types/trafficcontrol.go @@ -0,0 +1,28 @@ +// Copyright 2023 Antrea Authors +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package types + +// TrafficControlFlowPriority sets the priority for flows installed by OpenFlow client using InstallTrafficControlMarkFlows +// method. +type TrafficControlFlowPriority string + +const ( + // TrafficControlFlowPriorityHigh is not used yet. + TrafficControlFlowPriorityHigh TrafficControlFlowPriority = "high" + // TrafficControlFlowPriorityMedium is for user-defined TrafficControl CRs. + TrafficControlFlowPriorityMedium TrafficControlFlowPriority = "medium" + // TrafficControlFlowPriorityLow is not used yet. + TrafficControlFlowPriorityLow TrafficControlFlowPriority = "low" +) diff --git a/test/integration/agent/openflow_test.go b/test/integration/agent/openflow_test.go index 8070a45a052..210145441df 100644 --- a/test/integration/agent/openflow_test.go +++ b/test/integration/agent/openflow_test.go @@ -1874,7 +1874,7 @@ func TestTrafficControlFlows(t *testing.T) { returnOFPort := uint32(201) expectedFlows := prepareTrafficControlFlows(sourceOFPorts, targetOFPort, returnOFPort) c.InstallTrafficControlReturnPortFlow(returnOFPort) - c.InstallTrafficControlMarkFlows("tc", sourceOFPorts, targetOFPort, v1alpha2.DirectionBoth, v1alpha2.ActionRedirect) + c.InstallTrafficControlMarkFlows("tc", sourceOFPorts, targetOFPort, v1alpha2.DirectionBoth, v1alpha2.ActionRedirect, types.TrafficControlFlowPriorityMedium) for _, tableFlow := range expectedFlows { ofTestUtils.CheckFlowExists(t, ovsCtlClient, tableFlow.tableName, 0, true, tableFlow.flows) }