diff --git a/src/main/java/net/floodlightcontroller/core/IOFSwitch.java b/src/main/java/net/floodlightcontroller/core/IOFSwitch.java
index 67345322dfeb7f1e4d3707cb007562ea4d36a47c..08d2136879666f4288f4e895eaaf281add01dad6 100644
--- a/src/main/java/net/floodlightcontroller/core/IOFSwitch.java
+++ b/src/main/java/net/floodlightcontroller/core/IOFSwitch.java
@@ -18,6 +18,7 @@
package net.floodlightcontroller.core;
import java.io.IOException;
+import java.util.Collection;
import java.util.Date;
import java.util.List;
import java.util.Map;
@@ -108,9 +109,19 @@ public interface IOFSwitch {
* snapshot of the ports at the time the switch connected to the controller
* whereas this port list also reflects the port status messages that have
* been received.
- * @return Unmodifiable list of ports
+ * @return Unmodifiable list of ports not backed by the underlying collection
*/
- public List<OFPhysicalPort> getEnabledPorts();
+ public Collection<OFPhysicalPort> getEnabledPorts();
+
+ /**
+ * Get list of the port numbers of all enabled ports. This will typically
+ * be different from the list of ports in the OFFeaturesReply, since that
+ * one is a static snapshot of the ports at the time the switch connected
+ * to the controller whereas this port list also reflects the port status
+ * messages that have been received.
+ * @return Unmodifiable list of ports not backed by the underlying collection
+ */
+ public Collection<Short> getEnabledPortNumbers();
/**
* Retrieve the port object by the port number. The port object
@@ -120,7 +131,16 @@ public interface IOFSwitch {
* @return port object
*/
public OFPhysicalPort getPort(short portNumber);
-
+
+ /**
+ * Retrieve the port object by the port name. The port object
+ * is the one that reflects the port status updates that have been
+ * received, not the one from the features reply.
+ * @param portName
+ * @return port object
+ */
+ public OFPhysicalPort getPort(String portName);
+
/**
* Add or modify a switch port. This is called by the core controller
* code in response to a OFPortStatus message. It should not typically be
@@ -138,17 +158,36 @@ public interface IOFSwitch {
public void deletePort(short portNumber);
/**
- * Get the portmap
- * @return
+ * Delete a port for the switch. This is called by the core controller
+ * code in response to a OFPortStatus message. It should not typically be
+ * called by other floodlight applications.
+ * @param portName
*/
- public Map<Short, OFPhysicalPort> getPorts();
+ public void deletePort(String portName);
+
+ /**
+ * Get list of all ports. This will typically be different from
+ * the list of ports in the OFFeaturesReply, since that one is a static
+ * snapshot of the ports at the time the switch connected to the controller
+ * whereas this port list also reflects the port status messages that have
+ * been received.
+ * @return Unmodifiable list of ports
+ */
+ public Collection<OFPhysicalPort> getPorts();
+ /**
+ * @param portName
+ * @return Whether a port is enabled per latest port status message
+ * (not configured down nor link down nor in spanning tree blocking state)
+ */
+ public boolean portEnabled(short portName);
+
/**
* @param portNumber
* @return Whether a port is enabled per latest port status message
* (not configured down nor link down nor in spanning tree blocking state)
*/
- public boolean portEnabled(short portNumber);
+ public boolean portEnabled(String portName);
/**
* @param port
diff --git a/src/main/java/net/floodlightcontroller/core/internal/Controller.java b/src/main/java/net/floodlightcontroller/core/internal/Controller.java
index a29f16a4a9339ae78cb72fa2448af8f1e2323660..628ee15b969f84be23b58e463afd9e55c1906f25 100644
--- a/src/main/java/net/floodlightcontroller/core/internal/Controller.java
+++ b/src/main/java/net/floodlightcontroller/core/internal/Controller.java
@@ -1597,7 +1597,7 @@ public class Controller implements IFloodlightProviderService,
storageSource.updateRowAsync(SWITCH_TABLE_NAME, switchInfo);
// Update the ports
- for (OFPhysicalPort port: sw.getPorts().values()) {
+ for (OFPhysicalPort port: sw.getPorts()) {
updatePortInfo(sw, port);
}
}
diff --git a/src/main/java/net/floodlightcontroller/core/internal/OFSwitchImpl.java b/src/main/java/net/floodlightcontroller/core/internal/OFSwitchImpl.java
index 58395cf1f5d8836aa54728f5dc4e57a211697356..e95d35e4229470e3c8e9719655228f0dc6c7d55c 100644
--- a/src/main/java/net/floodlightcontroller/core/internal/OFSwitchImpl.java
+++ b/src/main/java/net/floodlightcontroller/core/internal/OFSwitchImpl.java
@@ -22,6 +22,7 @@ import java.net.SocketAddress;
import java.util.ArrayList;
import java.util.Arrays;
import java.util.Collection;
+import java.util.Collections;
import java.util.Date;
import java.util.HashMap;
import java.util.LinkedList;
@@ -86,7 +87,16 @@ public class OFSwitchImpl implements IOFSwitch {
protected String stringId;
protected Channel channel;
protected AtomicInteger transactionIdSource;
- protected Map<Short, OFPhysicalPort> ports;
+ // Lock to protect modification of the port maps. We only need to
+ // synchronize on modifications. For read operations we are fine since
+ // we rely on ConcurrentMaps which works for our use case.
+ private Object portLock;
+ // Map port numbers to the appropriate OFPhysicalPort
+ protected ConcurrentHashMap<Short, OFPhysicalPort> portsByNumber;
+ // Map port names to the appropriate OFPhyiscalPort
+ // XXX: The OF spec doesn't specify if port names need to be unique but
+ // according it's always the case in practice.
+ protected ConcurrentHashMap<String, OFPhysicalPort> portsByName;
protected Map<Integer,OFStatisticsFuture> statsFutureMap;
protected Map<Integer, IOFMessageListener> iofMsgListenersMap;
protected boolean connected;
@@ -134,7 +144,9 @@ public class OFSwitchImpl implements IOFSwitch {
this.attributes = new ConcurrentHashMap<Object, Object>();
this.connectedSince = new Date();
this.transactionIdSource = new AtomicInteger();
- this.ports = new ConcurrentHashMap<Short, OFPhysicalPort>();
+ this.portLock = new Object();
+ this.portsByNumber = new ConcurrentHashMap<Short, OFPhysicalPort>();
+ this.portsByName = new ConcurrentHashMap<String, OFPhysicalPort>();
this.connected = true;
this.statsFutureMap = new ConcurrentHashMap<Integer,OFStatisticsFuture>();
this.iofMsgListenersMap = new ConcurrentHashMap<Integer,IOFMessageListener>();
@@ -145,7 +157,7 @@ public class OFSwitchImpl implements IOFSwitch {
this.pendingRoleRequests = new LinkedList<OFSwitchImpl.PendingRoleRequestEntry>();
// Defaults properties for an ideal switch
- this.setAttribute(PROP_FASTWILDCARDS, (Integer) OFMatch.OFPFW_ALL);
+ this.setAttribute(PROP_FASTWILDCARDS, OFMatch.OFPFW_ALL);
this.setAttribute(PROP_SUPPORTS_OFPP_FLOOD, new Boolean(true));
this.setAttribute(PROP_SUPPORTS_OFPP_TABLE, new Boolean(true));
}
@@ -175,16 +187,19 @@ public class OFSwitchImpl implements IOFSwitch {
return this.attributes.containsKey(name);
}
+ @Override
@JsonIgnore
public Channel getChannel() {
return this.channel;
}
+ @JsonIgnore
public void setChannel(Channel channel) {
this.channel = channel;
}
// TODO: document the difference between the different write functions
+ @Override
public void write(OFMessage m, FloodlightContext bc) throws IOException {
if (m instanceof OFFlowMod) {
byte[] bcast = new byte[] {-1, -1, -1, -1, -1, -1};
@@ -215,6 +230,7 @@ public class OFSwitchImpl implements IOFSwitch {
}
}
+ @Override
public void write(List<OFMessage> msglist, FloodlightContext bc) throws IOException {
for (OFMessage m : msglist) {
if (role == Role.SLAVE) {
@@ -239,61 +255,107 @@ public class OFSwitchImpl implements IOFSwitch {
this.channel.write(msglist);
}
+ @Override
public void disconnectOutputStream() {
channel.close();
}
+ @Override
@JsonIgnore
public OFFeaturesReply getFeaturesReply() {
return this.featuresReply;
}
- public synchronized void setFeaturesReply(OFFeaturesReply featuresReply) {
- this.featuresReply = featuresReply;
- for (OFPhysicalPort port : featuresReply.getPorts()) {
- ports.put(port.getPortNumber(), port);
+ @Override
+ @JsonIgnore
+ public void setFeaturesReply(OFFeaturesReply featuresReply) {
+ synchronized(portLock) {
+ this.featuresReply = featuresReply;
+ for (OFPhysicalPort port : featuresReply.getPorts()) {
+ setPort(port);
+ }
+ this.stringId = HexString.toHexString(featuresReply.getDatapathId());
}
- this.stringId = HexString.toHexString(featuresReply.getDatapathId());
}
+ @Override
@JsonIgnore
- public synchronized List<OFPhysicalPort> getEnabledPorts() {
+ public Collection<OFPhysicalPort> getEnabledPorts() {
List<OFPhysicalPort> result = new ArrayList<OFPhysicalPort>();
- for (OFPhysicalPort port : ports.values()) {
+ for (OFPhysicalPort port : portsByNumber.values()) {
if (portEnabled(port)) {
result.add(port);
}
}
return result;
}
-
- public synchronized OFPhysicalPort getPort(short portNumber) {
- return ports.get(portNumber);
+
+ @Override
+ @JsonIgnore
+ public Collection<Short> getEnabledPortNumbers() {
+ List<Short> result = new ArrayList<Short>();
+ for (OFPhysicalPort port : portsByNumber.values()) {
+ if (portEnabled(port)) {
+ result.add(port.getPortNumber());
+ }
+ }
+ return result;
}
- public synchronized void setPort(OFPhysicalPort port) {
- ports.put(port.getPortNumber(), port);
+ @Override
+ public OFPhysicalPort getPort(short portNumber) {
+ return portsByNumber.get(portNumber);
}
+ @Override
+ public OFPhysicalPort getPort(String portName) {
+ return portsByName.get(portName);
+ }
+
+ @Override
@JsonIgnore
- public Map<Short, OFPhysicalPort> getPorts() {
- return ports;
+ public void setPort(OFPhysicalPort port) {
+ synchronized(portLock) {
+ portsByNumber.put(port.getPortNumber(), port);
+ portsByName.put(port.getName(), port);
+ }
}
+ @Override
@JsonProperty("ports")
- public Collection<OFPhysicalPort> getPortCollection() {
- return ports.values();
+ public Collection<OFPhysicalPort> getPorts() {
+ return Collections.unmodifiableCollection(portsByNumber.values());
}
-
- public synchronized void deletePort(short portNumber) {
- ports.remove(portNumber);
+
+ @Override
+ public void deletePort(short portNumber) {
+ synchronized(portLock) {
+ portsByName.remove(portsByNumber.get(portNumber).getName());
+ portsByNumber.remove(portNumber);
+ }
+ }
+
+ @Override
+ public void deletePort(String portName) {
+ synchronized(portLock) {
+ portsByNumber.remove(portsByName.get(portName).getPortNumber());
+ portsByName.remove(portName);
+ }
}
- public synchronized boolean portEnabled(short portNumber) {
- if (ports.get(portNumber) == null) return false;
- return portEnabled(ports.get(portNumber));
+ @Override
+ public boolean portEnabled(short portNumber) {
+ if (portsByNumber.get(portNumber) == null) return false;
+ return portEnabled(portsByNumber.get(portNumber));
}
+ @Override
+ public boolean portEnabled(String portName) {
+ if (portsByName.get(portName) == null) return false;
+ return portEnabled(portsByName.get(portName));
+ }
+
+ @Override
public boolean portEnabled(OFPhysicalPort port) {
if (port == null)
return false;
@@ -407,10 +469,12 @@ public class OFSwitchImpl implements IOFSwitch {
/**
* @param floodlightProvider the floodlightProvider to set
*/
+ @JsonIgnore
public void setFloodlightProvider(IFloodlightProviderService floodlightProvider) {
this.floodlightProvider = floodlightProvider;
}
+ @JsonIgnore
public void setThreadPoolService(IThreadPoolService tp) {
this.threadPool = tp;
}
@@ -422,6 +486,7 @@ public class OFSwitchImpl implements IOFSwitch {
}
@Override
+ @JsonIgnore
public synchronized void setConnected(boolean connected) {
this.connected = connected;
}
@@ -438,6 +503,7 @@ public class OFSwitchImpl implements IOFSwitch {
}
@Override
+ @JsonIgnore
public void setSwitchProperties(OFDescriptionStatistics description) {
if (switchFeatures != null) {
switchFeatures.setFromDescription(this, description);
@@ -483,6 +549,7 @@ public class OFSwitchImpl implements IOFSwitch {
}
+ @Override
public void flush() {
Map<OFSwitchImpl,List<OFMessage>> msg_buffer_map = local_msg_buffer.get();
List<OFMessage> msglist = msg_buffer_map.get(this);
diff --git a/src/main/java/net/floodlightcontroller/flowcache/FCQueryObj.java b/src/main/java/net/floodlightcontroller/flowcache/FCQueryObj.java
index f78aaaeed818ed6b24b5dcceb8152315f4454e2e..cce3401801836649a113c3ffc5d5f5ed8a06514e 100644
--- a/src/main/java/net/floodlightcontroller/flowcache/FCQueryObj.java
+++ b/src/main/java/net/floodlightcontroller/flowcache/FCQueryObj.java
@@ -69,4 +69,49 @@ public class FCQueryObj {
+ srcDevice + ", callerName=" + callerName + ", evType="
+ evType + ", callerOpaqueObj=" + callerOpaqueObj + "]";
}
+
+ @Override
+ public boolean equals(Object obj) {
+ if (this == obj)
+ return true;
+ if (obj == null)
+ return false;
+ if (getClass() != obj.getClass())
+ return false;
+ FCQueryObj other = (FCQueryObj) obj;
+ if (applInstName == null) {
+ if (other.applInstName != null)
+ return false;
+ } else if (!applInstName.equals(other.applInstName))
+ return false;
+ if (callerName == null) {
+ if (other.callerName != null)
+ return false;
+ } else if (!callerName.equals(other.callerName))
+ return false;
+ if (callerOpaqueObj == null) {
+ if (other.callerOpaqueObj != null)
+ return false;
+ } else if (!callerOpaqueObj.equals(other.callerOpaqueObj))
+ return false;
+ if (dstDevice == null) {
+ if (other.dstDevice != null)
+ return false;
+ } else if (!dstDevice.equals(other.dstDevice))
+ return false;
+ if (evType != other.evType)
+ return false;
+ if (fcQueryHandler != other.fcQueryHandler)
+ return false;
+ if (srcDevice == null) {
+ if (other.srcDevice != null)
+ return false;
+ } else if (!srcDevice.equals(other.srcDevice))
+ return false;
+ if (!Arrays.equals(vlans, other.vlans))
+ return false;
+ return true;
+ }
+
+
}
diff --git a/src/main/java/net/floodlightcontroller/topology/TopologyManager.java b/src/main/java/net/floodlightcontroller/topology/TopologyManager.java
index 71751f9950738f9a58460927c8175322ab08cb9a..404f065cd07acfff42077d8a105bc7025a025d57 100644
--- a/src/main/java/net/floodlightcontroller/topology/TopologyManager.java
+++ b/src/main/java/net/floodlightcontroller/topology/TopologyManager.java
@@ -752,9 +752,11 @@ public class TopologyManager implements
for(long sid: switches) {
IOFSwitch sw = floodlightProvider.getSwitches().get(sid);
if (sw == null) continue;
+ Collection<Short> enabledPorts = sw.getEnabledPortNumbers();
+ if (enabledPorts == null)
+ continue;
Set<Short> ports = new HashSet<Short>();
- if (sw.getPorts() == null) continue;
- ports.addAll(sw.getPorts().keySet());
+ ports.addAll(enabledPorts);
// all the ports known to topology // without tunnels.
// out of these, we need to choose only those that are
diff --git a/src/test/java/net/floodlightcontroller/core/internal/ControllerTest.java b/src/test/java/net/floodlightcontroller/core/internal/ControllerTest.java
index 3e6794d552e4de6b80d6f0821d426e47e24af840..6e897cafd2b5f072157b10a1f44be1a549a02108 100644
--- a/src/test/java/net/floodlightcontroller/core/internal/ControllerTest.java
+++ b/src/test/java/net/floodlightcontroller/core/internal/ControllerTest.java
@@ -166,7 +166,7 @@ public class ControllerTest extends FloodlightTestCase {
expect(channel.getRemoteAddress()).andReturn(null);
expect(sw.getFeaturesReply()).andReturn(featuresReply).anyTimes();
- expect(sw.getPorts()).andReturn(new HashMap<Short,OFPhysicalPort>());
+ expect(sw.getPorts()).andReturn(new ArrayList<OFPhysicalPort>());
}
/**
@@ -504,7 +504,7 @@ public class ControllerTest extends FloodlightTestCase {
expect(newsw.getChannel()).andReturn(channel2);
expect(channel2.getRemoteAddress()).andReturn(null);
expect(newsw.getFeaturesReply()).andReturn(new OFFeaturesReply()).anyTimes();
- expect(newsw.getPorts()).andReturn(new HashMap<Short,OFPhysicalPort>());
+ expect(newsw.getPorts()).andReturn(new ArrayList<OFPhysicalPort>());
controller.activeSwitches.put(0L, oldsw);
replay(newsw, channel, channel2);