From: Suraj Ranjan Date: Wed, 20 Apr 2016 06:27:15 +0000 (+0530) Subject: TR: Local routes not appeared on deletion of VPN X-Git-Url: https://git.opendaylight.org/gerrit/gitweb?p=vpnservice.git;a=commitdiff_plain;h=93224bfe847f9f0d81fa8d1dec4ed8f2c9af8295 TR: Local routes not appeared on deletion of VPN -Datapath broken when router dissociated from VPN -Enables tentative fixes to ensure clean up of datapath on removal of an external VPN, when routers are present in it. -Similarly enables a tentative fix to ensure clean up of datapath on disassociation of routers from a VPN. Change-Id: I224d39b33181d52c1332b1548a91eee167814a85 Signed-off-by: Suraj Ranjan Co-Authored-By: Vivekanandan Narasimhan --- diff --git a/fibmanager/fibmanager-impl/src/main/java/org/opendaylight/vpnservice/fibmanager/FibManager.java b/fibmanager/fibmanager-impl/src/main/java/org/opendaylight/vpnservice/fibmanager/FibManager.java index 57a336e7..28419440 100644 --- a/fibmanager/fibmanager-impl/src/main/java/org/opendaylight/vpnservice/fibmanager/FibManager.java +++ b/fibmanager/fibmanager-impl/src/main/java/org/opendaylight/vpnservice/fibmanager/FibManager.java @@ -159,19 +159,19 @@ public class FibManager extends AbstractDataChangeListener implements @Override protected void add(final InstanceIdentifier identifier, final VrfEntry vrfEntry) { - LOG.trace("key: " + identifier + ", value=" + vrfEntry ); + LOG.trace("Add key: " + identifier + ", value=" + vrfEntry ); createFibEntries(identifier, vrfEntry); } @Override protected void remove(InstanceIdentifier identifier, VrfEntry vrfEntry) { - LOG.trace("key: " + identifier + ", value=" + vrfEntry); + LOG.trace("Remove key: " + identifier + ", value=" + vrfEntry); deleteFibEntries(identifier, vrfEntry); } @Override protected void update(InstanceIdentifier identifier, VrfEntry original, VrfEntry update) { - LOG.trace("key: " + identifier + ", original=" + original + ", update=" + update ); + LOG.trace("Update key: " + identifier + ", original=" + original + ", update=" + update ); createFibEntries(identifier, update); } diff --git a/vpnmanager/vpnmanager-impl/src/main/java/org/opendaylight/vpnservice/VpnConstants.java b/vpnmanager/vpnmanager-impl/src/main/java/org/opendaylight/vpnservice/VpnConstants.java index 62d6db72..a764d396 100644 --- a/vpnmanager/vpnmanager-impl/src/main/java/org/opendaylight/vpnservice/VpnConstants.java +++ b/vpnmanager/vpnmanager-impl/src/main/java/org/opendaylight/vpnservice/VpnConstants.java @@ -21,7 +21,8 @@ public class VpnConstants { public static final BigInteger COOKIE_VM_INGRESS_TABLE = new BigInteger("8000001", 16); public static final BigInteger COOKIE_L3_BASE = new BigInteger("8000000", 16); public static final String FLOWID_PREFIX = "L3."; - public static final long WAIT_TIME_IN_MILLISECONDS = 5000; + public static final long MIN_WAIT_TIME_IN_MILLISECONDS = 10000; + public static final long MAX_WAIT_TIME_IN_MILLISECONDS = 90000; public static final int ELAN_GID_MIN = 200000; public static final short ELAN_SMAC_TABLE = 50; public static final short FIB_TABLE = 21; diff --git a/vpnmanager/vpnmanager-impl/src/main/java/org/opendaylight/vpnservice/VpnInterfaceManager.java b/vpnmanager/vpnmanager-impl/src/main/java/org/opendaylight/vpnservice/VpnInterfaceManager.java index 2a9a9613..f128faca 100644 --- a/vpnmanager/vpnmanager-impl/src/main/java/org/opendaylight/vpnservice/VpnInterfaceManager.java +++ b/vpnmanager/vpnmanager-impl/src/main/java/org/opendaylight/vpnservice/VpnInterfaceManager.java @@ -499,7 +499,7 @@ public class VpnInterfaceManager extends AbstractDataChangeListener adjacencies = VpnUtil.read(broker, LogicalDatastoreType.OPERATIONAL, path); String rd = VpnUtil.getVpnRd(broker, intf.getVpnInstanceName()); + LOG.trace("removeAdjacenciesFromVpn: For interface {} RD recovered for vpn {} as rd {}", intf.getName(), + intf.getVpnInstanceName(), rd); if (adjacencies.isPresent()) { List nextHops = adjacencies.get().getAdjacency(); @@ -801,42 +803,57 @@ public class VpnInterfaceManager extends AbstractDataChangeListener identifier, VpnInterface del) { final VpnInterfaceKey key = identifier.firstKeyOf(VpnInterface.class, VpnInterfaceKey.class); String interfaceName = key.getName(); + String vpnName = del.getVpnInstanceName(); + + LOG.trace("VpnInterfaceOpListener removed: interface name {} vpnName {}", interfaceName, vpnName); + //decrement the vpn interface count in Vpn Instance Op Data + InstanceIdentifier + id = VpnUtil.getVpnInstanceToVpnIdIdentifier(vpnName); + Optional vpnInstance + = VpnUtil.read(broker, LogicalDatastoreType.CONFIGURATION, id); + + if (vpnInstance.isPresent()) { + String rd = null; + rd = vpnInstance.get().getVrfId(); + //String rd = getRouteDistinguisher(del.getVpnInstanceName()); + + VpnInstanceOpDataEntry vpnInstOp = VpnUtil.getVpnInstanceOpData(broker, rd); + LOG.trace("VpnInterfaceOpListener removed: interface name {} rd {} vpnName {} in Vpn Op Instance {}", + interfaceName, rd, vpnName, vpnInstOp); + + if (vpnInstOp != null) { + Long ifCnt = 0L; + ifCnt = vpnInstOp.getVpnInterfaceCount(); + LOG.trace("VpnInterfaceOpListener removed: interface name {} rd {} vpnName {} Intf count {}", + interfaceName, rd, vpnName, ifCnt); + if ((ifCnt != null) && (ifCnt > 0)) { + VpnUtil.asyncUpdate(broker, LogicalDatastoreType.OPERATIONAL, + VpnUtil.getVpnInstanceOpDataIdentifier(rd), + VpnUtil.updateIntfCntInVpnInstOpData(ifCnt - 1, rd), VpnUtil.DEFAULT_CALLBACK); + } - //increment the vpn interface count in Vpn Instance Op Data - Long ifCnt = 0L; - String rd = getRouteDistinguisher(del.getVpnInstanceName()); - if(rd == null || rd.isEmpty()) rd = del.getVpnInstanceName(); - VpnInstanceOpDataEntry vpnInstOp = VpnUtil.getVpnInstanceOpData(broker, rd); - if(vpnInstOp != null && vpnInstOp.getVpnInterfaceCount() != null) { - ifCnt = vpnInstOp.getVpnInterfaceCount(); - } - - LOG.trace("VpnInterfaceOpListener remove: interface name {} rd {} interface count in Vpn Op Instance {}", interfaceName, rd, ifCnt); - - if(ifCnt != 0) { - VpnUtil.asyncUpdate(broker, LogicalDatastoreType.OPERATIONAL, - VpnUtil.getVpnInstanceOpDataIdentifier(rd), - VpnUtil.updateIntfCntInVpnInstOpData(ifCnt - 1, rd), VpnUtil.DEFAULT_CALLBACK); - } - - // Vpn Interface removed => No more adjacencies from it. - // Hence clean up interface from vpn-dpn-interface list. - Adjacency adjacency = del.getAugmentation(Adjacencies.class).getAdjacency().get(0); - Optional prefixToInterface = Optional.absent(); - prefixToInterface = VpnUtil.read(broker, LogicalDatastoreType.OPERATIONAL, - VpnUtil.getPrefixToInterfaceIdentifier(vpnInstOp.getVpnId(), - VpnUtil.getIpPrefix(adjacency.getIpAddress()))); - if (!prefixToInterface.isPresent()) { - prefixToInterface = VpnUtil.read(broker, LogicalDatastoreType.OPERATIONAL, - VpnUtil.getPrefixToInterfaceIdentifier(vpnInstOp.getVpnId(), - VpnUtil.getIpPrefix(adjacency.getNextHopIp()))); - } - if (prefixToInterface.isPresent()) { - VpnUtil.delete(broker, LogicalDatastoreType.OPERATIONAL, - VpnUtil.getPrefixToInterfaceIdentifier(vpnInstOp.getVpnId(), - prefixToInterface.get().getIpAddress()), - VpnUtil.DEFAULT_CALLBACK); - updateDpnDbs(prefixToInterface.get().getDpnId(), del.getVpnInstanceName(), interfaceName, false); + // Vpn Interface removed => No more adjacencies from it. + // Hence clean up interface from vpn-dpn-interface list. + Adjacency adjacency = del.getAugmentation(Adjacencies.class).getAdjacency().get(0); + Optional prefixToInterface = Optional.absent(); + prefixToInterface = VpnUtil.read(broker, LogicalDatastoreType.OPERATIONAL, + VpnUtil.getPrefixToInterfaceIdentifier(vpnInstOp.getVpnId(), + VpnUtil.getIpPrefix(adjacency.getIpAddress()))); + if (!prefixToInterface.isPresent()) { + prefixToInterface = VpnUtil.read(broker, LogicalDatastoreType.OPERATIONAL, + VpnUtil.getPrefixToInterfaceIdentifier(vpnInstOp.getVpnId(), + VpnUtil.getIpPrefix(adjacency.getNextHopIp()))); + } + if (prefixToInterface.isPresent()) { + VpnUtil.delete(broker, LogicalDatastoreType.OPERATIONAL, + VpnUtil.getPrefixToInterfaceIdentifier(vpnInstOp.getVpnId(), + prefixToInterface.get().getIpAddress()), + VpnUtil.DEFAULT_CALLBACK); + updateDpnDbs(prefixToInterface.get().getDpnId(), del.getVpnInstanceName(), interfaceName, false); + } + } + } else { + LOG.error("rd not retrievable as vpninstancetovpnid for vpn {} is absent, trying rd as ", vpnName, vpnName); } notifyTaskIfRequired(interfaceName); } diff --git a/vpnmanager/vpnmanager-impl/src/main/java/org/opendaylight/vpnservice/VpnManager.java b/vpnmanager/vpnmanager-impl/src/main/java/org/opendaylight/vpnservice/VpnManager.java index 88c73c2d..b41225ce 100644 --- a/vpnmanager/vpnmanager-impl/src/main/java/org/opendaylight/vpnservice/VpnManager.java +++ b/vpnmanager/vpnmanager-impl/src/main/java/org/opendaylight/vpnservice/VpnManager.java @@ -98,7 +98,7 @@ public class VpnManager extends AbstractDataChangeListener implemen getWildCardPath(), VpnManager.this, DataChangeScope.SUBTREE); fibListenerRegistration = db.registerDataChangeListener(LogicalDatastoreType.OPERATIONAL, getFibEntryListenerPath(), fibListener, DataChangeScope.BASE); - opListenerRegistration = db.registerDataChangeListener(LogicalDatastoreType.CONFIGURATION, + opListenerRegistration = db.registerDataChangeListener(LogicalDatastoreType.OPERATIONAL, getVpnInstanceOpListenerPath(), vpnInstOpListener, DataChangeScope.SUBTREE); } catch (final Exception e) { @@ -115,14 +115,14 @@ public class VpnManager extends AbstractDataChangeListener implemen this.vpnInterfaceManager = vpnInterfaceManager; } - private void waitForOpDataRemoval(String id) { + private void waitForOpDataRemoval(String id, long timeout) { //wait till DCN for update on VPN Instance Op Data signals that vpn interfaces linked to this vpn instance is zero Runnable notifyTask = new VpnNotifyTask(); synchronized (id.intern()) { vpnOpMap.put(id, notifyTask); synchronized (notifyTask) { try { - notifyTask.wait(VpnConstants.WAIT_TIME_IN_MILLISECONDS); + notifyTask.wait(timeout); } catch (InterruptedException e) { } } @@ -132,30 +132,69 @@ public class VpnManager extends AbstractDataChangeListener implemen @Override protected void remove(InstanceIdentifier identifier, VpnInstance del) { - LOG.trace("Remove VPN event - Key: {}, value: {}", identifier, del); + LOG.trace("Remove VPN event key: {}, value: {}", identifier, del); String vpnName = del.getVpnInstanceName(); - //Clean up vpn Interface - InstanceIdentifier vpnInterfacesId = InstanceIdentifier.builder(VpnInterfaces.class).build(); - Optional optionalVpnInterfaces = read(LogicalDatastoreType.OPERATIONAL, vpnInterfacesId); + String rd = del.getIpv4Family().getRouteDistinguisher(); - if(optionalVpnInterfaces.isPresent()) { - List vpnInterfaces = optionalVpnInterfaces.get().getVpnInterface(); - for(VpnInterface vpnInterface : vpnInterfaces) { - if(vpnInterface.getVpnInstanceName().equals(vpnName)) { - LOG.debug("VpnInterface {} will be removed from VPN {}", vpnInterface.getName(), vpnName); - vpnInterfaceManager.remove( - VpnUtil.getVpnInterfaceIdentifier(vpnInterface.getName()), vpnInterface); +// //Clean up vpn Interface +// InstanceIdentifier vpnInterfacesId = InstanceIdentifier.builder(VpnInterfaces.class).build(); +// Optional optionalVpnInterfaces = read(LogicalDatastoreType.OPERATIONAL, vpnInterfacesId); +// +// if(optionalVpnInterfaces.isPresent()) { +// List vpnInterfaces = optionalVpnInterfaces.get().getVpnInterface(); +// for(VpnInterface vpnInterface : vpnInterfaces) { +// if(vpnInterface.getVpnInstanceName().equals(vpnName)) { +// LOG.debug("VpnInterface {} will be removed from VPN {}", vpnInterface.getName(), vpnName); +// vpnInterfaceManager.remove( +// VpnUtil.getVpnInterfaceIdentifier(vpnInterface.getName()), vpnInterface); +// } +// } +// } + + //TODO(vpnteam): Entire code would need refactoring to listen only on the parent object - VPNInstance + Optional vpnOpValue = null; + if ((rd != null) && (!rd.isEmpty())) { + vpnOpValue = VpnUtil.read(broker, LogicalDatastoreType.OPERATIONAL, + VpnUtil.getVpnInstanceOpDataIdentifier(rd)); + } else { + vpnOpValue = VpnUtil.read(broker, LogicalDatastoreType.OPERATIONAL, + VpnUtil.getVpnInstanceOpDataIdentifier(vpnName)); + } + + if ((vpnOpValue != null) && (vpnOpValue.isPresent())) { + VpnInstanceOpDataEntry vpnOpEntry = null; + long timeout = VpnConstants.MIN_WAIT_TIME_IN_MILLISECONDS; + Long intfCount = 0L; + + vpnOpEntry = vpnOpValue.get(); + intfCount = vpnOpEntry.getVpnInterfaceCount(); + if (intfCount != null && intfCount > 0) { + // Minimum wait time of 10 seconds for one VPN Interface clearance (inclusive of full trace on) + timeout = intfCount * 10000; + // Maximum wait time of 90 seconds for all VPN Interfaces clearance (inclusive of full trace on) + if (timeout > VpnConstants.MAX_WAIT_TIME_IN_MILLISECONDS) { + timeout = VpnConstants.MAX_WAIT_TIME_IN_MILLISECONDS; } + LOG.trace("VPNInstance removal interface count at {} for for rd {}, vpnname {}", + intfCount, rd, vpnName); + } + LOG.trace("VPNInstance removal thread waiting for {} seconds for rd {}, vpnname {}", + (timeout/1000), rd, vpnName); + + if ((rd != null) && (!rd.isEmpty())) { + waitForOpDataRemoval(rd, timeout); + } else { + waitForOpDataRemoval(vpnName, timeout); } + + LOG.trace("Returned out of waiting for Op Data removal for rd {}, vpnname {}", rd, vpnName); } + InstanceIdentifier - vpnIdentifier = VpnUtil.getVpnInstanceToVpnIdIdentifier(vpnName); + vpnIdentifier = VpnUtil.getVpnInstanceToVpnIdIdentifier(vpnName); delete(LogicalDatastoreType.CONFIGURATION, vpnIdentifier); - - VpnUtil.releaseId(idManager, VpnConstants.VPN_IDPOOL_NAME, vpnName); - String rd = del.getIpv4Family().getRouteDistinguisher(); - + LOG.trace("Removed vpnIdentifier for rd{} vpnname {}", rd, vpnName); if (rd !=null) { try { @@ -163,24 +202,26 @@ public class VpnManager extends AbstractDataChangeListener implemen } catch (Exception e) { LOG.error("Exception when removing VRF from BGP", e); } - waitForOpDataRemoval(rd); + delete(LogicalDatastoreType.OPERATIONAL, VpnUtil.getVpnInstanceOpDataIdentifier(rd)); } else { - waitForOpDataRemoval(vpnName); + delete(LogicalDatastoreType.OPERATIONAL, VpnUtil.getVpnInstanceOpDataIdentifier(vpnName)); } + + VpnUtil.releaseId(idManager, VpnConstants.VPN_IDPOOL_NAME, vpnName); } @Override protected void update(InstanceIdentifier identifier, VpnInstance original, VpnInstance update) { - LOG.trace("Update event - Key: {}, value: {}", identifier, update); + LOG.trace("Update VPN event key: {}, value: {}", identifier, update); } @Override protected void add(InstanceIdentifier identifier, VpnInstance value) { - LOG.trace("VPN Instance key: {}, value: {}", identifier, value); + LOG.trace("Add VPN event key: {}, value: {}", identifier, value); VpnAfConfig config = value.getIpv4Family(); String rd = config.getRouteDistinguisher(); @@ -230,7 +271,6 @@ public class VpnManager extends AbstractDataChangeListener implemen } } //Try to add up vpn Interfaces if already in Operational Datastore - LOG.trace("Trying to add the vpn interfaces -1."); InstanceIdentifier vpnInterfacesId = InstanceIdentifier.builder(VpnInterfaces.class).build(); Optional optionalVpnInterfaces = read(LogicalDatastoreType.CONFIGURATION, vpnInterfacesId);