Cosmetic refactoring for vRouter to bring CPRM and SSFI closer together.

Change-Id: I86836adff041195fae0dca5dec1a45d94444e10b
diff --git a/apps/routing/src/main/java/org/onosproject/routing/impl/ControlPlaneRedirectManager.java b/apps/routing/src/main/java/org/onosproject/routing/impl/ControlPlaneRedirectManager.java
index 7c00613..f72963c 100644
--- a/apps/routing/src/main/java/org/onosproject/routing/impl/ControlPlaneRedirectManager.java
+++ b/apps/routing/src/main/java/org/onosproject/routing/impl/ControlPlaneRedirectManager.java
@@ -138,7 +138,7 @@
 
         asyncDeviceFetcher = AsyncDeviceFetcher.create(deviceService);
 
-        readConfig();
+        processRouterConfig();
 
         // FIXME There can be an issue when this component is deactivated before vRouter
         applicationService.registerDeactivateHook(this.appId, () -> {
@@ -155,19 +155,10 @@
         asyncDeviceFetcher.shutdown();
     }
 
-    private RouterInterfaceManager createRouter(DeviceId deviceId, Set<String> configuredInterfaces) {
-        return new RouterInterfaceManager(deviceId,
-                configuredInterfaces,
-                interfaceService,
-                intf -> provisionInterface(intf, true),
-                intf -> provisionInterface(intf, false)
-        );
-    }
-
     /**
      * Sets up the router interfaces if router config is available.
      */
-    private void readConfig() {
+    private void processRouterConfig() {
         ApplicationId routingAppId =
                 coreService.registerApplication(RoutingService.ROUTER_APP_ID);
 
@@ -195,21 +186,50 @@
         }
     }
 
-    private void provisionInterface(Interface intf, boolean install) {
+    /**
+     * Cleans up after router config was removed.
+     */
+    private void removeRouterConfig() {
+        if (interfaceManager != null) {
+            interfaceManager.cleanup();
+        }
+    }
+
+    private RouterInterfaceManager createRouter(DeviceId deviceId, Set<String> configuredInterfaces) {
+        return new RouterInterfaceManager(deviceId,
+                configuredInterfaces,
+                interfaceService,
+                this::provisionInterface,
+                this::unprovisionInterface);
+    }
+
+    private void provisionInterface(Interface intf) {
+        updateInterfaceObjectives(intf, true);
+    }
+
+    private void unprovisionInterface(Interface intf) {
+        updateInterfaceObjectives(intf, false);
+    }
+
+    /**
+     * Installs or removes flow objectives relating to a give interface.
+     *
+     * @param intf interface to change objectives for
+     * @param install true to install the objectives, false to remove them
+     */
+    private void updateInterfaceObjectives(Interface intf, boolean install) {
         updateInterfaceForwarding(intf, install);
         updateOspfForwarding(intf, install);
     }
 
     /**
-     * Installs or removes the basic forwarding flows for each interface
-     * based on the flag used.
+     * Installs or removes the basic forwarding flows for each interface.
      *
      * @param intf the Interface on which event is received
-     * @param install true to create an add objective, false to create a remove
-     *            objective
-     **/
+     * @param install true to install the objectives, false to remove them
+     */
     private void updateInterfaceForwarding(Interface intf, boolean install) {
-        log.debug("Adding interface objectives for {}", intf);
+        log.debug("{} interface objectives for {}", operation(install), intf);
 
         DeviceId deviceId = intf.connectPoint().deviceId();
         PortNumber controlPlanePort = controlPlaneConnectPoint.port();
@@ -319,100 +339,18 @@
         }
     }
 
-    static TrafficSelector.Builder buildBaseSelectorBuilder(PortNumber inPort,
-                                                             MacAddress srcMac,
-                                                             MacAddress dstMac,
-                                                             VlanId vlanId) {
-        TrafficSelector.Builder selectorBuilder = DefaultTrafficSelector.builder();
-        if (inPort != null) {
-            selectorBuilder.matchInPort(inPort);
-        }
-        if (srcMac != null) {
-            selectorBuilder.matchEthSrc(srcMac);
-        }
-        if (dstMac != null) {
-            selectorBuilder.matchEthDst(dstMac);
-        }
-        if (vlanId != null) {
-            selectorBuilder.matchVlanId(vlanId);
-        }
-        return selectorBuilder;
-    }
-
-    static TrafficSelector buildIPDstSelector(IpPrefix dstIp,
-                                               PortNumber inPort,
-                                               MacAddress srcMac,
-                                               MacAddress dstMac,
-                                               VlanId vlanId) {
-        TrafficSelector.Builder selector = buildBaseSelectorBuilder(inPort, srcMac, dstMac, vlanId);
-        if (dstIp.isIp4()) {
-            selector.matchEthType(TYPE_IPV4);
-            selector.matchIPDst(dstIp);
-        } else {
-            selector.matchEthType(TYPE_IPV6);
-            selector.matchIPv6Dst(dstIp);
-        }
-        return selector.build();
-    }
-
-    static TrafficSelector buildIPSrcSelector(IpPrefix srcIp,
-                                               PortNumber inPort,
-                                               MacAddress srcMac,
-                                               MacAddress dstMac,
-                                               VlanId vlanId) {
-        TrafficSelector.Builder selector = buildBaseSelectorBuilder(inPort, srcMac, dstMac, vlanId);
-        if (srcIp.isIp4()) {
-            selector.matchEthType(TYPE_IPV4);
-            selector.matchIPSrc(srcIp);
-        } else {
-            selector.matchEthType(TYPE_IPV6);
-            selector.matchIPv6Src(srcIp);
-        }
-        return selector.build();
-    }
-
-    static TrafficSelector buildArpSelector(PortNumber inPort,
-                                             VlanId vlanId,
-                                             Ip4Address arpSpa,
-                                             MacAddress srcMac) {
-        TrafficSelector.Builder selector = buildBaseSelectorBuilder(inPort, null, null, vlanId);
-        selector.matchEthType(TYPE_ARP);
-        if (arpSpa != null) {
-            selector.matchArpSpa(arpSpa);
-        }
-        if (srcMac != null) {
-            selector.matchEthSrc(srcMac);
-        }
-        return selector.build();
-    }
-
-    static TrafficSelector buildNdpSelector(PortNumber inPort,
-                                             VlanId vlanId,
-                                             IpPrefix srcIp,
-                                             byte subProto,
-                                             MacAddress srcMac) {
-        TrafficSelector.Builder selector = buildBaseSelectorBuilder(inPort, null, null, vlanId);
-        selector.matchEthType(TYPE_IPV6)
-                .matchIPProtocol(PROTOCOL_ICMP6)
-                .matchIcmpv6Type(subProto);
-        if (srcIp != null) {
-            selector.matchIPv6Src(srcIp);
-        }
-        if (srcMac != null) {
-            selector.matchEthSrc(srcMac);
-        }
-        return selector.build();
-    }
-
     /**
      * Installs or removes OSPF forwarding rules.
      *
      * @param intf the interface on which event is received
      * @param install true to create an add objective, false to create a remove
      *            objective
-     **/
+     */
     private void updateOspfForwarding(Interface intf, boolean install) {
-        // FIXME IPv6 support has not been implemented yet
+        // TODO IPv6 support has not been implemented yet
+
+        log.debug("{} OSPF flows for {}", operation(install), intf);
+
         // OSPF to router
         TrafficSelector toSelector = DefaultTrafficSelector.builder()
                 .matchInPort(intf.connectPoint().port())
@@ -433,10 +371,9 @@
             cpNextId = modifyNextObjective(deviceId, controlPlanePort,
                                            intf.vlan(), false, install);
         }
-        log.debug("OSPF flows intf:{} nextid:{}", intf, cpNextId);
-        log.debug("install={}", install);
         flowObjectiveService.forward(intf.connectPoint().deviceId(),
-                buildForwardingObjective(toSelector, null, cpNextId, install ? ospfEnabled : install, ACL_PRIORITY));
+                buildForwardingObjective(toSelector, null, cpNextId,
+                        install ? ospfEnabled : install, ACL_PRIORITY));
     }
 
     /**
@@ -512,6 +449,103 @@
         return add ? fobBuilder.add() : fobBuilder.remove();
     }
 
+
+    static TrafficSelector.Builder buildBaseSelectorBuilder(PortNumber inPort,
+                                                            MacAddress srcMac,
+                                                            MacAddress dstMac,
+                                                            VlanId vlanId) {
+        TrafficSelector.Builder selectorBuilder = DefaultTrafficSelector.builder();
+        if (inPort != null) {
+            selectorBuilder.matchInPort(inPort);
+        }
+        if (srcMac != null) {
+            selectorBuilder.matchEthSrc(srcMac);
+        }
+        if (dstMac != null) {
+            selectorBuilder.matchEthDst(dstMac);
+        }
+        if (vlanId != null) {
+            selectorBuilder.matchVlanId(vlanId);
+        }
+        return selectorBuilder;
+    }
+
+    static TrafficSelector buildIPDstSelector(IpPrefix dstIp,
+                                              PortNumber inPort,
+                                              MacAddress srcMac,
+                                              MacAddress dstMac,
+                                              VlanId vlanId) {
+        TrafficSelector.Builder selector = buildBaseSelectorBuilder(inPort, srcMac, dstMac, vlanId);
+        if (dstIp.isIp4()) {
+            selector.matchEthType(TYPE_IPV4);
+            selector.matchIPDst(dstIp);
+        } else {
+            selector.matchEthType(TYPE_IPV6);
+            selector.matchIPv6Dst(dstIp);
+        }
+        return selector.build();
+    }
+
+    static TrafficSelector buildIPSrcSelector(IpPrefix srcIp,
+                                              PortNumber inPort,
+                                              MacAddress srcMac,
+                                              MacAddress dstMac,
+                                              VlanId vlanId) {
+        TrafficSelector.Builder selector = buildBaseSelectorBuilder(inPort, srcMac, dstMac, vlanId);
+        if (srcIp.isIp4()) {
+            selector.matchEthType(TYPE_IPV4);
+            selector.matchIPSrc(srcIp);
+        } else {
+            selector.matchEthType(TYPE_IPV6);
+            selector.matchIPv6Src(srcIp);
+        }
+        return selector.build();
+    }
+
+    static TrafficSelector buildArpSelector(PortNumber inPort,
+                                            VlanId vlanId,
+                                            Ip4Address arpSpa,
+                                            MacAddress srcMac) {
+        TrafficSelector.Builder selector = buildBaseSelectorBuilder(inPort, null, null, vlanId);
+        selector.matchEthType(TYPE_ARP);
+        if (arpSpa != null) {
+            selector.matchArpSpa(arpSpa);
+        }
+        if (srcMac != null) {
+            selector.matchEthSrc(srcMac);
+        }
+        return selector.build();
+    }
+
+    static TrafficSelector buildNdpSelector(PortNumber inPort,
+                                            VlanId vlanId,
+                                            IpPrefix srcIp,
+                                            byte subProto,
+                                            MacAddress srcMac) {
+        TrafficSelector.Builder selector = buildBaseSelectorBuilder(inPort, null, null, vlanId);
+        selector.matchEthType(TYPE_IPV6)
+                .matchIPProtocol(PROTOCOL_ICMP6)
+                .matchIcmpv6Type(subProto);
+        if (srcIp != null) {
+            selector.matchIPv6Src(srcIp);
+        }
+        if (srcMac != null) {
+            selector.matchEthSrc(srcMac);
+        }
+        return selector.build();
+    }
+
+    private int getPriorityFromPrefix(IpPrefix prefix) {
+        return (prefix.isIp4()) ?
+               IPV4_PRIORITY * prefix.prefixLength() + MIN_IP_PRIORITY :
+               IPV6_PRIORITY * prefix.prefixLength() + MIN_IP_PRIORITY;
+    }
+
+    private String operation(boolean install) {
+        return install ? "Installing" : "Removing";
+    }
+
+
     /**
      * Listener for network config events.
      */
@@ -523,14 +557,14 @@
                 switch (event.type()) {
                     case CONFIG_ADDED:
                     case CONFIG_UPDATED:
-                        readConfig();
+                        processRouterConfig();
                         break;
                     case CONFIG_REGISTERED:
                         break;
                     case CONFIG_UNREGISTERED:
                         break;
                     case CONFIG_REMOVED:
-                        removeConfig();
+                        removeRouterConfig();
                         break;
                 default:
                     break;
@@ -681,17 +715,5 @@
         }
     }
 
-    private int getPriorityFromPrefix(IpPrefix prefix) {
-        return (prefix.isIp4()) ?
-                IPV4_PRIORITY * prefix.prefixLength() + MIN_IP_PRIORITY :
-                IPV6_PRIORITY * prefix.prefixLength() + MIN_IP_PRIORITY;
-    }
-
-    private void removeConfig() {
-        if (interfaceManager != null) {
-            interfaceManager.cleanup();
-        }
-    }
-
 }
 
diff --git a/apps/routing/src/main/java/org/onosproject/routing/impl/SingleSwitchFibInstaller.java b/apps/routing/src/main/java/org/onosproject/routing/impl/SingleSwitchFibInstaller.java
index 4d4141b..b55dba6 100644
--- a/apps/routing/src/main/java/org/onosproject/routing/impl/SingleSwitchFibInstaller.java
+++ b/apps/routing/src/main/java/org/onosproject/routing/impl/SingleSwitchFibInstaller.java
@@ -172,7 +172,7 @@
 
         asyncDeviceFetcher = AsyncDeviceFetcher.create(deviceService);
 
-        updateConfig();
+        processRouterConfig();
 
         // FIXME: There can be an issue when this component is deactivated before vRouter.
         //        This will be addressed in CORD-710.
@@ -181,15 +181,6 @@
         log.info("Started");
     }
 
-    private RouterInterfaceManager createRouter(DeviceId deviceId, Set<String> configuredInterfaces) {
-        return new RouterInterfaceManager(deviceId,
-                configuredInterfaces,
-                interfaceService,
-                intf -> processIntfFilter(true, intf),
-                intf -> processIntfFilter(false, intf)
-        );
-    }
-
     @Deactivate
     protected void deactivate() {
          // FIXME: This will also remove flows when an instance goes down.
@@ -217,22 +208,7 @@
         log.info("routeToNextHop set to {}", routeToNextHop);
     }
 
-    //remove filtering objectives and routes before deactivate.
-    private void cleanUp() {
-        //remove the route listener
-        routeService.removeListener(routeListener);
-
-        //clean up the routes.
-        for (Map.Entry<IpPrefix, IpAddress> routes: prefixToNextHop.entrySet()) {
-            deleteRoute(new ResolvedRoute(routes.getKey(), null, null, null));
-        }
-
-        if (interfaceManager != null) {
-            interfaceManager.cleanup();
-        }
-    }
-
-    private void updateConfig() {
+    private void processRouterConfig() {
         RouterConfig routerConfig =
                 networkConfigService.getConfig(routerAppId, RoutingService.ROUTER_CONFIG_CLASS);
 
@@ -258,6 +234,31 @@
         }
     }
 
+    /*
+     * Removes filtering objectives and routes before deactivate.
+     */
+    private void cleanUp() {
+        //remove the route listener
+        routeService.removeListener(routeListener);
+
+        //clean up the routes.
+        for (Map.Entry<IpPrefix, IpAddress> routes: prefixToNextHop.entrySet()) {
+            deleteRoute(new ResolvedRoute(routes.getKey(), null, null, null));
+        }
+
+        if (interfaceManager != null) {
+            interfaceManager.cleanup();
+        }
+    }
+
+    private RouterInterfaceManager createRouter(DeviceId deviceId, Set<String> configuredInterfaces) {
+        return new RouterInterfaceManager(deviceId,
+                configuredInterfaces,
+                interfaceService,
+                this::provisionInterface,
+                this::unprovisionInterface);
+    }
+
     private void updateRoute(ResolvedRoute route) {
         addNextHop(route);
 
@@ -411,14 +412,32 @@
         return group;
     }*/
 
-    //process filtering objective for interface add/remove.
-    private void processIntfFilter(boolean install, Interface intf) {
-        createFilteringObjective(install, intf);
-        createMcastFilteringObjective(install, intf);
+    private void provisionInterface(Interface intf) {
+        updateInterfaceFilters(intf, true);
     }
 
-    //create filtering objective for interface
-    private void createFilteringObjective(boolean install, Interface intf) {
+    private void unprovisionInterface(Interface intf) {
+        updateInterfaceFilters(intf, false);
+    }
+
+    /**
+     * Installs or removes flow objectives relating to an interface.
+     *
+     * @param intf interface to update objectives for
+     * @param install true to install the objectives, false to remove them
+     */
+    private void updateInterfaceFilters(Interface intf, boolean install) {
+        updateFilteringObjective(intf, install);
+        updateMcastFilteringObjective(intf, install);
+    }
+
+    /**
+     * Installs or removes unicast filtering objectives relating to an interface.
+     *
+     * @param intf interface to update objectives for
+     * @param install true to install the objectives, false to remove them
+     */
+    private void updateFilteringObjective(Interface intf, boolean install) {
         VlanId assignedVlan = (egressVlan().equals(VlanId.NONE)) ?
                 VlanId.vlanId(ASSIGNED_VLAN) :
                 egressVlan();
@@ -444,8 +463,13 @@
         }
     }
 
-    //create filtering objective for multicast traffic
-    private void createMcastFilteringObjective(boolean install, Interface intf) {
+    /**
+     * Installs or removes multicast filtering objectives relating to an interface.
+     *
+     * @param intf interface to update objectives for
+     * @param install true to install the objectives, false to remove them
+     */
+    private void updateMcastFilteringObjective(Interface intf, boolean install) {
         VlanId assignedVlan = (egressVlan().equals(VlanId.NONE)) ?
                 VlanId.vlanId(ASSIGNED_VLAN) :
                 egressVlan();
@@ -490,6 +514,9 @@
         return (mcastConfig != null) ? mcastConfig.egressVlan() : VlanId.NONE;
     }
 
+    /**
+     * Listener for route changes.
+     */
     private class InternalRouteListener implements RouteListener {
         @Override
         public void event(RouteEvent event) {
@@ -518,7 +545,7 @@
                 switch (event.type()) {
                 case CONFIG_ADDED:
                 case CONFIG_UPDATED:
-                    updateConfig();
+                    processRouterConfig();
                     break;
                 case CONFIG_REGISTERED:
                     break;