diff --git a/pkg/agent/controller/traceflow/packetin.go b/pkg/agent/controller/traceflow/packetin.go index e3644dc8b29..5d6202b9d44 100644 --- a/pkg/agent/controller/traceflow/packetin.go +++ b/pkg/agent/controller/traceflow/packetin.go @@ -129,10 +129,7 @@ func (c *Controller) parsePacketIn(pktIn *ofctrl.PacketIn) (*opsv1alpha1.Tracefl if err != nil { return nil, nil, err } - ob := new(opsv1alpha1.Observation) - ob.Component = opsv1alpha1.NetworkPolicy - ob.ComponentInfo = openflow.GetFlowTableName(openflow.EgressRuleTable) - ob.Action = opsv1alpha1.Forwarded + ob := getNetworkPolicyObservation(tableID, false) npRef := c.ofClient.GetPolicyFromConjunction(egressInfo) if npRef != nil { ob.NetworkPolicy = npRef.ToString() @@ -146,10 +143,7 @@ func (c *Controller) parsePacketIn(pktIn *ofctrl.PacketIn) (*opsv1alpha1.Tracefl if err != nil { return nil, nil, err } - ob := new(opsv1alpha1.Observation) - ob.Component = opsv1alpha1.NetworkPolicy - ob.ComponentInfo = openflow.GetFlowTableName(openflow.IngressRuleTable) - ob.Action = opsv1alpha1.Forwarded + ob := getNetworkPolicyObservation(tableID, true) npRef := c.ofClient.GetPolicyFromConjunction(ingressInfo) if npRef != nil { ob.NetworkPolicy = npRef.ToString() @@ -158,11 +152,21 @@ func (c *Controller) parsePacketIn(pktIn *ofctrl.PacketIn) (*opsv1alpha1.Tracefl } // Get drop table. - if tableID == uint8(openflow.EgressDefaultTable) || tableID == uint8(openflow.IngressDefaultTable) { - ob := new(opsv1alpha1.Observation) - ob.Action = opsv1alpha1.Dropped - ob.Component = opsv1alpha1.NetworkPolicy - ob.ComponentInfo = openflow.GetFlowTableName(binding.TableIDType(tableID)) + if tableID == uint8(openflow.EgressMetricTable) || tableID == uint8(openflow.IngressMetricTable) { + ob := getNetworkPolicyObservation(tableID, tableID == uint8(openflow.IngressMetricTable)) + if match = getMatchRegField(matchers, uint32(openflow.CNPDropConjunctionIDReg)); match != nil { + dropConjInfo, err := getInfoInReg(match, nil) + if err != nil { + return nil, nil, err + } + npRef := c.ofClient.GetPolicyFromConjunction(dropConjInfo) + if npRef != nil { + ob.NetworkPolicy = npRef.ToString() + } + } + obs = append(obs, *ob) + } else if tableID == uint8(openflow.EgressDefaultTable) || tableID == uint8(openflow.IngressDefaultTable) { + ob := getNetworkPolicyObservation(tableID, tableID == uint8(openflow.IngressDefaultTable)) obs = append(obs, *ob) } @@ -229,3 +233,30 @@ func getInfoInCtNwDstField(matchers *ofctrl.Matchers) (string, error) { } return regValue.String(), nil } + +func getNetworkPolicyObservation(tableID uint8, ingress bool) *opsv1alpha1.Observation { + ob := new(opsv1alpha1.Observation) + ob.Component = opsv1alpha1.NetworkPolicy + if ingress { + switch tableID { + case uint8(openflow.IngressMetricTable), uint8(openflow.IngressDefaultTable): + // Packet dropped by ANP/default drop rule + ob.ComponentInfo = openflow.GetFlowTableName(binding.TableIDType(tableID)) + ob.Action = opsv1alpha1.Dropped + default: + ob.ComponentInfo = openflow.GetFlowTableName(openflow.IngressRuleTable) + ob.Action = opsv1alpha1.Forwarded + } + } else { + switch tableID { + case uint8(openflow.EgressMetricTable), uint8(openflow.EgressDefaultTable): + // Packet dropped by ANP/default drop rule + ob.ComponentInfo = openflow.GetFlowTableName(binding.TableIDType(tableID)) + ob.Action = opsv1alpha1.Dropped + default: + ob.ComponentInfo = openflow.GetFlowTableName(openflow.EgressRuleTable) + ob.Action = opsv1alpha1.Forwarded + } + } + return ob +} diff --git a/pkg/agent/controller/traceflow/packetin_test.go b/pkg/agent/controller/traceflow/packetin_test.go new file mode 100644 index 00000000000..4c78d26b564 --- /dev/null +++ b/pkg/agent/controller/traceflow/packetin_test.go @@ -0,0 +1,91 @@ +// Copyright 2020 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 traceflow + +import ( + "reflect" + "testing" + + "github.com/vmware-tanzu/antrea/pkg/agent/openflow" + opsv1alpha1 "github.com/vmware-tanzu/antrea/pkg/apis/ops/v1alpha1" +) + +func Test_getNetworkPolicyObservation(t *testing.T) { + type args struct { + tableID uint8 + ingress bool + } + tests := []struct { + name string + args args + want *opsv1alpha1.Observation + }{ + { + name: "ingress metric drop", + args: args{ + tableID: uint8(openflow.IngressMetricTable), + ingress: true, + }, + want: &opsv1alpha1.Observation{ + Component: opsv1alpha1.NetworkPolicy, + ComponentInfo: "IngressMetric", + Action: opsv1alpha1.Dropped, + }, + }, + { + name: "ingress accept", + args: args{ + tableID: uint8(openflow.L2ForwardingOutTable), + ingress: true, + }, + want: &opsv1alpha1.Observation{ + Component: opsv1alpha1.NetworkPolicy, + ComponentInfo: "IngressRule", + Action: opsv1alpha1.Forwarded, + }, + }, + { + name: "egress default drop", + args: args{ + tableID: uint8(openflow.EgressDefaultTable), + ingress: false, + }, + want: &opsv1alpha1.Observation{ + Component: opsv1alpha1.NetworkPolicy, + ComponentInfo: "EgressDefaultRule", + Action: opsv1alpha1.Dropped, + }, + }, + { + name: "egress accept", + args: args{ + tableID: uint8(openflow.L2ForwardingOutTable), + ingress: false, + }, + want: &opsv1alpha1.Observation{ + Component: opsv1alpha1.NetworkPolicy, + ComponentInfo: "EgressRule", + Action: opsv1alpha1.Forwarded, + }, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + if got := getNetworkPolicyObservation(tt.args.tableID, tt.args.ingress); !reflect.DeepEqual(got, tt.want) { + t.Errorf("getNetworkPolicyObservation() = %v, want %v", got, tt.want) + } + }) + } +} diff --git a/pkg/agent/openflow/client.go b/pkg/agent/openflow/client.go index 2921fce1832..e51b2f0241a 100644 --- a/pkg/agent/openflow/client.go +++ b/pkg/agent/openflow/client.go @@ -741,6 +741,7 @@ func (c *client) InstallTraceflowFlows(dataplaneTag uint8) error { flows := []binding.Flow{} c.conjMatchFlowLock.Lock() defer c.conjMatchFlowLock.Unlock() + // Copy default drop rules for _, ctx := range c.globalConjMatchFlowCache { if ctx.dropFlow != nil { flows = append( @@ -752,6 +753,20 @@ func (c *client) InstallTraceflowFlows(dataplaneTag uint8) error { Done()) } } + // Copy Antrea NetworkPolicy drop rules + for _, conj := range c.policyCache.List() { + for _, flow := range conj.(*policyRuleConjunction).metricFlows { + if flow.IsDropFlow() { + flows = append( + flows, + flow.CopyToBuilder(priorityNormal+2, false). + MatchRegRange(int(TraceflowReg), uint32(dataplaneTag), OfTraceflowMarkRange). + SetHardTimeout(300). + Action().SendToController(1). + Done()) + } + } + } return c.AddAll(flows) } diff --git a/pkg/agent/openflow/client_test.go b/pkg/agent/openflow/client_test.go index 70a0c6ec366..fc3bfb388fd 100644 --- a/pkg/agent/openflow/client_test.go +++ b/pkg/agent/openflow/client_test.go @@ -23,6 +23,7 @@ import ( "testing" "time" + "github.com/contiv/ofnet/ofctrl" "github.com/golang/mock/gomock" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" @@ -31,6 +32,7 @@ import ( "github.com/vmware-tanzu/antrea/pkg/agent/openflow/cookie" oftest "github.com/vmware-tanzu/antrea/pkg/agent/openflow/testing" ofconfig "github.com/vmware-tanzu/antrea/pkg/ovs/openflow" + ovsoftest "github.com/vmware-tanzu/antrea/pkg/ovs/openflow/testing" "github.com/vmware-tanzu/antrea/pkg/ovs/ovsconfig" ) @@ -237,3 +239,56 @@ func TestConcurrentFlowInstallation(t *testing.T) { } } + +func Test_client_InstallTraceflowFlows(t *testing.T) { + type ofSwitch struct { + ofctrl.OFSwitch + } + type fields struct { + } + type args struct { + dataplaneTag uint8 + } + tests := []struct { + name string + fields fields + args args + wantErr bool + prepareFunc func(*gomock.Controller) *client + }{ + { + name: "traceflow flow", + fields: fields{}, + args: args{dataplaneTag: 1}, + wantErr: false, + prepareFunc: prepareTraceflowFlow, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + ctrl := gomock.NewController(t) + defer ctrl.Finish() + c := tt.prepareFunc(ctrl) + if err := c.InstallTraceflowFlows(tt.args.dataplaneTag); (err != nil) != tt.wantErr { + t.Errorf("InstallTraceflowFlows() error = %v, wantErr %v", err, tt.wantErr) + } + }) + } +} + +func prepareTraceflowFlow(ctrl *gomock.Controller) *client { + ofClient := NewClient(bridgeName, bridgeMgmtAddr, true, true) + c := ofClient.(*client) + c.cookieAllocator = cookie.NewAllocator(0) + c.nodeConfig = &config.NodeConfig{} + m := ovsoftest.NewMockBridge(ctrl) + m.EXPECT().AddFlowsInBundle(gomock.Any(), nil, nil).Return(nil).Times(3) + c.bridge = m + + mFlow := ovsoftest.NewMockFlow(ctrl) + ctx := &conjMatchFlowContext{dropFlow: mFlow} + mFlow.EXPECT().CopyToBuilder(priorityNormal+2, false).Return(c.pipeline[EgressDefaultTable].BuildFlow(priorityNormal + 2)).Times(1) + c.globalConjMatchFlowCache["mockContext"] = ctx + c.policyCache.Add(&policyRuleConjunction{metricFlows: []ofconfig.Flow{c.dropRuleMetricFlow(123, false)}}) + return c +} diff --git a/pkg/agent/openflow/pipeline.go b/pkg/agent/openflow/pipeline.go index 7a9d224d179..6af823437df 100644 --- a/pkg/agent/openflow/pipeline.go +++ b/pkg/agent/openflow/pipeline.go @@ -201,7 +201,7 @@ const ( EgressReg regType = 5 IngressReg regType = 6 TraceflowReg regType = 9 // Use reg9[28..31] to store traceflow dataplaneTag. - // CNPDropConjunctionIDReg reuses reg3 which will also be used for storing endpoint IP to store the rule ID. Since + // CNPDropConjunctionIDReg reuses reg3 which is also used for storing endpoint IP to store the rule ID. Since // the service selection will finish when a packet hitting NetworkPolicy related rules, there is no conflict. CNPDropConjunctionIDReg regType = 3 // marksRegServiceNeedLB indicates a packet need to do service selection. @@ -1006,7 +1006,7 @@ func (c *client) conjunctionActionFlow(conjunctionID uint32, tableID binding.Tab MatchConjID(conjunctionID). MatchPriority(ofPriority). Action().LoadRegRange(int(conjReg), conjunctionID, binding.Range{0, 31}). // Traceflow. - Action().CT(true, nextTable, CtZone). // CT action requires commit flag if actions other than NAT without arguments are specified. + Action().CT(true, nextTable, CtZone). // CT action requires commit flag if actions other than NAT without arguments are specified. LoadToLabelRange(uint64(conjunctionID), &labelRange). CTDone(). Cookie(c.cookieAllocator.Request(cookie.Policy).Raw()). @@ -1044,7 +1044,6 @@ func (c *client) conjunctionActionDropFlow(conjunctionID uint32, tableID binding Cookie(c.cookieAllocator.Request(cookie.Policy).Raw()). Done() } - } func (c *client) Disconnect() error { diff --git a/pkg/ovs/openflow/interfaces.go b/pkg/ovs/openflow/interfaces.go index 9e8acca1678..437d1391bfd 100644 --- a/pkg/ovs/openflow/interfaces.go +++ b/pkg/ovs/openflow/interfaces.go @@ -156,6 +156,7 @@ type Flow interface { // It copies the original actions of the Flow only if copyActions is set to true, and // resets the priority in the new FlowBuilder if the provided priority is not 0. CopyToBuilder(priority uint16, copyActions bool) FlowBuilder + IsDropFlow() bool } type Action interface { diff --git a/pkg/ovs/openflow/ofctrl_action.go b/pkg/ovs/openflow/ofctrl_action.go index 739fbf63144..cde8dcd9e89 100644 --- a/pkg/ovs/openflow/ofctrl_action.go +++ b/pkg/ovs/openflow/ofctrl_action.go @@ -17,6 +17,7 @@ type ofFlowAction struct { // Drop is an action to drop packets. func (a *ofFlowAction) Drop() FlowBuilder { a.builder.Drop() + a.builder.isDropFlow = true return a.builder } @@ -318,11 +319,13 @@ func (a *ofFlowAction) Note(notes string) FlowBuilder { } func (a *ofFlowAction) SendToController(reason uint8) FlowBuilder { - controllerAct := &ofctrl.NXController{ - ControllerID: a.builder.ofFlow.Table.Switch.GetControllerID(), - Reason: reason, + if a.builder.ofFlow.Table != nil && a.builder.ofFlow.Table.Switch != nil { + controllerAct := &ofctrl.NXController{ + ControllerID: a.builder.ofFlow.Table.Switch.GetControllerID(), + Reason: reason, + } + a.builder.ApplyAction(controllerAct) } - a.builder.ApplyAction(controllerAct) return a.builder } diff --git a/pkg/ovs/openflow/ofctrl_flow.go b/pkg/ovs/openflow/ofctrl_flow.go index 2969cf86118..61acfcf3d93 100644 --- a/pkg/ovs/openflow/ofctrl_flow.go +++ b/pkg/ovs/openflow/ofctrl_flow.go @@ -31,6 +31,8 @@ type ofFlow struct { // ctStates is a temporary variable to maintain openflow13.CTStates. When FlowBuilder.Done is called, it is used to // set the CtStates field in ofctrl.Flow.Match. ctStates *openflow13.CTStates + // isDropFlow is true if this flow actions contain "drop" + isDropFlow bool } // Reset updates the ofFlow.Flow.Table field with ofFlow.table.Table. @@ -134,9 +136,16 @@ func (f *ofFlow) CopyToBuilder(priority uint16, copyActions bool) FlowBuilder { matchers: f.matchers, protocol: f.protocol, } + if copyActions { + newFlow.isDropFlow = f.isDropFlow + } return &ofFlowBuilder{newFlow} } +func (f *ofFlow) IsDropFlow() bool { + return f.isDropFlow +} + func (r *Range) ToNXRange() *openflow13.NXRange { return openflow13.NewNXRange(int(r[0]), int(r[1])) } diff --git a/pkg/ovs/openflow/ofctrl_flow_test.go b/pkg/ovs/openflow/ofctrl_flow_test.go index 39aa21d6a1b..4bc23da6633 100644 --- a/pkg/ovs/openflow/ofctrl_flow_test.go +++ b/pkg/ovs/openflow/ofctrl_flow_test.go @@ -29,3 +29,24 @@ func TestCopyToBuilder(t *testing.T) { newFlow2 := oriFlow.CopyToBuilder(newPriority, false) assert.Equal(t, newPriority, newFlow2.Done().(*ofFlow).Match.Priority) } + +func TestCopyToBuilder_Drop(t *testing.T) { + table := &ofTable{ + id: 0, + next: 1, + } + oriFlow := table.BuildFlow(uint16(100)).MatchProtocol(ProtocolIP). + Cookie(uint64(1004)). + MatchRegRange(1, 0x101, Range{0, 15}). + MatchCTStateNew(true).MatchCTStateTrk(true). + Action().Drop(). + Done() + newFlow := oriFlow.CopyToBuilder(0, false) + assert.Equal(t, oriFlow.MatchString(), newFlow.Done().MatchString()) + assert.Equal(t, oriFlow.(*ofFlow).Match, newFlow.Done().(*ofFlow).Match) + assert.Equal(t, false, newFlow.Done().IsDropFlow()) + newPriority := uint16(200) + newFlow2 := oriFlow.CopyToBuilder(newPriority, true) + assert.Equal(t, newPriority, newFlow2.Done().(*ofFlow).Match.Priority) + assert.Equal(t, true, newFlow2.Done().IsDropFlow()) +} diff --git a/pkg/ovs/openflow/testing/mock_openflow.go b/pkg/ovs/openflow/testing/mock_openflow.go index f74c570b9f5..9c9b9913e8d 100644 --- a/pkg/ovs/openflow/testing/mock_openflow.go +++ b/pkg/ovs/openflow/testing/mock_openflow.go @@ -462,6 +462,20 @@ func (mr *MockFlowMockRecorder) GetBundleMessage(arg0 interface{}) *gomock.Call return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetBundleMessage", reflect.TypeOf((*MockFlow)(nil).GetBundleMessage), arg0) } +// IsDropFlow mocks base method +func (m *MockFlow) IsDropFlow() bool { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "IsDropFlow") + ret0, _ := ret[0].(bool) + return ret0 +} + +// IsDropFlow indicates an expected call of IsDropFlow +func (mr *MockFlowMockRecorder) GetDropFlag() *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "IsDropFlow", reflect.TypeOf((*MockFlow)(nil).IsDropFlow)) +} + // KeyString mocks base method func (m *MockFlow) KeyString() string { m.ctrl.T.Helper()