From 21fffd29a468d748e0d8cbf962d329b045359dee Mon Sep 17 00:00:00 2001 From: Pier Luigi Date: Fri, 19 Jan 2018 10:24:53 +0100 Subject: [PATCH] [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 --- .../segmentrouting/McastHandler.java | 31 ++++++++++++++----- .../pipeline/ofdpa/Ofdpa2GroupHandler.java | 27 ++++++++-------- 2 files changed, 37 insertions(+), 21 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 083e9e401d..e77ba97cc9 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 @@ public class McastHandler { IpAddress mcastIp, VlanId assignedVlan) { McastStoreKey mcastStoreKey = new McastStoreKey(mcastIp, deviceId); ImmutableSet.Builder 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 @@ public class McastHandler { 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 @@ public class McastHandler { (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 @@ public class McastHandler { (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 @@ public class McastHandler { * @return next objective builder */ private NextObjective.Builder nextObjBuilder(IpAddress mcastIp, - VlanId assignedVlan, Set outPorts) { - int nextId = srManager.flowObjectiveService.allocateNextId(); + VlanId assignedVlan, Set 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 93730c77b0..bb215ae50b 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 @@ public class Ofdpa2GroupHandler { }); } - private void createL3MulticastGroup(NextObjective nextObj, VlanId vlanId, - List groupInfos) { + private List createL3MulticastBucket(List groupInfos) { List 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 @@ public class Ofdpa2GroupHandler { 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 groupInfos) { + // Let's create a new list mcast buckets + List l3McastBuckets = createL3MulticastBucket(groupInfos); int l3MulticastIndex = getNextAvailableIndex(); int l3MulticastGroupId = L3_MULTICAST_TYPE | @@ -1337,18 +1346,8 @@ public class Ofdpa2GroupHandler { List> allActiveKeys, List groupInfos, VlanId assignedVlan) { - // create the buckets to add to the outermost L3 Multicast group - List 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 newBuckets = createL3MulticastBucket(groupInfos); // get the group being edited Group l3mcastGroup = retrieveTopLevelGroup(allActiveKeys, nextObj.id());