From cfcf4c6d9d81ad607513761b5077eb6ffb0ae917 Mon Sep 17 00:00:00 2001
From: Srinivasan Ramasubramanian <srini@bigswitch.com>
Date: Sun, 24 Jun 2012 07:49:37 -0700
Subject: [PATCH] Refactoring Route class and getRoute() methods to maintain
and provide a sequence of switch-ports, rather than links. Update unit tests
accordingly.
---
.../forwarding/Forwarding.java | 10 +-
.../routing/ForwardingBase.java | 177 +++++++-----------
.../floodlightcontroller/routing/Route.java | 64 ++-----
.../topology/TopologyInstance.java | 37 +++-
.../forwarding/ForwardingTest.java | 20 +-
.../routing/RouteTest.java | 28 +--
6 files changed, 151 insertions(+), 185 deletions(-)
diff --git a/src/main/java/net/floodlightcontroller/forwarding/Forwarding.java b/src/main/java/net/floodlightcontroller/forwarding/Forwarding.java
index e8aeeca05..59c4e3925 100644
--- a/src/main/java/net/floodlightcontroller/forwarding/Forwarding.java
+++ b/src/main/java/net/floodlightcontroller/forwarding/Forwarding.java
@@ -160,8 +160,10 @@ public class Forwarding extends ForwardingBase implements IFloodlightModule {
(srcCluster != null) &&
(dstCluster != null)) {
Route route =
- routingEngine.getRoute(srcDap.getSwitchDPID(),
- dstDap.getSwitchDPID());
+ routingEngine.getRoute(srcDap.getSwitchDPID(),
+ (short)srcDap.getPort(),
+ dstDap.getSwitchDPID(),
+ (short)dstDap.getPort());
if ((route != null) || validLocalHop(srcDap, dstDap)) {
int bufferId = OFPacketOut.BUFFER_ID_NONE;
if (log.isTraceEnabled()) {
@@ -174,8 +176,8 @@ public class Forwarding extends ForwardingBase implements IFloodlightModule {
long cookie =
AppCookie.makeCookie(FORWARDING_APP_ID, 0);
pushRoute(route, match, 0,
- srcDap, dstDap, bufferId,
- sw, pi, cookie, cntx,
+ bufferId,
+ pi, sw.getId(), cookie, cntx,
requestFlowRemovedNotifn, false,
OFFlowMod.OFPFC_ADD);
}
diff --git a/src/main/java/net/floodlightcontroller/routing/ForwardingBase.java b/src/main/java/net/floodlightcontroller/routing/ForwardingBase.java
index a0158bd5c..595c1ef86 100644
--- a/src/main/java/net/floodlightcontroller/routing/ForwardingBase.java
+++ b/src/main/java/net/floodlightcontroller/routing/ForwardingBase.java
@@ -23,7 +23,6 @@ import java.util.Comparator;
import java.util.HashSet;
import java.util.Iterator;
import java.util.List;
-import java.util.Map;
import net.floodlightcontroller.core.FloodlightContext;
import net.floodlightcontroller.core.IFloodlightProviderService;
@@ -38,9 +37,9 @@ import net.floodlightcontroller.devicemanager.SwitchPort;
import net.floodlightcontroller.packet.Ethernet;
import net.floodlightcontroller.routing.IRoutingService;
import net.floodlightcontroller.routing.IRoutingDecision;
-import net.floodlightcontroller.routing.Link;
import net.floodlightcontroller.routing.Route;
import net.floodlightcontroller.topology.ITopologyService;
+import net.floodlightcontroller.topology.NodePortTuple;
import net.floodlightcontroller.util.TimedCache;
import org.openflow.protocol.OFFlowMod;
@@ -52,7 +51,6 @@ import org.openflow.protocol.OFPort;
import org.openflow.protocol.OFType;
import org.openflow.protocol.action.OFAction;
import org.openflow.protocol.action.OFActionOutput;
-import org.openflow.util.HexString;
import org.openflow.util.U16;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
@@ -168,9 +166,10 @@ public abstract class ForwardingBase implements
*/
public boolean pushRoute(Route route, OFMatch match,
Integer wildcard_hints,
- SwitchPort srcSwPort,
- SwitchPort dstSwPort, int bufferId,
- IOFSwitch srcSwitch, OFPacketIn pi, long cookie,
+ int bufferId,
+ OFPacketIn pi,
+ long pinSwitch,
+ long cookie,
FloodlightContext cntx,
boolean reqeustFlowRemovedNotifn,
boolean doFlush,
@@ -183,6 +182,7 @@ public abstract class ForwardingBase implements
OFActionOutput action = new OFActionOutput();
List<OFAction> actions = new ArrayList<OFAction>();
actions.add(action);
+
fm.setIdleTimeout((short)5)
.setBufferId(OFPacketOut.BUFFER_ID_NONE)
.setCookie(cookie)
@@ -191,116 +191,79 @@ public abstract class ForwardingBase implements
.setActions(actions)
.setLengthU(OFFlowMod.MINIMUM_LENGTH+OFActionOutput.MINIMUM_LENGTH);
- Map<Long, IOFSwitch> switches = floodlightProvider.getSwitches();
- IOFSwitch sw = switches.get(dstSwPort.getSwitchDPID());
- ((OFActionOutput)fm.getActions().get(0)).
- setPort((short)dstSwPort.getPort());
-
- if (route != null) {
- for (int routeIndx = route.getPath().size() - 1; routeIndx >= 0; --routeIndx) {
- Link link = route.getPath().get(routeIndx);
- fm.setMatch(wildcard(match, sw, wildcard_hints));
- fm.getMatch().setInputPort(link.getDstPort());
- try {
- counterStore.updatePktOutFMCounterStore(sw, fm);
- if (log.isTraceEnabled()) {
- log.trace("Pushing Route flowmod routeIndx={} " +
- "sw={} inPort={} outPort={}",
- new Object[] {routeIndx,
- sw,
- fm.getMatch().getInputPort(),
- ((OFActionOutput)
- fm.getActions()
- .get(0)).getPort() });
- }
- sw.write(fm, cntx);
- if (doFlush) {
- sw.flush();
- }
-
- // Push the packet out the source switch
- if (sw.getId() == srcSwitch.getId()) {
- pushPacket(srcSwitch,
- match,
- pi,
- ((OFActionOutput) fm.getActions().get(0)).getPort(),
- cntx);
- srcSwitchIncluded = true;
- }
- } catch (IOException e) {
- log.error("Failure writing flow mod", e);
- }
- try {
- fm = fm.clone();
- } catch (CloneNotSupportedException e) {
- log.error("Failure cloning flow mod", e);
- }
+ List<NodePortTuple> switchPortList = route.getPath();
- // setup for the next loop iteration
- ((OFActionOutput)fm.getActions().get(0)).setPort(link.getSrcPort());
- if (routeIndx > 0) {
- sw =
- floodlightProvider.getSwitches()
- .get(route.getPath()
- .get(routeIndx - 1)
- .getDst());
- } else {
- sw =
- floodlightProvider.getSwitches()
- .get(route.getId().getSrc());
+ for (int indx = switchPortList.size()-1; indx > 0; indx -= 2) {
+ // indx and indx-1 will always have the same switch DPID.
+ long switchDPID = switchPortList.get(indx).getNodeId();
+ IOFSwitch sw = floodlightProvider.getSwitches().get(switchDPID);
+ if (sw == null) {
+ if (log.isWarnEnabled()) {
+ log.warn("Unable to push route, switch at DPID {} " +
+ "not available", switchDPID);
}
- if (sw == null) {
- if (log.isWarnEnabled()) {
- log.warn("Unable to push route, switch at DPID {} not available",
- (routeIndx > 0)
- ? HexString.toHexString(route.getPath()
- .get(routeIndx - 1)
- .getDst())
- : HexString.toHexString(route.getId()
- .getSrc()));
- }
- return srcSwitchIncluded;
+ return srcSwitchIncluded;
+ }
+
+ // set the match.
+ fm.setMatch(wildcard(match, sw, wildcard_hints));
+
+ // set buffer id if it is the source switch
+ if (1 == indx) {
+ //fm.setMatch(match);
+ fm.setBufferId(bufferId);
+ //fm.setMatch(wildcard(match, sw, wildcard_hints));
+ // Set the flag to request flow-mod removal notifications only for the
+ // source switch. The removal message is used to maintain the flow
+ // cache. Don't set the flag for ARP messages - TODO generalize check
+ if ((reqeustFlowRemovedNotifn)
+ && (match.getDataLayerType() != Ethernet.TYPE_ARP)) {
+ fm.setFlags(OFFlowMod.OFPFF_SEND_FLOW_REM);
+ match.setWildcards(fm.getMatch().getWildcards());
}
}
- }
- // set the original match for the first switch, and buffer id
- fm.setMatch(match);
- fm.setBufferId(bufferId);
- fm.setMatch(wildcard(match, sw, wildcard_hints));
- fm.getMatch().setInputPort((short) srcSwPort.getPort());
- // Set the flag to request flow-mod removal notifications only for the
- // source switch. The removal message is used to maintain the flow
- // cache. Don't set the flag for ARP messages - TODO generalize check
- if ((reqeustFlowRemovedNotifn)
- && (match.getDataLayerType() != Ethernet.TYPE_ARP)) {
- fm.setFlags(OFFlowMod.OFPFF_SEND_FLOW_REM);
- match.setWildcards(fm.getMatch().getWildcards());
- }
+ short outPort = switchPortList.get(indx).getPortId();
+ short inPort = switchPortList.get(indx-1).getPortId();
+ // set input and output ports on the switch
+ fm.getMatch().setInputPort(inPort);
+ ((OFActionOutput)fm.getActions().get(0)).setPort(outPort);
- try {
- counterStore.updatePktOutFMCounterStore(sw, fm);
- if (log.isDebugEnabled()) {
- log.debug("pushRoute flowmod sw={} inPort={} outPort={}",
- new Object[] { sw, fm.getMatch().getInputPort(),
- ((OFActionOutput)fm.getActions().get(0)).getPort() });
- }
- sw.write(fm, cntx);
- if (doFlush) {
- sw.flush();
+ try {
+ counterStore.updatePktOutFMCounterStore(sw, fm);
+ if (log.isTraceEnabled()) {
+ log.trace("Pushing Route flowmod routeIndx={} " +
+ "sw={} inPort={} outPort={}",
+ new Object[] {indx,
+ sw,
+ fm.getMatch().getInputPort(),
+ ((OFActionOutput)
+ fm.getActions()
+ .get(0)).getPort() });
+ }
+ sw.write(fm, cntx);
+ if (doFlush) {
+ sw.flush();
+ }
+
+ // Push the packet out the source switch
+ if (sw.getId() == pinSwitch) {
+ pushPacket(sw,
+ match,
+ pi,
+ ((OFActionOutput) fm.getActions().get(0)).getPort(),
+ cntx);
+ srcSwitchIncluded = true;
+ }
+ } catch (IOException e) {
+ log.error("Failure writing flow mod", e);
}
- if (sw.getId() == srcSwitch.getId()) {
- pushPacket(srcSwitch,
- match,
- pi,
- ((OFActionOutput) fm.getActions().get(0)).getPort(),
- cntx);
- srcSwitchIncluded = true;
+ try {
+ fm = fm.clone();
+ } catch (CloneNotSupportedException e) {
+ log.error("Failure cloning flow mod", e);
}
- match = fm.getMatch();
- } catch (IOException e) {
- log.error("Failure writing flow mod", e);
}
return srcSwitchIncluded;
diff --git a/src/main/java/net/floodlightcontroller/routing/Route.java b/src/main/java/net/floodlightcontroller/routing/Route.java
index 865ec97f1..211a924f7 100755
--- a/src/main/java/net/floodlightcontroller/routing/Route.java
+++ b/src/main/java/net/floodlightcontroller/routing/Route.java
@@ -20,6 +20,8 @@ package net.floodlightcontroller.routing;
import java.util.ArrayList;
import java.util.List;
+import net.floodlightcontroller.topology.NodePortTuple;
+
/**
* Represents a route between two switches
*
@@ -27,52 +29,18 @@ import java.util.List;
*/
public class Route implements Comparable<Route> {
protected RouteId id;
- protected List<Link> path;
+ protected List<NodePortTuple> switchPorts;
- public Route(RouteId id, List<Link> path) {
+ public Route(RouteId id, List<NodePortTuple> switchPorts) {
super();
this.id = id;
- this.path = path;
+ this.switchPorts = switchPorts;
}
public Route(Long src, Long dst) {
super();
this.id = new RouteId(src, dst);
- this.path = new ArrayList<Link>();
- }
-
- /**
- * Concise way to instantiate a route. The format of objects must be:
- * (Short/Integer outPort, Short/Integer inPort, Long dstDpid)*
- * @param srcDpid
- * @param objects
- */
- public Route(Long srcId, Object... routeElements) {
- super();
- this.path = new ArrayList<Link>();
- if (routeElements.length % 3 > 0)
- throw new RuntimeException("routeElements must be a multiple of 3");
- long s;
- Short srcPort, dstPort;
-
- s = srcId;
- for (int i = 0; i < routeElements.length; i += 3) {
- if (routeElements[i] instanceof Short)
- srcPort = (Short) routeElements[i];
- else
- srcPort = ((Integer)routeElements[i]).shortValue();
-
- if (routeElements[i+1] instanceof Short)
- dstPort = (Short) routeElements[i+1];
- else
- dstPort = ((Integer)routeElements[i+1]).shortValue();
-
- this.path.add(new Link(s, srcPort, ((Long) routeElements[i + 2]), dstPort));
-
- s = (Long) routeElements[i + 2];
- }
- this.id = new RouteId(srcId, (this.path.size() == 0) ? srcId
- : this.path.get(this.path.size() - 1).getDst());
+ this.switchPorts = new ArrayList<NodePortTuple>();
}
/**
@@ -92,15 +60,15 @@ public class Route implements Comparable<Route> {
/**
* @return the path
*/
- public List<Link> getPath() {
- return path;
+ public List<NodePortTuple> getPath() {
+ return switchPorts;
}
/**
* @param path the path to set
*/
- public void setPath(List<Link> path) {
- this.path = path;
+ public void setPath(List<NodePortTuple> switchPorts) {
+ this.switchPorts = switchPorts;
}
@Override
@@ -108,7 +76,7 @@ public class Route implements Comparable<Route> {
final int prime = 5791;
int result = 1;
result = prime * result + ((id == null) ? 0 : id.hashCode());
- result = prime * result + ((path == null) ? 0 : path.hashCode());
+ result = prime * result + ((switchPorts == null) ? 0 : switchPorts.hashCode());
return result;
}
@@ -126,17 +94,17 @@ public class Route implements Comparable<Route> {
return false;
} else if (!id.equals(other.id))
return false;
- if (path == null) {
- if (other.path != null)
+ if (switchPorts == null) {
+ if (other.switchPorts != null)
return false;
- } else if (!path.equals(other.path))
+ } else if (!switchPorts.equals(other.switchPorts))
return false;
return true;
}
@Override
public String toString() {
- return "Route [id=" + id + ", path=" + path + "]";
+ return "Route [id=" + id + ", switchPorts=" + switchPorts + "]";
}
/**
@@ -144,6 +112,6 @@ public class Route implements Comparable<Route> {
*/
@Override
public int compareTo(Route o) {
- return ((Integer)path.size()).compareTo(o.path.size());
+ return ((Integer)switchPorts.size()).compareTo(o.switchPorts.size());
}
}
diff --git a/src/main/java/net/floodlightcontroller/topology/TopologyInstance.java b/src/main/java/net/floodlightcontroller/topology/TopologyInstance.java
index d2ea16d6a..906ab25ce 100644
--- a/src/main/java/net/floodlightcontroller/topology/TopologyInstance.java
+++ b/src/main/java/net/floodlightcontroller/topology/TopologyInstance.java
@@ -4,6 +4,7 @@ import java.util.HashMap;
import java.util.HashSet;
import java.util.Iterator;
import java.util.LinkedList;
+import java.util.List;
import java.util.Map;
import java.util.PriorityQueue;
import java.util.Set;
@@ -534,16 +535,21 @@ public class TopologyInstance {
}
protected Route buildroute(RouteId id, long srcId, long dstId) {
- LinkedList<Link> path = new LinkedList<Link>();
+ NodePortTuple npt;
+
+ LinkedList<NodePortTuple> switchPorts =
+ new LinkedList<NodePortTuple>();
if (destinationRootedTrees == null) return null;
if (destinationRootedTrees.get(dstId) == null) return null;
- Map<Long, Link> nexthoplinks = destinationRootedTrees.get(dstId).getLinks();
+ Map<Long, Link> nexthoplinks =
+ destinationRootedTrees.get(dstId).getLinks();
if (!switches.contains(srcId) || !switches.contains(dstId)) {
// This is a switch that is not connected to any other switch
- // hence there was no update for links (and hence it is not in the network)
+ // hence there was no update for links (and hence it is not
+ // in the network)
log.debug("buildroute: Standalone switch: {}", srcId);
// The only possible non-null path for this case is
@@ -552,14 +558,19 @@ public class TopologyInstance {
} else if ((nexthoplinks!=null) && (nexthoplinks.get(srcId)!=null)) {
while (srcId != dstId) {
Link l = nexthoplinks.get(srcId);
- path.addLast(l);
+
+ npt = new NodePortTuple(l.getSrc(), l.getSrcPort());
+ switchPorts.addLast(npt);
+ npt = new NodePortTuple(l.getDst(), l.getDstPort());
+ switchPorts.addLast(npt);
srcId = nexthoplinks.get(srcId).getDst();
}
}
// else, no path exists, and path equals null
Route result = null;
- if (path != null && !path.isEmpty()) result = new Route(id, path);
+ if (switchPorts != null && !switchPorts.isEmpty())
+ result = new Route(id, switchPorts);
if (log.isTraceEnabled()) {
log.trace("buildroute: {}", result);
}
@@ -591,9 +602,19 @@ public class TopologyInstance {
protected Route getRoute(long srcId, short srcPort,
long dstId, short dstPort) {
- // currently ignores source and dst ports.
- // if needed we can get different paths.
- return getRoute(srcId, dstId);
+ NodePortTuple npt;
+ Route r = getRoute(srcId, dstId);
+ if (r == null && srcId != dstId) return null;
+
+ RouteId id = new RouteId(srcId, dstId);
+
+ List<NodePortTuple> nptList= r.getPath();
+ npt = new NodePortTuple(srcId, srcPort);
+ nptList.add(0, npt); // add src port to the front
+ npt = new NodePortTuple(dstId, dstPort);
+ nptList.add(npt); // add dst port to the end
+
+ return new Route(id, nptList);
}
protected Route getRoute(long srcId, long dstId) {
diff --git a/src/test/java/net/floodlightcontroller/forwarding/ForwardingTest.java b/src/test/java/net/floodlightcontroller/forwarding/ForwardingTest.java
index bdaf264b3..96f1a2bc3 100644
--- a/src/test/java/net/floodlightcontroller/forwarding/ForwardingTest.java
+++ b/src/test/java/net/floodlightcontroller/forwarding/ForwardingTest.java
@@ -49,11 +49,11 @@ import net.floodlightcontroller.packet.IPacket;
import net.floodlightcontroller.packet.IPv4;
import net.floodlightcontroller.packet.UDP;
import net.floodlightcontroller.routing.IRoutingService;
-import net.floodlightcontroller.routing.Link;
import net.floodlightcontroller.routing.Route;
import net.floodlightcontroller.test.FloodlightTestCase;
import net.floodlightcontroller.threadpool.IThreadPoolService;
import net.floodlightcontroller.topology.ITopologyService;
+import net.floodlightcontroller.topology.NodePortTuple;
import net.floodlightcontroller.flowcache.FlowReconcileManager;
import net.floodlightcontroller.flowcache.IFlowReconcileService;
import net.floodlightcontroller.forwarding.Forwarding;
@@ -257,9 +257,13 @@ public class ForwardingTest extends FloodlightTestCase {
dstDevice1);
Route route = new Route(1L, 2L);
- route.setPath(new ArrayList<Link>());
- route.getPath().add(new Link(1L, (short)3, 2L, (short)1));
- expect(routingEngine.getRoute(1L, 2L)).andReturn(route).atLeastOnce();
+ List<NodePortTuple> nptList = new ArrayList<NodePortTuple>();
+ nptList.add(new NodePortTuple(1L, (short)1));
+ nptList.add(new NodePortTuple(1L, (short)3));
+ nptList.add(new NodePortTuple(2L, (short)1));
+ nptList.add(new NodePortTuple(2L, (short)3));
+ route.setPath(nptList);
+ expect(routingEngine.getRoute(1L, (short)1, 2L, (short)3)).andReturn(route).atLeastOnce();
// Expected Flow-mods
OFMatch match = new OFMatch();
@@ -337,8 +341,12 @@ public class ForwardingTest extends FloodlightTestCase {
put(cntx,
IDeviceService.CONTEXT_DST_DEVICE,
dstDevice2);
- expect(routingEngine.getRoute(1L, 1L)).andReturn(null).atLeastOnce();
-
+
+ Route route = new Route(1L, 1L);
+ route.getPath().add(new NodePortTuple(1L, (short)1));
+ route.getPath().add(new NodePortTuple(1L, (short)3));
+ expect(routingEngine.getRoute(1L, (short)1, 1L, (short)3)).andReturn(route).atLeastOnce();
+
// Expected Flow-mods
OFMatch match = new OFMatch();
match.loadFromPacket(testPacketSerialized, (short) 1);
diff --git a/src/test/java/net/floodlightcontroller/routing/RouteTest.java b/src/test/java/net/floodlightcontroller/routing/RouteTest.java
index 433ab7c23..3bd039898 100644
--- a/src/test/java/net/floodlightcontroller/routing/RouteTest.java
+++ b/src/test/java/net/floodlightcontroller/routing/RouteTest.java
@@ -19,9 +19,9 @@ package net.floodlightcontroller.routing;
import org.junit.Test;
-import net.floodlightcontroller.routing.Link;
import net.floodlightcontroller.routing.Route;
import net.floodlightcontroller.test.FloodlightTestCase;
+import net.floodlightcontroller.topology.NodePortTuple;
/**
*
@@ -32,28 +32,32 @@ public class RouteTest extends FloodlightTestCase {
public void testCloneable() throws Exception {
Route r1 = new Route(1L, 2L);
Route r2 = new Route(1L, 3L);
-
+
assertNotSame(r1, r2);
assertNotSame(r1.getId(), r2.getId());
r1 = new Route(1L, 3L);
- r1.getPath().add(new Link(1L, (short)1, 2L, (short)1));
- r1.getPath().add(new Link(2L, (short)2, 3L, (short)1));
-
- r2.getPath().add(new Link(1L, (short)1, 2L, (short)1));
- r2.getPath().add(new Link(2L, (short)2, 3L, (short)1));
-
+ r1.getPath().add(new NodePortTuple(1L, (short)1));
+ r1.getPath().add(new NodePortTuple(2L, (short)1));
+ r1.getPath().add(new NodePortTuple(2L, (short)2));
+ r1.getPath().add(new NodePortTuple(3L, (short)1));
+
+ r2.getPath().add(new NodePortTuple(1L, (short)1));
+ r2.getPath().add(new NodePortTuple(2L, (short)1));
+ r2.getPath().add(new NodePortTuple(2L, (short)2));
+ r2.getPath().add(new NodePortTuple(3L, (short)1));
+
assertEquals(r1, r2);
- Link temp = r2.getPath().remove(0);
+ NodePortTuple temp = r2.getPath().remove(0);
assertNotSame(r1, r2);
r2.getPath().add(0, temp);
assertEquals(r1, r2);
- r2.getPath().remove(0);
- temp = new Link(1L, (short)1, 2L, (short)5);
- r2.getPath().add(0, temp);
+ r2.getPath().remove(1);
+ temp = new NodePortTuple(2L, (short)5);
+ r2.getPath().add(1, temp);
assertNotSame(r1, r2);
}
}
--
GitLab