An atempt is made to reduce DS operations before fixed flows are

programmed for a VM

Below are change details:
(a) In AclInterfaceStateListener, DS operation to read Interface config
object was made all the time. Now, updated to check config Interface only
when AclInterface refernce is not present at that time (ie., with the
changes, config Interface is checked only when Interface state event is
triggered before Interface state event)
(b) After performing Port lookup DS write, current behaviour is to wait for
completion of this write operation. As what ever written to port lookup is
used only during delete operation, moved add/delete port lookup to a
separate thread with the same key using jobCoordinator. With this, Port
lookup DS write is unblocked and add/delete operation of it is also been
synchronized

Change-Id: Ic6300a82006d6a8c0db3ac7bb2168612a190697f
Signed-off-by: Shashidhar Raja <shashidharr@altencalsoftlabs.com>

diff --git a/aclservice/impl/src/main/java/org/opendaylight/netvirt/aclservice/listeners/AclInterfaceListener.java b/aclservice/impl/src/main/java/org/opendaylight/netvirt/aclservice/listeners/AclInterfaceListener.java
index ce8dd8d966beeeccc3ed786930274eb9fd8c4fb3..8fcb8d496a59c29df0eb58a612af08c03e401219 100644 (file)
--- a/aclservice/impl/src/main/java/org/opendaylight/netvirt/aclservice/listeners/AclInterfaceListener.java
+++ b/aclservice/impl/src/main/java/org/opendaylight/netvirt/aclservice/listeners/AclInterfaceListener.java
@@ -7,11 +7,9 @@
  */
 package org.opendaylight.netvirt.aclservice.listeners;
 
-import com.google.common.util.concurrent.Futures;
 import java.util.ArrayList;
 import java.util.HashSet;
 import java.util.List;
-import java.util.concurrent.ExecutionException;
 import javax.annotation.PostConstruct;
 import javax.inject.Inject;
 import javax.inject.Singleton;
@@ -152,21 +150,11 @@ public class AclInterfaceListener extends AsyncDataTreeChangeListenerBase<Interf
                     LOG.debug("On update event, notify ACL service manager to update ACL for interface: {}",
                             interfaceId);
                     // handle add for AclPortsLookup before processing update
-                    try {
-                        Futures.allAsList(aclServiceUtils.addAclPortsLookupForInterfaceUpdate(aclInterfaceBefore,
-                                aclInterfaceAfter)).get();
-                    } catch (InterruptedException | ExecutionException e) {
-                        LOG.error("Error adding ACL ports for interface update", e);
-                    }
+                    aclServiceUtils.addAclPortsLookupForInterfaceUpdate(aclInterfaceBefore, aclInterfaceAfter);
 
                     aclServiceManager.notify(aclInterfaceAfter, aclInterfaceBefore, AclServiceManager.Action.UPDATE);
                     // handle delete for AclPortsLookup after processing update
-                    try {
-                        Futures.allAsList(aclServiceUtils.deleteAclPortsLookupForInterfaceUpdate(aclInterfaceBefore,
-                                aclInterfaceAfter)).get();
-                    } catch (InterruptedException | ExecutionException e) {
-                        LOG.error("Error deleting ACL ports for interface update", e);
-                    }
+                    aclServiceUtils.deleteAclPortsLookupForInterfaceUpdate(aclInterfaceBefore, aclInterfaceAfter);
                 }
             }
             updateCacheWithAclChange(aclInterfaceBefore, aclInterfaceAfter);

diff --git a/aclservice/impl/src/main/java/org/opendaylight/netvirt/aclservice/listeners/AclInterfaceStateListener.java b/aclservice/impl/src/main/java/org/opendaylight/netvirt/aclservice/listeners/AclInterfaceStateListener.java
index 4e92980c1d7bb26ae2c7f551d8da32a4b71257f1..288307fc404186664d3e0fa9dc66803b2d52acfa 100644 (file)
--- a/aclservice/impl/src/main/java/org/opendaylight/netvirt/aclservice/listeners/AclInterfaceStateListener.java
+++ b/aclservice/impl/src/main/java/org/opendaylight/netvirt/aclservice/listeners/AclInterfaceStateListener.java
@@ -130,19 +130,6 @@ public class AclInterfaceStateListener extends AsyncDataTreeChangeListenerBase<I
         if (!L2vlan.class.equals(added.getType())) {
             return;
         }
-        org.opendaylight.yang.gen.v1.urn.ietf.params.xml.ns.yang.ietf.interfaces.rev140508.interfaces.Interface iface;
-        iface = interfaceManager.getInterfaceInfoFromConfigDataStore(added.getName());
-        if (iface == null) {
-            LOG.error("No interface with name {} available in interfaceConfig, servicing interfaceState ADD"
-                    + "for ACL failed", added.getName());
-            return;
-        }
-        InterfaceAcl aclInPort = iface.augmentation(InterfaceAcl.class);
-        if (aclInPort == null) {
-            LOG.trace("Interface {} is not an ACL Interface, ignoring ADD interfaceState event",
-                    added.getName());
-            return;
-        }
 
         AclInterface aclInterface = aclInterfaceCache.addOrUpdate(added.getName(), (prevAclInterface, builder) -> {
             builder.dpId(AclServiceUtils.getDpIdFromIterfaceState(added)).lPortTag(added.getIfIndex())
@@ -155,15 +142,31 @@ public class AclInterfaceStateListener extends AsyncDataTreeChangeListenerBase<I
                     builder.subnetInfo(subnetInfo);
                 }
                 SortedSet<Integer> ingressRemoteAclTags =
-                        aclServiceUtils.getRemoteAclTags(aclInPort.getSecurityGroups(), DirectionIngress.class);
+                        aclServiceUtils.getRemoteAclTags(prevAclInterface.getSecurityGroups(), DirectionIngress.class);
                 SortedSet<Integer> egressRemoteAclTags =
-                        aclServiceUtils.getRemoteAclTags(aclInPort.getSecurityGroups(), DirectionEgress.class);
+                        aclServiceUtils.getRemoteAclTags(prevAclInterface.getSecurityGroups(), DirectionEgress.class);
                 builder.ingressRemoteAclTags(ingressRemoteAclTags).egressRemoteAclTags(egressRemoteAclTags);
             }
         });
 
+        List<Uuid> aclList = aclInterface.getSecurityGroups();
+        if (aclList == null) {
+            org.opendaylight.yang.gen.v1.urn.ietf.params.xml.ns.yang.ietf.interfaces.rev140508.interfaces
+                    .Interface iface = interfaceManager.getInterfaceInfoFromConfigDataStore(added.getName());
+            if (iface == null) {
+                LOG.error("No interface with name {} available in interfaceConfig, servicing interfaceState ADD"
+                        + "for ACL failed", added.getName());
+                return;
+            }
+            InterfaceAcl aclInPort = iface.augmentation(InterfaceAcl.class);
+            if (aclInPort == null) {
+                LOG.trace("Interface {} is not an ACL Interface, ignoring ADD interfaceState event",
+                        added.getName());
+                return;
+            }
+        }
+
         if (AclServiceUtils.isOfInterest(aclInterface)) {
-            List<Uuid> aclList = aclInterface.getSecurityGroups();
             if (aclList != null) {
                 aclDataUtil.addAclInterfaceMap(aclList, aclInterface);
             }

diff --git a/aclservice/impl/src/main/java/org/opendaylight/netvirt/aclservice/utils/AclServiceUtils.java b/aclservice/impl/src/main/java/org/opendaylight/netvirt/aclservice/utils/AclServiceUtils.java
index 936e008710360d4606ebf07c895e441d03585c0b..c852f271ece41250b48ec333bebb8e8bfff98924 100644 (file)
--- a/aclservice/impl/src/main/java/org/opendaylight/netvirt/aclservice/utils/AclServiceUtils.java
+++ b/aclservice/impl/src/main/java/org/opendaylight/netvirt/aclservice/utils/AclServiceUtils.java
@@ -69,6 +69,7 @@ import org.opendaylight.genius.mdsalutil.matches.MatchUdpDestinationPort;
 import org.opendaylight.genius.mdsalutil.matches.MatchUdpSourcePort;
 import org.opendaylight.genius.mdsalutil.nxmatches.NxMatchRegister;
 import org.opendaylight.genius.mdsalutil.packet.IPProtocols;
+import org.opendaylight.infrautils.jobcoordinator.JobCoordinator;
 import org.opendaylight.netvirt.aclservice.api.AclServiceManager.MatchCriteria;
 import org.opendaylight.netvirt.aclservice.api.utils.AclInterface;
 import org.opendaylight.yang.gen.v1.urn.ietf.params.xml.ns.yang.ietf.access.control.list.rev160218.AccessLists;
@@ -165,15 +166,17 @@ public final class AclServiceUtils {
     private final AclDataUtil aclDataUtil;
     private final AclserviceConfig config;
     private final IdManagerService idManager;
+    private final JobCoordinator jobCoordinator;
 
     @Inject
     public AclServiceUtils(DataBroker dataBroker, AclDataUtil aclDataUtil, AclserviceConfig config,
-            IdManagerService idManager) {
+            IdManagerService idManager, JobCoordinator jobCoordinator) {
         this.dataBroker = dataBroker;
         this.txRunner = new ManagedNewTransactionRunnerImpl(dataBroker);
         this.aclDataUtil = aclDataUtil;
         this.config = config;
         this.idManager = idManager;
+        this.jobCoordinator = jobCoordinator;
     }
 
     /**
@@ -1264,41 +1267,35 @@ public final class AclServiceUtils {
                 .build();
     }
 
-    public List<ListenableFuture<Void>> addAclPortsLookupForInterfaceUpdate(AclInterface portBefore,
-            AclInterface portAfter) {
-        List<ListenableFuture<Void>> futures = new ArrayList<>();
+    public void addAclPortsLookupForInterfaceUpdate(AclInterface portBefore, AclInterface portAfter) {
         LOG.debug("Processing interface additions for port {}", portAfter.getInterfaceId());
         List<AllowedAddressPairs> addedAllowedAddressPairs = getUpdatedAllowedAddressPairs(
                 portAfter.getAllowedAddressPairs(), portBefore.getAllowedAddressPairs());
         if (addedAllowedAddressPairs != null && !addedAllowedAddressPairs.isEmpty()) {
-            futures.addAll(addAclPortsLookup(portAfter, portAfter.getSecurityGroups(), addedAllowedAddressPairs));
+            addAclPortsLookup(portAfter, portAfter.getSecurityGroups(), addedAllowedAddressPairs);
         }
 
         List<Uuid> addedAcls = getUpdatedAclList(portAfter.getSecurityGroups(), portBefore.getSecurityGroups());
         if (addedAcls != null && !addedAcls.isEmpty()) {
-            futures.addAll(addAclPortsLookup(portAfter, addedAcls, portAfter.getAllowedAddressPairs()));
+            addAclPortsLookup(portAfter, addedAcls, portAfter.getAllowedAddressPairs());
         }
-        return futures;
     }
 
-    public List<ListenableFuture<Void>> deleteAclPortsLookupForInterfaceUpdate(AclInterface portBefore,
-            AclInterface portAfter) {
-        List<ListenableFuture<Void>> futures = new ArrayList<>();
+    public void deleteAclPortsLookupForInterfaceUpdate(AclInterface portBefore, AclInterface portAfter) {
         LOG.debug("Processing interface removals for port {}", portAfter.getInterfaceId());
         List<AllowedAddressPairs> deletedAllowedAddressPairs = getUpdatedAllowedAddressPairs(
                 portBefore.getAllowedAddressPairs(), portAfter.getAllowedAddressPairs());
         if (deletedAllowedAddressPairs != null && !deletedAllowedAddressPairs.isEmpty()) {
-            futures.addAll(deleteAclPortsLookup(portAfter, portAfter.getSecurityGroups(), deletedAllowedAddressPairs));
+            deleteAclPortsLookup(portAfter, portAfter.getSecurityGroups(), deletedAllowedAddressPairs);
         }
 
         List<Uuid> deletedAcls = getUpdatedAclList(portBefore.getSecurityGroups(), portAfter.getSecurityGroups());
         if (deletedAcls != null && !deletedAcls.isEmpty()) {
-            futures.addAll(deleteAclPortsLookup(portAfter, deletedAcls, portAfter.getAllowedAddressPairs()));
+            deleteAclPortsLookup(portAfter, deletedAcls, portAfter.getAllowedAddressPairs());
         }
-        return futures;
     }
 
-    public List<ListenableFuture<Void>> addAclPortsLookup(AclInterface port, List<Uuid> aclList,
+    public void addAclPortsLookup(AclInterface port, List<Uuid> aclList,
             List<AllowedAddressPairs> allowedAddresses) {
         String portId = port.getInterfaceId();
         LOG.trace("Adding AclPortsLookup for port={}, acls={}, AAPs={}", portId, aclList, allowedAddresses);
@@ -1306,12 +1303,13 @@ public final class AclServiceUtils {
         if (aclList == null || allowedAddresses == null || allowedAddresses.isEmpty()) {
             LOG.warn("aclList or allowedAddresses is null. port={}, acls={}, AAPs={}", portId, aclList,
                     allowedAddresses);
-            return Collections.emptyList();
+            return;
         }
-        List<ListenableFuture<Void>> futures = new ArrayList<>();
+
         for (Uuid aclId : aclList) {
             String aclName = aclId.getValue();
-            synchronized (aclName.intern()) {
+            jobCoordinator.enqueueJob(aclName.intern(), () -> {
+                List<ListenableFuture<Void>> futures = new ArrayList<>();
                 futures.add(txRunner.callWithNewWriteOnlyTransactionAndSubmit(tx -> {
                     for (AllowedAddressPairs aap : allowedAddresses) {
                         PortIds portIdObj =
@@ -1322,12 +1320,12 @@ public final class AclServiceUtils {
                                 WriteTransaction.CREATE_MISSING_PARENTS);
                     }
                 }));
-            }
+                return futures;
+            });
         }
-        return futures;
     }
 
-    public List<ListenableFuture<Void>> deleteAclPortsLookup(AclInterface port, List<Uuid> aclList,
+    public void deleteAclPortsLookup(AclInterface port, List<Uuid> aclList,
             List<AllowedAddressPairs> allowedAddresses) {
         String portId = port.getInterfaceId();
         LOG.trace("Deleting AclPortsLookup for port={}, acls={}, AAPs={}", portId, aclList, allowedAddresses);
@@ -1335,12 +1333,13 @@ public final class AclServiceUtils {
         if (aclList == null || allowedAddresses == null || allowedAddresses.isEmpty()) {
             LOG.warn("aclList or allowedAddresses is null. port={}, acls={}, AAPs={}", portId, aclList,
                     allowedAddresses);
-            return Collections.emptyList();
+            return;
         }
-        List<ListenableFuture<Void>> futures = new ArrayList<>();
+
         for (Uuid aclId : aclList) {
             String aclName = aclId.getValue();
-            synchronized (aclName.intern()) {
+            jobCoordinator.enqueueJob(aclName.intern(), () -> {
+                List<ListenableFuture<Void>> futures = new ArrayList<>();
                 futures.add(txRunner.callWithNewWriteOnlyTransactionAndSubmit(tx -> {
                     for (AllowedAddressPairs aap : allowedAddresses) {
                         InstanceIdentifier<PortIds> path =
@@ -1350,9 +1349,9 @@ public final class AclServiceUtils {
 
                     cleanUpStaleEntriesInAclPortsLookup(aclName, tx);
                 }));
-            }
+                return futures;
+            });
         }
-        return futures;
     }
 
     private void cleanUpStaleEntriesInAclPortsLookup(String aclName, WriteTransaction tx) {