diff --git a/src/main/java/net/floodlightcontroller/core/internal/Controller.java b/src/main/java/net/floodlightcontroller/core/internal/Controller.java index 6852adfbc8302d03681ba147fdf281f303c4d67e..77674efe0a7a318a81361ace47ad9037c3dc0184 100644 --- a/src/main/java/net/floodlightcontroller/core/internal/Controller.java +++ b/src/main/java/net/floodlightcontroller/core/internal/Controller.java @@ -67,6 +67,7 @@ import net.floodlightcontroller.storage.IStorageSourceService; import net.floodlightcontroller.storage.OperatorPredicate; import net.floodlightcontroller.storage.StorageException; import net.floodlightcontroller.threadpool.IThreadPoolService; +import net.floodlightcontroller.util.StackTraceUtil; import org.jboss.netty.bootstrap.ServerBootstrap; import org.jboss.netty.buffer.ChannelBuffer; @@ -1597,11 +1598,11 @@ public class Controller return; } catch (StorageException e) { log.error("Storage exception in controller " + - "updates loop; terminating process", - e); + "updates loop; terminating process: {} {}", + e, StackTraceUtil.stackTraceToString(e)); return; } catch (Exception e) { - log.error("Exception in controller updates loop", e); + log.error("Exception in controller updates loop {} {}", e, StackTraceUtil.stackTraceToString(e)); } } } diff --git a/src/main/java/net/floodlightcontroller/linkdiscovery/LinkInfo.java b/src/main/java/net/floodlightcontroller/linkdiscovery/LinkInfo.java index 6421328f45355927bcc4606856b1d2d7e5608370..7c8d7491c9f3a391d869b027ecb4c32e4b79f952 100644 --- a/src/main/java/net/floodlightcontroller/linkdiscovery/LinkInfo.java +++ b/src/main/java/net/floodlightcontroller/linkdiscovery/LinkInfo.java @@ -153,7 +153,7 @@ public class LinkInfo { @Override public String toString() { return "LinkInfo [unicastValidTime=" + ((unicastValidTime == null) ? "null" : unicastValidTime) - + "multicastValidTime=" + ((multicastValidTime == null) ? "null" : multicastValidTime) + + ", multicastValidTime=" + ((multicastValidTime == null) ? "null" : multicastValidTime) + ", srcPortState=" + ((srcPortState == null) ? "null" : srcPortState) + ", dstPortState=" + ((dstPortState == null) ? "null" : srcPortState) + "]"; diff --git a/src/main/java/net/floodlightcontroller/linkdiscovery/internal/LinkDiscoveryManager.java b/src/main/java/net/floodlightcontroller/linkdiscovery/internal/LinkDiscoveryManager.java index abd8d91565b97053067c76e7410760e9424ba72f..b3fb351dff049a98087faebb60926a4943c0f7b5 100644 --- a/src/main/java/net/floodlightcontroller/linkdiscovery/internal/LinkDiscoveryManager.java +++ b/src/main/java/net/floodlightcontroller/linkdiscovery/internal/LinkDiscoveryManager.java @@ -73,10 +73,10 @@ import net.floodlightcontroller.storage.IStorageSourceListener; import net.floodlightcontroller.storage.OperatorPredicate; import net.floodlightcontroller.storage.StorageException; import net.floodlightcontroller.threadpool.IThreadPoolService; -import net.floodlightcontroller.topology.ITopologyListener; import net.floodlightcontroller.topology.web.TopologyWebRoutable; import net.floodlightcontroller.util.EventHistory; import net.floodlightcontroller.util.EventHistory.EvAction; +import net.floodlightcontroller.util.StackTraceUtil; import org.openflow.protocol.OFMessage; import org.openflow.protocol.OFPacketIn; @@ -181,7 +181,6 @@ public class LinkDiscoveryManager /* topology aware components are called in the order they were added to the * the array */ protected ArrayList<ILinkDiscoveryListener> linkDiscoveryAware; - protected ArrayList<ITopologyListener> topologyAware; protected BlockingQueue<LDUpdate> updates; protected Thread updatesThread; @@ -210,15 +209,21 @@ public class LinkDiscoveryManager do { LDUpdate update = updates.take(); - if (topologyAware != null) { - for (ILinkDiscoveryListener lda : linkDiscoveryAware) { // order maintained - if (log.isDebugEnabled()) { - log.debug("Dispatching link discovery update {} {} {} {} {}", - new Object[]{update.getOperation(), - update.getSrc(), update.getSrcPort(), - update.getDst(), update.getDstPort()}); + if (log.isTraceEnabled()) { + log.trace("Dispatching link discovery update {} {} {} {} {} for {}", + new Object[]{update.getOperation(), + HexString.toHexString(update.getSrc()), update.getSrcPort(), + HexString.toHexString(update.getDst()), update.getDstPort(), + linkDiscoveryAware}); + } + if (linkDiscoveryAware != null) { + try { + for (ILinkDiscoveryListener lda : linkDiscoveryAware) { // order maintained + lda.linkDiscoveryUpdate(update); } - lda.linkDiscoveryUpdate(update); + } + catch (Exception e) { + log.error("Error in link discovery updates loop: {} {}", e, StackTraceUtil.stackTraceToString(e)); } } } while (updates.peek() != null); @@ -287,8 +292,10 @@ public class LinkDiscoveryManager // set the portId to the outgoing port portBB.putShort(port.getPortNumber()); - log.trace("Sending LLDP out of interface: {}/{}", - sw.toString(), port.toString()); + if (log.isTraceEnabled()) { + log.trace("Sending LLDP out of interface: {}/{}", + sw.toString(), port.getPortNumber()); + } // serialize and wrap in a packet out byte[] data = ethernet.serialize(); @@ -329,6 +336,7 @@ public class LinkDiscoveryManager } protected void sendLLDPs(IOFSwitch sw) { + if (sw.getEnabledPorts() != null) { for (OFPhysicalPort port : sw.getEnabledPorts()) { sendLLDPs(sw, port, true); @@ -339,7 +347,9 @@ public class LinkDiscoveryManager } protected void sendLLDPs() { - log.trace("Sending LLDP packets out of all the enabled ports"); + if (log.isTraceEnabled()) { + log.trace("Sending LLDP packets out of all the enabled ports on switch {}"); + } Map<Long, IOFSwitch> switches = floodlightProvider.getSwitches(); for (Entry<Long, IOFSwitch> entry : switches.entrySet()) { @@ -531,6 +541,11 @@ public class LinkDiscoveryManager lock.writeLock().lock(); try { LinkInfo oldLinkInfo = links.put(lt, newLinkInfo); + if (log.isTraceEnabled()) { + log.trace("addOrUpdateLink: {} {}", + lt, + (newLinkInfo.getMulticastValidTime()!=null) ? "multicast" : "unicast"); + } UpdateOperation updateOperation = null; boolean linkChanged = false; @@ -565,7 +580,10 @@ public class LinkDiscoveryManager updateOperation = UpdateOperation.ADD_OR_UPDATE; linkChanged = true; - log.debug("Added link {}", lt); + if (log.isDebugEnabled()) { + log.debug("Added link {}", lt); + } + // Add to event history evHistTopoLink(lt.getSrc().getSw().getId(), lt.getDst().getSw().getId(), @@ -618,7 +636,9 @@ public class LinkDiscoveryManager if (linkChanged) { updateOperation = UpdateOperation.ADD_OR_UPDATE; - log.debug("Updated link {}", lt); + if (log.isDebugEnabled()) { + log.debug("Updated link {}", lt); + } // Add to event history evHistTopoLink(lt.getSrc().getSw().getId(), lt.getDst().getSw().getId(), @@ -681,8 +701,8 @@ public class LinkDiscoveryManager removeLinkFromStorage(lt); - if (log.isTraceEnabled()) { - log.trace("Deleted link {}", lt); + if (log.isDebugEnabled()) { + log.debug("Deleted link {}", lt); } } } finally { @@ -726,10 +746,11 @@ public class LinkDiscoveryManager if (this.portLinks.containsKey(tuple)) { if (log.isDebugEnabled()) { log.debug("handlePortStatus: Switch {} port #{} " + - "reason {}; removing links", + "reason {}; removing links {}", new Object[] {HexString.toHexString(sw.getId()), ps.getDesc().getPortNumber(), - ps.getReason()}); + ps.getReason(), + this.portLinks.get(tuple)}); } eraseList.addAll(this.portLinks.get(tuple)); deleteLinks(eraseList, "Port Status Changed"); @@ -819,6 +840,10 @@ public class LinkDiscoveryManager lock.writeLock().lock(); try { if (switchLinks.containsKey(sw)) { + if (log.isDebugEnabled()) { + log.debug("Handle switchRemoved. Switch {}; removing links {}", + sw, switchLinks.get(sw)); + } // add all tuples with an endpoint on this switch to erase list eraseList.addAll(switchLinks.get(sw)); deleteLinks(eraseList, "Switch Removed"); @@ -930,9 +955,11 @@ public class LinkDiscoveryManager if (retLink != null) { linkInfo = this.links.get(retLink); } else { - log.debug("getLinkInfo: No link out of {} links is from port {}, "+ - "isSrcPort {}", - new Object[] {links.size(), idPort, isSrcPort}); + if (log.isDebugEnabled()) { + log.debug("getLinkInfo: No link out of {} links is from port {}, "+ + "isSrcPort {}", + new Object[] {links.size(), idPort, isSrcPort}); + } } return linkInfo; @@ -1138,11 +1165,6 @@ public class LinkDiscoveryManager linkDiscoveryAware.add(listener); } -// @Override - public void addListener(ITopologyListener listener) { - topologyAware.add(listener); - } - /** * Register a link discovery aware component * @param linkDiscoveryAwareComponent @@ -1161,25 +1183,6 @@ public class LinkDiscoveryManager this.linkDiscoveryAware.remove(linkDiscoveryAwareComponent); } - - /** - * Register a topology aware component - * @param topoAwareComponent - */ - public void addTopologyAware(ITopologyListener topoAwareComponent) { - // TODO make this a copy on write set or lock it somehow - this.topologyAware.add(topoAwareComponent); - } - - /** - * Deregister a topology aware component - * @param topoAwareComponent - */ - public void removeTopologyAware(ITopologyListener topoAwareComponent) { - // TODO make this a copy on write set or lock it somehow - this.topologyAware.remove(topoAwareComponent); - } - /** * Sets the IStorageSource to use for ITology * @param storageSource the storage source to use @@ -1244,8 +1247,10 @@ public class LinkDiscoveryManager updated_switches.add(sw); } } else { - log.debug("Update for switch which has no entry in switch " + - "list (dpid={}), a delete action.", (String)key); + if (log.isDebugEnabled()) { + log.debug("Update for switch which has no entry in switch " + + "list (dpid={}), a delete action.", (String)key); + } } } @@ -1253,12 +1258,16 @@ public class LinkDiscoveryManager // Set SWITCH_IS_CORE_SWITCH to it's inverse value if (sw.hasAttribute(IOFSwitch.SWITCH_IS_CORE_SWITCH)) { sw.removeAttribute(IOFSwitch.SWITCH_IS_CORE_SWITCH); - log.debug("SWITCH_IS_CORE_SWITCH set to False for {}", sw); + if (log.isDebugEnabled()) { + log.debug("SWITCH_IS_CORE_SWITCH set to False for {}", sw); + } updates.add(new LDUpdate(sw.getId(), SwitchType.BASIC_SWITCH)); } else { sw.setAttribute(IOFSwitch.SWITCH_IS_CORE_SWITCH, new Boolean(true)); - log.debug("SWITCH_IS_CORE_SWITCH set to True for {}", sw); + if (log.isDebugEnabled()) { + log.debug("SWITCH_IS_CORE_SWITCH set to True for {}", sw); + } updates.add(new LDUpdate(sw.getId(), SwitchType.CORE_SWITCH)); } } @@ -1316,7 +1325,6 @@ public class LinkDiscoveryManager threadPool = context.getServiceImpl(IThreadPoolService.class); // We create this here because there is no ordering guarantee - this.topologyAware = new ArrayList<ITopologyListener>(); this.linkDiscoveryAware = new ArrayList<ILinkDiscoveryListener>(); this.lock = new ReentrantReadWriteLock(); this.updates = new LinkedBlockingQueue<LDUpdate>(); @@ -1375,7 +1383,9 @@ public class LinkDiscoveryManager Runnable timeoutLinksTimer = new Runnable() { @Override public void run() { - log.trace("Running timeoutLinksTimer"); + if (log.isTraceEnabled()) { + log.trace("Running timeoutLinksTimer"); + } try { timeoutLinks(); if (!shuttingDown) { diff --git a/src/main/java/net/floodlightcontroller/topology/TopologyManager.java b/src/main/java/net/floodlightcontroller/topology/TopologyManager.java index 4cd19effdb828a60ed806eedbb75cb210aab5db4..f01ab44a708cff5a1b26641e41fcdb469368c381 100644 --- a/src/main/java/net/floodlightcontroller/topology/TopologyManager.java +++ b/src/main/java/net/floodlightcontroller/topology/TopologyManager.java @@ -23,6 +23,7 @@ import net.floodlightcontroller.routing.IRoutingService; import net.floodlightcontroller.routing.Link; import net.floodlightcontroller.routing.Route; import net.floodlightcontroller.threadpool.IThreadPoolService; +import net.floodlightcontroller.util.StackTraceUtil; import org.openflow.protocol.OFPhysicalPort.OFPortState; @@ -59,9 +60,15 @@ IRoutingService, ILinkDiscoveryListener { protected class NewInstanceWorker implements Runnable { @Override public void run() { - applyUpdates(); - createNewInstance(); - informListeners(); + try { + applyUpdates(); + createNewInstance(); + informListeners(); + } + catch (Exception e) { + log.error("Error in topology instance task thread: {} {}", + e, StackTraceUtil.stackTraceToString(e)); + } } } @@ -71,7 +78,7 @@ IRoutingService, ILinkDiscoveryListener { try { update = ldUpdates.take(); } catch (Exception e) { - log.error("Error reading link discovery update. {}", e); + log.error("Error reading link discovery update. {} {}", e, StackTraceUtil.stackTraceToString(e)); } if (log.isTraceEnabled()) { log.info("Applying update: {}", update); diff --git a/src/main/java/net/floodlightcontroller/util/StackTraceUtil.java b/src/main/java/net/floodlightcontroller/util/StackTraceUtil.java new file mode 100644 index 0000000000000000000000000000000000000000..e55f79e55a33a6b18edba7e5ed460ff4bc4c32eb --- /dev/null +++ b/src/main/java/net/floodlightcontroller/util/StackTraceUtil.java @@ -0,0 +1,19 @@ +package net.floodlightcontroller.util; + +import java.io.PrintWriter; +import java.io.StringWriter; + +/** + * Utility class to get a stacktrace as a nice string + * + * @author gregor + * + */ + +public class StackTraceUtil { + public static String stackTraceToString(Throwable t) { + StringWriter sw = new StringWriter(); + t.printStackTrace(new PrintWriter(sw)); + return sw.toString(); + } +} diff --git a/src/test/java/net/floodlightcontroller/linkdiscovery/internal/LinkDiscoveryManagerTest.java b/src/test/java/net/floodlightcontroller/linkdiscovery/internal/LinkDiscoveryManagerTest.java index c46889e4896e90d9d32af111f8d4e3b05be4c4b5..c6521b93ae79aefc5d695483312033b8d2dda46d 100644 --- a/src/test/java/net/floodlightcontroller/linkdiscovery/internal/LinkDiscoveryManagerTest.java +++ b/src/test/java/net/floodlightcontroller/linkdiscovery/internal/LinkDiscoveryManagerTest.java @@ -42,7 +42,6 @@ import net.floodlightcontroller.storage.IStorageSourceService; import net.floodlightcontroller.storage.memory.MemoryStorageSource; import net.floodlightcontroller.test.FloodlightTestCase; import net.floodlightcontroller.threadpool.IThreadPoolService; -import net.floodlightcontroller.topology.ITopologyListener; import net.floodlightcontroller.topology.ITopologyService; import net.floodlightcontroller.topology.TopologyManager; @@ -71,7 +70,6 @@ public class LinkDiscoveryManagerTest extends FloodlightTestCase { FloodlightModuleContext cntx = new FloodlightModuleContext(); topology = new LinkDiscoveryManager(); TopologyManager routingEngine = new TopologyManager(); - topology.topologyAware = new ArrayList<ITopologyListener>(); topology.linkDiscoveryAware = new ArrayList<ILinkDiscoveryListener>(); MockThreadPoolService tp = new MockThreadPoolService(); cntx.addService(IThreadPoolService.class, tp);