diff --git a/src/main/java/net/floodlightcontroller/core/internal/Controller.java b/src/main/java/net/floodlightcontroller/core/internal/Controller.java index 6b183ca95d2dcd960e70f4d32b6b12bf812e7916..532ffb64744150ec6fbdcdfbb88590892e475e43 100644 --- a/src/main/java/net/floodlightcontroller/core/internal/Controller.java +++ b/src/main/java/net/floodlightcontroller/core/internal/Controller.java @@ -757,9 +757,10 @@ public class Controller implements IFloodlightProviderService, // OFError message. // If role is MASTER we will promote switch to active // list when we receive the switch's role reply messages - log.debug("This controller's role is {}, " + - "sending initial role request msg to {}", - role, sw); + if (log.isDebugEnabled()) + log.debug("This controller's role is {}, " + + "sending initial role request msg to {}", + role, sw); Collection<IOFSwitch> swList = new ArrayList<IOFSwitch>(1); swList.add(sw); roleChanger.submitRequest(swList, role); diff --git a/src/main/java/net/floodlightcontroller/core/module/FloodlightModuleLoader.java b/src/main/java/net/floodlightcontroller/core/module/FloodlightModuleLoader.java index 45fe9970bdb6f2207e8567384057b43715c46ff5..0da2b06f3141a832ccb15e7cd1daa50b72e321a3 100644 --- a/src/main/java/net/floodlightcontroller/core/module/FloodlightModuleLoader.java +++ b/src/main/java/net/floodlightcontroller/core/module/FloodlightModuleLoader.java @@ -44,129 +44,129 @@ public class FloodlightModuleLoader { protected static Object lock = new Object(); protected FloodlightModuleContext floodlightModuleContext; - + public static final String COMPILED_CONF_FILE = "floodlightdefault.properties"; public static final String FLOODLIGHT_MODULES_KEY = "floodlight.modules"; - public FloodlightModuleLoader() { - floodlightModuleContext = new FloodlightModuleContext(); - } - - /** - * Finds all IFloodlightModule(s) in the classpath. It creates 3 Maps. - * serviceMap -> Maps a service to a module - * moduleServiceMap -> Maps a module to all the services it provides - * moduleNameMap -> Maps the string name to the module - * @throws FloodlightModuleException If two modules are specified in the configuration - * that provide the same service. - */ - protected static void findAllModules(Collection<String> mList) throws FloodlightModuleException { - synchronized (lock) { - if (serviceMap != null) return; - serviceMap = - new HashMap<Class<? extends IFloodlightService>, - Collection<IFloodlightModule>>(); - moduleServiceMap = - new HashMap<IFloodlightModule, - Collection<Class<? extends - IFloodlightService>>>(); - moduleNameMap = new HashMap<String, IFloodlightModule>(); - - // Get all the current modules in the classpath - ClassLoader cl = Thread.currentThread().getContextClassLoader(); - ServiceLoader<IFloodlightModule> moduleLoader - = ServiceLoader.load(IFloodlightModule.class, cl); - // Iterate for each module, iterate through and add it's services - Iterator<IFloodlightModule> moduleIter = moduleLoader.iterator(); - while (moduleIter.hasNext()) { - IFloodlightModule m = null; - try { - m = moduleIter.next(); - } catch (ServiceConfigurationError sce) { - logger.debug("Could not find module"); - //moduleIter.remove(); - continue; - } - //} - //for (IFloodlightModule m : moduleLoader) { - if (logger.isDebugEnabled()) { - logger.debug("Found module " + m.getClass().getName()); - } + public FloodlightModuleLoader() { + floodlightModuleContext = new FloodlightModuleContext(); + } + + /** + * Finds all IFloodlightModule(s) in the classpath. It creates 3 Maps. + * serviceMap -> Maps a service to a module + * moduleServiceMap -> Maps a module to all the services it provides + * moduleNameMap -> Maps the string name to the module + * @throws FloodlightModuleException If two modules are specified in the configuration + * that provide the same service. + */ + protected static void findAllModules(Collection<String> mList) throws FloodlightModuleException { + synchronized (lock) { + if (serviceMap != null) return; + serviceMap = + new HashMap<Class<? extends IFloodlightService>, + Collection<IFloodlightModule>>(); + moduleServiceMap = + new HashMap<IFloodlightModule, + Collection<Class<? extends + IFloodlightService>>>(); + moduleNameMap = new HashMap<String, IFloodlightModule>(); + + // Get all the current modules in the classpath + ClassLoader cl = Thread.currentThread().getContextClassLoader(); + ServiceLoader<IFloodlightModule> moduleLoader + = ServiceLoader.load(IFloodlightModule.class, cl); + // Iterate for each module, iterate through and add it's services + Iterator<IFloodlightModule> moduleIter = moduleLoader.iterator(); + while (moduleIter.hasNext()) { + IFloodlightModule m = null; + try { + m = moduleIter.next(); + } catch (ServiceConfigurationError sce) { + logger.debug("Could not find module"); + //moduleIter.remove(); + continue; + } + //} + //for (IFloodlightModule m : moduleLoader) { + if (logger.isDebugEnabled()) { + logger.debug("Found module " + m.getClass().getName()); + } - // Set up moduleNameMap - moduleNameMap.put(m.getClass().getCanonicalName(), m); + // Set up moduleNameMap + moduleNameMap.put(m.getClass().getCanonicalName(), m); - // Set up serviceMap - Collection<Class<? extends IFloodlightService>> servs = - m.getModuleServices(); - if (servs != null) { - moduleServiceMap.put(m, servs); - for (Class<? extends IFloodlightService> s : servs) { - Collection<IFloodlightModule> mods = - serviceMap.get(s); - if (mods == null) { - mods = new ArrayList<IFloodlightModule>(); - serviceMap.put(s, mods); - } - mods.add(m); - // Make sure they haven't specified duplicate modules in the config - int dupInConf = 0; - for (IFloodlightModule cMod : mods) { - if (mList.contains(cMod.getClass().getCanonicalName())) - dupInConf += 1; - } - - if (dupInConf > 1) { - String duplicateMods = ""; + // Set up serviceMap + Collection<Class<? extends IFloodlightService>> servs = + m.getModuleServices(); + if (servs != null) { + moduleServiceMap.put(m, servs); + for (Class<? extends IFloodlightService> s : servs) { + Collection<IFloodlightModule> mods = + serviceMap.get(s); + if (mods == null) { + mods = new ArrayList<IFloodlightModule>(); + serviceMap.put(s, mods); + } + mods.add(m); + // Make sure they haven't specified duplicate modules in the config + int dupInConf = 0; + for (IFloodlightModule cMod : mods) { + if (mList.contains(cMod.getClass().getCanonicalName())) + dupInConf += 1; + } + + if (dupInConf > 1) { + String duplicateMods = ""; for (IFloodlightModule mod : mods) { duplicateMods += mod.getClass().getCanonicalName() + ", "; } - throw new FloodlightModuleException("ERROR! The configuraiton" + - " file specifies more than one module that provides the service " + - s.getCanonicalName() +". Please specify only ONE of the " + - "following modules in the config file: " + duplicateMods); - } - } - } - } - } - } - - /** - * Loads the modules from a specified configuration file. - * @param fName The configuration file path - * @return An IFloodlightModuleContext with all the modules to be started - * @throws FloodlightModuleException - */ - @LogMessageDocs({ - @LogMessageDoc(level="INFO", - message="Loading modules from file {file name}", - explanation="The controller is initializing its module " + - "configuration from the specified properties file"), - @LogMessageDoc(level="INFO", - message="Loading default modules", - explanation="The controller is initializing its module " + - "configuration to the default configuration"), - @LogMessageDoc(level="ERROR", - message="Could not load module configuration file", - explanation="The controller failed to read the " + - "module configuration file", - recommendation="Verify that the module configuration is " + - "present. " + LogMessageDoc.CHECK_CONTROLLER), - @LogMessageDoc(level="ERROR", + throw new FloodlightModuleException("ERROR! The configuraiton" + + " file specifies more than one module that provides the service " + + s.getCanonicalName() +". Please specify only ONE of the " + + "following modules in the config file: " + duplicateMods); + } + } + } + } + } + } + + /** + * Loads the modules from a specified configuration file. + * @param fName The configuration file path + * @return An IFloodlightModuleContext with all the modules to be started + * @throws FloodlightModuleException + */ + @LogMessageDocs({ + @LogMessageDoc(level="INFO", + message="Loading modules from file {file name}", + explanation="The controller is initializing its module " + + "configuration from the specified properties file"), + @LogMessageDoc(level="INFO", + message="Loading default modules", + explanation="The controller is initializing its module " + + "configuration to the default configuration"), + @LogMessageDoc(level="ERROR", + message="Could not load module configuration file", + explanation="The controller failed to read the " + + "module configuration file", + recommendation="Verify that the module configuration is " + + "present. " + LogMessageDoc.CHECK_CONTROLLER), + @LogMessageDoc(level="ERROR", message="Could not load default modules", explanation="The controller failed to read the default " + "module configuration", recommendation=LogMessageDoc.CHECK_CONTROLLER) - }) - public IFloodlightModuleContext loadModulesFromConfig(String fName) - throws FloodlightModuleException { - Properties prop = new Properties(); - - File f = new File(fName); - if (f.isFile()) { + }) + public IFloodlightModuleContext loadModulesFromConfig(String fName) + throws FloodlightModuleException { + Properties prop = new Properties(); + + File f = new File(fName); + if (f.isFile()) { logger.info("Loading modules from file {}", fName); try { prop.load(new FileInputStream(fName)); @@ -191,21 +191,21 @@ public class FloodlightModuleLoader { Collection<String> configMods = new ArrayList<String>(); configMods.addAll(Arrays.asList(moduleList.split(","))); return loadModulesFromList(configMods, prop); - } - - /** - * Loads modules (and their dependencies) specified in the list - * @param mList The array of fully qualified module names - * @param ignoreList The list of Floodlight services NOT to - * load modules for. Used for unit testing. - * @return The ModuleContext containing all the loaded modules - * @throws FloodlightModuleException - */ - protected IFloodlightModuleContext loadModulesFromList(Collection<String> configMods, Properties prop, - Collection<IFloodlightService> ignoreList) throws FloodlightModuleException { - logger.debug("Starting module loader"); - if (logger.isDebugEnabled() && ignoreList != null) - logger.debug("Not loading module services " + ignoreList.toString()); + } + + /** + * Loads modules (and their dependencies) specified in the list + * @param mList The array of fully qualified module names + * @param ignoreList The list of Floodlight services NOT to + * load modules for. Used for unit testing. + * @return The ModuleContext containing all the loaded modules + * @throws FloodlightModuleException + */ + protected IFloodlightModuleContext loadModulesFromList(Collection<String> configMods, Properties prop, + Collection<IFloodlightService> ignoreList) throws FloodlightModuleException { + logger.debug("Starting module loader"); + if (logger.isDebugEnabled() && ignoreList != null) + logger.debug("Not loading module services " + ignoreList.toString()); findAllModules(configMods); @@ -232,25 +232,25 @@ public class FloodlightModuleLoader { // If the module provies a service that is in the // services ignorelist don't load it. if ((ignoreList != null) && (module.getModuleServices() != null)) { - for (IFloodlightService ifs : ignoreList) { - for (Class<?> intsIgnore : ifs.getClass().getInterfaces()) { - //System.out.println(intsIgnore.getName()); - // Check that the interface extends IFloodlightService - //if (intsIgnore.isAssignableFrom(IFloodlightService.class)) { - //System.out.println(module.getClass().getName()); - if (intsIgnore.isAssignableFrom(module.getClass())) { - // We now ignore loading this module. - logger.debug("Not loading module " + - module.getClass().getCanonicalName() + - " because interface " + - intsIgnore.getCanonicalName() + - " is in the ignore list."); - - continue; - } - //} - } - } + for (IFloodlightService ifs : ignoreList) { + for (Class<?> intsIgnore : ifs.getClass().getInterfaces()) { + //System.out.println(intsIgnore.getName()); + // Check that the interface extends IFloodlightService + //if (intsIgnore.isAssignableFrom(IFloodlightService.class)) { + //System.out.println(module.getClass().getName()); + if (intsIgnore.isAssignableFrom(module.getClass())) { + // We now ignore loading this module. + logger.debug("Not loading module " + + module.getClass().getCanonicalName() + + " because interface " + + intsIgnore.getCanonicalName() + + " is in the ignore list."); + + continue; + } + //} + } + } } // Add the module to be loaded @@ -304,27 +304,27 @@ public class FloodlightModuleLoader { startupModules(moduleSet); return floodlightModuleContext; - } - - /** - * Loads modules (and their dependencies) specified in the list. - * @param configMods The collection of fully qualified module names to load. - * @param prop The list of properties that are configuration options. - * @return The ModuleContext containing all the loaded modules. - * @throws FloodlightModuleException - */ - public IFloodlightModuleContext loadModulesFromList(Collection<String> configMods, Properties prop) + } + + /** + * Loads modules (and their dependencies) specified in the list. + * @param configMods The collection of fully qualified module names to load. + * @param prop The list of properties that are configuration options. + * @return The ModuleContext containing all the loaded modules. + * @throws FloodlightModuleException + */ + public IFloodlightModuleContext loadModulesFromList(Collection<String> configMods, Properties prop) throws FloodlightModuleException { - return loadModulesFromList(configMods, prop, null); + return loadModulesFromList(configMods, prop, null); } - - /** - * Add a module to the set of modules to load and register its services - * @param moduleMap the module map - * @param moduleSet the module set - * @param module the module to add - */ - protected void addModule(Map<Class<? extends IFloodlightService>, + + /** + * Add a module to the set of modules to load and register its services + * @param moduleMap the module map + * @param moduleSet the module set + * @param module the module to add + */ + protected void addModule(Map<Class<? extends IFloodlightService>, IFloodlightModule> moduleMap, Collection<IFloodlightModule> moduleSet, IFloodlightModule module) { @@ -337,7 +337,7 @@ public class FloodlightModuleLoader { } moduleSet.add(module); } - } + } /** * Allocate service implementations and then init all the modules @@ -407,10 +407,10 @@ public class FloodlightModuleLoader { message="Module {module} not found or loaded. " + "Not adding configuration option {key} = {value}", explanation="Ignoring a configuration parameter for a " + - "module that is not loaded.") + "module that is not loaded.") protected void parseConfigParameters(Properties prop) { - if (prop == null) return; - + if (prop == null) return; + Enumeration<?> e = prop.propertyNames(); while (e.hasMoreElements()) { String key = (String) e.nextElement(); @@ -434,7 +434,7 @@ public class FloodlightModuleLoader { IFloodlightModule mod = moduleNameMap.get(moduleName); if (mod == null) { logger.warn("Module {} not found or loaded. " + - "Not adding configuration option {} = {}", + "Not adding configuration option {} = {}", new Object[]{moduleName, configKey, configValue}); } else { floodlightModuleContext.addConfigParam(mod, configKey, configValue); diff --git a/src/main/java/net/floodlightcontroller/linkdiscovery/internal/LinkDiscoveryManager.java b/src/main/java/net/floodlightcontroller/linkdiscovery/internal/LinkDiscoveryManager.java index 59040ac72af670a7c2cce4dffb5f8dc5d1ad0b64..1d11ac6c3eaba013295154d8bccfb6ef76710799 100644 --- a/src/main/java/net/floodlightcontroller/linkdiscovery/internal/LinkDiscoveryManager.java +++ b/src/main/java/net/floodlightcontroller/linkdiscovery/internal/LinkDiscoveryManager.java @@ -146,6 +146,7 @@ public class LinkDiscoveryManager implements IOFMessageListener, private static final byte[] LLDP_STANDARD_DST_MAC_STRING = HexString.fromHexString("01:80:c2:00:00:0e"); private static final long LINK_LOCAL_MASK = 0xfffffffffff0L; private static final long LINK_LOCAL_VALUE = 0x0180c2000000L; + protected static int EVENT_HISTORY_SIZE = 1024; // in seconds // BigSwitch OUI is 5C:16:C7, so 5D:16:C7 is the multicast version // private static final String LLDP_BSN_DST_MAC_STRING = @@ -790,12 +791,18 @@ public class LinkDiscoveryManager implements IOFMessageListener, return Command.STOP; } - if (isLinkDiscoverySuppressed(sw, pi.getInPort())) - return Command.STOP; + if (isLinkDiscoverySuppressed(sw, pi.getInPort())) { + if (log.isTraceEnabled()) { + log.trace("Got a LLDP=[{}] from a supressed switchport sw = {}, port = {}. Not fowarding it.", + new Object[] {lldp.toString(), HexString.toHexString(sw), pi.getInPort()}); + } + return Command.STOP; + } // If this is a malformed LLDP, or not from us, exit - if (lldp.getPortId() == null || lldp.getPortId().getLength() != 3) - return Command.CONTINUE; + if (lldp.getPortId() == null || lldp.getPortId().getLength() != 3) { + return Command.CONTINUE; + } long myId = ByteBuffer.wrap(controllerTLV.getValue()).getLong(); long otherId = 0; @@ -837,7 +844,7 @@ public class LinkDiscoveryManager implements IOFMessageListener, // the packet as a regular packet. if (isStandard) { if (log.isTraceEnabled()) { - log.trace("Getting standard LLDP from a different controller and quelching it."); + log.trace("Got a standard LLDP=[{}]. Not fowarding it.", lldp.toString()); } return Command.STOP; } else if (myId < otherId) { @@ -1910,6 +1917,18 @@ public class LinkDiscoveryManager implements IOFMessageListener, threadPool = context.getServiceImpl(IThreadPoolService.class); restApi = context.getServiceImpl(IRestApiService.class); + // read our config options + Map<String, String> configOptions = context.getConfigParams(this); + try { + String histSize = configOptions.get("eventhistorysize"); + if (histSize != null) { + EVENT_HISTORY_SIZE = Short.parseShort(histSize); + } + } catch (NumberFormatException e) { + log.warn("Error event history size, using default of {} seconds", EVENT_HISTORY_SIZE); + } + log.debug("Event history size set to {}", EVENT_HISTORY_SIZE); + // Set the autoportfast feature to false. this.autoPortFastFeature = false; @@ -1925,12 +1944,9 @@ public class LinkDiscoveryManager implements IOFMessageListener, this.quarantineQueue = new LinkedBlockingQueue<NodePortTuple>(); this.maintenanceQueue = new LinkedBlockingQueue<NodePortTuple>(); - this.evHistTopologySwitch = new EventHistory<EventHistoryTopologySwitch>( - "Topology: Switch"); - this.evHistTopologyLink = new EventHistory<EventHistoryTopologyLink>( - "Topology: Link"); - this.evHistTopologyCluster = new EventHistory<EventHistoryTopologyCluster>( - "Topology: Cluster"); + this.evHistTopologySwitch = new EventHistory<EventHistoryTopologySwitch>(EVENT_HISTORY_SIZE); + this.evHistTopologyLink = new EventHistory<EventHistoryTopologyLink>(EVENT_HISTORY_SIZE); + this.evHistTopologyCluster = new EventHistory<EventHistoryTopologyCluster>(EVENT_HISTORY_SIZE); } @Override diff --git a/src/main/java/net/floodlightcontroller/packet/LLDP.java b/src/main/java/net/floodlightcontroller/packet/LLDP.java index 8c2c457ca16bd83c45719a1a03af4f95d5c7329f..5a1104104749d514977ce392fd2be327737ef89b 100644 --- a/src/main/java/net/floodlightcontroller/packet/LLDP.java +++ b/src/main/java/net/floodlightcontroller/packet/LLDP.java @@ -201,4 +201,19 @@ public class LLDP extends BasePacket { return false; return true; } + + @Override + public String toString() { + String str = ""; + str += "chassisId=" + ((this.chassisId == null) ? "null" : this.chassisId.toString()); + str += " portId=" + ((this.portId == null) ? "null" : this.portId.toString()); + str += " ttl=" + ((this.ttl == null) ? "null" : this.ttl.toString()); + str += " etherType=" + Integer.toString(this.ethType, 16).toUpperCase(); + str += " optionalTlvList=["; + if (this.optionalTLVList != null) { + for (LLDPTLV l : optionalTLVList) str += l.toString() + ", "; + } + str += "]"; + return str; + } } diff --git a/src/main/java/net/floodlightcontroller/packet/LLDPTLV.java b/src/main/java/net/floodlightcontroller/packet/LLDPTLV.java index 055232165d2c2590b6234dab5901b9bad3c4886b..9fa020be22b5c04913403aea887aa641369a2c7b 100644 --- a/src/main/java/net/floodlightcontroller/packet/LLDPTLV.java +++ b/src/main/java/net/floodlightcontroller/packet/LLDPTLV.java @@ -137,4 +137,14 @@ public class LLDPTLV { return false; return true; } + + @Override + public String toString() { + String str = "type=" + Integer.toString(this.type, 16).toUpperCase() + + " length=" + this.length + + " value="; + for (byte b : this.value) + str+= Integer.toString(b, 16).toUpperCase(); + return str; + } } diff --git a/src/main/java/net/floodlightcontroller/util/EventHistory.java b/src/main/java/net/floodlightcontroller/util/EventHistory.java index 69031ba9effe1121d3303373af94fd829a2c26ed..2ca1b798a365648ed6b2b8c5e4fc126146ae5eba 100644 --- a/src/main/java/net/floodlightcontroller/util/EventHistory.java +++ b/src/main/java/net/floodlightcontroller/util/EventHistory.java @@ -13,15 +13,11 @@ import java.util.ArrayList; public class EventHistory<T> { public static final int EV_HISTORY_DEFAULT_SIZE = 1024; - public String description; public int event_history_size; public int current_index; public boolean full; // true if all are in use public ArrayList<Event> events; - public String getDescription() { - return description; - } public int getEvent_history_size() { return event_history_size; } @@ -65,7 +61,7 @@ public class EventHistory<T> { } // Constructor - public EventHistory(int maxEvents, String desc) { + public EventHistory(int maxEvents) { events = new ArrayList<Event>(maxEvents); for (int idx = 0; idx < maxEvents; idx++) { @@ -76,23 +72,20 @@ public class EventHistory<T> { evH.base_info.idx = idx; events.add(idx, evH); } - - description = "Event-History:" + desc; + event_history_size = maxEvents; current_index = 0; full = false; } // Constructor for default size - public EventHistory(String desc) { - this(EV_HISTORY_DEFAULT_SIZE, desc); + public EventHistory() { + this(EV_HISTORY_DEFAULT_SIZE); } // Copy constructor - copy latest k items of the event history public EventHistory(EventHistory<T> eventHist, int latestK) { - if (eventHist == null) { - description = "No event found"; return; } int curSize = (eventHist.full)?eventHist.event_history_size: @@ -119,8 +112,7 @@ public class EventHistory<T> { evH.base_info.idx = idx; events.add(idx, evH); } - - description = eventHist.description; + event_history_size = size; current_index = 0; // since it is full full = true; diff --git a/src/main/java/org/openflow/protocol/OFPacketOut.java b/src/main/java/org/openflow/protocol/OFPacketOut.java index d00b9bd7f29307a093e393db5e8cf91d903b1ad2..ef4aa61f1b0e86a245bc6dd5ba9aa55c0ba2557d 100644 --- a/src/main/java/org/openflow/protocol/OFPacketOut.java +++ b/src/main/java/org/openflow/protocol/OFPacketOut.java @@ -1,7 +1,7 @@ /** * Copyright (c) 2008 The Board of Trustees of The Leland Stanford Junior * 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 @@ -47,6 +47,7 @@ public class OFPacketOut extends OFMessage implements OFActionFactoryAware { super(); this.type = OFType.PACKET_OUT; this.length = U16.t(MINIMUM_LENGTH); + this.bufferId = BUFFER_ID_NONE; } /** @@ -62,6 +63,10 @@ public class OFPacketOut extends OFMessage implements OFActionFactoryAware { * @param bufferId */ public OFPacketOut setBufferId(int bufferId) { + if (packetData != null && packetData.length > 0 && bufferId != BUFFER_ID_NONE) { + throw new IllegalArgumentException( + "PacketOut should not have both bufferId and packetData set"); + } this.bufferId = bufferId; return this; } @@ -79,6 +84,10 @@ public class OFPacketOut extends OFMessage implements OFActionFactoryAware { * @param packetData */ public OFPacketOut setPacketData(byte[] packetData) { + if (packetData != null && packetData.length > 0 && bufferId != BUFFER_ID_NONE) { + throw new IllegalArgumentException( + "PacketOut should not have both bufferId and packetData set"); + } this.packetData = packetData; return this; } @@ -167,10 +176,12 @@ public class OFPacketOut extends OFMessage implements OFActionFactoryAware { this.actions = this.actionFactory.parseActions(data, getActionsLengthU()); this.packetData = new byte[getLengthU() - MINIMUM_LENGTH - getActionsLengthU()]; data.readBytes(this.packetData); + validate(); } @Override public void writeTo(ChannelBuffer data) { + validate(); super.writeTo(data); data.writeInt(bufferId); data.writeShort(inPort); @@ -182,6 +193,14 @@ public class OFPacketOut extends OFMessage implements OFActionFactoryAware { data.writeBytes(this.packetData); } + /** validate the invariants of this OFMessage hold */ + public void validate() { + if (!((bufferId != BUFFER_ID_NONE) ^ (packetData != null && packetData.length > 0))) { + throw new IllegalStateException( + "OFPacketOut must have exactly one of (bufferId, packetData) set (not one, not both)"); + } + } + @Override public int hashCode() { final int prime = 293; diff --git a/src/test/java/org/openflow/protocol/OFPacketOutTest.java b/src/test/java/org/openflow/protocol/OFPacketOutTest.java new file mode 100644 index 0000000000000000000000000000000000000000..55b54552fe03b279f20f076dd8857994dded0e09 --- /dev/null +++ b/src/test/java/org/openflow/protocol/OFPacketOutTest.java @@ -0,0 +1,45 @@ +package org.openflow.protocol; + +import org.junit.Test; + +public class OFPacketOutTest { + + @Test(expected = IllegalArgumentException.class) + public void testBothBufferIdAndPayloadSet() { + OFPacketOut packetOut = new OFPacketOut(); + packetOut.setBufferId(12); + packetOut.setPacketData(new byte[] { 1, 2, 3 }); + } + + @Test + public void testOnlyBufferIdSet() { + OFPacketOut packetOut = new OFPacketOut(); + packetOut.setBufferId(12); + packetOut.setPacketData(null); + packetOut.setPacketData(new byte[] {}); + packetOut.validate(); + } + + @Test(expected = IllegalStateException.class) + public void testNeitherBufferIdNorPayloadSet() { + OFPacketOut packetOut = new OFPacketOut(); + packetOut.setBufferId(OFPacketOut.BUFFER_ID_NONE); + packetOut.setPacketData(null); + packetOut.validate(); + } + + @Test(expected = IllegalStateException.class) + public void testNeitherBufferIdNorPayloadSet2() { + OFPacketOut packetOut = new OFPacketOut(); + packetOut.setBufferId(OFPacketOut.BUFFER_ID_NONE); + packetOut.setPacketData(new byte[] {}); + packetOut.validate(); + } + + @Test(expected = IllegalStateException.class) + public void testNeitherBufferIdNorPayloadSet3() { + OFPacketOut packetOut = new OFPacketOut(); + packetOut.validate(); + } + +}