NETVIRT-1068: Upstreaming fixes 2 86/68286/25
authorHANAMANTAGOUD V Kandagal <hanamantagoud.v.kandagal@ericsson.com>
Thu, 15 Feb 2018 09:14:18 +0000 (14:44 +0530)
committerSam Hague <shague@redhat.com>
Wed, 4 Apr 2018 16:21:38 +0000 (09:21 -0700)
Issue-1 : When VM is configured with extra-route, refcount in l3nexthop is
incremented. It gets incremented further due to following reasons:

(a) After initial extra-route configuration using command - neutron
router-update RouterA destination=IP-A,nexthop=prefix-A , if another
update is done using command - neutron router-update RouterA
destination=IP-A,nexthop=prefix-B , neutron router listener calls update
on prefix-A as well as prefix-B. On prefix-A , secondary adj (IP-A) is
removed , where as its added on prefix-B. This back-to-back update creates
race-condition in Vrf Engine , leading inconsistencies in l3nexthop,
VpnExtraRoute, VpnInterfaceOp DS. (b) After initial extra-route
configuration using command - neutron router-update RouterA
destination=IP-A,nexthop=prefix-A, if cluster reboot is performed ,
TEP-ADD event triggered the update of FIB entry for IP-A. Update call in
FIB leads to increase in refcount of l3nexthop for prefix-A.

After refcount has reached high number due to issue-(a) and (b) , if VM is
migrated , group will not be programmed on destination DPN,, thus leading
to VM becoming un-reachable.

Fix: For issue-(a) , a temporary fix of 2sec delay is introduced in
neutron. A better fix/design need to be thought to avoid race condition.
For issue-(b) , after cluster reboot , secondary-adj FIB updated is
avoided due to wrong check in updateVpnInterfaceOnTepAdd method. refcount
is removed from l3nexthop yang because , after cluster reboot , due to
multiple Add/Update replays for a given prefix, refcount goes higher than
it should be. Hence prefixes using a group themselves are stored in
l3nexthop , so that even if spurious Add/Update are triggered post cluster
reboot for same prefix , its not updated.

Change-Id: I4618303345db1241b1826b018424b3df0f8bd9ec
Signed-off-by: HANAMANTAGOUD V Kandagal <hanamantagoud.v.kandagal@ericsson.com>
Signed-off-by: Sam Hague <shague@redhat.com>
fibmanager/api/src/main/yang/l3nexthop.yang
fibmanager/impl/pom.xml
fibmanager/impl/src/main/java/org/opendaylight/netvirt/fibmanager/BaseVrfEntryHandler.java
fibmanager/impl/src/main/java/org/opendaylight/netvirt/fibmanager/FibUtil.java
fibmanager/impl/src/main/java/org/opendaylight/netvirt/fibmanager/NexthopManager.java
fibmanager/impl/src/main/java/org/opendaylight/netvirt/fibmanager/VrfEntryListener.java
fibmanager/impl/src/main/resources/org/opendaylight/blueprint/fibmanager.xml
neutronvpn/impl/src/main/java/org/opendaylight/netvirt/neutronvpn/NeutronRouterChangeListener.java
vpnmanager/impl/src/main/java/org/opendaylight/netvirt/vpnmanager/VpnInterfaceManager.java

index 61715d6f9c7337570dbabde79adbf4f4a23a8967..d4ebf631bffc88b383a6cdbbb72f4d206c9c974b 100644 (file)
@@ -16,7 +16,10 @@ module l3nexthop {
                 leaf IpAddress {type string;}
                 leaf egressPointer {type uint32;}
                 leaf dpnId {type uint64;}
-                leaf flowrefCount {type uint16; }
+                list ip-adjacencies {
+                    key  "ip-adjacency";
+                    leaf ip-adjacency {type string;}
+                }
             }
         }
         list tunnelNexthops{
index d956f9afc5614b34a4e919e1a3a5bccf9daf3f59..ef79cc0abf647661942142f07e549ee780767f79 100644 (file)
@@ -61,6 +61,11 @@ and is available at http://www.eclipse.org/legal/epl-v10.html
             <artifactId>vpnmanager-api</artifactId>
             <version>${project.version}</version>
         </dependency>
+        <dependency>
+            <groupId>org.opendaylight.genius</groupId>
+            <artifactId>lockmanager-api</artifactId>
+            <version>${genius.version}</version>
+        </dependency>
         <dependency>
             <groupId>org.opendaylight.openflowplugin.model</groupId>
             <artifactId>model-flow-service</artifactId>
index cb5c3034ab59904fcaeb47df2700405462d7a141..938e0ededaa6e3837ebbe721eb4a1874ff5f1f86 100644 (file)
@@ -129,7 +129,8 @@ public class BaseVrfEntryHandler implements AutoCloseable {
 
     protected void deleteLocalAdjacency(final BigInteger dpId, final long vpnId, final String ipAddress,
                               final String ipPrefixAddress) {
-        LOG.trace("deleteLocalAdjacency called with dpid {}, vpnId{}, ipAddress {}", dpId, vpnId, ipAddress);
+        LOG.trace("deleteLocalAdjacency called with dpid {}, vpnId{}, primaryIpAddress {} currIpPrefix {}",
+                dpId, vpnId, ipAddress, ipPrefixAddress);
         try {
             nextHopManager.removeLocalNextHop(dpId, vpnId, ipAddress, ipPrefixAddress);
         } catch (NullPointerException e) {
index 3633258e7bda8a47b4ac442be6b25dd286ea6e9f..9a8543cb612ab150ad4ee057d4b651b0ce06e1ff 100644 (file)
@@ -47,6 +47,12 @@ import org.opendaylight.yang.gen.v1.urn.opendaylight.genius.itm.op.rev160406.Dpn
 import org.opendaylight.yang.gen.v1.urn.opendaylight.genius.itm.op.rev160406.dpn.endpoints.DPNTEPsInfo;
 import org.opendaylight.yang.gen.v1.urn.opendaylight.genius.itm.op.rev160406.dpn.endpoints.DPNTEPsInfoKey;
 import org.opendaylight.yang.gen.v1.urn.opendaylight.genius.itm.op.rev160406.dpn.endpoints.dpn.teps.info.TunnelEndPoints;
+import org.opendaylight.yang.gen.v1.urn.opendaylight.genius.lockmanager.rev160413.LockManagerService;
+import org.opendaylight.yang.gen.v1.urn.opendaylight.genius.lockmanager.rev160413.TimeUnits;
+import org.opendaylight.yang.gen.v1.urn.opendaylight.genius.lockmanager.rev160413.TryLockInput;
+import org.opendaylight.yang.gen.v1.urn.opendaylight.genius.lockmanager.rev160413.TryLockInputBuilder;
+import org.opendaylight.yang.gen.v1.urn.opendaylight.genius.lockmanager.rev160413.UnlockInput;
+import org.opendaylight.yang.gen.v1.urn.opendaylight.genius.lockmanager.rev160413.UnlockInputBuilder;
 import org.opendaylight.yang.gen.v1.urn.opendaylight.group.types.rev131018.BucketId;
 import org.opendaylight.yang.gen.v1.urn.opendaylight.group.types.rev131018.GroupId;
 import org.opendaylight.yang.gen.v1.urn.opendaylight.group.types.rev131018.group.Buckets;
@@ -811,4 +817,39 @@ public class FibUtil {
         int broadcast = network | ~netmask;
         return InetAddresses.toAddrString(InetAddresses.fromInteger(broadcast));
     }
+
+    public static boolean lockCluster(LockManagerService lockManager, String lockName, long tryLockPeriod) {
+        TryLockInput input = new TryLockInputBuilder().setLockName(lockName).setTime(tryLockPeriod)
+                .setTimeUnit(TimeUnits.Milliseconds).build();
+        Future<RpcResult<Void>> result = lockManager.tryLock(input);
+        boolean lockAcquired;
+        try {
+            if ((result != null) && (result.get().isSuccessful())) {
+                LOG.debug("lockCluster: Acquired lock for {}", lockName);
+                lockAcquired = true;
+            } else {
+                LOG.error("lockCluster: Unable to getLock for {}", lockName);
+                lockAcquired = false;
+            }
+        } catch (InterruptedException | ExecutionException e) {
+            LOG.error("lockCluster: Exception while trying to getLock for {}", lockName, e);
+            lockAcquired = false;
+        }
+
+        return lockAcquired;
+    }
+
+    public static void unlockCluster(LockManagerService lockManager, String lockName) {
+        UnlockInput input = new UnlockInputBuilder().setLockName(lockName).build();
+        Future<RpcResult<Void>> result = lockManager.unlock(input);
+        try {
+            if ((result != null) && (result.get().isSuccessful())) {
+                LOG.debug("unlockCluster: Unlocked {}", lockName);
+            } else {
+                LOG.error("unlockCluster: Unable to release lock for {}", lockName);
+            }
+        } catch (InterruptedException | ExecutionException e) {
+            LOG.error("unlockCluster: Unable to unlock {}", lockName, e);
+        }
+    }
 }
index 4747b80f6d4b7172e4719ec60ade70825168c73d..6d222cee0f26795de93b79182605d8dd3640a753 100644 (file)
@@ -85,6 +85,7 @@ import org.opendaylight.yang.gen.v1.urn.opendaylight.genius.itm.rpcs.rev160406.G
 import org.opendaylight.yang.gen.v1.urn.opendaylight.genius.itm.rpcs.rev160406.GetTunnelInterfaceNameInputBuilder;
 import org.opendaylight.yang.gen.v1.urn.opendaylight.genius.itm.rpcs.rev160406.GetTunnelInterfaceNameOutput;
 import org.opendaylight.yang.gen.v1.urn.opendaylight.genius.itm.rpcs.rev160406.ItmRpcService;
+import org.opendaylight.yang.gen.v1.urn.opendaylight.genius.lockmanager.rev160413.LockManagerService;
 import org.opendaylight.yang.gen.v1.urn.opendaylight.group.service.rev130918.AddGroupInput;
 import org.opendaylight.yang.gen.v1.urn.opendaylight.group.service.rev130918.AddGroupInputBuilder;
 import org.opendaylight.yang.gen.v1.urn.opendaylight.group.service.rev130918.AddGroupOutput;
@@ -107,6 +108,9 @@ import org.opendaylight.yang.gen.v1.urn.opendaylight.netvirt.l3nexthop.rev150409
 import org.opendaylight.yang.gen.v1.urn.opendaylight.netvirt.l3nexthop.rev150409.l3nexthop.vpnnexthops.VpnNexthop;
 import org.opendaylight.yang.gen.v1.urn.opendaylight.netvirt.l3nexthop.rev150409.l3nexthop.vpnnexthops.VpnNexthopBuilder;
 import org.opendaylight.yang.gen.v1.urn.opendaylight.netvirt.l3nexthop.rev150409.l3nexthop.vpnnexthops.VpnNexthopKey;
+import org.opendaylight.yang.gen.v1.urn.opendaylight.netvirt.l3nexthop.rev150409.l3nexthop.vpnnexthops.vpnnexthop.IpAdjacencies;
+import org.opendaylight.yang.gen.v1.urn.opendaylight.netvirt.l3nexthop.rev150409.l3nexthop.vpnnexthops.vpnnexthop.IpAdjacenciesBuilder;
+import org.opendaylight.yang.gen.v1.urn.opendaylight.netvirt.l3nexthop.rev150409.l3nexthop.vpnnexthops.vpnnexthop.IpAdjacenciesKey;
 import org.opendaylight.yang.gen.v1.urn.opendaylight.netvirt.l3vpn.rev130911.ConfTransportTypeL3vpn;
 import org.opendaylight.yang.gen.v1.urn.opendaylight.netvirt.l3vpn.rev130911.ConfTransportTypeL3vpnBuilder;
 import org.opendaylight.yang.gen.v1.urn.opendaylight.netvirt.l3vpn.rev130911.dpid.l3vpn.lb.nexthops.DpnLbNexthops;
@@ -129,6 +133,7 @@ public class NexthopManager implements AutoCloseable {
     private static final Logger LOG = LoggerFactory.getLogger(NexthopManager.class);
     private static final String NEXTHOP_ID_POOL_NAME = "nextHopPointerPool";
     private static final long WAIT_TIME_FOR_SYNC_INSTALL = Long.getLong("wait.time.sync.install", 300L);
+    private static final long WAIT_TIME_TO_ACQUIRE_LOCK = 3000L;
 
     private final DataBroker dataBroker;
     private final IMdsalApiManager mdsalApiManager;
@@ -136,6 +141,7 @@ public class NexthopManager implements AutoCloseable {
     private final ItmRpcService itmManager;
     private final IdManagerService idManager;
     private final IElanService elanService;
+    private LockManagerService lockManager;
     private final SalGroupService salGroupService;
     private final JobCoordinator jobCoordinator;
     private final FibUtil fibUtil;
@@ -157,6 +163,7 @@ public class NexthopManager implements AutoCloseable {
                           final IdManagerService idManager,
                           final OdlInterfaceRpcService interfaceManager,
                           final ItmRpcService itmManager,
+                          final LockManagerService lockManager,
                           final IElanService elanService,
                           final SalGroupService salGroupService,
                           final JobCoordinator jobCoordinator,
@@ -170,6 +177,7 @@ public class NexthopManager implements AutoCloseable {
         this.salGroupService = salGroupService;
         this.jobCoordinator = jobCoordinator;
         this.fibUtil = fibUtil;
+        this.lockManager = lockManager;
         createIdPool();
     }
 
@@ -328,73 +336,83 @@ public class NexthopManager implements AutoCloseable {
     }
 
     public long createLocalNextHop(long vpnId, BigInteger dpnId, String ifName,
-            String ipNextHopAddress, String ipPrefixAddress, String gwMacAddress, String jobKey) {
+                                   String primaryIpAddress, String currDestIpPrefix,
+                                   String gwMacAddress, String jobKey) {
         String vpnName = fibUtil.getVpnNameFromId(vpnId);
         if (vpnName == null) {
             return 0;
         }
-        String macAddress = fibUtil.getMacAddressFromPrefix(ifName, vpnName, ipPrefixAddress);
-        String ipAddress = macAddress != null ? ipPrefixAddress : ipNextHopAddress;
-        long groupId = createNextHopPointer(getNextHopKey(vpnId, ipAddress));
+        String macAddress = fibUtil.getMacAddressFromPrefix(ifName, vpnName, primaryIpAddress);
+
+        long groupId = createNextHopPointer(getNextHopKey(vpnId, primaryIpAddress));
         if (groupId == 0) {
-            LOG.error("Unable to allocate groupId for vpnId {} , prefix {}  IntfName {}, nextHopAddr {}",
-                    vpnId, ipAddress, ifName, ipNextHopAddress);
+            LOG.error("Unable to allocate groupId for vpnId {} , IntfName {}, primaryIpAddress {} curIpPrefix {}",
+                    vpnId, ifName, primaryIpAddress, currDestIpPrefix);
             return groupId;
         }
-        String nextHopLockStr = vpnId + ipAddress;
+        String nextHopLockStr = vpnId + primaryIpAddress;
         jobCoordinator.enqueueJob(jobKey, () -> {
-            synchronized (nextHopLockStr.intern()) {
-                VpnNexthop nexthop = getVpnNexthop(vpnId, ipAddress);
-                LOG.trace("nexthop: {} retrieved for vpnId {}, prefix {}, ifName {} on dpn {}", nexthop, vpnId,
-                        ipAddress, ifName, dpnId);
-                if (nexthop == null) {
-                    String encMacAddress = macAddress == null
-                        ? fibUtil.getMacAddressFromPrefix(ifName, vpnName, ipAddress) : macAddress;
-                    List<BucketInfo> listBucketInfo = new ArrayList<>();
-                    List<ActionInfo> listActionInfo = new ArrayList<>();
-                    int actionKey = 0;
-                    // MAC re-write
-                    if (encMacAddress != null) {
-                        if (gwMacAddress != null) {
-                            LOG.trace("The Local NextHop Group Source Mac {} for VpnInterface {} on VPN {}",
-                                    gwMacAddress, ifName, vpnId);
-                            listActionInfo
-                                    .add(new ActionSetFieldEthernetSource(actionKey++, new MacAddress(gwMacAddress)));
+            try {
+                if (FibUtil.lockCluster(lockManager, nextHopLockStr, WAIT_TIME_TO_ACQUIRE_LOCK)) {
+                    VpnNexthop nexthop = getVpnNexthop(vpnId, primaryIpAddress);
+                    LOG.trace("nexthop: {} retrieved for vpnId {}, prefix {}, ifName {} on dpn {}", nexthop, vpnId,
+                            primaryIpAddress, ifName, dpnId);
+                    if (nexthop == null) {
+                        String encMacAddress = macAddress == null
+                                ? fibUtil.getMacAddressFromPrefix(ifName, vpnName, primaryIpAddress) : macAddress;
+                        List<BucketInfo> listBucketInfo = new ArrayList<>();
+                        List<ActionInfo> listActionInfo = new ArrayList<>();
+                        int actionKey = 0;
+                        // MAC re-write
+                        if (encMacAddress != null) {
+                            if (gwMacAddress != null) {
+                                LOG.trace("The Local NextHop Group Source Mac {} for VpnInterface {} on VPN {}",
+                                        gwMacAddress, ifName, vpnId);
+                                listActionInfo.add(new ActionSetFieldEthernetSource(actionKey++,
+                                        new MacAddress(gwMacAddress)));
+                            }
+                            listActionInfo.add(new ActionSetFieldEthernetDestination(actionKey++,
+                                    new MacAddress(encMacAddress)));
+                            // listActionInfo.add(0, new ActionPopMpls());
+                        } else {
+                            // FIXME: Log message here.
+                            LOG.debug("mac address for new local nexthop is null");
                         }
-                        listActionInfo
-                                .add(new ActionSetFieldEthernetDestination(actionKey++, new MacAddress(encMacAddress)));
-                        // listActionInfo.add(0, new ActionPopMpls());
+                        listActionInfo.addAll(getEgressActionsForInterface(ifName, actionKey));
+                        BucketInfo bucket = new BucketInfo(listActionInfo);
+
+                        listBucketInfo.add(bucket);
+                        GroupEntity groupEntity = MDSALUtil.buildGroupEntity(dpnId, groupId, primaryIpAddress,
+                                GroupTypes.GroupAll, listBucketInfo);
+                        LOG.trace("Install LNH Group: id {}, mac address {}, interface {} for prefix {}", groupId,
+                                encMacAddress, ifName, primaryIpAddress);
+                        //Try to install group directly on the DPN bypassing the FRM, in order to avoid waiting for the
+                        // group to get installed before programming the flows
+                        installGroupOnDpn(groupId, dpnId, primaryIpAddress, listBucketInfo,
+                                getNextHopKey(vpnId, primaryIpAddress), GroupTypes.GroupAll);
+                        // install Group
+                        mdsalApiManager.syncInstallGroup(groupEntity);
+                        // update MD-SAL DS
+                        addVpnNexthopToDS(dpnId, vpnId, primaryIpAddress, currDestIpPrefix, groupId);
+
                     } else {
-                        // FIXME: Log message here.
-                        LOG.debug("mac address for new local nexthop is null");
+                        // Ignore adding new prefix , if it already exists
+                        List<IpAdjacencies> prefixesList = nexthop.getIpAdjacencies();
+                        IpAdjacencies prefix = new IpAdjacenciesBuilder().setIpAdjacency(currDestIpPrefix).build();
+                        if (prefixesList != null && prefixesList.contains(prefix)) {
+                            LOG.trace("Prefix {} is already present in l3nextHop {} ", currDestIpPrefix, nexthop);
+                        } else {
+                            IpAdjacenciesBuilder ipPrefixesBuilder =
+                                    new IpAdjacenciesBuilder().setKey(new IpAdjacenciesKey(currDestIpPrefix));
+                            LOG.trace("Updating prefix {} to vpnNextHop {} Operational DS", currDestIpPrefix, nexthop);
+                            MDSALUtil.syncWrite(dataBroker, LogicalDatastoreType.OPERATIONAL,
+                                    getVpnNextHopIpPrefixIdentifier(vpnId, primaryIpAddress, currDestIpPrefix),
+                                    ipPrefixesBuilder.build());
+                        }
                     }
-                    listActionInfo.addAll(getEgressActionsForInterface(ifName, actionKey));
-                    BucketInfo bucket = new BucketInfo(listActionInfo);
-
-                    listBucketInfo.add(bucket);
-                    GroupEntity groupEntity = MDSALUtil.buildGroupEntity(dpnId, groupId, ipAddress, GroupTypes.GroupAll,
-                            listBucketInfo);
-                    LOG.trace("Install LNH Group: id {}, mac address {}, interface {} for prefix {}", groupId,
-                            encMacAddress, ifName, ipAddress);
-                    //Try to install group directly on the DPN bypassing the FRM, in order to avoid waiting for the
-                    // group to get installed before programming the flows
-                    installGroupOnDpn(groupId, dpnId, ipAddress, listBucketInfo, getNextHopKey(vpnId, ipAddress),
-                            GroupTypes.GroupAll);
-                    // install Group
-                    mdsalApiManager.syncInstallGroup(groupEntity);
-                    // update MD-SAL DS
-                    addVpnNexthopToDS(dpnId, vpnId, ipAddress, groupId);
-
-                } else {
-                    // nexthop exists already; a new flow is going to point to
-                    // it, increment the flowrefCount by 1
-                    int flowrefCnt = nexthop.getFlowrefCount() + 1;
-                    VpnNexthop nh = new VpnNexthopBuilder().setKey(new VpnNexthopKey(ipAddress))
-                            .setFlowrefCount(flowrefCnt).build();
-                    LOG.trace("Updating vpnnextHop {} for refCount {} to Operational DS", nh, flowrefCnt);
-                    MDSALUtil.syncUpdate(dataBroker, LogicalDatastoreType.OPERATIONAL, getVpnNextHopIdentifier(vpnId,
-                            ipAddress), nh);
                 }
+            } finally {
+                FibUtil.unlockCluster(lockManager, nextHopLockStr);
             }
             return Collections.emptyList();
         });
@@ -425,29 +443,34 @@ public class NexthopManager implements AutoCloseable {
         }
     }
 
-    protected void addVpnNexthopToDS(BigInteger dpnId, long vpnId, String ipPrefix, long egressPointer) {
+    protected void addVpnNexthopToDS(BigInteger dpnId, long vpnId, String primaryIpAddr,
+                                     String currIpAddr, long egressPointer) {
         InstanceIdentifierBuilder<VpnNexthops> idBuilder = InstanceIdentifier.builder(L3nexthop.class)
             .child(VpnNexthops.class, new VpnNexthopsKey(vpnId));
 
+        List<IpAdjacencies> ipPrefixesList = new ArrayList<>();
+        IpAdjacencies prefix = new IpAdjacenciesBuilder().setIpAdjacency(currIpAddr).build();
+        ipPrefixesList.add(prefix);
         // Add nexthop to vpn node
         VpnNexthop nh = new VpnNexthopBuilder()
-            .setKey(new VpnNexthopKey(ipPrefix))
+            .setKey(new VpnNexthopKey(primaryIpAddr))
             .setDpnId(dpnId)
-            .setIpAddress(ipPrefix)
-            .setFlowrefCount(1)
+            .setIpAdjacencies(ipPrefixesList)
             .setEgressPointer(egressPointer).build();
 
         InstanceIdentifier<VpnNexthop> id1 = idBuilder
-            .child(VpnNexthop.class, new VpnNexthopKey(ipPrefix)).build();
+            .child(VpnNexthop.class, new VpnNexthopKey(primaryIpAddr)).build();
         LOG.trace("Adding vpnnextHop {} to Operational DS", nh);
         MDSALUtil.syncWrite(dataBroker, LogicalDatastoreType.OPERATIONAL, id1, nh);
 
     }
 
-    protected InstanceIdentifier<VpnNexthop> getVpnNextHopIdentifier(long vpnId, String ipAddress) {
-        InstanceIdentifier<VpnNexthop> id = InstanceIdentifier.builder(L3nexthop.class)
-            .child(VpnNexthops.class, new VpnNexthopsKey(vpnId)).child(VpnNexthop.class,
-                new VpnNexthopKey(ipAddress)).build();
+    protected InstanceIdentifier<IpAdjacencies> getVpnNextHopIpPrefixIdentifier(long vpnId, String primaryIpAddress,
+                                                                                String ipPrefix) {
+        InstanceIdentifier<IpAdjacencies> id = InstanceIdentifier.builder(L3nexthop.class)
+                .child(VpnNexthops.class, new VpnNexthopsKey(vpnId))
+                .child(VpnNexthop.class, new VpnNexthopKey(primaryIpAddress))
+                .child(IpAdjacencies.class, new IpAdjacenciesKey(ipPrefix)).build();
         return id;
     }
 
@@ -518,43 +541,40 @@ public class NexthopManager implements AutoCloseable {
         MDSALUtil.syncDelete(dataBroker, LogicalDatastoreType.OPERATIONAL, id);
     }
 
-    public void removeLocalNextHop(BigInteger dpnId, Long vpnId, String ipNextHopAddress, String ipPrefixAddress) {
-        String ipPrefixStr = vpnId + ipPrefixAddress;
-        VpnNexthop prefixNh = null;
-        synchronized (ipPrefixStr.intern()) {
-            prefixNh = getVpnNexthop(vpnId, ipPrefixAddress);
-        }
-        String ipAddress = prefixNh != null ? ipPrefixAddress : ipNextHopAddress;
-
-        String nextHopLockStr = vpnId + ipAddress;
-        synchronized (nextHopLockStr.intern()) {
-            VpnNexthop nh = getVpnNexthop(vpnId, ipAddress);
-            if (nh != null) {
-                int newFlowrefCnt = nh.getFlowrefCount() - 1;
-                if (newFlowrefCnt == 0) { //remove the group only if there are no more flows using this group
-                    GroupEntity groupEntity = MDSALUtil.buildGroupEntity(
-                        dpnId, nh.getEgressPointer(), ipAddress, GroupTypes.GroupAll,
-                            Collections.EMPTY_LIST /*listBucketInfo*/);
-                    // remove Group ...
-                    mdsalApiManager.removeGroup(groupEntity);
-                    //update MD-SAL DS
-                    removeVpnNexthopFromDS(vpnId, ipAddress);
-                    //release groupId
-                    removeNextHopPointer(getNextHopKey(vpnId, ipAddress));
-                    LOG.debug("Local Next hop {} for {} {} on dpn {} successfully deleted",
-                        nh.getEgressPointer(), vpnId, ipAddress, dpnId);
+    public void removeLocalNextHop(BigInteger dpnId, Long vpnId, String primaryIpAddress, String currDestIpPrefix) {
+        String nextHopLockStr = vpnId + primaryIpAddress;
+        try {
+            if (FibUtil.lockCluster(lockManager, nextHopLockStr, WAIT_TIME_TO_ACQUIRE_LOCK)) {
+                VpnNexthop nh = getVpnNexthop(vpnId, primaryIpAddress);
+                if (nh != null) {
+                    List<IpAdjacencies> prefixesList = nh.getIpAdjacencies();
+                    IpAdjacencies prefix = new IpAdjacenciesBuilder().setIpAdjacency(currDestIpPrefix).build();
+                    prefixesList.remove(prefix);
+                    if (prefixesList.isEmpty()) { //remove the group only if there are no more flows using this group
+                        GroupEntity groupEntity = MDSALUtil.buildGroupEntity(dpnId, nh.getEgressPointer(),
+                                primaryIpAddress, GroupTypes.GroupAll, Collections.EMPTY_LIST);
+                        // remove Group ...
+                        mdsalApiManager.removeGroup(groupEntity);
+                        //update MD-SAL DS
+                        removeVpnNexthopFromDS(vpnId, primaryIpAddress);
+                        //release groupId
+                        removeNextHopPointer(getNextHopKey(vpnId, primaryIpAddress));
+                        LOG.debug("Local Next hop {} for {} {} on dpn {} successfully deleted",
+                                nh.getEgressPointer(), vpnId, primaryIpAddress, dpnId);
+                    } else {
+                        //remove the currIpPrefx from IpPrefixList of the vpnNexthop
+                        LOG.trace("Removing the prefix {} from vpnNextHop {} Operational DS", currDestIpPrefix, nh);
+                        MDSALUtil.syncDelete(dataBroker, LogicalDatastoreType.OPERATIONAL,
+                                getVpnNextHopIpPrefixIdentifier(vpnId, primaryIpAddress, currDestIpPrefix));
+                    }
                 } else {
-                    //just update the flowrefCount of the vpnNexthop
-                    VpnNexthop currNh = new VpnNexthopBuilder().setKey(new VpnNexthopKey(ipAddress))
-                        .setFlowrefCount(newFlowrefCnt).build();
-                    LOG.trace("Updating vpnnextHop {} for refCount {} to Operational DS", currNh, newFlowrefCnt);
-                    MDSALUtil.syncUpdate(dataBroker, LogicalDatastoreType.OPERATIONAL, getVpnNextHopIdentifier(vpnId,
-                            ipAddress), currNh);
+                    //throw error
+                    LOG.error("Local NextHop for VpnId {} curIpPrefix {} on dpn {} primaryIpAddress {} not deleted",
+                            vpnId, currDestIpPrefix, dpnId, primaryIpAddress);
                 }
-            } else {
-                //throw error
-                LOG.error("Local Next hop for Prefix {} VpnId {} on dpn {} not deleted", ipAddress, vpnId, dpnId);
             }
+        } finally {
+            FibUtil.unlockCluster(lockManager, nextHopLockStr);
         }
     }
 
index d8d62e53a51ff13f6664ddfd25227a8bb1365bf4..89a6c8b1d0678654eefbcbbc87adb2e016d1c157 100755 (executable)
@@ -731,7 +731,7 @@ public class VrfEntryListener extends AsyncDataTreeChangeListenerBase<VrfEntry,
                     localNextHopSeen = true;
                     BigInteger dpnId =
                             checkCreateLocalFibEntry(localNextHopInfoLocal, localNextHopInfoLocal.getIpAddress(),
-                                    vpnId, rd, vrfEntry, vpnId, vpnExtraRoute, vpnExtraRoutes);
+                                    vpnId, rd, vrfEntry, vpnExtraRoute, vpnExtraRoutes);
                     returnLocalDpnId.add(dpnId);
                 }
             }
@@ -760,15 +760,12 @@ public class VrfEntryListener extends AsyncDataTreeChangeListenerBase<VrfEntry,
                                         label, localNextHopInfo.getVpnInterfaceName(), lri.getDpnId());
                                 if (vpnExtraRoutes.isEmpty()) {
                                     BigInteger dpnId = checkCreateLocalFibEntry(localNextHopInfo, localNextHopIP,
-                                            vpnId, rd, vrfEntry, lri.getParentVpnid(), null /*vpnExtraRoute*/,
-                                            vpnExtraRoutes);
+                                            vpnId, rd, vrfEntry, null, vpnExtraRoutes);
                                     returnLocalDpnId.add(dpnId);
                                 } else {
                                     for (Routes extraRoutes : vpnExtraRoutes) {
-                                        BigInteger dpnId = checkCreateLocalFibEntry(localNextHopInfo,
-                                                localNextHopIP,
-                                                vpnId, rd, vrfEntry, lri.getParentVpnid(),
-                                                extraRoutes, vpnExtraRoutes);
+                                        BigInteger dpnId = checkCreateLocalFibEntry(localNextHopInfo, localNextHopIP,
+                                                vpnId, rd, vrfEntry, extraRoutes, vpnExtraRoutes);
                                         returnLocalDpnId.add(dpnId);
                                     }
                                 }
@@ -782,7 +779,7 @@ public class VrfEntryListener extends AsyncDataTreeChangeListenerBase<VrfEntry,
             }
         } else {
             BigInteger dpnId = checkCreateLocalFibEntry(localNextHopInfo, localNextHopIP, vpnId,
-                    rd, vrfEntry, vpnId, /*routes*/ null, /*vpnExtraRoutes*/ null);
+                    rd, vrfEntry, /*routes*/ null, /*vpnExtraRoutes*/ null);
             if (dpnId != null) {
                 returnLocalDpnId.add(dpnId);
             }
@@ -792,7 +789,7 @@ public class VrfEntryListener extends AsyncDataTreeChangeListenerBase<VrfEntry,
 
     private BigInteger checkCreateLocalFibEntry(Prefixes localNextHopInfo, String localNextHopIP,
                                                 final Long vpnId, final String rd,
-                                                final VrfEntry vrfEntry, Long parentVpnId,
+                                                final VrfEntry vrfEntry,
                                                 Routes routes, List<Routes> vpnExtraRoutes) {
         String vpnName = fibUtil.getVpnNameFromId(vpnId);
         if (localNextHopInfo != null) {
@@ -827,20 +824,20 @@ public class VrfEntryListener extends AsyncDataTreeChangeListenerBase<VrfEntry,
                     localNextHopIP = routes.getNexthopIpList().get(0) + NwConstants.IPV6PREFIX;
                 }
                 if (vpnExtraRoutes.size() > 1) {
-                    groupId = nextHopManager.createNextHopGroups(parentVpnId, rd, dpnId, vrfEntry, routes,
+                    groupId = nextHopManager.createNextHopGroups(vpnId, rd, dpnId, vrfEntry, routes,
                             vpnExtraRoutes);
-                    localGroupId = nextHopManager.getLocalNextHopGroup(parentVpnId, localNextHopIP);
+                    localGroupId = nextHopManager.getLocalNextHopGroup(vpnId, localNextHopIP);
                 } else if (routes.getNexthopIpList().size() > 1) {
-                    groupId = nextHopManager.createNextHopGroups(parentVpnId, rd, dpnId, vrfEntry, routes,
+                    groupId = nextHopManager.createNextHopGroups(vpnId, rd, dpnId, vrfEntry, routes,
                             vpnExtraRoutes);
                     localGroupId = groupId;
                 } else {
-                    groupId = nextHopManager.createLocalNextHop(parentVpnId, dpnId, interfaceName, localNextHopIP,
+                    groupId = nextHopManager.createLocalNextHop(vpnId, dpnId, interfaceName, localNextHopIP,
                             prefix, gwMacAddress, jobKey);
                     localGroupId = groupId;
                 }
             } else {
-                groupId = nextHopManager.createLocalNextHop(parentVpnId, dpnId, interfaceName, localNextHopIP, prefix,
+                groupId = nextHopManager.createLocalNextHop(vpnId, dpnId, interfaceName, localNextHopIP, prefix,
                         gwMacAddress, jobKey);
                 localGroupId = groupId;
             }
@@ -1041,7 +1038,7 @@ public class VrfEntryListener extends AsyncDataTreeChangeListenerBase<VrfEntry,
                 if (localNextHopInfo != null) {
                     String localNextHopIP = localNextHopInfo.getIpAddress();
                     BigInteger dpnId = checkDeleteLocalFibEntry(localNextHopInfo, localNextHopIP,
-                            vpnId, rd, vrfEntry, isExtraroute, vpnId /*parentVpnId*/);
+                            vpnId, rd, vrfEntry, isExtraroute);
                     if (!dpnId.equals(BigInteger.ZERO)) {
                         LOG.trace("Deleting ECMP group for prefix {}, dpn {}", vrfEntry.getDestPrefix(), dpnId);
                         nextHopManager.setupLoadBalancingNextHop(vpnId, dpnId,
@@ -1066,7 +1063,7 @@ public class VrfEntryListener extends AsyncDataTreeChangeListenerBase<VrfEntry,
                         PrefixesBuilder prefixBuilder = new PrefixesBuilder();
                         prefixBuilder.setDpnId(lri.getDpnId());
                         BigInteger dpnId = checkDeleteLocalFibEntry(prefixBuilder.build(), nextHopAddressList.get(0),
-                                vpnId, rd, vrfEntry, isExtraroute, lri.getParentVpnid());
+                                vpnId, rd, vrfEntry, isExtraroute);
                         if (!dpnId.equals(BigInteger.ZERO)) {
                             returnLocalDpnId.add(dpnId);
                         }
@@ -1078,7 +1075,7 @@ public class VrfEntryListener extends AsyncDataTreeChangeListenerBase<VrfEntry,
             LOG.debug("Obtained prefix to interface for rd {} prefix {}", rd, vrfEntry.getDestPrefix());
             String localNextHopIP = localNextHopInfo.getIpAddress();
             BigInteger dpnId = checkDeleteLocalFibEntry(localNextHopInfo, localNextHopIP,
-                vpnId, rd, vrfEntry, isExtraroute, vpnId /*parentVpnId*/);
+                vpnId, rd, vrfEntry, isExtraroute);
             if (!dpnId.equals(BigInteger.ZERO)) {
                 returnLocalDpnId.add(dpnId);
             }
@@ -1089,7 +1086,7 @@ public class VrfEntryListener extends AsyncDataTreeChangeListenerBase<VrfEntry,
 
     private BigInteger checkDeleteLocalFibEntry(Prefixes localNextHopInfo, final String localNextHopIP,
                                                 final Long vpnId, final String rd, final VrfEntry vrfEntry,
-                                                boolean isExtraroute, final Long parentVpnId) {
+                                                boolean isExtraroute) {
         if (localNextHopInfo != null) {
             final BigInteger dpnId = localNextHopInfo.getDpnId();
             if (Prefixes.PrefixCue.Nat.equals(localNextHopInfo.getPrefixCue())) {
@@ -1125,7 +1122,7 @@ public class VrfEntryListener extends AsyncDataTreeChangeListenerBase<VrfEntry,
             //TODO: verify below adjacency call need to be optimized (?)
             //In case of the removal of the extra route, the loadbalancing group is updated
             if (!isExtraroute) {
-                baseVrfEntryHandler.deleteLocalAdjacency(dpnId, parentVpnId, localNextHopIP, vrfEntry.getDestPrefix());
+                baseVrfEntryHandler.deleteLocalAdjacency(dpnId, vpnId, localNextHopIP, vrfEntry.getDestPrefix());
             }
             return dpnId;
         }
index c014f5335fccd9baa2a7082a016380ae33a862cd..2c5523c00629e21ecdf70f82a8acda08b9d11d15 100644 (file)
@@ -27,7 +27,8 @@
                    interface="org.opendaylight.yang.gen.v1.urn.opendaylight.genius.interfacemanager.rpcs.rev160406.OdlInterfaceRpcService"/>
   <odl:rpc-service id="salGroupService"
                    interface="org.opendaylight.yang.gen.v1.urn.opendaylight.group.service.rev130918.SalGroupService" />
-
+  <odl:rpc-service id="lockManagerService"
+                   interface="org.opendaylight.yang.gen.v1.urn.opendaylight.genius.lockmanager.rev160413.LockManagerService" />
   <odl:rpc-implementation ref="fibRpcServiceImpl"/>
 
   <service ref="fibManagerImpl"
index 2dd668edd091ee3f44028ff86625a9d78d204d51..e30555f17a55858666948e6aab98967a5bba8b71 100644 (file)
@@ -116,11 +116,24 @@ public class NeutronRouterChangeListener extends AsyncDataTreeChangeListenerBase
         if (!oldRoutes.equals(newRoutes)) {
             newRoutes.removeIf(oldRoutes::remove);
 
-            handleChangedRoutes(vpnId, newRoutes, NwConstants.ADD_FLOW);
-
             if (!oldRoutes.isEmpty()) {
                 handleChangedRoutes(vpnId, oldRoutes, NwConstants.DEL_FLOW);
             }
+
+            //After initial extra-route configuration using cmd-"neutron router-update RouterA destination=IP-A,
+            // nexthop=prefix-A",if another update is done using command - "neutron router-update RouterA
+            // destination=IP-A,nexthop=prefix-B",neutron router listener calls update on prefix-A as well as prefix-B.
+            // On prefix-A , secondary adj (IP-A) is removed ,where as its added on prefix-B. This back-to-back update
+            // creates race-condition in Vrf Engine ,leading inconsistencies in l3nexthop, VpnExtraRoute,
+            // VpnInterfaceOp DS. Hence a temporary fix of 2sec delay is introduced in neutron.
+            // A better fix/design need to be thought to avoid race condition
+            try {
+                Thread.sleep(2000); // sleep for 2sec
+            } catch (java.lang.InterruptedException e) {
+                LOG.error("Exception while sleeping", e);
+            }
+
+            handleChangedRoutes(vpnId, newRoutes, NwConstants.ADD_FLOW);
         }
 
         nvpnNatManager.handleExternalNetworkForRouter(original, update);
index 0db0d9cf05c2782061cdecb4b0ebd8c7203fd2a3..008819fa5c5ed561e386b1f7ce3da2b20200ef63 100755 (executable)
@@ -903,7 +903,7 @@ public class VpnInterfaceManager extends AsyncDataTreeChangeListenerBase<VpnInte
             List<String> nextHopList = adj.getNextHopIpList();
             // If TEP is added , update the nexthop of primary adjacency.
             // Secondary adj nexthop is already pointing to primary adj IP address.
-            if (nextHopList != null && !nextHopList.isEmpty() && nextHopList.get(0).equalsIgnoreCase(srcTepIp)) {
+            if (nextHopList != null && !nextHopList.isEmpty()) {
                 /* everything right already */
             } else {
                 isNextHopAddReqd = true;