Automated cherry pick of #4992: Use LOCAL instead of CONTROLLER as the in_port of packet-out #4986: Fix policy not being cleaned if a policy with the same name

pkg/agent/proxy/proxier.go

@@ -23,6 +23,7 @@ import (
 	"sync"
 	"time"
 
+	"antrea.io/libOpenflow/openflow15"
 	"antrea.io/libOpenflow/protocol"
 	"antrea.io/ofnet/ofctrl"
 	corev1 "k8s.io/api/core/v1"
@@ -1034,12 +1035,15 @@ func (p *proxier) HandlePacketIn(pktIn *ofctrl.PacketIn) error {
 		return fmt.Errorf("error when getting match field inPort")
 	}
 	outPort := inPortField.GetValue().(uint32)
+	// It cannot use CONTROLLER (the default value when inPort is 0) as the inPort due to a bug in Windows ovsext
+	// driver, otherwise the Windows OS would crash. See https://github.com/openvswitch/ovs-issues/issues/280.
+	inPort := uint32(openflow15.P_LOCAL)
 	return openflow.SendRejectPacketOut(p.ofClient,
 		srcMAC,
 		dstMAC,
 		srcIP,
 		dstIP,
-		0,
+		inPort,
 		outPort,
 		isIPv6,
 		ethernetPkt,

pkg/controller/networkpolicy/networkpolicy_controller.go

@@ -1425,21 +1425,24 @@ func (n *NetworkPolicyController) syncInternalNetworkPolicy(key *controlplane.Ne
 	switch key.Type {
 	case controlplane.AntreaClusterNetworkPolicy:
 		cnp, err := n.cnpLister.Get(key.Name)
-		if err != nil {
+		// We need to check if the UID matches because it's possible another policy is created with the same name after
+		// the policy is deleted. It's safe to just delete the internal NetworkPolicy associated with the old policy as
+		// the two policies are different items in the workqueue and internalNetworkPolicyStore due to different UIDs.
+		if err != nil || cnp.UID != key.UID {
 			n.deleteInternalNetworkPolicy(internalNetworkPolicyName)
 			return nil
 		}
 		newInternalNetworkPolicy, newAppliedToGroups, newAddressGroups = n.processClusterNetworkPolicy(cnp)
 	case controlplane.AntreaNetworkPolicy:
 		anp, err := n.anpLister.NetworkPolicies(key.Namespace).Get(key.Name)
-		if err != nil {
+		if err != nil || anp.UID != key.UID {
 			n.deleteInternalNetworkPolicy(internalNetworkPolicyName)
 			return nil
 		}
 		newInternalNetworkPolicy, newAppliedToGroups, newAddressGroups = n.processAntreaNetworkPolicy(anp)
 	case controlplane.K8sNetworkPolicy:
 		knp, err := n.networkPolicyLister.NetworkPolicies(key.Namespace).Get(key.Name)
-		if err != nil {
+		if err != nil || knp.UID != key.UID {
 			n.deleteInternalNetworkPolicy(internalNetworkPolicyName)
 			return nil
 		}

pkg/controller/networkpolicy/networkpolicy_controller_test.go

@@ -3044,6 +3044,92 @@ func TestSyncInternalNetworkPolicy(t *testing.T) {
 	checkGroupItemExistence(t, c.appliedToGroupStore)
 }
 
+// TestSyncInternalNetworkPolicyWithSameName verifies SyncInternalNetworkPolicy can work correctly when processing
+// multiple NetworkPolicies that have the same name.
+func TestSyncInternalNetworkPolicyWithSameName(t *testing.T) {
+	// policyA and policyB have the same name but different UIDs.
+	policyA := &networkingv1.NetworkPolicy{
+		ObjectMeta: metav1.ObjectMeta{Namespace: "default", Name: "foo", UID: "uidA"},
+		Spec: networkingv1.NetworkPolicySpec{
+			PodSelector: selectorA,
+		},
+	}
+	policyB := &networkingv1.NetworkPolicy{
+		ObjectMeta: metav1.ObjectMeta{Namespace: "default", Name: "foo", UID: "uidB"},
+		Spec: networkingv1.NetworkPolicySpec{
+			PodSelector: selectorB,
+		},
+	}
+
+	selectorAGroup := getNormalizedUID(antreatypes.NewGroupSelector("default", &selectorA, nil, nil, nil).NormalizedName)
+	selectorBGroup := getNormalizedUID(antreatypes.NewGroupSelector("default", &selectorB, nil, nil, nil).NormalizedName)
+	expectedPolicyA := &antreatypes.NetworkPolicy{
+		UID:      "uidA",
+		Name:     "uidA",
+		SpanMeta: antreatypes.SpanMeta{NodeNames: sets.NewString()},
+		SourceRef: &controlplane.NetworkPolicyReference{
+			Type:      controlplane.K8sNetworkPolicy,
+			Namespace: "default",
+			Name:      "foo",
+			UID:       "uidA",
+		},
+		AppliedToGroups: []string{selectorAGroup},
+		Rules:           []controlplane.NetworkPolicyRule{},
+	}
+	expectedPolicyB := &antreatypes.NetworkPolicy{
+		UID:      "uidB",
+		Name:     "uidB",
+		SpanMeta: antreatypes.SpanMeta{NodeNames: sets.NewString()},
+		SourceRef: &controlplane.NetworkPolicyReference{
+			Type:      controlplane.K8sNetworkPolicy,
+			Namespace: "default",
+			Name:      "foo",
+			UID:       "uidB",
+		},
+		AppliedToGroups: []string{selectorBGroup},
+		Rules:           []controlplane.NetworkPolicyRule{},
+	}
+
+	// Add and sync policyA first, it should create an AppliedToGroup.
+	_, c := newController()
+	c.networkPolicyStore.Add(policyA)
+	networkPolicyRefA := getKNPReference(policyA)
+	assert.NoError(t, c.syncInternalNetworkPolicy(networkPolicyRefA))
+	obj, exists, _ := c.internalNetworkPolicyStore.Get(expectedPolicyA.Name)
+	require.True(t, exists)
+	assert.Equal(t, expectedPolicyA, obj.(*antreatypes.NetworkPolicy))
+	checkGroupItemExistence(t, c.appliedToGroupStore, selectorAGroup)
+
+	// Delete policyA and add policyB, then sync them concurrently, the resources associated with policyA should be deleted.
+	c.networkPolicyStore.Delete(policyA)
+	c.networkPolicyStore.Add(policyB)
+	networkPolicyRefB := getKNPReference(policyB)
+	var wg sync.WaitGroup
+	wg.Add(2)
+	go func() {
+		defer wg.Done()
+		assert.NoError(t, c.syncInternalNetworkPolicy(networkPolicyRefA))
+	}()
+	go func() {
+		defer wg.Done()
+		assert.NoError(t, c.syncInternalNetworkPolicy(networkPolicyRefB))
+	}()
+	wg.Wait()
+	_, exists, _ = c.internalNetworkPolicyStore.Get(expectedPolicyA.Name)
+	require.False(t, exists)
+	obj, exists, _ = c.internalNetworkPolicyStore.Get(expectedPolicyB.Name)
+	require.True(t, exists)
+	assert.Equal(t, expectedPolicyB, obj.(*antreatypes.NetworkPolicy))
+	checkGroupItemExistence(t, c.appliedToGroupStore, selectorBGroup)
+
+	// Delete policyB and sync it, the resources associated with policyB should be deleted.
+	c.networkPolicyStore.Delete(policyB)
+	assert.NoError(t, c.syncInternalNetworkPolicy(networkPolicyRefB))
+	_, exists, _ = c.internalNetworkPolicyStore.Get(expectedPolicyB.Name)
+	require.False(t, exists)
+	checkGroupItemExistence(t, c.appliedToGroupStore)
+}
+
 // TestSyncInternalNetworkPolicyConcurrently verifies SyncInternalNetworkPolicy can create and delete AppliedToGroups
 // and AddressGroups correctly when concurrently processing multiple NetworkPolicies that refer to the same groups.
 func TestSyncInternalNetworkPolicyConcurrently(t *testing.T) {