Merge pull request #450 from jonohart/master
Fix for an issue with writing switches to the database.
diff --git a/src/main/java/net/floodlightcontroller/devicemanager/internal/DeviceManagerImpl.java b/src/main/java/net/floodlightcontroller/devicemanager/internal/DeviceManagerImpl.java
index 031a528..087756c 100755
--- a/src/main/java/net/floodlightcontroller/devicemanager/internal/DeviceManagerImpl.java
+++ b/src/main/java/net/floodlightcontroller/devicemanager/internal/DeviceManagerImpl.java
@@ -39,21 +39,22 @@
import net.floodlightcontroller.core.FloodlightContext;
import net.floodlightcontroller.core.IFloodlightProviderService;
+import net.floodlightcontroller.core.IFloodlightProviderService.Role;
import net.floodlightcontroller.core.IHAListener;
import net.floodlightcontroller.core.IInfoProvider;
import net.floodlightcontroller.core.IOFMessageListener;
import net.floodlightcontroller.core.IOFSwitch;
-import net.floodlightcontroller.core.IFloodlightProviderService.Role;
+import net.floodlightcontroller.core.IUpdate;
import net.floodlightcontroller.core.module.FloodlightModuleContext;
import net.floodlightcontroller.core.module.IFloodlightModule;
import net.floodlightcontroller.core.module.IFloodlightService;
import net.floodlightcontroller.core.util.SingletonTask;
import net.floodlightcontroller.devicemanager.IDevice;
+import net.floodlightcontroller.devicemanager.IDeviceListener;
import net.floodlightcontroller.devicemanager.IDeviceService;
import net.floodlightcontroller.devicemanager.IEntityClass;
import net.floodlightcontroller.devicemanager.IEntityClassListener;
import net.floodlightcontroller.devicemanager.IEntityClassifierService;
-import net.floodlightcontroller.devicemanager.IDeviceListener;
import net.floodlightcontroller.devicemanager.SwitchPort;
import net.floodlightcontroller.devicemanager.web.DeviceRoutable;
import net.floodlightcontroller.flowcache.IFlowReconcileListener;
@@ -71,8 +72,6 @@
import net.floodlightcontroller.topology.ITopologyService;
import net.floodlightcontroller.util.MultiIterator;
import net.onrc.onos.ofcontroller.linkdiscovery.ILinkDiscovery.LDUpdate;
-import static net.floodlightcontroller.devicemanager.internal.
-DeviceManagerImpl.DeviceUpdate.Change.*;
import org.openflow.protocol.OFMatchWithSwDpid;
import org.openflow.protocol.OFMessage;
@@ -197,14 +196,14 @@
*/
protected Set<IDeviceListener> deviceListeners;
+ public enum DeviceUpdateType {
+ ADD, DELETE, CHANGE, MOVED;
+ }
+
/**
* A device update event to be dispatched
*/
- protected static class DeviceUpdate {
- public enum Change {
- ADD, DELETE, CHANGE;
- }
-
+ protected class DeviceUpdate implements IUpdate {
/**
* The affected device
*/
@@ -213,18 +212,18 @@
/**
* The change that was made
*/
- protected Change change;
+ protected DeviceUpdateType updateType;
/**
* If not added, then this is the list of fields changed
*/
protected EnumSet<DeviceField> fieldsChanged;
- public DeviceUpdate(IDevice device, Change change,
+ public DeviceUpdate(IDevice device, DeviceUpdateType updateType,
EnumSet<DeviceField> fieldsChanged) {
super();
this.device = device;
- this.change = change;
+ this.updateType = updateType;
this.fieldsChanged = fieldsChanged;
}
@@ -232,9 +231,49 @@
public String toString() {
String devIdStr = device.getEntityClass().getName() + "::" +
device.getMACAddressString();
- return "DeviceUpdate [device=" + devIdStr + ", change=" + change
+ return "DeviceUpdate [device=" + devIdStr + ", updateType=" + updateType
+ ", fieldsChanged=" + fieldsChanged + "]";
}
+
+ @Override
+ public void dispatch() {
+ if (logger.isTraceEnabled()) {
+ logger.trace("Dispatching device update: {}", this);
+ }
+ for (IDeviceListener listener : deviceListeners) {
+ switch (updateType) {
+ case ADD:
+ listener.deviceAdded(device);
+ break;
+ case DELETE:
+ listener.deviceRemoved(device);
+ break;
+ case CHANGE:
+ for (DeviceField field : fieldsChanged) {
+ switch (field) {
+ case IPV4:
+ listener.deviceIPV4AddrChanged(device);
+ break;
+ case SWITCH:
+ case PORT:
+ //listener.deviceMoved(update.device);
+ break;
+ case VLAN:
+ listener.deviceVlanChanged(device);
+ break;
+ default:
+ logger.debug("Unknown device field changed {}",
+ fieldsChanged.toString());
+ break;
+ }
+ }
+ break;
+ case MOVED:
+ listener.deviceMoved(device);
+ break;
+ }
+ }
+ }
}
@@ -1104,7 +1143,7 @@
// generate new device update
deviceUpdates =
updateUpdates(deviceUpdates,
- new DeviceUpdate(device, ADD, null));
+ new DeviceUpdate(device, DeviceUpdateType.ADD, null));
break;
}
@@ -1162,7 +1201,7 @@
if (changedFields.size() > 0)
deviceUpdates =
updateUpdates(deviceUpdates,
- new DeviceUpdate(newDevice, CHANGE,
+ new DeviceUpdate(newDevice, DeviceUpdateType.CHANGE,
changedFields));
// update the device map with a replace call
@@ -1211,7 +1250,7 @@
// generate new device update
deviceUpdates =
updateUpdates(deviceUpdates,
- new DeviceUpdate(dev, DELETE, null));
+ new DeviceUpdate(dev, DeviceUpdateType.DELETE, null));
}
}
@@ -1267,6 +1306,15 @@
* @param updates the updates to process.
*/
protected void processUpdates(Queue<DeviceUpdate> updates) {
+ if (updates == null) {
+ return;
+ }
+
+ DeviceUpdate update;
+ while (null != (update = updates.poll())) {
+ floodlightProvider.publishUpdate(update);
+ }
+ /*
if (updates == null) return;
DeviceUpdate update = null;
while (null != (update = updates.poll())) {
@@ -1304,6 +1352,7 @@
}
}
}
+ */
}
/**
@@ -1481,7 +1530,7 @@
changedFields.addAll(findChangedFields(newDevice, e));
}
if (changedFields.size() > 0)
- deviceUpdates.add(new DeviceUpdate(d, CHANGE,
+ deviceUpdates.add(new DeviceUpdate(d, DeviceUpdateType.CHANGE,
changedFields));
if (!deviceMap.replace(newDevice.getDeviceKey(),
@@ -1495,7 +1544,7 @@
continue;
}
} else {
- deviceUpdates.add(new DeviceUpdate(d, DELETE, null));
+ deviceUpdates.add(new DeviceUpdate(d, DeviceUpdateType.DELETE, null));
if (!deviceMap.remove(d.getDeviceKey(), d))
// concurrent modification; try again
// need to use device that is the map now for the next
@@ -1665,9 +1714,11 @@
* @param updates the updates to process.
*/
protected void sendDeviceMovedNotification(Device d) {
- for (IDeviceListener listener : deviceListeners) {
+ /*for (IDeviceListener listener : deviceListeners) {
listener.deviceMoved(d);
- }
+ }*/
+ floodlightProvider.publishUpdate(
+ new DeviceUpdate(d, DeviceUpdateType.MOVED, null));
}
/**
@@ -1705,8 +1756,7 @@
new LinkedList<DeviceUpdate>();
// delete this device and then re-learn all the entities
this.deleteDevice(device);
- deviceUpdates.add(new DeviceUpdate(device,
- DeviceUpdate.Change.DELETE, null));
+ deviceUpdates.add(new DeviceUpdate(device, DeviceUpdateType.DELETE, null));
if (!deviceUpdates.isEmpty())
processUpdates(deviceUpdates);
for (Entity entity: device.entities ) {
diff --git a/src/main/java/net/onrc/onos/ofcontroller/core/ISwitchStorage.java b/src/main/java/net/onrc/onos/ofcontroller/core/ISwitchStorage.java
index 1c243c0..2cfab3f 100644
--- a/src/main/java/net/onrc/onos/ofcontroller/core/ISwitchStorage.java
+++ b/src/main/java/net/onrc/onos/ofcontroller/core/ISwitchStorage.java
@@ -34,6 +34,10 @@
*/
public boolean deleteSwitch(String dpid);
/*
+ * Deactivate the switch and associated ports
+ */
+ public boolean deactivateSwitch(String dpid);
+ /*
* Update the port details
*/
public boolean updatePort(String dpid, short port, int state, String desc);
diff --git a/src/main/java/net/onrc/onos/ofcontroller/core/internal/DeviceStorageImpl.java b/src/main/java/net/onrc/onos/ofcontroller/core/internal/DeviceStorageImpl.java
index 6438ffd..cfc411c 100644
--- a/src/main/java/net/onrc/onos/ofcontroller/core/internal/DeviceStorageImpl.java
+++ b/src/main/java/net/onrc/onos/ofcontroller/core/internal/DeviceStorageImpl.java
@@ -77,9 +77,6 @@
changeDeviceAttachments(device, obj);
changeDeviceIpv4Addresses(device, obj);
- /*for (Integer intIpv4Address : device.getIPv4Addresses()) {
- obj.addIpv4Address(ope.ensureIpv4Address(intIpv4Address));
- }*/
obj.setMACAddress(device.getMACAddressString());
obj.setType("device");
@@ -89,7 +86,7 @@
//log.debug("Adding device {}",device.getMACAddressString());
} catch (TitanException e) {
ope.rollback();
- log.error(":addDevice mac:{} failed", device.getMACAddressString());
+ log.error("Adding device {} failed", device.getMACAddressString(), e);
obj = null;
}
diff --git a/src/main/java/net/onrc/onos/ofcontroller/core/internal/SwitchStorageImpl.java b/src/main/java/net/onrc/onos/ofcontroller/core/internal/SwitchStorageImpl.java
index 7f0c259..6377605 100644
--- a/src/main/java/net/onrc/onos/ofcontroller/core/internal/SwitchStorageImpl.java
+++ b/src/main/java/net/onrc/onos/ofcontroller/core/internal/SwitchStorageImpl.java
@@ -62,6 +62,14 @@
* @param state The state of the switch like ACTIVE, INACTIVE
* @param dmope The DM_OPERATION of the switch
*/
+ /*
+ * Jono, 11/8/2013
+ * We don't need this update method that demultiplexes DM_OPERATIONS,
+ * we can have clients just call the required methods directly.
+ * We especially don't need this update method to re-implement
+ * the functions of other methods.
+ */
+ @Deprecated
@Override
public boolean updateSwitch(String dpid, SwitchState state, DM_OPERATION dmope) {
boolean success = false;
@@ -191,7 +199,7 @@
} catch (Exception e) {
op.rollback();
e.printStackTrace();
- log.info("SwitchStorage:addSwitch dpid:{} failed", dpid);
+ log.error("SwitchStorage:addSwitch dpid:{} failed", dpid, e);
}
return success;
@@ -220,6 +228,32 @@
return success;
}
+
+ public boolean deactivateSwitch(String dpid) {
+ boolean success = false;
+
+ try {
+ ISwitchObject switchObject = op.searchSwitch(dpid);
+ if (switchObject != null) {
+ setSwitchStateImpl(switchObject, SwitchState.INACTIVE);
+
+ for (IPortObject portObject : switchObject.getPorts()) {
+ portObject.setState("INACTIVE");
+ }
+ op.commit();
+ success = true;
+ }
+ else {
+ log.warn("Switch {} not found when trying to deactivate", dpid);
+ }
+ } catch (Exception e) {
+ // TODO what type of exception is thrown when we can't commit?
+ op.rollback();
+ log.error("SwitchStorage:deactivateSwitch {} failed", dpid, e);
+ }
+
+ return success;
+ }
public boolean updatePort(String dpid, short portNum, int state, String desc) {
boolean success = false;
@@ -259,6 +293,8 @@
if(((OFPortConfig.OFPPC_PORT_DOWN.getValue() & phport.getConfig()) > 0) ||
((OFPortState.OFPPS_LINK_DOWN.getValue() & phport.getState()) > 0)) {
// just dispatch to deletePort()
+ // TODO This is wrong. We need to make sure the port is in the
+ // DB with the correct info and port state.
return deletePort(dpid, phport.getPortNumber());
}
@@ -305,7 +341,8 @@
IPortObject p = sw.getPort(port);
if (p != null) {
log.info("SwitchStorage:deletePort dpid:{} port:{} found and set INACTIVE", dpid, port);
- deletePortImpl(p);
+ //deletePortImpl(p);
+ p.setState("INACTIVE");
op.commit();
}
}
@@ -313,7 +350,7 @@
} catch (Exception e) {
op.rollback();
e.printStackTrace();
- log.info("SwitchStorage:deletePort dpid:{} port:{} failed", dpid, port);
+ log.error("SwitchStorage:deletePort dpid:{} port:{} failed", dpid, port);
}
return success;
diff --git a/src/main/java/net/onrc/onos/ofcontroller/floodlightlistener/NetworkGraphPublisher.java b/src/main/java/net/onrc/onos/ofcontroller/floodlightlistener/NetworkGraphPublisher.java
index 648f6f1..6364a2f 100644
--- a/src/main/java/net/onrc/onos/ofcontroller/floodlightlistener/NetworkGraphPublisher.java
+++ b/src/main/java/net/onrc/onos/ofcontroller/floodlightlistener/NetworkGraphPublisher.java
@@ -108,7 +108,8 @@
List<Link> reverseLinks = linkStore.getReverseLinks(HexString.toHexString(dpid));
links.addAll(reverseLinks);
- if (swStore.updateSwitch(HexString.toHexString(dpid), SwitchState.INACTIVE, DM_OPERATION.UPDATE)) {
+ //if (swStore.updateSwitch(HexString.toHexString(dpid), SwitchState.INACTIVE, DM_OPERATION.UPDATE)) {
+ if (swStore.deactivateSwitch(HexString.toHexString(dpid))) {
registryService.releaseControl(dpid);
// TODO publish UPDATE_SWITCH event here
@@ -268,6 +269,7 @@
@Override
public void removedSwitch(IOFSwitch sw) {
+ /*
if (registryService.hasControl(sw.getId())) {
// Get the affected ports
List<Short> ports = swStore.getPorts(HexString.toHexString(sw.getId()));
@@ -300,6 +302,7 @@
}
}
}
+ */
}
@Override
diff --git a/src/main/resources/floodlightdefault.properties b/src/main/resources/floodlightdefault.properties
index 8decafb..9f83ec8 100644
--- a/src/main/resources/floodlightdefault.properties
+++ b/src/main/resources/floodlightdefault.properties
@@ -2,7 +2,6 @@
net.floodlightcontroller.core.FloodlightProvider,\
net.floodlightcontroller.threadpool.ThreadPool,\
net.floodlightcontroller.devicemanager.internal.DeviceManagerImpl,\
-net.floodlightcontroller.jython.JythonDebugInterface,\
net.floodlightcontroller.counter.CounterStore,\
net.floodlightcontroller.perfmon.PktInProcessingTime,\
net.floodlightcontroller.ui.web.StaticWebRoutable,\
@@ -10,7 +9,6 @@
net.onrc.onos.registry.controller.ZookeeperRegistry
net.floodlightcontroller.restserver.RestApiServer.port = 8080
net.floodlightcontroller.core.FloodlightProvider.openflowport = 6633
-net.floodlightcontroller.jython.JythonDebugInterface.port = 6655
net.floodlightcontroller.forwarding.Forwarding.idletimeout = 5
net.floodlightcontroller.forwarding.Forwarding.hardtimeout = 0
net.onrc.onos.ofcontroller.floodlightlistener.NetworkGraphPublisher.dbconf = /tmp/cassandra.titan