From 8a95dc4bfc46ee682c22bc6106ed21b16dc675bc Mon Sep 17 00:00:00 2001 From: Gregor Maier <gregor.maier@bigswitch.com> Date: Wed, 15 May 2013 15:58:09 -0700 Subject: [PATCH] Port names should be case-insentive Comaprisions of port names and retrivials from IOFSwitch.getPort(String) are now case-insentivie. However, we still store port names with their original case. This is a quick-fix were we lower-case the port name before adding it to the port map in PortManager. Should really use a map with case-insensitive keys... --- .../net/floodlightcontroller/core/IOFSwitch.java | 1 + .../floodlightcontroller/core/ImmutablePort.java | 5 ++++- .../floodlightcontroller/core/OFSwitchBase.java | 16 +++++++++++----- 3 files changed, 16 insertions(+), 6 deletions(-) diff --git a/src/main/java/net/floodlightcontroller/core/IOFSwitch.java b/src/main/java/net/floodlightcontroller/core/IOFSwitch.java index 5602a2632..017837e40 100644 --- a/src/main/java/net/floodlightcontroller/core/IOFSwitch.java +++ b/src/main/java/net/floodlightcontroller/core/IOFSwitch.java @@ -255,6 +255,7 @@ public interface IOFSwitch { * 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. + * Port names are case insentive * @param portName * @return port object */ diff --git a/src/main/java/net/floodlightcontroller/core/ImmutablePort.java b/src/main/java/net/floodlightcontroller/core/ImmutablePort.java index 5731b92d4..b04b92925 100644 --- a/src/main/java/net/floodlightcontroller/core/ImmutablePort.java +++ b/src/main/java/net/floodlightcontroller/core/ImmutablePort.java @@ -21,6 +21,9 @@ import org.openflow.protocol.OFPhysicalPort.PortSpeed; * instead of integer bitmaps to represent * OFPortConfig, OFPortState, and OFPortFeature bitmaps. * + * Port names are stored with the original case but equals() and XXXX use + * case-insentivie comparisions for port names!! + * * TODO: create a Builder so we can easily construct OFPhysicalPorts * TODO: should we verify / ensure that the features make sense, i.e., that * currentFeatures IsSubsetOf advertisedFeatures IsSubsetOf @@ -281,7 +284,7 @@ public class ImmutablePort { if (portNumber != other.portNumber) return false; if (name == null) { if (other.name != null) return false; - } else if (!name.equals(other.name)) return false; + } else if (!name.equalsIgnoreCase(other.name)) return false; if (advertisedFeatures == null) { if (other.advertisedFeatures != null) return false; } else if (!advertisedFeatures.equals(other.advertisedFeatures)) diff --git a/src/main/java/net/floodlightcontroller/core/OFSwitchBase.java b/src/main/java/net/floodlightcontroller/core/OFSwitchBase.java index 211dc57a9..0c0739e87 100644 --- a/src/main/java/net/floodlightcontroller/core/OFSwitchBase.java +++ b/src/main/java/net/floodlightcontroller/core/OFSwitchBase.java @@ -176,6 +176,8 @@ public abstract class OFSwitchBase implements IOFSwitch { * with the previous mapping(s) the class will delete all previous ports * with name or number of the new port and then add the new port. * + * Port names are stored as-is but they are compared case-insensitive + * * The methods that change the stored ports return a list of * PortChangeEvents that represent the changes that have been applied * to the port list so that IOFSwitchListeners can be notified about the @@ -232,7 +234,7 @@ public abstract class OFSwitchBase implements IOFSwitch { for(ImmutablePort p: newPortsByNumber.values()) { newPortList.add(p); - newPortsByName.put(p.getName(), p); + newPortsByName.put(p.getName().toLowerCase(), p); if (p.isEnabled()) { newEnabledPortList.add(p); newEnabledPortNumbers.add(p.getPortNumber()); @@ -288,7 +290,7 @@ public abstract class OFSwitchBase implements IOFSwitch { events.add(new PortChangeEvent(prevPort, PortChangeType.DELETE)); // is there another port that has delPort's name? - prevPort = portsByName.get(delPort.getName()); + prevPort = portsByName.get(delPort.getName().toLowerCase()); if (prevPort != null) { newPortByNumber.remove(prevPort.getPortNumber()); events.add(new PortChangeEvent(prevPort, @@ -421,7 +423,7 @@ public abstract class OFSwitchBase implements IOFSwitch { // We now need to check if there exists a previous port sharing // the same name as the new/updated port. - prevPort = portsByName.get(newPort.getName()); + prevPort = portsByName.get(newPort.getName().toLowerCase()); if (prevPort != null) { // There exists a previous port with the same port // name but the port number is different (otherwise we @@ -526,7 +528,8 @@ public abstract class OFSwitchBase implements IOFSwitch { duplicatePort.toBriefString()); throw new IllegalArgumentException(msg); } - duplicatePort = newPortsByName.put(p.getName(), p); + duplicatePort = + newPortsByName.put(p.getName().toLowerCase(), p); if (duplicatePort != null) { String msg = String.format("Cannot have two ports " + "with the same name: %s <-> %s", @@ -560,9 +563,12 @@ public abstract class OFSwitchBase implements IOFSwitch { } public ImmutablePort getPort(String name) { + if (name == null) { + throw new NullPointerException("Port name must not be null"); + } lock.readLock().lock(); try { - return portsByName.get(name); + return portsByName.get(name.toLowerCase()); } finally { lock.readLock().unlock(); } -- GitLab