Bug fixes for route-path programming
- Leaves should avoid going through other leaves to reach destination
- dsNextObjective store should only be purged by new master before a full reroute
- move full reroute for master change to a different threadpool from retry filters
Change-Id: I33be83bfb90d5848cfe94ed0fcdc17f2cca6d73f
diff --git a/apps/segmentrouting/app/src/main/java/org/onosproject/segmentrouting/DefaultRoutingHandler.java b/apps/segmentrouting/app/src/main/java/org/onosproject/segmentrouting/DefaultRoutingHandler.java
index c64d452..31936e2 100644
--- a/apps/segmentrouting/app/src/main/java/org/onosproject/segmentrouting/DefaultRoutingHandler.java
+++ b/apps/segmentrouting/app/src/main/java/org/onosproject/segmentrouting/DefaultRoutingHandler.java
@@ -86,6 +86,9 @@
private volatile Status populationStatus;
private ScheduledExecutorService executorService
= newScheduledThreadPool(1, groupedThreads("retryftr", "retry-%d", log));
+ private ScheduledExecutorService executorServiceMstChg
+ = newScheduledThreadPool(1, groupedThreads("masterChg", "mstch-%d", log));
+
private Instant lastRoutingChange = Instant.EPOCH;
// Distributed store to keep track of ONOS instance that should program the
@@ -189,6 +192,14 @@
return (now - last) > STABLITY_THRESHOLD;
}
+ /**
+ * Gracefully shuts down the defaultRoutingHandler. Typically called when
+ * the app is deactivated
+ */
+ public void shutdown() {
+ executorService.shutdown();
+ executorServiceMstChg.shutdown();
+ }
//////////////////////////////////////
// Route path handling
@@ -1204,8 +1215,8 @@
void checkFullRerouteForMasterChange(DeviceId devId, MastershipEvent me) {
// give small delay to absorb mastership events that are caused by
// device that has disconnected from cluster
- executorService.schedule(new MasterChange(devId, me),
- MASTER_CHANGE_DELAY, TimeUnit.MILLISECONDS);
+ executorServiceMstChg.schedule(new MasterChange(devId, me),
+ MASTER_CHANGE_DELAY, TimeUnit.MILLISECONDS);
}
protected final class MasterChange implements Runnable {
@@ -1261,9 +1272,9 @@
if (srManager.mastershipService.isLocalMaster(devId)) {
// old master could have died when populating filters
populatePortAddressingRules(devId);
+ // old master could have died when creating groups
+ srManager.purgeHashedNextObjectiveStore(devId);
}
- // old master could have died when creating groups
- srManager.purgeHashedNextObjectiveStore(devId);
// XXX right now we have no fine-grained way to only make changes
// for the route paths affected by this device.
populateAllRoutingRules();
@@ -1663,11 +1674,13 @@
if (!target.equals(targetSw)) {
continue;
}
- if (!targetIsEdge && itrIdx > 1) {
- // optimization for spines to not use leaves to get
- // to a spine or other leaves
+ // optimization for spines to not use leaves to get
+ // to a spine or other leaves. Also leaves should not use other
+ // leaves to get to the destination
+ if ((!targetIsEdge && itrIdx > 1) || targetIsEdge) {
boolean pathdevIsEdge = false;
for (ArrayList<DeviceId> via : swViaMap.get(targetSw)) {
+ log.debug("Evaluating next-hop in path: {}", via);
for (DeviceId pathdev : via) {
try {
pathdevIsEdge = srManager.deviceConfiguration
@@ -1695,9 +1708,12 @@
nextHops.add(via.get(0));
}
}
+ log.debug("target {} --> dst: {} has next-hops:{}", targetSw,
+ dstSw, nextHops);
return nextHops;
}
}
+ log.debug("No next hops found for target:{} --> dst: {}", targetSw, dstSw);
return ImmutableSet.of(); //no next-hops found
}
diff --git a/apps/segmentrouting/app/src/main/java/org/onosproject/segmentrouting/SegmentRoutingManager.java b/apps/segmentrouting/app/src/main/java/org/onosproject/segmentrouting/SegmentRoutingManager.java
index 864a924..0322201 100644
--- a/apps/segmentrouting/app/src/main/java/org/onosproject/segmentrouting/SegmentRoutingManager.java
+++ b/apps/segmentrouting/app/src/main/java/org/onosproject/segmentrouting/SegmentRoutingManager.java
@@ -519,6 +519,7 @@
deviceListener = null;
groupHandlerMap.forEach((k, v) -> v.shutdown());
groupHandlerMap.clear();
+ defaultRoutingHandler.shutdown();
dsNextObjStore.destroy();
vlanNextObjStore.destroy();
@@ -1284,7 +1285,14 @@
.forEach(entry -> entry.getValue().cleanUpForNeighborDown(device.id()));
}
+ /**
+ * Purge the destinationSet nextObjective store of entries with this device
+ * as key. Erases app-level knowledge of hashed groups in this device.
+ *
+ * @param devId the device identifier
+ */
void purgeHashedNextObjectiveStore(DeviceId devId) {
+ log.debug("Purging hashed next-obj store for dev:{}", devId);
dsNextObjStore.entrySet().stream()
.filter(entry -> entry.getKey().deviceId().equals(devId))
.forEach(entry -> dsNextObjStore.remove(entry.getKey()));