From: Pramila Singh Date: Thu, 25 Apr 2013 21:57:42 +0000 (-0700) Subject: Logging and Code clean-up for protocol_plugins.core X-Git-Tag: releasepom-0.1.0~527^2 X-Git-Url: https://git.opendaylight.org/gerrit/gitweb?p=controller.git;a=commitdiff_plain;h=d4526d295217d6d6d60434e179a2a78c1b2eb52b Logging and Code clean-up for protocol_plugins.core Change-Id: Ia34dd373ba16d7a70be0440d7f076e7dab02ffdd Signed-off-by: Pramila Singh --- diff --git a/opendaylight/protocol_plugins/openflow/src/main/java/org/opendaylight/controller/protocol_plugin/openflow/core/internal/Controller.java b/opendaylight/protocol_plugins/openflow/src/main/java/org/opendaylight/controller/protocol_plugin/openflow/core/internal/Controller.java index 9ec5b10ea8..8e6a30fcee 100644 --- a/opendaylight/protocol_plugins/openflow/src/main/java/org/opendaylight/controller/protocol_plugin/openflow/core/internal/Controller.java +++ b/opendaylight/protocol_plugins/openflow/src/main/java/org/opendaylight/controller/protocol_plugin/openflow/core/internal/Controller.java @@ -1,4 +1,3 @@ - /* * Copyright (c) 2013 Cisco Systems, Inc. and others. All rights reserved. * @@ -53,7 +52,8 @@ public class Controller implements IController, CommandProvider { private AtomicInteger switchInstanceNumber; /* - * this thread monitors the switchEvents queue for new incoming events from switch + * this thread monitors the switchEvents queue for new incoming events from + * switch */ private class EventHandler implements Runnable { @Override @@ -64,17 +64,13 @@ public class Controller implements IController, CommandProvider { SwitchEvent ev = switchEvents.take(); SwitchEvent.SwitchEventType eType = ev.getEventType(); ISwitch sw = ev.getSwitch(); - if (eType != SwitchEvent.SwitchEventType.SWITCH_MESSAGE) { - //logger.debug("Received " + ev.toString() + " from " + sw.toString()); - } switch (eType) { case SWITCH_ADD: Long sid = sw.getId(); ISwitch existingSwitch = switches.get(sid); if (existingSwitch != null) { - logger.info(" Replacing existing " - + existingSwitch.toString() + " with New " - + sw.toString()); + logger.info("Replacing existing {} with New {}", + existingSwitch.toString(), sw.toString()); disconnectSwitch(existingSwitch); } switches.put(sid, sw); @@ -97,7 +93,7 @@ public class Controller implements IController, CommandProvider { } break; default: - logger.error("unknow switch event " + eType.ordinal()); + logger.error("Unknown switch event {}", eType.ordinal()); } } catch (InterruptedException e) { switchEvents.clear(); @@ -111,10 +107,10 @@ public class Controller implements IController, CommandProvider { /** * Function called by the dependency manager when all the required * dependencies are satisfied - * + * */ public void init() { - logger.debug("OpenFlowCore init"); + logger.debug("Initializing!"); this.switches = new ConcurrentHashMap(); this.switchEvents = new LinkedBlockingQueue(); this.messageListeners = new ConcurrentHashMap(); @@ -124,13 +120,12 @@ public class Controller implements IController, CommandProvider { } /** - * Function called by dependency manager after "init ()" is called - * and after the services provided by the class are registered in - * the service registry - * + * Function called by dependency manager after "init ()" is called and after + * the services provided by the class are registered in the service registry + * */ public void start() { - logger.debug("OpenFlowCore start() is called"); + logger.debug("Starting!"); /* * start a thread to handle event coming from the switch */ @@ -142,15 +137,15 @@ public class Controller implements IController, CommandProvider { try { controllerIO.start(); } catch (IOException ex) { - logger.error("Caught exception: " + ex + " during start"); + logger.error("Caught exception while starting:", ex); } } - + /** - * Function called by the dependency manager before the services - * exported by the component are unregistered, this will be - * followed by a "destroy ()" calls - * + * Function called by the dependency manager before the services exported by + * the component are unregistered, this will be followed by a "destroy ()" + * calls + * */ public void stop() { for (Iterator> it = switches.entrySet().iterator(); it @@ -161,17 +156,17 @@ public class Controller implements IController, CommandProvider { } switchEventThread.interrupt(); try { - controllerIO.shutDown(); + controllerIO.shutDown(); } catch (IOException ex) { - logger.error("Caught exception: " + ex + " during stop"); + logger.error("Caught exception while stopping:", ex); } } /** - * Function called by the dependency manager when at least one - * dependency become unsatisfied or when the component is shutting - * down because for example bundle is being stopped. - * + * Function called by the dependency manager when at least one dependency + * become unsatisfied or when the component is shutting down because for + * example bundle is being stopped. + * */ public void destroy() { } @@ -180,18 +175,20 @@ public class Controller implements IController, CommandProvider { public void addMessageListener(OFType type, IMessageListener listener) { IMessageListener currentListener = this.messageListeners.get(type); if (currentListener != null) { - logger.warn(type.toString() + " already listened by " - + currentListener.toString()); + logger.warn("{} is already listened by {}", type.toString(), + currentListener.toString()); } this.messageListeners.put(type, listener); - logger.debug(type.toString() + " is now listened by " - + listener.toString()); + logger.debug("{} is now listened by {}", type.toString(), + listener.toString()); } @Override public void removeMessageListener(OFType type, IMessageListener listener) { IMessageListener currentListener = this.messageListeners.get(type); if ((currentListener != null) && (currentListener == listener)) { + logger.debug("{} listener {} is Removed", type.toString(), + listener.toString()); this.messageListeners.remove(type); } } @@ -199,17 +196,20 @@ public class Controller implements IController, CommandProvider { @Override public void addSwitchStateListener(ISwitchStateListener listener) { if (this.switchStateListener != null) { - logger.warn(this.switchStateListener.toString() - + "already listened to switch events"); + logger.warn("Switch events are already listened by {}", + this.switchStateListener.toString()); } this.switchStateListener = listener; - logger.debug(listener.toString() + " now listens to switch events"); + logger.debug("Switch events are now listened by {}", + listener.toString()); } @Override public void removeSwitchStateListener(ISwitchStateListener listener) { if ((this.switchStateListener != null) && (this.switchStateListener == listener)) { + logger.debug("SwitchStateListener {} is Removed", + listener.toString()); this.switchStateListener = null; } } @@ -227,7 +227,11 @@ public class Controller implements IController, CommandProvider { SwitchHandler switchHandler = new SwitchHandler(this, sc, instanceName); switchHandler.start(); - logger.info(instanceName + " connected: " + sc.toString()); + if (sc.isConnected()) { + logger.info("Switch:{} is connected to the Controller", sc + .getRemoteAddress().toString().split("/")[1]); + } + } catch (IOException e) { return; } @@ -237,11 +241,8 @@ public class Controller implements IController, CommandProvider { if (((SwitchHandler) sw).isOperational()) { Long sid = sw.getId(); if (this.switches.remove(sid, sw)) { - logger.warn(sw.toString() + " is disconnected"); + logger.warn("{} is Disconnected", sw.toString()); notifySwitchDeleted(sw); - } else { - //logger.warn(sw.toString() + " has been replaced by " + - // this.switches.get(sid)); } } ((SwitchHandler) sw).stop(); @@ -268,7 +269,7 @@ public class Controller implements IController, CommandProvider { } } - public void takeSwtichEventAdd(ISwitch sw) { + public void takeSwitchEventAdd(ISwitch sw) { SwitchEvent ev = new SwitchEvent( SwitchEvent.SwitchEventType.SWITCH_ADD, sw, null); addSwitchEvent(ev); @@ -317,7 +318,8 @@ public class Controller implements IController, CommandProvider { while (iter.hasNext()) { Long sid = iter.next(); Date date = switches.get(sid).getConnectedDate(); - String switchInstanceName = ((SwitchHandler) switches.get(sid)).getInstanceName(); + String switchInstanceName = ((SwitchHandler) switches.get(sid)) + .getInstanceName(); s.append(switchInstanceName + "/" + HexString.toHexString(sid) + " connected since " + date.toString() + "\n"); } @@ -369,7 +371,7 @@ public class Controller implements IController, CommandProvider { @Override public String getHelp() { StringBuffer help = new StringBuffer(); - help.append("-- Open Flow Controller --\n"); + help.append("---Open Flow Controller---\n"); help.append("\t controllerShowSwitches\n"); help.append("\t controllerReset\n"); help.append("\t controllerShowConnConfig\n"); diff --git a/opendaylight/protocol_plugins/openflow/src/main/java/org/opendaylight/controller/protocol_plugin/openflow/core/internal/ControllerIO.java b/opendaylight/protocol_plugins/openflow/src/main/java/org/opendaylight/controller/protocol_plugin/openflow/core/internal/ControllerIO.java index a4b7584337..868e208651 100644 --- a/opendaylight/protocol_plugins/openflow/src/main/java/org/opendaylight/controller/protocol_plugin/openflow/core/internal/ControllerIO.java +++ b/opendaylight/protocol_plugins/openflow/src/main/java/org/opendaylight/controller/protocol_plugin/openflow/core/internal/ControllerIO.java @@ -1,4 +1,3 @@ - /* * Copyright (c) 2013 Cisco Systems, Inc. and others. All rights reserved. * @@ -40,8 +39,8 @@ public class ControllerIO { try { openFlowPort = Short.decode(portString).shortValue(); } catch (NumberFormatException e) { - logger.warn("Invalid port:" + portString + ", use default(" - + openFlowPort + ")"); + logger.warn("Invalid port:{}, use default({})", portString, + openFlowPort); } } } @@ -72,8 +71,8 @@ public class ControllerIO { SelectionKey skey = selectedKeys.next(); selectedKeys.remove(); if (skey.isValid() && skey.isAcceptable()) { - ((Controller) listener).handleNewConnection(selector, - serverSelectionKey); + ((Controller) listener).handleNewConnection( + selector, serverSelectionKey); } } } catch (Exception e) { @@ -83,7 +82,7 @@ public class ControllerIO { } }, "ControllerI/O Thread"); controllerIOThread.start(); - logger.info("Controller is now listening on port " + openFlowPort); + logger.info("Controller is now listening on port {}", openFlowPort); } public void shutDown() throws IOException { diff --git a/opendaylight/protocol_plugins/openflow/src/main/java/org/opendaylight/controller/protocol_plugin/openflow/core/internal/SwitchHandler.java b/opendaylight/protocol_plugins/openflow/src/main/java/org/opendaylight/controller/protocol_plugin/openflow/core/internal/SwitchHandler.java index bbf3e91462..ea659c82e1 100644 --- a/opendaylight/protocol_plugins/openflow/src/main/java/org/opendaylight/controller/protocol_plugin/openflow/core/internal/SwitchHandler.java +++ b/opendaylight/protocol_plugins/openflow/src/main/java/org/opendaylight/controller/protocol_plugin/openflow/core/internal/SwitchHandler.java @@ -356,11 +356,6 @@ public class SwitchHandler implements ISwitch { } for (OFMessage msg : msgs) { logger.trace("Message received: {}", msg.toString()); - /* - * if ((msg.getType() != OFType.ECHO_REQUEST) && (msg.getType() != - * OFType.ECHO_REPLY)) { logger.debug(msg.getType().toString() + - * " received from sw " + toString()); } - */ this.lastMsgReceivedTimeStamp = System.currentTimeMillis(); OFType type = msg.getType(); switch (type) { @@ -422,21 +417,14 @@ public class SwitchHandler implements ISwitch { } private void processPortStatusMsg(OFPortStatus msg) { - // short portNumber = msg.getDesc().getPortNumber(); OFPhysicalPort port = msg.getDesc(); if (msg.getReason() == (byte) OFPortReason.OFPPR_MODIFY.ordinal()) { updatePhysicalPort(port); - // logger.debug("Port " + portNumber + " on " + toString() + - // " modified"); } else if (msg.getReason() == (byte) OFPortReason.OFPPR_ADD.ordinal()) { updatePhysicalPort(port); - // logger.debug("Port " + portNumber + " on " + toString() + - // " added"); } else if (msg.getReason() == (byte) OFPortReason.OFPPR_DELETE .ordinal()) { deletePhysicalPort(port); - // logger.debug("Port " + portNumber + " on " + toString() + - // " deleted"); } } @@ -457,8 +445,9 @@ public class SwitchHandler implements ISwitch { reportSwitchStateChange(false); } else { // send a probe to see if the switch is still alive - // logger.debug("Send idle probe (Echo Request) to " - // + switchName()); + logger.debug( + "Send idle probe (Echo Request) to {}", + toString()); probeSent = true; OFMessage echo = factory .getMessage(OFType.ECHO_REQUEST); @@ -500,8 +489,7 @@ public class SwitchHandler implements ISwitch { private void reportError(Exception e) { if (e instanceof AsynchronousCloseException || e instanceof InterruptedException - || e instanceof SocketException - || e instanceof IOException) { + || e instanceof SocketException || e instanceof IOException) { logger.debug("Caught exception {}", e.getMessage()); } else { logger.warn("Caught exception ", e); @@ -512,7 +500,7 @@ public class SwitchHandler implements ISwitch { private void reportSwitchStateChange(boolean added) { if (added) { - ((Controller) core).takeSwtichEventAdd(this); + ((Controller) core).takeSwitchEventAdd(this); } else { ((Controller) core).takeSwitchEventDelete(this); } @@ -567,7 +555,7 @@ public class SwitchHandler implements ISwitch { .getValue() | OFPortFeatures.OFPPF_1GB_HD .getValue() | OFPortFeatures.OFPPF_10GB_FD - .getValue())); + .getValue())); } private void deletePhysicalPort(OFPhysicalPort port) { @@ -583,11 +571,16 @@ public class SwitchHandler implements ISwitch { @Override public String toString() { - return ("[" - + this.socket.toString() - + " SWID " - + (isOperational() ? HexString.toHexString(this.sid) - : "unkbown") + "]"); + try { + return ("Switch:" + + socket.getRemoteAddress().toString().split("/")[1] + + " SWID:" + (isOperational() ? HexString + .toHexString(this.sid) : "unknown")); + } catch (Exception e) { + return (isOperational() ? HexString.toHexString(this.sid) + : "unknown"); + } + } @Override @@ -823,14 +816,14 @@ public class SwitchHandler implements ISwitch { } /** - * Sends synchronous Barrier message + * Sends synchronous Barrier message */ @Override public Object sendBarrierMessage() { OFBarrierRequest barrierMsg = new OFBarrierRequest(); - return syncSend(barrierMsg); + return syncSend(barrierMsg); } - + /** * This method returns the switch liveness timeout value. If controller did * not receive any message from the switch for such a long period,