Bug fix for route population optimization
Change-Id: Ibdaeaff86a03a64670e08db45b050af93a092df7
diff --git a/src/main/java/org/onosproject/segmentrouting/DefaultRoutingHandler.java b/src/main/java/org/onosproject/segmentrouting/DefaultRoutingHandler.java
index 7bb8899..0fa2aca 100644
--- a/src/main/java/org/onosproject/segmentrouting/DefaultRoutingHandler.java
+++ b/src/main/java/org/onosproject/segmentrouting/DefaultRoutingHandler.java
@@ -43,6 +43,7 @@
private SegmentRoutingManager srManager;
private RoutingRulePopulator rulePopulator;
private HashMap<DeviceId, ECMPShortestPathGraph> currentEcmpSpgMap;
+ private HashMap<DeviceId, ECMPShortestPathGraph> updatedEcmpSpgMap;
private DeviceConfiguration config;
private Status populationStatus;
@@ -128,6 +129,18 @@
return true;
}
+ // Take the snapshots of the links
+ updatedEcmpSpgMap = new HashMap<>();
+ for (Device sw : srManager.deviceService.getDevices()) {
+ if (srManager.mastershipService.
+ getLocalRole(sw.id()) != MastershipRole.MASTER) {
+ continue;
+ }
+ ECMPShortestPathGraph ecmpSpgUpdated =
+ new ECMPShortestPathGraph(sw.id(), srManager);
+ updatedEcmpSpgMap.put(sw.id(), ecmpSpgUpdated);
+ }
+
log.info("Starts rule population from link change");
Set<ArrayList<DeviceId>> routeChanges;
@@ -168,37 +181,38 @@
if (populateEcmpRoutingRules(link.get(0), ecmpSpg)) {
currentEcmpSpgMap.put(link.get(0), ecmpSpg);
} else {
- log.warn("Failed to populate the flow ruls from {} to all", link.get(0));
+ log.warn("Failed to populate the flow rules from {} to all", link.get(0));
return false;
}
- continue;
- }
- DeviceId src = link.get(0);
- DeviceId dst = link.get(1);
- ECMPShortestPathGraph ecmpSpg = new ECMPShortestPathGraph(dst, srManager);
-
- currentEcmpSpgMap.put(dst, ecmpSpg);
- HashMap<Integer, HashMap<DeviceId, ArrayList<ArrayList<DeviceId>>>> switchVia =
- ecmpSpg.getAllLearnedSwitchesAndVia();
- for (Integer itrIdx : switchVia.keySet()) {
- HashMap<DeviceId, ArrayList<ArrayList<DeviceId>>> swViaMap =
- switchVia.get(itrIdx);
- for (DeviceId targetSw : swViaMap.keySet()) {
- if (!targetSw.equals(src)) {
- continue;
- }
- Set<DeviceId> nextHops = new HashSet<>();
- for (ArrayList<DeviceId> via : swViaMap.get(targetSw)) {
- if (via.isEmpty()) {
- nextHops.add(dst);
- } else {
- nextHops.add(via.get(0));
+ } else {
+ DeviceId src = link.get(0);
+ DeviceId dst = link.get(1);
+ log.trace("repopulateRoutingRulesForRoutes: running ECMP graph "
+ + "for device {}", dst);
+ ECMPShortestPathGraph ecmpSpg = updatedEcmpSpgMap.get(dst);
+ HashMap<Integer, HashMap<DeviceId, ArrayList<ArrayList<DeviceId>>>> switchVia =
+ ecmpSpg.getAllLearnedSwitchesAndVia();
+ for (Integer itrIdx : switchVia.keySet()) {
+ HashMap<DeviceId, ArrayList<ArrayList<DeviceId>>> swViaMap =
+ switchVia.get(itrIdx);
+ for (DeviceId targetSw : swViaMap.keySet()) {
+ if (!targetSw.equals(src)) {
+ continue;
+ }
+ Set<DeviceId> nextHops = new HashSet<>();
+ for (ArrayList<DeviceId> via : swViaMap.get(targetSw)) {
+ if (via.isEmpty()) {
+ nextHops.add(dst);
+ } else {
+ nextHops.add(via.get(0));
+ }
+ }
+ if (!populateEcmpRoutingRulePartial(targetSw, dst, nextHops)) {
+ return false;
}
}
- if (!populateEcmpRoutingRulePartial(targetSw, dst, nextHops)) {
- return false;
- }
}
+ currentEcmpSpgMap.put(dst, ecmpSpg);
}
}
return true;
@@ -239,6 +253,7 @@
}
}
}
+
}
return routes;
@@ -251,8 +266,16 @@
for (Device sw : srManager.deviceService.getDevices()) {
if (srManager.mastershipService.
getLocalRole(sw.id()) != MastershipRole.MASTER) {
+ log.warn("No mastership for {} and skip route optimization");
continue;
}
+
+ log.trace("link of {} - ", sw.id());
+ for (Link link: srManager.linkService.getDeviceLinks(sw.id())) {
+ log.trace("{} -> {} ", link.src().deviceId(), link.dst().deviceId());
+ }
+
+ log.debug("Checking route change for switch {}", sw.id());
ECMPShortestPathGraph ecmpSpg = currentEcmpSpgMap.get(sw.id());
if (ecmpSpg == null) {
log.debug("No existing ECMP path for Switch {}", sw.id());
@@ -261,20 +284,22 @@
routes.add(route);
continue;
}
- ECMPShortestPathGraph newEcmpSpg =
- new ECMPShortestPathGraph(sw.id(), srManager);
+ log.debug("computeRouteChange: running ECMP graph "
+ + "for device {}", sw.id());
+ ECMPShortestPathGraph newEcmpSpg = updatedEcmpSpgMap.get(sw.id());
+ currentEcmpSpgMap.put(sw.id(), newEcmpSpg);
HashMap<Integer, HashMap<DeviceId, ArrayList<ArrayList<DeviceId>>>> switchVia =
ecmpSpg.getAllLearnedSwitchesAndVia();
HashMap<Integer, HashMap<DeviceId, ArrayList<ArrayList<DeviceId>>>> switchViaUpdated =
newEcmpSpg.getAllLearnedSwitchesAndVia();
- for (Integer itrIdx : switchVia.keySet()) {
- HashMap<DeviceId, ArrayList<ArrayList<DeviceId>>> swViaMap =
- switchVia.get(itrIdx);
- for (DeviceId srcSw : swViaMap.keySet()) {
- ArrayList<ArrayList<DeviceId>> via1 = swViaMap.get(srcSw);
- ArrayList<ArrayList<DeviceId>> via2 = getVia(switchViaUpdated, srcSw);
- if (!via1.equals(via2)) {
+ for (Integer itrIdx : switchViaUpdated.keySet()) {
+ HashMap<DeviceId, ArrayList<ArrayList<DeviceId>>> swViaMapUpdated =
+ switchViaUpdated.get(itrIdx);
+ for (DeviceId srcSw : swViaMapUpdated.keySet()) {
+ ArrayList<ArrayList<DeviceId>> viaUpdated = swViaMapUpdated.get(srcSw);
+ ArrayList<ArrayList<DeviceId>> via = getVia(switchVia, srcSw);
+ if (via.isEmpty() || !viaUpdated.equals(via)) {
ArrayList<DeviceId> route = new ArrayList<>();
route.add(srcSw);
route.add(sw.id());
@@ -282,7 +307,15 @@
}
}
}
+ }
+ for (ArrayList<DeviceId> link: routes) {
+ log.trace("Link changes - ");
+ if (link.size() == 1) {
+ log.trace(" : {} - all", link.get(0));
+ } else {
+ log.trace(" : {} - {}", link.get(0), link.get(1));
+ }
}
return routes;
diff --git a/src/main/java/org/onosproject/segmentrouting/RoutingRulePopulator.java b/src/main/java/org/onosproject/segmentrouting/RoutingRulePopulator.java
index cacde7b..f3ccaa0 100644
--- a/src/main/java/org/onosproject/segmentrouting/RoutingRulePopulator.java
+++ b/src/main/java/org/onosproject/segmentrouting/RoutingRulePopulator.java
@@ -265,7 +265,7 @@
.withPriority(100))
.withFlag(ForwardingObjective.Flag.SPECIFIC);
log.debug("Installing MPLS forwarding objective in switch {}",
- deviceId);
+ deviceId);
srManager.flowObjectiveService.forward(deviceId,
fwdObjBuilder.add());
rulePopulationCounter.incrementAndGet();
@@ -299,15 +299,15 @@
}
if (!isECMPSupportedInTransitRouter() && !config.isEdgeDevice(deviceId)) {
- Link link = selectOneLink(deviceId, nextHops);
+ PortNumber port = selectOnePort(deviceId, nextHops);
DeviceId nextHop = (DeviceId) nextHops.toArray()[0];
- if (link == null) {
+ if (port == null) {
log.warn("No link from {} to {}", deviceId, nextHops);
return null;
}
tbuilder.setEthSrc(config.getDeviceMac(deviceId))
.setEthDst(config.getDeviceMac(nextHop))
- .setOutput(link.src().port());
+ .setOutput(port);
fwdBuilder.withTreatment(tbuilder.build());
} else {
NeighborSet ns = new NeighborSet(nextHops);
@@ -356,13 +356,16 @@
srManager.flowObjectiveService.filter(deviceId, fob.add());
}
- private Link selectOneLink(DeviceId srcId, Set<DeviceId> destIds) {
+ private PortNumber selectOnePort(DeviceId srcId, Set<DeviceId> destIds) {
- Set<Link> links = srManager.linkService.getDeviceEgressLinks(srcId);
- DeviceId destId = (DeviceId) destIds.toArray()[0];
- for (Link link : links) {
- if (link.dst().deviceId().equals(destId)) {
- return link;
+ Set<Link> links = srManager.linkService.getDeviceLinks(srcId);
+ for (DeviceId destId: destIds) {
+ for (Link link : links) {
+ if (link.dst().deviceId().equals(destId)) {
+ return link.src().port();
+ } else if (link.src().deviceId().equals(destId)) {
+ return link.dst().port();
+ }
}
}