Fixed issue which made it very slow to add switches because of refactored code which included a GremlinGroovy query
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 e9e2bd1..fd9d535 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
@@ -121,19 +121,24 @@
public void removeDevice(IDeviceObject deviceObject) {
String deviceMac = deviceObject.getMACAddress();
- for (IIpv4Address ipv4AddressVertex : deviceObject.getIpv4Addresses()) {
- ope.removeIpv4Address(ipv4AddressVertex);
- }
+ removeDeviceImpl(deviceObject);
try {
- ope.removeDevice(deviceObject);
ope.commit();
- log.error("DeviceStorage:removeDevice mac:{} done", deviceMac);
+ log.debug("DeviceStorage:removeDevice mac:{} done", deviceMac);
} catch (TitanException e) {
ope.rollback();
log.error("DeviceStorage:removeDevice mac:{} failed", deviceMac);
}
}
+
+ public void removeDeviceImpl(IDeviceObject deviceObject) {
+ for (IIpv4Address ipv4AddressVertex : deviceObject.getIpv4Addresses()) {
+ ope.removeIpv4Address(ipv4AddressVertex);
+ }
+
+ ope.removeDevice(deviceObject);
+ }
/***
* This function is for getting the Device from the DB by Mac address of the device.
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 59f59b7..dcfdc73 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
@@ -147,28 +147,28 @@
}
for (OFPhysicalPort port: sw.getPorts()) {
- addPort(dpid, port);
- /*IPortObject p = op.searchPort(dpid, port.getPortNumber());
- if (p != null) {
- log.debug("SwitchStorage:addPort dpid:{} port:{} exists", dpid, port.getPortNumber());
- setPortStateImpl(p, port.getState(), port.getName());
- p.setState("ACTIVE");
- if (curr.getPort(port.getPortNumber()) == null) {
- // The port exists but the switch has no "on" link to it
- curr.addPort(p);
- }
- } else {
- p = addPortImpl(curr, port.getPortNumber());
- setPortStateImpl(p, port.getState(), port.getName());
- } */
+ //addPort(dpid, port);
+ addPortImpl(curr, port);
+
}
+
+ // XXX for now delete devices when we change a port to prevent
+ // having stale devices.
+ DeviceStorageImpl deviceStorage = new DeviceStorageImpl();
+ deviceStorage.init("");
+ for (IPortObject portObject : curr.getPorts()) {
+ for (IDeviceObject deviceObject : portObject.getDevices()) {
+ // The deviceStorage has to remove on the object gained by its own
+ // FramedGraph, it can't use our objects from here
+ deviceStorage.removeDeviceImpl(deviceStorage.getDeviceByMac(deviceObject.getMACAddress()));
+ }
+ }
+
op.commit();
success = true;
} catch (Exception e) {
op.rollback();
- //e.printStackTrace();
- log.error("SwitchStorage:addSwitch dpid:{} failed: {}", dpid);
- log.error("switch write error", e);
+ log.error("SwitchStorage:addSwitch dpid:{} failed", dpid, e);
}
return success;
@@ -292,9 +292,6 @@
public boolean addPort(String dpid, OFPhysicalPort phport) {
boolean success = false;
- DeviceStorageImpl deviceStorage = new DeviceStorageImpl();
- deviceStorage.init("");
-
if(((OFPortConfig.OFPPC_PORT_DOWN.getValue() & phport.getConfig()) > 0) ||
((OFPortState.OFPPS_LINK_DOWN.getValue() & phport.getState()) > 0)) {
// just dispatch to deletePort()
@@ -307,27 +304,17 @@
ISwitchObject sw = op.searchSwitch(dpid);
if (sw != null) {
- IPortObject p = sw.getPort(phport.getPortNumber());
- log.info("SwitchStorage:addPort dpid:{} port:{}", dpid, phport.getPortNumber());
- if (p != null) {
- setPortStateImpl(p, phport.getState(), phport.getName());
-
- if (sw.getPort(phport.getPortNumber()) == null) {
- // The port exists but the switch has no "on" link to it
- sw.addPort(p);
- }
-
- // XXX for now delete devices when we change a port to prevent
- // having stale devices.
- for (IDeviceObject deviceObject : p.getDevices()) {
- deviceStorage.removeDevice(deviceObject);
- }
-
- log.error("SwitchStorage:addPort dpid:{} port:{} exists setting as ACTIVE", dpid, phport.getPortNumber());
- } else {
- addPortImpl(sw, phport.getPortNumber());
- setPortStateImpl(p, phport.getState(), phport.getName());
- }
+ IPortObject portObject = addPortImpl(sw, phport);
+
+ // XXX for now delete devices when we change a port to prevent
+ // having stale devices.
+ DeviceStorageImpl deviceStorage = new DeviceStorageImpl();
+ deviceStorage.init("");
+
+ for (IDeviceObject deviceObject : portObject.getDevices()) {
+ deviceStorage.removeDevice(deviceObject);
+ }
+
op.commit();
success = true;
} else {
@@ -337,8 +324,8 @@
op.rollback();
e.printStackTrace();
log.error("SwitchStorage:addPort dpid:{} port:{} failed", dpid, phport.getPortNumber());
- }
-
+ }
+
return success;
}
@@ -430,12 +417,59 @@
}
}
+
+ private IPortObject addPortImpl(ISwitchObject sw, OFPhysicalPort phport) {
+ IPortObject portObject = op.searchPort(sw.getDPID(), phport.getPortNumber());
+
+ log.info("SwitchStorage:addPort dpid:{} port:{}",
+ sw.getDPID(), phport.getPortNumber());
+
+ if (portObject != null) {
+ setPortStateImpl(portObject, phport.getState(), phport.getName());
+ portObject.setState("ACTIVE");
+
+ // This a convoluted way of checking if the port is attached
+ // or not, but doing it this way avoids using the
+ // ISwitchObject.getPort method which uses GremlinGroovy query
+ // and takes forever.
+ boolean attached = false;
+ for (IPortObject portsOnSwitch : sw.getPorts()) {
+ if (portsOnSwitch.getPortId() == portObject.getPortId()) {
+ attached = true;
+ break;
+ }
+ }
+
+ if (!attached) {
+ sw.addPort(portObject);
+ }
+
+ /*
+ if (sw.getPort(phport.getPortNumber()) == null) {
+ // The port exists but the switch has no "on" link to it
+ sw.addPort(portObject);
+ }*/
+
+ log.info("SwitchStorage:addPort dpid:{} port:{} exists setting as ACTIVE",
+ sw.getDPID(), phport.getPortNumber());
+ } else {
+ //addPortImpl(sw, phport.getPortNumber());
+ portObject = op.newPort(sw.getDPID(), phport.getPortNumber());
+ portObject.setState("ACTIVE");
+ setPortStateImpl(portObject, phport.getState(), phport.getName());
+ sw.addPort(portObject);
+ log.info("SwitchStorage:addPort dpid:{} port:{} done",
+ sw.getDPID(), phport.getPortNumber());
+ }
+
+ return portObject;
+ }
// TODO There's an issue here where a port with that ID could already
// exist when we try to add this one (because it's left over from an
// old topology). We need to remove an old port with the same ID when
// we add the new port. Also it seems that old ports like this are
// never cleaned up and will remain in the DB in the ACTIVE state forever.
- private IPortObject addPortImpl(ISwitchObject sw, short portNum) {
+ /*private IPortObject addPortImpl(ISwitchObject sw, short portNum) {
IPortObject p = op.newPort(sw.getDPID(), portNum);
p.setState("ACTIVE");
sw.addPort(p);
@@ -443,7 +477,7 @@
sw.getDPID(), portNum);
return p;
- }
+ }*/
private void setPortStateImpl(IPortObject port, Integer state, String desc) {
if (port != null) {