VPN-to-Router Associate/Disas Performance Improve

Problem Description:
====================
When BGP-VPN is getting associated with router, FIB entries
are getting programmed very slow. This will cause the
performance of the VPN Manager.

Solution:
=========
Delay root cause: Each VPN interface update is taking 2 sec time to
remove the old VPN ID and update the new VPN ID and VPN service
unbind and bound to new VPN Instance.

To avoid each VPN interface update 2sec time delay, have moved
the method "handleVpnSwapForVpnInterface" processing logic into
JobCoordinator to process each VPN interface update in parallel.
This way overall FIB entry update is consuming less time compare
to without having this code change.

Issue: NETVIRT-1182

Change-Id: Ic17cba3e325a0c63fead9d30ec857c0e7eb8dcfc
Signed-off-by: Karthikeyan Krishnan <karthikeyangceb007@gmail.com>

diff --git a/neutronvpn/impl/src/main/java/org/opendaylight/netvirt/neutronvpn/NeutronBgpvpnChangeListener.java b/neutronvpn/impl/src/main/java/org/opendaylight/netvirt/neutronvpn/NeutronBgpvpnChangeListener.java
index 106447a2bb868b4738e443eb47b01ddb60c33c9d..be20ebcf3b4b13c8eee36d813f3270d7cc77ea1b 100644 (file)
--- a/neutronvpn/impl/src/main/java/org/opendaylight/netvirt/neutronvpn/NeutronBgpvpnChangeListener.java
+++ b/neutronvpn/impl/src/main/java/org/opendaylight/netvirt/neutronvpn/NeutronBgpvpnChangeListener.java
@@ -304,7 +304,13 @@ public class NeutronBgpvpnChangeListener extends AsyncDataTreeChangeListenerBase
                     if (oldRouters != null && oldRouters.contains(routerId)) {
                         continue;
                     }
-                    if (validateRouteInfo(routerId)) {
+                    /* If the first time BGP-VPN is getting associated with router, then no need
+                       to validate if the router is already been associated with any other BGP-VPN.
+                       This will avoid unnecessary MD-SAL data store read operations in VPN-MAPS.
+                     */
+                    if (oldRouters == null || oldRouters.isEmpty()) {
+                        nvpnManager.associateRouterToVpn(vpnId, routerId);
+                    } else if (validateRouteInfo(routerId)) {
                         nvpnManager.associateRouterToVpn(vpnId, routerId);
                     }
                 }

diff --git a/vpnmanager/impl/src/main/java/org/opendaylight/netvirt/vpnmanager/VpnInterfaceManager.java b/vpnmanager/impl/src/main/java/org/opendaylight/netvirt/vpnmanager/VpnInterfaceManager.java
index a017a3599adb7e6c150a316d0072b5f46ad9fdf7..5ed64a5feb1a69847009563d811ac8b15079706a 100755 (executable)
--- a/vpnmanager/impl/src/main/java/org/opendaylight/netvirt/vpnmanager/VpnInterfaceManager.java
+++ b/vpnmanager/impl/src/main/java/org/opendaylight/netvirt/vpnmanager/VpnInterfaceManager.java
@@ -1213,15 +1213,14 @@ public class VpnInterfaceManager extends AsyncDataTreeChangeListenerBase<VpnInte
                 return Collections.singletonList(future);
             }, DJC_MAX_RETRIES);
         } else {
-            Interface interfaceState = InterfaceUtils.getInterfaceStateFromOperDS(dataBroker, interfaceName);
-            removeVpnInterfaceFromVpn(identifier, vpnInterface, vpnName, interfaceName, interfaceState);
+            removeVpnInterfaceFromVpn(identifier, vpnInterface, vpnName, interfaceName);
         }
     }
 
     @SuppressFBWarnings("DLS_DEAD_LOCAL_STORE")
     private void removeVpnInterfaceFromVpn(final InstanceIdentifier<VpnInterface> identifier,
                                 final VpnInterface vpnInterface, final String vpnName,
-                                final String interfaceName, final Interface interfaceState) {
+                                final String interfaceName) {
         LOG.info("remove: VPN Interface remove event - intfName {} vpn {} dpn {}" ,vpnInterface.getName(),
                 vpnName, vpnInterface.getDpnId());
         removeInterfaceFromUnprocessedList(identifier, vpnInterface);
@@ -1247,6 +1246,8 @@ public class VpnInterfaceManager extends AsyncDataTreeChangeListenerBase<VpnInte
                                         vpnName);
                                 return;
                             }
+                            Interface interfaceState = InterfaceUtils.getInterfaceStateFromOperDS(dataBroker,
+                                    interfaceName);
                             if (interfaceState != null) {
                                 try {
                                     dpId = InterfaceUtils.getDpIdFromInterface(interfaceState);
@@ -1518,21 +1519,21 @@ public class VpnInterfaceManager extends AsyncDataTreeChangeListenerBase<VpnInte
 
         LOG.info("VPN Interface update event - intfName {}", vpnInterfaceName);
         //handles switching between <internal VPN - external VPN>
-        if (handleVpnSwapForVpnInterface(identifier, original, update)) {
-            LOG.info("update: handled VPNInterface {} on dpn {} update"
-                     + "upon VPN swap from oldVpn(s) {} to newVpn(s) {}",
-                     original.getName(), dpnId,
-                     VpnHelper.getVpnInterfaceVpnInstanceNamesString(original.getVpnInstanceNames()),
-                     VpnHelper.getVpnInterfaceVpnInstanceNamesString(update.getVpnInstanceNames()));
-            return;
-        }
-        for (VpnInstanceNames vpnInterfaceVpnInstance : update.getVpnInstanceNames()) {
-            String newVpnName = vpnInterfaceVpnInstance.getVpnName();
-            List<Adjacency> copyNewAdjs = new ArrayList<>(newAdjs);
-            List<Adjacency> copyOldAdjs = new ArrayList<>(oldAdjs);
-            String primaryRd = vpnUtil.getPrimaryRd(newVpnName);
-            if (!vpnUtil.isVpnPendingDelete(primaryRd)) {
-                jobCoordinator.enqueueJob("VPNINTERFACE-" + vpnInterfaceName, () -> {
+        jobCoordinator.enqueueJob("VPNINTERFACE-" + vpnInterfaceName, () -> {
+            if (handleVpnSwapForVpnInterface(identifier, original, update)) {
+                LOG.info("update: handled VPNInterface {} on dpn {} update"
+                                + "upon VPN swap from oldVpn(s) {} to newVpn(s) {}",
+                        original.getName(), dpnId,
+                        VpnHelper.getVpnInterfaceVpnInstanceNamesString(original.getVpnInstanceNames()),
+                        VpnHelper.getVpnInterfaceVpnInstanceNamesString(update.getVpnInstanceNames()));
+                return Collections.emptyList();
+            }
+            for (VpnInstanceNames vpnInterfaceVpnInstance : update.getVpnInstanceNames()) {
+                String newVpnName = vpnInterfaceVpnInstance.getVpnName();
+                List<Adjacency> copyNewAdjs = new ArrayList<>(newAdjs);
+                List<Adjacency> copyOldAdjs = new ArrayList<>(oldAdjs);
+                String primaryRd = vpnUtil.getPrimaryRd(newVpnName);
+                if (!vpnUtil.isVpnPendingDelete(primaryRd)) {
                     // TODO Deal with sequencing — the config tx must only submitted if the oper tx goes in
                     List<ListenableFuture<Void>> futures = new ArrayList<>();
                     futures.add(txRunner.callWithNewWriteOnlyTransactionAndSubmit(confTx -> {
@@ -1605,12 +1606,13 @@ public class VpnInterfaceManager extends AsyncDataTreeChangeListenerBase<VpnInte
                                 update.getName(), update.getVpnInstanceNames());
                     }
                     return futures;
-                });
-            } else {
-                LOG.error("update: Ignoring update of vpnInterface {}, as newVpnInstance {} with primaryRd {}"
-                        + " is already marked for deletion", vpnInterfaceName, newVpnName, primaryRd);
+                } else {
+                    LOG.error("update: Ignoring update of vpnInterface {}, as newVpnInstance {} with primaryRd {}"
+                            + " is already marked for deletion", vpnInterfaceName, newVpnName, primaryRd);
+                }
             }
-        }
+            return Collections.emptyList();
+        });
     }
 
     private boolean handleVpnSwapForVpnInterface(InstanceIdentifier<VpnInterface> identifier,
@@ -1641,19 +1643,20 @@ public class VpnInterfaceManager extends AsyncDataTreeChangeListenerBase<VpnInte
             } catch (InterruptedException e) {
                 //Ignore
             }
+
+            final Adjacencies origAdjs = original.augmentation(Adjacencies.class);
+            final List<Adjacency> oldAdjs = (origAdjs != null && origAdjs.getAdjacency() != null)
+                    ? origAdjs.getAdjacency() : new ArrayList<>();
+            final Adjacencies updateAdjs = update.augmentation(Adjacencies.class);
+            final List<Adjacency> newAdjs = (updateAdjs != null && updateAdjs.getAdjacency() != null)
+                    ? updateAdjs.getAdjacency() : new ArrayList<>();
+
             for (String newVpnName: newVpnList) {
                 String primaryRd = vpnUtil.getPrimaryRd(newVpnName);
                 isSwap = Boolean.TRUE;
                 if (!vpnUtil.isVpnPendingDelete(primaryRd)) {
                     LOG.info("handleVpnSwapForVpnInterface: VPN Interface update event - intfName {} onto vpnName {}"
                             + "running config-driven swap addition", interfaceName, newVpnName);
-                    final Adjacencies origAdjs = original.augmentation(Adjacencies.class);
-                    final List<Adjacency> oldAdjs = (origAdjs != null && origAdjs.getAdjacency() != null)
-                            ? origAdjs.getAdjacency() : new ArrayList<>();
-                    final Adjacencies updateAdjs = update.augmentation(Adjacencies.class);
-                    final List<Adjacency> newAdjs = (updateAdjs != null && updateAdjs.getAdjacency() != null)
-                            ? updateAdjs.getAdjacency() : new ArrayList<>();
-
                     addVpnInterfaceCall(identifier, update, oldAdjs, newAdjs, newVpnName);
                     LOG.info("handleVpnSwapForVpnInterface: Processed Add for update on VPNInterface {}"
                                     + "from oldVpn(s) {} to newVpn {} upon VPN swap",