Refactored the forwarding module to use the new packet API.
It now uses PacketService for both receiving packet-ins and sending
packet-outs, so there is now no dependency on FloodlightProvider or
FlowPusher.
Fixed a bug where the packet module wasn't sending packet-outs correctly.
Change-Id: If738797cdcebabfed975875daf4b40df99226585
diff --git a/src/main/java/net/onrc/onos/apps/forwarding/Forwarding.java b/src/main/java/net/onrc/onos/apps/forwarding/Forwarding.java
index d8c01ce..a7b7fbc 100644
--- a/src/main/java/net/onrc/onos/apps/forwarding/Forwarding.java
+++ b/src/main/java/net/onrc/onos/apps/forwarding/Forwarding.java
@@ -11,20 +11,16 @@
import java.util.concurrent.ScheduledExecutorService;
import java.util.concurrent.TimeUnit;
-import net.floodlightcontroller.core.FloodlightContext;
-import net.floodlightcontroller.core.IFloodlightProviderService;
-import net.floodlightcontroller.core.IOFMessageListener;
-import net.floodlightcontroller.core.IOFSwitch;
import net.floodlightcontroller.core.module.FloodlightModuleContext;
import net.floodlightcontroller.core.module.IFloodlightModule;
import net.floodlightcontroller.core.module.IFloodlightService;
import net.floodlightcontroller.util.MACAddress;
+import net.onrc.onos.api.packet.IPacketListener;
+import net.onrc.onos.api.packet.IPacketService;
import net.onrc.onos.apps.proxyarp.IProxyArpService;
import net.onrc.onos.core.datagrid.IDatagridService;
-import net.onrc.onos.core.datagrid.IEventChannel;
import net.onrc.onos.core.datagrid.IEventChannelListener;
import net.onrc.onos.core.devicemanager.IOnosDeviceService;
-import net.onrc.onos.core.flowprogrammer.IFlowPusherService;
import net.onrc.onos.core.intent.Intent;
import net.onrc.onos.core.intent.Intent.IntentState;
import net.onrc.onos.core.intent.IntentMap;
@@ -35,25 +31,17 @@
import net.onrc.onos.core.intent.runtime.IPathCalcRuntimeService;
import net.onrc.onos.core.intent.runtime.IntentStateList;
import net.onrc.onos.core.packet.Ethernet;
-import net.onrc.onos.core.packetservice.BroadcastPacketOutNotification;
import net.onrc.onos.core.registry.IControllerRegistryService;
import net.onrc.onos.core.topology.Device;
import net.onrc.onos.core.topology.INetworkGraphService;
import net.onrc.onos.core.topology.LinkEvent;
import net.onrc.onos.core.topology.NetworkGraph;
+import net.onrc.onos.core.topology.Port;
import net.onrc.onos.core.topology.Switch;
import net.onrc.onos.core.util.Dpid;
import net.onrc.onos.core.util.FlowPath;
-import net.onrc.onos.core.util.Port;
import net.onrc.onos.core.util.SwitchPort;
-import org.openflow.protocol.OFMessage;
-import org.openflow.protocol.OFPacketIn;
-import org.openflow.protocol.OFPacketOut;
-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.slf4j.Logger;
import org.slf4j.LoggerFactory;
@@ -61,8 +49,9 @@
import com.google.common.collect.LinkedListMultimap;
import com.google.common.collect.ListMultimap;
-public class Forwarding implements IOFMessageListener, IFloodlightModule,
- IForwardingService, IEventChannelListener<Long, IntentStateList> {
+public class Forwarding implements /*IOFMessageListener,*/ IFloodlightModule,
+ IForwardingService, IEventChannelListener<Long, IntentStateList>,
+ IPacketListener {
private static final Logger log = LoggerFactory.getLogger(Forwarding.class);
private static final int SLEEP_TIME_FOR_DB_DEVICE_INSTALLED = 100; // milliseconds
@@ -72,13 +61,8 @@
private final String callerId = "Forwarding";
- private IFloodlightProviderService floodlightProvider;
- private IFlowPusherService flowPusher;
private IDatagridService datagrid;
-
- private IEventChannel<Long, BroadcastPacketOutNotification> eventChannel;
- private static final String SINGLE_PACKET_OUT_CHANNEL_NAME = "onos.forwarding.packet_out";
-
+ private IPacketService packetService;
private IControllerRegistryService controllerRegistryService;
private INetworkGraphService networkGraphService;
@@ -94,11 +78,11 @@
private final Object lock = new Object();
private static class PacketToPush {
- public final OFPacketOut packet;
+ public final Ethernet eth;
public final long dpid;
- public PacketToPush(OFPacketOut packet, long dpid) {
- this.packet = packet;
+ public PacketToPush(Ethernet eth, long dpid) {
+ this.eth = eth;
this.dpid = dpid;
}
}
@@ -167,8 +151,6 @@
public Collection<Class<? extends IFloodlightService>> getModuleDependencies() {
List<Class<? extends IFloodlightService>> dependencies =
new ArrayList<Class<? extends IFloodlightService>>();
- dependencies.add(IFloodlightProviderService.class);
- dependencies.add(IFlowPusherService.class);
dependencies.add(IControllerRegistryService.class);
dependencies.add(IOnosDeviceService.class);
dependencies.add(IDatagridService.class);
@@ -177,20 +159,17 @@
// We don't use the IProxyArpService directly, but reactive forwarding
// requires it to be loaded and answering ARP requests
dependencies.add(IProxyArpService.class);
+ dependencies.add(IPacketService.class);
return dependencies;
}
@Override
public void init(FloodlightModuleContext context) {
- floodlightProvider =
- context.getServiceImpl(IFloodlightProviderService.class);
- flowPusher = context.getServiceImpl(IFlowPusherService.class);
datagrid = context.getServiceImpl(IDatagridService.class);
controllerRegistryService = context.getServiceImpl(IControllerRegistryService.class);
networkGraphService = context.getServiceImpl(INetworkGraphService.class);
pathRuntime = context.getServiceImpl(IPathCalcRuntimeService.class);
-
- floodlightProvider.addOFMessageListener(OFType.PACKET_IN, this);
+ packetService = context.getServiceImpl(IPacketService.class);
pendingFlows = new HashMap<Path, PushedFlow>();
waitingPackets = LinkedListMultimap.create();
@@ -198,77 +177,41 @@
@Override
public void startUp(FloodlightModuleContext context) {
+ packetService.registerPacketListener(this);
- eventChannel = datagrid.createChannel(SINGLE_PACKET_OUT_CHANNEL_NAME,
- Long.class,
- BroadcastPacketOutNotification.class);
networkGraph = networkGraphService.getNetworkGraph();
intentMap = pathRuntime.getPathIntents();
datagrid.addListener("onos.pathintent_state", this, Long.class, IntentStateList.class);
}
@Override
- public String getName() {
- return "onosforwarding";
- }
-
- @Override
- public boolean isCallbackOrderingPrereq(OFType type, String name) {
- return (type == OFType.PACKET_IN) &&
- (name.equals("devicemanager") || name.equals("proxyarpmanager")
- || name.equals("onosdevicemanager"));
- }
-
- @Override
- public boolean isCallbackOrderingPostreq(OFType type, String name) {
- return false;
- }
-
- @Override
- public Command receive(
- IOFSwitch sw, OFMessage msg, FloodlightContext cntx) {
-
- if (msg.getType() != OFType.PACKET_IN || !(msg instanceof OFPacketIn)) {
- return Command.CONTINUE;
- }
-
- OFPacketIn pi = (OFPacketIn) msg;
-
- Ethernet eth = IFloodlightProviderService.bcStore.
- get(cntx, IFloodlightProviderService.CONTEXT_PI_PAYLOAD);
-
- log.debug("Receive PACKET_IN swId {}, portId {}", sw.getId(), pi.getInPort());
+ public void receive(Switch sw, Port inPort, Ethernet eth) {
+ log.debug("Receive PACKET_IN swId {}, portId {}", sw.getDpid(), inPort.getNumber());
if (eth.getEtherType() != Ethernet.TYPE_IPV4) {
- return Command.CONTINUE;
+ // Only handle IPv4 packets right now
+ return;
}
if (eth.isBroadcast() || eth.isMulticast()) {
- handleBroadcast(sw, pi, eth);
+ handleBroadcast(sw, inPort, eth);
} else {
// Unicast
- handlePacketIn(sw, pi, eth);
+ handlePacketIn(sw, inPort, eth);
}
-
- return Command.STOP;
}
- private void handleBroadcast(IOFSwitch sw, OFPacketIn pi, Ethernet eth) {
+ private void handleBroadcast(Switch sw, Port inPort, Ethernet eth) {
if (log.isTraceEnabled()) {
log.trace("Sending broadcast packet to other ONOS instances");
}
- //We don't use address information, so 0 is put into the third argument.
- BroadcastPacketOutNotification key =
- new BroadcastPacketOutNotification(
- eth.serialize(),
- 0, sw.getId(),
- pi.getInPort());
- eventChannel.addTransientEntry(eth.getDestinationMAC().toLong(), key);
+ packetService.broadcastPacket(eth,
+ new SwitchPort(sw.getDpid(), inPort.getNumber().shortValue()));
}
- private void handlePacketIn(IOFSwitch sw, OFPacketIn pi, Ethernet eth) {
- log.debug("Start handlePacketIn swId {}, portId {}", sw.getId(), pi.getInPort());
+ private void handlePacketIn(Switch sw, Port inPort, Ethernet eth) {
+ log.debug("Start handlePacketIn swId {}, portId {}", sw.getDpid(), inPort.getNumber());
String destinationMac =
HexString.toHexString(eth.getDestinationMACAddress());
@@ -281,22 +224,22 @@
destinationMac);
//Device is not in the DB, so wait it until the device is added.
- EXECUTOR_SERVICE.schedule(new WaitDeviceArp(sw, pi, eth), SLEEP_TIME_FOR_DB_DEVICE_INSTALLED, TimeUnit.MILLISECONDS);
+ EXECUTOR_SERVICE.schedule(new WaitDeviceArp(sw, inPort, eth), SLEEP_TIME_FOR_DB_DEVICE_INSTALLED, TimeUnit.MILLISECONDS);
return;
}
- continueHandlePacketIn(sw, pi, eth, deviceObject);
+ continueHandlePacketIn(sw, inPort, eth, deviceObject);
}
private class WaitDeviceArp implements Runnable {
- IOFSwitch sw;
- OFPacketIn pi;
+ Switch sw;
+ Port inPort;
Ethernet eth;
- public WaitDeviceArp(IOFSwitch sw, OFPacketIn pi, Ethernet eth) {
+ public WaitDeviceArp(Switch sw, Port inPort, Ethernet eth) {
super();
this.sw = sw;
- this.pi = pi;
+ this.inPort = inPort;
this.eth = eth;
}
@@ -305,15 +248,15 @@
Device deviceObject = networkGraph.getDeviceByMac(MACAddress.valueOf(eth.getDestinationMACAddress()));
if (deviceObject == null) {
log.debug("wait {}ms and device was not found. Send broadcast packet and the thread finish.", SLEEP_TIME_FOR_DB_DEVICE_INSTALLED);
- handleBroadcast(sw, pi, eth);
+ handleBroadcast(sw, inPort, eth);
return;
}
log.debug("wait {}ms and device {} was found, continue", SLEEP_TIME_FOR_DB_DEVICE_INSTALLED, deviceObject.getMacAddress());
- continueHandlePacketIn(sw, pi, eth, deviceObject);
+ continueHandlePacketIn(sw, inPort, eth, deviceObject);
}
}
- private void continueHandlePacketIn(IOFSwitch sw, OFPacketIn pi, Ethernet eth, Device deviceObject) {
+ private void continueHandlePacketIn(Switch sw, Port inPort, Ethernet eth, Device deviceObject) {
log.debug("Start continuehandlePacketIn");
@@ -322,7 +265,7 @@
if (!ports.hasNext()) {
log.debug("No attachment point found for device {} - broadcasting packet",
deviceObject.getMacAddress());
- handleBroadcast(sw, pi, eth);
+ handleBroadcast(sw, inPort, eth);
return;
}
@@ -332,11 +275,13 @@
Switch switchObject = portObject.getSwitch();
long destinationDpid = switchObject.getDpid();
- // TODO SwitchPort, Dpid and Port should probably be immutable
+ // TODO eliminate cast
SwitchPort srcSwitchPort = new SwitchPort(
- new Dpid(sw.getId()), new Port(pi.getInPort()));
+ new Dpid(sw.getDpid()),
+ new net.onrc.onos.core.util.Port((short) inPort.getNumber().longValue()));
SwitchPort dstSwitchPort = new SwitchPort(
- new Dpid(destinationDpid), new Port(destinationPort));
+ new Dpid(destinationDpid),
+ new net.onrc.onos.core.util.Port(destinationPort));
MACAddress srcMacAddress = MACAddress.valueOf(eth.getSourceMACAddress());
MACAddress dstMacAddress = MACAddress.valueOf(eth.getDestinationMACAddress());
@@ -355,8 +300,6 @@
existingFlow.intentId);
}
- OFPacketOut po = constructPacketOut(pi, sw);
-
// Find the correct port here. We just assume the PI is from
// the first hop switch, but this is definitely not always
// the case. We'll have to retrieve the flow from HZ every time
@@ -379,7 +322,7 @@
for (Iterator<LinkEvent> i = path.iterator(); i.hasNext();) {
LinkEvent le = (LinkEvent) i.next();
- if (le.getSrc().dpid == sw.getId()) {
+ if (le.getSrc().dpid.equals(sw.getDpid())) {
log.debug("src {} dst {}", le.getSrc(), le.getDst());
isflowEntryForThisSwitch = true;
break;
@@ -395,13 +338,15 @@
srcMacAddress, dstMacAddress);
} else {
log.debug("Sending packet out from sw {}, outport{}", sw, existingFlow.firstOutPort);
- sendPacketOut(sw, po, existingFlow.firstOutPort);
+
+ packetService.sendPacket(new SwitchPort(
+ sw.getDpid(), existingFlow.firstOutPort), eth);
}
} else {
// Flow path has not yet been installed to switches so save the
// packet out for later
- log.debug("Put a packet into the waitng list. flowId {}", existingFlow.intentId);
- waitingPackets.put(existingFlow.intentId, new PacketToPush(po, sw.getId()));
+ log.debug("Put a packet into the waiting list. flowId {}", existingFlow.intentId);
+ waitingPackets.put(existingFlow.intentId, new PacketToPush(eth, sw.getDpid()));
}
return;
}
@@ -412,41 +357,21 @@
String intentId = callerId + ":" + controllerRegistryService.getNextUniqueId();
IntentOperationList operations = new IntentOperationList();
ShortestPathIntent intent = new ShortestPathIntent(intentId,
- sw.getId(), pi.getInPort(), srcMacAddress.toLong(),
+ sw.getDpid(), inPort.getNumber(), srcMacAddress.toLong(),
destinationDpid, destinationPort, dstMacAddress.toLong());
IntentOperation.Operator operator = IntentOperation.Operator.ADD;
operations.add(operator, intent);
pathRuntime.executeIntentOperations(operations);
- OFPacketOut po = constructPacketOut(pi, sw);
-
// Add to waiting lists
pendingFlows.put(pathspec, new PushedFlow(intentId));
log.debug("Put a Path {} in the pending flow, intent ID {}", pathspec, intentId);
- waitingPackets.put(intentId, new PacketToPush(po, sw.getId()));
+ waitingPackets.put(intentId, new PacketToPush(eth, sw.getDpid()));
log.debug("Put a Packet in the wating list. related pathspec {}", pathspec);
}
}
- private OFPacketOut constructPacketOut(OFPacketIn pi, IOFSwitch sw) {
- OFPacketOut po = new OFPacketOut();
- po.setInPort(OFPort.OFPP_NONE)
- .setInPort(pi.getInPort())
- .setActions(new ArrayList<OFAction>())
- .setLengthU(OFPacketOut.MINIMUM_LENGTH);
-
- if (sw.getBuffers() == 0) {
- po.setBufferId(OFPacketOut.BUFFER_ID_NONE)
- .setPacketData(pi.getPacketData())
- .setLengthU(po.getLengthU() + po.getPacketData().length);
- } else {
- po.setBufferId(pi.getBufferId());
- }
-
- return po;
- }
-
@Override
public void flowsInstalled(Collection<FlowPath> installedFlowPaths) {
}
@@ -533,24 +458,14 @@
for (PacketToPush packet : packets) {
log.debug("Start packetToPush to sw {}, outPort {}, path {}", packet.dpid, existingFlow.firstOutPort, path);
- IOFSwitch sw = floodlightProvider.getSwitches().get(packet.dpid);
- sendPacketOut(sw, packet.packet, existingFlow.firstOutPort);
+ packetService.sendPacket(new SwitchPort(
+ packet.dpid, existingFlow.firstOutPort), packet.eth);
}
}
- private void sendPacketOut(IOFSwitch sw, OFPacketOut po, short outPort) {
- po.getActions().add(new OFActionOutput(outPort));
- po.setActionsLength((short)
- (po.getActionsLength() + OFActionOutput.MINIMUM_LENGTH));
- po.setLengthU(po.getLengthU() + OFActionOutput.MINIMUM_LENGTH);
-
- flowPusher.add(sw, po);
- }
-
@Override
public void entryAdded(IntentStateList value) {
entryUpdated(value);
-
}
@Override
diff --git a/src/main/java/net/onrc/onos/core/packetservice/BroadcastPacketOutNotification.java b/src/main/java/net/onrc/onos/core/packetservice/BroadcastPacketOutNotification.java
index 3e098ed..bf5c700 100644
--- a/src/main/java/net/onrc/onos/core/packetservice/BroadcastPacketOutNotification.java
+++ b/src/main/java/net/onrc/onos/core/packetservice/BroadcastPacketOutNotification.java
@@ -2,6 +2,9 @@
import java.util.Map;
+import net.onrc.onos.core.topology.NetworkGraph;
+import net.onrc.onos.core.topology.Port;
+
import com.google.common.collect.HashMultimap;
import com.google.common.collect.Multimap;
@@ -75,12 +78,25 @@
}
@Override
- public Multimap<Long, Short> calculateOutPorts(Multimap<Long, Short> localSwitches) {
+ public Multimap<Long, Short> calculateOutPorts(
+ Multimap<Long, Short> localPorts, NetworkGraph networkGraph) {
Multimap<Long, Short> outPorts = HashMultimap.create();
- for (Map.Entry<Long, Short> entry : localSwitches.entries()) {
- if (!entry.getKey().equals(inSwitch) ||
- !entry.getValue().equals(inPort)) {
+ for (Map.Entry<Long, Short> entry : localPorts.entries()) {
+ Port globalPort;
+ networkGraph.acquireReadLock();
+ try {
+ globalPort = networkGraph.getPort(entry.getKey(),
+ entry.getValue().longValue());
+ } finally {
+ networkGraph.releaseReadLock();
+ }
+
+ if ((!entry.getKey().equals(inSwitch) ||
+ !entry.getValue().equals(inPort)) &&
+ globalPort != null &&
+ globalPort.getOutgoingLink() == null) {
+
outPorts.put(entry.getKey(), entry.getValue());
}
}
diff --git a/src/main/java/net/onrc/onos/core/packetservice/PacketModule.java b/src/main/java/net/onrc/onos/core/packetservice/PacketModule.java
index 27bfbb5..1e3cae6 100644
--- a/src/main/java/net/onrc/onos/core/packetservice/PacketModule.java
+++ b/src/main/java/net/onrc/onos/core/packetservice/PacketModule.java
@@ -67,8 +67,14 @@
@Override
public void entryAdded(PacketOutNotification value) {
+ Multimap<Long, Short> localPorts = HashMultimap.create();
+ for (IOFSwitch sw : floodlightProvider.getSwitches().values()) {
+ for (OFPhysicalPort port : sw.getEnabledPorts()) {
+ localPorts.put(sw.getId(), port.getPortNumber());
+ }
+ }
Multimap<Long, Short> outPorts = value.calculateOutPorts(
- findLocalEdgePorts());
+ localPorts, networkGraph);
sendPacketToSwitches(outPorts, value.getPacketData());
}
@@ -164,6 +170,8 @@
}
if (networkGraphSwitch == null || inPort == null) {
+ // We can't send packets for switches or ports that aren't in the
+ // network graph yet
return Command.CONTINUE;
}
@@ -221,21 +229,6 @@
PacketOutNotification.class);
}
- private Multimap<Long, Short> findLocalEdgePorts() {
- Multimap<Long, Short> edgePorts = HashMultimap.create();
- Map<Long, IOFSwitch> localSwitches = floodlightProvider.getSwitches();
- for (IOFSwitch sw : localSwitches.values()) {
- for (OFPhysicalPort localPort : sw.getEnabledPorts()) {
- Port globalPort =
- networkGraph.getPort(sw.getId(), (long) localPort.getPortNumber());
- if (globalPort.getOutgoingLink() == null) {
- edgePorts.put(sw.getId(), localPort.getPortNumber());
- }
- }
- }
- return edgePorts;
- }
-
private void sendPacketToSwitches(Multimap<Long, Short> outPorts,
byte[] packetData) {
for (Long dpid : outPorts.keySet()) {
diff --git a/src/main/java/net/onrc/onos/core/packetservice/PacketOutNotification.java b/src/main/java/net/onrc/onos/core/packetservice/PacketOutNotification.java
index 8fe8efe..d95ddf2 100644
--- a/src/main/java/net/onrc/onos/core/packetservice/PacketOutNotification.java
+++ b/src/main/java/net/onrc/onos/core/packetservice/PacketOutNotification.java
@@ -3,6 +3,8 @@
import java.io.Serializable;
import java.util.Arrays;
+import net.onrc.onos.core.topology.NetworkGraph;
+
import com.google.common.collect.Multimap;
/**
@@ -51,11 +53,12 @@
* calculate that list, given the list of edge ports controlled by this
* instance.
*
- * @param localSwitches the map of locally-controlled edge ports
+ * @param localPorts the map of locally-controlled ports
+ * @param networkGraph an instance of the global network graph
* @return a multimap of ports that the packet should be sent out,
* in the form
* {@code {dpid1 => {portnum1, portnum2, ...}, dpid2 => {portnum1}, ...}}
*/
public abstract Multimap<Long, Short> calculateOutPorts(
- Multimap<Long, Short> localEdgePorts);
+ Multimap<Long, Short> localPorts, NetworkGraph networkGraph);
}
diff --git a/src/main/java/net/onrc/onos/core/packetservice/SinglePacketOutNotification.java b/src/main/java/net/onrc/onos/core/packetservice/SinglePacketOutNotification.java
index e6260e0..cd604ea 100644
--- a/src/main/java/net/onrc/onos/core/packetservice/SinglePacketOutNotification.java
+++ b/src/main/java/net/onrc/onos/core/packetservice/SinglePacketOutNotification.java
@@ -1,5 +1,7 @@
package net.onrc.onos.core.packetservice;
+import net.onrc.onos.core.topology.NetworkGraph;
+
import com.google.common.collect.HashMultimap;
import com.google.common.collect.Multimap;
@@ -14,6 +16,12 @@
private final long outSwitch;
private final short outPort;
+ protected SinglePacketOutNotification() {
+ address = 0;
+ outSwitch = 0;
+ outPort = 0;
+ }
+
/**
* Class constructor.
*
@@ -60,10 +68,11 @@
}
@Override
- public Multimap<Long, Short> calculateOutPorts(Multimap<Long, Short> localSwitches) {
+ public Multimap<Long, Short> calculateOutPorts(
+ Multimap<Long, Short> localPorts, NetworkGraph networkGraph) {
Multimap<Long, Short> outPorts = HashMultimap.create();
- if (localSwitches.containsEntry(outSwitch, outPort)) {
+ if (localPorts.containsEntry(outSwitch, outPort)) {
outPorts.put(outSwitch, outPort);
}
diff --git a/src/main/java/net/onrc/onos/core/topology/Switch.java b/src/main/java/net/onrc/onos/core/topology/Switch.java
index d9796be..690cea8 100644
--- a/src/main/java/net/onrc/onos/core/topology/Switch.java
+++ b/src/main/java/net/onrc/onos/core/topology/Switch.java
@@ -15,7 +15,6 @@
public Port getPort(Long number);
-
// Graph traversal API
// XXX What is the Definition of neighbor? Link exist in both direction or one-way is sufficient to be a neighbor, etc.
public Iterable<Switch> getNeighbors();