Stale flows in ACL tables 216/246 46/77246/2
authorSomashekar Byrappa <somashekar.b@altencalsoftlabs.com>
Wed, 24 Oct 2018 07:13:35 +0000 (12:43 +0530)
committerSam Hague <shague@redhat.com>
Thu, 25 Oct 2018 23:00:18 +0000 (23:00 +0000)
Issue:
------
Stale flows with VM IP's are observed in ACL tables 216/246 when a VM
is deleted which was having SG with --remote-group rules.

Solution:
---------
The issue is due to race-conditions.
Processing VM deletion is handled via DJC where-as deletion of
AclInterface from cache is not. This was resulting in stale flows.
Now, DJC with same key (ACL-ID) is used even for deletion of
AclInterface from cache which will sequence the operations.

JIRA: NETVIRT-1469

Change-Id: I315831b62a2ac5954fa4177a755c828a1f43d476
Signed-off-by: Somashekar Byrappa <somashekar.b@altencalsoftlabs.com>
aclservice/impl/src/main/java/org/opendaylight/netvirt/aclservice/listeners/AclInterfaceStateListener.java
aclservice/impl/src/main/java/org/opendaylight/netvirt/aclservice/utils/AclDataUtil.java

index 40e4ebc12b220021ad52a2f6f1644e9e4c05a0cb..6e67892e7f1386ce22a2ffc237b2cd2f0f853ffa 100644 (file)
@@ -7,6 +7,7 @@
  */
 package org.opendaylight.netvirt.aclservice.listeners;
 
+import java.util.Collections;
 import java.util.List;
 import java.util.SortedSet;
 import javax.annotation.PostConstruct;
@@ -17,6 +18,7 @@ import org.opendaylight.controller.md.sal.binding.api.DataBroker;
 import org.opendaylight.controller.md.sal.common.api.data.LogicalDatastoreType;
 import org.opendaylight.genius.datastoreutils.AsyncDataTreeChangeListenerBase;
 import org.opendaylight.genius.interfacemanager.interfaces.IInterfaceManager;
+import org.opendaylight.infrautils.jobcoordinator.JobCoordinator;
 import org.opendaylight.netvirt.aclservice.api.AclInterfaceCache;
 import org.opendaylight.netvirt.aclservice.api.AclServiceManager;
 import org.opendaylight.netvirt.aclservice.api.AclServiceManager.Action;
@@ -52,11 +54,12 @@ public class AclInterfaceStateListener extends AsyncDataTreeChangeListenerBase<I
     private final IInterfaceManager interfaceManager;
     private final AclInterfaceCache aclInterfaceCache;
     private final AclServiceUtils aclServiceUtils;
+    protected final JobCoordinator jobCoordinator;
 
     @Inject
     public AclInterfaceStateListener(AclServiceManager aclServiceManger, AclClusterUtil aclClusterUtil,
             DataBroker dataBroker, AclDataUtil aclDataUtil, IInterfaceManager interfaceManager,
-            AclInterfaceCache aclInterfaceCache, AclServiceUtils aclServicUtils,
+            AclInterfaceCache aclInterfaceCache, AclServiceUtils aclServicUtils, JobCoordinator jobCoordinator,
             ServiceRecoveryRegistry serviceRecoveryRegistry) {
         super(Interface.class, AclInterfaceStateListener.class);
         this.aclServiceManger = aclServiceManger;
@@ -66,6 +69,7 @@ public class AclInterfaceStateListener extends AsyncDataTreeChangeListenerBase<I
         this.interfaceManager = interfaceManager;
         this.aclInterfaceCache = aclInterfaceCache;
         this.aclServiceUtils = aclServicUtils;
+        this.jobCoordinator = jobCoordinator;
         serviceRecoveryRegistry.addRecoverableListener(AclServiceUtils.getRecoverServiceRegistryKey(), this);
     }
 
@@ -105,7 +109,12 @@ public class AclInterfaceStateListener extends AsyncDataTreeChangeListenerBase<I
                 }
             }
             if (aclList != null) {
-                aclDataUtil.removeAclInterfaceMap(aclList, aclInterface);
+                for (Uuid acl : aclList) {
+                    jobCoordinator.enqueueJob(acl.getValue().intern(), () -> {
+                        aclDataUtil.removeAclInterfaceMap(acl, aclInterface);
+                        return Collections.emptyList();
+                    });
+                }
             }
         }
     }
index 0ca87944899dfeb035539992898f325ee72975f6..dd4ab5d387e02ef505cabd98b1b72bbe8d710c56 100644 (file)
@@ -91,10 +91,14 @@ public class AclDataUtil implements AclDataCache {
 
     public void removeAclInterfaceMap(List<Uuid> aclList, AclInterface port) {
         for (Uuid acl : aclList) {
-            ConcurrentMap<String, AclInterface> interfaceMap = aclInterfaceMap.get(acl);
-            if (interfaceMap != null) {
-                interfaceMap.remove(port.getInterfaceId());
-            }
+            removeAclInterfaceMap(acl, port);
+        }
+    }
+
+    public void removeAclInterfaceMap(Uuid acl, AclInterface port) {
+        ConcurrentMap<String, AclInterface> interfaceMap = aclInterfaceMap.get(acl);
+        if (interfaceMap != null) {
+            interfaceMap.remove(port.getInterfaceId());
         }
     }