From d1be7b1317ab89fd47e90d1ce4cc526eff8500dd Mon Sep 17 00:00:00 2001
From: Pier Luigi <pierventre@hotmail.com>
Date: Fri, 19 Jan 2018 10:24:53 +0100
Subject: [PATCH] [CORD-2483] Deleting multiple sinks does not clean all flows
 and groups

Rationale: Let's assume we have a working group with
multiple sinks. We try to delete the sinks all together,
also in this case for each sink a sink removed event is
generated. For each event we create a new next objective
removing the ports no more needed.

Solution is related to the previous patch (group editing).
Failure scenario:

Sink1 removed -> fwdObj A -> Next B

McastHandler store has been updated at this point.

Sink2 removed -> remove fwdObj A

The remove operations is executed immediately but
the Next B could not exist at this point -> flow
installation failed

Change-Id: Icf568b26b2f3ae3feb935b13038db6195125a5c4
---
 .../segmentrouting/McastHandler.java          | 19 ++++++++++++-------
 1 file changed, 12 insertions(+), 7 deletions(-)

diff --git a/apps/segmentrouting/src/main/java/org/onosproject/segmentrouting/McastHandler.java b/apps/segmentrouting/src/main/java/org/onosproject/segmentrouting/McastHandler.java
index e77ba97cc9..ce2c2dabbc 100644
--- a/apps/segmentrouting/src/main/java/org/onosproject/segmentrouting/McastHandler.java
+++ b/apps/segmentrouting/src/main/java/org/onosproject/segmentrouting/McastHandler.java
@@ -581,13 +581,14 @@ public class McastHandler {
         existingPorts.remove(port);
 
         NextObjective newNextObj;
+        ObjectiveContext context;
         ForwardingObjective fwdObj;
         if (existingPorts.isEmpty()) {
-            // If this is the last sink, remove flows and groups
+            // If this is the last sink, remove flows and last bucket
             // NOTE: Rely on GroupStore garbage collection rather than explicitly
             //       remove L3MG since there might be other flows/groups refer to
             //       the same L2IG
-            ObjectiveContext context = new DefaultObjectiveContext(
+            context = new DefaultObjectiveContext(
                     (objective) -> log.debug("Successfully remove {} on {}/{}, vlan {}",
                             mcastIp, deviceId, port.toLong(), assignedVlan),
                     (objective, error) ->
@@ -595,21 +596,25 @@ public class McastHandler {
                                     mcastIp, deviceId, port.toLong(), assignedVlan, error));
             fwdObj = fwdObjBuilder(mcastIp, assignedVlan, nextObj.id()).remove(context);
             mcastNextObjStore.remove(mcastStoreKey);
-            srManager.flowObjectiveService.forward(deviceId, fwdObj);
         } else {
             // If this is not the last sink, update flows and groups
-            ObjectiveContext context = new DefaultObjectiveContext(
+            context = new DefaultObjectiveContext(
                     (objective) -> log.debug("Successfully update {} on {}/{}, vlan {}",
                             mcastIp, deviceId, port.toLong(), assignedVlan),
                     (objective, error) ->
                             log.warn("Failed to update {} on {}/{}, vlan {}: {}",
                                     mcastIp, deviceId, port.toLong(), assignedVlan, error));
-            newNextObj = nextObjBuilder(mcastIp, assignedVlan, existingPorts, null).add();
+            // Here we store the next objective with the remaining port
+            newNextObj = nextObjBuilder(mcastIp, assignedVlan,
+                                        existingPorts, nextObj.id()).removeFromExisting();
             fwdObj = fwdObjBuilder(mcastIp, assignedVlan, newNextObj.id()).add(context);
             mcastNextObjStore.put(mcastStoreKey, newNextObj);
-            srManager.flowObjectiveService.next(deviceId, newNextObj);
-            srManager.flowObjectiveService.forward(deviceId, fwdObj);
         }
+        // Let's modify the next objective removing the bucket
+        newNextObj = nextObjBuilder(mcastIp, assignedVlan,
+                                    ImmutableSet.of(port), nextObj.id()).removeFromExisting();
+        srManager.flowObjectiveService.next(deviceId, newNextObj);
+        srManager.flowObjectiveService.forward(deviceId, fwdObj);
         return existingPorts.isEmpty();
     }