Added getFactory() method to IOFSwitch to get a message factory appropriate
for the switch's OpenFlow version.
This prevents users of the switch having to discriminate what factory to use
based on the switch OpenFlow version.
Change-Id: Iac0454856e35f4429649a6f116da34f4c048f25d
diff --git a/src/main/java/net/floodlightcontroller/core/IFloodlightProviderService.java b/src/main/java/net/floodlightcontroller/core/IFloodlightProviderService.java
index 7f4f77d..5f73d9a 100644
--- a/src/main/java/net/floodlightcontroller/core/IFloodlightProviderService.java
+++ b/src/main/java/net/floodlightcontroller/core/IFloodlightProviderService.java
@@ -27,7 +27,6 @@
import net.onrc.onos.core.util.OnosInstanceId;
import org.projectfloodlight.openflow.protocol.OFControllerRole;
-import org.projectfloodlight.openflow.protocol.OFFactory;
import org.projectfloodlight.openflow.protocol.OFType;
/**
@@ -193,14 +192,6 @@
//************************
/**
- * Gets the Factory
- *
- * @return an OpenFlow message factory
- */
- public OFFactory getOFMessageFactory_13();
- public OFFactory getOFMessageFactory_10();
-
- /**
* Publish updates to Controller updates queue
*
* @param IUpdate
diff --git a/src/main/java/net/floodlightcontroller/core/IOFSwitch.java b/src/main/java/net/floodlightcontroller/core/IOFSwitch.java
index 54e4408..530da78 100644
--- a/src/main/java/net/floodlightcontroller/core/IOFSwitch.java
+++ b/src/main/java/net/floodlightcontroller/core/IOFSwitch.java
@@ -35,10 +35,9 @@
import org.projectfloodlight.openflow.protocol.OFActionType;
import org.projectfloodlight.openflow.protocol.OFCapabilities;
import org.projectfloodlight.openflow.protocol.OFDescStatsReply;
-import org.projectfloodlight.openflow.protocol.OFFeaturesReply;
+import org.projectfloodlight.openflow.protocol.OFFactory;
import org.projectfloodlight.openflow.protocol.OFMessage;
import org.projectfloodlight.openflow.protocol.OFPortDesc;
-import org.projectfloodlight.openflow.protocol.OFPortDescStatsReply;
import org.projectfloodlight.openflow.protocol.OFPortStatus;
import org.projectfloodlight.openflow.protocol.OFStatsReply;
import org.projectfloodlight.openflow.protocol.OFStatsRequest;
@@ -205,10 +204,26 @@
*/
public Set<OFActionType> getActions();
- public void setOFVersion(OFVersion ofv);
+ /**
+ * Gets an OpenFlow message factory that can be used to create messages to
+ * send to this switch.
+ *
+ * @return an OFFactory for the correct OpenFlow version
+ */
+ public OFFactory getFactory();
+ /**
+ * Gets the OpenFlow version (eg. OF1.0, OF1.3) for this switch.
+ *
+ * @return the OpenFlow version of the switch
+ */
public OFVersion getOFVersion();
+ /**
+ * Sets the OpenFlow version (eg. OF1.0, OF1.3) for this switch.
+ */
+ public void setOFVersion(OFVersion ofv);
+
//************************
// Switch port related
diff --git a/src/main/java/net/floodlightcontroller/core/internal/Controller.java b/src/main/java/net/floodlightcontroller/core/internal/Controller.java
index e80311b..b47962f 100644
--- a/src/main/java/net/floodlightcontroller/core/internal/Controller.java
+++ b/src/main/java/net/floodlightcontroller/core/internal/Controller.java
@@ -35,7 +35,6 @@
import java.util.concurrent.BlockingQueue;
import java.util.concurrent.ConcurrentHashMap;
import java.util.concurrent.ConcurrentMap;
-import java.util.concurrent.CopyOnWriteArrayList;
import java.util.concurrent.CopyOnWriteArraySet;
import java.util.concurrent.Executors;
import java.util.concurrent.LinkedBlockingQueue;
@@ -938,12 +937,21 @@
// }
// }
- @Override
+
+ /**
+ * Gets an OpenFlow message factory for version 1.0.
+ *
+ * @return an OpenFlow 1.0 message factory
+ */
public OFFactory getOFMessageFactory_10() {
return factory10;
}
- @Override
+ /**
+ * Gets an OpenFlow message factory for version 1.3.
+ *
+ * @return an OpenFlow 1.3 message factory
+ */
public OFFactory getOFMessageFactory_13() {
return factory13;
}
diff --git a/src/main/java/net/floodlightcontroller/core/internal/OFSwitchImplBase.java b/src/main/java/net/floodlightcontroller/core/internal/OFSwitchImplBase.java
index c3d6e63..1b2b0cf 100644
--- a/src/main/java/net/floodlightcontroller/core/internal/OFSwitchImplBase.java
+++ b/src/main/java/net/floodlightcontroller/core/internal/OFSwitchImplBase.java
@@ -62,6 +62,8 @@
import org.projectfloodlight.openflow.protocol.OFActionType;
import org.projectfloodlight.openflow.protocol.OFCapabilities;
import org.projectfloodlight.openflow.protocol.OFDescStatsReply;
+import org.projectfloodlight.openflow.protocol.OFFactories;
+import org.projectfloodlight.openflow.protocol.OFFactory;
import org.projectfloodlight.openflow.protocol.OFFeaturesReply;
import org.projectfloodlight.openflow.protocol.OFMessage;
import org.projectfloodlight.openflow.protocol.OFPortConfig;
@@ -269,14 +271,19 @@
return stringId;
}
- /** Retrieve the openflow version (eg. OF1.0, OF1.3) for this switch
- */
- public OFVersion getOFVersion() {
- return ofversion;
+ @Override
+ public OFFactory getFactory() {
+ return OFFactories.getFactory(ofversion);
}
+ @Override
+ public OFVersion getOFVersion() {
+ return ofversion;
+ }
+
+ @Override
public void setOFVersion(OFVersion ofv) {
- ofversion = ofv;
+ ofversion = ofv;
}
/**
@@ -1223,10 +1230,10 @@
public void clearAllFlowMods() {
// Delete all pre-existing flows
- // by default if match is not specified, then an empty list of matches
- // is sent, resulting in a wildcard-all flows
- // XXX fix this later
- OFMessage fm = floodlightProvider.getOFMessageFactory_13()
+ // by default if match is not specified, then an empty list of matches
+ // is sent, resulting in a wildcard-all flows
+ // XXX fix this later to be sure it works for both 1.0 and 1.3
+ OFMessage fm = getFactory()
.buildFlowDelete()
.setOutPort(OFPort.ANY)
.setOutGroup(OFGroup.ANY)
diff --git a/src/main/java/net/onrc/onos/core/drivermanager/OFSwitchImplCPqD13.java b/src/main/java/net/onrc/onos/core/drivermanager/OFSwitchImplCPqD13.java
index c2bb344..efeaf6e 100644
--- a/src/main/java/net/onrc/onos/core/drivermanager/OFSwitchImplCPqD13.java
+++ b/src/main/java/net/onrc/onos/core/drivermanager/OFSwitchImplCPqD13.java
@@ -118,7 +118,7 @@
throw new SwitchDriverSubHandshakeAlreadyStarted();
}
startDriverHandshakeCalled = true;
- factory = floodlightProvider.getOFMessageFactory_13();
+ factory = getFactory();
// configureSwitch();
sendBarrier(true);
}
diff --git a/src/main/java/net/onrc/onos/core/drivermanager/OFSwitchImplOVS13.java b/src/main/java/net/onrc/onos/core/drivermanager/OFSwitchImplOVS13.java
index efe43a0..668f84d 100644
--- a/src/main/java/net/onrc/onos/core/drivermanager/OFSwitchImplOVS13.java
+++ b/src/main/java/net/onrc/onos/core/drivermanager/OFSwitchImplOVS13.java
@@ -44,7 +44,7 @@
throw new SwitchDriverSubHandshakeAlreadyStarted();
}
startDriverHandshakeCalled = true;
- factory = floodlightProvider.getOFMessageFactory_13();
+ factory = getFactory();
configureSwitch();
}
diff --git a/src/main/java/net/onrc/onos/core/flowprogrammer/FlowPusher.java b/src/main/java/net/onrc/onos/core/flowprogrammer/FlowPusher.java
index 419dc43..a02f0d5 100644
--- a/src/main/java/net/onrc/onos/core/flowprogrammer/FlowPusher.java
+++ b/src/main/java/net/onrc/onos/core/flowprogrammer/FlowPusher.java
@@ -33,7 +33,6 @@
import org.projectfloodlight.openflow.protocol.OFFlowMod;
import org.projectfloodlight.openflow.protocol.OFMessage;
import org.projectfloodlight.openflow.protocol.OFType;
-import org.projectfloodlight.openflow.protocol.OFVersion;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
@@ -245,7 +244,6 @@
private FloodlightModuleContext context = null;
private IThreadPoolService threadPool = null;
private IFloodlightProviderService floodlightProvider = null;
- protected Map<OFVersion, OFFactory> ofFactoryMap = null;
// Map of threads versus dpid
private Map<Long, FlowPusherThread> threadMap = null;
@@ -405,9 +403,6 @@
.getServiceImpl(IFloodlightProviderService.class);
this.threadPool = context.getServiceImpl(IThreadPoolService.class);
- ofFactoryMap = new HashMap<>();
- ofFactoryMap.put(OFVersion.OF_10, floodlightProvider.getOFMessageFactory_10());
- ofFactoryMap.put(OFVersion.OF_13, floodlightProvider.getOFMessageFactory_13());
floodlightProvider.addOFMessageListener(OFType.BARRIER_REPLY, this);
}
@@ -415,12 +410,6 @@
* Begin processing queue.
*/
public void start() {
- // TODO BOC
- // if (factory == null) {
- // log.error("FlowPusher not yet initialized.");
- // return;
- // }
-
threadMap = new HashMap<Long, FlowPusherThread>();
for (long i = 0; i < numberThread; ++i) {
FlowPusherThread thread = new FlowPusherThread();
@@ -612,7 +601,7 @@
//
// Create the OpenFlow Flow Modification Entry to push
//
- OFFlowMod fm = flowEntry.buildFlowMod(ofFactoryMap.get(sw.getOFVersion()));
+ OFFlowMod fm = flowEntry.buildFlowMod(sw.getFactory());
// log.trace("Pushing flow mod {}", fm);
return addMessageImpl(sw, fm, priority);
}
@@ -682,7 +671,7 @@
}
protected OFBarrierRequest createBarrierRequest(IOFSwitch sw) {
- OFFactory factory = ofFactoryMap.get(sw.getOFVersion());
+ OFFactory factory = sw.getFactory();
if (factory == null) {
log.error("No OF Message Factory for switch {} with OFVersion {}", sw,
sw.getOFVersion());
diff --git a/src/main/java/net/onrc/onos/core/linkdiscovery/LinkDiscoveryManager.java b/src/main/java/net/onrc/onos/core/linkdiscovery/LinkDiscoveryManager.java
index 4c87501..fbd06b7 100644
--- a/src/main/java/net/onrc/onos/core/linkdiscovery/LinkDiscoveryManager.java
+++ b/src/main/java/net/onrc/onos/core/linkdiscovery/LinkDiscoveryManager.java
@@ -57,7 +57,6 @@
import net.onrc.onos.core.registry.IControllerRegistryService;
import net.onrc.onos.core.util.SwitchPort;
-import org.projectfloodlight.openflow.protocol.OFFactories;
import org.projectfloodlight.openflow.protocol.OFFactory;
import org.projectfloodlight.openflow.protocol.OFMessage;
import org.projectfloodlight.openflow.protocol.OFPacketIn;
@@ -68,7 +67,6 @@
import org.projectfloodlight.openflow.protocol.OFPortState;
import org.projectfloodlight.openflow.protocol.OFPortStatus;
import org.projectfloodlight.openflow.protocol.OFType;
-import org.projectfloodlight.openflow.protocol.OFVersion;
import org.projectfloodlight.openflow.protocol.action.OFAction;
import org.projectfloodlight.openflow.types.OFBufferId;
import org.projectfloodlight.openflow.types.OFPort;
@@ -106,9 +104,6 @@
private static final Logger log =
LoggerFactory.getLogger(LinkDiscoveryManager.class);
- // TODO Remove these factories.
- protected OFFactory factory13 = OFFactories.getFactory(OFVersion.OF_13);
- protected OFFactory factory10 = OFFactories.getFactory(OFVersion.OF_10);
private IFloodlightProviderService controller;
@@ -322,8 +317,7 @@
sw, port);
}
- OFFactory factory = (iofSwitch.getOFVersion() == OFVersion.OF_10)
- ? factory10 : factory13;
+ OFFactory factory = iofSwitch.getFactory();
OFPacketOut po = createLLDPPacketOut(sw, ofpPort, isReverse, factory);
try {
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 63143cd..ca4c4b1 100644
--- a/src/main/java/net/onrc/onos/core/packetservice/PacketModule.java
+++ b/src/main/java/net/onrc/onos/core/packetservice/PacketModule.java
@@ -54,7 +54,6 @@
private Topology topology;
private IDatagridService datagrid;
private IFlowPusherService flowPusher;
- private OFFactory factory;
private IEventChannel<Long, PacketOutNotification> packetOutEventChannel;
@@ -221,7 +220,6 @@
.getTopology();
datagrid = context.getServiceImpl(IDatagridService.class);
flowPusher = context.getServiceImpl(IFlowPusherService.class);
- factory = floodlightProvider.getOFMessageFactory_10();
}
@Override
@@ -243,6 +241,8 @@
continue;
}
+ OFFactory factory = sw.getFactory();
+
List<OFAction> actions = new ArrayList<>();
for (Short port : outPorts.get(dpid)) {
actions.add(factory.actions().output(OFPort.of(port), Short.MAX_VALUE));
diff --git a/src/test/java/net/floodlightcontroller/core/test/MockFloodlightProvider.java b/src/test/java/net/floodlightcontroller/core/test/MockFloodlightProvider.java
index a56a273..c5c15c1 100644
--- a/src/test/java/net/floodlightcontroller/core/test/MockFloodlightProvider.java
+++ b/src/test/java/net/floodlightcontroller/core/test/MockFloodlightProvider.java
@@ -48,12 +48,9 @@
import net.onrc.onos.core.packet.Ethernet;
import net.onrc.onos.core.util.OnosInstanceId;
-import org.projectfloodlight.openflow.protocol.OFFactories;
-import org.projectfloodlight.openflow.protocol.OFFactory;
import org.projectfloodlight.openflow.protocol.OFMessage;
import org.projectfloodlight.openflow.protocol.OFPacketIn;
import org.projectfloodlight.openflow.protocol.OFType;
-import org.projectfloodlight.openflow.protocol.OFVersion;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
@@ -72,10 +69,6 @@
protected ConcurrentHashMap<Long, IOFSwitch> activeMasterSwitches;
protected ConcurrentHashMap<Long, IOFSwitch> activeEqualSwitches;
- // protected BasicFactory factory;
- protected static OFFactory factory13 = OFFactories.getFactory(OFVersion.OF_13);
- protected static OFFactory factory10 = OFFactories.getFactory(OFVersion.OF_10);
-
/**
*
*/
@@ -317,18 +310,6 @@
}
@Override
- public OFFactory getOFMessageFactory_13() {
- // TODO to be checked
- return factory13;
- }
-
- @Override
- public OFFactory getOFMessageFactory_10() {
- // TODO to be checked
- return factory10;
- }
-
- @Override
public Counters getCounters() {
// TODO Auto-generated method stub
return null;
diff --git a/src/test/java/net/floodlightcontroller/util/OFMessageDamperMockSwitch.java b/src/test/java/net/floodlightcontroller/util/OFMessageDamperMockSwitch.java
index 8aadaf3..545753d 100644
--- a/src/test/java/net/floodlightcontroller/util/OFMessageDamperMockSwitch.java
+++ b/src/test/java/net/floodlightcontroller/util/OFMessageDamperMockSwitch.java
@@ -25,6 +25,7 @@
import org.projectfloodlight.openflow.protocol.OFActionType;
import org.projectfloodlight.openflow.protocol.OFCapabilities;
import org.projectfloodlight.openflow.protocol.OFDescStatsReply;
+import org.projectfloodlight.openflow.protocol.OFFactory;
import org.projectfloodlight.openflow.protocol.OFMessage;
import org.projectfloodlight.openflow.protocol.OFPortDesc;
import org.projectfloodlight.openflow.protocol.OFPortStatus;
@@ -475,4 +476,10 @@
}
+ @Override
+ public OFFactory getFactory() {
+ // TODO Auto-generated method stub
+ return null;
+ }
+
}
\ No newline at end of file
diff --git a/src/test/java/net/onrc/onos/core/flowprogrammer/FlowPusherTest.java b/src/test/java/net/onrc/onos/core/flowprogrammer/FlowPusherTest.java
index ae30949..28b445c 100644
--- a/src/test/java/net/onrc/onos/core/flowprogrammer/FlowPusherTest.java
+++ b/src/test/java/net/onrc/onos/core/flowprogrammer/FlowPusherTest.java
@@ -472,8 +472,6 @@
//factory10 = EasyMock.createMock(OFFactories.getFactory(OFVersion.OF_10).getClass());
flProviderService = EasyMock.createMock(IFloodlightProviderService.class);
threadPoolService = EasyMock.createMock(IThreadPoolService.class);
- EasyMock.expect(flProviderService.getOFMessageFactory_10()).andReturn(factory10).anyTimes();
- EasyMock.expect(flProviderService.getOFMessageFactory_13()).andReturn(null).anyTimes();
EasyMock.expect(modContext.getServiceImpl(EasyMock.eq(IThreadPoolService.class)))
.andReturn(threadPoolService).once();
diff --git a/src/test/java/net/onrc/onos/core/linkdiscovery/LinkDiscoveryManagerTest.java b/src/test/java/net/onrc/onos/core/linkdiscovery/LinkDiscoveryManagerTest.java
index 7f848a8..8c63293 100644
--- a/src/test/java/net/onrc/onos/core/linkdiscovery/LinkDiscoveryManagerTest.java
+++ b/src/test/java/net/onrc/onos/core/linkdiscovery/LinkDiscoveryManagerTest.java
@@ -136,6 +136,7 @@
IOFSwitch mockSwitch = createNiceMock(IOFSwitch.class);
expect(mockSwitch.getId()).andReturn(id).anyTimes();
expect(mockSwitch.portEnabled(EasyMock.anyShort())).andReturn(true).anyTimes();
+ expect(mockSwitch.getFactory()).andReturn(factory10).anyTimes();
return mockSwitch;
}