Adapt FIBs to new route interface

Change-Id: I8ac19ad578aac4607fd8319186b6568a21edc1fd

Adapt FIBs to new route interface
Change-Id: I8ac19ad578aac4607fd8319186b6568a21edc1fd
Jonathan Hart · Gerrit Code Review
Commit a2eb9ffddeec827da95fc40845b656dff97c2940 a2eb9ffd 1 parent d2b2888a
Showing 8 changed files with 93 additions and 145 deletions
apps/routing/src/main/java/org/onosproject/routing/impl/IntentSynchronizer.java
apps/routing/src/main/java/org/onosproject/routing/impl/SingleSwitchFibInstaller.java
apps/routing/src/test/java/org/onosproject/routing/impl/SingleSwitchFibInstallerTest.java
apps/sdnip/src/main/java/org/onosproject/sdnip/SdnIp.java
apps/sdnip/src/main/java/org/onosproject/sdnip/SdnIpFib.java
apps/sdnip/src/test/java/org/onosproject/sdnip/SdnIpFibTest.java
apps/vrouter/src/main/java/org/onosproject/vrouter/Vrouter.java
core/api/src/main/java/org/onosproject/net/host/InterfaceIpAddress.java
--- a/apps/routing/src/main/java/org/onosproject/routing/impl/IntentSynchronizer.java
View file @a2eb9ff
+++ b/apps/routing/src/main/java/org/onosproject/routing/impl/IntentSynchronizer.java
View file @a2eb9ff
@@ -89,9 +89,9 @@ public class IntentSynchronizer implements IntentSynchronizationService,
     @Activate
     public void activate() {
-        intentsSynchronizerExecutor = createExecutor();
         this.localNodeId = clusterService.getLocalNode().id();
         this.appId = coreService.registerApplication(APP_NAME);
+        intentsSynchronizerExecutor = createExecutor();
         leadershipService.addListener(leadershipEventListener);
         leadershipService.runForLeadership(appId.name());
--- a/apps/routing/src/main/java/org/onosproject/routing/impl/SingleSwitchFibInstaller.java
View file @a2eb9ff
+++ b/apps/routing/src/main/java/org/onosproject/routing/impl/SingleSwitchFibInstaller.java
View file @a2eb9ff
@@ -17,9 +17,7 @@
 package org.onosproject.routing.impl;
 import com.google.common.collect.ConcurrentHashMultiset;
-import com.google.common.collect.HashMultimap;
 import com.google.common.collect.Maps;
-import com.google.common.collect.Multimap;
 import com.google.common.collect.Multiset;
 import org.apache.felix.scr.annotations.Activate;
 import org.apache.felix.scr.annotations.Component;
@@ -40,6 +38,10 @@ import org.onosproject.incubator.net.intf.Interface;
 import org.onosproject.incubator.net.intf.InterfaceEvent;
 import org.onosproject.incubator.net.intf.InterfaceListener;
 import org.onosproject.incubator.net.intf.InterfaceService;
+import org.onosproject.incubator.net.routing.ResolvedRoute;
+import org.onosproject.incubator.net.routing.RouteEvent;
+import org.onosproject.incubator.net.routing.RouteListener;
+import org.onosproject.incubator.net.routing.RouteService;
 import org.onosproject.net.ConnectPoint;
 import org.onosproject.net.DeviceId;
 import org.onosproject.net.config.NetworkConfigEvent;
@@ -56,24 +58,19 @@ import org.onosproject.net.flow.criteria.Criteria;
 import org.onosproject.net.flowobjective.DefaultFilteringObjective;
 import org.onosproject.net.flowobjective.DefaultForwardingObjective;
 import org.onosproject.net.flowobjective.DefaultNextObjective;
+import org.onosproject.net.flowobjective.DefaultObjectiveContext;
 import org.onosproject.net.flowobjective.FilteringObjective;
 import org.onosproject.net.flowobjective.FlowObjectiveService;
 import org.onosproject.net.flowobjective.ForwardingObjective;
 import org.onosproject.net.flowobjective.NextObjective;
 import org.onosproject.net.flowobjective.ObjectiveContext;
-import org.onosproject.net.flowobjective.DefaultObjectiveContext;
-import org.onosproject.routing.FibEntry;
-import org.onosproject.routing.FibListener;
-import org.onosproject.routing.FibUpdate;
 import org.onosproject.routing.RoutingService;
 import org.onosproject.routing.config.RouterConfig;
 import org.osgi.service.component.ComponentContext;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
-import java.util.Collection;
 import java.util.Dictionary;
-import java.util.HashMap;
 import java.util.List;
 import java.util.Map;
 import java.util.Set;
@@ -96,7 +93,7 @@ public class SingleSwitchFibInstaller {
     protected CoreService coreService;
     @Reference(cardinality = ReferenceCardinality.MANDATORY_UNARY)
-    protected RoutingService routingService;
+    protected RouteService routeService;
     @Reference(cardinality = ReferenceCardinality.MANDATORY_UNARY)
     protected InterfaceService interfaceService;
@@ -137,11 +134,9 @@ public class SingleSwitchFibInstaller {
     // Mapping from next hop IP to next hop object containing group info
     private final Map<IpAddress, Integer> nextHops = Maps.newHashMap();
-    // Stores FIB updates that are waiting for groups to be set up
-    private final Multimap<NextHopGroupKey, FibEntry> pendingUpdates = HashMultimap.create();
-
     //interface object for event
     private InternalInterfaceListener internalInterfaceList = new InternalInterfaceListener();
+    private InternalRouteListener routeListener = new InternalRouteListener();
     @Activate
     protected void activate(ComponentContext context) {
@@ -155,9 +150,6 @@ public class SingleSwitchFibInstaller {
         interfaceService.addListener(internalInterfaceList);
-        routingService.addFibListener(new InternalFibListener());
-        routingService.start();
-
         updateConfig();
         log.info("Started");
@@ -165,10 +157,8 @@ public class SingleSwitchFibInstaller {
     @Deactivate
     protected void deactivate() {
-        routingService.stop();
+        routeService.removeListener(routeListener);
-
         deviceService.removeListener(deviceListener);
-
         interfaceService.removeListener(internalInterfaceList);
         //processIntfFilters(false, configService.getInterfaces()); //TODO necessary?
@@ -208,6 +198,8 @@ public class SingleSwitchFibInstaller {
         interfaces = routerConfig.getInterfaces();
         log.info("Using interfaces: {}", interfaces.isEmpty() ? "all" : interfaces);
+        routeService.addListener(routeListener);
+
         updateDevice();
     }
@@ -229,55 +221,30 @@ public class SingleSwitchFibInstaller {
         }
     }
-    private void updateFibEntry(Collection<FibUpdate> updates) {
+    private void updateRoute(ResolvedRoute route) {
-        Map<FibEntry, Integer> toInstall = new HashMap<>(updates.size());
+        addNextHop(route);
-
-        for (FibUpdate update : updates) {
-            FibEntry entry = update.entry();
-
-            addNextHop(entry);
-
-            Integer nextId;
-            synchronized (pendingUpdates) {
-                nextId = nextHops.get(entry.nextHopIp());
-            }
-
-            toInstall.put(update.entry(), nextId);
-        }
-
-        installFlows(toInstall);
-    }
-
-    private void installFlows(Map<FibEntry, Integer> entriesToInstall) {
-
-        for (Map.Entry<FibEntry, Integer> entry : entriesToInstall.entrySet()) {
-            FibEntry fibEntry = entry.getKey();
-            Integer nextId = entry.getValue();
-            flowObjectiveService.forward(deviceId,
+        Integer nextId;
-                    generateRibForwardingObj(fibEntry.prefix(), nextId).add());
+        synchronized (this) {
-            log.trace("Sending forwarding objective {} -> nextId:{}", fibEntry, nextId);
+            nextId = nextHops.get(route.nextHop());
         }
+        flowObjectiveService.forward(deviceId,
+                generateRibForwardingObj(route.prefix(), nextId).add());
+        log.trace("Sending forwarding objective {} -> nextId:{}", route, nextId);
     }
-    private synchronized void deleteFibEntry(Collection<FibUpdate> withdraws) {
+    private synchronized void deleteRoute(ResolvedRoute route) {
-
+        //Integer nextId = nextHops.get(route.nextHop());
-        for (FibUpdate update : withdraws) {
-            FibEntry entry = update.entry();
-            //Integer nextId = nextHops.get(entry.nextHopIp());
-
-           /* Group group = deleteNextHop(entry.prefix());
-            if (group == null) {
-                log.warn("Group not found when deleting {}", entry);
-                return;
-            }*/
-
-            flowObjectiveService.forward(deviceId,
-                    generateRibForwardingObj(entry.prefix(), null).remove());
-        }
+        /* Group group = deleteNextHop(route.prefix());
+        if (group == null) {
+            log.warn("Group not found when deleting {}", route);
+            return;
+        }*/
+        flowObjectiveService.forward(deviceId,
+                generateRibForwardingObj(route.prefix(), null).remove());
     }
     private ForwardingObjective.Builder generateRibForwardingObj(IpPrefix prefix,
@@ -306,20 +273,20 @@ public class SingleSwitchFibInstaller {
         return fwdBuilder;
     }
-    private synchronized void addNextHop(FibEntry entry) {
+    private synchronized void addNextHop(ResolvedRoute route) {
-        prefixToNextHop.put(entry.prefix(), entry.nextHopIp());
+        prefixToNextHop.put(route.prefix(), route.nextHop());
-        if (nextHopsCount.count(entry.nextHopIp()) == 0) {
+        if (nextHopsCount.count(route.nextHop()) == 0) {
             // There was no next hop in the multiset
-            Interface egressIntf = interfaceService.getMatchingInterface(entry.nextHopIp());
+            Interface egressIntf = interfaceService.getMatchingInterface(route.nextHop());
             if (egressIntf == null) {
-                log.warn("no egress interface found for {}", entry);
+                log.warn("no egress interface found for {}", route);
                 return;
             }
-            NextHopGroupKey groupKey = new NextHopGroupKey(entry.nextHopIp());
+            NextHopGroupKey groupKey = new NextHopGroupKey(route.nextHop());
-            NextHop nextHop = new NextHop(entry.nextHopIp(), entry.nextHopMac(), groupKey);
+            NextHop nextHop = new NextHop(route.nextHop(), route.nextHopMac(), groupKey);
             TrafficTreatment.Builder treatment = DefaultTrafficTreatment.builder()
                     .setEthSrc(egressIntf.mac())
@@ -356,12 +323,12 @@ public class SingleSwitchFibInstaller {
             if (routeToNextHop) {
                 // Install route to next hop
                 ForwardingObjective fob =
-                        generateRibForwardingObj(IpPrefix.valueOf(entry.nextHopIp(), 32), nextId).add();
+                        generateRibForwardingObj(IpPrefix.valueOf(route.nextHop(), 32), nextId).add();
                 flowObjectiveService.forward(deviceId, fob);
             }
         }
-        nextHopsCount.add(entry.nextHopIp());
+        nextHopsCount.add(route.nextHop());
     }
     /*private synchronized Group deleteNextHop(IpPrefix prefix) {
@@ -452,13 +419,21 @@ public class SingleSwitchFibInstaller {
         flowObjectiveService.filter(deviceId, filter);
     }
-    private class InternalFibListener implements FibListener {
+    private class InternalRouteListener implements RouteListener {
-
         @Override
-        public void update(Collection<FibUpdate> updates,
+        public void event(RouteEvent event) {
-                           Collection<FibUpdate> withdraws) {
+            ResolvedRoute route = event.subject();
-            SingleSwitchFibInstaller.this.deleteFibEntry(withdraws);
+            switch (event.type()) {
-            SingleSwitchFibInstaller.this.updateFibEntry(updates);
+            case ROUTE_ADDED:
+            case ROUTE_UPDATED:
+                updateRoute(route);
+                break;
+            case ROUTE_REMOVED:
+                deleteRoute(route);
+                break;
+            default:
+                break;
+            }
         }
     }
--- a/apps/routing/src/test/java/org/onosproject/routing/impl/SingleSwitchFibInstallerTest.java
View file @a2eb9ff
+++ b/apps/routing/src/test/java/org/onosproject/routing/impl/SingleSwitchFibInstallerTest.java
View file @a2eb9ff
--- a/apps/sdnip/src/main/java/org/onosproject/sdnip/SdnIp.java
View file @a2eb9ff
+++ b/apps/sdnip/src/main/java/org/onosproject/sdnip/SdnIp.java
View file @a2eb9ff
@@ -73,7 +73,6 @@ public class SdnIp {
     private static List<String> components = new ArrayList<>();
     static {
         components.add("org.onosproject.routing.bgp.BgpSessionManager");
-        components.add("org.onosproject.routing.impl.Router");
         components.add(org.onosproject.sdnip.SdnIpFib.class.getName());
     }
--- a/apps/sdnip/src/main/java/org/onosproject/sdnip/SdnIpFib.java
View file @a2eb9ff
+++ b/apps/sdnip/src/main/java/org/onosproject/sdnip/SdnIpFib.java
View file @a2eb9ff
@@ -34,6 +34,10 @@ import org.onosproject.incubator.net.intf.Interface;
 import org.onosproject.incubator.net.intf.InterfaceEvent;
 import org.onosproject.incubator.net.intf.InterfaceListener;
 import org.onosproject.incubator.net.intf.InterfaceService;
+import org.onosproject.incubator.net.routing.ResolvedRoute;
+import org.onosproject.incubator.net.routing.RouteEvent;
+import org.onosproject.incubator.net.routing.RouteListener;
+import org.onosproject.incubator.net.routing.RouteService;
 import org.onosproject.net.ConnectPoint;
 import org.onosproject.net.flow.DefaultTrafficSelector;
 import org.onosproject.net.flow.DefaultTrafficTreatment;
@@ -43,21 +47,15 @@ import org.onosproject.net.intent.Constraint;
 import org.onosproject.net.intent.Key;
 import org.onosproject.net.intent.MultiPointToSinglePointIntent;
 import org.onosproject.net.intent.constraint.PartialFailureConstraint;
-import org.onosproject.routing.FibListener;
-import org.onosproject.routing.FibUpdate;
 import org.onosproject.routing.IntentSynchronizationService;
-import org.onosproject.routing.RoutingService;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
-import java.util.Collection;
 import java.util.HashSet;
 import java.util.Map;
 import java.util.Set;
 import java.util.concurrent.ConcurrentHashMap;
-import static com.google.common.base.Preconditions.checkArgument;
-
 /**
  * FIB component of SDN-IP.
  */
@@ -75,9 +73,9 @@ public class SdnIpFib {
     protected CoreService coreService;
     @Reference(cardinality = ReferenceCardinality.MANDATORY_UNARY)
-    protected RoutingService routingService;
+    protected RouteService routeService;
-    private final InternalFibListener fibListener = new InternalFibListener();
+    private final InternalRouteListener routeListener = new InternalRouteListener();
     private final InternalInterfaceListener interfaceListener = new InternalInterfaceListener();
     private static final int PRIORITY_OFFSET = 100;
@@ -96,69 +94,41 @@ public class SdnIpFib {
         interfaceService.addListener(interfaceListener);
-        routingService.addFibListener(fibListener);
+        routeService.addListener(routeListener);
-        routingService.start();
     }
     @Deactivate
     public void deactivate() {
         interfaceService.removeListener(interfaceListener);
-        // TODO remove listener
+        routeService.removeListener(routeListener);
-        routingService.stop();
     }
-    private void update(Collection<FibUpdate> updates, Collection<FibUpdate> withdraws) {
+    private void update(ResolvedRoute route) {
-        int submitCount = 0, withdrawCount = 0;
-        //
-        // NOTE: Semantically, we MUST withdraw existing intents before
-        // submitting new intents.
-        //
         synchronized (this) {
-            MultiPointToSinglePointIntent intent;
+            IpPrefix prefix = route.prefix();
-
+            MultiPointToSinglePointIntent intent =
-            //
+                    generateRouteIntent(prefix, route.nextHop(), route.nextHopMac());
-            // Prepare the Intent batch operations for the intents to withdraw
+
-            //
+            if (intent == null) {
-            for (FibUpdate withdraw : withdraws) {
+                log.debug("SDN-IP no interface found for route {}", route);
-                checkArgument(withdraw.type() == FibUpdate.Type.DELETE,
+                return;
-                        "FibUpdate with wrong type in withdraws list");
-
-                IpPrefix prefix = withdraw.entry().prefix();
-                intent = routeIntents.remove(prefix);
-                if (intent == null) {
-                    log.trace("SDN-IP No intent in routeIntents to delete " +
-                            "for prefix: {}", prefix);
-                    continue;
-                }
-                intentSynchronizer.withdraw(intent);
-                withdrawCount++;
             }
-            //
+            routeIntents.put(prefix, intent);
-            // Prepare the Intent batch operations for the intents to submit
+            intentSynchronizer.submit(intent);
-            //
+        }
-            for (FibUpdate update : updates) {
+    }
-                checkArgument(update.type() == FibUpdate.Type.UPDATE,
-                        "FibUpdate with wrong type in updates list");
-
-                IpPrefix prefix = update.entry().prefix();
-                intent = generateRouteIntent(prefix, update.entry().nextHopIp(),
-                        update.entry().nextHopMac());
-
-                if (intent == null) {
-                    // This preserves the old semantics - if an intent can't be
-                    // generated, we don't do anything with that prefix. But
-                    // perhaps we should withdraw the old intent anyway?
-                    continue;
-                }
-                routeIntents.put(prefix, intent);
+    private void withdraw(ResolvedRoute route) {
-                intentSynchronizer.submit(intent);
+        synchronized (this) {
-                submitCount++;
+            IpPrefix prefix = route.prefix();
+            MultiPointToSinglePointIntent intent = routeIntents.remove(prefix);
+            if (intent == null) {
+                log.trace("SDN-IP no intent in routeIntents to delete " +
+                        "for prefix: {}", prefix);
+                return;
             }
-
+            intentSynchronizer.withdraw(intent);
-            log.debug("SDN-IP submitted {}/{}, withdrew = {}/{}", submitCount,
-                    updates.size(), withdrawCount, withdraws.size());
         }
     }
@@ -292,10 +262,20 @@ public class SdnIpFib {
         }
     }
-    private class InternalFibListener implements FibListener {
+    private class InternalRouteListener implements RouteListener {
         @Override
-        public void update(Collection<FibUpdate> updates, Collection<FibUpdate> withdraws) {
+        public void event(RouteEvent event) {
-            SdnIpFib.this.update(updates, withdraws);
+            switch (event.type()) {
+            case ROUTE_ADDED:
+            case ROUTE_UPDATED:
+                update(event.subject());
+                break;
+            case ROUTE_REMOVED:
+                withdraw(event.subject());
+                break;
+            default:
+                break;
+            }
         }
     }
--- a/apps/sdnip/src/test/java/org/onosproject/sdnip/SdnIpFibTest.java
View file @a2eb9ff
+++ b/apps/sdnip/src/test/java/org/onosproject/sdnip/SdnIpFibTest.java
View file @a2eb9ff
--- a/apps/vrouter/src/main/java/org/onosproject/vrouter/Vrouter.java
View file @a2eb9ff
+++ b/apps/vrouter/src/main/java/org/onosproject/vrouter/Vrouter.java
View file @a2eb9ff
@@ -49,7 +49,6 @@ public class Vrouter {
     private final List<String> components = ImmutableList.<String>builder()
             .add("org.onosproject.routing.fpm.FpmManager")
-            .add("org.onosproject.routing.impl.Router")
             .add("org.onosproject.routing.impl.SingleSwitchFibInstaller")
             .add("org.onosproject.routing.impl.ControlPlaneRedirectManager")
             .build();
--- a/core/api/src/main/java/org/onosproject/net/host/InterfaceIpAddress.java
View file @a2eb9ff
+++ b/core/api/src/main/java/org/onosproject/net/host/InterfaceIpAddress.java
View file @a2eb9ff
@@ -182,11 +182,6 @@ public class InterfaceIpAddress {
     @Override
     public String toString() {
-        /*return toStringHelper(this).add("ipAddress", ipAddress)
-            .add("subnetAddress", subnetAddress)
-            .add("broadcastAddress", broadcastAddress)
-            .add("peerAddress", peerAddress)
-            .omitNullValues().toString();*/
         return ipAddress.toString() + "/" + subnetAddress.prefixLength();
     }
 }