Handling multiple layers of spines.
Also in this commit:
- Triggering swap group creation and accounting for it in DestinationSet
- Fixes in ofdpa2 and ofdpa3 pipeline to allow SR Continue operation
- Renaming mplsSet in DestinationSet to notBos
- Removing unused RandomDestinationSet
- Bug fix in ofdpa driver for swap group chain creation
- Bug fix in ofdpa driver for verify group operation
- Better internal bookeeping of device ports and associated neighbors
Change-Id: I2b8f1c4c0b305ef847d57ca7a5320943e06d190d
diff --git a/apps/segmentrouting/app/src/main/java/org/onosproject/segmentrouting/DefaultRoutingHandler.java b/apps/segmentrouting/app/src/main/java/org/onosproject/segmentrouting/DefaultRoutingHandler.java
index 98d5d4a..46d94ed 100644
--- a/apps/segmentrouting/app/src/main/java/org/onosproject/segmentrouting/DefaultRoutingHandler.java
+++ b/apps/segmentrouting/app/src/main/java/org/onosproject/segmentrouting/DefaultRoutingHandler.java
@@ -787,7 +787,7 @@
* @param destSw1 Device ID of final destination switch to which the rules will forward
* @param destSw2 Device ID of paired destination switch to which the rules will forward
* A null deviceId indicates packets should only be sent to destSw1
- * @param nextHops Map indication a list of next hops per destSw
+ * @param nextHops Map of a set of next hops per destSw
* @param subnets Subnets to be populated. If empty, populate all configured subnets.
* @return true if it succeeds in populating rules
*/ // refactor
@@ -822,7 +822,8 @@
subnets = (subnets != null && !subnets.isEmpty())
? Sets.newHashSet(subnets)
: Sets.newHashSet(config.getSubnets(destSw1));
- // XXX - Rethink this
+ // XXX - Rethink this - ignoring routerIPs in all other switches
+ // even edge to edge switches
/*subnets.add(dest1RouterIpv4.toIpPrefix());
if (dest1RouterIpv6 != null) {
subnets.add(dest1RouterIpv6.toIpPrefix());
@@ -843,26 +844,6 @@
if (!result) {
return false;
}
- /* XXX rethink this
- IpPrefix routerIpPrefix = destRouterIpv4.toIpPrefix();
- log.debug("* populateEcmpRoutingRulePartial in device {} towards {} "
- + "for router IP {}", targetSw, destSw, routerIpPrefix);
- result = rulePopulator.populateIpRuleForRouter(targetSw, routerIpPrefix,
- destSw, nextHops);
- if (!result) {
- return false;
- }
- // If present we deal with IPv6 loopback.
- if (destRouterIpv6 != null) {
- routerIpPrefix = destRouterIpv6.toIpPrefix();
- log.debug("* populateEcmpRoutingRulePartial in device {} towards {}"
- + " for v6 router IP {}", targetSw, destSw, routerIpPrefix);
- result = rulePopulator.populateIpRuleForRouter(targetSw, routerIpPrefix,
- destSw, nextHops);
- if (!result) {
- return false;
- }
- }*/
}
if (!targetIsEdge && dest1IsEdge) {
@@ -877,11 +858,20 @@
return false;
}
if (dest1RouterIpv6 != null) {
- result = rulePopulator.populateMplsRule(targetSw, destSw1,
- nextHops.get(destSw1),
- dest1RouterIpv6);
- if (!result) {
- return false;
+ int v4sid = 0, v6sid = 0;
+ try {
+ v4sid = config.getIPv4SegmentId(destSw1);
+ v6sid = config.getIPv6SegmentId(destSw1);
+ } catch (DeviceConfigNotFoundException e) {
+ log.warn(e.getMessage());
+ }
+ if (v4sid != v6sid) {
+ result = rulePopulator.populateMplsRule(targetSw, destSw1,
+ nextHops.get(destSw1),
+ dest1RouterIpv6);
+ if (!result) {
+ return false;
+ }
}
}
}
@@ -1435,11 +1425,26 @@
continue;
}
if (!targetIsEdge && itrIdx > 1) {
- // optimization for spines to not use other leaves to get
- // to a leaf to avoid loops
- log.debug("Avoiding {} hop path for non-edge targetSw:{}"
- + " --> dstSw:{}", itrIdx, targetSw, dstSw);
- break;
+ // optimization for spines to not use leaves to get
+ // to a spine or other leaves
+ boolean pathdevIsEdge = false;
+ for (ArrayList<DeviceId> via : swViaMap.get(targetSw)) {
+ for (DeviceId pathdev : via) {
+ try {
+ pathdevIsEdge = srManager.deviceConfiguration
+ .isEdgeDevice(pathdev);
+ } catch (DeviceConfigNotFoundException e) {
+ log.warn(e.getMessage());
+ }
+ if (pathdevIsEdge) {
+ log.debug("Avoiding {} hop path for non-edge targetSw:{}"
+ + " --> dstSw:{} which goes through an edge"
+ + " device {} in path {}", itrIdx,
+ targetSw, dstSw, pathdev, via);
+ return ImmutableSet.of();
+ }
+ }
+ }
}
Set<DeviceId> nextHops = new HashSet<>();
for (ArrayList<DeviceId> via : swViaMap.get(targetSw)) {
diff --git a/apps/segmentrouting/app/src/main/java/org/onosproject/segmentrouting/LinkHandler.java b/apps/segmentrouting/app/src/main/java/org/onosproject/segmentrouting/LinkHandler.java
index be99764..63473a7 100644
--- a/apps/segmentrouting/app/src/main/java/org/onosproject/segmentrouting/LinkHandler.java
+++ b/apps/segmentrouting/app/src/main/java/org/onosproject/segmentrouting/LinkHandler.java
@@ -105,35 +105,33 @@
if (!isLinkValid(link)) {
return;
}
- if (srManager.deviceConfiguration == null ||
- !srManager.deviceConfiguration.isConfigured(link.src().deviceId())) {
- updateSeenLink(link, true);
- // XXX revisit - what about devicePortMap
- log.warn("Source device of this link is not configured.. "
- + "not processing further");
- return;
- }
-
// Irrespective of whether the local is a MASTER or not for this device,
// create group handler instance and push default TTP flow rules if needed,
// as in a multi-instance setup, instances can initiate groups for any
- // device.
+ // device. Also update local groupHandler stores.
DefaultGroupHandler groupHandler = srManager.groupHandlerMap
- .get(link.src().deviceId());
+ .get(link.src().deviceId());
if (groupHandler != null) {
groupHandler.portUpForLink(link);
} else {
- // XXX revisit/cleanup
Device device = srManager.deviceService.getDevice(link.src().deviceId());
if (device != null) {
- log.warn("processLinkAdded: Link Added "
- + "Notification without Device Added "
- + "event, still handling it");
+ log.warn("processLinkAdded: Link Added notification without "
+ + "Device Added event, still handling it");
srManager.processDeviceAdded(device);
groupHandler = srManager.groupHandlerMap.get(link.src().deviceId());
- groupHandler.portUpForLink(link);
+ if (groupHandler != null) {
+ groupHandler.portUpForLink(link);
+ }
}
}
+ if (srManager.deviceConfiguration == null ||
+ !srManager.deviceConfiguration.isConfigured(link.src().deviceId())) {
+ updateSeenLink(link, true);
+ log.warn("Source device of this link is not configured.. "
+ + "not processing further");
+ return;
+ }
/*
// process link only if it is bidirectional
@@ -221,7 +219,7 @@
srManager.defaultRoutingHandler
.populateRoutingRulesForLinkStatusChange(link, null, null);
- // update local groupHandler stores
+ // attempt rehashing for parallel links
DefaultGroupHandler groupHandler = srManager.groupHandlerMap
.get(link.src().deviceId());
if (groupHandler != null) {
@@ -236,8 +234,8 @@
.deviceId())) ? "not parallel"
: "not master");
}
- // ensure local stores are updated
- groupHandler.portDown(link.src().port());
+ // ensure local stores are updated after all rerouting or rehashing
+ groupHandler.portDownForLink(link);
} else {
log.warn("group handler not found for dev:{} when removing link: {}",
link.src().deviceId(), link);
@@ -273,14 +271,14 @@
return true;
}
try {
- if (!devConfig.isEdgeDevice(srcId)
+ /*if (!devConfig.isEdgeDevice(srcId)
&& !devConfig.isEdgeDevice(dstId)) {
// ignore links between spines
// XXX revisit when handling multi-stage fabrics
log.warn("Links between spines not allowed...ignoring "
+ "link {}", link);
return false;
- }
+ }*/
if (devConfig.isEdgeDevice(srcId)
&& devConfig.isEdgeDevice(dstId)) {
// ignore links between leaves if they are not pair-links
diff --git a/apps/segmentrouting/app/src/main/java/org/onosproject/segmentrouting/RoutingRulePopulator.java b/apps/segmentrouting/app/src/main/java/org/onosproject/segmentrouting/RoutingRulePopulator.java
index c152b99..bd57248 100644
--- a/apps/segmentrouting/app/src/main/java/org/onosproject/segmentrouting/RoutingRulePopulator.java
+++ b/apps/segmentrouting/app/src/main/java/org/onosproject/segmentrouting/RoutingRulePopulator.java
@@ -352,32 +352,25 @@
DestinationSet ds;
TrafficTreatment treatment;
- // If the next hop is the same as the final destination, then MPLS label
- // is not set.
- /*if (nextHops.size() == 1 && nextHops.toArray()[0].equals(destSw)) {
- tbuilder.immediate().decNwTtl();
- ds = new DestinationSet(false, destSw);
- treatment = tbuilder.build();
- } else {
- ds = new DestinationSet(false, segmentId, destSw);
- treatment = null;
- }*/
if (destSw2 == null) {
// single dst - create destination set based on next-hop
+ // If the next hop is the same as the final destination, then MPLS
+ // label is not set.
Set<DeviceId> nhd1 = nextHops.get(destSw1);
if (nhd1.size() == 1 && nhd1.iterator().next().equals(destSw1)) {
tbuilder.immediate().decNwTtl();
- ds = new DestinationSet(false, destSw1);
+ ds = new DestinationSet(false, false, destSw1);
treatment = tbuilder.build();
} else {
- ds = new DestinationSet(false, segmentId1, destSw1);
+ ds = new DestinationSet(false, false, segmentId1, destSw1);
treatment = null;
}
} else {
// dst pair - IP rules for dst-pairs are always from other edge nodes
// the destination set needs to have both destinations, even if there
// are no next hops to one of them
- ds = new DestinationSet(false, segmentId1, destSw1, segmentId2, destSw2);
+ ds = new DestinationSet(false, false, segmentId1, destSw1,
+ segmentId2, destSw2);
treatment = null;
}
@@ -394,7 +387,7 @@
}
int nextId = grpHandler.getNextObjectiveId(ds, nextHops,
- metabuilder.build(), true);
+ metabuilder.build(), false);
if (nextId <= 0) {
log.warn("No next objective in {} for ds: {}", targetSw, ds);
return false;
@@ -461,13 +454,75 @@
}
/**
- * Deals with !MPLS Bos use case.
+ * Populates MPLS flow rules in the target device to point towards the
+ * destination device.
+ *
+ * @param targetSwId target device ID of the switch to set the rules
+ * @param destSwId destination switch device ID
+ * @param nextHops next hops switch ID list
+ * @param routerIp the router ip of the destination switch
+ * @return true if all rules are set successfully, false otherwise
+ */
+ boolean populateMplsRule(DeviceId targetSwId, DeviceId destSwId,
+ Set<DeviceId> nextHops, IpAddress routerIp) {
+ int segmentId;
+ try {
+ if (routerIp.isIp4()) {
+ segmentId = config.getIPv4SegmentId(destSwId);
+ } else {
+ segmentId = config.getIPv6SegmentId(destSwId);
+ }
+ } catch (DeviceConfigNotFoundException e) {
+ log.warn(e.getMessage() + " Aborting populateMplsRule.");
+ return false;
+ }
+
+ List<ForwardingObjective> fwdObjs = new ArrayList<>();
+ Collection<ForwardingObjective> fwdObjsMpls;
+ // Generates the transit rules used by the standard "routing".
+ fwdObjsMpls = handleMpls(targetSwId, destSwId, nextHops, segmentId,
+ routerIp, true);
+ if (fwdObjsMpls.isEmpty()) {
+ return false;
+ }
+ fwdObjs.addAll(fwdObjsMpls);
+
+ // Generates the transit rules used by the MPLS Pwaas.
+ int pwSrLabel;
+ try {
+ pwSrLabel = config.getPWRoutingLabel(destSwId);
+ } catch (DeviceConfigNotFoundException e) {
+ log.warn(e.getMessage()
+ + " Aborting populateMplsRule. No label for PseudoWire traffic.");
+ return false;
+ }
+ fwdObjsMpls = handleMpls(targetSwId, destSwId, nextHops, pwSrLabel,
+ routerIp, false);
+ if (fwdObjsMpls.isEmpty()) {
+ return false;
+ }
+ fwdObjs.addAll(fwdObjsMpls);
+
+ for (ForwardingObjective fwdObj : fwdObjs) {
+ log.debug("Sending MPLS fwd obj {} for SID {}-> next {} in sw: {}",
+ fwdObj.id(), segmentId, fwdObj.nextId(), targetSwId);
+ srManager.flowObjectiveService.forward(targetSwId, fwdObj);
+ rulePopulationCounter.incrementAndGet();
+ }
+
+ return true;
+ }
+
+ /**
+ * Differentiates between popping and swapping labels when building an MPLS
+ * forwarding objective.
*
* @param targetSwId the target sw
* @param destSwId the destination sw
* @param nextHops the set of next hops
- * @param segmentId the segmentId to match
- * @param routerIp the router ip
+ * @param segmentId the segmentId to match representing the destination
+ * switch
+ * @param routerIp the router ip representing the destination switch
* @return a collection of fwdobjective
*/
private Collection<ForwardingObjective> handleMpls(
@@ -497,9 +552,6 @@
log.debug("populateMplsRule: Installing MPLS forwarding objective for "
+ "label {} in switch {} with pop to next-hops {}",
segmentId, targetSwId, nextHops);
- // Not-bos pop case (php for the current label). If MPLS-ECMP
- // has been configured, the application we will request the
- // installation for an MPLS-ECMP group.
ForwardingObjective.Builder fwdObjNoBosBuilder =
getMplsForwardingObjective(targetSwId,
nextHops,
@@ -507,6 +559,7 @@
isMplsBos,
metabuilder.build(),
routerIp,
+ segmentId,
destSwId);
// Error case, we cannot handle, exit.
if (fwdObjNoBosBuilder == null) {
@@ -515,13 +568,11 @@
fwdObjBuilders.add(fwdObjNoBosBuilder);
} else {
- // next hop is not destination, SR CONTINUE case (swap with self)
+ // next hop is not destination, irrespective of the number of next
+ // hops (1 or more) -- SR CONTINUE case (swap with self)
log.debug("Installing MPLS forwarding objective for "
+ "label {} in switch {} without pop to next-hops {}",
segmentId, targetSwId, nextHops);
- // Not-bos pop case. If MPLS-ECMP has been configured, the
- // application we will request the installation for an MPLS-ECMP
- // group.
ForwardingObjective.Builder fwdObjNoBosBuilder =
getMplsForwardingObjective(targetSwId,
nextHops,
@@ -529,6 +580,7 @@
isMplsBos,
metabuilder.build(),
routerIp,
+ segmentId,
destSwId);
// Error case, we cannot handle, exit.
if (fwdObjNoBosBuilder == null) {
@@ -541,7 +593,6 @@
List<ForwardingObjective> fwdObjs = Lists.newArrayList();
// We add the final property to the fwdObjs.
for (ForwardingObjective.Builder fwdObjBuilder : fwdObjBuilders) {
-
((Builder) ((Builder) fwdObjBuilder
.fromApp(srManager.appId)
.makePermanent())
@@ -559,71 +610,25 @@
ForwardingObjective fob = fwdObjBuilder.add(context);
fwdObjs.add(fob);
-
}
return fwdObjs;
}
/**
- * Populates MPLS flow rules in the target device to point towards the
- * destination device.
+ * Returns a Forwarding Objective builder for the MPLS rule that references
+ * the desired Next Objective. Creates a DestinationSet that allows the
+ * groupHandler to create or find the required next objective.
*
- * @param targetSwId target device ID of the switch to set the rules
- * @param destSwId destination switch device ID
- * @param nextHops next hops switch ID list
- * @param routerIp the router ip
- * @return true if all rules are set successfully, false otherwise
+ * @param targetSw the target sw
+ * @param nextHops the set of next hops
+ * @param phpRequired true if penultimate-hop-popping is required
+ * @param isBos true if matched label is bottom-of-stack
+ * @param meta metadata for creating next objective
+ * @param routerIp the router ip representing the destination switch
+ * @param destSw the destination sw
+ * @return the mpls forwarding objective builder
*/
- boolean populateMplsRule(DeviceId targetSwId, DeviceId destSwId,
- Set<DeviceId> nextHops,
- IpAddress routerIp) {
-
- int segmentId;
- try {
- if (routerIp.isIp4()) {
- segmentId = config.getIPv4SegmentId(destSwId);
- } else {
- segmentId = config.getIPv6SegmentId(destSwId);
- }
- } catch (DeviceConfigNotFoundException e) {
- log.warn(e.getMessage() + " Aborting populateMplsRule.");
- return false;
- }
-
- List<ForwardingObjective> fwdObjs = new ArrayList<>();
- Collection<ForwardingObjective> fwdObjsMpls;
- // Generates the transit rules used by the standard "routing".
- fwdObjsMpls = handleMpls(targetSwId, destSwId, nextHops, segmentId, routerIp, true);
- if (fwdObjsMpls.isEmpty()) {
- return false;
- }
- fwdObjs.addAll(fwdObjsMpls);
-
- // Generates the transit rules used by the MPLS Pwaas.
- int pwSrLabel;
- try {
- pwSrLabel = config.getPWRoutingLabel(destSwId);
- } catch (DeviceConfigNotFoundException e) {
- log.warn(e.getMessage() + " Aborting populateMplsRule. No label for PseudoWire traffic.");
- return false;
- }
- fwdObjsMpls = handleMpls(targetSwId, destSwId, nextHops, pwSrLabel, routerIp, false);
- if (fwdObjsMpls.isEmpty()) {
- return false;
- }
- fwdObjs.addAll(fwdObjsMpls);
-
- for (ForwardingObjective fwdObj : fwdObjs) {
- log.debug("Sending MPLS fwd obj {} for SID {}-> next {} in sw: {}",
- fwdObj.id(), segmentId, fwdObj.nextId(), targetSwId);
- srManager.flowObjectiveService.forward(targetSwId, fwdObj);
- rulePopulationCounter.incrementAndGet();
- }
-
- return true;
- }
-
private ForwardingObjective.Builder getMplsForwardingObjective(
DeviceId targetSw,
Set<DeviceId> nextHops,
@@ -631,13 +636,15 @@
boolean isBos,
TrafficSelector meta,
IpAddress routerIp,
+ int segmentId,
DeviceId destSw) {
ForwardingObjective.Builder fwdBuilder = DefaultForwardingObjective
.builder().withFlag(ForwardingObjective.Flag.SPECIFIC);
TrafficTreatment.Builder tbuilder = DefaultTrafficTreatment.builder();
-
+ DestinationSet ds = null;
+ boolean simple = false;
if (phpRequired) {
// php case - pop should always be flow-action
log.debug("getMplsForwardingObjective: php required");
@@ -649,49 +656,54 @@
tbuilder.deferred().popMpls(EthType.EtherType.IPV6.ethType());
}
tbuilder.decNwTtl();
+ // standard case -> BoS == True; pop results in IP packet and forwarding
+ // is via an ECMP group
+ ds = new DestinationSet(false, false, destSw);
} else {
tbuilder.deferred().popMpls(EthType.EtherType.MPLS_UNICAST.ethType())
.decMplsTtl();
+ // double-label case -> BoS == False, pop results in MPLS packet
+ // depending on configuration we can ECMP this packet or choose one output
+ if (srManager.getMplsEcmp()) {
+ ds = new DestinationSet(true, false, destSw);
+ } else {
+ ds = new DestinationSet(true, false, destSw);
+ simple = true;
+ }
}
} else {
// swap with self case - SR CONTINUE
- log.debug("getMplsForwardingObjective: php not required");
+ log.debug("getMplsForwardingObjective: swap with self");
tbuilder.deferred().decMplsTtl();
+ // swap results in MPLS packet with same BoS bit regardless of bit value
+ // depending on configuration we can ECMP this packet or choose one output
+ if (srManager.getMplsEcmp()) {
+ ds = new DestinationSet(false, true, segmentId, destSw);
+ } else {
+ ds = new DestinationSet(false, true, segmentId, destSw);
+ simple = true;
+ }
}
fwdBuilder.withTreatment(tbuilder.build());
- // if MPLS-ECMP == True we will build a standard NeighborSet.
- // Otherwise a RandomNeighborSet.
- DestinationSet ns = DestinationSet.destinationSet(false, false, destSw);
- if (!isBos && this.srManager.getMplsEcmp()) {
- ns = DestinationSet.destinationSet(false, true, destSw);
- } else if (!isBos && !this.srManager.getMplsEcmp()) {
- ns = DestinationSet.destinationSet(true, true, destSw);
- }
-
- log.debug("Trying to get a nextObjId for mpls rule on device:{} to ns:{}",
- targetSw, ns);
+ log.debug("Trying to get a nextObjId for mpls rule on device:{} to ds:{}",
+ targetSw, ds);
DefaultGroupHandler gh = srManager.getGroupHandler(targetSw);
if (gh == null) {
log.warn("getNextObjectiveId query - groupHandler for device {} "
+ "not found", targetSw);
return null;
}
- // If BoS == True, all forwarding is via L3 ECMP group.
- // If Bos == False, the forwarding can be via MPLS-ECMP group or through
- // MPLS-Interface group. This depends on the configuration of the option
- // MPLS-ECMP.
- // The metadata informs the driver that the next-Objective will be used
- // by MPLS flows and if Bos == False the driver will use MPLS groups.
+
Map<DeviceId, Set<DeviceId>> dstNextHops = new HashMap<>();
dstNextHops.put(destSw, nextHops);
- int nextId = gh.getNextObjectiveId(ns, dstNextHops, meta, isBos);
+ int nextId = gh.getNextObjectiveId(ds, dstNextHops, meta, simple);
if (nextId <= 0) {
- log.warn("No next objective in {} for ns: {}", targetSw, ns);
+ log.warn("No next objective in {} for ds: {}", targetSw, ds);
return null;
} else {
- log.debug("nextObjId found:{} for mpls rule on device:{} to ns:{}",
- nextId, targetSw, ns);
+ log.debug("nextObjId found:{} for mpls rule on device:{} to ds:{}",
+ nextId, targetSw, ds);
}
fwdBuilder.nextStep(nextId);
diff --git a/apps/segmentrouting/app/src/main/java/org/onosproject/segmentrouting/SegmentRoutingManager.java b/apps/segmentrouting/app/src/main/java/org/onosproject/segmentrouting/SegmentRoutingManager.java
index 559f60c..c6bb015 100644
--- a/apps/segmentrouting/app/src/main/java/org/onosproject/segmentrouting/SegmentRoutingManager.java
+++ b/apps/segmentrouting/app/src/main/java/org/onosproject/segmentrouting/SegmentRoutingManager.java
@@ -1263,6 +1263,11 @@
.populateRoutingRulesForLinkStatusChange(null, null, device.id());
defaultRoutingHandler.purgeEcmpGraph(device.id());
xConnectHandler.removeDevice(device.id());
+
+ // Cleanup all internal groupHandler stores for this device. Should be
+ // done after all rerouting or rehashing has been completed
+ groupHandlerMap.entrySet()
+ .forEach(entry -> entry.getValue().cleanUpForNeighborDown(device.id()));
}
private void processPortUpdated(Device device, Port port) {
diff --git a/apps/segmentrouting/app/src/main/java/org/onosproject/segmentrouting/TunnelHandler.java b/apps/segmentrouting/app/src/main/java/org/onosproject/segmentrouting/TunnelHandler.java
index e18b920..1dd845b 100644
--- a/apps/segmentrouting/app/src/main/java/org/onosproject/segmentrouting/TunnelHandler.java
+++ b/apps/segmentrouting/app/src/main/java/org/onosproject/segmentrouting/TunnelHandler.java
@@ -222,7 +222,7 @@
deviceIds.add(config.getDeviceId(sid));
}
// For these NeighborSet isMpls is meaningless.
- DestinationSet ns = new DestinationSet(false,
+ DestinationSet ns = new DestinationSet(false, false,
tunnel.labelIds().get(2),
DeviceId.NONE);
diff --git a/apps/segmentrouting/app/src/main/java/org/onosproject/segmentrouting/config/DeviceConfiguration.java b/apps/segmentrouting/app/src/main/java/org/onosproject/segmentrouting/config/DeviceConfiguration.java
index 7ea430d..070a7ec 100644
--- a/apps/segmentrouting/app/src/main/java/org/onosproject/segmentrouting/config/DeviceConfiguration.java
+++ b/apps/segmentrouting/app/src/main/java/org/onosproject/segmentrouting/config/DeviceConfiguration.java
@@ -196,7 +196,6 @@
@Override
public int getIPv4SegmentId(DeviceId deviceId) throws DeviceConfigNotFoundException {
SegmentRouterInfo srinfo = deviceConfigMap.get(deviceId);
- log.info("DEVICE MAP IS {}", deviceConfigMap);
if (srinfo != null) {
log.trace("getIPv4SegmentId for device{} is {}", deviceId, srinfo.ipv4NodeSid);
return srinfo.ipv4NodeSid;
diff --git a/apps/segmentrouting/app/src/main/java/org/onosproject/segmentrouting/grouphandler/DefaultGroupHandler.java b/apps/segmentrouting/app/src/main/java/org/onosproject/segmentrouting/grouphandler/DefaultGroupHandler.java
index 3cf1331..1e0e6fd 100644
--- a/apps/segmentrouting/app/src/main/java/org/onosproject/segmentrouting/grouphandler/DefaultGroupHandler.java
+++ b/apps/segmentrouting/app/src/main/java/org/onosproject/segmentrouting/grouphandler/DefaultGroupHandler.java
@@ -188,7 +188,8 @@
}
/**
- * Updates local stores for link-src device/port to neighbor (link-dst).
+ * Updates local stores for link-src-device/port to neighbor (link-dst) for
+ * link that has come up.
*
* @param link the infrastructure link
*/
@@ -207,11 +208,13 @@
}
/**
- * Updates local stores for port that has gone down.
+ * Updates local stores for link-src-device/port to neighbor (link-dst) for
+ * link that has gone down.
*
- * @param port port number that has gone down
+ * @param link the infrastructure link
*/
- public void portDown(PortNumber port) {
+ public void portDownForLink(Link link) {
+ PortNumber port = link.src().port();
if (portDeviceMap.get(port) == null) {
log.warn("portDown: unknown port");
return;
@@ -224,6 +227,18 @@
}
/**
+ * Cleans up local stores for removed neighbor device.
+ *
+ * @param neighborId the device identifier for the neighbor device
+ */
+ public void cleanUpForNeighborDown(DeviceId neighborId) {
+ Set<PortNumber> ports = devicePortMap.remove(neighborId);
+ if (ports != null) {
+ ports.forEach(p -> portDeviceMap.remove(p));
+ }
+ }
+
+ /**
* Checks all groups in the src-device of link for neighbor sets that include
* the dst-device of link, and edits the hash groups according to link up
* or down. Should only be called by the master instance of the src-switch
@@ -248,8 +263,11 @@
.stream()
.filter(entry -> entry.getKey().deviceId().equals(deviceId))
// Filter out PW transit groups or include them if MPLS ECMP is supported
- .filter(entry -> !entry.getKey().destinationSet().mplsSet() ||
- (entry.getKey().destinationSet().mplsSet() && srManager.getMplsEcmp()))
+ .filter(entry -> !entry.getKey().destinationSet().notBos() ||
+ (entry.getKey().destinationSet().notBos() && srManager.getMplsEcmp()))
+ // Filter out simple SWAP groups or include them if MPLS ECMP is supported
+ .filter(entry -> !entry.getKey().destinationSet().swap() ||
+ (entry.getKey().destinationSet().swap() && srManager.getMplsEcmp()))
.filter(entry -> entry.getValue().containsNextHop(link.dst().deviceId()))
.map(entry -> entry.getKey())
.collect(Collectors.toSet());
@@ -452,6 +470,21 @@
int edgeLabel = dskey.destinationSet().getEdgeLabel(destSw);
Integer nextId = nhops.nextId();
+ // some store elements may not be hashed next-objectives - ignore them
+ if (isSimpleNextObjective(dskey)) {
+ log.debug("Ignoring {} of SIMPLE nextObj for targetSw:{}"
+ + " -> dstSw:{} with current nextHops:{} to new"
+ + " nextHops: {} in nextId:{}",
+ (revoke) ? "removal" : "addition", targetSw, destSw,
+ currNeighbors, nextHops, nextId);
+ if ((revoke && !nextHops.isEmpty())
+ || (!revoke && !nextHops.equals(currNeighbors))) {
+ log.warn("Simple next objective cannot be edited to "
+ + "move from {} to {}", currNeighbors, nextHops);
+ }
+ continue;
+ }
+
if (currNeighbors == null || nextHops == null) {
log.warn("fixing hash groups but found currNeighbors:{} or nextHops:{}"
+ " in targetSw:{} for dstSw:{}", currNeighbors, nextHops,
@@ -514,6 +547,7 @@
new NextNeighbors(currentNextHops.dstNextHops(),
currentNextHops.nextId()));
}
+
// even if one fails and others succeed, return false so ECMPspg not updated
return success;
}
@@ -615,6 +649,18 @@
}
/**
+ * Returns true if the destination set is meant for swap or multi-labeled
+ * packet transport, and MPLS ECMP is not supported.
+ *
+ * @param dskey the key representing the destination set
+ * @return true if destination set is meant for simple next objectives
+ */
+ boolean isSimpleNextObjective(DestinationSetNextObjectiveStoreKey dskey) {
+ return (dskey.destinationSet().notBos() || dskey.destinationSet().swap())
+ && !srManager.getMplsEcmp();
+ }
+
+ /**
* Adds or removes a port that has been configured with a vlan to a broadcast group
* for bridging. Should only be called by the master instance for this device.
*
@@ -678,23 +724,26 @@
}
/**
- * Returns the next objective of type hashed associated with the destination set.
- * In addition, updates the existing next-objective if new route-route paths found
- * have resulted in the addition of new next-hops to a particular destination.
- * If there is no existing next objective for this destination set, this method
- * would create a next objective and return the nextId. Optionally metadata can be
- * passed in for the creation of the next objective.
+ * Returns the next objective of type hashed (or simple) associated with the
+ * destination set. In addition, updates the existing next-objective if new
+ * route-paths found have resulted in the addition of new next-hops to a
+ * particular destination. If there is no existing next objective for this
+ * destination set, this method would create a next objective and return the
+ * nextId. Optionally metadata can be passed in for the creation of the next
+ * objective. If the parameter simple is true then a simple next objective
+ * is created instead of a hashed one.
*
* @param ds destination set
* @param nextHops a map of per destination next hops
* @param meta metadata passed into the creation of a Next Objective
- * @param isBos if Bos is set
+ * @param simple if true, a simple next objective will be created instead of
+ * a hashed next objective
* @return int if found or -1 if there are errors in the creation of the
- * neighbor set.
+ * neighbor set.
*/
public int getNextObjectiveId(DestinationSet ds,
Map<DeviceId, Set<DeviceId>> nextHops,
- TrafficSelector meta, boolean isBos) {
+ TrafficSelector meta, boolean simple) {
NextNeighbors next = dsNextObjStore.
get(new DestinationSetNextObjectiveStoreKey(deviceId, ds));
if (next == null) {
@@ -708,7 +757,7 @@
(nsStoreEntry.getKey().deviceId().equals(deviceId)))
.collect(Collectors.toList()));
- createGroupFromDestinationSet(ds, nextHops, meta, isBos);
+ createGroupFromDestinationSet(ds, nextHops, meta, simple);
next = dsNextObjStore.
get(new DestinationSetNextObjectiveStoreKey(deviceId, ds));
if (next == null) {
@@ -833,39 +882,38 @@
}
// Update portToDevice database
- DeviceId prev = portDeviceMap.putIfAbsent(portToNeighbor, neighborId);
+ // should always update as neighbor could have changed on this port
+ DeviceId prev = portDeviceMap.put(portToNeighbor, neighborId);
if (prev != null) {
- log.debug("Device: {} port: {} already has neighbor: {} ",
+ log.debug("Device/port: {}/{} previous neighbor: {}, current neighbor: {} ",
deviceId, portToNeighbor, prev, neighborId);
}
}
/**
* Creates a NextObjective for a hash group in this device from a given
- * DestinationSet.
+ * DestinationSet. If the parameter simple is true, a simple next objective
+ * is created instead.
*
* @param ds the DestinationSet
* @param neighbors a map for each destination and its next-hops
* @param meta metadata passed into the creation of a Next Objective
- * @param isBos if BoS is set
+ * @param simple if true, a simple next objective will be created instead of
+ * a hashed next objective
*/
public void createGroupFromDestinationSet(DestinationSet ds,
Map<DeviceId, Set<DeviceId>> neighbors,
TrafficSelector meta,
- boolean isBos) {
+ boolean simple) {
int nextId = flowObjectiveService.allocateNextId();
- NextObjective.Type type = NextObjective.Type.HASHED;
+ NextObjective.Type type = (simple) ? NextObjective.Type.SIMPLE
+ : NextObjective.Type.HASHED;
if (neighbors == null || neighbors.isEmpty()) {
log.warn("createGroupsFromDestinationSet: needs at least one neighbor"
+ "to create group in dev:{} for ds: {} with next-hops {}",
deviceId, ds, neighbors);
return;
}
- // If Bos == False and MPLS-ECMP == false, we have
- // to use simple group and we will pick a single neighbor for a single dest.
- if (!isBos && !srManager.getMplsEcmp()) {
- type = NextObjective.Type.SIMPLE;
- }
NextObjective.Builder nextObjBuilder = DefaultNextObjective
.builder()
@@ -878,7 +926,7 @@
// create treatment buckets for each neighbor for each dst Device
// except in the special case where we only want to pick a single
- // neighbor for a simple group
+ // neighbor/port for a simple nextObj
boolean foundSingleNeighbor = false;
boolean treatmentAdded = false;
Map<DeviceId, Set<DeviceId>> dstNextHops = new ConcurrentHashMap<>();
@@ -912,8 +960,8 @@
}
// For each port to the neighbor, we create a new treatment
Set<PortNumber> neighborPorts = devicePortMap.get(neighborId);
- // In this case we are using a SIMPLE group. We randomly pick a port
- if (!isBos && !srManager.getMplsEcmp()) {
+ // In this case we need a SIMPLE nextObj. We randomly pick a port
+ if (simple) {
int size = devicePortMap.get(neighborId).size();
int index = RandomUtils.nextInt(0, size);
neighborPorts = Collections.singleton(
@@ -928,9 +976,14 @@
tBuilder.setEthDst(neighborMac).setEthSrc(nodeMacAddr);
int edgeLabel = ds.getEdgeLabel(dst);
if (edgeLabel != DestinationSet.NO_EDGE_LABEL) {
- tBuilder.pushMpls()
- .copyTtlOut()
- .setMpls(MplsLabel.mplsLabel(edgeLabel));
+ if (simple) {
+ // swap label case
+ tBuilder.setMpls(MplsLabel.mplsLabel(edgeLabel));
+ } else {
+ // ecmp with label push case
+ tBuilder.pushMpls().copyTtlOut()
+ .setMpls(MplsLabel.mplsLabel(edgeLabel));
+ }
}
tBuilder.setOutput(sp);
nextObjBuilder.addTreatment(tBuilder.build());
@@ -1395,8 +1448,11 @@
.stream()
.filter(entry -> entry.getKey().deviceId().equals(deviceId))
// Filter out PW transit groups or include them if MPLS ECMP is supported
- .filter(entry -> !entry.getKey().destinationSet().mplsSet() ||
- (entry.getKey().destinationSet().mplsSet() && srManager.getMplsEcmp()))
+ .filter(entry -> !entry.getKey().destinationSet().notBos() ||
+ (entry.getKey().destinationSet().notBos() && srManager.getMplsEcmp()))
+ // Filter out simple SWAP groups or include them if MPLS ECMP is supported
+ .filter(entry -> !entry.getKey().destinationSet().swap() ||
+ (entry.getKey().destinationSet().swap() && srManager.getMplsEcmp()))
.map(entry -> entry.getKey())
.collect(Collectors.toSet());
for (DestinationSetNextObjectiveStoreKey dsKey : dsKeySet) {
@@ -1434,8 +1490,11 @@
+ " port/label {}/{} to dst {} via {}",
deviceId, nid, port, edgeLabel,
dstDev, neighbor);
- nextObjBuilder.addTreatment(treatmentBuilder(port,
- neighborMac, edgeLabel));
+ nextObjBuilder
+ .addTreatment(treatmentBuilder(port,
+ neighborMac,
+ dsKey.destinationSet().swap(),
+ edgeLabel));
});
});
});
@@ -1450,16 +1509,22 @@
}
TrafficTreatment treatmentBuilder(PortNumber outport, MacAddress dstMac,
- int edgeLabel) {
+ boolean swap, int edgeLabel) {
TrafficTreatment.Builder tBuilder =
DefaultTrafficTreatment.builder();
tBuilder.setOutput(outport)
.setEthDst(dstMac)
.setEthSrc(nodeMacAddr);
if (edgeLabel != DestinationSet.NO_EDGE_LABEL) {
- tBuilder.pushMpls()
- .copyTtlOut()
- .setMpls(MplsLabel.mplsLabel(edgeLabel));
+ if (swap) {
+ // swap label case
+ tBuilder.setMpls(MplsLabel.mplsLabel(edgeLabel));
+ } else {
+ // ecmp with label push case
+ tBuilder.pushMpls()
+ .copyTtlOut()
+ .setMpls(MplsLabel.mplsLabel(edgeLabel));
+ }
}
return tBuilder.build();
}
diff --git a/apps/segmentrouting/app/src/main/java/org/onosproject/segmentrouting/grouphandler/DestinationSet.java b/apps/segmentrouting/app/src/main/java/org/onosproject/segmentrouting/grouphandler/DestinationSet.java
index 7f839cf..0626bc8 100644
--- a/apps/segmentrouting/app/src/main/java/org/onosproject/segmentrouting/grouphandler/DestinationSet.java
+++ b/apps/segmentrouting/app/src/main/java/org/onosproject/segmentrouting/grouphandler/DestinationSet.java
@@ -28,15 +28,17 @@
import static org.slf4j.LoggerFactory.getLogger;
/**
- * Representation of a set of destination switch dpids along with their edge-node
- * labels. Meant to be used as a lookup-key in a hash-map to retrieve an ECMP-group
- * that hashes packets towards a specific destination switch,
- * or paired-destination switches.
+ * Representation of a set of destination switch dpids along with their
+ * edge-node labels. Meant to be used as a lookup-key in a hash-map to retrieve
+ * an ECMP-group that hashes packets towards a specific destination switch, or
+ * paired-destination switches. May also be used to represent cases where the
+ * forwarding does not use ECMP groups (ie SIMPLE next objectives)
*/
public class DestinationSet {
public static final int NO_EDGE_LABEL = -1;
private static final int NOT_ASSIGNED = 0;
- private boolean mplsSet;
+ private boolean notBos; // XXX replace with enum
+ private boolean swap; // XXX replace with enum
private final DeviceId dstSw1;
private final int edgeLabel1;
private final DeviceId dstSw2;
@@ -48,12 +50,14 @@
/**
* Constructor for a single destination with no Edge label.
*
- * @param isMplsSet indicates if it is a mpls destination set
+ * @param isNotBos indicates if it is meant for a non bottom-of-stack label
+ * @param isSwap indicates if it is meant for a swap action
* @param dstSw the destination switch
*/
- public DestinationSet(boolean isMplsSet, DeviceId dstSw) {
+ public DestinationSet(boolean isNotBos, boolean isSwap, DeviceId dstSw) {
this.edgeLabel1 = NO_EDGE_LABEL;
- this.mplsSet = isMplsSet;
+ this.notBos = isNotBos;
+ this.swap = isSwap;
this.dstSw1 = dstSw;
this.edgeLabel2 = NOT_ASSIGNED;
this.dstSw2 = null;
@@ -62,13 +66,15 @@
/**
* Constructor for a single destination with Edge label.
*
- * @param isMplsSet indicates if it is a mpls destination set
+ * @param isNotBos indicates if it is meant for a non bottom-of-stack label
+ * @param isSwap indicates if it is meant for a swap action
* @param edgeLabel label to be pushed as part of group operation
* @param dstSw the destination switch
*/
- public DestinationSet(boolean isMplsSet,
+ public DestinationSet(boolean isNotBos, boolean isSwap,
int edgeLabel, DeviceId dstSw) {
- this.mplsSet = isMplsSet;
+ this.notBos = isNotBos;
+ this.swap = isSwap;
this.edgeLabel1 = edgeLabel;
this.dstSw1 = dstSw;
this.edgeLabel2 = NOT_ASSIGNED;
@@ -76,19 +82,23 @@
}
/**
- * Constructor for paired destination switches and their associated
- * edge labels.
+ * Constructor for paired destination switches and their associated edge
+ * labels.
*
- * @param isMplsSet indicates if it is a mpls destination set
- * @param edgeLabel1 label to be pushed as part of group operation for dstSw1
+ * @param isNotBos indicates if it is meant for a non bottom-of-stack label
+ * @param isSwap indicates if it is meant for a swap action
+ * @param edgeLabel1 label to be pushed as part of group operation for
+ * dstSw1
* @param dstSw1 one of the paired destination switches
- * @param edgeLabel2 label to be pushed as part of group operation for dstSw2
+ * @param edgeLabel2 label to be pushed as part of group operation for
+ * dstSw2
* @param dstSw2 the other paired destination switch
*/
- public DestinationSet(boolean isMplsSet,
+ public DestinationSet(boolean isNotBos, boolean isSwap,
int edgeLabel1, DeviceId dstSw1,
int edgeLabel2, DeviceId dstSw2) {
- this.mplsSet = isMplsSet;
+ this.notBos = isNotBos;
+ this.swap = isSwap;
if (dstSw1.toString().compareTo(dstSw2.toString()) <= 0) {
this.edgeLabel1 = edgeLabel1;
this.dstSw1 = dstSw1;
@@ -108,52 +118,13 @@
public DestinationSet() {
this.edgeLabel1 = NOT_ASSIGNED;
this.edgeLabel2 = NOT_ASSIGNED;
- this.mplsSet = true;
+ this.notBos = true;
+ this.swap = true;
this.dstSw1 = DeviceId.NONE;
this.dstSw2 = DeviceId.NONE;
}
/**
- * Factory method for DestinationSet hierarchy.
- *
- * @param random the expected behavior.
- * @param isMplsSet indicates if it is a mpls destination set
- * @param dstSw the destination switch
- * @return the destination set object.
- */
- public static DestinationSet destinationSet(boolean random,
- boolean isMplsSet, DeviceId dstSw) {
- return random ? new RandomDestinationSet(dstSw)
- : new DestinationSet(isMplsSet, dstSw);
- }
-
- /**
- * Factory method for DestinationSet hierarchy.
- *
- * @param random the expected behavior.
- * @param isMplsSet indicates if it is a mpls destination set
- * @param edgeLabel label to be pushed as part of group operation
- * @param dstSw the destination switch
- * @return the destination set object
- */
- public static DestinationSet destinationSet(boolean random,
- boolean isMplsSet, int edgeLabel,
- DeviceId dstSw) {
- return random ? new RandomDestinationSet(edgeLabel, dstSw)
- : new DestinationSet(isMplsSet, edgeLabel, dstSw);
- }
-
- /**
- * Factory method for DestinationSet hierarchy.
- *
- * @param random the expected behavior.
- * @return the destination set object
- */
- public static DestinationSet destinationSet(boolean random) {
- return random ? new RandomDestinationSet() : new DestinationSet();
- }
-
- /**
* Gets the label associated with given destination switch.
*
* @param dstSw the destination switch
@@ -183,12 +154,21 @@
}
/**
- * Gets the value of mplsSet.
+ * Gets the value of notBos.
*
- * @return the value of mplsSet
+ * @return the value of notBos
*/
- public boolean mplsSet() {
- return mplsSet;
+ public boolean notBos() {
+ return notBos;
+ }
+
+ /**
+ * Gets the value of swap.
+ *
+ * @return the value of swap
+ */
+ public boolean swap() {
+ return swap;
}
// The list of destination ids and label are used for comparison.
@@ -202,7 +182,8 @@
}
DestinationSet that = (DestinationSet) o;
boolean equal = (this.edgeLabel1 == that.edgeLabel1 &&
- this.mplsSet == that.mplsSet &&
+ this.notBos == that.notBos &&
+ this.swap == that.swap &&
this.dstSw1.equals(that.dstSw1));
if (this.dstSw2 != null && that.dstSw2 == null ||
this.dstSw2 == null && that.dstSw2 != null) {
@@ -219,15 +200,26 @@
@Override
public int hashCode() {
if (dstSw2 == null) {
- return Objects.hash(mplsSet, edgeLabel1, dstSw1);
+ return Objects.hash(notBos, swap, edgeLabel1, dstSw1);
}
- return Objects.hash(mplsSet, edgeLabel1, dstSw1, edgeLabel2, dstSw2);
+ return Objects.hash(notBos, swap, edgeLabel1, dstSw1, edgeLabel2,
+ dstSw2);
}
@Override
public String toString() {
+ String type;
+ if (!notBos && !swap) {
+ type = "default";
+ } else if (!notBos && swap) {
+ type = "swapbos";
+ } else if (notBos && !swap) {
+ type = "not-bos";
+ } else {
+ type = "swap-nb";
+ }
ToStringHelper h = toStringHelper(this)
- .add("MplsSet", mplsSet)
+ .add("Type", type)
.add("DstSw1", dstSw1)
.add("Label1", edgeLabel1);
if (dstSw2 != null) {
diff --git a/apps/segmentrouting/app/src/main/java/org/onosproject/segmentrouting/grouphandler/RandomDestinationSet.java b/apps/segmentrouting/app/src/main/java/org/onosproject/segmentrouting/grouphandler/RandomDestinationSet.java
deleted file mode 100644
index 3671b03..0000000
--- a/apps/segmentrouting/app/src/main/java/org/onosproject/segmentrouting/grouphandler/RandomDestinationSet.java
+++ /dev/null
@@ -1,60 +0,0 @@
-/*
- * Copyright 2016-present Open Networking Foundation
- *
- * Licensed under the Apache License, Version 2.0 (the "License");
- * you may not use this file except in compliance with the License.
- * You may obtain a copy of the License at
- *
- * http://www.apache.org/licenses/LICENSE-2.0
- *
- * Unless required by applicable law or agreed to in writing, software
- * distributed under the License is distributed on an "AS IS" BASIS,
- * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
- * See the License for the specific language governing permissions and
- * limitations under the License.
- */
-
-package org.onosproject.segmentrouting.grouphandler;
-
-import org.onosproject.net.DeviceId;
-
-/**
- * Extends its super class modifying its internal behavior.
- * Pick a neighbor will pick a random neighbor.
- */
-public class RandomDestinationSet extends DestinationSet {
-
- public RandomDestinationSet(DeviceId dstSw) {
- super(true, dstSw);
- }
-
- public RandomDestinationSet(int edgeLabel,
- DeviceId dstSw) {
- super(true, edgeLabel, dstSw);
- }
-
- public RandomDestinationSet() {
- super();
- }
-
- // XXX revisit the need for this class since neighbors no longer stored here
- // will be handled when we fix pseudowires for dual-Tor scenarios
-
-
- /*@Override
- public DeviceId getFirstNeighbor() {
- if (getDeviceIds().isEmpty()) {
- return DeviceId.NONE;
- }
- int size = getDeviceIds().size();
- int index = RandomUtils.nextInt(0, size);
- return Iterables.get(getDeviceIds(), index);
- }*/
-
- @Override
- public String toString() {
- return " RandomNeighborset Sw: " //+ getDeviceIds()
- + " and Label: " //+ getEdgeLabel()
- + " for destination: "; // + getDestinationSw();
- }
-}
diff --git a/apps/segmentrouting/app/src/main/java/org/onosproject/segmentrouting/mcast/McastHandler.java b/apps/segmentrouting/app/src/main/java/org/onosproject/segmentrouting/mcast/McastHandler.java
index cd59ee5..732d884 100644
--- a/apps/segmentrouting/app/src/main/java/org/onosproject/segmentrouting/mcast/McastHandler.java
+++ b/apps/segmentrouting/app/src/main/java/org/onosproject/segmentrouting/mcast/McastHandler.java
@@ -131,7 +131,7 @@
private boolean isMcastStable() {
long last = (long) (lastMcastChange.toEpochMilli() / 1000.0);
long now = (long) (Instant.now().toEpochMilli() / 1000.0);
- log.debug("Mcast stable since {}s", now - last);
+ log.trace("Mcast stable since {}s", now - last);
return (now - last) > MCAST_STABLITY_THRESHOLD;
}
diff --git a/apps/segmentrouting/app/src/test/java/org/onosproject/segmentrouting/grouphandler/DestinationSetTest.java b/apps/segmentrouting/app/src/test/java/org/onosproject/segmentrouting/grouphandler/DestinationSetTest.java
index f890852..6abf6e3 100644
--- a/apps/segmentrouting/app/src/test/java/org/onosproject/segmentrouting/grouphandler/DestinationSetTest.java
+++ b/apps/segmentrouting/app/src/test/java/org/onosproject/segmentrouting/grouphandler/DestinationSetTest.java
@@ -23,7 +23,7 @@
import static org.junit.Assert.assertTrue;
public class DestinationSetTest {
- DestinationSet ds1, ds2, ds3, ds4;
+ DestinationSet ds1, ds2, ds3, ds4, ds5, ds6;
DeviceId d201, d202;
int el201, el202;
@@ -33,30 +33,36 @@
d202 = DeviceId.deviceId("of:0000000000000202");
el201 = 201;
el202 = 202;
- ds1 = new DestinationSet(false, d201);
- ds2 = new DestinationSet(false, el201, d201);
- ds3 = new DestinationSet(false, el201, d201, el202, d202);
- ds4 = new DestinationSet(false,
+ ds1 = new DestinationSet(false, false, d201);
+ ds2 = new DestinationSet(false, false, el201, d201);
+ ds3 = new DestinationSet(false, false, el201, d201, el202, d202);
+ ds4 = new DestinationSet(false, false,
DestinationSet.NO_EDGE_LABEL, d201,
DestinationSet.NO_EDGE_LABEL, d202);
+ ds5 = new DestinationSet(true, false, d201); // not-bos
+ ds6 = new DestinationSet(false, true, el201, d201); // swap group
}
@Test
public void testIsValid() {
- assertTrue(!ds1.mplsSet());
+ assertTrue(!ds1.notBos());
+ assertTrue(!ds1.swap());
assertTrue(ds1.getEdgeLabel(d201) == DestinationSet.NO_EDGE_LABEL);
assertTrue(ds1.getDestinationSwitches().size() == 1);
- assertTrue(!ds2.mplsSet());
+ assertTrue(!ds2.notBos());
+ assertTrue(!ds2.swap());
assertTrue(ds2.getEdgeLabel(d201) == el201);
assertTrue(ds2.getDestinationSwitches().size() == 1);
- assertTrue(!ds3.mplsSet());
+ assertTrue(!ds3.notBos());
+ assertTrue(!ds3.swap());
assertTrue(ds3.getEdgeLabel(d201) == el201);
assertTrue(ds3.getEdgeLabel(d202) == el202);
assertTrue(ds3.getDestinationSwitches().size() == 2);
- assertTrue(!ds4.mplsSet());
+ assertTrue(!ds4.notBos());
+ assertTrue(!ds4.swap());
assertTrue(ds4.getEdgeLabel(d201) == DestinationSet.NO_EDGE_LABEL);
assertTrue(ds4.getEdgeLabel(d202) == DestinationSet.NO_EDGE_LABEL);
assertTrue(ds4.getDestinationSwitches().size() == 2);
@@ -65,85 +71,104 @@
assertFalse(ds1.equals(ds4));
assertFalse(ds3.equals(ds4));
assertFalse(ds2.equals(ds3));
+ assertFalse(ds1.equals(ds3));
+
+ assertFalse(ds1.equals(ds5));
+ assertFalse(ds2.equals(ds6));
}
@Test
public void testOneDestinationWithoutLabel() {
- DestinationSet testds = new DestinationSet(false, d201);
+ DestinationSet testds = new DestinationSet(false, false, d201);
assertTrue(testds.equals(ds1)); // match
- testds = new DestinationSet(true, d201);
- assertFalse(testds.equals(ds1)); //wrong mplsSet
+ testds = new DestinationSet(true, false, d201);
+ assertFalse(testds.equals(ds1)); // wrong notBos
+ assertTrue(testds.equals(ds5)); // correct notBos
- testds = new DestinationSet(false, d202);
+ testds = new DestinationSet(false, false, d202);
assertFalse(testds.equals(ds1)); //wrong device
- testds = new DestinationSet(false, el201, d201);
+ testds = new DestinationSet(false, false, el201, d201);
assertFalse(testds.equals(ds1)); // wrong label
- testds = new DestinationSet(false, -1, d201, -1, d202);
+ testds = new DestinationSet(false, false, -1, d201, -1, d202);
assertFalse(testds.equals(ds1)); // 2-devs should not match
+
+ testds = new DestinationSet(false, true, d201);
+ assertFalse(testds.equals(ds1)); // wrong swap
+ assertFalse(testds.equals(ds6)); // wrong label
+
+ testds = new DestinationSet(false, true, el201, d201);
+ assertTrue(testds.equals(ds6)); // correct swap
+
+ testds = new DestinationSet(false, true, DestinationSet.NO_EDGE_LABEL, d201);
+ assertFalse(testds.equals(ds6)); // wrong label
+
+ testds = new DestinationSet(true, true, el201, d201);
+ assertFalse(testds.equals(ds6)); // wrong notbos
}
@Test
public void testOneDestinationWithLabel() {
- DestinationSet testds = new DestinationSet(false, 203, d202);
+ DestinationSet testds = new DestinationSet(false, false, 203, d202);
assertFalse(testds.equals(ds2)); //wrong label
- testds = new DestinationSet(true, 201, d201);
- assertFalse(testds.equals(ds2)); //wrong mplsSet
+ testds = new DestinationSet(true, false, 201, d201);
+ assertFalse(testds.equals(ds2)); // wrong notBos
- testds = new DestinationSet(false, 201, d202);
+ testds = new DestinationSet(false, false, 201, d202);
assertFalse(testds.equals(ds2)); //wrong device
- testds = new DestinationSet(false, 201, DeviceId.deviceId("of:0000000000000201"));
+ testds = new DestinationSet(false, false, 201,
+ DeviceId.deviceId("of:0000000000000201"));
assertTrue(testds.equals(ds2)); // match
- testds = new DestinationSet(false, d201);
+ testds = new DestinationSet(false, false, d201);
assertFalse(testds.equals(ds2)); // wrong label
- testds = new DestinationSet(false, el201, d201, el202, d202);
+ testds = new DestinationSet(false, false, el201, d201, el202, d202);
assertFalse(testds.equals(ds1)); // 2-devs should not match
}
@Test
public void testDestPairWithLabel() {
- DestinationSet testds = new DestinationSet(false, el201, d201, el202, d202);
+ DestinationSet testds = new DestinationSet(false, false, el201, d201, el202, d202);
assertTrue(testds.equals(ds3)); // match same switches, same order
assertTrue(testds.hashCode() == ds3.hashCode());
- testds = new DestinationSet(false, el202, d202, el201, d201);
+ testds = new DestinationSet(false, false, el202, d202, el201, d201);
assertTrue(testds.equals(ds3)); // match same switches, order reversed
assertTrue(testds.hashCode() == ds3.hashCode());
- testds = new DestinationSet(false, el202, d202);
+ testds = new DestinationSet(false, false, el202, d202);
assertFalse(testds.equals(ds3)); // one less switch should not match
assertFalse(testds.hashCode() == ds3.hashCode());
- testds = new DestinationSet(false, el201, d201);
+ testds = new DestinationSet(false, false, el201, d201);
assertFalse(testds.equals(ds3)); // one less switch should not match
assertFalse(testds.hashCode() == ds3.hashCode());
- testds = new DestinationSet(false, el201, d201, 0, DeviceId.NONE);
+ testds = new DestinationSet(false, false, el201, d201, 0, DeviceId.NONE);
assertFalse(testds.equals(ds3)); // one less switch should not match
assertFalse(testds.hashCode() == ds3.hashCode());
- testds = new DestinationSet(false, el201, d202, el201, d201);
+ testds = new DestinationSet(false, false, el201, d202, el201, d201);
assertFalse(testds.equals(ds3)); // wrong labels
assertFalse(testds.hashCode() == ds3.hashCode());
- testds = new DestinationSet(true, el202, d202, el201, d201);
- assertFalse(testds.equals(ds3)); // wrong mpls set
+ testds = new DestinationSet(true, false, el202, d202, el201, d201);
+ assertFalse(testds.equals(ds3)); // wrong not bos
assertFalse(testds.hashCode() == ds3.hashCode());
- testds = new DestinationSet(false, el202, d202, el201, d202);
+ testds = new DestinationSet(false, false, el202, d202, el201, d202);
assertFalse(testds.equals(ds3)); // wrong device
assertFalse(testds.hashCode() == ds3.hashCode());
- testds = new DestinationSet(false,
+ testds = new DestinationSet(false, false,
el202, DeviceId.deviceId("of:0000000000000205"),
el201, d201);
assertFalse(testds.equals(ds3)); // wrong device
@@ -152,39 +177,39 @@
@Test
public void testDestPairWithoutLabel() {
- DestinationSet testds = new DestinationSet(false, -1, d201, -1, d202);
+ DestinationSet testds = new DestinationSet(false, false, -1, d201, -1, d202);
assertTrue(testds.equals(ds4)); // match same switches, same order
assertTrue(testds.hashCode() == ds4.hashCode());
- testds = new DestinationSet(false, -1, d202, -1, d201);
+ testds = new DestinationSet(false, false, -1, d202, -1, d201);
assertTrue(testds.equals(ds4)); // match same switches, order reversed
assertTrue(testds.hashCode() == ds4.hashCode());
- testds = new DestinationSet(false, -1, d202);
+ testds = new DestinationSet(false, false, -1, d202);
assertFalse(testds.equals(ds4)); // one less switch should not match
assertFalse(testds.hashCode() == ds4.hashCode());
- testds = new DestinationSet(false, -1, d201);
+ testds = new DestinationSet(false, false, -1, d201);
assertFalse(testds.equals(ds4)); // one less switch should not match
assertFalse(testds.hashCode() == ds4.hashCode());
- testds = new DestinationSet(false, -1, d201, 0, DeviceId.NONE);
+ testds = new DestinationSet(false, false, -1, d201, 0, DeviceId.NONE);
assertFalse(testds.equals(ds4)); // one less switch should not match
assertFalse(testds.hashCode() == ds4.hashCode());
- testds = new DestinationSet(false, el201, d201, -1, d202);
+ testds = new DestinationSet(false, false, el201, d201, -1, d202);
assertFalse(testds.equals(ds4)); // wrong labels
assertFalse(testds.hashCode() == ds4.hashCode());
- testds = new DestinationSet(true, -1, d202, -1, d201);
+ testds = new DestinationSet(true, false, -1, d202, -1, d201);
assertFalse(testds.equals(ds4)); // wrong mpls set
assertFalse(testds.hashCode() == ds4.hashCode());
- testds = new DestinationSet(false, -1, d202, -1, d202);
+ testds = new DestinationSet(false, false, -1, d202, -1, d202);
assertFalse(testds.equals(ds4)); // wrong device
assertFalse(testds.hashCode() == ds4.hashCode());
- testds = new DestinationSet(false,
+ testds = new DestinationSet(false, false,
-1, DeviceId.deviceId("of:0000000000000205"),
-1, d201);
assertFalse(testds.equals(ds4)); // wrong device
diff --git a/drivers/default/src/main/java/org/onosproject/driver/pipeline/ofdpa/CpqdOfdpa2GroupHandler.java b/drivers/default/src/main/java/org/onosproject/driver/pipeline/ofdpa/CpqdOfdpa2GroupHandler.java
index 401e924..078e120 100644
--- a/drivers/default/src/main/java/org/onosproject/driver/pipeline/ofdpa/CpqdOfdpa2GroupHandler.java
+++ b/drivers/default/src/main/java/org/onosproject/driver/pipeline/ofdpa/CpqdOfdpa2GroupHandler.java
@@ -99,8 +99,9 @@
portNum = ((Instructions.OutputInstruction) ins).port().toLong();
innerTtb.add(ins);
} else {
- log.warn("Driver does not handle this type of TrafficTreatment"
- + " instruction in nextObjectives: {}", ins.type());
+ log.debug("Driver does not handle this type of TrafficTreatment"
+ + " instruction in l2l3chain: {} - {}", ins.type(),
+ ins);
}
}
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 fdee8d6..2c2f3ba 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
@@ -245,14 +245,14 @@
/**
* As per the OFDPA 2.0 TTP, packets are sent out of ports by using
- * a chain of groups. The simple Next Objective passed
- * in by the application has to be broken up into a group chain
- * comprising of an L3 Unicast Group that points to an L2 Interface
- * Group which in-turn points to an output port or an MPLS Interface Group
- * that points to an L2 Interface Group. In some cases, the simple
- * next Objective can just be an L2 interface without the need for chaining.
- * Further, if the label is set to the Next objective then the Group chain
- * MPLS Swap - MPLS Interface - L2 Interface is created
+ * a chain of groups. The simple Next Objective passed in by the application
+ * is broken up into a group chain. The following chains can be created
+ * depending on the parameters in the Next Objective.
+ * 1. L2 Interface group (no chaining)
+ * 2. L3 Unicast group -> L2 Interface group
+ * 3. MPLS Interface group -> L2 Interface group
+ * 4. MPLS Swap group -> MPLS Interface group -> L2 Interface group
+ * 5. PW initiation group chain
*
* @param nextObj the nextObjective of type SIMPLE
*/
@@ -275,7 +275,6 @@
mplsSwap = true;
mplsLabel = ((L2ModificationInstruction.ModMplsLabelInstruction) l2ins).label();
}
-
}
}
@@ -284,24 +283,20 @@
return;
}
- boolean isMpls = false;
- // In order to understand if it is a pseudo wire related
+ // In order to understand if it is a pseudowire related
// next objective we look for the tunnel id in the meta.
boolean isPw = false;
if (nextObj.meta() != null) {
- isMpls = isNotMplsBos(nextObj.meta());
-
TunnelIdCriterion tunnelIdCriterion = (TunnelIdCriterion) nextObj
.meta()
.getCriterion(TUNNEL_ID);
if (tunnelIdCriterion != null) {
isPw = true;
}
-
}
if (mplsSwap && !isPw) {
- log.debug("Creating a MPLS Swap - MPLS Interface - L2 Interface group chain.");
+ log.debug("Creating a MPLS Swap -> MPLS Interface -> L2 Interface group chain.");
// break up simple next objective to GroupChain objects
GroupInfo groupInfo = createL2L3Chain(treatment, nextObj.id(),
@@ -314,38 +309,44 @@
}
Deque<GroupKey> gkeyChain = new ArrayDeque<>();
- gkeyChain.addFirst(groupInfo.innerMostGroupDesc().appCookie());
- gkeyChain.addFirst(groupInfo.nextGroupDesc().appCookie());
+ gkeyChain.addFirst(groupInfo.innerMostGroupDesc().appCookie()); // l2 interface
+ gkeyChain.addFirst(groupInfo.nextGroupDesc().appCookie()); // mpls interface
// creating the mpls swap group and adding it to the chain
- GroupChainElem groupChainElem;
- GroupKey groupKey;
- GroupDescription groupDescription;
int nextGid = groupInfo.nextGroupDesc().givenGroupId();
int index = getNextAvailableIndex();
- groupDescription = createMplsSwap(
+ GroupDescription swapGroupDescription = createMplsSwap(
nextGid,
OfdpaMplsGroupSubType.MPLS_SWAP_LABEL,
index,
mplsLabel,
nextObj.appId()
);
+ // ensure swap group is added after L2L3 chain
+ GroupKey swapGroupKey = swapGroupDescription.appCookie();
+ GroupChainElem swapChainElem = new GroupChainElem(swapGroupDescription,
+ 1, false, deviceId);
+ updatePendingGroups(groupInfo.nextGroupDesc().appCookie(), swapChainElem);
+ gkeyChain.addFirst(swapGroupKey);
- groupKey = new DefaultGroupKey(Ofdpa2Pipeline.appKryo.serialize(index));
- groupChainElem = new GroupChainElem(groupDescription, 1, false, deviceId);
- updatePendingGroups(groupInfo.nextGroupDesc().appCookie(), groupChainElem);
- gkeyChain.addFirst(groupKey);
-
- // create a new List from singletonList that is mutable
- OfdpaNextGroup ofdpaGrp = new OfdpaNextGroup(Collections.singletonList(gkeyChain), nextObj);
- updatePendingNextObjective(groupInfo.nextGroupDesc().appCookie(), ofdpaGrp);
+ // ensure nextObjective waits on the outermost groupKey
+ List<Deque<GroupKey>> allGroupKeys = Lists.newArrayList();
+ allGroupKeys.add(gkeyChain);
+ OfdpaNextGroup ofdpaGrp = new OfdpaNextGroup(allGroupKeys, nextObj);
+ updatePendingNextObjective(swapGroupKey, ofdpaGrp);
// now we are ready to send the l2 groupDescription (inner), as all the stores
// that will get async replies have been updated. By waiting to update
// the stores, we prevent nasty race conditions.
groupService.addGroup(groupInfo.innerMostGroupDesc());
} else if (!isPw) {
+ boolean isMpls = false;
+ if (nextObj.meta() != null) {
+ isMpls = isNotMplsBos(nextObj.meta());
+ }
+ log.debug("Creating a {} -> L2 Interface group chain.",
+ (isMpls) ? "MPLS Interface" : "L3 Unicast");
// break up simple next objective to GroupChain objects
GroupInfo groupInfo = createL2L3Chain(treatment, nextObj.id(),
nextObj.appId(), isMpls,
@@ -359,8 +360,9 @@
Deque<GroupKey> gkeyChain = new ArrayDeque<>();
gkeyChain.addFirst(groupInfo.innerMostGroupDesc().appCookie());
gkeyChain.addFirst(groupInfo.nextGroupDesc().appCookie());
- OfdpaNextGroup ofdpaGrp =
- new OfdpaNextGroup(Collections.singletonList(gkeyChain), nextObj);
+ List<Deque<GroupKey>> allGroupKeys = Lists.newArrayList();
+ allGroupKeys.add(gkeyChain);
+ OfdpaNextGroup ofdpaGrp = new OfdpaNextGroup(allGroupKeys, nextObj);
// store l3groupkey with the ofdpaNextGroup for the nextObjective that depends on it
updatePendingNextObjective(groupInfo.nextGroupDesc().appCookie(), ofdpaGrp);
@@ -424,9 +426,7 @@
int index,
MplsLabel mplsLabel,
ApplicationId applicationId) {
-
TrafficTreatment.Builder treatment = DefaultTrafficTreatment.builder();
-
treatment.setMpls(mplsLabel);
// We point the group to the next group.
@@ -436,16 +436,14 @@
// Finally we build the group description.
int groupId = makeMplsLabelGroupId(subtype, index);
GroupKey groupKey = new DefaultGroupKey(
- Ofdpa2Pipeline.appKryo.serialize(index)
- );
+ Ofdpa2Pipeline.appKryo.serialize(index));
return new DefaultGroupDescription(
deviceId,
INDIRECT,
new GroupBuckets(Collections.singletonList(groupBucket)),
groupKey,
groupId,
- applicationId
- );
+ applicationId);
}
/**
@@ -542,7 +540,8 @@
innerTtb.add(ins);
} else {
log.debug("Driver does not handle this type of TrafficTreatment"
- + " instruction in nextObjectives: {}", ins.type());
+ + " instruction in l2l3chain: {} - {}", ins.type(),
+ ins);
}
}
@@ -963,7 +962,7 @@
}
Deque<GroupKey> gKeyChain = new ArrayDeque<>();
- // XXX we only deal with 0 and 1 label push right now
+ // here we only deal with 0 and 1 label push
if (labelsPushed == 0) {
GroupInfo noLabelGroupInfo;
TrafficSelector metaSelector = nextObj.meta();
@@ -1661,12 +1660,17 @@
// Detect situation where the next data has more buckets
// (not duplicates) respect to the next objective
- if (allActiveKeys.size() > nextObjective.next().size()) {
+ if (allActiveKeys.size() > nextObjective.next().size() &&
+ // ignore specific case of empty group
+ !(nextObjective.next().size() == 0 && allActiveKeys.size() == 1
+ && allActiveKeys.get(0).size() == 1)) {
log.warn("Mismatch detected between next and flowobjstore for device {}: " +
- "nextId:{}, nextObjective-size:{} next-size:{} .. correcting",
- deviceId, nextObjective.id(), nextObjective.next().size(), allActiveKeys.size());
- List<Integer> otherIndices = indicesToRemoveFromNextGroup(allActiveKeys, nextObjective,
- groupService, deviceId);
+ "nextId:{}, nextObjective-size:{} next-size:{} .. correcting",
+ deviceId, nextObjective.id(), nextObjective.next().size(),
+ allActiveKeys.size());
+ List<Integer> otherIndices =
+ indicesToRemoveFromNextGroup(allActiveKeys, nextObjective,
+ groupService, deviceId);
// Filter out the indices not present
otherIndices = otherIndices.stream()
.filter(index -> !indicesToRemove.contains(index))
diff --git a/drivers/default/src/main/java/org/onosproject/driver/pipeline/ofdpa/Ofdpa2Pipeline.java b/drivers/default/src/main/java/org/onosproject/driver/pipeline/ofdpa/Ofdpa2Pipeline.java
index b481c3f..0dc8b18 100644
--- a/drivers/default/src/main/java/org/onosproject/driver/pipeline/ofdpa/Ofdpa2Pipeline.java
+++ b/drivers/default/src/main/java/org/onosproject/driver/pipeline/ofdpa/Ofdpa2Pipeline.java
@@ -1164,7 +1164,6 @@
TrafficSelector selector = fwd.selector();
EthTypeCriterion ethType =
(EthTypeCriterion) selector.getCriterion(Criterion.Type.ETH_TYPE);
- boolean popMpls = false;
boolean emptyGroup = false;
int forTableId;
TrafficSelector.Builder filteredSelector = DefaultTrafficSelector.builder();
@@ -1233,7 +1232,6 @@
for (Instruction instr : fwd.treatment().allInstructions()) {
if (instr instanceof L2ModificationInstruction &&
((L2ModificationInstruction) instr).subtype() == L2SubType.MPLS_POP) {
- popMpls = true;
// OF-DPA does not pop in MPLS table in some cases. For the L3 VPN, it requires
// setting the MPLS_TYPE so pop can happen down the pipeline
if (requireMplsPop()) {
@@ -1266,35 +1264,24 @@
}
if (fwd.nextId() != null) {
- if (forTableId == MPLS_TABLE_1 && !popMpls) {
- log.warn("SR CONTINUE case cannot be handled as MPLS ECMP "
- + "is not implemented in OF-DPA yet. Aborting this flow {} -> next:{}"
- + "in this device {}", fwd.id(), fwd.nextId(), deviceId);
- // XXX We could convert to forwarding to a single-port, via a MPLS interface,
- // or a MPLS SWAP (with-same) but that would have to be handled in the next-objective.
- // Also the pop-mpls logic used here won't work in non-BoS case.
- fail(fwd, ObjectiveError.FLOWINSTALLATIONFAILED);
- return Collections.emptySet();
- }
-
NextGroup next = getGroupForNextObjective(fwd.nextId());
if (next != null) {
List<Deque<GroupKey>> gkeys = appKryo.deserialize(next.data());
// we only need the top level group's key to point the flow to it
Group group = groupService.getGroup(deviceId, gkeys.get(0).peekFirst());
- if (isNotMplsBos(selector) && group.type().equals(SELECT)) {
- log.warn("SR CONTINUE case cannot be handled as MPLS ECMP "
- + "is not implemented in OF-DPA yet. Aborting this flow {} -> next:{}"
- + "in this device {}", fwd.id(), fwd.nextId(), deviceId);
- fail(fwd, ObjectiveError.FLOWINSTALLATIONFAILED);
- return Collections.emptySet();
- }
if (group == null) {
log.warn("Group with key:{} for next-id:{} not found in dev:{}",
gkeys.get(0).peekFirst(), fwd.nextId(), deviceId);
fail(fwd, ObjectiveError.GROUPMISSING);
return Collections.emptySet();
}
+ if (isNotMplsBos(selector) && group.type().equals(SELECT)) {
+ log.warn("SR CONTINUE case cannot be handled as MPLS ECMP "
+ + "is not implemented in OF-DPA yet. Aborting flow {} -> next:{} "
+ + "in this device {}", fwd.id(), fwd.nextId(), deviceId);
+ fail(fwd, ObjectiveError.FLOWINSTALLATIONFAILED);
+ return Collections.emptySet();
+ }
tb.deferred().group(group.id());
// retrying flows may be necessary due to bug CORD-554
if (gkeys.size() == 1 && gkeys.get(0).size() == 1) {
@@ -1639,16 +1626,53 @@
return mappings;
}
+ /**
+ * Returns true iff the given selector matches on BOS==true, indicating that
+ * the selector is trying to match on a label that is bottom-of-stack.
+ *
+ * @param selector the given match
+ * @return true iff BoS==true; false if BOS==false, or BOS matching is not
+ * expressed in the given selector
+ */
static boolean isMplsBos(TrafficSelector selector) {
MplsBosCriterion bosCriterion = (MplsBosCriterion) selector.getCriterion(MPLS_BOS);
return bosCriterion != null && bosCriterion.mplsBos();
}
+ /**
+ * Returns true iff the given selector matches on BOS==false, indicating
+ * that the selector is trying to match on a label that is not the
+ * bottom-of-stack label.
+ *
+ * @param selector the given match
+ * @return true iff BoS==false;
+ * false if BOS==true, or BOS matching is not expressed in the given selector
+ */
static boolean isNotMplsBos(TrafficSelector selector) {
MplsBosCriterion bosCriterion = (MplsBosCriterion) selector.getCriterion(MPLS_BOS);
return bosCriterion != null && !bosCriterion.mplsBos();
}
+ /**
+ * Returns true iff the forwarding objective includes a treatment to pop the
+ * MPLS label.
+ *
+ * @param fwd the given forwarding objective
+ * @return true iff mpls pop treatment exists
+ */
+ static boolean isMplsPop(ForwardingObjective fwd) {
+ if (fwd.treatment() != null) {
+ for (Instruction instr : fwd.treatment().allInstructions()) {
+ if (instr instanceof L2ModificationInstruction
+ && ((L2ModificationInstruction) instr)
+ .subtype() == L2SubType.MPLS_POP) {
+ return true;
+ }
+ }
+ }
+ return false;
+ }
+
private static boolean isIpv6(TrafficSelector selector) {
EthTypeCriterion ethTypeCriterion = (EthTypeCriterion) selector.getCriterion(ETH_TYPE);
return ethTypeCriterion != null && ethTypeCriterion.ethType().toShort() == Ethernet.TYPE_IPV6;
diff --git a/drivers/default/src/main/java/org/onosproject/driver/pipeline/ofdpa/Ofdpa3GroupHandler.java b/drivers/default/src/main/java/org/onosproject/driver/pipeline/ofdpa/Ofdpa3GroupHandler.java
index 99c6c61..e0e5099 100644
--- a/drivers/default/src/main/java/org/onosproject/driver/pipeline/ofdpa/Ofdpa3GroupHandler.java
+++ b/drivers/default/src/main/java/org/onosproject/driver/pipeline/ofdpa/Ofdpa3GroupHandler.java
@@ -342,9 +342,8 @@
mplsTreatment.add(ins);
break;
default:
- log.warn("Driver does not handle this type of TrafficTreatment"
- + " instruction in nextObjectives: {} - {}",
- ins.type(), ins);
+ log.warn("Driver does not handle TrafficTreatment"
+ + " L2Mod {} for pw next-obj", l2ins.subtype());
break;
}
} else if (ins.type() == Instruction.Type.OUTPUT) {
@@ -358,14 +357,13 @@
mplsTreatment.add(ins);
break;
default:
- log.warn("Driver does not handle this type of TrafficTreatment"
- + " instruction in nextObjectives: {} - {}",
- ins.type(), ins);
+ log.warn("Driver does not handle TrafficTreatment"
+ + " L3Mod for pw next-obj", l3ins.subtype());
}
} else {
log.warn("Driver does not handle this type of TrafficTreatment"
- + " instruction in nextObjectives: {} - {}",
+ + " instruction for pw next-obj: {} - {}",
ins.type(), ins);
}
}
diff --git a/drivers/default/src/main/java/org/onosproject/driver/pipeline/ofdpa/Ofdpa3Pipeline.java b/drivers/default/src/main/java/org/onosproject/driver/pipeline/ofdpa/Ofdpa3Pipeline.java
index 0059b64..7ea79bd 100644
--- a/drivers/default/src/main/java/org/onosproject/driver/pipeline/ofdpa/Ofdpa3Pipeline.java
+++ b/drivers/default/src/main/java/org/onosproject/driver/pipeline/ofdpa/Ofdpa3Pipeline.java
@@ -386,9 +386,13 @@
@Override
protected Collection<FlowRule> processEthTypeSpecific(ForwardingObjective fwd) {
- if (isNotMplsBos(fwd.selector())) {
+ // if its not-bos, we go to MPLS_TYPE_TABLE regardless of whether we pop or swap
+ // if it is bos, we go to MPLS_TYPE_TABLE only if we swap
+ if (isNotMplsBos(fwd.selector())
+ || (isMplsBos(fwd.selector()) && !isMplsPop(fwd))) {
return processEthTypeSpecificInternal(fwd, true, MPLS_TYPE_TABLE);
}
+ // if it is bos, and we pop, we go to MPLS_L3_TYPE_TABLE
return processEthTypeSpecificInternal(fwd, true, MPLS_L3_TYPE_TABLE);
}