Bug fix for route population optimization

Change-Id: Ibdaeaff86a03a64670e08db45b050af93a092df7

Bug fix for route population optimization
Change-Id: Ibdaeaff86a03a64670e08db45b050af93a092df7
sangho · Gerrit Code Review
Commit 45b009c7bcec0d2ec8974eaebb5f7498ee6565e9 45b009c7 1 parent 9bb32ab7
Showing 3 changed files with 59 additions and 23 deletions
apps/segmentrouting/src/main/java/org/onosproject/segmentrouting/DefaultRoutingHandler.java
apps/segmentrouting/src/main/java/org/onosproject/segmentrouting/RoutingRulePopulator.java
drivers/src/main/resources/onos-drivers.xml
--- a/apps/segmentrouting/src/main/java/org/onosproject/segmentrouting/DefaultRoutingHandler.java
View file @45b009c
+++ b/apps/segmentrouting/src/main/java/org/onosproject/segmentrouting/DefaultRoutingHandler.java
View file @45b009c
@@ -43,6 +43,7 @@ public class DefaultRoutingHandler {
     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 @@ public class DefaultRoutingHandler {
                 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,16 +181,15 @@ public class DefaultRoutingHandler {
                 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;
+            } else {
-            }
                 DeviceId src = link.get(0);
                 DeviceId dst = link.get(1);
-            ECMPShortestPathGraph ecmpSpg = new ECMPShortestPathGraph(dst, srManager);
+                log.trace("repopulateRoutingRulesForRoutes: running ECMP graph "
-
+                        + "for device {}", dst);
-            currentEcmpSpgMap.put(dst, ecmpSpg);
+                ECMPShortestPathGraph ecmpSpg = updatedEcmpSpgMap.get(dst);
                 HashMap<Integer, HashMap<DeviceId, ArrayList<ArrayList<DeviceId>>>> switchVia =
                         ecmpSpg.getAllLearnedSwitchesAndVia();
                 for (Integer itrIdx : switchVia.keySet()) {
@@ -200,6 +212,8 @@ public class DefaultRoutingHandler {
                         }
                     }
                 }
+                currentEcmpSpgMap.put(dst, ecmpSpg);
+            }
         }
         return true;
     }
@@ -239,6 +253,7 @@ public class DefaultRoutingHandler {
                     }
                 }
             }
+
         }
         return routes;
@@ -251,8 +266,16 @@ public class DefaultRoutingHandler {
         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 @@ public class DefaultRoutingHandler {
                 routes.add(route);
                 continue;
             }
-            ECMPShortestPathGraph newEcmpSpg =
+            log.debug("computeRouteChange: running ECMP graph "
-                    new ECMPShortestPathGraph(sw.id(), srManager);
+                    + "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()) {
+            for (Integer itrIdx : switchViaUpdated.keySet()) {
-                HashMap<DeviceId, ArrayList<ArrayList<DeviceId>>> swViaMap =
+                HashMap<DeviceId, ArrayList<ArrayList<DeviceId>>> swViaMapUpdated =
-                        switchVia.get(itrIdx);
+                        switchViaUpdated.get(itrIdx);
-                for (DeviceId srcSw : swViaMap.keySet()) {
+                for (DeviceId srcSw : swViaMapUpdated.keySet()) {
-                    ArrayList<ArrayList<DeviceId>> via1 = swViaMap.get(srcSw);
+                    ArrayList<ArrayList<DeviceId>> viaUpdated = swViaMapUpdated.get(srcSw);
-                    ArrayList<ArrayList<DeviceId>> via2 = getVia(switchViaUpdated, srcSw);
+                    ArrayList<ArrayList<DeviceId>> via = getVia(switchVia, srcSw);
-                    if (!via1.equals(via2)) {
+                    if (via.isEmpty() || !viaUpdated.equals(via)) {
                         ArrayList<DeviceId> route = new ArrayList<>();
                         route.add(srcSw);
                         route.add(sw.id());
@@ -282,7 +307,15 @@ public class DefaultRoutingHandler {
                     }
                 }
             }
+        }
+        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;
--- a/apps/segmentrouting/src/main/java/org/onosproject/segmentrouting/RoutingRulePopulator.java
View file @45b009c
+++ b/apps/segmentrouting/src/main/java/org/onosproject/segmentrouting/RoutingRulePopulator.java
View file @45b009c
@@ -299,15 +299,15 @@ public class RoutingRulePopulator {
         }
         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 @@ public class RoutingRulePopulator {
         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);
+        Set<Link> links = srManager.linkService.getDeviceLinks(srcId);
-        DeviceId destId = (DeviceId) destIds.toArray()[0];
+        for (DeviceId destId: destIds) {
             for (Link link : links) {
                 if (link.dst().deviceId().equals(destId)) {
-                return link;
+                    return link.src().port();
+                } else if (link.src().deviceId().equals(destId)) {
+                    return link.dst().port();
+                }
             }
         }
--- a/drivers/src/main/resources/onos-drivers.xml
View file @45b009c
+++ b/drivers/src/main/resources/onos-drivers.xml
View file @45b009c
@@ -38,7 +38,7 @@
     <driver name="cpqd" manufacturer="Stanford University, Ericsson Research and CPqD Research"
             hwVersion="OpenFlow 1.3 Reference Userspace Switch" swVersion=".*">
         <behaviour api="org.onosproject.net.behaviour.Pipeliner"
-                   impl="org.onosproject.driver.pipeline.DefaultSingleTablePipeline"/>
+                   impl="org.onosproject.driver.pipeline.SpringOpenTTP"/>
         <behaviour api="org.onosproject.openflow.controller.driver.OpenFlowSwitchDriver"
                    impl="org.onosproject.driver.handshaker.OFSwitchImplSpringOpenTTP"/>
     </driver>