Fix K8s NP Service Logging

pkg/agent/controller/networkpolicy/audit_logging.go

@@ -206,26 +206,35 @@ func getNetworkPolicyInfo(pktIn *ofctrl.PacketIn, c *Controller, ob *logInfo) er
 		}
 	}
 
-	// Set match to corresponding ingress/egress reg according to disposition.
-	match = getMatch(matchers, tableID, disposition)
-
-	if match != nil {
-		// Get NetworkPolicy full name and OF priority of the conjunction.
-		conjID, err := getInfoInReg(match, nil)
+	// Get K8s default deny action, if traffic is default deny, no conjunction could be matched.
+	if match = getMatchRegField(matchers, openflow.APDenyRegMark.GetField()); match != nil {
+		cnpDenyRegVal, err := getInfoInReg(match, openflow.APDenyRegMark.GetField().GetRange().ToNXRange())
 		if err != nil {
-			return fmt.Errorf("received error while unloading conjunction id from reg: %v", err)
-		}
-		ob.npRef, ob.ofPriority, ob.ruleName = c.ofClient.GetPolicyInfoFromConjunction(conjID)
-		if ob.npRef == "" || ob.ofPriority == "" {
-			return fmt.Errorf("networkpolicy not found for conjunction id: %v", conjID)
+			return fmt.Errorf("received error while unloading deny mark from reg: %v", err)
 		}
-		// Placeholder for K8s NetworkPolicies without rule names.
-		if ob.ruleName == "" {
-			ob.ruleName = "<nil>"
+		isK8sDefaultDeny := (cnpDenyRegVal == 0) && (disposition == openflow.DispositionDrop || disposition == openflow.DispositionRej)
+		if isK8sDefaultDeny {
+			// For K8s NetworkPolicy implicit drop action, we cannot get Namespace/name.
+			ob.npRef, ob.ofPriority, ob.ruleName = string(v1beta2.K8sNetworkPolicy), "<nil>", "<nil>"
+			return nil
 		}
-	} else {
-		// For K8s NetworkPolicy implicit drop action, we cannot get Namespace/name.
-		ob.npRef, ob.ofPriority, ob.ruleName = string(v1beta2.K8sNetworkPolicy), "<nil>", "<nil>"
+	}
+
+	// Set match to corresponding conjunction ID field according to disposition.
+	match = getMatch(matchers, tableID, disposition)
+
+	// Get NetworkPolicy full name and OF priority of the conjunction.
+	conjID, err := getInfoInReg(match, nil)
+	if err != nil {
+		return fmt.Errorf("received error while unloading conjunction id from reg: %v", err)
+	}
+	ob.npRef, ob.ofPriority, ob.ruleName = c.ofClient.GetPolicyInfoFromConjunction(conjID)
+	if ob.npRef == "" || ob.ofPriority == "" {
+		return fmt.Errorf("networkpolicy not found for conjunction id: %v", conjID)
+	}
+	// Placeholder for K8s NetworkPolicies without rule names.
+	if ob.ruleName == "" {
+		ob.ruleName = "<nil>"
 	}
 	return nil
 }

pkg/agent/controller/networkpolicy/audit_logging_test.go

@@ -260,7 +260,8 @@ func TestGetNetworkPolicyInfo(t *testing.T) {
 	testK8sRef := "K8sNetworkPolicy:default/test-anp"
 	testPriority, testRule := "61800", "test-rule"
 	allowDispositionData := []byte{0x11, 0x00, 0x00, 0x11}
-	dropDispositionData := []byte{0x11, 0x00, 0x08, 0x11}
+	dropCNPDispositionData := []byte{0x11, 0x00, 0x0c, 0x11}
+	dropK8sDispositionData := []byte{0x11, 0x00, 0x08, 0x11}
 	redirectDispositionData := []byte{0x11, 0x10, 0x00, 0x11}
 	ingressData := []byte{0x11, 0x11, 0x11, 0x11, 0x11, 0x11, 0x11, 0x11}
 	tests := []struct {
@@ -311,7 +312,7 @@ func TestGetNetworkPolicyInfo(t *testing.T) {
 				mockClient.GetPolicyInfoFromConjunction(gomock.Any()).Return(
 					testANPRef, testPriority, testRule)
 			},
-			dispositionData: dropDispositionData,
+			dispositionData: dropCNPDispositionData,
 			wantOb: &logInfo{
 				tableName:   openflow.AntreaPolicyIngressRuleTable.GetName(),
 				disposition: actionDrop,
@@ -323,7 +324,7 @@ func TestGetNetworkPolicyInfo(t *testing.T) {
 		{
 			name:            "K8s Drop",
 			tableID:         openflow.IngressDefaultTable.GetID(),
-			dispositionData: dropDispositionData,
+			dispositionData: dropK8sDispositionData,
 			wantOb: &logInfo{
 				tableName:   openflow.IngressDefaultTable.GetName(),
 				disposition: actionDrop,
@@ -359,7 +360,7 @@ func TestGetNetworkPolicyInfo(t *testing.T) {
 			if tc.expectedCalls != nil {
 				regID := openflow.TFIngressConjIDField.GetRegID()
 				if tc.wantOb.disposition == actionDrop {
-					regID = openflow.CNPConjIDField.GetRegID()
+					regID = openflow.APConjIDField.GetRegID()
 				}
 				ingressMatch := generateMatch(regID, ingressData)
 				matchers = append(matchers, ingressMatch)

pkg/agent/controller/networkpolicy/packetin.go

@@ -78,7 +78,7 @@ func getMatchRegField(matchers *ofctrl.Matchers, field *binding.RegField) *ofctr
 func getMatch(matchers *ofctrl.Matchers, tableID uint8, disposition uint32) *ofctrl.MatchField {
 	// Get match from CNPDenyConjIDReg if disposition is Drop or Reject.
 	if disposition == openflow.DispositionDrop || disposition == openflow.DispositionRej {
-		return getMatchRegField(matchers, openflow.CNPConjIDField)
+		return getMatchRegField(matchers, openflow.APConjIDField)
 	}
 	// Get match from ingress/egress reg if disposition is Allow or Pass.
 	for _, table := range append(openflow.GetAntreaPolicyEgressTables(), openflow.EgressRuleTable) {

pkg/agent/controller/traceflow/packetin.go

@@ -228,7 +228,7 @@ func (c *Controller) parsePacketIn(pktIn *ofctrl.PacketIn) (*crdv1alpha1.Tracefl
 	// Get drop table.
 	if tableID == openflow.EgressMetricTable.GetID() || tableID == openflow.IngressMetricTable.GetID() {
 		ob := getNetworkPolicyObservation(tableID, tableID == openflow.IngressMetricTable.GetID())
-		if match := getMatchRegField(matchers, openflow.CNPConjIDField); match != nil {
+		if match := getMatchRegField(matchers, openflow.APConjIDField); match != nil {
 			notAllowConjInfo, err := getRegValue(match, nil)
 			if err != nil {
 				return nil, nil, nil, err

pkg/agent/openflow/fields.go

@@ -57,8 +57,9 @@ var (
 	// reg0[9]: Field to indicate whether the packet's source / destination MAC address needs to be rewritten.
 	RewriteMACRegMark    = binding.NewOneBitRegMark(0, 9)
 	NotRewriteMACRegMark = binding.NewOneBitZeroRegMark(0, 9)
-	// reg0[10]: Mark to indicate the packet is denied(Drop/Reject).
-	CnpDenyRegMark = binding.NewOneBitRegMark(0, 10)
+	// reg0[10]: Mark to indicate the packet is denied(Drop/Reject) for Antrea Policy.
+	// K8s default drop will not be recorded in this reg.
+	APDenyRegMark = binding.NewOneBitRegMark(0, 10)
 	// reg0[11..12]: Field to indicate disposition of Antrea Policy. It could have more bits to support more dispositions
 	// that Antrea Policy support in the future. Marks in this field include:
 	//   - 0b00: allow
@@ -100,9 +101,9 @@ var (
 	// reg3(NXM_NX_REG3)
 	// Field to store the selected Service Endpoint IP
 	EndpointIPField = binding.NewRegField(3, 0, 31)
-	// Field to store the conjunction ID which is for rule in CNP. It shares the same register with EndpointIPField,
+	// Field to store the conjunction ID which is for rule in Antrea Policy. It shares the same register with EndpointIPField,
 	// since the service selection will finish when a packet hitting NetworkPolicy related rules.
-	CNPConjIDField = binding.NewRegField(3, 0, 31)
+	APConjIDField = binding.NewRegField(3, 0, 31)
 
 	// reg4(NXM_NX_REG4)
 	// reg4[0..15]: Field to store the selected Service Endpoint port.

pkg/agent/openflow/network_policy_test.go

@@ -479,18 +479,18 @@ func TestBatchInstallPolicyRuleFlows(t *testing.T) {
 						Action().CT(true, IngressMetricTable.GetID(), CtZone, nil).LoadToLabelField(10, IngressRuleCTLabel).CTDone().Done(),
 					AntreaPolicyIngressRuleTable.ofTable.BuildFlow(priority100).Cookie(cookiePolicy).
 						MatchConjID(11).
-						Action().LoadToRegField(CNPConjIDField, 11).
-						Action().LoadRegMark(CnpDenyRegMark).
+						Action().LoadToRegField(APConjIDField, 11).
+						Action().LoadRegMark(APDenyRegMark).
 						Action().GotoTable(IngressMetricTable.GetID()).Done(),
 					AntreaPolicyIngressRuleTable.ofTable.BuildFlow(priority200).Cookie(cookiePolicy).
 						MatchConjID(12).
-						Action().LoadToRegField(CNPConjIDField, 12).
-						Action().LoadRegMark(CnpDenyRegMark).
+						Action().LoadToRegField(APConjIDField, 12).
+						Action().LoadRegMark(APDenyRegMark).
 						Action().GotoTable(IngressMetricTable.GetID()).Done(),
 					AntreaPolicyIngressRuleTable.ofTable.BuildFlow(priority201).Cookie(cookiePolicy).
 						MatchConjID(13).
-						Action().LoadToRegField(CNPConjIDField, 13).
-						Action().LoadRegMark(CnpDenyRegMark).
+						Action().LoadToRegField(APConjIDField, 13).
+						Action().LoadRegMark(APDenyRegMark).
 						Action().GotoTable(IngressMetricTable.GetID()).Done(),
 					AntreaPolicyIngressRuleTable.ofTable.BuildFlow(priority100).Cookie(cookiePolicy).
 						MatchProtocol(binding.ProtocolIP).MatchSrcIP(net.ParseIP("192.168.1.40")).
@@ -546,13 +546,13 @@ func TestBatchInstallPolicyRuleFlows(t *testing.T) {
 						MatchProtocol(binding.ProtocolIP).MatchCTStateNew(false).MatchCTLabelField(0, 10, IngressRuleCTLabel).
 						Action().NextTable().Done(),
 					IngressMetricTable.ofTable.BuildFlow(priorityNormal).Cookie(cookiePolicy).
-						MatchRegMark(CnpDenyRegMark).MatchRegFieldWithValue(CNPConjIDField, 11).
+						MatchRegMark(APDenyRegMark).MatchRegFieldWithValue(APConjIDField, 11).
 						Action().Drop().Done(),
 					IngressMetricTable.ofTable.BuildFlow(priorityNormal).Cookie(cookiePolicy).
-						MatchRegMark(CnpDenyRegMark).MatchRegFieldWithValue(CNPConjIDField, 12).
+						MatchRegMark(APDenyRegMark).MatchRegFieldWithValue(APConjIDField, 12).
 						Action().Drop().Done(),
 					IngressMetricTable.ofTable.BuildFlow(priorityNormal).Cookie(cookiePolicy).
-						MatchRegMark(CnpDenyRegMark).MatchRegFieldWithValue(CNPConjIDField, 13).
+						MatchRegMark(APDenyRegMark).MatchRegFieldWithValue(APConjIDField, 13).
 						Action().Drop().Done(),
 					IngressDefaultTable.ofTable.BuildFlow(priority200).Cookie(cookiePolicy).
 						MatchTunnelID(uint64(UnknownLabelIdentity)).

pkg/agent/openflow/pipeline.go

@@ -1667,7 +1667,7 @@ func (f *featureNetworkPolicy) allowRulesMetricFlows(conjunctionID uint32, ingre
 	if metricTable == MulticastEgressMetricTable || metricTable == MulticastIngressMetricTable {
 		flow := metricTable.ofTable.BuildFlow(priorityNormal).
 			Cookie(f.cookieAllocator.Request(f.category).Raw()).
-			MatchRegFieldWithValue(CNPConjIDField, conjunctionID).
+			MatchRegFieldWithValue(APConjIDField, conjunctionID).
 			Action().GotoTable(metricTable.GetNext()).
 			Done()
 		flows = append(flows, flow)
@@ -1696,8 +1696,8 @@ func (f *featureNetworkPolicy) denyRuleMetricFlow(conjunctionID uint32, ingress
 	}
 	return metricTable.ofTable.BuildFlow(priorityNormal).
 		Cookie(f.cookieAllocator.Request(f.category).Raw()).
-		MatchRegMark(CnpDenyRegMark).
-		MatchRegFieldWithValue(CNPConjIDField, conjunctionID).
+		MatchRegMark(APDenyRegMark).
+		MatchRegFieldWithValue(APConjIDField, conjunctionID).
 		Action().Drop().
 		Done()
 }
@@ -1826,7 +1826,7 @@ func (f *featureNetworkPolicy) conjunctionActionFlow(conjunctionID uint32, table
 	// Any matched flow will be resubmitted to next table in corresponding metric tables.
 	if f.enableMulticast && (tableID == MulticastEgressRuleTable.GetID() || tableID == MulticastIngressRuleTable.GetID()) {
 		flow := table.BuildFlow(ofPriority).MatchConjID(conjunctionID).
-			Action().LoadToRegField(CNPConjIDField, conjunctionID).
+			Action().LoadToRegField(APConjIDField, conjunctionID).
 			Action().NextTable().
 			Cookie(f.cookieAllocator.Request(f.category).Raw()).
 			Done()
@@ -1858,8 +1858,8 @@ func (f *featureNetworkPolicy) conjunctionActionDenyFlow(conjunctionID uint32, t
 	}
 	flowBuilder := table.BuildFlow(ofPriority).
 		MatchConjID(conjunctionID).
-		Action().LoadToRegField(CNPConjIDField, conjunctionID).
-		Action().LoadRegMark(CnpDenyRegMark)
+		Action().LoadToRegField(APConjIDField, conjunctionID).
+		Action().LoadRegMark(APDenyRegMark)
 
 	var customReason int
 	if f.enableDenyTracking {