Updated mfwd delete behavior and fixed some small bugs

Change-Id: Id354a88507e94b83fd0007c084cdbf35093bbd1a

@@ -37,9 +37,32 @@ public class McastDeleteCommand extends AbstractShellCommand {
             required = true, multiValued = false)
     String gAddr = null;
 
+    @Argument(index = 2, name = "egressList",
+            description = "Egress id/port",
+            required = false, multiValued = true)
+    String[] egressList = null;
+
+
     @Override
     protected void execute() {
+
+        boolean deleted = false;
         McastRouteTable mrib = McastRouteTable.getInstance();
+
+        if (egressList == null) {
             mrib.removeRoute(sAddr, gAddr);
+            deleted = true;
+        } else {
+            // check list for validity before we begin to delete.
+            for (String egress : egressList) {
+                deleted = mrib.removeEgress(sAddr, gAddr, egress);
+            }
+        }
+
+        if (deleted) {
+            print("Successful delete");
+        } else {
+            print("Failed to delete");
+        }
     }
 }

@@ -79,8 +79,7 @@ public class McastIntentManager {
         TrafficSelector.Builder selector = DefaultTrafficSelector.builder();
         TrafficTreatment treatment = DefaultTrafficTreatment.emptyTreatment();
 
-        if (mroute.getIngressPoint() == null ||
-                mroute.getEgressPoints().isEmpty()) {
+        if (mroute.getIngressPoint() == null) {
             return null;
         }
 
@@ -96,16 +95,22 @@ public class McastIntentManager {
                 .matchIPDst(mroute.getGaddr())
                 .matchIPSrc(mroute.getSaddr());
 
-        SinglePointToMultiPointIntent intent =
-                SinglePointToMultiPointIntent.builder()
+
+        SinglePointToMultiPointIntent.Builder builder =  SinglePointToMultiPointIntent.builder()
                         .appId(McastForwarding.getAppId())
                         .selector(selector.build())
                         .treatment(treatment)
-                        .ingressPoint(mroute.getIngressPoint().getConnectPoint())
-                        .egressPoints(mroute.getEgressConnectPoints()).
-                        build();
+                        .ingressPoint(mroute.getIngressPoint().getConnectPoint());
+
+        // allowing intent to be pushed without egress points means we can drop packets.
+        if (!mroute.getEgressPoints().isEmpty()) {
+            builder.egressPoints(mroute.getEgressConnectPoints());
+        }
 
+        SinglePointToMultiPointIntent intent = builder.build();
         intentService.submit(intent);
+        mroute.setDirty(false);
+
         return intent;
     }
 
@@ -114,9 +119,10 @@ public class McastIntentManager {
      *
      * @param mroute the mcast route whose intent we want to remove
      */
-    public void withdrawIntent(McastRouteBase mroute) {
+    public void withdrawIntent(McastRoute mroute) {
         Intent intent = intentService.getIntent(mroute.getIntentKey());
         intentService.withdraw(intent);
+        mroute.setDirty(false);
     }
 
     /**

@@ -57,6 +57,21 @@ interface McastRoute {
     public boolean isIp6();
 
     /**
+     * Get the dirty state.
+     *
+     * @return whether this route is dirty or not.
+     */
+    public boolean getDirty();
+
+    /**
+     * Set the dirty state to indicate that something changed.
+     * This may require an update to the flow tables (intents).
+     *
+     * @param dirty set the dirty bit
+     */
+    public void setDirty(boolean dirty);
+
+    /**
      * Add the ingress ConnectPoint.
      *
      * @param cpstr string representing a ConnectPoint

@@ -17,7 +17,6 @@ package org.onosproject.mfwd.impl;
 
 import static com.google.common.base.Preconditions.checkNotNull;
 
-import org.apache.commons.collections.set.ListOrderedSet;
 import org.onlab.packet.IpPrefix;
 import org.onosproject.net.ConnectPoint;
 import org.onosproject.net.intent.SinglePointToMultiPointIntent;
@@ -267,6 +266,33 @@ public class McastRouteBase implements McastRoute {
     }
 
     /**
+     * Remove an egress from McastConnectPoint.
+     *
+     * @param connectPoint the egress connect point
+     * @return boolean result of removal
+     */
+    public boolean removeEgressPoint(String connectPoint) {
+        checkNotNull(connectPoint);
+        return this.removeEgressPoint(ConnectPoint.deviceConnectPoint(connectPoint));
+    }
+
+    /**
+     * Remove an egress from McastConnectPoint.
+     *
+     * @param cp the egress connect point
+     * @return boolean result of removal
+     */
+    public boolean removeEgressPoint(ConnectPoint cp) {
+        boolean removed = false;
+        McastConnectPoint mcp = this.findEgressConnectPoint(checkNotNull(cp));
+        if (mcp != null) {
+            removed = egressPoints.remove(mcp);
+            setDirty(true);
+        }
+        return removed;
+    }
+
+    /**
      * Add an egress McastConnectPoint.
      *
      * @param cpstr deviceId/port of the connect point
@@ -292,7 +318,7 @@ public class McastRouteBase implements McastRoute {
      * @return Set of egress ConnectPoints
      */
     public Set<ConnectPoint> getEgressConnectPoints() {
-        Set<ConnectPoint> cps = new ListOrderedSet();
+        Set<ConnectPoint> cps = new HashSet<ConnectPoint>();
 
         for (McastConnectPoint mcp : egressPoints) {
             cps.add(mcp.getConnectPoint());
@@ -417,7 +443,7 @@ public class McastRouteBase implements McastRoute {
         out += "intent: ";
         out += (intentKey == null) ? "not installed" : this.intentKey.toString();
         out += "\n\tingress: ";
-        out += (ingressPoint == null) ? "NULL" : ingressPoint.toString();
+        out += (ingressPoint == null) ? "NULL" : ingressPoint.getConnectPoint().toString();
         out += "\n\tegress: {\n";
         if (egressPoints != null && !egressPoints.isEmpty()) {
             for (McastConnectPoint eg : egressPoints) {

@@ -17,6 +17,7 @@ package org.onosproject.mfwd.impl;
 
 import org.apache.felix.scr.annotations.Service;
 import org.onlab.packet.IpPrefix;
+
 import java.util.Map;
 import java.util.concurrent.ConcurrentHashMap;
 import static com.google.common.base.Preconditions.checkNotNull;
@@ -191,6 +192,30 @@ public final class McastRouteTable {
     }
 
     /**
+     * Delete a specific egress from the MRIB.
+     *
+     * @param saddr source address * or x.x.x.x or x.x.x.x/y
+     * @param gaddr group address x.x.x.x or x.x.x.x/y
+     * @param egress group address x.x.x.x or x.x.x.x/y
+     * @return boolean if egress was deleted
+     */
+    public boolean removeEgress(String saddr, String gaddr, String egress) {
+
+        IpPrefix gpfx = IpPrefix.valueOf(gaddr);
+        IpPrefix spfx = IpPrefix.valueOf(0, 0);
+        if (saddr != null && !saddr.equals("*")) {
+            spfx = IpPrefix.valueOf(saddr);
+        }
+
+        McastRouteSource src = (McastRouteSource) findBestMatch(spfx, gpfx);
+        boolean removed = src.removeEgressPoint(egress);
+        if (removed) {
+            src.setIntent();
+        }
+        return removed;
+    }
+
+    /**
      * Delete a multicast route from the MRIB.
      *
      * @param saddr source address * or x.x.x.x or x.x.x.x/y