[CORD-2484] Deleting multicast route does not clean flows and groups
Change-Id: I6b669b6c202430af070a8977c23d63e2d072bf51
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 e917b30..a5ef34c 100644
--- a/apps/segmentrouting/src/main/java/org/onosproject/segmentrouting/McastHandler.java
+++ b/apps/segmentrouting/src/main/java/org/onosproject/segmentrouting/McastHandler.java
@@ -264,6 +264,69 @@
}
/**
+ * Processes the ROUTE_REMOVED event.
+ *
+ * @param event McastEvent with ROUTE_REMOVED type
+ */
+ protected void processRouteRemoved(McastEvent event) {
+ log.info("processRouteRemoved {}", event);
+ McastRouteInfo mcastRouteInfo = event.subject();
+ if (!mcastRouteInfo.source().isPresent()) {
+ log.info("Incompleted McastRouteInfo. Abort.");
+ return;
+ }
+ // Get group ip and ingress connect point
+ IpAddress mcastIp = mcastRouteInfo.route().group();
+ ConnectPoint source = mcastRouteInfo.source().get();
+
+ processRouteRemovedInternal(source, mcastIp);
+ }
+
+ /**
+ * Removes the entire mcast tree related to this group.
+ *
+ * @param mcastIp multicast group IP address
+ */
+ private void processRouteRemovedInternal(ConnectPoint source, IpAddress mcastIp) {
+ lastMcastChange = Instant.now();
+ mcastLock();
+ try {
+ log.debug("Processing route down for group {}", mcastIp);
+
+ // Find out the ingress, transit and egress device of the affected group
+ DeviceId ingressDevice = getDevice(mcastIp, McastRole.INGRESS)
+ .stream().findAny().orElse(null);
+ DeviceId transitDevice = getDevice(mcastIp, McastRole.TRANSIT)
+ .stream().findAny().orElse(null);
+ Set<DeviceId> egressDevices = getDevice(mcastIp, McastRole.EGRESS);
+
+ // Verify leadership on the operation
+ if (!isLeader(source)) {
+ log.debug("Skip {} due to lack of leadership", mcastIp);
+ return;
+ }
+
+ // If there are egress devices, sinks could be only on the ingress
+ if (!egressDevices.isEmpty()) {
+ egressDevices.forEach(
+ deviceId -> removeGroupFromDevice(deviceId, mcastIp, assignedVlan(null))
+ );
+ }
+ // Transit could be null
+ if (transitDevice != null) {
+ removeGroupFromDevice(transitDevice, mcastIp, assignedVlan(null));
+ }
+ // Ingress device should be not null
+ if (ingressDevice != null) {
+ removeGroupFromDevice(ingressDevice, mcastIp, assignedVlan(source));
+ }
+
+ } finally {
+ mcastUnlock();
+ }
+ }
+
+ /**
* Removes a path from source to sink for given multicast group.
*
* @param source connect point of the multicast source
@@ -275,10 +338,9 @@
lastMcastChange = Instant.now();
mcastLock();
try {
- // 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 {}",
- mcastIp, source.deviceId());
+ // Verify leadership on the operation
+ if (!isLeader(source)) {
+ log.debug("Skip {} due to lack of leadership", mcastIp);
return;
}
@@ -486,24 +548,10 @@
return;
}
- // Continue only when we have the mastership on the operation
- if (!srManager.mastershipService.isLocalMaster(source.deviceId())) {
- // When the source is available we just check the mastership
- if (srManager.deviceService.isAvailable(source.deviceId())) {
- log.debug("Skip {} due to lack of mastership of the source device {}",
- mcastIp, source.deviceId());
- return;
- }
- // Fallback with Leadership service
- // source id is used a topic
- NodeId leader = srManager.leadershipService.runForLeadership(
- source.deviceId().toString()).leaderNodeId();
- // Verify if this node is the leader
- if (!srManager.clusterService.getLocalNode().id().equals(leader)) {
- log.debug("Skip {} due to lack of leadership on the topic {}",
- mcastIp, source.deviceId());
- return;
- }
+ // Verify leadership on the operation
+ if (!isLeader(source)) {
+ log.debug("Skip {} due to lack of leadership", mcastIp);
+ return;
}
// If it exists, we have to remove it in any case
@@ -1189,6 +1237,26 @@
}
}
+ private boolean isLeader(ConnectPoint source) {
+ // Continue only when we have the mastership on the operation
+ if (!srManager.mastershipService.isLocalMaster(source.deviceId())) {
+ // When the source is available we just check the mastership
+ if (srManager.deviceService.isAvailable(source.deviceId())) {
+ return false;
+ }
+ // Fallback with Leadership service
+ // source id is used a topic
+ NodeId leader = srManager.leadershipService.runForLeadership(
+ source.deviceId().toString()).leaderNodeId();
+ // Verify if this node is the leader
+ if (!srManager.clusterService.getLocalNode().id().equals(leader)) {
+ return false;
+ }
+ }
+ // Done
+ return true;
+ }
+
/**
* Performs bucket verification operation for all mcast groups in the devices.
* Firstly, it verifies that mcast is stable before trying verification operation.
diff --git a/apps/segmentrouting/src/main/java/org/onosproject/segmentrouting/SegmentRoutingManager.java b/apps/segmentrouting/src/main/java/org/onosproject/segmentrouting/SegmentRoutingManager.java
index 2b607af..4f85368 100644
--- a/apps/segmentrouting/src/main/java/org/onosproject/segmentrouting/SegmentRoutingManager.java
+++ b/apps/segmentrouting/src/main/java/org/onosproject/segmentrouting/SegmentRoutingManager.java
@@ -1478,8 +1478,10 @@
case SINK_REMOVED:
mcastHandler.processSinkRemoved(event);
break;
- case ROUTE_ADDED:
case ROUTE_REMOVED:
+ mcastHandler.processRouteRemoved(event);
+ break;
+ case ROUTE_ADDED:
default:
break;
}
diff --git a/core/store/dist/src/main/java/org/onosproject/store/mcast/impl/DistributedMcastStore.java b/core/store/dist/src/main/java/org/onosproject/store/mcast/impl/DistributedMcastStore.java
index 6b1eff5..b9fb869 100644
--- a/core/store/dist/src/main/java/org/onosproject/store/mcast/impl/DistributedMcastStore.java
+++ b/core/store/dist/src/main/java/org/onosproject/store/mcast/impl/DistributedMcastStore.java
@@ -41,6 +41,7 @@
import org.onosproject.store.service.Versioned;
import org.slf4j.Logger;
+import java.util.Collections;
import java.util.Map;
import java.util.Optional;
import java.util.Set;
@@ -156,8 +157,15 @@
}
break;
case REMOVE:
+ // Verify old data is not null
+ checkNotNull(oldData);
+ // Create a route removed event with just the route
+ // and the source connect point
notifyDelegate(new McastEvent(McastEvent.Type.ROUTE_REMOVED,
- mcastRouteInfo(route)));
+ mcastRouteInfo(route,
+ Collections.emptySet(),
+ oldData.source()
+ )));
break;
default:
log.warn("Unknown mcast operation type: {}", event.type());