diff --git a/src/main/java/net/floodlightcontroller/core/IOFSwitch.java b/src/main/java/net/floodlightcontroller/core/IOFSwitch.java index e07e5781f6f9576adbae8defc0c6a6daf9444e48..5602a2632005be490b9fce1f2dc9ddb721e8f67b 100644 --- a/src/main/java/net/floodlightcontroller/core/IOFSwitch.java +++ b/src/main/java/net/floodlightcontroller/core/IOFSwitch.java @@ -24,8 +24,6 @@ import java.util.Date; import java.util.List; import java.util.Map; import java.util.concurrent.Future; -import java.util.concurrent.locks.Lock; - import net.floodlightcontroller.core.IFloodlightProviderService.Role; import net.floodlightcontroller.core.internal.Controller; import net.floodlightcontroller.debugcounter.IDebugCounterService; @@ -34,7 +32,7 @@ import net.floodlightcontroller.threadpool.IThreadPoolService; import org.jboss.netty.channel.Channel; import org.openflow.protocol.OFFeaturesReply; import org.openflow.protocol.OFMessage; -import org.openflow.protocol.OFPhysicalPort; +import org.openflow.protocol.OFPortStatus; import org.openflow.protocol.OFStatisticsRequest; import org.openflow.protocol.statistics.OFDescriptionStatistics; import org.openflow.protocol.statistics.OFStatistics; @@ -82,6 +80,26 @@ public interface IOFSwitch { } } + /** + * Describes a change of an open flow port + */ + public static class PortChangeEvent { + public final ImmutablePort port; + public final PortChangeType type; + /** + * @param port + * @param type + */ + public PortChangeEvent(ImmutablePort port, + PortChangeType type) { + this.port = port; + this.type = type; + } + } + + /** + * the type of change that happened to an open flow port + */ public enum PortChangeType { ADD, OTHER_UPDATE, DELETE, UP, DOWN, } @@ -212,7 +230,7 @@ public interface IOFSwitch { * been received. * @return Unmodifiable list of ports not backed by the underlying collection */ - public Collection<OFPhysicalPort> getEnabledPorts(); + public Collection<ImmutablePort> getEnabledPorts(); /** * Get list of the port numbers of all enabled ports. This will typically @@ -231,7 +249,7 @@ public interface IOFSwitch { * @param portNumber * @return port object */ - public OFPhysicalPort getPort(short portNumber); + public ImmutablePort getPort(short portNumber); /** * Retrieve the port object by the port name. The port object @@ -240,31 +258,16 @@ public interface IOFSwitch { * @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 - * called by other floodlight applications. - * @param port - */ - public void setPort(OFPhysicalPort port); - - /** - * 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 portNumber - */ - public void deletePort(short portNumber); + public ImmutablePort getPort(String portName); /** - * Delete a port for the switch. This is called by the core controller + * 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 * called by other floodlight applications. - * @param portName + * @param ps the port status message */ - public void deletePort(String portName); + public List<PortChangeEvent> processOFPortStatus(OFPortStatus ps); /** * Get list of all ports. This will typically be different from @@ -274,7 +277,7 @@ public interface IOFSwitch { * been received. * @return Unmodifiable list of ports */ - public Collection<OFPhysicalPort> getPorts(); + public Collection<ImmutablePort> getPorts(); /** * @param portNumber @@ -290,12 +293,12 @@ public interface IOFSwitch { */ public boolean portEnabled(String portName); - /** - * @param port - * @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(OFPhysicalPort port); + public List<PortChangeEvent> + comparePorts(Collection<ImmutablePort> ports); + + public List<PortChangeEvent> + setPorts(Collection<ImmutablePort> ports); + /** * Get the datapathId of the switch @@ -500,23 +503,6 @@ public interface IOFSwitch { */ public void flush(); - /** - * Return a read lock that must be held while calling the listeners for - * messages from the switch. Holding the read lock prevents the active - * switch list from being modified out from under the listeners. - * @return - */ - public Lock getListenerReadLock(); - - /** - * Return a write lock that must be held when the controllers modifies the - * list of active switches. This is to ensure that the active switch list - * doesn't change out from under the listeners as they are handling a - * message from the switch. - * @return - */ - public Lock getListenerWriteLock(); - /*********************************************** * The following method can be overridden by * specific types of switches diff --git a/src/main/java/net/floodlightcontroller/core/IOFSwitchListener.java b/src/main/java/net/floodlightcontroller/core/IOFSwitchListener.java index 420cf906208bb5565f67a24b518b752860fafd28..e0c7b53f8b29eb8349f28d9f42cc97b43b309c8f 100644 --- a/src/main/java/net/floodlightcontroller/core/IOFSwitchListener.java +++ b/src/main/java/net/floodlightcontroller/core/IOFSwitchListener.java @@ -17,8 +17,6 @@ package net.floodlightcontroller.core; -import org.openflow.protocol.OFPhysicalPort; - /** * Switch lifecycle notifications. * @@ -77,7 +75,7 @@ public interface IOFSwitchListener { * @param type */ public void switchPortChanged(long switchId, - OFPhysicalPort port, + ImmutablePort port, IOFSwitch.PortChangeType type); /** diff --git a/src/main/java/net/floodlightcontroller/core/ImmutablePhysicalPort.java b/src/main/java/net/floodlightcontroller/core/ImmutablePhysicalPort.java deleted file mode 100644 index c69ad82bebbfaa66bd0361b577b252501405965f..0000000000000000000000000000000000000000 --- a/src/main/java/net/floodlightcontroller/core/ImmutablePhysicalPort.java +++ /dev/null @@ -1,118 +0,0 @@ -package net.floodlightcontroller.core; - -import java.util.Arrays; -import java.util.Collections; -import java.util.EnumSet; -import java.util.Set; - -import net.floodlightcontroller.util.EnumBitmaps; - -import org.openflow.protocol.OFPhysicalPort; -import org.openflow.protocol.OFPhysicalPort.OFPortConfig; -import org.openflow.protocol.OFPhysicalPort.OFPortFeatures; -import org.openflow.protocol.OFPhysicalPort.OFPortState; - - -/** - * An immutable version of an OFPhysical port. In addition, it uses EnumSets - * instead of integer bitmaps to represent - * OFPortConfig, OFPortState, and OFPortFeature bitmaps. - * - * @author gregor - * - */ -public class ImmutablePhysicalPort { - private final short portNumber; - private final byte[] hardwareAddress; - private final String name; - private final EnumSet<OFPortConfig> config; - private final int state; // OFPortState is a mess - private final EnumSet<OFPortFeatures> currentFeatures; - private final EnumSet<OFPortFeatures> advertisedFeatures; - private final EnumSet<OFPortFeatures> supportedFeatures; - private final EnumSet<OFPortFeatures> peerFeatures; - - public static ImmutablePhysicalPort fromOFPhysicalPort(OFPhysicalPort p) { - return new ImmutablePhysicalPort( - p.getPortNumber(), - Arrays.copyOf(p.getHardwareAddress(), 6), - p.getName(), - EnumBitmaps.toEnumSet(OFPortConfig.class, p.getConfig()), - p.getState(), - EnumBitmaps.toEnumSet(OFPortFeatures.class, - p.getCurrentFeatures()), - EnumBitmaps.toEnumSet(OFPortFeatures.class, - p.getAdvertisedFeatures()), - EnumBitmaps.toEnumSet(OFPortFeatures.class, - p.getSupportedFeatures()), - EnumBitmaps.toEnumSet(OFPortFeatures.class, - p.getPeerFeatures()) - ); - } - - /** - * @param portNumber - * @param hardwareAddress - * @param name - * @param config - * @param state - * @param currentFeatures - * @param advertisedFeatures - * @param supportedFeatures - * @param peerFeatures - */ - private ImmutablePhysicalPort(short portNumber, byte[] hardwareAddress, - String name, EnumSet<OFPortConfig> config, - int state, - EnumSet<OFPortFeatures> currentFeatures, - EnumSet<OFPortFeatures> advertisedFeatures, - EnumSet<OFPortFeatures> supportedFeatures, - EnumSet<OFPortFeatures> peerFeatures) { - this.portNumber = portNumber; - this.hardwareAddress = hardwareAddress; - this.name = name; - this.config = config; - this.state = state; - this.currentFeatures = currentFeatures; - this.advertisedFeatures = advertisedFeatures; - this.supportedFeatures = supportedFeatures; - this.peerFeatures = peerFeatures; - } - - public short getPortNumber() { - return portNumber; - } - - public byte[] getHardwareAddress() { - // FIXME: don't use arrays. - return Arrays.copyOf(hardwareAddress, 6); - } - - public String getName() { - return name; - } - - public Set<OFPortConfig> getConfig() { - return Collections.unmodifiableSet(config); - } - - public int getState() { - return state; - } - - public Set<OFPortFeatures> getCurrentFeatures() { - return Collections.unmodifiableSet(currentFeatures); - } - - public Set<OFPortFeatures> getAdvertisedFeatures() { - return Collections.unmodifiableSet(advertisedFeatures); - } - - public Set<OFPortFeatures> getSupportedFeatures() { - return Collections.unmodifiableSet(supportedFeatures); - } - - public Set<OFPortFeatures> getPeerFeatures() { - return Collections.unmodifiableSet(peerFeatures); - } -} diff --git a/src/main/java/net/floodlightcontroller/core/ImmutablePort.java b/src/main/java/net/floodlightcontroller/core/ImmutablePort.java new file mode 100644 index 0000000000000000000000000000000000000000..406a9ebdec600a0a843a9a59f0c0bc0a24a6102c --- /dev/null +++ b/src/main/java/net/floodlightcontroller/core/ImmutablePort.java @@ -0,0 +1,351 @@ +package net.floodlightcontroller.core; + +import java.util.ArrayList; +import java.util.Arrays; +import java.util.Collection; +import java.util.Collections; +import java.util.EnumSet; +import java.util.List; +import java.util.Set; + +import net.floodlightcontroller.util.EnumBitmaps; +import org.openflow.protocol.OFPhysicalPort; +import org.openflow.protocol.OFPhysicalPort.OFPortConfig; +import org.openflow.protocol.OFPhysicalPort.OFPortFeatures; +import org.openflow.protocol.OFPhysicalPort.OFPortState; +import org.openflow.protocol.OFPhysicalPort.PortSpeed; + + +/** + * An immutable version of an OFPhysical port. In addition, it uses EnumSets + * instead of integer bitmaps to represent + * OFPortConfig, OFPortState, and OFPortFeature bitmaps. + * + * 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 + * supportedFeatures + * + * @author gregor + * + */ +public class ImmutablePort { + private final short portNumber; + private final byte[] hardwareAddress; + private final String name; + private final EnumSet<OFPortConfig> config; + private final boolean portStateLinkDown; + private final OFPortState stpState; + private final EnumSet<OFPortFeatures> currentFeatures; + private final EnumSet<OFPortFeatures> advertisedFeatures; + private final EnumSet<OFPortFeatures> supportedFeatures; + private final EnumSet<OFPortFeatures> peerFeatures; + + public static ImmutablePort fromOFPhysicalPort(OFPhysicalPort p) { + if (p == null) { + throw new NullPointerException("OFPhysicalPort must not be null"); + } + + return new ImmutablePort( + + p.getPortNumber(), + Arrays.copyOf(p.getHardwareAddress(), 6), + p.getName(), + EnumBitmaps.toEnumSet(OFPortConfig.class, p.getConfig()), + OFPortState.isPortDown(p.getState()), + OFPortState.getStpState(p.getState()), + EnumBitmaps.toEnumSet(OFPortFeatures.class, + p.getCurrentFeatures()), + EnumBitmaps.toEnumSet(OFPortFeatures.class, + p.getAdvertisedFeatures()), + EnumBitmaps.toEnumSet(OFPortFeatures.class, + p.getSupportedFeatures()), + EnumBitmaps.toEnumSet(OFPortFeatures.class, + p.getPeerFeatures()) + ); + } + + public static ImmutablePort create(String name, Short portNumber) { + return new ImmutablePort(portNumber, + new byte[] { 0, 0, 0, 0, 0, 0 }, + name, + EnumSet.noneOf(OFPortConfig.class), + false, + OFPortState.OFPPS_STP_LISTEN, + EnumSet.noneOf(OFPortFeatures.class), + EnumSet.noneOf(OFPortFeatures.class), + EnumSet.noneOf(OFPortFeatures.class), + EnumSet.noneOf(OFPortFeatures.class)); + } + + /** + * Private constructor. Use factory methods. + * + * Verifies pre-conditions of arguments + * Does NOT make defensive copies. Calling factory methods are required + * to copy defensively. + * + * @param portNumber + * @param hardwareAddress + * @param name + * @param config + * @param portStateLinkDown + * @param portStateStp + * @param currentFeatures + * @param advertisedFeatures + * @param supportedFeatures + * @param peerFeatures + */ + private ImmutablePort(short portNumber, byte[] hardwareAddress, + String name, EnumSet<OFPortConfig> config, + boolean portStateLinkDown, + OFPortState portStateStp, + EnumSet<OFPortFeatures> currentFeatures, + EnumSet<OFPortFeatures> advertisedFeatures, + EnumSet<OFPortFeatures> supportedFeatures, + EnumSet<OFPortFeatures> peerFeatures) { + if (name == null) { + throw new NullPointerException("Port name must not be null"); + } + if (hardwareAddress== null) { + throw new NullPointerException("Hardware address must not be null"); + } + if (hardwareAddress.length != 6) { + throw new IllegalArgumentException("Harware address must be 6 " + + "bytes long but hardware address is " + + Arrays.toString(hardwareAddress)); + } + this.portNumber = portNumber; + this.hardwareAddress = hardwareAddress; + this.name = name; + this.config = config; + this.portStateLinkDown = portStateLinkDown; + this.stpState = portStateStp; + this.currentFeatures = currentFeatures; + this.advertisedFeatures = advertisedFeatures; + this.supportedFeatures = supportedFeatures; + this.peerFeatures = peerFeatures; + } + + public short getPortNumber() { + return portNumber; + } + + public byte[] getHardwareAddress() { + // FIXME: don't use arrays. + return Arrays.copyOf(hardwareAddress, 6); + } + + public String getName() { + return name; + } + + public Set<OFPortConfig> getConfig() { + return Collections.unmodifiableSet(config); + } + + /** + * Returns true if the OFPortState indicates the port is down + * @return + */ + public boolean isLinkDown() { + return portStateLinkDown; + } + + /** + * Returns the STP state portion of the OFPortState. The returned + * enum constant will be one of the four STP states and will have + * isStpState() return true + * @return + */ + public OFPortState getStpState() { + return this.stpState; + } + + public Set<OFPortFeatures> getCurrentFeatures() { + return Collections.unmodifiableSet(currentFeatures); + } + + public Set<OFPortFeatures> getAdvertisedFeatures() { + return Collections.unmodifiableSet(advertisedFeatures); + } + + public Set<OFPortFeatures> getSupportedFeatures() { + return Collections.unmodifiableSet(supportedFeatures); + } + + public Set<OFPortFeatures> getPeerFeatures() { + return Collections.unmodifiableSet(peerFeatures); + } + + + /** + * Returns true if the port is up, i.e., it's neither administratively + * down nor link down. It currently does NOT take STP state into + * consideration + * @return + */ + public boolean isEnabled() { + return (!portStateLinkDown && + !config.contains(OFPortConfig.OFPPC_PORT_DOWN)); + } + + /** + * @return the speed of the port (from currentFeatures) if the port is + * enabled, otherwise return SPEED_NONE + */ + public PortSpeed getCurrentPortSpeed() { + if (!isEnabled()) + return PortSpeed.SPEED_NONE; + PortSpeed maxSpeed = PortSpeed.SPEED_NONE; + for (OFPortFeatures f: currentFeatures) + PortSpeed.max(maxSpeed, f.getSpeed()); + return maxSpeed; + } + + public OFPhysicalPort toOFPhysicalPort() { + OFPhysicalPort ofpp = new OFPhysicalPort(); + ofpp.setHardwareAddress(this.getHardwareAddress()); + ofpp.setName(this.getName()); + ofpp.setConfig(EnumBitmaps.toBitmap(this.getConfig())); + int state = this.getStpState().getValue(); + if (this.isLinkDown()) + state |= OFPortState.OFPPS_LINK_DOWN.getValue(); + ofpp.setState(state); + ofpp.setCurrentFeatures(EnumBitmaps.toBitmap(this.getCurrentFeatures())); + ofpp.setAdvertisedFeatures( + EnumBitmaps.toBitmap(this.getAdvertisedFeatures())); + ofpp.setSupportedFeatures( + EnumBitmaps.toBitmap(this.getSupportedFeatures())); + ofpp.setPeerFeatures(EnumBitmaps.toBitmap(this.getPeerFeatures())); + return ofpp; + } + + /** + * Return a brief String describing this port containing the port number + * and port name + * @return + */ + public String toBriefString() { + return String.format("%d (%s)", portNumber, name); + + } + + /* (non-Javadoc) + * @see java.lang.Object#hashCode() + */ + @Override + public int hashCode() { + final int prime = 31; + int result = 1; + result = prime + * result + + ((advertisedFeatures == null) ? 0 + : advertisedFeatures.hashCode()); + result = prime * result + ((config == null) ? 0 : config.hashCode()); + result = prime + * result + + ((currentFeatures == null) ? 0 + : currentFeatures.hashCode()); + result = prime * result + Arrays.hashCode(hardwareAddress); + result = prime * result + ((name == null) ? 0 : name.hashCode()); + result = prime * result + + ((peerFeatures == null) ? 0 : peerFeatures.hashCode()); + result = prime * result + portNumber; + result = prime * result + (portStateLinkDown ? 1231 : 1237); + result = prime * result + + ((stpState == null) ? 0 : stpState.hashCode()); + result = prime + * result + + ((supportedFeatures == null) ? 0 + : supportedFeatures.hashCode()); + return result; + } + + /* (non-Javadoc) + * @see java.lang.Object#equals(java.lang.Object) + */ + @Override + public boolean equals(Object obj) { + if (this == obj) return true; + if (obj == null) return false; + if (getClass() != obj.getClass()) return false; + ImmutablePort other = (ImmutablePort) obj; + if (portNumber != other.portNumber) return false; + if (name == null) { + if (other.name != null) return false; + } else if (!name.equals(other.name)) return false; + if (advertisedFeatures == null) { + if (other.advertisedFeatures != null) return false; + } else if (!advertisedFeatures.equals(other.advertisedFeatures)) + return false; + if (config == null) { + if (other.config != null) return false; + } else if (!config.equals(other.config)) return false; + if (currentFeatures == null) { + if (other.currentFeatures != null) return false; + } else if (!currentFeatures.equals(other.currentFeatures)) + return false; + if (!Arrays.equals(hardwareAddress, other.hardwareAddress)) + return false; + if (peerFeatures == null) { + if (other.peerFeatures != null) return false; + } else if (!peerFeatures.equals(other.peerFeatures)) return false; + if (portStateLinkDown != other.portStateLinkDown) return false; + if (stpState != other.stpState) return false; + if (supportedFeatures == null) { + if (other.supportedFeatures != null) return false; + } else if (!supportedFeatures.equals(other.supportedFeatures)) + return false; + return true; + } + + /** + * Convert a Collection of OFPhysicalPorts to a list of ImmutablePorts. + * All OFPhysicalPorts in the Collection must be non-null and valid. + * No other checks (name / number uniqueness) are performed + * @param ports + * @return a list of {@link ImmutablePort}s. This is list is owned by + * the caller. The returned list is not thread-safe + * @throws NullPointerException if any OFPhysicalPort or important fields + * of any OFPhysicalPort are null + * @throws IllegalArgumentException + */ + public static List<ImmutablePort> + immutablePortListOf(Collection<OFPhysicalPort> ports) { + if (ports == null) { + throw new NullPointerException("Port list must not be null"); + } + ArrayList<ImmutablePort> immutablePorts = + new ArrayList<ImmutablePort>(ports.size()); + for (OFPhysicalPort p: ports) + immutablePorts.add(fromOFPhysicalPort(p)); + return immutablePorts; + } + + /** + * Convert a Collection of ImmutablePort to a list of OFPhyscialPorts. + * All ImmutablePorts in the Collection must be non-null. + * No other checks (name / number uniqueness) are performed + * @param ports + * @return a list of {@link OFPhysicalPort}s. This is list is owned by + * the caller. The returned list is not thread-safe + * @throws NullPointerException if any {@link ImmutablePort} or the port + * list is null + * @throws IllegalArgumentException + */ + public static List<OFPhysicalPort> + ofPhysicalPortListOf(Collection<ImmutablePort> ports) { + if (ports == null) { + throw new NullPointerException("Port list must not be null"); + } + ArrayList<OFPhysicalPort> ofppList= + new ArrayList<OFPhysicalPort>(ports.size()); + for (ImmutablePort p: ports) { + if (p == null) + throw new NullPointerException("Port must not be null"); + ofppList.add(p.toOFPhysicalPort()); + } + return ofppList; + } +} diff --git a/src/main/java/net/floodlightcontroller/core/OFSwitchBase.java b/src/main/java/net/floodlightcontroller/core/OFSwitchBase.java index 98a5b4345a846c5bbc37354bb4e7ea11032f3c56..33b58015e38ade9f46cff261474c2a53792dbf9c 100644 --- a/src/main/java/net/floodlightcontroller/core/OFSwitchBase.java +++ b/src/main/java/net/floodlightcontroller/core/OFSwitchBase.java @@ -32,7 +32,6 @@ import java.util.concurrent.ConcurrentMap; import java.util.concurrent.Future; import java.util.concurrent.atomic.AtomicInteger; import java.util.concurrent.atomic.AtomicLong; -import java.util.concurrent.locks.Lock; import java.util.concurrent.locks.ReentrantReadWriteLock; import net.floodlightcontroller.core.IFloodlightProviderService.Role; @@ -64,15 +63,13 @@ import org.openflow.protocol.OFFlowMod; import org.openflow.protocol.OFMatch; import org.openflow.protocol.OFMessage; import org.openflow.protocol.OFPacketIn; -import org.openflow.protocol.OFPhysicalPort; -import org.openflow.protocol.OFPhysicalPort.OFPortConfig; -import org.openflow.protocol.OFPhysicalPort.OFPortState; +import org.openflow.protocol.OFPortStatus.OFPortReason; import org.openflow.protocol.OFPort; +import org.openflow.protocol.OFPortStatus; import org.openflow.protocol.OFStatisticsRequest; import org.openflow.protocol.OFType; import org.openflow.protocol.statistics.OFDescriptionStatistics; import org.openflow.protocol.statistics.OFStatistics; -import org.openflow.util.HexString; import org.openflow.util.U16; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -104,25 +101,16 @@ public abstract class OFSwitchBase implements IOFSwitch { */ private Channel channel; private final AtomicInteger transactionIdSource; - // 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. - protected 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. - private final ConcurrentHashMap<String, OFPhysicalPort> portsByName; private final Map<Integer,OFStatisticsFuture> statsFutureMap; private final Map<Integer, IOFMessageListener> iofMsgListenersMap; private final Map<Integer,OFFeaturesReplyFuture> featuresFutureMap; private volatile boolean connected; private volatile Role role; private final TimedCache<Long> timedCache; - private final ReentrantReadWriteLock listenerLock; private final ConcurrentMap<Short, AtomicLong> portBroadcastCacheHitMap; + private final PortManager portManager; + // Private members for throttling private boolean writeThrottleEnabled = false; protected boolean packetInThrottleEnabled = false; // used by test @@ -159,26 +147,463 @@ public abstract class OFSwitchBase implements IOFSwitch { this.attributes = new ConcurrentHashMap<Object, Object>(); this.connectedSince = new Date(); this.transactionIdSource = new AtomicInteger(); - this.portLock = new Object(); - this.portsByNumber = new ConcurrentHashMap<Short, OFPhysicalPort>(); - this.portsByName = new ConcurrentHashMap<String, OFPhysicalPort>(); this.connected = false; this.statsFutureMap = new ConcurrentHashMap<Integer,OFStatisticsFuture>(); this.featuresFutureMap = new ConcurrentHashMap<Integer,OFFeaturesReplyFuture>(); this.iofMsgListenersMap = new ConcurrentHashMap<Integer,IOFMessageListener>(); this.role = null; this.timedCache = new TimedCache<Long>(100, 5*1000 ); // 5 seconds interval - this.listenerLock = new ReentrantReadWriteLock(); this.portBroadcastCacheHitMap = new ConcurrentHashMap<Short, AtomicLong>(); this.description = new OFDescriptionStatistics(); this.lastMessageTime = System.currentTimeMillis(); + this.portManager = new PortManager(); + // Defaults properties for an ideal switch this.setAttribute(PROP_FASTWILDCARDS, OFMatch.OFPFW_ALL); this.setAttribute(PROP_SUPPORTS_OFPP_FLOOD, Boolean.valueOf(true)); this.setAttribute(PROP_SUPPORTS_OFPP_TABLE, Boolean.valueOf(true)); } + /** + * Manages the ports of this switch. + * + * Provides methods to query and update the stored ports. The class ensures + * that every port name and port number is unique. When updating ports + * the class checks if port number <-> port name mappings have change due + * to the update. If a new port P has number and port that are inconsistent + * 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. + * + * 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 + * changes. + * + * Implementation notes: + * - We keep several different representations of the ports to allow for + * fast lookups + * - Ports are stored in unchangeable lists. When a port is modified new + * data structures are allocated. + * - We use a read-write-lock for synchronization, so multiple readers are + * allowed. + */ + protected class PortManager { + private ReentrantReadWriteLock lock; + private List<ImmutablePort> portList; + private List<ImmutablePort> enabledPortList; + private List<Short> enabledPortNumbers; + private Map<Short,ImmutablePort> portsByNumber; + private Map<String,ImmutablePort> portsByName; + + public PortManager() { + this.lock = new ReentrantReadWriteLock(); + this.portList = Collections.emptyList(); + this.enabledPortList = Collections.emptyList(); + this.enabledPortNumbers = Collections.emptyList(); + this.portsByName = Collections.emptyMap(); + this.portsByNumber = Collections.emptyMap(); + } + + /** + * Set the internal data structure storing this switch's port + * to the ports specified by newPortsByNumber + * + * CALLER MUST HOLD WRITELOCK + * + * @param newPortsByNumber + * @throws IllegaalStateException if called without holding the + * writelock + */ + private void updatePortsWithNewPortsByNumber( + Map<Short,ImmutablePort> newPortsByNumber) { + if (!lock.writeLock().isHeldByCurrentThread()) { + throw new IllegalStateException("Method called without " + + "holding writeLock"); + } + Map<String,ImmutablePort> newPortsByName = + new HashMap<String, ImmutablePort>(); + List<ImmutablePort> newPortList = + new ArrayList<ImmutablePort>(); + List<ImmutablePort> newEnabledPortList = + new ArrayList<ImmutablePort>(); + List<Short> newEnabledPortNumbers = new ArrayList<Short>(); + + for(ImmutablePort p: newPortsByNumber.values()) { + newPortList.add(p); + newPortsByName.put(p.getName(), p); + if (p.isEnabled()) { + newEnabledPortList.add(p); + newEnabledPortNumbers.add(p.getPortNumber()); + } + } + portsByName = Collections.unmodifiableMap(newPortsByName); + portsByNumber = + Collections.unmodifiableMap(newPortsByNumber); + enabledPortList = + Collections.unmodifiableList(newEnabledPortList); + enabledPortNumbers = + Collections.unmodifiableList(newEnabledPortNumbers); + portList = Collections.unmodifiableList(newPortList); + } + + /** + * Handle a OFPortStatus delete message for the given port. + * Updates the internal port maps/lists of this switch and returns + * the PortChangeEvents caused by the delete. If the given port + * exists as it, it will be deleted. If the name<->number for the + * given port is inconsistent with the ports stored by this switch + * the method will delete all ports with the number or name of the + * given port. + * + * This method will increment error/warn counters and log + * + * @param delPort the port from the port status message that should + * be deleted. + * @return list of port changes applied to this switch + */ + private List<PortChangeEvent> + handlePortStatusDelete(ImmutablePort delPort) { + lock.writeLock().lock(); + List<PortChangeEvent> events = Collections.emptyList(); + try { + Map<Short,ImmutablePort> newPortByNumber = + new HashMap<Short, ImmutablePort>(portsByNumber); + ImmutablePort prevPort = + portsByNumber.get(delPort.getPortNumber()); + if (prevPort == null) { + // so such port. That's weird + } else if (prevPort.getName().equals(delPort.getName())) { + // port exists with consistent name-number mapping + newPortByNumber.remove(delPort.getPortNumber()); + events = Collections.singletonList( + new PortChangeEvent(delPort, PortChangeType.DELETE)); + } else { + // port with same number exists but its name differs. This + // is weird. The best we can do is to delete the existing + // port(s) that have delPort's name and number. + events = new ArrayList<PortChangeEvent>(2); + newPortByNumber.remove(delPort.getPortNumber()); + events.add(new PortChangeEvent(prevPort, + PortChangeType.DELETE)); + // is there another port that has delPort's name? + prevPort = portsByName.get(delPort.getName()); + if (prevPort != null) { + newPortByNumber.remove(prevPort.getPortNumber()); + events.add(new PortChangeEvent(prevPort, + PortChangeType.DELETE)); + } + } + updatePortsWithNewPortsByNumber(newPortByNumber); + return events; + } finally { + lock.writeLock().unlock(); + } + } + + /** + * Handle a OFPortStatus message, update the internal data structures + * that store ports and return the list of OFChangeEvents. + * + * This method will increment error/warn counters and log + * + * @param ps + * @return + */ + public List<PortChangeEvent> handlePortStatusMessage(OFPortStatus ps) { + if (ps == null) { + throw new NullPointerException("OFPortStatus message must " + + "not be null"); + } + lock.writeLock().lock(); + try { + List<PortChangeEvent> events; + ImmutablePort port = + ImmutablePort.fromOFPhysicalPort(ps.getDesc()); + OFPortReason reason = OFPortReason.fromReasonCode(ps.getReason()); + if (reason == null) { + throw new IllegalArgumentException("Unknown PortStatus " + + "reason code " + ps.getReason()); + } + + if (log.isDebugEnabled()) { + log.debug("Handling OFPortStatus: {} for {}", + reason, port.toBriefString()); + } + + if (reason == OFPortReason.OFPPR_DELETE) + return handlePortStatusDelete(port); + + Map<Short,ImmutablePort> newPortByNumber = + new HashMap<Short, ImmutablePort>(portsByNumber); + events = getSinglePortChanges(port); + for (PortChangeEvent e: events) { + switch(e.type) { + case DELETE: + newPortByNumber.remove(e.port.getPortNumber()); + break; + case ADD: + if (reason != OFPortReason.OFPPR_ADD) { + // weird case + } + // fall through + case DOWN: + case OTHER_UPDATE: + case UP: + // update or add the port in the map + newPortByNumber.put(e.port.getPortNumber(), e.port); + break; + } + } + updatePortsWithNewPortsByNumber(newPortByNumber); + return events; + } finally { + lock.writeLock().unlock(); + } + + } + + /** + * Given a new or modified port newPort, returns the list of + * PortChangeEvents to "transform" the current ports stored by + * this switch to include / represent the new port. The ports stored + * by this switch are <b>NOT</b> updated. + * + * This method acquires the readlock and is thread-safe by itself. + * Most callers will need to acquire the write lock before calling + * this method though (if the caller wants to update the ports stored + * by this switch) + * + * @param newPort the new or modified port. + * @return the list of changes + */ + public List<PortChangeEvent> + getSinglePortChanges(ImmutablePort newPort) { + lock.readLock().lock(); + try { + List<PortChangeEvent> events = new ArrayList<PortChangeEvent>(); + // Check if we have a port by the same number in our + // old map. + ImmutablePort prevPort = + portsByNumber.get(newPort.getPortNumber()); + if (newPort.equals(prevPort)) { + // nothing has changed + return events; + } + + if (prevPort != null && + prevPort.getName().equals(newPort.getName())) { + // A simple modify of a exiting port + // A previous port with this number exists and it's name + // also matches the new port. Find the differences + if (prevPort.isEnabled() && !newPort.isEnabled()) { + events.add(new PortChangeEvent(newPort, + PortChangeType.DOWN)); + } else if (!prevPort.isEnabled() && newPort.isEnabled()) { + events.add(new PortChangeEvent(newPort, + PortChangeType.UP)); + } else { + events.add(new PortChangeEvent(newPort, + PortChangeType.OTHER_UPDATE)); + } + return events; + } + + if (prevPort != null) { + // There exists a previous port with the same port + // number but the port name is different (otherwise we would + // never have gotten here) + // Remove the port. Name-number mapping(s) have changed + events.add(new PortChangeEvent(prevPort, + PortChangeType.DELETE)); + } + + // 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()); + if (prevPort != null) { + // There exists a previous port with the same port + // name but the port number is different (otherwise we + // never have gotten here). + // Remove the port. Name-number mapping(s) have changed + events.add(new PortChangeEvent(prevPort, + PortChangeType.DELETE)); + } + + // We always need to add the new port. Either no previous port + // existed or we just deleted previous ports with inconsistent + // name-number mappings + events.add(new PortChangeEvent(newPort, PortChangeType.ADD)); + return events; + } finally { + lock.readLock().unlock(); + } + } + + /** + * Compare the current ports of this switch to the newPorts list and + * return the changes that would be applied to transfort the current + * ports to the new ports. No internal data structures are updated + * see {@link #compareAndUpdatePorts(List, boolean)} + * + * @param newPorts the list of new ports + * @return The list of differences between the current ports and + * newPortList + */ + public List<PortChangeEvent> + comparePorts(Collection<ImmutablePort> newPorts) { + return compareAndUpdatePorts(newPorts, false); + } + + /** + * Compare the current ports of this switch to the newPorts list and + * return the changes that would be applied to transform the current + * ports to the new ports. No internal data structures are updated + * see {@link #compareAndUpdatePorts(List, boolean)} + * + * @param newPorts the list of new ports + * @return The list of differences between the current ports and + * newPortList + */ + public List<PortChangeEvent> + updatePorts(Collection<ImmutablePort> newPorts) { + return compareAndUpdatePorts(newPorts, false); + } + + /** + * Compare the current ports stored in this switch instance with the + * new port list given and return the differences in the form of + * PortChangeEvents. If the doUpdate flag is true, newPortList will + * replace the current list of this switch (and update the port maps) + * + * Implementation note: + * Since this method can optionally modify the current ports and + * since it's not possible to upgrade a read-lock to a write-lock + * we need to hold the write-lock for the entire operation. If this + * becomes a problem and if compares() are common we can consider + * splitting in two methods but this requires lots of code duplication + * + * @param newPorts the list of new ports. + * @param doUpdate If true the newPortList will replace the current + * port list for this switch. If false this switch will not be changed. + * @return The list of differences between the current ports and + * newPorts + * @throws NullPointerException if newPortsList is null + * @throws IllegalArgumentException if either port names or port numbers + * are duplicated in the newPortsList. + */ + private List<PortChangeEvent> compareAndUpdatePorts( + Collection<ImmutablePort> newPorts, + boolean doUpdate) { + if (newPorts == null) { + throw new NullPointerException("newPortsList must not be null"); + } + lock.writeLock().lock(); + try { + List<PortChangeEvent> events = new ArrayList<PortChangeEvent>(); + + Map<Short,ImmutablePort> newPortsByNumber = + new HashMap<Short, ImmutablePort>(); + Map<String,ImmutablePort> newPortsByName = + new HashMap<String, ImmutablePort>(); + List<ImmutablePort> newEnabledPortList = + new ArrayList<ImmutablePort>(); + List<Short> newEnabledPortNumbers = + new ArrayList<Short>(); + List<ImmutablePort> newPortsList = + new ArrayList<ImmutablePort>(newPorts); + + for (ImmutablePort p: newPortsList) { + // Add the port to the new maps and lists and check + // that every port is unique + ImmutablePort duplicatePort; + duplicatePort = newPortsByNumber.put(p.getPortNumber(), p); + if (duplicatePort != null) { + String msg = String.format("Cannot have two ports " + + "with the same number: %s <-> %s", + p.toBriefString(), + duplicatePort.toBriefString()); + throw new IllegalArgumentException(msg); + } + duplicatePort = newPortsByName.put(p.getName(), p); + if (duplicatePort != null) { + String msg = String.format("Cannot have two ports " + + "with the same name: %s <-> %s", + p.toBriefString(), + duplicatePort.toBriefString()); + throw new IllegalArgumentException(msg); + } + if (p.isEnabled()) { + newEnabledPortList.add(p); + newEnabledPortNumbers.add(p.getPortNumber()); + } + + // get changes + events.addAll(getSinglePortChanges(p)); + } + + if (doUpdate) { + portsByName = Collections.unmodifiableMap(newPortsByName); + portsByNumber = + Collections.unmodifiableMap(newPortsByNumber); + enabledPortList = + Collections.unmodifiableList(newEnabledPortList); + enabledPortNumbers = + Collections.unmodifiableList(newEnabledPortNumbers); + portList = Collections.unmodifiableList(newPortsList); + } + return events; + } finally { + lock.writeLock().unlock(); + } + } + + public ImmutablePort getPort(String name) { + lock.readLock().lock(); + try { + return portsByName.get(name); + } finally { + lock.readLock().unlock(); + } + } + + public ImmutablePort getPort(Short portNumber) { + lock.readLock().lock(); + try { + return portsByNumber.get(portNumber); + } finally { + lock.readLock().unlock(); + } + } + + public List<ImmutablePort> getPorts() { + lock.readLock().lock(); + try { + return portList; + } finally { + lock.readLock().unlock(); + } + } + + public List<ImmutablePort> getEnabledPorts() { + lock.readLock().lock(); + try { + return enabledPortList; + } finally { + lock.readLock().unlock(); + } + } + + public List<Short> getEnabledPortNumbers() { + lock.readLock().lock(); + try { + return enabledPortNumbers; + } finally { + lock.readLock().unlock(); + } + } + } + @Override public boolean attributeEquals(String name, Object other) { @@ -342,115 +767,78 @@ public abstract class OFSwitchBase implements IOFSwitch { @Override @JsonIgnore public void setFeaturesReply(OFFeaturesReply featuresReply) { - synchronized(portLock) { - if (stringId == null) { - /* ports are updated via port status message, so we - * only fill in ports on initial connection. - */ - for (OFPhysicalPort port : featuresReply.getPorts()) { - setPort(port); - } - } - this.datapathId = featuresReply.getDatapathId(); - this.capabilities = featuresReply.getCapabilities(); - this.buffers = featuresReply.getBuffers(); - this.actions = featuresReply.getActions(); - this.tables = featuresReply.getTables(); - this.stringId = HexString.toHexString(this.datapathId); + if (stringId == null) { + /* ports are updated via port status message, so we + * only fill in ports on initial connection. + */ + List<ImmutablePort> immutablePorts = ImmutablePort + .immutablePortListOf(featuresReply.getPorts()); + portManager.updatePorts(immutablePorts); } - } + this.datapathId = featuresReply.getDatapathId(); + this.capabilities = featuresReply.getCapabilities(); + this.buffers = featuresReply.getBuffers(); + this.actions = featuresReply.getActions(); + this.tables = featuresReply.getTables(); +} @Override @JsonIgnore - public Collection<OFPhysicalPort> getEnabledPorts() { - List<OFPhysicalPort> result = - new ArrayList<OFPhysicalPort>(portsByNumber.size()); - for (OFPhysicalPort port : portsByNumber.values()) { - if (portEnabled(port)) { - result.add(port); - } - } - return result; + public Collection<ImmutablePort> getEnabledPorts() { + return portManager.getEnabledPorts(); } @Override @JsonIgnore public Collection<Short> getEnabledPortNumbers() { - List<Short> result = - new ArrayList<Short>(portsByNumber.size()); - for (OFPhysicalPort port : portsByNumber.values()) { - if (portEnabled(port)) { - result.add(port.getPortNumber()); - } - } - return result; + return portManager.getEnabledPortNumbers(); } @Override - public OFPhysicalPort getPort(short portNumber) { - return portsByNumber.get(portNumber); + public ImmutablePort getPort(short portNumber) { + return portManager.getPort(portNumber); } @Override - public OFPhysicalPort getPort(String portName) { - return portsByName.get(portName); + public ImmutablePort getPort(String portName) { + return portManager.getPort(portName); } @Override @JsonIgnore - public void setPort(OFPhysicalPort port) { - synchronized(portLock) { - portsByNumber.put(port.getPortNumber(), port); - portsByName.put(port.getName(), port); - } + public List<PortChangeEvent> processOFPortStatus(OFPortStatus ps) { + return portManager.handlePortStatusMessage(ps); } @Override @JsonProperty("ports") - public Collection<OFPhysicalPort> getPorts() { - return Collections.unmodifiableCollection(portsByNumber.values()); + public Collection<ImmutablePort> getPorts() { + return portManager.getPorts(); } @Override - public void deletePort(short portNumber) { - synchronized(portLock) { - portsByName.remove(portsByNumber.get(portNumber).getName()); - portsByNumber.remove(portNumber); - } + public List<PortChangeEvent> comparePorts(Collection<ImmutablePort> ports) { + return portManager.comparePorts(ports); } @Override - public void deletePort(String portName) { - synchronized(portLock) { - portsByNumber.remove(portsByName.get(portName).getPortNumber()); - portsByName.remove(portName); - } + @JsonIgnore + public List<PortChangeEvent> setPorts(Collection<ImmutablePort> ports) { + return portManager.updatePorts(ports); } @Override public boolean portEnabled(short portNumber) { - if (portsByNumber.get(portNumber) == null) return false; - return portEnabled(portsByNumber.get(portNumber)); + ImmutablePort p = portManager.getPort(portNumber); + if (p == null) return false; + return p.isEnabled(); } @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; - if ((port.getConfig() & OFPortConfig.OFPPC_PORT_DOWN.getValue()) > 0) - return false; - if ((port.getState() & OFPortState.OFPPS_LINK_DOWN.getValue()) > 0) - return false; - // Port STP state doesn't work with multiple VLANs, so ignore it for now - // if ((port.getState() & OFPortState.OFPPS_STP_MASK.getValue()) == OFPortState.OFPPS_STP_BLOCK.getValue()) - // return false; - return true; + ImmutablePort p = portManager.getPort(portName); + if (p == null) return false; + return p.isEnabled(); } @Override @@ -690,30 +1078,6 @@ public abstract class OFSwitchBase implements IOFSwitch { } } - /** - * Return a read lock that must be held while calling the listeners for - * messages from the switch. Holding the read lock prevents the active - * switch list from being modified out from under the listeners. - * @return - */ - @Override - @JsonIgnore - public Lock getListenerReadLock() { - return listenerLock.readLock(); - } - - /** - * Return a write lock that must be held when the controllers modifies the - * list of active switches. This is to ensure that the active switch list - * doesn't change out from under the listeners as they are handling a - * message from the switch. - * @return - */ - @Override - @JsonIgnore - public Lock getListenerWriteLock() { - return listenerLock.writeLock(); - } /** * Get the IP Address for the switch diff --git a/src/main/java/net/floodlightcontroller/core/SwitchSyncRepresentation.java b/src/main/java/net/floodlightcontroller/core/SwitchSyncRepresentation.java index 00d8125e2feff51972a5f613d113ec339cdbf639..6284ba3c38785c2e5dc21421d012147f74c165ed 100644 --- a/src/main/java/net/floodlightcontroller/core/SwitchSyncRepresentation.java +++ b/src/main/java/net/floodlightcontroller/core/SwitchSyncRepresentation.java @@ -4,10 +4,12 @@ import java.util.ArrayList; import java.util.Collection; import java.util.List; +import net.floodlightcontroller.util.EnumBitmaps; import net.floodlightcontroller.util.MACAddress; import org.openflow.protocol.OFFeaturesReply; import org.openflow.protocol.OFPhysicalPort; +import org.openflow.protocol.OFPhysicalPort.OFPortState; import org.openflow.protocol.statistics.OFDescriptionStatistics; import org.openflow.util.HexString; @@ -43,7 +45,7 @@ public class SwitchSyncRepresentation { @JsonProperty public int peerFeatures; - public static SyncedPort fromOFPhysicalPort(OFPhysicalPort p) { + public static SyncedPort fromImmutablePort(ImmutablePort p) { SyncedPort rv = new SyncedPort(); rv.portNumber = p.getPortNumber(); if (p.getHardwareAddress() == null) { @@ -53,12 +55,16 @@ public class SwitchSyncRepresentation { MACAddress.valueOf(p.getHardwareAddress()).toLong(); } rv.name = p.getName(); - rv.config = p.getConfig(); - rv.state = p.getState(); - rv.currentFeatures = p.getCurrentFeatures(); - rv.advertisedFeatures = p.getAdvertisedFeatures(); - rv.supportedFeatures = p.getSupportedFeatures(); - rv.peerFeatures = p.getPeerFeatures(); + rv.config = EnumBitmaps.toBitmap(p.getConfig()); + rv.state = p.getStpState().getValue(); + if (p.isLinkDown()) + rv.state |= OFPortState.OFPPS_LINK_DOWN.getValue(); + rv.currentFeatures = EnumBitmaps.toBitmap(p.getCurrentFeatures()); + rv.advertisedFeatures = + EnumBitmaps.toBitmap(p.getAdvertisedFeatures()); + rv.supportedFeatures = + EnumBitmaps.toBitmap(p.getSupportedFeatures()); + rv.peerFeatures = EnumBitmaps.toBitmap(p.getPeerFeatures()); return rv; } @@ -156,7 +162,8 @@ public class SwitchSyncRepresentation { this.tables = fr.getTables(); this.capabilities = fr.getCapabilities(); this.actions = fr.getActions(); - this.ports = toSyncedPortList(fr.getPorts()); + this.ports = toSyncedPortList( + ImmutablePort.immutablePortListOf(fr.getPorts())); this.manufacturerDescription = d.getManufacturerDescription(); this.hardwareDescription = d.getHardwareDescription(); @@ -165,10 +172,10 @@ public class SwitchSyncRepresentation { this.datapathDescription = d.getDatapathDescription(); } - private static List<SyncedPort> toSyncedPortList(Collection<OFPhysicalPort> ports) { + private static List<SyncedPort> toSyncedPortList(Collection<ImmutablePort> ports) { List<SyncedPort> rv = new ArrayList<SyncedPort>(ports.size()); - for (OFPhysicalPort p: ports) { - rv.add(SyncedPort.fromOFPhysicalPort(p)); + for (ImmutablePort p: ports) { + rv.add(SyncedPort.fromImmutablePort(p)); } return rv; } diff --git a/src/main/java/net/floodlightcontroller/core/internal/Controller.java b/src/main/java/net/floodlightcontroller/core/internal/Controller.java index 4365fd5a73b998de1bfd93d7ef2282fd31b6c472..81a057e37cd3e8198b7c97d48baf0335f7fd7880 100644 --- a/src/main/java/net/floodlightcontroller/core/internal/Controller.java +++ b/src/main/java/net/floodlightcontroller/core/internal/Controller.java @@ -53,10 +53,12 @@ import net.floodlightcontroller.core.IInfoProvider; import net.floodlightcontroller.core.IListener.Command; import net.floodlightcontroller.core.IOFMessageListener; import net.floodlightcontroller.core.IOFSwitch; +import net.floodlightcontroller.core.IOFSwitch.PortChangeEvent; import net.floodlightcontroller.core.IOFSwitch.PortChangeType; import net.floodlightcontroller.core.IOFSwitchDriver; import net.floodlightcontroller.core.IOFSwitchListener; import net.floodlightcontroller.core.IReadyForReconcileListener; +import net.floodlightcontroller.core.ImmutablePort; import net.floodlightcontroller.core.OFSwitchBase; import net.floodlightcontroller.core.RoleInfo; import net.floodlightcontroller.core.SwitchSyncRepresentation; @@ -88,7 +90,6 @@ import org.jboss.netty.channel.group.DefaultChannelGroup; import org.jboss.netty.channel.socket.nio.NioServerSocketChannelFactory; import org.openflow.protocol.OFMessage; import org.openflow.protocol.OFPacketIn; -import org.openflow.protocol.OFPhysicalPort; import org.openflow.protocol.OFType; import org.openflow.protocol.factory.BasicFactory; import org.openflow.protocol.statistics.OFDescriptionStatistics; @@ -1006,7 +1007,7 @@ public class Controller implements IFloodlightProviderService, * @param sw */ public synchronized void switchPortsChanged(IOFSwitch sw, - OFPhysicalPort port, + ImmutablePort port, PortChangeType type) { if (role != Role.MASTER) { counters.invalidPortsChanged.increment(); @@ -1182,24 +1183,20 @@ public class Controller implements IFloodlightProviderService, /** * Check if the two switches differ in their ports or in other * fields and if they differ enqueue a switch update - * @param sw1 - * @param sw2 + * @param oldSw + * @param newSw */ - private synchronized void sendNotificationsIfSwitchDiffers(IOFSwitch sw1, - IOFSwitch sw2) { - // FIXME - Set<OFPhysicalPort> sw1Ports = - new HashSet<OFPhysicalPort>(sw1.getPorts()); - Set<OFPhysicalPort> sw2Ports = - new HashSet<OFPhysicalPort>(sw2.getPorts()); - if (! sw1Ports.equals(sw2Ports)) { - /* FIXME FIXME FIXME - addUpdateToQueue( - new SwitchUpdate(sw2.getId(), + private synchronized void + sendNotificationsIfSwitchDiffers(IOFSwitch oldSw, + IOFSwitch newSw) { + List<PortChangeEvent> portDiffs = + oldSw.comparePorts(newSw.getPorts()); + for (PortChangeEvent ev: portDiffs) { + SwitchUpdate update = + new SwitchUpdate(newSw.getId(), SwitchUpdateType.PORTCHANGED, - null, null)); - */ - + ev.port, ev.type); + addUpdateToQueue(update); } } @@ -1384,7 +1381,7 @@ public class Controller implements IFloodlightProviderService, private class SwitchUpdate implements IUpdate { private final long swId; private final SwitchUpdateType switchUpdateType; - private final OFPhysicalPort port; + private final ImmutablePort port; private final PortChangeType changeType; @@ -1393,7 +1390,7 @@ public class Controller implements IFloodlightProviderService, } public SwitchUpdate(long swId, SwitchUpdateType switchUpdateType, - OFPhysicalPort port, + ImmutablePort port, PortChangeType changeType) { if (switchUpdateType == SwitchUpdateType.PORTCHANGED) { if (port == null) { @@ -1594,8 +1591,24 @@ public class Controller implements IFloodlightProviderService, * @param sw */ void notifyPortChanged(IOFSwitch sw, - OFPhysicalPort port, + ImmutablePort port, PortChangeType changeType) { + if (sw == null) { + String msg = String.format("Switch must not be null. " + + "port=%s, changeType=%s", port, changeType); + throw new NullPointerException(msg); + } + if (port == null) { + String msg = String.format("Port must not be null. " + + "switch=%s, changeType=%s", sw, changeType); + throw new NullPointerException(msg); + } + if (changeType == null) { + String msg = String.format("ChangeType must not be null. " + + "switch=%s, port=%s", sw, port); + throw new NullPointerException(msg); + } + this.switchManager.switchPortsChanged(sw, port, changeType); } diff --git a/src/main/java/net/floodlightcontroller/core/internal/OFChannelHandler.java b/src/main/java/net/floodlightcontroller/core/internal/OFChannelHandler.java index 169e539c22b11aa68fdcc3b6dd86cc93404e7ccb..d6aec0bb26d2001c2bfd87f0d695c01d7aae15b0 100644 --- a/src/main/java/net/floodlightcontroller/core/internal/OFChannelHandler.java +++ b/src/main/java/net/floodlightcontroller/core/internal/OFChannelHandler.java @@ -11,7 +11,7 @@ import java.util.concurrent.RejectedExecutionException; import net.floodlightcontroller.core.FloodlightContext; import net.floodlightcontroller.core.IOFSwitch; import net.floodlightcontroller.core.IFloodlightProviderService.Role; -import net.floodlightcontroller.core.IOFSwitch.PortChangeType; +import net.floodlightcontroller.core.IOFSwitch.PortChangeEvent; import net.floodlightcontroller.core.annotations.LogMessageDoc; import net.floodlightcontroller.core.annotations.LogMessageDocs; import net.floodlightcontroller.core.internal.Controller.Counters; @@ -42,7 +42,6 @@ import org.openflow.protocol.OFGetConfigRequest; import org.openflow.protocol.OFHello; import org.openflow.protocol.OFMessage; import org.openflow.protocol.OFPacketIn; -import org.openflow.protocol.OFPhysicalPort; import org.openflow.protocol.OFPortStatus; import org.openflow.protocol.OFQueueGetConfigReply; import org.openflow.protocol.OFSetConfig; @@ -58,7 +57,6 @@ import org.openflow.protocol.OFError.OFFlowModFailedCode; import org.openflow.protocol.OFError.OFHelloFailedCode; import org.openflow.protocol.OFError.OFPortModFailedCode; import org.openflow.protocol.OFError.OFQueueOpFailedCode; -import org.openflow.protocol.OFPortStatus.OFPortReason; import org.openflow.protocol.factory.BasicFactory; import org.openflow.protocol.factory.MessageParseException; import org.openflow.protocol.statistics.OFDescriptionStatistics; @@ -980,43 +978,16 @@ class OFChannelHandler */ protected void handlePortStatusMessage(OFChannelHandler h, OFPortStatus m) { - short portNumber = m.getDesc().getPortNumber(); - OFPhysicalPort port = m.getDesc(); - OFPortReason reason = OFPortReason.fromReasonCode(m.getReason()); - PortChangeType changeType = null; - - switch(reason) { - case OFPPR_MODIFY: - boolean oldEnabled = - h.sw.portEnabled(port.getPortNumber()); - - h.sw.setPort(port); - boolean newEnabled = - h.sw.portEnabled(port.getPortNumber()); - - if (!oldEnabled && newEnabled) - changeType = PortChangeType.UP; - else if (oldEnabled && !newEnabled) - changeType = PortChangeType.DOWN; - else changeType = PortChangeType.OTHER_UPDATE; - - log.debug("Port #{} modified for {}", portNumber, h.sw); - break; - case OFPPR_ADD: - h.sw.setPort(port); - if (h.sw.portEnabled(port.getPortNumber())) - changeType = PortChangeType.UP; - else - changeType = PortChangeType.ADD; - log.debug("Port #{} added for {}", portNumber, h.sw); - break; - case OFPPR_DELETE: - h.sw.deletePort(portNumber); - changeType = PortChangeType.DELETE; - log.debug("Port #{} deleted for {}", portNumber, h.sw); - break; + if (h.sw == null) { + String msg = getSwitchStateMessage(h, m, + "State machine error: switch is null. Should never " + + "happen"); + throw new SwitchStateException(msg); + } + List<PortChangeEvent> changes = h.sw.processOFPortStatus(m); + for (PortChangeEvent ev: changes) { + h.controller.notifyPortChanged(h.sw, ev.port, ev.type); } - h.controller.notifyPortChanged(h.sw, port, changeType); } /** diff --git a/src/main/java/net/floodlightcontroller/linkdiscovery/internal/LinkDiscoveryManager.java b/src/main/java/net/floodlightcontroller/linkdiscovery/internal/LinkDiscoveryManager.java index 3f6050d43df62d655e66d60ad9636d9255b08f0b..302859e970a2b7aa0436468805f7f2b725a73ba5 100644 --- a/src/main/java/net/floodlightcontroller/linkdiscovery/internal/LinkDiscoveryManager.java +++ b/src/main/java/net/floodlightcontroller/linkdiscovery/internal/LinkDiscoveryManager.java @@ -48,6 +48,7 @@ import net.floodlightcontroller.core.IInfoProvider; import net.floodlightcontroller.core.IOFMessageListener; import net.floodlightcontroller.core.IOFSwitch; import net.floodlightcontroller.core.IOFSwitchListener; +import net.floodlightcontroller.core.ImmutablePort; import net.floodlightcontroller.core.annotations.LogMessageCategory; import net.floodlightcontroller.core.annotations.LogMessageDoc; import net.floodlightcontroller.core.annotations.LogMessageDocs; @@ -258,7 +259,7 @@ public class LinkDiscoveryManager implements IOFMessageListener, boolean isStandard, boolean isReverse) { IOFSwitch iofSwitch = floodlightProvider.getSwitch(sw); - OFPhysicalPort ofpPort = iofSwitch.getPort(port); + ImmutablePort ofpPort = iofSwitch.getPort(port); if (log.isTraceEnabled()) { log.trace("Sending LLDP packet out of swich: {}, port: {}", @@ -681,10 +682,11 @@ public class LinkDiscoveryManager implements IOFMessageListener, return Command.STOP; } - OFPhysicalPort physicalPort = remoteSwitch.getPort(remotePort); + OFPhysicalPort physicalPort = + remoteSwitch.getPort(remotePort).toOFPhysicalPort(); int srcPortState = (physicalPort != null) ? physicalPort.getState() : 0; - physicalPort = iofSwitch.getPort(inPort); + physicalPort = iofSwitch.getPort(inPort).toOFPhysicalPort(); int dstPortState = (physicalPort != null) ? physicalPort.getState() : 0; @@ -962,7 +964,7 @@ public class LinkDiscoveryManager implements IOFMessageListener, IOFSwitch iofSwitch = floodlightProvider.getSwitch(sw); if (iofSwitch == null) return; - OFPhysicalPort ofp = iofSwitch.getPort(port); + OFPhysicalPort ofp = iofSwitch.getPort(port).toOFPhysicalPort(); if (ofp == null) return; int srcPortState = ofp.getState(); @@ -1007,7 +1009,7 @@ public class LinkDiscoveryManager implements IOFMessageListener, if (port == OFPort.OFPP_LOCAL.getValue()) return false; - OFPhysicalPort ofpPort = iofSwitch.getPort(port); + ImmutablePort ofpPort = iofSwitch.getPort(port); if (ofpPort == null) { if (log.isTraceEnabled()) { log.trace("Null physical port. sw={}, port={}", @@ -1043,7 +1045,7 @@ public class LinkDiscoveryManager implements IOFMessageListener, if (port == OFPort.OFPP_LOCAL.getValue()) return false; - OFPhysicalPort ofpPort = iofSwitch.getPort(port); + ImmutablePort ofpPort = iofSwitch.getPort(port); if (ofpPort == null) { if (log.isTraceEnabled()) { log.trace("Null physical port. sw={}, port={}", @@ -1099,7 +1101,7 @@ public class LinkDiscoveryManager implements IOFMessageListener, return; IOFSwitch iofSwitch = floodlightProvider.getSwitch(sw); - OFPhysicalPort ofpPort = iofSwitch.getPort(port); + OFPhysicalPort ofpPort = iofSwitch.getPort(port).toOFPhysicalPort(); if (log.isTraceEnabled()) { log.trace("Sending LLDP packet out of swich: {}, port: {}", @@ -1143,7 +1145,7 @@ public class LinkDiscoveryManager implements IOFMessageListener, IOFSwitch iofSwitch = floodlightProvider.getSwitch(sw); if (iofSwitch == null) continue; if (iofSwitch.getEnabledPorts() != null) { - for (OFPhysicalPort ofp : iofSwitch.getEnabledPorts()) { + for (ImmutablePort ofp : iofSwitch.getEnabledPorts()) { if (isLinkDiscoverySuppressed(sw, ofp.getPortNumber())) continue; if (autoPortFastFeature @@ -1598,7 +1600,7 @@ public class LinkDiscoveryManager implements IOFMessageListener, */ @Override public void switchPortChanged(long switchId, - OFPhysicalPort port, + ImmutablePort port, IOFSwitch.PortChangeType type) { switch (type) { diff --git a/src/main/java/net/floodlightcontroller/staticflowentry/StaticFlowEntryPusher.java b/src/main/java/net/floodlightcontroller/staticflowentry/StaticFlowEntryPusher.java index 464d07928d4dde88b498d49bfc72c9d089f77f22..c192dde8739f588a9c0d8c8f70a26bccb1490ee0 100644 --- a/src/main/java/net/floodlightcontroller/staticflowentry/StaticFlowEntryPusher.java +++ b/src/main/java/net/floodlightcontroller/staticflowentry/StaticFlowEntryPusher.java @@ -36,6 +36,7 @@ import net.floodlightcontroller.core.IHAListener; import net.floodlightcontroller.core.IOFMessageListener; import net.floodlightcontroller.core.IOFSwitch; import net.floodlightcontroller.core.IOFSwitchListener; +import net.floodlightcontroller.core.ImmutablePort; import net.floodlightcontroller.core.annotations.LogMessageCategory; import net.floodlightcontroller.core.annotations.LogMessageDoc; import net.floodlightcontroller.core.module.FloodlightModuleContext; @@ -55,7 +56,6 @@ import org.openflow.protocol.OFFlowMod; import org.openflow.protocol.OFFlowRemoved; import org.openflow.protocol.OFMatch; import org.openflow.protocol.OFMessage; -import org.openflow.protocol.OFPhysicalPort; import org.openflow.protocol.OFType; import org.openflow.util.HexString; import org.openflow.util.U16; @@ -360,7 +360,7 @@ public class StaticFlowEntryPusher @Override public void switchPortChanged(long switchId, - OFPhysicalPort port, + ImmutablePort port, IOFSwitch.PortChangeType type) { // no-op } diff --git a/src/main/java/org/openflow/protocol/OFPhysicalPort.java b/src/main/java/org/openflow/protocol/OFPhysicalPort.java index 86bae5bef1321838fd58bda7097ba4c7feab5414..88de8f6570cb3d62370d1d2726fb6350fae36547 100644 --- a/src/main/java/org/openflow/protocol/OFPhysicalPort.java +++ b/src/main/java/org/openflow/protocol/OFPhysicalPort.java @@ -104,37 +104,37 @@ public class OFPhysicalPort { } public enum OFPortState { - OFPPS_LINK_DOWN (1 << 0) { + OFPPS_LINK_DOWN (1 << 0, false) { @Override public String toString() { return "link-down (0x1)"; } }, - OFPPS_STP_LISTEN (0 << 8) { + OFPPS_STP_LISTEN (0 << 8, true) { @Override public String toString() { return "listen (0x0)"; } }, - OFPPS_STP_LEARN (1 << 8) { + OFPPS_STP_LEARN (1 << 8, true) { @Override public String toString() { return "learn-no-relay (0x100)"; } }, - OFPPS_STP_FORWARD (2 << 8) { + OFPPS_STP_FORWARD (2 << 8, true) { @Override public String toString() { return "forward (0x200)"; } }, - OFPPS_STP_BLOCK (3 << 8) { + OFPPS_STP_BLOCK (3 << 8, true) { @Override public String toString() { return "block-broadcast (0x300)"; } }, - OFPPS_STP_MASK (3 << 8) { + OFPPS_STP_MASK (3 << 8, false) { // used for STP but not an STP state @Override public String toString() { return "block-broadcast (0x300)"; @@ -142,9 +142,45 @@ public class OFPhysicalPort { }; protected int value; + protected boolean isStpState; - private OFPortState(int value) { + private OFPortState(int value, boolean isStpState) { this.value = value; + this.isStpState = isStpState; + } + + /** + * Returns true if this constants represents one of the four STP + * states + * @return + */ + public boolean isStpState() { + return isStpState; + } + + /** + * return the STP state represented by the given integer. + * the returned value will have isStpState() == true + * @param state + * @return + */ + public static OFPortState getStpState(int state) { + // this ain't pretty + state = state & OFPortState.OFPPS_STP_MASK.getValue(); + for (OFPortState s: OFPortState.values()) { + if (!s.isStpState()) + continue; + if (state == s.getValue()) + return s; + } + return null; // will never happen + } + + public static boolean isPortDown(int state) { + if ((state & OFPPS_LINK_DOWN.getValue()) != 0) + return true; + else + return false; } /** @@ -155,74 +191,103 @@ public class OFPhysicalPort { } } + /** + * Represents the speed of a port + */ + public enum PortSpeed { + /** no speed set */ + SPEED_NONE(0), + SPEED_10MB(10), + SPEED_100MB(100), + SPEED_1GB(1000), + SPEED_10GB(10000); + + private long speedInBps; + private PortSpeed(int speedInMbps) { + this.speedInBps = speedInMbps * 1000*1000; + } + + public long getSpeedBps() { + return this.speedInBps; + } + + public static PortSpeed max(PortSpeed s1, PortSpeed s2) { + return (s1.getSpeedBps() > s2.getSpeedBps()) ? s1 : s2; + } + + public static PortSpeed min(PortSpeed s1, PortSpeed s2) { + return (s1.getSpeedBps() < s2.getSpeedBps()) ? s1 : s2; + } + } + public enum OFPortFeatures implements EnumBitmaps.BitmapableEnum { - OFPPF_10MB_HD (1 << 0) { + OFPPF_10MB_HD (1 << 0, PortSpeed.SPEED_10MB) { @Override public String toString() { return "10mb-hd (0x1)"; } }, - OFPPF_10MB_FD (1 << 1) { + OFPPF_10MB_FD (1 << 1, PortSpeed.SPEED_10MB) { @Override public String toString() { return "10mb-fd (0x2)"; } }, - OFPPF_100MB_HD (1 << 2) { + OFPPF_100MB_HD (1 << 2, PortSpeed.SPEED_100MB) { @Override public String toString() { return "100mb-hd (0x4)"; } }, - OFPPF_100MB_FD (1 << 3) { + OFPPF_100MB_FD (1 << 3, PortSpeed.SPEED_100MB) { @Override public String toString() { return "100mb-fd (0x8)"; } }, - OFPPF_1GB_HD (1 << 4) { + OFPPF_1GB_HD (1 << 4, PortSpeed.SPEED_1GB) { @Override public String toString() { return "1gb-hd (0x10)"; } }, - OFPPF_1GB_FD (1 << 5) { + OFPPF_1GB_FD (1 << 5, PortSpeed.SPEED_1GB) { @Override public String toString() { return "1gb-fd (0x20)"; } }, - OFPPF_10GB_FD (1 << 6) { + OFPPF_10GB_FD (1 << 6, PortSpeed.SPEED_10GB) { @Override public String toString() { return "10gb-fd (0x40)"; } }, - OFPPF_COPPER (1 << 7) { + OFPPF_COPPER (1 << 7, PortSpeed.SPEED_NONE) { @Override public String toString() { return "copper (0x80)"; } }, - OFPPF_FIBER (1 << 8) { + OFPPF_FIBER (1 << 8, PortSpeed.SPEED_NONE) { @Override public String toString() { return "fiber (0x100)"; } }, - OFPPF_AUTONEG (1 << 9) { + OFPPF_AUTONEG (1 << 9, PortSpeed.SPEED_NONE) { @Override public String toString() { return "autoneg (0x200)"; } }, - OFPPF_PAUSE (1 << 10) { + OFPPF_PAUSE (1 << 10, PortSpeed.SPEED_NONE) { @Override public String toString() { return "pause (0x400)"; } }, - OFPPF_PAUSE_ASYM (1 << 11) { + OFPPF_PAUSE_ASYM (1 << 11, PortSpeed.SPEED_NONE) { @Override public String toString() { return "pause-asym (0x800)"; @@ -230,18 +295,30 @@ public class OFPhysicalPort { }; protected int value; + protected PortSpeed speed; - private OFPortFeatures(int value) { + private OFPortFeatures(int value, PortSpeed speed) { this.value = value; + if (speed == null) + throw new NullPointerException(); + this.speed = speed; } /** - * @return the value + * @return the bitmap value for this constant */ @Override public int getValue() { return value; } + + /** + * @return the port speed associated with this constant. If the + * constant doesn't represent a port speed it returns SPEED_NONE + */ + public PortSpeed getSpeed() { + return speed; + } } protected short portNumber; diff --git a/src/test/java/net/floodlightcontroller/core/internal/ControllerTest.java b/src/test/java/net/floodlightcontroller/core/internal/ControllerTest.java index a8be0c5222d01b1f65e6990ee9633c1a6b3d86c3..5dbedbdd155c2eaaa878b22fe97c5239a3f86c0d 100644 --- a/src/test/java/net/floodlightcontroller/core/internal/ControllerTest.java +++ b/src/test/java/net/floodlightcontroller/core/internal/ControllerTest.java @@ -44,6 +44,7 @@ import net.floodlightcontroller.core.IOFSwitch.PortChangeType; import net.floodlightcontroller.core.IOFSwitchDriver; import net.floodlightcontroller.core.IOFSwitchListener; import net.floodlightcontroller.core.IReadyForReconcileListener; +import net.floodlightcontroller.core.ImmutablePort; import net.floodlightcontroller.core.OFMessageFilterManager; import net.floodlightcontroller.core.RoleInfo; import net.floodlightcontroller.core.SwitchSyncRepresentation; @@ -232,6 +233,8 @@ public class ControllerTest extends FloodlightTestCase { featuresReply = createOFFeaturesReply(); featuresReply.setDatapathId(dpid); } + List<ImmutablePort> ports = + ImmutablePort.immutablePortListOf(featuresReply.getPorts()); expect(sw.getId()).andReturn(dpid).anyTimes(); expect(sw.getStringId()).andReturn(dpidString).anyTimes(); @@ -245,7 +248,7 @@ public class ControllerTest extends FloodlightTestCase { expect(sw.getActions()) .andReturn(featuresReply.getActions()).atLeastOnce(); expect(sw.getPorts()) - .andReturn(featuresReply.getPorts()).atLeastOnce(); + .andReturn(ports).atLeastOnce(); } @SuppressWarnings("unchecked") @@ -1258,14 +1261,6 @@ public class ControllerTest extends FloodlightTestCase { } - private static OFPhysicalPort createOFPhysicalPort(String name, int number) { - OFPhysicalPort p = new OFPhysicalPort(); - p.setHardwareAddress(new byte [] { 0, 0, 0, 0, 0, 0 }); - p.setPortNumber((short)number); - p.setName(name); - return p; - } - /** * This test goes through the SLAVE->MASTER program flow. We'll start as * SLAVE. Add switches to the store while slave, update these switches @@ -1312,6 +1307,8 @@ public class ControllerTest extends FloodlightTestCase { OFPhysicalPort p = createOFPhysicalPort("P1", 1); List<OFPhysicalPort> ports1a = Collections.singletonList(p); fr1a.setPorts(ports1a); + List<ImmutablePort> ports1aImmutable = + ImmutablePort.immutablePortListOf(ports1a); // an alternative featuers reply OFFeaturesReply fr1b = createOFFeaturesReply(); fr1b.setDatapathId(1L); @@ -1322,6 +1319,8 @@ public class ControllerTest extends FloodlightTestCase { p = createOFPhysicalPort("P2", 42000); ports1b.add(p); fr1b.setPorts(ports1b); + List<ImmutablePort> ports1bImmutable = + ImmutablePort.immutablePortListOf(ports1b); // Switch 2 // no actual IOFSwitch here because we simply add features reply @@ -1330,6 +1329,8 @@ public class ControllerTest extends FloodlightTestCase { fr2a.setDatapathId(2L); List<OFPhysicalPort> ports2a = new ArrayList<OFPhysicalPort>(ports1a); fr2a.setPorts(ports2a); + List<ImmutablePort> ports2aImmutable = + ImmutablePort.immutablePortListOf(ports2a); // an alternative features reply OFFeaturesReply fr2b = createOFFeaturesReply(); fr2b.setDatapathId(2L); @@ -1363,8 +1364,8 @@ public class ControllerTest extends FloodlightTestCase { assertNotNull("Switch should be present", sw); assertEquals(1L, sw.getId()); assertFalse("Switch should be inactive", sw.isActive()); - assertEquals(new HashSet<OFPhysicalPort>(ports1a), - new HashSet<OFPhysicalPort>(sw.getPorts())); + assertEquals(new HashSet<ImmutablePort>(ports1aImmutable), + new HashSet<ImmutablePort>(sw.getPorts())); // add switch 2 with fr2a to store reset(switchListener); @@ -1380,8 +1381,8 @@ public class ControllerTest extends FloodlightTestCase { assertNotNull("Switch should be present", sw); assertEquals(2L, sw.getId()); assertFalse("Switch should be inactive", sw.isActive()); - assertEquals(new HashSet<OFPhysicalPort>(ports2a), - new HashSet<OFPhysicalPort>(sw.getPorts())); + assertEquals(new HashSet<ImmutablePort>(ports2aImmutable), + new HashSet<ImmutablePort>(sw.getPorts())); // add switch 3 to store reset(switchListener); @@ -1429,8 +1430,8 @@ public class ControllerTest extends FloodlightTestCase { assertNotNull("Switch should be present", sw); assertEquals(1L, sw.getId()); assertFalse("Switch should be inactive", sw.isActive()); - assertEquals(new HashSet<OFPhysicalPort>(ports1b), - new HashSet<OFPhysicalPort>(sw.getPorts())); + assertEquals(new HashSet<ImmutablePort>(ports1bImmutable), + new HashSet<ImmutablePort>(sw.getPorts())); // Check getAllSwitchDpids() and getAllSwitchMap() Set<Long> expectedDpids = new HashSet<Long>(); @@ -1952,7 +1953,7 @@ public class ControllerTest extends FloodlightTestCase { OFFeaturesReply fr2 = createOFFeaturesReply(); fr1.setDatapathId(dpid); OFPhysicalPort p2 = createOFPhysicalPort("Port1", 1); - p2.setAdvertisedFeatures(0xFFFFFFFF); // just some bogus values + p2.setAdvertisedFeatures(0x2); // just some bogus values fr2.setPorts(Collections.singletonList(p2)); OFDescriptionStatistics desc = createOFDescriptionStatistics(); @@ -1971,11 +1972,13 @@ public class ControllerTest extends FloodlightTestCase { controller.addOFSwitchListener(listener); // setup switch with the new, second features reply (and thus ports) setupSwitchForAddSwitch(sw, dpid, desc, fr2); - listener.switchPortChanged(dpid, p2, PortChangeType.OTHER_UPDATE); + listener.switchPortChanged(dpid, ImmutablePort.fromOFPhysicalPort(p2), + PortChangeType.OTHER_UPDATE); expectLastCall().once(); replay(listener); replay(sw); - controller.notifyPortChanged(sw, p2, PortChangeType.OTHER_UPDATE); + controller.notifyPortChanged(sw, ImmutablePort.fromOFPhysicalPort(p2), + PortChangeType.OTHER_UPDATE); controller.processUpdateQueueForTesting(); verify(listener); verify(sw); diff --git a/src/test/java/net/floodlightcontroller/core/internal/OFChannelHandlerTest.java b/src/test/java/net/floodlightcontroller/core/internal/OFChannelHandlerTest.java index 510c5ce108fabf2f99c58d17a13a7420f0d32462..380bc7325728cae5509c18cc0abea0bce7a5da97 100644 --- a/src/test/java/net/floodlightcontroller/core/internal/OFChannelHandlerTest.java +++ b/src/test/java/net/floodlightcontroller/core/internal/OFChannelHandlerTest.java @@ -11,7 +11,9 @@ import java.util.Set; import net.floodlightcontroller.core.FloodlightContext; import net.floodlightcontroller.core.IOFSwitch; import net.floodlightcontroller.core.IFloodlightProviderService.Role; +import net.floodlightcontroller.core.IOFSwitch.PortChangeEvent; import net.floodlightcontroller.core.IOFSwitch.PortChangeType; +import net.floodlightcontroller.core.ImmutablePort; import net.floodlightcontroller.debugcounter.DebugCounter; import net.floodlightcontroller.debugcounter.IDebugCounterService; import net.floodlightcontroller.storage.IResultSet; @@ -1174,62 +1176,49 @@ public class OFChannelHandlerTest { BasicFactory.getInstance().getMessage(OFType.PORT_STATUS); ps.setDesc(p); + // The events we expect sw.handlePortStatus to return + // We'll just use the same list for all valid OFPortReasons and add + // arbitrary events for arbitrary ports that are not necessarily + // related to the port status message. Our goal + // here is not to return the correct set of events but the make sure + // that a) sw.handlePortStatus is called + // b) the list of events sw.handlePortStatus returns is sent + // as IOFSwitchListener notifications. + List<PortChangeEvent> events = new ArrayList<PortChangeEvent>(); + ImmutablePort p1 = ImmutablePort.create("eth1", (short)1); + ImmutablePort p2 = ImmutablePort.create("eth2", (short)2); + ImmutablePort p3 = ImmutablePort.create("eth3", (short)3); + ImmutablePort p4 = ImmutablePort.create("eth4", (short)4); + ImmutablePort p5 = ImmutablePort.create("eth5", (short)5); + events.add(new PortChangeEvent(p1, PortChangeType.ADD)); + events.add(new PortChangeEvent(p2, PortChangeType.DELETE)); + events.add(new PortChangeEvent(p3, PortChangeType.UP)); + events.add(new PortChangeEvent(p4, PortChangeType.DOWN)); + events.add(new PortChangeEvent(p5, PortChangeType.OTHER_UPDATE)); + + + for (OFPortReason reason: OFPortReason.values()) { + ps.setReason(reason.getReasonCode()); + + reset(sw); + expect(sw.inputThrottled(anyObject(OFMessage.class))) + .andReturn(false).anyTimes(); + expect(sw.getId()).andReturn(dpid).anyTimes(); + + expect(sw.processOFPortStatus(ps)).andReturn(events).once(); + replay(sw); - // Add port - ps.setReason(OFPortReason.OFPPR_ADD.getReasonCode()); - reset(sw); - expect(sw.inputThrottled(anyObject(OFMessage.class))) - .andReturn(false).anyTimes(); - expect(sw.getId()).andReturn(dpid).anyTimes(); - sw.setPort(p); - expectLastCall().once(); - expect(sw.portEnabled((short)1)).andReturn(true).anyTimes(); - replay(sw); - - reset(controller); - controller.notifyPortChanged(sw, p, PortChangeType.UP); - expectLastCall().once(); - sendMessageToHandlerNoControllerReset( - Collections.<OFMessage>singletonList(ps)); - verify(sw); - verify(controller); - - // modify port - ps.setReason(OFPortReason.OFPPR_MODIFY.getReasonCode()); - reset(sw); - expect(sw.inputThrottled(anyObject(OFMessage.class))) - .andReturn(false).anyTimes(); - expect(sw.getId()).andReturn(dpid).anyTimes(); - sw.setPort(p); - expectLastCall().once(); - expect(sw.portEnabled((short)1)).andReturn(true).anyTimes(); - replay(sw); - - reset(controller); - controller.notifyPortChanged(sw, p, PortChangeType.OTHER_UPDATE); - expectLastCall().once(); - sendMessageToHandlerNoControllerReset( - Collections.<OFMessage>singletonList(ps)); - verify(sw); - verify(controller); - - // delete port - ps.setReason(OFPortReason.OFPPR_DELETE.getReasonCode()); - reset(sw); - expect(sw.inputThrottled(anyObject(OFMessage.class))) - .andReturn(false).anyTimes(); - expect(sw.getId()).andReturn(dpid).anyTimes(); - sw.deletePort(p.getPortNumber()); - expectLastCall().once(); - replay(sw); - - reset(controller); - controller.notifyPortChanged(sw, p, PortChangeType.DELETE); - expectLastCall().once(); - sendMessageToHandlerNoControllerReset( - Collections.<OFMessage>singletonList(ps)); - verify(sw); - verify(controller); + reset(controller); + controller.notifyPortChanged(sw, p1, PortChangeType.ADD); + controller.notifyPortChanged(sw, p2, PortChangeType.DELETE); + controller.notifyPortChanged(sw, p3, PortChangeType.UP); + controller.notifyPortChanged(sw, p4, PortChangeType.DOWN); + controller.notifyPortChanged(sw, p5, PortChangeType.OTHER_UPDATE); + sendMessageToHandlerNoControllerReset( + Collections.<OFMessage>singletonList(ps)); + verify(sw); + verify(controller); + } } /** diff --git a/src/test/java/net/floodlightcontroller/devicemanager/internal/DeviceManagerImplTest.java b/src/test/java/net/floodlightcontroller/devicemanager/internal/DeviceManagerImplTest.java index dc23400ac6f7dc5f18719185b52319cd3ea555cb..e9443b810c2ab8bd94ee97a9f2e01a0c2a83ceff 100644 --- a/src/test/java/net/floodlightcontroller/devicemanager/internal/DeviceManagerImplTest.java +++ b/src/test/java/net/floodlightcontroller/devicemanager/internal/DeviceManagerImplTest.java @@ -52,6 +52,7 @@ import java.util.concurrent.ConcurrentHashMap; import net.floodlightcontroller.core.IFloodlightProviderService; import net.floodlightcontroller.core.IOFSwitch; import net.floodlightcontroller.core.IFloodlightProviderService.Role; +import net.floodlightcontroller.core.ImmutablePort; import net.floodlightcontroller.core.module.FloodlightModuleContext; import net.floodlightcontroller.core.module.FloodlightModuleException; import net.floodlightcontroller.core.test.MockThreadPoolService; @@ -87,7 +88,6 @@ import org.junit.Before; import org.junit.Test; import org.openflow.protocol.OFPacketIn; import org.openflow.protocol.OFPacketIn.OFPacketInReason; -import org.openflow.protocol.OFPhysicalPort; import org.openflow.protocol.OFPort; import org.openflow.protocol.OFType; import org.openflow.util.HexString; @@ -118,13 +118,12 @@ public class DeviceManagerImplTest extends FloodlightTestCase { private IOFSwitch makeSwitchMock(long id) { IOFSwitch mockSwitch = createMock(IOFSwitch.class); + ImmutablePort port = ImmutablePort.create("p1", (short)1); expect(mockSwitch.getId()).andReturn(id).anyTimes(); - expect(mockSwitch.getStringId()). - andReturn(HexString.toHexString(id, 6)).anyTimes(); - expect(mockSwitch.getPort(anyShort())). - andReturn(new OFPhysicalPort()).anyTimes(); - expect(mockSwitch.portEnabled(isA(OFPhysicalPort.class))). - andReturn(true).anyTimes(); + expect(mockSwitch.getStringId()) + .andReturn(HexString.toHexString(id, 6)).anyTimes(); + expect(mockSwitch.getPort(anyShort())) + .andReturn(port).anyTimes(); return mockSwitch; } diff --git a/src/test/java/net/floodlightcontroller/linkdiscovery/internal/LinkDiscoveryManagerTest.java b/src/test/java/net/floodlightcontroller/linkdiscovery/internal/LinkDiscoveryManagerTest.java index 6eecf116f8bd3eeafea6d58e80aeea9407b3b88e..0dfa0d29a0d731f2c4c9235d9d4950379b65224c 100644 --- a/src/test/java/net/floodlightcontroller/linkdiscovery/internal/LinkDiscoveryManagerTest.java +++ b/src/test/java/net/floodlightcontroller/linkdiscovery/internal/LinkDiscoveryManagerTest.java @@ -36,6 +36,7 @@ import net.floodlightcontroller.core.FloodlightContext; import net.floodlightcontroller.core.IFloodlightProviderService; import net.floodlightcontroller.core.IListener.Command; import net.floodlightcontroller.core.IOFSwitch; +import net.floodlightcontroller.core.ImmutablePort; import net.floodlightcontroller.core.module.FloodlightModuleContext; import net.floodlightcontroller.core.test.MockThreadPoolService; import net.floodlightcontroller.linkdiscovery.ILinkDiscoveryListener; @@ -428,9 +429,12 @@ public class LinkDiscoveryManagerTest extends FloodlightTestCase { Capture<OFMessage> wc; Capture<FloodlightContext> fc; Set<Short> qPorts; - OFPhysicalPort p1 = new OFPhysicalPort(); - p1.setHardwareAddress(HexString.fromHexString("5c:16:c7:00:00:01")); - p1.setCurrentFeatures(0); + OFPhysicalPort ofpp = new OFPhysicalPort(); + ofpp.setName("eth4242"); + ofpp.setPortNumber((short)4242); + ofpp.setHardwareAddress(HexString.fromHexString("5c:16:c7:00:00:01")); + ofpp.setCurrentFeatures(0); + ImmutablePort p1 = ImmutablePort.fromOFPhysicalPort(ofpp); IOFSwitch sw1 = createMockSwitch(1L); // Set switch map in floodlightProvider. diff --git a/src/test/java/net/floodlightcontroller/test/FloodlightTestCase.java b/src/test/java/net/floodlightcontroller/test/FloodlightTestCase.java index b0e83cc115fbc7293be6ec70dae7a78dbd325a3e..94496dc2266131b7499085087b29d58eaf785cfa 100644 --- a/src/test/java/net/floodlightcontroller/test/FloodlightTestCase.java +++ b/src/test/java/net/floodlightcontroller/test/FloodlightTestCase.java @@ -1,7 +1,7 @@ /** -* Copyright 2011, Big Switch Networks, Inc. +* Copyright 2011, Big Switch Networks, Inc. * Originally created by David Erickson, Stanford University -* +* * Licensed under the Apache License, Version 2.0 (the "License"); you may * not use this file except in compliance with the License. You may obtain * a copy of the License at @@ -28,6 +28,7 @@ import net.floodlightcontroller.packet.Ethernet; import org.junit.Test; import org.openflow.protocol.OFMessage; import org.openflow.protocol.OFPacketIn; +import org.openflow.protocol.OFPhysicalPort; import org.openflow.protocol.OFType; /** @@ -66,30 +67,38 @@ public class FloodlightTestCase extends TestCase { OFPacketIn pi = (OFPacketIn)m; Ethernet eth = new Ethernet(); eth.deserialize(pi.getPacketData(), 0, pi.getPacketData().length); - IFloodlightProviderService.bcStore.put(bc, - IFloodlightProviderService.CONTEXT_PI_PAYLOAD, + IFloodlightProviderService.bcStore.put(bc, + IFloodlightProviderService.CONTEXT_PI_PAYLOAD, eth); } if (srcDevice != null) { - IDeviceService.fcStore.put(bc, - IDeviceService.CONTEXT_SRC_DEVICE, + IDeviceService.fcStore.put(bc, + IDeviceService.CONTEXT_SRC_DEVICE, srcDevice); } if (dstDevice != null) { - IDeviceService.fcStore.put(bc, - IDeviceService.CONTEXT_DST_DEVICE, + IDeviceService.fcStore.put(bc, + IDeviceService.CONTEXT_DST_DEVICE, dstDevice); } return bc; } - + @Override public void setUp() throws Exception { mockFloodlightProvider = new MockFloodlightProvider(); } - + @Test public void testSanity() throws Exception { assertTrue(true); } + + public static OFPhysicalPort createOFPhysicalPort(String name, int number) { + OFPhysicalPort p = new OFPhysicalPort(); + p.setHardwareAddress(new byte [] { 0, 0, 0, 0, 0, 0 }); + p.setPortNumber((short)number); + p.setName(name); + return p; + } } diff --git a/src/test/java/net/floodlightcontroller/util/OFMessageDamperMockSwitch.java b/src/test/java/net/floodlightcontroller/util/OFMessageDamperMockSwitch.java index c32c4ba338808cf765e0eaef5933ce9bbe72953b..5e5c0955ed85e89e8fbc43a74861d00e56b6076a 100644 --- a/src/test/java/net/floodlightcontroller/util/OFMessageDamperMockSwitch.java +++ b/src/test/java/net/floodlightcontroller/util/OFMessageDamperMockSwitch.java @@ -25,12 +25,11 @@ import java.util.Date; import java.util.List; import java.util.Map; import java.util.concurrent.Future; -import java.util.concurrent.locks.Lock; - import net.floodlightcontroller.core.FloodlightContext; import net.floodlightcontroller.core.IOFMessageListener; import net.floodlightcontroller.core.IOFSwitch; import net.floodlightcontroller.core.IFloodlightProviderService.Role; +import net.floodlightcontroller.core.ImmutablePort; import net.floodlightcontroller.core.internal.Controller; import net.floodlightcontroller.debugcounter.IDebugCounterService; import net.floodlightcontroller.threadpool.IThreadPoolService; @@ -38,7 +37,7 @@ import net.floodlightcontroller.threadpool.IThreadPoolService; import org.jboss.netty.channel.Channel; import org.openflow.protocol.OFFeaturesReply; import org.openflow.protocol.OFMessage; -import org.openflow.protocol.OFPhysicalPort; +import org.openflow.protocol.OFPortStatus; import org.openflow.protocol.OFStatisticsRequest; import org.openflow.protocol.statistics.OFDescriptionStatistics; import org.openflow.protocol.statistics.OFStatistics; @@ -142,7 +141,7 @@ public class OFMessageDamperMockSwitch implements IOFSwitch { } @Override - public Collection<OFPhysicalPort> getEnabledPorts() { + public Collection<ImmutablePort> getEnabledPorts() { assertTrue("Unexpected method call", false); return null; } @@ -154,34 +153,19 @@ public class OFMessageDamperMockSwitch implements IOFSwitch { } @Override - public OFPhysicalPort getPort(short portNumber) { + public ImmutablePort getPort(short portNumber) { assertTrue("Unexpected method call", false); return null; } @Override - public OFPhysicalPort getPort(String portName) { + public ImmutablePort getPort(String portName) { assertTrue("Unexpected method call", false); return null; } @Override - public void setPort(OFPhysicalPort port) { - assertTrue("Unexpected method call", false); - } - - @Override - public void deletePort(short portNumber) { - assertTrue("Unexpected method call", false); - } - - @Override - public void deletePort(String portName) { - assertTrue("Unexpected method call", false); - } - - @Override - public Collection<OFPhysicalPort> getPorts() { + public Collection<ImmutablePort> getPorts() { assertTrue("Unexpected method call", false); return null; } @@ -198,12 +182,6 @@ public class OFMessageDamperMockSwitch implements IOFSwitch { return false; } - @Override - public boolean portEnabled(OFPhysicalPort port) { - assertTrue("Unexpected method call", false); - return false; - } - @Override public long getId() { assertTrue("Unexpected method call", false); @@ -334,97 +312,81 @@ public class OFMessageDamperMockSwitch implements IOFSwitch { @Override public Future<OFFeaturesReply> querySwitchFeaturesReply() throws IOException { - // TODO Auto-generated method stub + fail("Unexpected method call"); return null; } @Override public void deliverOFFeaturesReply(OFMessage reply) { - // TODO Auto-generated method stub - + fail("Unexpected method call"); } @Override public void cancelFeaturesReply(int transactionId) { - // TODO Auto-generated method stub - + fail("Unexpected method call"); } @Override public int getBuffers() { - // TODO Auto-generated method stub + fail("Unexpected method call"); return 0; } @Override public int getActions() { - // TODO Auto-generated method stub + fail("Unexpected method call"); return 0; } @Override public int getCapabilities() { - // TODO Auto-generated method stub + fail("Unexpected method call"); return 0; } @Override public byte getTables() { - // TODO Auto-generated method stub + fail("Unexpected method call"); return 0; } @Override public void setChannel(Channel channel) { - // TODO Auto-generated method stub - + fail("Unexpected method call"); } @Override public void setFloodlightProvider(Controller controller) { + fail("Unexpected method call"); // TODO Auto-generated method stub } @Override public void setThreadPoolService(IThreadPoolService threadPool) { - // TODO Auto-generated method stub - - } - - @Override - public Lock getListenerReadLock() { - // TODO Auto-generated method stub - return null; - } - - @Override - public Lock getListenerWriteLock() { - // TODO Auto-generated method stub - return null; + fail("Unexpected method call"); } @Override public void setHARole(Role role) { - // TODO Auto-generated method stub - + fail("Unexpected method call"); } @Override public OFPortType getPortType(short port_num) { - // TODO Auto-generated method stub + fail("Unexpected method call"); return null; } @Override public boolean isFastPort(short port_num) { - // TODO Auto-generated method stub + fail("Unexpected method call"); return false; } @Override public List<Short> getUplinkPorts() { - // TODO Auto-generated method stub + fail("Unexpected method call"); return null; } @@ -448,25 +410,43 @@ public class OFMessageDamperMockSwitch implements IOFSwitch { @Override public boolean inputThrottled(OFMessage ofm) { - // TODO Auto-generated method stub + fail("Unexpected method call"); return false; } @Override public boolean isOverloaded() { - // TODO Auto-generated method stub + fail("Unexpected method call"); return false; } @Override public boolean isWriteThrottleEnabled() { - // TODO Auto-generated method stub + fail("Unexpected method call"); return false; } @Override public void setDebugCounterService(IDebugCounterService debugCounters) { - // TODO Auto-generated method stub + fail("Unexpected method call"); + } + + @Override + public List<PortChangeEvent> processOFPortStatus(OFPortStatus ps) { + fail("Unexpected method call"); + return null; + } + + @Override + public List<PortChangeEvent> + comparePorts(Collection<ImmutablePort> ports) { + fail("Unexpected method call"); + return null; } + @Override + public List<PortChangeEvent> setPorts(Collection<ImmutablePort> ports) { + fail("Unexpected method call"); + return null; + } }