[CORD-2483] Mcast does not forward traffic to all the sinks
Rationale: if we add more than one sink a number
of sink add events are generated. For each event
we create a mcast path, right now we do not support
Mcast group editing. This means that a new nextobjective
will be created for each sink.
In particular, this patch enables the mcast group editing
to solve the following issue:
Sink1 arrives -> fwdObj A -> Next B (this has one output)
Sink2 arrives -> fwdObj A -> Next C (this has two outputs)
Next B and Next C shares a part of the chain. Reordering happens
during the creation of the Nexts:
Next C created -> flow A -> Group C
Next B created -> flow A -> Group B
Failure, traffic does not reach all the sinks. Other side effect is the
disalignment between SR app and flow/group because McastHandler believes
mcast group is associated to the Next C
It includes minor refactoring of the group handler
Change-Id: Ib59ba6b63ff411ed46ca8216677046a78cc92ac6
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 083e9e4..e77ba97 100644
--- a/apps/segmentrouting/src/main/java/org/onosproject/segmentrouting/McastHandler.java
+++ b/apps/segmentrouting/src/main/java/org/onosproject/segmentrouting/McastHandler.java
@@ -504,9 +504,15 @@
IpAddress mcastIp, VlanId assignedVlan) {
McastStoreKey mcastStoreKey = new McastStoreKey(mcastIp, deviceId);
ImmutableSet.Builder<PortNumber> portBuilder = ImmutableSet.builder();
+ NextObjective newNextObj;
if (!mcastNextObjStore.containsKey(mcastStoreKey)) {
// First time someone request this mcast group via this device
portBuilder.add(port);
+ // New nextObj
+ newNextObj = nextObjBuilder(mcastIp, assignedVlan,
+ portBuilder.build(), null).add();
+ // Store the new port
+ mcastNextObjStore.put(mcastStoreKey, newNextObj);
} else {
// This device already serves some subscribers of this mcast group
NextObjective nextObj = mcastNextObjStore.get(mcastStoreKey).value();
@@ -516,7 +522,18 @@
log.info("NextObj for {}/{} already exists. Abort", deviceId, port);
return;
}
- portBuilder.addAll(existingPorts).add(port);
+ // Let's add the port and reuse the previous one
+ portBuilder.addAll(existingPorts).add(port).build();
+ // Reuse previous nextObj
+ newNextObj = nextObjBuilder(mcastIp, assignedVlan,
+ portBuilder.build(), nextObj.id()).addToExisting();
+ // Store the final next objective and send only the difference to the driver
+ mcastNextObjStore.put(mcastStoreKey, newNextObj);
+ // Add just the new port
+ portBuilder = ImmutableSet.builder();
+ portBuilder.add(port);
+ newNextObj = nextObjBuilder(mcastIp, assignedVlan,
+ portBuilder.build(), nextObj.id()).addToExisting();
}
// Create, store and apply the new nextObj and fwdObj
ObjectiveContext context = new DefaultObjectiveContext(
@@ -525,11 +542,8 @@
(objective, error) ->
log.warn("Failed to add {} on {}/{}, vlan {}: {}",
mcastIp, deviceId, port.toLong(), assignedVlan, error));
- NextObjective newNextObj =
- nextObjBuilder(mcastIp, assignedVlan, portBuilder.build()).add();
ForwardingObjective fwdObj =
fwdObjBuilder(mcastIp, assignedVlan, newNextObj.id()).add(context);
- mcastNextObjStore.put(mcastStoreKey, newNextObj);
srManager.flowObjectiveService.next(deviceId, newNextObj);
srManager.flowObjectiveService.forward(deviceId, fwdObj);
}
@@ -590,7 +604,7 @@
(objective, error) ->
log.warn("Failed to update {} on {}/{}, vlan {}: {}",
mcastIp, deviceId, port.toLong(), assignedVlan, error));
- newNextObj = nextObjBuilder(mcastIp, assignedVlan, existingPorts).add();
+ newNextObj = nextObjBuilder(mcastIp, assignedVlan, existingPorts, null).add();
fwdObj = fwdObjBuilder(mcastIp, assignedVlan, newNextObj.id()).add(context);
mcastNextObjStore.put(mcastStoreKey, newNextObj);
srManager.flowObjectiveService.next(deviceId, newNextObj);
@@ -655,8 +669,11 @@
* @return next objective builder
*/
private NextObjective.Builder nextObjBuilder(IpAddress mcastIp,
- VlanId assignedVlan, Set<PortNumber> outPorts) {
- int nextId = srManager.flowObjectiveService.allocateNextId();
+ VlanId assignedVlan, Set<PortNumber> outPorts, Integer nextId) {
+ // If nextId is null allocate a new one
+ if (nextId == null) {
+ nextId = srManager.flowObjectiveService.allocateNextId();
+ }
TrafficSelector metadata =
DefaultTrafficSelector.builder()
diff --git a/drivers/default/src/main/java/org/onosproject/driver/pipeline/ofdpa/Ofdpa2GroupHandler.java b/drivers/default/src/main/java/org/onosproject/driver/pipeline/ofdpa/Ofdpa2GroupHandler.java
index 93730c7..bb215ae 100644
--- a/drivers/default/src/main/java/org/onosproject/driver/pipeline/ofdpa/Ofdpa2GroupHandler.java
+++ b/drivers/default/src/main/java/org/onosproject/driver/pipeline/ofdpa/Ofdpa2GroupHandler.java
@@ -804,9 +804,9 @@
});
}
- private void createL3MulticastGroup(NextObjective nextObj, VlanId vlanId,
- List<GroupInfo> groupInfos) {
+ private List<GroupBucket> createL3MulticastBucket(List<GroupInfo> groupInfos) {
List<GroupBucket> l3McastBuckets = new ArrayList<>();
+ // For each inner group
groupInfos.forEach(groupInfo -> {
// Points to L3 interface group if there is one.
// Otherwise points to L2 interface group directly.
@@ -817,6 +817,15 @@
GroupBucket abucket = DefaultGroupBucket.createAllGroupBucket(ttb.build());
l3McastBuckets.add(abucket);
});
+ // Done return the new list of buckets
+ return l3McastBuckets;
+ }
+
+
+ private void createL3MulticastGroup(NextObjective nextObj, VlanId vlanId,
+ List<GroupInfo> groupInfos) {
+ // Let's create a new list mcast buckets
+ List<GroupBucket> l3McastBuckets = createL3MulticastBucket(groupInfos);
int l3MulticastIndex = getNextAvailableIndex();
int l3MulticastGroupId = L3_MULTICAST_TYPE |
@@ -1337,18 +1346,8 @@
List<Deque<GroupKey>> allActiveKeys,
List<GroupInfo> groupInfos,
VlanId assignedVlan) {
- // create the buckets to add to the outermost L3 Multicast group
- List<GroupBucket> newBuckets = Lists.newArrayList();
- groupInfos.forEach(groupInfo -> {
- // Points to L3 interface group if there is one.
- // Otherwise points to L2 interface group directly.
- GroupDescription nextGroupDesc = (groupInfo.nextGroupDesc() != null) ?
- groupInfo.nextGroupDesc() : groupInfo.innerMostGroupDesc();
- TrafficTreatment.Builder treatmentBuilder = DefaultTrafficTreatment.builder();
- treatmentBuilder.group(new GroupId(nextGroupDesc.givenGroupId()));
- GroupBucket newBucket = DefaultGroupBucket.createAllGroupBucket(treatmentBuilder.build());
- newBuckets.add(newBucket);
- });
+ // Create the buckets to add to the outermost L3 Multicast group
+ List<GroupBucket> newBuckets = createL3MulticastBucket(groupInfos);
// get the group being edited
Group l3mcastGroup = retrieveTopLevelGroup(allActiveKeys, nextObj.id());