[CORD-2838][CORD-2833] Revisit McastHandler and handle shortest paths with pair links

Includes also a refactoring of the path computation

Change-Id: Iff63780a3bb3e895e55c52211290c19d993e1905
diff --git a/apps/segmentrouting/app/BUCK b/apps/segmentrouting/app/BUCK
index 9674f76..41ae39c 100644
--- a/apps/segmentrouting/app/BUCK
+++ b/apps/segmentrouting/app/BUCK
@@ -8,6 +8,8 @@
     '//core/store/serializers:onos-core-serializers',
     '//incubator/api:onos-incubator-api',
     '//apps/route-service/api:onos-apps-route-service-api',
+    '//apps/mcast/api:onos-apps-mcast-api',
+    '//apps/mcast/cli:onos-apps-mcast-cli',
 ]
 
 TEST_DEPS = [
diff --git a/apps/segmentrouting/app/pom.xml b/apps/segmentrouting/app/pom.xml
index fbb4ecf..9d45cbb 100644
--- a/apps/segmentrouting/app/pom.xml
+++ b/apps/segmentrouting/app/pom.xml
@@ -51,14 +51,6 @@
             <artifactId>jersey-container-servlet</artifactId>
         </dependency>
         <dependency>
-            <groupId>org.osgi</groupId>
-            <artifactId>org.osgi.compendium</artifactId>
-        </dependency>
-        <dependency>
-            <groupId>org.osgi</groupId>
-            <artifactId>org.osgi.core</artifactId>
-        </dependency>
-        <dependency>
             <groupId>org.onosproject</groupId>
             <artifactId>onlab-junit</artifactId>
             <scope>test</scope>
@@ -88,5 +80,15 @@
             <classifier>tests</classifier>
             <scope>test</scope>
         </dependency>
+        <dependency>
+            <groupId>org.onosproject</groupId>
+            <artifactId>onos-app-mcast-api</artifactId>
+            <version>${project.version}</version>
+        </dependency>
+        <dependency>
+            <groupId>org.onosproject</groupId>
+            <artifactId>onos-app-mcast-cli</artifactId>
+            <version>${project.version}</version>
+        </dependency>
     </dependencies>
 </project>
diff --git a/apps/segmentrouting/app/src/main/java/org/onosproject/segmentrouting/SRLinkWeigher.java b/apps/segmentrouting/app/src/main/java/org/onosproject/segmentrouting/SRLinkWeigher.java
new file mode 100644
index 0000000..01e07a7
--- /dev/null
+++ b/apps/segmentrouting/app/src/main/java/org/onosproject/segmentrouting/SRLinkWeigher.java
@@ -0,0 +1,123 @@
+/*
+ * Copyright 2018-present Open Networking Foundation
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.onosproject.segmentrouting;
+
+import org.onlab.graph.DefaultEdgeWeigher;
+import org.onlab.graph.ScalarWeight;
+import org.onlab.graph.Weight;
+import org.onosproject.net.DeviceId;
+import org.onosproject.net.Link;
+import org.onosproject.net.PortNumber;
+import org.onosproject.net.topology.LinkWeigher;
+import org.onosproject.net.topology.TopologyEdge;
+import org.onosproject.net.topology.TopologyVertex;
+import org.onosproject.segmentrouting.config.DeviceConfigNotFoundException;
+
+import java.util.Set;
+
+/**
+ * Link weigher for multicast related path computations.
+ */
+public final class SRLinkWeigher
+        extends DefaultEdgeWeigher<TopologyVertex, TopologyEdge>
+        implements LinkWeigher {
+
+    private final SegmentRoutingManager srManager;
+    private final DeviceId srcPath;
+    private final Set<Link> linksToEnforce;
+
+    // Weight for the link to avoids. The high level idea is to build
+    // a constrained shortest path computation. 100 should provide a good
+    // threshold
+    public static final ScalarWeight LINK_TO_AVOID_WEIGHT = new ScalarWeight(HOP_WEIGHT_VALUE + 100);
+
+    /**
+     * Creates a SRLinkWeigher object.
+     *
+     * @param srManager SegmentRoutingManager object
+     * @param srcPath the source of the paths
+     * @param linksToEnforce links to be enforced by the path computation
+     */
+    public SRLinkWeigher(SegmentRoutingManager srManager, DeviceId srcPath,
+                         Set<Link> linksToEnforce) {
+        this.srManager = srManager;
+        this.srcPath = srcPath;
+        this.linksToEnforce = linksToEnforce;
+    }
+
+    @Override
+    public Weight weight(TopologyEdge edge) {
+        // 1) We need to avoid some particular paths like leaf-spine-leaf-*
+        // 2) Properly handle the pair links
+
+        // If the link is a pair link just return infinite value
+        if (isPairLink(edge.link())) {
+            return ScalarWeight.NON_VIABLE_WEIGHT;
+        }
+
+        // To avoid that the paths go through other leaves we need to influence
+        // the path computation to return infinite value for all other links having
+        // as a src a leaf different from the source we are passing to the weigher
+        DeviceId srcDeviceLink = edge.link().src().deviceId();
+        // Identify the link as leaf-spine link
+        boolean isLeafSpine;
+        try {
+            isLeafSpine = srManager.deviceConfiguration().isEdgeDevice(srcDeviceLink);
+        } catch (DeviceConfigNotFoundException e) {
+            isLeafSpine = false;
+        }
+        // If it is not the source just return infinite value
+        if (isLeafSpine && !srcDeviceLink.equals(srcPath)) {
+            return ScalarWeight.NON_VIABLE_WEIGHT;
+        }
+
+        // If the links are not in the list of the links to be enforce
+        if (!linksToEnforce.isEmpty() && !linksToEnforce.contains(edge.link())) {
+            // 100 should be a good confidence threshold
+            return LINK_TO_AVOID_WEIGHT;
+        }
+
+        // All other cases we return
+        return new ScalarWeight(HOP_WEIGHT_VALUE);
+    }
+
+    // Utility method to verify is a link is a pair-link
+    private boolean isPairLink(Link link) {
+        // Take src id, src port, dst id and dst port
+        final DeviceId srcId = link.src().deviceId();
+        final PortNumber srcPort = link.src().port();
+        final DeviceId dstId = link.dst().deviceId();
+        final PortNumber dstPort = link.dst().port();
+        // init as true
+        boolean isPairLink = true;
+        try {
+            // If one of this condition is not true; it is not a pair link
+            if (!(srManager.deviceConfiguration().isEdgeDevice(srcId) &&
+                    srManager.deviceConfiguration().isEdgeDevice(dstId) &&
+                    srManager.deviceConfiguration().getPairDeviceId(srcId).equals(dstId) &&
+                    srManager.deviceConfiguration().getPairLocalPort(srcId).equals(srcPort) &&
+                    srManager.deviceConfiguration().getPairLocalPort(dstId).equals(dstPort))) {
+                isPairLink = false;
+            }
+        } catch (DeviceConfigNotFoundException e) {
+            // Configuration not provided
+            isPairLink = false;
+        }
+        return isPairLink;
+    }
+
+}
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 b6aabf3..fe01fd3 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
@@ -44,6 +44,9 @@
 import org.onosproject.core.CoreService;
 import org.onosproject.event.Event;
 import org.onosproject.mastership.MastershipService;
+import org.onosproject.mcast.api.McastEvent;
+import org.onosproject.mcast.api.McastListener;
+import org.onosproject.mcast.api.MulticastRouteService;
 import org.onosproject.net.ConnectPoint;
 import org.onosproject.net.Device;
 import org.onosproject.net.DeviceId;
@@ -77,9 +80,6 @@
 import org.onosproject.net.link.LinkEvent;
 import org.onosproject.net.link.LinkListener;
 import org.onosproject.net.link.LinkService;
-import org.onosproject.net.mcast.McastEvent;
-import org.onosproject.net.mcast.McastListener;
-import org.onosproject.net.mcast.MulticastRouteService;
 import org.onosproject.net.neighbour.NeighbourResolutionService;
 import org.onosproject.net.packet.InboundPacket;
 import org.onosproject.net.packet.PacketContext;
@@ -1096,10 +1096,11 @@
                         routeHandler.processRouteRemoved((RouteEvent) event);
                     } else if (event.type() == RouteEvent.Type.ALTERNATIVE_ROUTES_CHANGED) {
                         routeHandler.processAlternativeRoutesChanged((RouteEvent) event);
-                    } else if (event.type() == McastEvent.Type.SOURCE_ADDED ||
-                            event.type() == McastEvent.Type.SOURCE_UPDATED ||
-                            event.type() == McastEvent.Type.SINK_ADDED ||
-                            event.type() == McastEvent.Type.SINK_REMOVED ||
+                    } else if (event.type() == McastEvent.Type.SOURCES_ADDED ||
+                            event.type() == McastEvent.Type.SOURCES_REMOVED ||
+                            event.type() == McastEvent.Type.SINKS_ADDED ||
+                            event.type() == McastEvent.Type.SINKS_REMOVED ||
+                            event.type() == McastEvent.Type.ROUTE_ADDED ||
                             event.type() == McastEvent.Type.ROUTE_REMOVED) {
                         mcastHandler.processMcastEvent((McastEvent) event);
                     } else if (event.type() == NetworkConfigEvent.Type.CONFIG_ADDED) {
@@ -1503,10 +1504,10 @@
         @Override
         public void event(McastEvent event) {
             switch (event.type()) {
-                case SOURCE_ADDED:
-                case SOURCE_UPDATED:
-                case SINK_ADDED:
-                case SINK_REMOVED:
+                case SOURCES_ADDED:
+                case SOURCES_REMOVED:
+                case SINKS_ADDED:
+                case SINKS_REMOVED:
                 case ROUTE_REMOVED:
                     log.trace("Schedule Mcast event {}", event);
                     scheduleEventHandlerIfNotScheduled(event);
diff --git a/apps/segmentrouting/app/src/main/java/org/onosproject/segmentrouting/cli/McastNextListCommand.java b/apps/segmentrouting/app/src/main/java/org/onosproject/segmentrouting/cli/McastNextListCommand.java
index 341c87d..a713130 100644
--- a/apps/segmentrouting/app/src/main/java/org/onosproject/segmentrouting/cli/McastNextListCommand.java
+++ b/apps/segmentrouting/app/src/main/java/org/onosproject/segmentrouting/cli/McastNextListCommand.java
@@ -13,13 +13,15 @@
  * See the License for the specific language governing permissions and
  * limitations under the License.
  */
+
 package org.onosproject.segmentrouting.cli;
 
 import com.google.common.collect.Maps;
-import org.apache.karaf.shell.commands.Argument;
 import org.apache.karaf.shell.commands.Command;
+import org.apache.karaf.shell.commands.Option;
 import org.onlab.packet.IpAddress;
 import org.onosproject.cli.AbstractShellCommand;
+import org.onosproject.mcast.cli.McastGroupCompleter;
 import org.onosproject.net.DeviceId;
 import org.onosproject.segmentrouting.SegmentRoutingService;
 import org.onosproject.segmentrouting.storekey.McastStoreKey;
@@ -37,19 +39,24 @@
         description = "Lists all mcast nextids")
 public class McastNextListCommand extends AbstractShellCommand {
 
+    // OSGi workaround to introduce package dependency
+    McastGroupCompleter completer;
+
     // Format for group line
     private static final String FORMAT_MAPPING = "group=%s, deviceIds-nextIds=%s";
 
-    @Argument(index = 0, name = "mcastIp", description = "mcast Ip",
+    @Option(name = "-gAddr", aliases = "--groupAddress",
+            description = "IP Address of the multicast group",
+            valueToShowInHelp = "224.0.0.0",
             required = false, multiValued = false)
-    String mcastIp;
+    String gAddr = null;
 
     @Override
     protected void execute() {
         // Verify mcast group
         IpAddress mcastGroup = null;
-        if (!isNullOrEmpty(mcastIp)) {
-            mcastGroup = IpAddress.valueOf(mcastIp);
+        if (!isNullOrEmpty(gAddr)) {
+            mcastGroup = IpAddress.valueOf(gAddr);
 
         }
         // Get SR service
diff --git a/apps/segmentrouting/app/src/main/java/org/onosproject/segmentrouting/cli/McastTreeListCommand.java b/apps/segmentrouting/app/src/main/java/org/onosproject/segmentrouting/cli/McastTreeListCommand.java
index dc86bef..d1de73c 100644
--- a/apps/segmentrouting/app/src/main/java/org/onosproject/segmentrouting/cli/McastTreeListCommand.java
+++ b/apps/segmentrouting/app/src/main/java/org/onosproject/segmentrouting/cli/McastTreeListCommand.java
@@ -18,10 +18,11 @@
 
 import com.google.common.collect.ArrayListMultimap;
 import com.google.common.collect.Multimap;
-import org.apache.karaf.shell.commands.Argument;
 import org.apache.karaf.shell.commands.Command;
+import org.apache.karaf.shell.commands.Option;
 import org.onlab.packet.IpAddress;
 import org.onosproject.cli.AbstractShellCommand;
+import org.onosproject.mcast.cli.McastGroupCompleter;
 import org.onosproject.net.ConnectPoint;
 import org.onosproject.net.DeviceId;
 import org.onosproject.segmentrouting.SegmentRoutingService;
@@ -46,21 +47,26 @@
         description = "Lists all mcast trees")
 public class McastTreeListCommand extends AbstractShellCommand {
 
+    // OSGi workaround to introduce package dependency
+    McastGroupCompleter completer;
+
     // Format for group line
     private static final String G_FORMAT_MAPPING = "group=%s, ingress=%s, transit=%s, egress=%s";
     // Format for sink line
     private static final String S_FORMAT_MAPPING = "\tsink=%s\tpath=%s";
 
-    @Argument(index = 0, name = "mcastIp", description = "mcast Ip",
+    @Option(name = "-gAddr", aliases = "--groupAddress",
+            description = "IP Address of the multicast group",
+            valueToShowInHelp = "224.0.0.0",
             required = false, multiValued = false)
-    String mcastIp;
+    String gAddr = null;
 
     @Override
     protected void execute() {
         // Verify mcast group
         IpAddress mcastGroup = null;
-        if (!isNullOrEmpty(mcastIp)) {
-            mcastGroup = IpAddress.valueOf(mcastIp);
+        if (!isNullOrEmpty(gAddr)) {
+            mcastGroup = IpAddress.valueOf(gAddr);
 
         }
         // Get SR service
diff --git a/apps/segmentrouting/app/src/main/java/org/onosproject/segmentrouting/mcast/McastHandler.java b/apps/segmentrouting/app/src/main/java/org/onosproject/segmentrouting/mcast/McastHandler.java
index a791ecb..968ad3b 100644
--- a/apps/segmentrouting/app/src/main/java/org/onosproject/segmentrouting/mcast/McastHandler.java
+++ b/apps/segmentrouting/app/src/main/java/org/onosproject/segmentrouting/mcast/McastHandler.java
@@ -33,6 +33,9 @@
 import org.onosproject.cluster.NodeId;
 import org.onosproject.core.ApplicationId;
 import org.onosproject.core.CoreService;
+import org.onosproject.mcast.api.McastEvent;
+import org.onosproject.mcast.api.McastRoute;
+import org.onosproject.mcast.api.McastRouteUpdate;
 import org.onosproject.net.config.basics.McastConfig;
 import org.onosproject.net.ConnectPoint;
 import org.onosproject.net.DeviceId;
@@ -54,11 +57,10 @@
 import org.onosproject.net.flowobjective.ForwardingObjective;
 import org.onosproject.net.flowobjective.NextObjective;
 import org.onosproject.net.flowobjective.ObjectiveContext;
-import org.onosproject.net.mcast.McastEvent;
-import org.onosproject.net.mcast.McastRoute;
-import org.onosproject.net.mcast.McastRouteInfo;
+import org.onosproject.net.topology.LinkWeigher;
 import org.onosproject.net.topology.Topology;
 import org.onosproject.net.topology.TopologyService;
+import org.onosproject.segmentrouting.SRLinkWeigher;
 import org.onosproject.segmentrouting.SegmentRoutingManager;
 import org.onosproject.segmentrouting.SegmentRoutingService;
 import org.onosproject.segmentrouting.config.DeviceConfigNotFoundException;
@@ -78,6 +80,7 @@
 import java.util.Comparator;
 import java.util.List;
 import java.util.Map;
+import java.util.Map.Entry;
 import java.util.Optional;
 import java.util.Set;
 import java.util.concurrent.ScheduledExecutorService;
@@ -88,10 +91,9 @@
 
 import static java.util.concurrent.Executors.newScheduledThreadPool;
 import static org.onlab.util.Tools.groupedThreads;
+
+import static org.onosproject.mcast.api.McastEvent.Type.*;
 import static org.onosproject.net.flow.criteria.Criterion.Type.VLAN_VID;
-import static org.onosproject.net.mcast.McastEvent.Type.ROUTE_REMOVED;
-import static org.onosproject.net.mcast.McastEvent.Type.SOURCE_ADDED;
-import static org.onosproject.net.mcast.McastEvent.Type.SOURCE_UPDATED;
 import static org.onosproject.segmentrouting.SegmentRoutingManager.INTERNAL_VLAN;
 import static org.onosproject.segmentrouting.mcast.McastRole.EGRESS;
 import static org.onosproject.segmentrouting.mcast.McastRole.INGRESS;
@@ -139,63 +141,116 @@
 
     private void enqueueMcastEvent(McastEvent mcastEvent) {
         log.debug("Enqueue mcastEvent {}", mcastEvent);
-        final McastRouteInfo mcastRouteInfo = mcastEvent.subject();
+        final McastRouteUpdate mcastRouteUpdate = mcastEvent.subject();
+        final McastRouteUpdate prevUpdate = mcastEvent.prevSubject();
+        final IpAddress group = prevUpdate.route().group();
         // Let's create the keys of the cache
         ImmutableSet.Builder<ConnectPoint> sinksBuilder = ImmutableSet.builder();
+        Set<ConnectPoint> sinks;
         // For this event we will have a set of sinks
-        if (mcastEvent.type() == SOURCE_ADDED ||
-                mcastEvent.type() == SOURCE_UPDATED ||
-                mcastEvent.type() == ROUTE_REMOVED) {
-            // Add all the sinks
-            sinksBuilder.addAll(mcastRouteInfo.sinks());
+        if (mcastEvent.type() == SOURCES_ADDED ||
+                mcastEvent.type() == SOURCES_REMOVED) {
+            // FIXME To be addressed with multiple sources support
+            sinks = mcastRouteUpdate.sinks()
+                    .values()
+                    .stream()
+                    .flatMap(Collection::stream)
+                    .collect(Collectors.toSet());
         } else {
-            // We have just one sink in this case
-            ConnectPoint sink = mcastRouteInfo.sink().orElse(null);
-            // It is always true, unless something of bad happened
-            // in the mcast route store
-            if (sink != null) {
-                sinksBuilder.add(sink);
+            Set<ConnectPoint> prevSinks = prevUpdate.sinks()
+                    .values()
+                    .stream()
+                    .flatMap(Collection::stream)
+                    .collect(Collectors.toSet());
+            if (mcastEvent.type() == ROUTE_REMOVED) {
+                // Get the old sinks, since current subject is null
+                sinks = prevSinks;
+            } else {
+                // Get new sinks
+                Set<ConnectPoint> newsinks = mcastRouteUpdate.sinks()
+                        .values()
+                        .stream()
+                        .flatMap(Collection::stream)
+                        .collect(Collectors.toSet());
+                // If it is a SINKS_ADDED event
+                if (mcastEvent.type() == SINKS_ADDED) {
+                    // Let's do the difference between current and prev subjects
+                    sinks = Sets.difference(newsinks, prevSinks);
+                } else {
+                    // Let's do the difference between prev and current subjects
+                    sinks = Sets.difference(prevSinks, newsinks);
+                }
             }
         }
+        // Add all the sinks
+        sinksBuilder.addAll(sinks);
         // Push the elements in the cache
         sinksBuilder.build().forEach(sink -> {
-            McastCacheKey cacheKey = new McastCacheKey(mcastRouteInfo.route().group(),
-                                                       sink);
+            McastCacheKey cacheKey = new McastCacheKey(group, sink);
             mcastEventCache.put(cacheKey, mcastEvent);
         });
     }
 
     private void dequeueMcastEvent(McastEvent mcastEvent) {
         log.debug("Dequeue mcastEvent {}", mcastEvent);
-        final McastRouteInfo mcastRouteInfo = mcastEvent.subject();
+        final McastRouteUpdate mcastRouteUpdate = mcastEvent.subject();
+        final McastRouteUpdate prevUpdate = mcastEvent.prevSubject();
         // Get source, mcast group
-        ConnectPoint source = mcastRouteInfo.source().orElse(null);
-        IpAddress mcastIp = mcastRouteInfo.route().group();
+        // FIXME To be addressed with multiple sources support
+        ConnectPoint prevSource = prevUpdate.sources()
+                .stream()
+                .findFirst()
+                .orElse(null);
+        IpAddress mcastIp = prevUpdate.route().group();
+        Set<ConnectPoint> prevSinks = prevUpdate.sinks()
+                .values()
+                .stream()
+                .flatMap(Collection::stream)
+                .collect(Collectors.toSet());
+        Set<ConnectPoint> newSinks;
+        // Sinks to handled by SINKS_ADDED and SINKS_REMOVED procedures
+        Set<ConnectPoint> sinks;
         // According to the event type let's call the proper method
         switch (mcastEvent.type()) {
-            case SOURCE_ADDED:
-                // Get all the sinks and process
-                Set<ConnectPoint> sinks = mcastRouteInfo.sinks();
-                sinks.forEach(sink -> processSinkAddedInternal(source, sink, mcastIp));
+            case SOURCES_ADDED:
+                // FIXME To be addressed with multiple sources support
+                // Get all the sinks
+                //Set<ConnectPoint> sinks = mcastRouteInfo.sinks();
+                // Compute the Mcast tree
+                //Map<ConnectPoint, List<Path>> mcasTree = computeSinkMcastTree(source.deviceId(), sinks);
+                // Process the given sinks using the pre-computed paths
+                //mcasTree.forEach((sink, paths) -> processSinkAddedInternal(source, sink, mcastIp, paths));
                 break;
-            case SOURCE_UPDATED:
+            case SOURCES_REMOVED:
+                // FIXME To be addressed with multiple sources support
                 // Get old source
-                ConnectPoint oldSource = mcastEvent.prevSubject().source().orElse(null);
+                //ConnectPoint oldSource = mcastEvent.prevSubject().source().orElse(null);
                 // Just the first cached element will be processed
-                processSourceUpdatedInternal(mcastIp, source, oldSource);
+                //processSourceUpdatedInternal(mcastIp, source, oldSource);
                 break;
             case ROUTE_REMOVED:
                 // Process the route removed, just the first cached element will be processed
-                processRouteRemovedInternal(source, mcastIp);
+                processRouteRemovedInternal(prevSource, mcastIp);
                 break;
-            case SINK_ADDED:
-                // Get the only sink and process
-                ConnectPoint sink = mcastRouteInfo.sink().orElse(null);
-                processSinkAddedInternal(source, sink, mcastIp);
+            case SINKS_ADDED:
+                // Get the only sinks to be processed (new ones)
+                newSinks = mcastRouteUpdate.sinks()
+                        .values()
+                        .stream()
+                        .flatMap(Collection::stream)
+                        .collect(Collectors.toSet());
+                sinks = Sets.difference(newSinks, prevSinks);
+                sinks.forEach(sink -> processSinkAddedInternal(prevSource, sink, mcastIp, null));
                 break;
-            case SINK_REMOVED:
-                sink = mcastRouteInfo.sink().orElse(null);
-                processSinkRemovedInternal(source, sink, mcastIp);
+            case SINKS_REMOVED:
+                // Get the only sinks to be processed (old ones)
+                newSinks = mcastRouteUpdate.sinks()
+                        .values()
+                        .stream()
+                        .flatMap(Collection::stream)
+                        .collect(Collectors.toSet());
+                sinks = Sets.difference(prevSinks, newSinks);
+                sinks.forEach(sink -> processSinkRemovedInternal(prevSource, sink, mcastIp));
                 break;
             default:
                 break;
@@ -283,11 +338,26 @@
      */
     public void init() {
         srManager.multicastRouteService.getRoutes().forEach(mcastRoute -> {
-            ConnectPoint source = srManager.multicastRouteService.fetchSource(mcastRoute);
-            Set<ConnectPoint> sinks = srManager.multicastRouteService.fetchSinks(mcastRoute);
-            sinks.forEach(sink -> {
-                processSinkAddedInternal(source, sink, mcastRoute.group());
-            });
+            // FIXME To be addressed with multiple sources support
+            ConnectPoint source = srManager.multicastRouteService.sources(mcastRoute)
+                    .stream()
+                    .findFirst()
+                    .orElse(null);
+            Set<ConnectPoint> sinks = srManager.multicastRouteService.sinks(mcastRoute);
+            // Filter out all the working sinks, we do not want to move them
+            sinks = sinks.stream()
+                    .filter(sink -> {
+                        McastStoreKey mcastKey = new McastStoreKey(mcastRoute.group(), sink.deviceId());
+                        Versioned<NextObjective> verMcastNext = mcastNextObjStore.get(mcastKey);
+                        return verMcastNext == null ||
+                                !getPorts(verMcastNext.value().next()).contains(sink.port());
+                    })
+                    .collect(Collectors.toSet());
+            // Compute the Mcast tree
+            Map<ConnectPoint, List<Path>> mcasTree = computeSinkMcastTree(source.deviceId(), sinks);
+            // Process the given sinks using the pre-computed paths
+            mcasTree.forEach((sink, paths) -> processSinkAddedInternal(source, sink,
+                                                                       mcastRoute.group(), paths));
         });
     }
 
@@ -306,12 +376,6 @@
      */
     public void processMcastEvent(McastEvent event) {
         log.info("process {}", event);
-        // Verify if it is a complete event
-        McastRouteInfo mcastRouteInfo = event.subject();
-        if (!mcastRouteInfo.isComplete()) {
-            log.info("Incompleted McastRouteInfo. Abort {}", event.type());
-            return;
-        }
         // Just enqueue for now
         enqueueMcastEvent(event);
     }
@@ -451,7 +515,8 @@
             }
 
             // If this is the last sink on the device, also update upstream
-            Optional<Path> mcastPath = getPath(source.deviceId(), sink.deviceId(), mcastIp);
+            Optional<Path> mcastPath = getPath(source.deviceId(), sink.deviceId(),
+                                               mcastIp, null);
             if (mcastPath.isPresent()) {
                 List<Link> links = Lists.newArrayList(mcastPath.get().links());
                 Collections.reverse(links);
@@ -482,7 +547,7 @@
      * @param mcastIp multicast group IP address
      */
     private void processSinkAddedInternal(ConnectPoint source, ConnectPoint sink,
-            IpAddress mcastIp) {
+            IpAddress mcastIp, List<Path> allPaths) {
         lastMcastChange = Instant.now();
         mcastLock();
         try {
@@ -511,7 +576,8 @@
             }
 
             // Find a path. If present, create/update groups and flows for each hop
-            Optional<Path> mcastPath = getPath(source.deviceId(), sink.deviceId(), mcastIp);
+            Optional<Path> mcastPath = getPath(source.deviceId(), sink.deviceId(),
+                                               mcastIp, allPaths);
             if (mcastPath.isPresent()) {
                 List<Link> links = mcastPath.get().links();
 
@@ -581,7 +647,7 @@
                 // Continue only when this instance is the master of source device
                 if (!srManager.mastershipService.isLocalMaster(source.deviceId())) {
                     log.debug("Skip {} due to lack of mastership of the source device {}",
-                             source.deviceId());
+                             mcastIp, source.deviceId());
                     return;
                 }
 
@@ -592,9 +658,15 @@
                 // Remove transit-facing ports on the ingress device
                 removeIngressTransitPorts(mcastIp, ingressDevice, source);
 
+                // Compute mcast tree for the the egress devices
+                Map<DeviceId, List<Path>> mcastTree = computeMcastTree(ingressDevice, egressDevices);
+
                 // Construct a new path for each egress device
-                egressDevices.forEach(egressDevice -> {
-                    Optional<Path> mcastPath = getPath(ingressDevice, egressDevice, mcastIp);
+                mcastTree.forEach((egressDevice, paths) -> {
+                    // We try to enforce the sinks path on the mcast tree
+                    Optional<Path> mcastPath = getPath(ingressDevice, egressDevice,
+                                                       mcastIp, paths);
+                    // If a path is present, let's install it
                     if (mcastPath.isPresent()) {
                         installPath(mcastIp, source, mcastPath.get());
                     } else {
@@ -680,9 +752,14 @@
                             return;
                         }
                     }
+
+                    // Compute mcast tree for the the egress devices
+                    Map<DeviceId, List<Path>> mcastTree = computeMcastTree(ingressDevice, egressDevices);
+
                     // Construct a new path for each egress device
-                    egressDevices.forEach(egressDevice -> {
-                        Optional<Path> mcastPath = getPath(ingressDevice, egressDevice, mcastIp);
+                    mcastTree.forEach((egressDevice, paths) -> {
+                        Optional<Path> mcastPath = getPath(ingressDevice, egressDevice,
+                                                           mcastIp, null);
                         // If there is a new path
                         if (mcastPath.isPresent()) {
                             // Let's install the new mcast path for this egress
@@ -1072,31 +1149,157 @@
         return builder.build();
     }
 
-    // Utility method to verify is a link is a pair-link
-    private boolean isPairLink(Link link) {
-        // Take src id, src port, dst id and dst port
-        final DeviceId srcId = link.src().deviceId();
-        final PortNumber srcPort = link.src().port();
-        final DeviceId dstId = link.dst().deviceId();
-        final PortNumber dstPort = link.dst().port();
-        // init as true
-        boolean isPairLink = true;
-        try {
-            // If one of this condition is not true; it is not a pair link
-            if (!(srManager.deviceConfiguration().isEdgeDevice(srcId) &&
-                  srManager.deviceConfiguration().isEdgeDevice(dstId) &&
-                  srManager.deviceConfiguration().getPairDeviceId(srcId).equals(dstId) &&
-                  srManager.deviceConfiguration().getPairLocalPort(srcId).equals(srcPort) &&
-                  srManager.deviceConfiguration().getPairLocalPort(dstId).equals(dstPort))) {
-                    isPairLink = false;
-                }
-        } catch (DeviceConfigNotFoundException e) {
-            // Configuration not provided
-            log.warn("Could not check if the link {} is pairlink "
-                             + "config not yet provided", link);
-            isPairLink = false;
+    /**
+     * Go through all the paths, looking for shared links to be used
+     * in the final path computation.
+     *
+     * @param egresses egress devices
+     * @param availablePaths all the available paths towards the egress
+     * @return shared links between egress devices
+     */
+    private Set<Link> exploreMcastTree(Set<DeviceId> egresses,
+                                       Map<DeviceId, List<Path>> availablePaths) {
+        // Length of the shortest path
+        int minLength = Integer.MAX_VALUE;
+        int length;
+        // Current paths
+        List<Path> currentPaths;
+        // Verify the source can still reach all the egresses
+        for (DeviceId egress : egresses) {
+            // From the source we cannot reach all the sinks
+            // just continue and let's figured out after
+            currentPaths = availablePaths.get(egress);
+            if (currentPaths.isEmpty()) {
+                continue;
+            }
+            // Get the length of the first one available,
+            // update the min length and save the paths
+            length = currentPaths.get(0).links().size();
+            if (length < minLength) {
+                minLength = length;
+            }
         }
-        return isPairLink;
+        // If there are no paths
+        if (minLength == Integer.MAX_VALUE) {
+            return Collections.emptySet();
+        }
+        // Iterate looking for shared links
+        int index = 0;
+        // Define the sets for the intersection
+        Set<Link> sharedLinks = Sets.newHashSet();
+        Set<Link> currentSharedLinks;
+        Set<Link> currentLinks;
+        DeviceId deviceToRemove = null;
+        // Let's find out the shared links
+        while (index < minLength) {
+            // Initialize the intersection with the paths related to the first egress
+            currentPaths = availablePaths.get(
+                    egresses.stream()
+                            .findFirst()
+                            .orElse(null)
+            );
+            currentSharedLinks = Sets.newHashSet();
+            // Iterate over the paths and take the "index" links
+            for (Path path : currentPaths) {
+                currentSharedLinks.add(path.links().get(index));
+            }
+            // Iterate over the remaining egress
+            for (DeviceId egress : egresses) {
+                // Iterate over the paths and take the "index" links
+                currentLinks = Sets.newHashSet();
+                for (Path path : availablePaths.get(egress)) {
+                    currentLinks.add(path.links().get(index));
+                }
+                // Do intersection
+                currentSharedLinks = Sets.intersection(currentSharedLinks, currentLinks);
+                // If there are no shared paths exit and record the device to remove
+                // we have to retry with a subset of sinks
+                if (currentSharedLinks.isEmpty()) {
+                    deviceToRemove = egress;
+                    index = minLength;
+                    break;
+                }
+            }
+            sharedLinks.addAll(currentSharedLinks);
+            index++;
+        }
+        // If the shared links is empty and there are egress
+        // let's retry another time with less sinks, we can
+        // still build optimal subtrees
+        if (sharedLinks.isEmpty() && egresses.size() > 1 && deviceToRemove != null) {
+            egresses.remove(deviceToRemove);
+            sharedLinks = exploreMcastTree(egresses, availablePaths);
+        }
+        return sharedLinks;
+    }
+
+    /**
+     * Build Mcast tree having as root the given source and as leaves the given egress points.
+     *
+     * @param source source of the tree
+     * @param sinks leaves of the tree
+     * @return the computed Mcast tree
+     */
+    private Map<ConnectPoint, List<Path>> computeSinkMcastTree(DeviceId source,
+                                                                Set<ConnectPoint> sinks) {
+        // Get the egress devices, remove source from the egress if present
+        Set<DeviceId> egresses = sinks.stream()
+                .map(ConnectPoint::deviceId)
+                .filter(deviceId -> !deviceId.equals(source))
+                .collect(Collectors.toSet());
+        Map<DeviceId, List<Path>> mcastTree = computeMcastTree(source, egresses);
+        // Build final tree nad return it as it is
+        final Map<ConnectPoint, List<Path>> finalTree = Maps.newHashMap();
+        mcastTree.forEach((egress, paths) ->
+            sinks.stream().filter(sink -> sink.deviceId().equals(egress))
+                    .forEach(sink -> finalTree.put(sink, mcastTree.get(sink.deviceId()))));
+        return finalTree;
+    }
+
+    /**
+     * Build Mcast tree having as root the given source and as leaves the given egress.
+     *
+     * @param source source of the tree
+     * @param egresses leaves of the tree
+     * @return the computed Mcast tree
+     */
+    private Map<DeviceId, List<Path>> computeMcastTree(DeviceId source,
+                                                       Set<DeviceId> egresses) {
+        // Pre-compute all the paths
+        Map<DeviceId, List<Path>> availablePaths = Maps.newHashMap();
+        // No links to enforce
+        egresses.forEach(egress -> availablePaths.put(egress, getPaths(source, egress,
+                                                                       Collections.emptySet())));
+        // Explore the topology looking for shared links amongst the egresses
+        Set<Link> linksToEnforce = exploreMcastTree(Sets.newHashSet(egresses), availablePaths);
+        // Remove all the paths from the previous computation
+        availablePaths.clear();
+        // Build the final paths enforcing the shared links between egress devices
+        egresses.forEach(egress -> availablePaths.put(egress, getPaths(source, egress,
+                                                                       linksToEnforce)));
+        return availablePaths;
+    }
+
+    /**
+     * Gets path from src to dst computed using the custom link weigher.
+     *
+     * @param src source device ID
+     * @param dst destination device ID
+     * @return list of paths from src to dst
+     */
+    private List<Path> getPaths(DeviceId src, DeviceId dst, Set<Link> linksToEnforce) {
+        // Takes a snapshot of the topology
+        final Topology currentTopology = topologyService.currentTopology();
+        // Build a specific link weigher for this path computation
+        final LinkWeigher linkWeigher = new SRLinkWeigher(srManager, src, linksToEnforce);
+        // We will use our custom link weigher for our path
+        // computations and build the list of valid paths
+        List<Path> allPaths = Lists.newArrayList(
+                topologyService.getPaths(currentTopology, src, dst, linkWeigher)
+        );
+        // If there are no valid paths, just exit
+        log.debug("{} path(s) found from {} to {}", allPaths.size(), src, dst);
+        return allPaths;
     }
 
     /**
@@ -1107,18 +1310,17 @@
      * @param src source device ID
      * @param dst destination device ID
      * @param mcastIp multicast group
+     * @param allPaths paths list
      * @return an optional path from src to dst
      */
-    private Optional<Path> getPath(DeviceId src, DeviceId dst, IpAddress mcastIp) {
-        // Takes a snapshot of the topology
-        final Topology currentTopology = topologyService.currentTopology();
-        List<Path> allPaths = Lists.newArrayList(
-                topologyService.getPaths(currentTopology, src, dst)
-        );
-        // Create list of valid paths
-        allPaths.removeIf(path -> path.links().stream().anyMatch(this::isPairLink));
-        // If there are no valid paths, just exit
-        log.debug("{} path(s) found from {} to {}", allPaths.size(), src, dst);
+    private Optional<Path> getPath(DeviceId src, DeviceId dst,
+                                   IpAddress mcastIp, List<Path> allPaths) {
+        // Firstly we get all the valid paths, if the supplied are null
+        if (allPaths == null) {
+            allPaths = getPaths(src, dst, Collections.emptySet());
+        }
+
+        // If there are no paths just exit
         if (allPaths.isEmpty()) {
             return Optional.empty();
         }
@@ -1205,7 +1407,7 @@
         return mcastRoleStore.entrySet().stream()
                 .filter(entry -> entry.getKey().mcastIp().equals(mcastIp) &&
                         entry.getValue().value() == role)
-                .map(Map.Entry::getKey).map(McastStoreKey::deviceId)
+                .map(Entry::getKey).map(McastStoreKey::deviceId)
                 .collect(Collectors.toSet());
     }
 
@@ -1215,11 +1417,15 @@
      * @param mcastIp multicast IP
      * @return source connect point or null if not found
      */
+    // FIXME To be addressed with multiple sources support
     private ConnectPoint getSource(IpAddress mcastIp) {
-        return srManager.multicastRouteService.getRoutes().stream()
-                .filter(mcastRoute -> mcastRoute.group().equals(mcastIp))
-                .map(mcastRoute -> srManager.multicastRouteService.fetchSource(mcastRoute))
-                .findAny().orElse(null);
+        // FIXME we should support different types of routes
+        McastRoute mcastRoute = srManager.multicastRouteService.getRoutes().stream()
+                .filter(mcastRouteInternal -> mcastRouteInternal.group().equals(mcastIp))
+                .findFirst().orElse(null);
+        return mcastRoute == null ? null : srManager.multicastRouteService.sources(mcastRoute)
+                .stream()
+                .findFirst().orElse(null);
     }
     /**
      * Gets sinks of given multicast group.
@@ -1228,10 +1434,12 @@
      * @return set of sinks or empty set if not found
      */
     private Set<ConnectPoint> getSinks(IpAddress mcastIp) {
-        return srManager.multicastRouteService.getRoutes().stream()
-                .filter(mcastRoute -> mcastRoute.group().equals(mcastIp))
-                .map(mcastRoute -> srManager.multicastRouteService.fetchSinks(mcastRoute))
-                .findAny().orElse(Collections.emptySet());
+        // FIXME we should support different types of routes
+        McastRoute mcastRoute = srManager.multicastRouteService.getRoutes().stream()
+                .filter(mcastRouteInternal -> mcastRouteInternal.group().equals(mcastIp))
+                .findFirst().orElse(null);
+        return mcastRoute == null ?
+                Collections.emptySet() : srManager.multicastRouteService.sinks(mcastRoute);
     }
 
     /**
@@ -1246,7 +1454,7 @@
         return mcastNextObjStore.entrySet().stream()
                 .filter(entry -> entry.getKey().deviceId().equals(deviceId) &&
                         getPorts(entry.getValue().value().next()).contains(port))
-                .map(Map.Entry::getKey).map(McastStoreKey::mcastIp)
+                .map(Entry::getKey).map(McastStoreKey::mcastIp)
                 .collect(Collectors.toSet());
     }
 
@@ -1259,7 +1467,7 @@
     private Set<IpAddress> getAffectedGroups(DeviceId deviceId) {
         return mcastNextObjStore.entrySet().stream()
                 .filter(entry -> entry.getKey().deviceId().equals(deviceId))
-                .map(Map.Entry::getKey).map(McastStoreKey::mcastIp)
+                .map(Entry::getKey).map(McastStoreKey::mcastIp)
                 .collect(Collectors.toSet());
     }
 
@@ -1433,7 +1641,10 @@
             // Iterates over the route and updates properly the filtering objective
             // on the source device.
             srManager.multicastRouteService.getRoutes().forEach(mcastRoute -> {
-                ConnectPoint source = srManager.multicastRouteService.fetchSource(mcastRoute);
+                // FIXME To be addressed with multiple sources support
+                ConnectPoint source = srManager.multicastRouteService.sources(mcastRoute)
+                        .stream()
+                        .findFirst().orElse(null);
                 if (source.deviceId().equals(deviceId) && source.port().equals(portNum)) {
                     if (install) {
                         addFilterToDevice(deviceId, portNum, vlanId, mcastRoute.group(), INGRESS);
@@ -1570,12 +1781,12 @@
         if (mcastIp != null) {
             return mcastNextObjStore.entrySet().stream()
                     .filter(mcastEntry -> mcastIp.equals(mcastEntry.getKey().mcastIp()))
-                    .collect(Collectors.toMap(Map.Entry::getKey,
+                    .collect(Collectors.toMap(Entry::getKey,
                                               entry -> entry.getValue().value().id()));
         }
         // Otherwise take all the groups
         return mcastNextObjStore.entrySet().stream()
-                .collect(Collectors.toMap(Map.Entry::getKey,
+                .collect(Collectors.toMap(Entry::getKey,
                                           entry -> entry.getValue().value().id()));
     }
 
@@ -1584,12 +1795,12 @@
         if (mcastIp != null) {
             return mcastRoleStore.entrySet().stream()
                     .filter(mcastEntry -> mcastIp.equals(mcastEntry.getKey().mcastIp()))
-                    .collect(Collectors.toMap(Map.Entry::getKey,
+                    .collect(Collectors.toMap(Entry::getKey,
                                               entry -> entry.getValue().value()));
         }
         // Otherwise take all the groups
         return mcastRoleStore.entrySet().stream()
-                .collect(Collectors.toMap(Map.Entry::getKey,
+                .collect(Collectors.toMap(Entry::getKey,
                                           entry -> entry.getValue().value()));
     }
 
diff --git a/apps/segmentrouting/app/src/main/resources/OSGI-INF/blueprint/shell-config.xml b/apps/segmentrouting/app/src/main/resources/OSGI-INF/blueprint/shell-config.xml
index cfd7502..647eb49 100644
--- a/apps/segmentrouting/app/src/main/resources/OSGI-INF/blueprint/shell-config.xml
+++ b/apps/segmentrouting/app/src/main/resources/OSGI-INF/blueprint/shell-config.xml
@@ -71,24 +71,22 @@
         </command>
         <command>
             <action class="org.onosproject.segmentrouting.cli.McastNextListCommand"/>
-            <completers>
-                <ref component-id="mcastGroupCompleter"/>
-                <ref component-id="nullCompleter"/>
-            </completers>
+            <optional-completers>
+                <entry key="-gAddr" value-ref="mcastGroupCompleter"/>
+            </optional-completers>
         </command>
         <command>
             <action class="org.onosproject.segmentrouting.cli.McastTreeListCommand"/>
-            <completers>
-                <ref component-id="mcastGroupCompleter"/>
-                <ref component-id="nullCompleter"/>
-            </completers>
+            <optional-completers>
+                <entry key="-gAddr" value-ref="mcastGroupCompleter"/>
+            </optional-completers>
         </command>
     </command-bundle>
 
     <bean id="nullCompleter" class="org.apache.karaf.shell.console.completer.NullCompleter"/>
     <bean id="deviceIdCompleter" class="org.onosproject.cli.net.DeviceIdCompleter"/>
     <bean id="pseudowireIdCompleter" class="org.onosproject.segmentrouting.cli.PseudowireIdCompleter"/>
-    <bean id="mcastGroupCompleter" class="org.onosproject.cli.net.McastGroupCompleter"/>
+    <bean id="mcastGroupCompleter" class="org.onosproject.mcast.cli.McastGroupCompleter"/>
 
 </blueprint>