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/app/src/main/java/org/onosproject/segmentrouting/grouphandler/DefaultGroupHandler.java b/app/src/main/java/org/onosproject/segmentrouting/grouphandler/DefaultGroupHandler.java
index 3cf1331..1e0e6fd 100644
--- a/app/src/main/java/org/onosproject/segmentrouting/grouphandler/DefaultGroupHandler.java
+++ b/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/app/src/main/java/org/onosproject/segmentrouting/grouphandler/DestinationSet.java b/app/src/main/java/org/onosproject/segmentrouting/grouphandler/DestinationSet.java
index 7f839cf..0626bc8 100644
--- a/app/src/main/java/org/onosproject/segmentrouting/grouphandler/DestinationSet.java
+++ b/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/app/src/main/java/org/onosproject/segmentrouting/grouphandler/RandomDestinationSet.java b/app/src/main/java/org/onosproject/segmentrouting/grouphandler/RandomDestinationSet.java
deleted file mode 100644
index 3671b03..0000000
--- a/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();
- }
-}